-
Notifications
You must be signed in to change notification settings - Fork 697
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
Bsi app 4.4 a20to21 #11997
Bsi app 4.4 a20to21 #11997
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-sigs/prow repository. |
🤖 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: |
applications/openshift/general/kube_descheduler_operator_exists/rule.yml
Outdated
Show resolved
Hide resolved
applications/openshift/general/kube_descheduler_lifecycle_policy/rule.yml
Outdated
Show resolved
Hide resolved
/test e2e-aws-ocp4-bsi |
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.
Not required right now, but it would be nice to have e2e tests.
Something similar to container_security_operator/tests .
name: yamlfile_value | ||
vars: | ||
ocp_data: "true" | ||
filepath: {{{ openshift_filtered_path('/apis/apiextensions.k8s.io/v1/customresourcedefinitions?limit=500', jqfilter) }}} |
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.
By the way, would it be more reliable and simpler to check for the operator's subscription instead of looking for its CRDs?
Check what container_security_operator_exists rule does.
content/applications/openshift/risk-assessment/container_security_operator_exists/rule.yml
Line 55 in cd0c045
filepath: '/apis/operators.coreos.com/v1alpha1/namespaces/openshift-operators/subscriptions/container-security-operator' |
/hold for test |
@yuumasato Could you please let us know if the PR still needs modification or is ready to merge? Thanks |
Rule upstream-ocp4-bsi-kube-descheduler-operator-exists failed with below error.
@ermeratos Could you please help me with the issue? Thanks |
8b49082
to
12f28c1
Compare
Verification failed with 4.17.0-0.nightly-2024-07-20-191204 + #519 code + #11997 code
|
@BhargaviGudi can you post the output of I suspect something changed in the structure of the subscription resource. Also: is container-security-operator-exists rule checked and if yes, does it fail? (this rule is not part of the bsi profile iirc, but should be in stig or others). |
Verification failed. |
@sluetze Hi, rule I have the following descheduler:
|
It seems like the path doesn't point to the right place as CO says it doesn't exist: |
Do you have the permission patch for the compliance operator active? |
with ComplianceAsCode/content#11997 the compliance-operator needs read access to kubedescheduler resources for the validation. We do this on clusterlevel, as the namespaces where the kubedescheduler can be deployed is configurable
applications/openshift/general/kube_descheduler_lifecycle_policy/rule.yml
Show resolved
Hide resolved
applications/openshift/general/kube_descheduler_lifecycle_policy/rule.yml
Outdated
Show resolved
Hide resolved
I also played a bit with the jqfilters here as well: |
Co-authored-by: Watson Yuuma Sato <wsato@redhat.com>
There are at least two more issues.
the descheduler doesn't act if the I ll review the complete rule again to ensure it does what we intent to. I am sorry for this. |
Code Climate has analyzed commit ab8fd59 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.4% (0.0% change). View more on Code Climate. |
Nice finds @sluetze
Still, I think the |
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 great to me.
I just have a small nit over what I think is a typo.
Description:
Notes / Rules for BSI APP4.4.A21 added.
Rationale:
As we have multiple customers asking for a BSI profile to be included in the compliance-operator, we are contributing a profile. To provide a better review process, the individual controle are implemented as separate PRs.