content
content copied to clipboard
Fix #11130 -- respect different package names for (NetworkManager/network-manager) in ansible plays
See #11130
Hi @benhosmereop. Thanks for your PR.
I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
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/test-infra repository.
Start a new ephemeral environment with changes proposed in this pull request:
rhel8 (from CTF) Environment (using Fedora as testing environment)
This datastream diff is auto generated by the check Compare DS/Generate Diff
Click here to see the full diff
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_wireless_disable_interfaces' differs.
--- xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
+++ xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
@@ -25,7 +25,7 @@
name: '{{ item }}'
state: present
with_items:
- - NetworkManager
+ - ''
tags:
- CCE-83501-7
- DISA-STIG-RHEL-08-040110
Sorry, I'm on the street now but afaik the old fix was fine. It would check if you had network-Manager installed before running the command, instead of installing network-Manager to run the command. We just didn't catch the pull request that altered it to install network manager :/ I will try to get your fix and something to prevent the case I've mentioned together. And do note that I really appreciate your help and contribution, I was just trying to make sure we get it right before merging and creating a new issue. At no point I was preventing your contribution.
I wil let you know of the progress
On Fri, Sep 22, 2023, 20:08 benhosmereop @.***> wrote:
@.**** commented on this pull request.
In linux_os/guide/system/network/network-wireless/wireless_software/wireless_disable_interfaces/ansible/shared.yml https://github.com/ComplianceAsCode/content/pull/11133#discussion_r1334688286 :
@@ -9,7 +9,8 @@ name: "{{ item }}" state: present with_items:
- NetworkManager
- #- NetworkManager
- "{{{ platform_package_overrides.NetworkManager }}}"
Thanks for clarifying that @dodys https://github.com/dodys. I think we both agree that surprising users in a negative way is never good. Why not remove the ansible remediations entirely? Why were they there in the first place if they don't work?
In some use-cases the BASH doesn't work. I'll offer how I came here myself: I'm using packer as part of a larger baseline project. In packer, the BASH scripts fail and are unusable. However, my customer is already heavily invested in Ansible, so I chose to pursue that.
BASH is a great solution, but not for everyone.
— Reply to this email directly, view it on GitHub https://github.com/ComplianceAsCode/content/pull/11133#discussion_r1334688286, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMBZCHRHGWPCOC64UD4QHLX3XH3FANCNFSM6AAAAAA5BSYNTQ . You are receiving this because you were mentioned.Message ID: @.***>
Code Climate has analyzed commit 8bd560a9 and detected 0 issues on this pull request.
The test coverage on the diff in this pull request is 100.0% (50% is the threshold).
This pull request will bring the total coverage in the repository to 56.8% (0.0% change).
View more on Code Climate.
@marcusburghardt you might want to follow up on this bug/PR as this is not about Ubuntu anymore. There is still missing some changes to the platforms, or this will apply to Ubuntu incorrectly.
Some changes are necessary in this PR to not break Ubuntu. Ideally, a separate Ansible remediation for Ubuntu should be created in alignment to ubuntu.sh. I just noticed that the author account was deleted on GitHub, so I am also closing this PR.