feat: rename LG_TOKEN to LG_RESERVATION

doc/man/client.rst

@@ -25,8 +25,8 @@ LG_PLACE
 ~~~~~~~~
 This variable can be used to specify a place without using the ``-p`` option, the ``-p`` option overrides it.
 
-LG_TOKEN
-~~~~~~~~
+LG_RESERVATION (previously LG_TOKEN)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 This variable can be used to specify a reservation for the ``wait`` command and
 for the ``+`` place expansion.
 

doc/usage.rst

@@ -67,7 +67,7 @@ tags).
   $ labgrid-client reserve board=imx6-foo
   Reservation 'SP37P5OQRU':
     owner: rettich/jlu
-    token: SP37P5OQRU
+    id   : SP37P5OQRU
     state: waiting
     filters:
       main: board=imx6-foo
@@ -76,7 +76,7 @@ tags).
 
 As soon as any matching place becomes free, the reservation state will change
 from ``waiting`` to ``allocated``.
-Then, you can use the reservation token prefixed by ``+`` to refer to the
+Then, you can use the reservation id prefixed by ``+`` to refer to the
 allocated place for locking and usage.
 While a place is allocated for a reservation, only the owner of the reservation
 can lock that place.
@@ -86,15 +86,15 @@ can lock that place.
 
   $ labgrid-client wait SP37P5OQRU
   owner: rettich/jlu
-  token: SP37P5OQRU
+  id:    SP37P5OQRU
   state: waiting
   filters:
     main: board=imx6-foo
   created: 2019-08-06 12:56:49.779982
   timeout: 2019-08-06 12:58:14.900621
   …
   owner: rettich/jlu
-  token: SP37P5OQRU
+  id:    SP37P5OQRU
   state: allocated
   filters:
     main: board=imx6-foo
@@ -107,7 +107,7 @@ can lock that place.
   $ labgrid-client reservations
   Reservation 'SP37P5OQRU':
     owner: rettich/jlu
-    token: SP37P5OQRU
+    id:    SP37P5OQRU
     state: acquired
     filters:
       main: board=imx6-foo
@@ -117,28 +117,41 @@ can lock that place.
     timeout: 2019-08-06 12:59:11.840780
   $ labgrid-client -p +SP37P5OQRU console
 
+
+.. note::
+
+   Reservation identifiers were previously called tokens. They are ordinary
+   identifiers, not secret authentication tokens, so the old name misleadingly
+   implied a security property they do not have.
+   As part of this terminology change, ``LG_TOKEN`` has been renamed to
+   ``LG_RESERVATION``. During a transition period, ``labgrid-client reserve
+   --shell`` exports both ``LG_RESERVATION`` and the deprecated ``LG_TOKEN``.
+   Command output now uses ``id`` instead of ``token``.
+   The legacy ``LG_TOKEN`` variable remains accepted as input for backwards
+   compatibility, but scripts should migrate to ``LG_RESERVATION``.
+
 When using reservation in a CI job or to save some typing, the ``labgrid-client
 reserve`` command supports a ``--shell`` command to print code for evaluating
 in the shell.
-This sets the ``LG_TOKEN`` environment variable, which is then automatically
+This sets the ``LG_RESERVATION`` environment variable, which is then automatically
 used by ``wait`` and expanded via ``-p +``.
 
 .. code-block:: bash
 
   $ eval `labgrid-client reserve --shell board=imx6-foo`
-  $ echo $LG_TOKEN
+  $ echo $LG_RESERVATION
   ZDMZJZNLBF
   $ labgrid-client wait
   owner: rettich/jlu
-  token: ZDMZJZNLBF
+  id:    ZDMZJZNLBF
   state: waiting
   filters:
     main: board=imx6-foo
   created: 2019-08-06 13:05:30.987072
   timeout: 2019-08-06 13:06:44.629736
   …
   owner: rettich/jlu
-  token: ZDMZJZNLBF
+  id:    ZDMZJZNLBF
   state: allocated
   filters:
     main: board=imx6-foo

labgrid/remote/client.py

@@ -86,6 +86,22 @@ def __str__(self):
         return f"{self.message}:\n{errors_combined}"
 
 
+def _get_reservation_id_from_env():
+    reservation_id = os.environ.get("LG_RESERVATION")
+    legacy_reservation_id = os.environ.get("LG_TOKEN")
+    if reservation_id:
+        if legacy_reservation_id and legacy_reservation_id != reservation_id:
+            print(
+                "warning: LG_RESERVATION and deprecated LG_TOKEN are set to different values; using LG_RESERVATION",
+                file=sys.stderr,
+            )
+        return reservation_id
+
+    if legacy_reservation_id:
+        print("warning: LG_TOKEN is deprecated; use LG_RESERVATION instead", file=sys.stderr)
+    return legacy_reservation_id
+
+
 @attr.s(eq=False)
 class ClientSession:
     """The ClientSession encapsulates all the actions a Client can invoke on
@@ -440,19 +456,19 @@ def _match_places(self, pattern):
         """
         result = set()
 
-        # reservation token lookup
-        token = None
+        # reservation id lookup
+        reservation_id = None
         if pattern.startswith("+"):
-            token = pattern[1:]
-            if not token:
-                token = os.environ.get("LG_TOKEN", None)
-            if not token:
+            reservation_id = pattern[1:]
+            if not reservation_id:
+                reservation_id = _get_reservation_id_from_env()
+            if not reservation_id:
                 return []
             for name, place in self.places.items():
-                if place.reservation == token:
+                if place.reservation == reservation_id:
                     result.add(name)
             if not result:
-                raise UserError(f"reservation token {token} matches nothing")
+                raise UserError(f"reservation id {reservation_id} matches nothing")
             return list(result)
 
         # name and alias lookup
@@ -1574,28 +1590,30 @@ async def create_reservation(self):
 
         res = Reservation.from_pb2(response.reservation)
         if self.args.shell:
-            print(f"export LG_TOKEN={res.token}")
+            print(f"export LG_RESERVATION={res.id}")
+            # TODO: remove the deprecated LG_TOKEN export after one release.
+            print(f"export LG_TOKEN={res.id}")
         else:
-            print(f"Reservation '{res.token}':")
+            print(f"Reservation '{res.id}':")
             res.show(level=1)
         if self.args.wait:
             if not self.args.shell:
                 print("Waiting for allocation...")
-            await self._wait_reservation(res.token, verbose=False)
+            await self._wait_reservation(res.id, verbose=False)
 
     async def cancel_reservation(self):
-        token: str = self.args.token
+        reservation_id: str = self.args.reservation_id
 
-        request = labgrid_coordinator_pb2.CancelReservationRequest(token=token)
+        request = labgrid_coordinator_pb2.CancelReservationRequest(token=reservation_id)
 
         try:
             await self.stub.CancelReservation(request)
         except grpc.aio.AioRpcError as e:
             raise ServerError(e.details())
 
-    async def _wait_reservation(self, token: str, verbose=True):
+    async def _wait_reservation(self, reservation_id: str, verbose=True):
         while True:
-            request = labgrid_coordinator_pb2.PollReservationRequest(token=token)
+            request = labgrid_coordinator_pb2.PollReservationRequest(token=reservation_id)
 
             try:
                 response: labgrid_coordinator_pb2.PollReservationResponse = await self.stub.PollReservation(request)
@@ -1611,8 +1629,8 @@ async def _wait_reservation(self, token: str, verbose=True):
                 break
 
     async def wait_reservation(self):
-        token = self.args.token
-        await self._wait_reservation(token)
+        reservation_id = self.args.reservation_id
+        await self._wait_reservation(reservation_id)
 
     async def print_reservations(self):
         request = labgrid_coordinator_pb2.GetReservationsRequest()
