Skip to content
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

feature/rbac-roles - add role commands; namespaces.addgroup - use object_roles, not object_permissions #39

Merged
merged 6 commits into from
Aug 17, 2022

Conversation

himdel
Copy link
Collaborator

@himdel himdel commented Apr 8, 2022

(draft until the feature/rbac-roles galaxy_ng & ansible-hub-ui branches get merged in master)

This addresses compatibility with the new (4.6+) rbac roles:

AAH-1691:
galaxykit role list
galaxykit role create <rolename> <description> --permissions <permissions>
galaxykit role delete <rolename>
galaxykit role perm list <rolename>
galaxykit role perm add <rolename> <perm>
galaxykit role perm remove <rolename> <perm>

AAH-1692:
galaxykit group role add <group> <role>
galaxykit group role remove <group> <role>

No-Issue:
make galaxykit namespace addgroup <namespace> <group> work again, by sending object_roles instead of object_permissions

Removed:
galaxykit group perm list <groupname>
galaxykit group perm add <groupname> <perm>
galaxykit group perm remove <groupname> <perm>

Removed methods:

  • groups.get_permissions (replaced by roles.get_permissions)
  • groups.set_permissions (replaced by roles.set_permissions with add_permissions=[])
  • groups.delete_permission (replaced by roles.set_permissions with remove_permissions=[])
  • client.set_permissions (not sure if unused, or if we just need to change it to call roles.set_permissions instead of groups.set_permissions)

Also adding vim swapfiles to gitignore,
and adding a client.patch method, used by role perm add/remove.

hendersonreed
hendersonreed previously approved these changes Apr 11, 2022
Copy link
Contributor

@hendersonreed hendersonreed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! We'll need to remember that this is a breaking change for when we do our next version bump.

@himdel
Copy link
Collaborator Author

himdel commented Apr 11, 2022

Thanks!
Moving back to draft since we're not doing rbac for 4.5.0 anymore.

This needs to come back with the revert of today's/tomorrow's revert of ansible/galaxy_ng#1057 :)

EDIT: that evolved into the feature/rbac-roles branch

@himdel himdel marked this pull request as draft April 11, 2022 16:08
@himdel himdel changed the title namespaces.addgroup - use object_roles, not object_permissions feature/rbac-roles - add role commands; namespaces.addgroup - use object_roles, not object_permissions Jun 29, 2022
@himdel
Copy link
Collaborator Author

himdel commented Jun 30, 2022

@jerabekjiri This should be ready to address AAH-1691 & AAH-1692, do we need anything else to test the rbac branch properly?

@hendersonreed I've updated this and added some more backward incompatible changes (removes client.set_permissions and galaxykit group perm * commands), is this still ok (for when the feature branches gets merged in master)? Do you know if it'd be better to update client.set_permissions to update role permissions instead, instead of removing it?

jerabekjiri
jerabekjiri previously approved these changes Jul 1, 2022
Copy link
Contributor

@jerabekjiri jerabekjiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great 👍
tested and looks like it covers all we need :)

@hendersonreed
Copy link
Contributor

Backwards in-compatible changes are fine right now I think, particularly because the galaxykit usage on the QE side is still relatively minimal (migrating the IQE test suite to use more galaxykit features is an incremental process).

We'll just need to ensure that we bump the version properly when we release.

galaxykit/client.py Outdated Show resolved Hide resolved
@himdel
Copy link
Collaborator Author

himdel commented Jul 28, 2022

@chr-stian The last commits are adding a dependency on ansible again. Given the context of #54, #55 and #39 (comment), I'll ask you to create a separate PR for those changes.

I'm restoring this PR to the original state and rebasing on top of current main, my priority here is to have UI tests working so we can merge the branches soon :).

@chr-stian
Copy link
Collaborator

yes, wait_for_task throws GalaxyError. I could fix it easily by changing it to GalaxyClientError. But I can wait.

himdel added 6 commits July 28, 2022 17:50
```diff
+galaxykit group role *
+galaxykit role *
-galaxykit group perm *
+galaxykit role perm *
```
 (not sure if unused, or if we just need to change it to call roles.set_permissions instead of groups.set_permissions)
@himdel himdel marked this pull request as ready for review August 16, 2022 22:12
@himdel
Copy link
Collaborator Author

himdel commented Aug 17, 2022

@hendersonreed do we have your blessing to merge this one? :)

@hendersonreed
Copy link
Contributor

I think so! right now no one I know of is using master anyways, so this shouldn't break anything anyways, and the rbac branches have been merged.

@himdel himdel merged commit c5e8a94 into ansible:main Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants