Skip to content

Commit

Permalink
Merge pull request #1003 from cderici/no-contr-model-access-needed-fo…
Browse files Browse the repository at this point in the history
…r-connection

#1003

#### Description

This is a follow up PR, includes changes from #1002. If you're reviewing this and #1002 hasn't landed yet, you might see some additional changes that'll land with #1002. We need those changes to be able to test this change.

`ControllerAPIInfoForModels` call in `update_endpoints` after a controller connection 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, so the `update_endpoint` call at the end of a
regular connection fails.

Fixes #998 

#### QA Steps

We'll need a 2.9 controller, so:

```
 $ juju_29 bootstrap localhost issue998
```

Now we have the admin user on this by default, but we don't wanna use that. Let's create a new user (that doesn't have superuser so it can't read the controller model):

```sh
 $ juju add-user caner
 $ juju register
```

Now let's make a password for this user.

```sh
 $ juju change-user-password # to be able to access the admin user later
 $ juju logout
 $ juju login -u caner -c issue998
 $ juju change-user-password
<your-password>
 This should work just fine, we're just making sure the user is ready to go.
```

Now let's spawn a pylibjuju repl to test this change with this user (that can't access the `controller` model):

Wihtout this change, you'd see the error in #998. With this change the connection should work just fine:

```python
# cd pylibjuju directory
$ python -m asyncio
>>> from juju import controller; c=controller.Controller()
>>> await c.connect(username='caner', password=<your-password>)
>>>
exiting asyncio REPL...
# Connection works without any issues.
```

#### Notes & Discussion

JUJU-5268
  • Loading branch information
jujubot authored Jan 11, 2024
2 parents df709e3 + c7e4f13 commit beed55f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 18 deletions.
36 changes: 23 additions & 13 deletions juju/client/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ async def connect(self, **kwargs):
else:
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)

if not self.controller_name:
Expand All @@ -103,7 +115,7 @@ async def disconnect(self):
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.
"""
Expand All @@ -118,16 +130,15 @@ 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"]

Expand Down Expand Up @@ -176,8 +187,7 @@ async def connect_model(self, model_name=None, **kwargs):
# JujuData.
kwargs.update(endpoint=endpoints,
uuid=model_uuid,
username=account.get('user'),
password=account.get('password'),
account=account,
cacert=controller.get('ca-cert'),
bakery_client=self.bakery_client_for_controller(controller_name),
proxy=proxy)
Expand Down
16 changes: 11 additions & 5 deletions juju/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,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):
"""
Expand Down Expand Up @@ -281,6 +285,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)
Expand Down
4 changes: 4 additions & 0 deletions juju/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ class JujuUnitError(JujuError):
pass


class JujuPermissionError(JujuError):
pass


class JujuBackupError(JujuError):
pass

Expand Down

0 comments on commit beed55f

Please sign in to comment.