content icon indicating copy to clipboard operation
content copied to clipboard

Fix #11130 -- respect different package names for (NetworkManager/network-manager) in ansible plays

Open ghost opened this issue 2 years ago • 5 comments

See #11130

ghost avatar Sep 21 '23 13:09 ghost

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.

openshift-ci[bot] avatar Sep 21 '23 13:09 openshift-ci[bot]

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment) Open in Gitpod

Fedora Testing Environment Open in Gitpod

Oracle Linux 8 Environment Open in Gitpod

github-actions[bot] avatar Sep 21 '23 13:09 github-actions[bot]

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

github-actions[bot] avatar Sep 21 '23 13:09 github-actions[bot]

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: @.***>

dodys avatar Sep 22 '23 18:09 dodys

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.

qlty-cloud-legacy[bot] avatar Sep 25 '23 07:09 qlty-cloud-legacy[bot]

@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.

dodys avatar Apr 16 '24 19:04 dodys

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.

marcusburghardt avatar Apr 19 '24 12:04 marcusburghardt