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: #172 checks if ipv6.disable is present in GRUB_CMDLINE_LINUX bef… #173

Conversation

Jason-Hendry
Copy link
Contributor

Checks if ipv6.disable is present in GRUB_CMDLINE_LINUX before appending it to prevent duplication

Overall Review of Changes:
Adds a task to get the current value before appending
Adds a second condition to only append it wasn't changed by the previous task and doesn't already exist in the file

Issue Fixes:
172

Enhancements:
none

How has this been tested?:
A physical server, running Ubuntu 22.04 minimal installation, was already provisioned then ran the playbook multiple times.

With existing different value
image

With no existing value
image

When the current value matches the expected value
image

@uk-bolly
Copy link
Member

hi @Jason-Hendry

Thank you for the PR nice little tidy up. I was considering using the ansible facts for the GRUB_CMDLINE as a way to test also.
I have noted that it has failed pre-commit for lint layout on yaml.
If you could amended that am happy to merge this PR.

Many thanks again

uk-bolly

…MDLINE_LINUX before appending it to prevent duplication

Signed-off-by: Jason Hendry <[email protected]>
…s that do the same thing.

Signed-off-by: Jason Hendry <[email protected]>
@Jason-Hendry Jason-Hendry force-pushed the fix/ipv6disable-grub-mutliple-entries branch from 8f33536 to 4e67550 Compare November 22, 2023 03:46
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 tidy up thank you

@uk-bolly uk-bolly merged commit fa365a3 into ansible-lockdown:devel Nov 23, 2023
3 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