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 - support of exporting access lists #1

Merged
merged 11 commits into from
Oct 30, 2024

Conversation

przemkalit
Copy link
Contributor

What does this PR do?

This PR introduce a feature that I was working recently, which is exporting permission of objects using access list endpoint of API.

I don't know if you would like to allow this in the collection but I decided to share this feature, but let me know what do you think.

How should this be tested?

- name: Permission export
  hosts: localhost
  connection: local
  gather_facts: false

  tasks:
    - name: Export team permissions of an object
      ansible.builtin.include_role:
        name: filetree_create
        tasks_from: team_access_list
      vars:
        object_id: 1
        object_type: "job_templates"

    - name: Export user permissions of an object
      ansible.builtin.include_role:
        name: filetree_create
        tasks_from: user_access_list
      vars:
        object_id: 1
        object_type: "job_templates"

Is there a relevant Issue open for this?

N/A

Other Relevant info, PRs, etc

N/A

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.

I think it's a very good idea and I appreciate your contribution a lot, I'm sure it'll be very useful.

That said, one thing I would like to have here, is the possibility of using the object's name instead of the IDs, as the CasC intention is to avoid workiing with IDs and work only with the objects' names.

@przemkalit
Copy link
Contributor Author

przemkalit commented Oct 14, 2024

Ok I've added support for an object_name.

@przemkalit przemkalit requested a review from ivarmu October 15, 2024 13:32
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

Copy link
Contributor

@adonisgarciac adonisgarciac left a comment

Choose a reason for hiding this comment

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

How are you going to use these new task files? As I can see, you're not adding these new ones to all.yaml task file. And I think we shouldn't add them to all.yaml due to we want to export teams and user roles as we have been doing so far.

Take into consideration that if you export objects using access_list and you've done that before using "regular" way, you could end up having duplicated roles definition.

Have you tested this code? I'm not sure this can work or at least can provide an output useful for dispatch role. Can you share the output of it?

roles/filetree_create/README.md Outdated Show resolved Hide resolved
@przemkalit
Copy link
Contributor Author

How are you going to use these new task files? As I can see, you're not adding these new ones to all.yaml task file. And I think we shouldn't add them to all.yaml due to we want to export teams and user roles as we have been doing so far.

Take into consideration that if you export objects using access_list and you've done that before using "regular" way, you could end up having duplicated roles definition.

Have you tested this code? I'm not sure this can work or at least can provide an output useful for dispatch role. Can you share the output of it?

I did not want to add it to all.yml because, for me, it is a set of tasks that export the access list to an object. Now that I think about it, maybe I should add these tasks as a separate role?

I am using this in our content promotion solution and it is exporting properly the permission and then they are properly loaded with the dispatcher.

@adonisgarciac
Copy link
Contributor

If you don't want want to add it to all.yaml, will you use those file tasks calling them directly or how do you want to manage it?

are you defining controller_roles as a dict in you environment and it works? I expected it fails because aap_conifuguration.controller_roles expects a list: https://github.com/redhat-cop/infra.aap_configuration/blob/devel/roles/controller_roles/tasks/main.yml#L34

@przemkalit
Copy link
Contributor Author

If you don't want want to add it to all.yaml, will you use those file tasks calling them directly or how do you want to manage it?

It would be like that:

    - name: Export team permissions of an object
      ansible.builtin.include_role:
        name: filetree_create
        tasks_from: team_access_list
      vars:
        object_id: 1
        object_type: "job_templates"

are you defining controller_roles as a dict in you environment and it works? I expected it fails because aap_conifuguration.controller_roles expects a list: https://github.com/redhat-cop/infra.aap_configuration/blob/devel/roles/controller_roles/tasks/main.yml#L34

Yes, you are totally right, I don't know why in our environment it works without dictionary issue. I ran it as regular playbook in EE and it return the error, I've introduce the fix.

Copy link
Contributor

@adonisgarciac adonisgarciac left a comment

Choose a reason for hiding this comment

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

LGTM

@ivarmu ivarmu merged commit d69106b into redhat-cop:devel Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants