Skip to content
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

OCPBUGS-6780: Add a test to expose cleanup issue with SSBs #295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhmdnd
Copy link

@rhmdnd rhmdnd commented Apr 15, 2023

When you remove a profile from a ScanSettingBinding after it's already
been run, the Compliance Operator doesn't clean it up.

This commit adds a test to expose that.

When you remove a profile from a ScanSettingBinding after it's already
been run, the Compliance Operator doesn't clean it up.

This commit adds a test to expose that.
@openshift-ci-robot
Copy link
Collaborator

@rhmdnd: This pull request references Jira Issue OCPBUGS-6780, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

When you remove a profile from a ScanSettingBinding after it's already
been run, the Compliance Operator doesn't clean it up.

This commit adds a test to expose that.

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.

@openshift-ci
Copy link

openshift-ci bot commented Apr 15, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhmdnd

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhmdnd rhmdnd requested review from jhrozek and removed request for xiaojiey April 15, 2023 02:12
@rhmdnd
Copy link
Author

rhmdnd commented Apr 15, 2023

The assertions in the test still need some work to make sure we're checking the scan properly after updating the binding.

Throwing what I have in review for now.

}

// We might need a different utility to rerun a suite instead of scan
err = f.ReRunScan(scanSettingBinding.Name, f.OperatorNamespace)
Copy link
Author

@rhmdnd rhmdnd Apr 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails because the scan name isn't the binding name. Need to find a better way to assert this...

Or maybe we just go right into the assertions below about the scans. I guess it depends on where the cleanup happens. If it happens in the SSB controller, we probably won't need to rescan.

I'm open to recommendations here.

@@ -1451,3 +1471,28 @@ func (f *Framework) AssertHasCheck(suiteName, scanName string, check compv1alpha

return nil
}

func (f *Framework) ReRunScan(scanName, namespace string) error {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will conflict with a couple other patches I have that need this. Just a heads up for the rebase.

@xiaojiey
Copy link
Collaborator

/hold for test

@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

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.

@xiaojiey
Copy link
Collaborator

Below is also the steps to reproduce:

$ oc compliance bind -N test profile/ocp4-cis profile/ocp4-cis-node
Creating ScanSettingBinding test
$ oc get suite
NAME   PHASE   RESULT
test   DONE    NON-COMPLIANT
###delete one profile, and retrigger the scan:
$ oc patch ssb test --type='json' -p='[{"op": "remove", "path": "/profiles/1"}]'
scansettingbinding.compliance.openshift.io/test patched
$ oc get ssb -o yaml
apiVersion: v1
items:
- apiVersion: compliance.openshift.io/v1alpha1
  kind: ScanSettingBinding
  metadata:
    creationTimestamp: "2023-05-17T02:13:46Z"
    generation: 2
    name: test
    namespace: openshift-compliance
    resourceVersion: "38577"
    uid: 5d042f04-2a9b-4e2d-b566-8cbab0c22757
  profiles:
  - apiGroup: compliance.openshift.io/v1alpha1
    kind: Profile
    name: ocp4-cis
  settingsRef:
    apiGroup: compliance.openshift.io/v1alpha1
    kind: ScanSetting
    name: default
  status:
    conditions:
    - lastTransitionTime: "2023-05-17T02:13:46Z"
      message: The scan setting binding was successfully processed
      reason: Processed
      status: "True"
      type: Ready
    outputRef:
      apiGroup: compliance.openshift.io
      kind: ComplianceSuite
      name: test
kind: List
metadata:
  resourceVersion: ""
$ oc get suite -w
NAME   PHASE   RESULT
test   DONE    NON-COMPLIANT
$ oc get scan
NAME                   PHASE   RESULT
ocp4-cis               DONE    NON-COMPLIANT
ocp4-cis-node-master   DONE    COMPLIANT
ocp4-cis-node-worker   DONE    COMPLIANT
$ oc compliance rerun-now scansettingbinding test
Rerunning scans from 'test': ocp4-cis
Re-running scan 'openshift-compliance/ocp4-cis'
$ oc get scan -w
NAME                   PHASE     RESULT
ocp4-cis               RUNNING   NOT-AVAILABLE
ocp4-cis-node-master   DONE      COMPLIANT
ocp4-cis-node-worker   DONE      COMPLIANT
ocp4-cis               AGGREGATING   NOT-AVAILABLE
ocp4-cis               AGGREGATING   NOT-AVAILABLE
ocp4-cis               DONE          NON-COMPLIANT
$ oc get ccr -l compliance.openshift.io/scan-name=ocp4-cis-node-master | head
NAME                                                                  STATUS   SEVERITY
ocp4-cis-node-master-etcd-unique-ca                                   PASS     medium
ocp4-cis-node-master-file-groupowner-cni-conf                         PASS     medium
ocp4-cis-node-master-file-groupowner-controller-manager-kubeconfig    PASS     medium
ocp4-cis-node-master-file-groupowner-etcd-data-dir                    PASS     medium
ocp4-cis-node-master-file-groupowner-etcd-data-files                  PASS     medium
ocp4-cis-node-master-file-groupowner-etcd-member                      PASS     medium
ocp4-cis-node-master-file-groupowner-etcd-pki-cert-files              PASS     medium
ocp4-cis-node-master-file-groupowner-ip-allocations                   PASS     medium
ocp4-cis-node-master-file-groupowner-kube-apiserver                   PASS     medium

@xiaojiey
Copy link
Collaborator

/unhold

Copy link

openshift-ci bot commented Jul 19, 2024

@rhmdnd: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/go-build 3c6b02c link true /test go-build
ci/prow/verify 3c6b02c link true /test verify
ci/prow/unit 3c6b02c link true /test unit
ci/prow/e2e-rosa 3c6b02c link true /test e2e-rosa
ci/prow/e2e-aws-serial 3c6b02c link true /test e2e-aws-serial
ci/prow/e2e-aws-parallel 3c6b02c link true /test e2e-aws-parallel

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants