-
Notifications
You must be signed in to change notification settings - Fork 685
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
Control for BSI APP.4.4 #11342
Control for BSI APP.4.4 #11342
Conversation
Hi @ermeratos. 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 Once the patch is verified, the new status will be reflected by the 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. |
/label bsi |
@ermeratos: The label(s) 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. |
/label BSI |
@ermeratos: The label(s) 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. |
/packit retest-failed |
e0d49ed
to
30f8cfa
Compare
sorry for the ongoing changes on this pr. The learning curve is quite steep. but this should have been it for this PR. We had a few issues with not working rules which was caused of not knowing, that we cant reference rules outside of the product we are working in. |
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 very good to me.
I have a few remarks but they are not blocking.
I'm okay if you would like to keep moving forward and address them later.
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.
The profile names for OCP and RHCOS are versioned.
So the version should be added to the file name once it is decided what to use.
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.
You mean moving the current bsi-node.profile
to e.g. bsi-node-2023.profile
?
Should we also add an additional bsi-node.profile
file that "extends" the bsi-node-2023 profile (just like cis / cis-node do for example)?
How do you usually handle changes, when e.g. the 2024 version comes out? Remove the 2023 version and only have one version in place?
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.
You mean moving the current
bsi-node.profile
to e.g.bsi-node-2023.profile
?
Yes.
Should we also add an additional
bsi-node.profile
file that "extends" the bsi-node-2023 profile (just like cis / cis-node do for example)?
Yes.
The non-versioned profiles are always pointing to the latest version.
This way folks who want to stay on a specific version can use bsi-2022
and bsi-node-2022
, for example. And folks who want to keep "rolling" to the latest version can use bsi
and bsi-node
.
How do you usually handle changes, when e.g. the 2024 version comes out? Remove the 2023 version and only have one version in place?
The support for versioned profiles is quite new, so we haven't gone through a profile version update yet.
But I think that update approach will depend on the lifecycle of the policy and the transition period between versions.
Is an old version immediately deprecated a new release is out? Is there a transition period?
Regardless, I can imagine that the profile for and old version will exist and be shipped for a few releases until it is removed. So that people using them can move and adapt to the new version.
We still got a failing test on this PR (testing-farm:centos-stream-8-x86_64). Is this related to our code changes or a false positive @yuumasato? |
I don't think the failure is related to this PR. |
7cd6933
to
4321fcb
Compare
/hold for test |
Verification passed with 4.14.7 + compliance-operator from code + content from PR #11342 Scenario 1: ocp4-bsi and ocp4-bsi-node
Scenario 2: upstream-rhcos4-bsi
|
/unhold |
/ok-to-test |
Code Climate has analyzed commit 4321fcb 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 58.5% (0.0% change). View more on Code Climate. |
I'm merging the PR since the tests passed. |
Description:
Control/foundation for BSI APP.4.4 rules added
Rationale:
As we have multiple customers asking for a bsi profile to be included in the compliance-operator, we are contributing a profile. We start with a skeleton profile and will subsequently add more rules to this profile until we addressed all/most concerns