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

use ansible_facts to reference facts #54

Closed
wants to merge 1 commit into from

Conversation

mnasiadka
Copy link

By default, Ansible injects a variable for every fact, prefixed with ansible_. This can result in a large number of variables for each host, which at scale can incur a performance penalty. Ansible provides a configuration option [0] that can be set to False to prevent this injection of facts. In this case, facts should be referenced via ansible_facts..

This change updates all references to Ansible facts from using individual fact variables to using the items in the ansible_facts dictionary. This allows users to disable fact variable injection in their Ansible configuration, which may provide some performance improvement.

[0] https://docs.ansible.com/ansible/latest/reference_appendices/config.html#inject-facts-as-vars

Overall Review of Changes:
A general description of the changes made that are being requested for merge

Issue Fixes:
Please list (using linking) any open issues this PR addresses

Enhancements:
Please list any enhancements/features that are not open issue tickets

How has this been tested?:
Please give an overview of how these changes were tested. If they were not please use N/A

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

By default, Ansible injects a variable for every fact, prefixed with
ansible_. This can result in a large number of variables for each host,
which at scale can incur a performance penalty. Ansible provides a
configuration option [0] that can be set to False to prevent this
injection of facts. In this case, facts should be referenced via
ansible_facts..

This change updates all references to Ansible facts from using
individual fact variables to using the items in the
ansible_facts dictionary. This allows users to disable fact variable
injection in their Ansible configuration, which may provide some
performance improvement.

[0] https://docs.ansible.com/ansible/latest/reference_appendices/config.html#inject-facts-as-vars

Signed-off-by: Michal Nasiadka <[email protected]>
@mnasiadka
Copy link
Author

@uk-bolly If I can do anything to speed up merging of this - please let me know :)

uk-bolly added a commit that referenced this pull request Sep 6, 2023
Signed-off-by: Mark Bolwell <[email protected]>
@uk-bolly
Copy link
Member

uk-bolly commented Sep 6, 2023

hi @mnasiadka

Thank you for highlighting this and providing a very clear and concise fix, Apologies for the delay, we have had many other repositories to update along with bigger changes which affect each repository.
I am no longer able to rerun the workflow on this PR since all the changes, I have therefore included in the new PR to include the sept23 branch.
Full credits are given to again thank you for this.

Many thanks

uk-bolly

uk-bolly added a commit that referenced this pull request Sep 6, 2023
@uk-bolly uk-bolly mentioned this pull request Sep 6, 2023
@uk-bolly uk-bolly closed this Sep 19, 2023
jovial added a commit to stackhpc/RHEL9-CIS that referenced this pull request Nov 15, 2023
Also enabled testing of this in CI to prevent regressions.

See:
- https://docs.ansible.com/ansible/2.9/reference_appendices/config.html#inject-facts-as-vars
- ansible-lockdown#54

Signed-off-by: Will Szumski <[email protected]>
@jovial jovial mentioned this pull request Nov 15, 2023
jovial added a commit to stackhpc/RHEL9-CIS that referenced this pull request Nov 15, 2023
Also enabled testing of this in CI to prevent regressions.

See:
- https://docs.ansible.com/ansible/2.9/reference_appendices/config.html#inject-facts-as-vars
- ansible-lockdown#54

Signed-off-by: Will Szumski <[email protected]>
ipruteanu-sie pushed a commit to siemens/RHEL9-CIS that referenced this pull request Jan 31, 2024
Signed-off-by: Mark Bolwell <[email protected]>
Signed-off-by: Ionut Pruteanu <[email protected]>
ipruteanu-sie pushed a commit to siemens/RHEL9-CIS that referenced this pull request Jan 31, 2024
Signed-off-by: Mark Bolwell <[email protected]>
Signed-off-by: Ionut Pruteanu <[email protected]>
@uk-bolly uk-bolly mentioned this pull request Apr 30, 2024
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