CMP-4283: Enable all XCCDF groups when TP extends a profile#1199
CMP-4283: Enable all XCCDF groups when TP extends a profile#1199yuumasato wants to merge 4 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuumasato The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The ProfileBundle ends up looking like this: |
|
🤖 To deploy this PR, run the following command: |
| pb.SetAnnotations(annotations) | ||
|
|
||
| // Update the ProfileBundle with the new annotation | ||
| if err := pcfg.Client.Update(context.TODO(), pb); err != nil { |
There was a problem hiding this comment.
Should this be Patch to avoid updating the entire object?
|
@yuumasato: This pull request references CMP-4283 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
564c3e0 to
165141d
Compare
| // Extract all XCCDF Group IDs from the datastream and store in ProfileBundle annotation | ||
| if err := extractAndStoreXCCDFGroups(contentDom, pb, pcfg); err != nil { | ||
| log.Error(err, "Failed to extract XCCDF groups") | ||
| // Don't fail the whole parse if group extraction fails |
There was a problem hiding this comment.
Should we surface this error in case the user needs to use those rules, and fail the bundle as an error state? or if we do not want to fail entire profilebundle we could in TailoredProfileToXML, if the annotation is missing AND the TP extends a profile, enable groups based on a query of the live datastream or a sentinel "enable all groups" wildcard.
There was a problem hiding this comment.
We could have have the ProfileBundle in error state.
Another alternative is to have the tailoring controller extract the groups at creation time, rather than Profile Bundle creation.
That would avoid us having the annotation in PB.
There was a problem hiding this comment.
I think I prefer the current approach, with an annotation in the ProfileBundle.
Vincent056
left a comment
There was a problem hiding this comment.
I think the PR looks good, do you think you could add a unit test to test the parser and also e2e tests for a TP with extends and rules out of the extended profile's enabled groups?
165141d to
9f3feac
Compare
|
🤖 To deploy this PR, run the following command: |
|
Verification failed with the compliance operator deployed using the image from the PR on OCP 4.22: Verification steps:
--> There is no ccr for rule upstream-rhcos4-kerberos-disable-no-keytab. The rule object itself exists on the cluster: |
9f3feac to
31da5ee
Compare
|
🤖 To deploy this PR, run the following command: |
|
Pre-merge verification PASS on OCP 4.22 + CO installed from this PR. ✔️ Premerge verification steps:
Bonus step: Both profilebundles have metadata annotations: |
|
@Vincent056 unit and e2e tests were added. |
Keep track of all XCCDF Groups in the ProfileBundle and always enable them when a TailoredProfile extends a Profile. This ensures that any rule that is enabled has its parent Group enabled as well, ensuring that OpenSCAP can get to the rule that was enbled.
31da5ee to
e3757b9
Compare
|
Also rebased to pick up on #1215 |
|
🤖 To deploy this PR, run the following command: |
|
@yuumasato: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
Keep track of all XCCDF Groups in the ProfileBundle and always enable them when a TailoredProfile extends a Profile.
This ensures that any rule that is enabled has its parent Group enabled as well, ensuring that OpenSCAP can get to the rule that was enbled.
If a TailoredProfile enables a rule that is not part of an XCCDF group enabled by the extended profile, the rule won't be enabled at all by OpenSCAP. This is because data stream traversal will stop at the disabled group.
Issue discovered when testing ComplianceAsCode/content#14665