-
Notifications
You must be signed in to change notification settings - Fork 27
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
Remove product validation in ScanSettingBinding #489
Conversation
/hold for test |
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.
Is this something you really wanna do? I recall us having limitations on the scans we can launch from a specific scan setting binding, and this might become problematic when scheduling them
hi @JAORMX it's good to hear from you! I wonder if you still remember what the limitations were? I spent some time digging into the code, but couldn't figure out why it was designed this way. And I also did some testing with this PR to run scans with
All the scans were launched and working as expected, tho I will do more testing on it before making this change. |
Could be that we no longer have that! |
4eb3d86
to
b469c24
Compare
we are affected by ComplianceAsCode/content@6343659:
We might need to update the OCP version:
|
Per https://issues.redhat.com/browse/OCPBUGS-11856, seems it was fixed on 4.14 and higher versions |
tests/e2e/serial/main_test.go
Outdated
@@ -232,7 +232,7 @@ func TestSuiteScan(t *testing.T) { | |||
}, | |||
}, | |||
ID: "xccdf_org.ssgproject.content_rule_coreos_vsyscall_kernel_argument", | |||
Status: compv1alpha1.CheckResultInfo, | |||
Status: compv1alpha1.CheckResultFail, |
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.
Does this need to be pulled out into it's own PR since ComplianceAsCode/content#11329 landed?
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.
#490 create a PR here
@Vincent056 I retested the PR, I still got the same result for moderate profiles.
|
🤖 To deploy this PR, run the following command:
|
/hold for test |
Verification failed with 4.15.0-0.nightly-2024-02-12-213938 + compliance-operator from PR #489 Verification failed for moderate profiles.
|
This commit remove product validation in ScanSettingBinding so we can launch both rhcos4 and ocp4-node scan in one SSB.
🤖 To deploy this PR, run the following command:
|
Unable to create ssb with both rhcos4 and ocp4. Observing below error.
@Vincent056 Could you please help me check this issue? Thanks |
could you test without using |
also, |
|
Verification passed with 4.16.0-0.nightly-2024-02-29-062601 + compliance-operator from PR #489 1.Install CO from #489 code
|
/unhold |
/label qe-approved |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BhargaviGudi, JAORMX, Vincent056 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:
Approvers can indicate their approval by writing |
…g_kube_audit E2e tests have been flaky because of the file_permissions_var_log_kube_audit rule. This is because there is a bug in the API-server in old versions of OCP[1][2]. For now, we'll just check that the scan is not inconsistent until we upgrade to a version that has the fix. [1]https://bugzilla.redhat.com/show_bug.cgi?id=2001442 [2]ComplianceAsCode/content@6343659
🤖 To deploy this PR, run the following command:
|
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.
Only one comment on the e2e test. Otherwise, experimenting with this locally works as expected.
Thanks!
Since later OCP has addressed the bug in the API-server https://bugzilla.redhat.com/show_bug.cgi?id=2001442, we can remove the ResultInconsistent as the accept result.
🤖 To deploy this PR, run the following command:
|
@@ -183,27 +182,11 @@ func (r *ReconcileScanSettingBinding) Reconcile(ctx context.Context, request rec | |||
} | |||
} | |||
|
|||
scan, product, err := newCompScanFromBindingProfile(r, instance, profileObj, log) | |||
scan, _, err := newCompScanFromBindingProfile(r, instance, profileObj, log) |
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 like this is the only place where this is called, and since we're not using product
anymore we can probably clean it up in another patch.
/lgtm |
This commit removes product validation in ScanSettingBinding so we can launch both
rhcos4
andocp4-node
scan in one SSB.ex.