Change the grant addmodel permission to add-model #6393

Merged
merged 2 commits into from Oct 11, 2016

Conversation

Projects
None yet
5 participants
Contributor

macgreagoir commented Oct 6, 2016

Standardise the juju grant add model permission from "addmodel" to the
"add-model" phrasing used elsewhere in the CLI.

Fixes lp:1622433

QA steps:

  • juju add-user foo and juju grant foo add-model
    • User foo should be granted permission without error
  • juju grant foo addmodel (using the previous permission)
    • You should see an error that "addmodel" access is not valid

I'd like confirmation from Alexis/Rick that we can make this change without needing to support a deprecated "addmodel" permission. I'm also not sure the change is necessary. If the reason is that we can the permission name to match the command name, this is the only case for that. "superuser" permission doesn't match any CLI name. Nor does "read", "write" or "admin". The "addmodel" is a permission name which I think is a mistake to try and tie directly to any particular CLI name.

Owner

mitechie commented Oct 10, 2016

The goal we mentioned last week was to make it still accept the addmodel name.

This needs to match the CLI because that's what the documentation will mention. Having the ACL match what the user can do with that ACL helps align the model to the user. Having them mis-match creates more confusion and a lack of matching hits in the docs when searching for addmodel and not finding documentation for the add-model commands/etc.

Owner

wallyworld commented Oct 11, 2016

But the "add-model" vs "addmodel" issue only applies for that one permission. There's no "superuser" command which matches the "superuser" permission. There's no "write" or "admin" commands which match those permissions. Permissions and CLI names are totally orthogonal. It is possible that the addmodel permission may grant access to additional CLI commands besides just "add-model". Artificially tying permissions names to CLI names inconsistently is not the best IMO.

macgreagoir added some commits Oct 6, 2016

Change the grant addmodel permission to add-model
Standardise the `juju grant` add model permission from "addmodel" to the
"add-model" phrasing used elsewhere in the CLI.

Fixes lp:1622433
Add a backwards-compatible 'addmodel' access to grant and revoke
Juju grant and revoke now expect the add-model access permission. This
commit allows addmodel as an effective alias, for backwards
compatibility.

LGTM

$$merge$$

Contributor

jujubot commented Oct 11, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 29fdf5b into juju:master Oct 11, 2016

@macgreagoir macgreagoir deleted the macgreagoir:grant-add-model branch Oct 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment