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 #132

Closed

Conversation

jovial
Copy link
Contributor

@jovial jovial commented Oct 6, 2023

Overall Review of Changes:

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

Issue Fixes:
No issue, but let me know if you want me to create one.

Enhancements:

  • Allows use of inject_facts_as_vars=False.

How has this been tested?:
Ran the role against a host.

@jovial jovial force-pushed the feature/inject_facts_as_vars branch from 59762dd to f48cec5 Compare October 6, 2023 15:00
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.

@jovial jovial force-pushed the feature/inject_facts_as_vars branch from f48cec5 to 790eecb Compare October 6, 2023 15:04
@jovial jovial changed the title By default, Ansible injects a variable for every fact, prefixed with Use ansible_facts to reference facts Oct 6, 2023
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: Will Szumski <[email protected]>
@jovial jovial force-pushed the feature/inject_facts_as_vars branch from 790eecb to 52d62f5 Compare October 6, 2023 16:23
@jovial jovial marked this pull request as ready for review October 6, 2023 16:25
@jovial
Copy link
Contributor Author

jovial commented Oct 6, 2023

Equivalent to: ansible-lockdown/RHEL9-CIS#54

uk-bolly
uk-bolly previously approved these changes Oct 20, 2023
Copy link
Member

@uk-bolly uk-bolly left a comment

Choose a reason for hiding this comment

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

Great update thank you

@uk-bolly uk-bolly dismissed their stale review October 20, 2023 10:53

Required gpg signing

@uk-bolly
Copy link
Member

hi @jovial

Thank you for taking the time to raise this PR and helping to improve ansible-lockdown. It does really help us to improve with feedback.
While the change works we do require it to be GPG signed in order to be merged, if you could add a gpg signature to your commits that would be brilliant.

Thank you

uk-bolly

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

hi @jovial

Thank you again for you time, I have incorporated your changes into the community_work_fix branch with credits given as i need to try and get these fixes added as quickly as possible.

many thanks

uk-bolly

@uk-bolly
Copy link
Member

hi @jovial

Thanks again for taking the time to raise this PR. As mentioned we have managed to incorporate this into many other changes. With credit being given.
If the PR was GPG signed as well as signed-off this would have been accepted earlier.

many thanks once again

uk-bolly

@jovial
Copy link
Contributor Author

jovial commented Nov 13, 2023

Sorry, missed you comments. Thanks @uk-bolly - very much appreciated :)

@jovial jovial mentioned this pull request Nov 15, 2023
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