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-6827: ssb: Presume Platform scan if no scan type is given with annotations #195

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

jhrozek
Copy link

@jhrozek jhrozek commented Dec 19, 2022

Xiaojie noticed an issue in my previous PR to check for non-default roles, but I didn't have time to address
her feedback before the PR was merged.

Opening this PR as a way to track the comments. I'm not 100% sure that the PR is correct, so please treat as a WIP for now.

@jhrozek jhrozek changed the title ssb: Presume Platform scan if no scan type is given with annotations [WIP] ssb: Presume Platform scan if no scan type is given with annotations Dec 19, 2022
Copy link

@rhmdnd rhmdnd left a comment

Choose a reason for hiding this comment

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

Seems reasonable based on the context, but should we pull all relevant details into a bug report?

I'm a little unclear on the overall purpose and user impact.

@jhrozek jhrozek changed the title [WIP] ssb: Presume Platform scan if no scan type is given with annotations OCPBUGS-6827: ssb: Presume Platform scan if no scan type is given with annotations Feb 1, 2023
@openshift-ci-robot
Copy link
Collaborator

@jhrozek: This pull request references Jira Issue OCPBUGS-6827, which is invalid:

  • expected the bug to target the "4.13.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:

Xiaojie noticed an issue in my previous PR to check for non-default roles, but I didn't have time to address
her feedback before the PR was merged.

Opening this PR as a way to track the comments. I'm not 100% sure that the PR is correct, so please treat as a WIP for now.

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.

@jhrozek
Copy link
Author

jhrozek commented Feb 1, 2023

I pushed a new version, but I'll amend it one more time with trying to look at the profile that the TP extends.

@xiaojiey
Copy link
Collaborator

xiaojiey commented Feb 3, 2023

/hold for QE review

@xiaojiey
Copy link
Collaborator

xiaojiey commented Feb 6, 2023

Verification pass with 4.13.0-0.nightly-2023-02-03-145213 + code in the PR
#############Scenario 1: remediation could be applied for a custom mcp:
$ oc label node xiyuan-06-sx6t5-worker-a-hnc8n.c.openshift-qe.internal node-role.kubernetes.io/wscan=
node/xiyuan-06-sx6t5-worker-a-hnc8n.c.openshift-qe.internal labeled
$ oc create -f - <<EOF
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfigPool
metadata:
name: wscan
labels:
pools.operator.machineconfiguration.openshift.io/wscan: ""
spec:
machineConfigSelector:
matchExpressions:
- {key: machineconfiguration.openshift.io/role, operator: In, values: [worker,wscan]}
nodeSelector:
matchLabels:
node-role.kubernetes.io/wscan: ""
EOF
machineconfigpool.machineconfiguration.openshift.io/wscan created
$ oc get mcp -w
NAME CONFIG UPDATED UPDATING DEGRADED MACHINECOUNT READYMACHINECOUNT UPDATEDMACHINECOUNT DEGRADEDMACHINECOUNT AGE
master rendered-master-fb483d34474db31c547b351d5bbfd9e2 True False False 3 3 3 0 3h49m
worker rendered-worker-66e7870b76fa387357b40287e61bf219 True False False 3 3 3 0 3h49m
wscan 0 0 0 0 8s
wscan False True False 1 0 0 0 10s
worker rendered-worker-66e7870b76fa387357b40287e61bf219 True False False 2 2 2 0 3h49m
wscan False True False 1 0 0 0 15s
wscan rendered-wscan-66e7870b76fa387357b40287e61bf219 True False False 1 1 1 0 29s
^C
$ oc apply -f -<<EOFOF
apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSetting
metadata:
name: test
namespace: openshift-compliance
rawResultStorage:
nodeSelector:
node-role.kubernetes.io/master: ""
pvAccessModes:

  • ReadWriteOnce
    rotation: 3
    size: 1Gi
    tolerations:
  • effect: NoSchedule
    key: node-role.kubernetes.io/master
    operator: Exists
  • effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  • effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  • effect: NoSchedule
    key: node.kubernetes.io/memory-pressure
    operator: Exists
    roles:
  • wscan
    scanTolerations:
  • operator: Exists
    schedule: 0 1 * * *
    showNotApplicable: false
    strictNodeScan: true
    scanLimits: {
    "cpu": "150m",
    "memory": "512Mi"
    }
    debug: true
    autoApplyRemediations: true
    autoUpdateRemediations: true
    EOF
    scansetting.compliance.openshift.io/test created
    $ oc apply -f -<<EOF
    apiVersion: compliance.openshift.io/v1alpha1
    kind: TailoredProfile
    metadata:
    name: cis-wscan-tp
    spec:
    extends: ocp4-cis
    title: My modified nist profile with a custom value
    setValues:
    • name: ocp4-var-role-master
      value: wscan
      rationale: test for wscan nodes
    • name: ocp4-var-role-worker
      value: wscan
      rationale: test for wscan nodes
      description: cis-wscan-scan
      EOF
      tailoredprofile.compliance.openshift.io/cis-wscan-tp created
      $ oc apply -f -<<EOF
      apiVersion: compliance.openshift.io/v1alpha1
      kind: ScanSettingBinding
      metadata:
      name: my-ssb-r
      profiles:
    • name: ocp4-cis-node
      kind: Profile
      apiGroup: compliance.openshift.io/v1alpha1
    • name: cis-wscan-tp
      kind: TailoredProfile
      apiGroup: compliance.openshift.io/v1alpha1
      settingsRef:
      name: test
      kind: ScanSetting
      apiGroup: compliance.openshift.io/v1alpha1
      EOF
      scansettingbinding.compliance.openshift.io/my-ssb-r created
      $ oc get suite
      NAME PHASE RESULT
      my-ssb-r DONE NON-COMPLIANT
      $ oc get cr
      NAME STATE
      cis-wscan-tp-api-server-audit-log-maxsize Applied
      cis-wscan-tp-api-server-encryption-provider-cipher Applied
      cis-wscan-tp-api-server-encryption-provider-config Applied
      ocp4-cis-node-wscan-kubelet-enable-protect-kernel-defaults MissingDependencies
      ocp4-cis-node-wscan-kubelet-enable-protect-kernel-sysctl Applied
      ...
      ##############Scenario 2: the ssb will report Invalid when there is no tp for platform type remediation:
      $ oc compliance bind -N test -S test profile/ocp4-cis
      Creating ScanSettingBinding test
      $ oc get ssb test -o=jsonpath={.status} | jq -r
      {
      "conditions": [
      {
      "lastTransitionTime": "2023-02-06T06:07:17Z",
      "message": "The scanSetting references a non-default role, but either no tailored profile is set or the role variables are not set",
      "reason": "Invalid",
      "status": "False",
      "type": "Ready"
      }
      ]
      }

