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

Fix organization wide permissions in roles object diff #739

Conversation

bogdanmuresan
Copy link
Contributor

What does this PR do?

Attempts to fix role diff, when role is defined without specific resource_type - the role applies to all organization objects.
Sample controller_roles:

controller_roles:
  - team: Ansible
    organization: Acme
    role: execute

When this is applied, it will add the "execute" permissions for all job templates, therefore, when object_diff runs, it should not remove those roles, returned by the api as individual roles, one per job template.

How should this be tested?

Is there a relevant Issue open for this?

resolves #[number]

Other Relevant info, PRs, etc

@bogdanmuresan
Copy link
Contributor Author

@ivarmu can you please have a look over this PR as well?

Copy link
Contributor

@ivarmu ivarmu left a comment

Choose a reason for hiding this comment

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

LGTM. I'm not really sure that these auto created roles should be necessary at all, but it's a good idea to leave them intact as the are created (and hopefully also managed) by the core processes.

I've relaunched some tests to let them pass...

@ivarmu
Copy link
Contributor

ivarmu commented Jan 2, 2024

LGTM. I'm not really sure that these auto created roles should be necessary at all, but it's a good idea to leave them intact as the are created (and hopefully also managed) by the core processes.

I've relaunched some tests to let them pass...

I've confirmed these auto created roles are needed for the roles to work as expected. Good catch @bogdanmuresan!

@ivarmu ivarmu merged commit 82abc50 into redhat-cop:devel Jan 3, 2024
13 checks passed
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.

2 participants