-
Notifications
You must be signed in to change notification settings - Fork 758
NetworkManager package and PAM stat files fixes #14053
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
NetworkManager package and PAM stat files fixes #14053
Conversation
|
Hi @shanedell. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
e2261e7 to
8faf447
Compare
...em/network/network-wireless/wireless_software/wireless_disable_interfaces/ansible/shared.yml
Outdated
Show resolved
Hide resolved
|
/packit build |
1 similar comment
|
/packit build |
dodys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check the packit failures, it seems related to your changes to some of the password rules.
Also for Ubuntu, check the bash remediation, you might want to create an Ansible specific for Ubuntu as for this rule you don't necessarily need network-manager. Not all deployed systems might have it by default.
2e3a28f to
6307d3b
Compare
|
@dodys I am not sure what you are referring with the bash remediation. After building the |
You don't necessarily need network-manager for this specific rule, specially since people can use different network manager applications. The main point of the rule is to disable wireless interfaces and the ansible should match what we currently do in the bash remediation. |
d8deb79 to
4578983
Compare
|
@dodys I believe I should have gotten that change fixed here. |
...em/network/network-wireless/wireless_software/wireless_disable_interfaces/ansible/ubuntu.yml
Show resolved
Hide resolved
| # complexity = low | ||
| # disruption = medium | ||
|
|
||
| - name: Find wireless marker directories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The convention is to add {{{ rule_title }}} - to the task name. That helps identifying the purpose of the task in the context of whole profile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jan-cerny I added this to both the shared.yml and ubuntu.yml. I did notice in some of the other ansible code this isn't added, would this be a possible future PR you would be willing to accept?
I also noticed some different formatting differences, such as 4 spaces instead of 2 or using yes and no in some places then true and false in others. I believe ansible-lint usually uses 2 and prefers the use of true and false. This one isn't a huge deal but if its something wanting to be consistent with I would be willing to do a PR to do this.
- Fixed NetworkManager package issue for ubuntu2204 and ubuntu2404. - ubuntu didn't need Network manager so this created a custom wirless_disable_interfaces for ubuntu for ansible, similar to how it was done in bash. - Resolved issue where using the same register variable outside and inside the block were causing failures. - Add rule_title to all tasks for wireless_disable_interfactes ansible. Signed-off-by: Shane Dell <[email protected]>
4578983 to
8c5d2d0
Compare
|
@dodys Can you please check this again? |
dodys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
NetworkManager package and PAM stat files fixes
Description:
Rationale:
When testing the Ansible playbook for ubuntu2404 I ran into two issues:
NetworkManager, however for ubuntu2204 and ubuntu2404 the correct package name isnetwork-manager. This was noted in the product files, it just was not being overriden.result_pam_file_presentwas being used for both inside and outside of a block it would cause strange errors.Fixes NetworkManager package not overriden for ubuntu 22.04 and 24.04 #14038