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: no longer use "item" as a loop variable #217

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

berndfinger
Copy link
Contributor

By using a different loop variable, we can avoid WARNING messages related to loop variables when calling the selinux role from another role.

Fixes: #216 .

Enhancement: Avoid the warning message:

[WARNING]: TASK: fedora.linux_system_roles.selinux : Restore SELinux labels on filesystem tree: The loop variable 'item' is already in use.
You should set the `loop_var` value in the `loop_control` option for the task to something else to avoid variable collisions and unexpected behavior.

Reason: Although I am not aware of any errors when using the role selinux without this fix, the warning message (printed in bold and in a different than usual color - violet in my tests) adds lines to the output which can confuse users.

Result: The warning message `[WARNING]: TASK: ... "The loop variable 'item' is already in use." is no longer displayed when calling the role from another role.

Issue Tracker Tickets (Jira or BZ if any): N/A

By using a different loop variable, we can avoid WARNING messages
related to loop variables when calling the selinux role from another
role.

Fixes: linux-system-roles#216

Signed-off-by: Bernd Finger <bfinger@redhat.com>
@richm
Copy link
Contributor

richm commented Dec 7, 2023

Thanks - in fact I would suggest you go further and use __selinux_item as the loop variable to go along with the general practice of naming role private variables with the __$ROLENAME_ prefix.

Relates to linux-system-roles#216 .

Signed-off-by: Bernd Finger <bfinger@redhat.com>
@berndfinger
Copy link
Contributor Author

I like the idea of naming the loop variable to also contain the role name. See 5bff24d.

tasks/main.yml Outdated Show resolved Hide resolved
Co-authored-by: Richard Megginson <richm@stanfordalumni.org>
@richm
Copy link
Contributor

richm commented Dec 7, 2023

[citest]

@richm
Copy link
Contributor

richm commented Dec 7, 2023

These test failures are really strange - I don't think they are related to the PR - investigating

@richm
Copy link
Contributor

richm commented Dec 7, 2023

@berndfinger in tasks/selinux_load_module.yml item needs to be renamed to __selinux_item

Signed-off-by: Bernd Finger <bfinger@redhat.com>
@richm
Copy link
Contributor

richm commented Dec 8, 2023

[citest]

@richm richm merged commit c7e9dea into linux-system-roles:main Dec 8, 2023
31 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
2 participants