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

Make package installation for iptables and nftables mutually exclusive #11191

Conversation

teacup-on-rockingchair
Copy link
Contributor

Description:

  • Make package installation rules for iptables and nftables mutually exclusive

Rationale:

  • nftables related rule rely on nftables package being installed, same is valid for iptables related rules, but the package_installed rules do not reflect the conflict between the two approaches so remediation of one package_installed_ rule breaks the other dependant rules and vice versa

Review Hints:

  • Review hints here. Replace this text. Don't use the italics format!

  • Use this optional section to give any relevant information which could help the reviewer to more quickly and assertively understand and test the changes.

  • Good examples are useful commands, if it is better to review all commits together or in a suggested sequence, any relevant discussion in other PRs or issues, etc.

@openshift-ci
Copy link

openshift-ci bot commented Oct 11, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Oct 11, 2023
@github-actions
Copy link

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

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@vojtapolasek vojtapolasek added this to the 0.1.72 milestone Nov 29, 2023
Copy link
Member

@marcusburghardt marcusburghardt left a comment

Choose a reason for hiding this comment

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

There are valid cases where both packages are installed without problems. The problem is when conflicting services are enabled and this was already treated in the #10812. So I don't think we should create a mutually exclusive condition for iptables and nftables packages. Maybe these rules could be filtered out in the profile level.

@marcusburghardt marcusburghardt self-assigned this Dec 20, 2023
@marcusburghardt marcusburghardt modified the milestones: 0.1.72, 0.1.73 Jan 29, 2024
@marcusburghardt
Copy link
Member

@teacup-on-rockingchair , do you have plans for this PR?

@marcusburghardt marcusburghardt removed this from the 0.1.73 milestone Mar 6, 2024
@marcusburghardt
Copy link
Member

@teacup-on-rockingchair , do you have plans for this PR?

I believe that using the same approach as in #10812, where the applicability was treated in service level instead of package level, should make this PR good to be merged.

@marcusburghardt
Copy link
Member

ping @teacup-on-rockingchair : )

@teacup-on-rockingchair
Copy link
Contributor Author

ping @teacup-on-rockingchair : )

re :)

I am working on a solution that would use profile interactive variable for this problem.
For now will apply to SLE only , today I think I will be able to publish it at least as a draft I was just having doubts if to use this PR or new one.

@marcusburghardt
Copy link
Member

ping @teacup-on-rockingchair : )

re :)

I am working on a solution that would use profile interactive variable for this problem. For now will apply to SLE only , today I think I will be able to publish it at least as a draft I was just having doubts if to use this PR or new one.

Thanks for the update. All fine so. Take your time. :)

Regarding this PR, I believe it is almost ready to be merged with minor changes in the platform, as I mentioned here.

So, maybe it would be still valid to make these minor changes and merge this PR. Then you could open another PR to introduce this interactive variable. What do you think?

@teacup-on-rockingchair teacup-on-rockingchair marked this pull request as ready for review April 13, 2024 18:39
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Apr 13, 2024
Copy link

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11191
This image was built from commit: 77390db

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11191

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11191 make deploy-local

Copy link

codeclimate bot commented Apr 14, 2024

Code Climate has analyzed commit 77390db 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 59.2% (0.0% change).

View more on Code Climate.

@marcusburghardt
Copy link
Member

/packit build

@marcusburghardt marcusburghardt merged commit 6718a87 into ComplianceAsCode:master Apr 16, 2024
45 checks passed
@Mab879 Mab879 added this to the 0.1.73 milestone May 16, 2024
@Mab879 Mab879 added the Update Rule Issues or pull requests related to Rules updates. label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants