-
Notifications
You must be signed in to change notification settings - Fork 710
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
OCP4: Drop not-arch from file_permissions_cni_conf #11756
Conversation
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
/test |
@rhmdnd: The
Use
In response to this:
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. |
/test 4.13-e2e-aws-ocp4-cis-node |
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.
Looks good - just two very minor nits on wording and one question about the expected result of the old rule.
|
||
description: |- | ||
{{{ describe_file_permissions(file="/etc/cni/net.d/*", perms="0600") }}} | ||
This rule is to for architectures other than s390x. |
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.
nit: This rule is for all architectures except s390x.
Or just remove to
.
|
||
description: |- | ||
{{{ describe_file_permissions(file="/etc/cni/net.d/*", perms="0600") }}} | ||
This rule is to for architectures on s390x. |
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.
Similar comment as above.
applications/openshift/master/file_permissions_cni_conf_s390x/tests/ocp4/e2e.yml
Show resolved
Hide resolved
ea89570
to
7e6db43
Compare
/test 4.13-e2e-aws-ocp4-cis-node |
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
</criteria> | ||
|
||
</definition> | ||
</def-group> |
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.
I think this is breaking some of the build jobs.
@@ -0,0 +1,11 @@ | |||
<def-group oval_version="5.11"> |
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.
Strange - it doesn't look like this was picked up in the latest CI run.
/hold for test |
@Vincent056 I found the rule ocp4-file-permissions-cni-conf get removed. But I didn't see ocp4-file-permissions-cni-conf-not-x390 neither. Is it expected? Thanks.
|
1aee491
to
acf0e0e
Compare
@@ -0,0 +1,3 @@ | |||
--- | |||
# This will fail until OpenShift 4.14 is released and used by CI. |
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.
Specifically - this will fail until https://issues.redhat.com//browse/OCPBUGS-22995 is landed in a 4.14 branch and released as a z-stream (which explains why this is still failing even though we're using 4.14 in CI).
Similar to what we did in ComplianceAsCode@ebbd895, is a workaround for ComplianceAsCode#9077 that makes the filter to put rule in the correct profile.
acf0e0e
to
a1acf03
Compare
Code Climate has analyzed commit a1acf03 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.3% (0.0% change). View more on Code Climate. |
Verification pass with 4.16.0-0.nightly-2024-03-25-100907 + #11756:
|
/unhold |
/lgtm |
/test 4.13-e2e-aws-ocp4-cis-node |
@Vincent056: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
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
The e2e failures are unrelated to this specific rule, and after checking the e2e logs the rule is failing as expected. |
Similar to what we did in ebbd895, is a workaround for #9077 that makes the filter to put rule in the correct profile.