@xiaojiey
Copy link
Collaborator

xiaojiey commented Feb 6, 2023

/label qe-approved

@xiaojiey
Copy link
Collaborator

xiaojiey commented Feb 6, 2023

/unhold

@jhrozek
Copy link
Author

jhrozek commented Feb 6, 2023

I think I just bungled up the names of the scansettings and the roles. New patch was pushed and I only changed the e2e tests so I hope I can retain the qe-approved label.

@jhrozek
Copy link
Author

jhrozek commented Feb 7, 2023

/test e2e-aws
the cluster didn't install

@sheriff-rh
Copy link
Collaborator

Changelog looks good!

@jhrozek
Copy link
Author

jhrozek commented Feb 7, 2023

straight up regression doesn't need a px ack

@xiaojiey
Copy link
Collaborator

xiaojiey commented Feb 8, 2023

/jira refresh

@openshift-ci-robot
Copy link
Collaborator

@xiaojiey: This pull request references Jira Issue OCPBUGS-6827, which is invalid:

  • expected the bug to target the "4.13.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.

In response to this:

/jira refresh

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

xiaojiey commented Feb 8, 2023

/jira refresh

@openshift-ci-robot
Copy link
Collaborator

@xiaojiey: This pull request references Jira Issue OCPBUGS-6827, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.13.0) matches configured target version for branch (4.13.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @xiaojiey

In response to this:

/jira refresh

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 openshift-ci bot requested a review from xiaojiey February 8, 2023 01:32
@rhmdnd
Copy link

rhmdnd commented Feb 8, 2023

Opening this PR as a way to track the comments. I'm not 100% sure that the PR is correct, so please treat as a WIP for now.

Is this still true?

Copy link

@rhmdnd rhmdnd left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci
Copy link

openshift-ci bot commented Feb 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhrozek, 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

@openshift-merge-robot openshift-merge-robot merged commit 6f27cf6 into ComplianceAsCode:master Feb 9, 2023
@openshift-ci-robot
Copy link
Collaborator

@jhrozek: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-6827 has been moved to the MODIFIED state.

In response to this:

Xiaojie noticed an issue in my previous PR to check for non-default roles, but I didn't have time to address
her feedback before the PR was merged.

Opening this PR as a way to track the comments. I'm not 100% sure that the PR is correct, so please treat as a WIP for now.

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.

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.

6 participants