Merge pull request #1022 from cderici/forward-ports-from-29

#1022

#### Description

This brings onto the 3.x track some of the latest fixes from to the 2.9 track. Here're the details:

* Fix for #989 from #990
* Fix for #1001 from #1002
* Fix for #998 from #1003


#### QA Steps

No QA needed for #990. For 1002 and 1003 please refer to their QA steps. Though they are very related so I'd expect the QA for both of them can be done in one fell swoop.

* #1002 
* #1003 

All CI tests need to pass (there's still some known intermittent ones in there).

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

juju/client/connector.py

@@ -85,6 +85,18 @@ 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'))
+
+            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
@@ -117,7 +129,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 +142,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 +196,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)

juju/controller.py

@@ -140,11 +140,15 @@ 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:
+            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):
         """
@@ -288,6 +292,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)

juju/errors.py

@@ -86,6 +86,10 @@ class JujuUnitError(JujuError):
     pass
 
 
+class JujuPermissionError(JujuError):
+    pass
+
+
 class JujuBackupError(JujuError):
     pass