openshift / compliance-operator

Operator providing OpenShift cluster compliance checks
Apache License 2.0
110 stars 110 forks source link

Add instructions and check type to Rule object #703

Closed JAORMX closed 3 years ago

JAORMX commented 3 years ago

This enables folks browsing the rules to know what type of check will a specific rule do and how to manually audit it without having to run a scan.

Signed-off-by: Juan Antonio Osorio Robles jaosorior@redhat.com

JAORMX commented 3 years ago

Pull-request updated, HEAD is now 1fa6af5e520d1dc993ab2ee8e3ea74b5fecdd9a0

JAORMX commented 3 years ago

Pull-request updated, HEAD is now fb8e071364be2562b2619477f2fc5ba57f057e2b

JAORMX commented 3 years ago

/test all

jhrozek commented 3 years ago

I think this PR is good and can be merged, but I wonder if you considered using a different approach for the checkType - what if we instead looked at the list of enabled rules under xccdf-1.2:Profile and used the profile's <xccdf-1.2:platform idref="cpe:/o:redhat:openshift_container_platform_node:4"/> to determine if the check is platform or node? I guess we could even use a combination of the two.

JAORMX commented 3 years ago

I think this PR is good and can be merged, but I wonder if you considered using a different approach for the checkType - what if we instead looked at the list of enabled rules under xccdf-1.2:Profile and used the profile's <xccdf-1.2:platform idref="cpe:/o:redhat:openshift_container_platform_node:4"/> to determine if the check is platform or node? I guess we could even use a combination of the two.

@jhrozek I thought about doing that, but then figured that the approach wouldn't encompass all rules that would potentially be part of the data stream. There might be rules that we (for x or y reason) take out of a profile, but still would like to have customers be able to use. So, we wouldn't be able to accurately detect the type for those. By instead using the custom warning we use to detect the API path, we can know if it's node or Platform and encompass the case I mentioned.

jhrozek commented 3 years ago

I think this PR is good and can be merged, but I wonder if you considered using a different approach for the checkType - what if we instead looked at the list of enabled rules under xccdf-1.2:Profile and used the profile's <xccdf-1.2:platform idref="cpe:/o:redhat:openshift_container_platform_node:4"/> to determine if the check is platform or node? I guess we could even use a combination of the two.

@jhrozek I thought about doing that, but then figured that the approach wouldn't encompass all rules that would potentially be part of the data stream. There might be rules that we (for x or y reason) take out of a profile, but still would like to have customers be able to use. So, we wouldn't be able to accurately detect the type for those. By instead using the custom warning we use to detect the API path, we can know if it's node or Platform and encompass the case I mentioned.

Ah, good points. I didn't think about that, then your approach is better.

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JAORMX, jhrozek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/compliance-operator/blob/master/OWNERS)~~ [JAORMX,jhrozek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment