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

FIREWALL: Use rich rules instead of direct rules #69

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

aplopez
Copy link
Contributor

@aplopez aplopez commented Jul 17, 2024

Direct rules have been deprecated for some time now, so we switch to using rich rules.

However, we need a specific policy to which the rule will be applied. This policy is permanent (a requirement) and is created at setup, deleted at teardown, and remains active during the whole test. Only the rules are dynamic.

This works on RHEL 8 thourgh 10.

Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

Some inline comments, apart from that it looks good.

pytest_mh/utils/firewall.py Outdated Show resolved Hide resolved
pytest_mh/utils/firewall.py Show resolved Hide resolved
pytest_mh/utils/firewall.py Show resolved Hide resolved
pytest_mh/utils/firewall.py Show resolved Hide resolved
pytest_mh/utils/firewall.py Show resolved Hide resolved
Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for all the explanations!

Copy link
Contributor

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

I didn't review the code, but it looks ok conceptually.

As a rule of thumb - don't call any commands in init, use setup for that. This way, it is possible to call setup only when something from the utils is actually used (see mh_postpone_setup). And every change done must be reverted in teardown so we do not polute other tests.

@pbrezina
Copy link
Contributor

pbrezina commented Aug 2, 2024

Bump @thalman

Copy link
Collaborator

@thalman thalman left a comment

Choose a reason for hiding this comment

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

Sorry, I did the review earlier and forgot to ACK it. Approving now...

@pbrezina
Copy link
Contributor

pbrezina commented Aug 2, 2024

@aplopez Please, resolve conflict and we can merge.

@aplopez
Copy link
Contributor Author

aplopez commented Aug 5, 2024

@aplopez Please, resolve conflict and we can merge.

@pbrezina
Done

Direct rules have been deprecated for some time now, so we
switch to using rich rules.

However, we need a specific policy to which the rule will be
applied. This policy is permanent (a requirement) and is created
at start up, deleted at teardown, and remains active during the
whole test. Only the rules are dynamic.
@pbrezina pbrezina merged commit e3ebeea into next-actions:master Aug 7, 2024
4 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.

4 participants