From 7b7579e1cdf0566661b0d71069fd531f76463909 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 4 Dec 2023 11:14:24 -0700 Subject: [PATCH 1/6] Pass websocket Close frame into ConnectionClosed Fixes #989 --- juju/client/connection.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/juju/client/connection.py b/juju/client/connection.py index 0e3d9ef0c..45ee6d95e 100644 --- a/juju/client/connection.py +++ b/juju/client/connection.py @@ -470,7 +470,9 @@ async def close(self, to_reconnect=False): async def _recv(self, request_id): if not self.is_open: - raise websockets.exceptions.ConnectionClosed(0, 'websocket closed') + raise websockets.exceptions.ConnectionClosed( + websockets.frames.Close(websockets.frames.CloseCode.NORMAL_CLOSURE, + 'websocket closed')) try: return await self.messages.get(request_id) except GeneratorExit: @@ -641,7 +643,8 @@ async def rpc(self, msg, encoder=None): if self.monitor.status == Monitor.DISCONNECTED: # closed cleanly; shouldn't try to reconnect raise websockets.exceptions.ConnectionClosed( - 0, 'websocket closed') + websockets.frames.Close(websockets.frames.CloseCode.NORMAL_CLOSURE, + 'websocket closed')) try: await self._ws.send(outgoing) break From 2154a9e8bcec8a372d5d24eaabae195ca13dd09d Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 8 Jan 2024 15:30:47 -0700 Subject: [PATCH 2/6] User accounts.yaml as a backup to user provided credentials For both model and controller connections, accounts.yaml is required to have both username and password for connection to be established. This is not always true, for example, juju change-user-password actually removes the password from the accounts.yaml file. --- juju/client/connector.py | 46 +++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/juju/client/connector.py b/juju/client/connector.py index f1fe54a5e..2e40b0f07 100644 --- a/juju/client/connector.py +++ b/juju/client/connector.py @@ -85,6 +85,15 @@ async def connect(self, **kwargs): # connected to. if self._connection: await self._connection.close() + + account = kwargs.pop('account', {}) + # Prioritize the username and password that user provided + # If not enough, try to patch it with info from accounts.yaml + if 'username' not in kwargs and account.get('user'): + kwargs.update(username=account.get('user')) + if 'password' not in kwargs and account.get('password'): + kwargs.update(password=account.get('password')) + self._connection = await Connection.connect(**kwargs) # Check if we support the target controller @@ -117,7 +126,7 @@ async def disconnect(self, entity): await self._log_connection.close() self._log_connection = None - async def connect_controller(self, controller_name=None, specified_facades=None): + async def connect_controller(self, controller_name=None, specified_facades=None, **kwargs): """Connect to a controller by name. If the name is empty, it connect to the current controller. """ @@ -130,16 +139,16 @@ async def connect_controller(self, controller_name=None, specified_facades=None) proxy = proxy_from_config(controller.get("proxy-config", None)) - await self.connect( - endpoint=endpoints, - uuid=None, - username=accounts.get("user"), - password=accounts.get("password"), - cacert=controller.get("ca-cert"), - bakery_client=self.bakery_client_for_controller(controller_name), - specified_facades=specified_facades, - proxy=proxy, - ) + kwargs.update(endpoint=endpoints, + uuid=None, + account=accounts, + cacert=controller.get('ca-cert'), + bakery_client=self.bakery_client_for_controller(controller_name), + specified_facades=specified_facades, + proxy=proxy, + ) + await self.connect(**kwargs) + self.controller_name = controller_name self.controller_uuid = controller["uuid"] async def connect_model(self, _model_name=None, **kwargs): @@ -184,15 +193,12 @@ async def connect_model(self, _model_name=None, **kwargs): # TODO remove the need for base.CleanModel to subclass # JujuData. - kwargs.update( - endpoint=endpoints, - uuid=model_uuid, - username=account.get("user"), - password=account.get("password"), - cacert=controller.get("ca-cert"), - bakery_client=self.bakery_client_for_controller(controller_name), - proxy=proxy, - ) + kwargs.update(endpoint=endpoints, + uuid=model_uuid, + account=account, + cacert=controller.get('ca-cert'), + bakery_client=self.bakery_client_for_controller(controller_name), + proxy=proxy) await self.connect(**kwargs) # TODO this might be a good spot to trigger refreshing the # local cache (the connection to the model might help) From e203dd709235a2872c7e1fe1a6ca2fb2795cb8ef Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 8 Jan 2024 15:33:12 -0700 Subject: [PATCH 3/6] Ask for credentials if some missing before connect Fixes #1001 --- juju/client/connector.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/juju/client/connector.py b/juju/client/connector.py index 2e40b0f07..3a901c7d0 100644 --- a/juju/client/connector.py +++ b/juju/client/connector.py @@ -94,6 +94,9 @@ async def connect(self, **kwargs): if 'password' not in kwargs and account.get('password'): kwargs.update(password=account.get('password')) + if not ({'username', 'password'}.issubset(kwargs)): + required = {'username', 'password'}.difference(kwargs) + raise ValueError(f'Some authentication parameters are required : {",".join(required)}') self._connection = await Connection.connect(**kwargs) # Check if we support the target controller From 8e6dab750cc58d8f48d611ae5e8c6ae0cf8d9d81 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 8 Jan 2024 15:52:38 -0700 Subject: [PATCH 4/6] Update endpoints only if controller model can be accessed Fixes #998 ControllerAPIInfoForModels call requires the uuid of the controller model, and if the user doesn't have at least read access, they won't be able to have that, and the update_endpoint call at the end of a regular connection fails. --- juju/controller.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/juju/controller.py b/juju/controller.py index 10e55188f..32827669e 100644 --- a/juju/controller.py +++ b/juju/controller.py @@ -140,11 +140,14 @@ async def connect(self, *args, **kwargs): await self.update_endpoints() async def update_endpoints(self): - info = await self.info() - self._connector._connection.endpoints = [ - (e, info.results[0].cacert) - for e in info.results[0].addresses - ] + try: + info = await self.info() + self._connector._connection.endpoints = [ + (e, info.results[0].cacert) + for e in info.results[0].addresses + ] + except errors.JujuPermissionError: + pass async def connect_current(self): """ @@ -288,6 +291,8 @@ async def info(self): """ log.debug('Getting information') uuids = await self.model_uuids() + if 'controller' not in uuids: + raise errors.JujuPermissionError('Requires access to controller model.') controller_facade = client.ControllerFacade.from_connection(self.connection()) params = [client.Entity(tag.model(uuids["controller"]))] return await controller_facade.ControllerAPIInfoForModels(entities=params) From f02efc0fbc44d1ae6a392c5714cee36c60af999f Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 8 Jan 2024 15:57:45 -0700 Subject: [PATCH 5/6] Emit a warning in update_endpoints on failure --- juju/controller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/juju/controller.py b/juju/controller.py index 32827669e..97e9963f0 100644 --- a/juju/controller.py +++ b/juju/controller.py @@ -147,6 +147,7 @@ async def update_endpoints(self): for e in info.results[0].addresses ] except errors.JujuPermissionError: + log.warning("This user doesn't have at least read access to the controller model, so endpoints are not updated after connection.") pass async def connect_current(self): From fedeb48518d240ff4107d48bed5a40af7b4336d0 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 10 Jan 2024 13:16:08 -0700 Subject: [PATCH 6/6] Add JujuPermissionError in juju/errors --- juju/errors.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/juju/errors.py b/juju/errors.py index d02841583..9932057bf 100644 --- a/juju/errors.py +++ b/juju/errors.py @@ -86,6 +86,10 @@ class JujuUnitError(JujuError): pass +class JujuPermissionError(JujuError): + pass + + class JujuBackupError(JujuError): pass