@@ -1624,7 +1642,7 @@ async def print_reservations(self):
             raise ServerError(e.details())
 
         for res in sorted(reservations, key=lambda x: (-x.prio, x.created)):
-            print(f"Reservation '{res.token}':")
+            print(f"Reservation '{res.id}':")
             res.show(level=1)
 
     async def export(self, place, target):
@@ -2212,11 +2230,23 @@ def get_parser(auto_doc_mode=False) -> "argparse.ArgumentParser | AutoProgramArg
     subparser.set_defaults(func=ClientSession.create_reservation)
 
     subparser = subparsers.add_parser("cancel-reservation", help="cancel a reservation")
-    subparser.add_argument("token", type=str, nargs="?")
+    subparser.add_argument(
+        "reservation_id",
+        type=str,
+        nargs="?",
+        help="the reservation id (previously called token)",
+        metavar="reservation-id",
+    )
     subparser.set_defaults(func=ClientSession.cancel_reservation)
 
     subparser = subparsers.add_parser("wait", help="wait for a reservation to be allocated")
-    subparser.add_argument("token", type=str, nargs="?")
+    subparser.add_argument(
+        "reservation_id",
+        type=str,
+        nargs="?",
+        help="the reservation id (previously called token)",
+        metavar="reservation-id",
+    )
     subparser.set_defaults(func=ClientSession.wait_reservation)
 
     subparser = subparsers.add_parser("reservations", help="list current reservations")
@@ -2259,7 +2289,6 @@ def main():
     state = os.environ.get("STATE", None)
     state = os.environ.get("LG_STATE", state)
     initial_state = os.environ.get("LG_INITIAL_STATE", None)
-    token = os.environ.get("LG_TOKEN", None)
 
     parser = get_parser()
 
@@ -2283,11 +2312,12 @@ def main():
     if args.initial_state is None:
         args.initial_state = initial_state
 
-    if args.command in ["cancel-reservation", "wait"] and args.token is None:
-        if token:
-            args.token = token
+    if args.command in ["cancel-reservation", "wait"] and args.reservation_id is None:
+        reservation_id = _get_reservation_id_from_env()
+        if reservation_id:
+            args.reservation_id = reservation_id
         else:
-            print("Please provide a token", file=sys.stderr)
+            print("Please provide a reservation id (previously called token)", file=sys.stderr)
             exit(1)
 
     if args.verbose:

labgrid/remote/common.py

@@ -397,7 +397,7 @@ class ReservationState(enum.Enum):
 @attr.s(eq=False)
 class Reservation:
     owner = attr.ib(validator=attr.validators.instance_of(str))
-    token = attr.ib(
+    id = attr.ib(
         default=attr.Factory(lambda: "".join(random.choice(string.ascii_uppercase + string.digits) for i in range(10)))
     )
     state = attr.ib(
@@ -434,7 +434,7 @@ def expired(self):
     def show(self, level=0):
         indent = "  " * level
         print(indent + f"owner: {self.owner}")
-        print(indent + f"token: {self.token}")
+        print(indent + f"id:    {self.id}")
         print(indent + f"state: {self.state.name}")
         if self.prio:
             print(indent + f"prio: {self.prio}")
@@ -451,7 +451,7 @@ def show(self, level=0):
     def as_pb2(self):
         res = labgrid_coordinator_pb2.Reservation()
         res.owner = self.owner
-        res.token = self.token
+        res.token = self.id
         res.state = self.state.value
         res.prio = self.prio
         for name, fltr in self.filters.items():
@@ -477,7 +477,7 @@ def from_pb2(cls, pb2: labgrid_coordinator_pb2.Reservation):
             allocations[fltr_name] = [place_name]
         return cls(
             owner=pb2.owner,
-            token=pb2.token,
+            id=pb2.token,
             state=ReservationState(pb2.state),
             prio=pb2.prio,
             filters=filters,