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

Fixed roles diff when the roles are provided as a list #737

Merged

Conversation

bogdanmuresan
Copy link
Contributor

What does this PR do?

The PR fixes roles diff, when the roles are provided as a list in the "roles" key, and not individually in "role".
All the elements in the compare list need to be expanded once more, for each role in the roles list.

At the moment, when using roles as a list, all the controller_roles are set to state absent.

How should this be tested?

Is there a relevant Issue open for this?

resolves #[number]

Other Relevant info, PRs, etc

plugins/lookup/controller_object_diff.py Outdated Show resolved Hide resolved
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.

Why don't adding the role and roles to the already existing for ?

for item in compare_list_reduced:
expanded = False
dupitems = [
"target_team",
"target_teams",
"job_template",
"job_templates",
"workflow",
"workflows",
"inventory",
"inventories",
"project",
"projects",
"credential",
"credentials",
]

@bogdanmuresan
Copy link
Contributor Author

bogdanmuresan commented Dec 7, 2023

Why don't adding the role and roles to the already existing for ?

for item in compare_list_reduced:
expanded = False
dupitems = [
"target_team",
"target_teams",
"job_template",
"job_templates",
"workflow",
"workflows",
"inventory",
"inventories",
"project",
"projects",
"credential",
"credentials",
]

Because roles is another dimension. So assuming we have:

---
controller_roles:
  - team: MYTEAM
    projects:
      - project1
      - project2  
    roles:
     - use
     - update

compare_list_reduced will need to have 4 elements at the end, a combination of the projects with each role:
project1 - use, project1 - update, project 2 - use, project2 - update

I don't think this can be done in the first loop, which expands only the projects. I then need to loop once more through the expanded list and expand again for each role.

I guess I could move dupitems definition above the for, so it's scope also covers the second loop, but I thought it would be better to have them separate.

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

@ivarmu ivarmu merged commit 2a6f175 into redhat-cop:devel Dec 7, 2023
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