New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support managing users #148
Conversation
Could you add some TF plans and use cases to test this manually? |
One question here @juanmanuel-tirado do you think it might be better for us to disable users rather than deleting them, given that deleting a user in Juju doesn't actually delete them, but means you can no longer use that username? Disabling them would mean it's a reversible operation, so would be safer, but possibly less expected. |
That's a good question. Following the CRUD policy used in terraform I will go with the |
Reproducing the change locally:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I think the solution is perfectly valid. I just made some comments on how to set the Id of the resource. Could we please for the sake of completeness add some manual steps for QA? Something like in #149
I already saw the manual QA. Thanks.
I've filed https://bugs.launchpad.net/juju/+bug/2007258 about this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
This is a PR that enables managing users (
add
,remove
). It closes #143. It updates on a password change only as changing thedisplay name
does not correspond to a juju API call. To be consistent with the juju CLI, it won't manipulate model access here, that would be done withgrant
ing additional permission in #144 .