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

Add profile aliases for OpenShift versioned profiles #11241

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

rhmdnd
Copy link
Collaborator

@rhmdnd rhmdnd commented Nov 2, 2023

We've implement profile versioning for some of the OpenShift and RHCOS
profiles. However, users that have ScanSettingBindings that reference
profiles like ocp4-cis can still be affected by rolling updates to
that profile. For example, when we implement support for CIS OpenShift
1.5.0, the ocp4-cis profile will automatically update to the rules for
that profile. This is how we've historically maintained profiles.

Now that we have a versioning mechanism and we're using it, we can give
users the ability to pin to a specific version of a profile.

This commit duplicates the profiles and names them according to their
version. When a user wants to pin to a specific version of a profile,
they can use ocp4-cis-1.4.0 to run only the CIS OpenShift 1.4.0
rules, and they won't be affected by changes that implement the 1.5.0
profile when that is supported.

This change doesn't change the functionality of the operator or the
profiles, but gives users more flexibility for pinning to specific
profile versions.

@rhmdnd rhmdnd added the OpenShift OpenShift product related. label Nov 2, 2023
Copy link

github-actions bot commented Nov 2, 2023

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

In general LGTM.
@rhmdnd How about making the non-versioned profiles extend the versioned profiles in this PR? This way we ensure there is only one source of truth for the profiles, and don't risk missing changes.

So profile cis-node would extend cis-node-1.4.0, and when cis-node-1.5.0.profile exists, we just need to update the Profile title, description, and extends.

Copy link
Member

Choose a reason for hiding this comment

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

I briefly tested that OpenSCAP is able to digest profiles with extra dots at the end.
SCAPVAL also seems accepts this ID as valid, not that we care that much about for OCP content, but it is good to know.

@rhmdnd rhmdnd force-pushed the ocp-profile-version-aliases branch from 0232cd5 to 6e84fcc Compare November 9, 2023 22:52
@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 9, 2023

In general LGTM. @rhmdnd How about making the non-versioned profiles extend the versioned profiles in this PR? This way we ensure there is only one source of truth for the profiles, and don't risk missing changes.

So profile cis-node would extend cis-node-1.4.0, and when cis-node-1.5.0.profile exists, we just need to update the Profile title, description, and extends.

Good suggestion.

I noticed I couldn't supply a list of profiles where we have a single profile that extends multiple. Also - the selections was a necessary argument for each profile. I was thinking that would get inherited, too.

@yuumasato
Copy link
Member

I noticed I couldn't supply a list of profiles where we have a single profile that extends multiple. Also - the selections was a necessary argument for each profile. I was thinking that would get inherited, too.

I have filed #11268 to overcome this limitation and avoid repeating ourselves.

@yuumasato
Copy link
Member

I noticed I couldn't supply a list of profiles where we have a single profile that extends multiple. Also - the selections was a necessary argument for each profile. I was thinking that would get inherited, too.

I have filed #11268 to overcome this limitation and avoid repeating ourselves.

The PR is merged, the non-versioned profiles can now have no selections and just an extends.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 13, 2023

I noticed I couldn't supply a list of profiles where we have a single profile that extends multiple. Also - the selections was a necessary argument for each profile. I was thinking that would get inherited, too.

I have filed #11268 to overcome this limitation and avoid repeating ourselves.

The PR is merged, the non-versioned profiles can now have no selections and just an extends.

Thanks for this. I removed the selections from the versionless profiles.

I'm still curious about setting extends multiple times, since it doesn't support a list.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 28, 2023

OCP CI should look better after ComplianceAsCode/compliance-operator#485 merges.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 28, 2023

/test e2e-aws-ocp4-cis
/test e2e-aws-ocp4-cis-node
/test e2e-aws-ocp4-e8
/test e2e-aws-ocp4-high
/test e2e-aws-ocp4-high-node
/test e2e-aws-ocp4-moderate
/test e2e-aws-ocp4-moderate-node
/test e2e-aws-ocp4-pci-dss
/test e2e-aws-ocp4-pci-dss-node
/test e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig-node
/test e2e-aws-rhcos4-e8
/test e2e-aws-rhcos4-high
/test e2e-aws-rhcos4-moderate
/test e2e-aws-rhcos4-stig

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

/test e2e-aws-ocp4-cis
/test e2e-aws-ocp4-cis-node

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

Latest CIS runs failed because we're waiting on #11207 to merge, but that's blocked on testing farm failures we're still investigating.

@vojtapolasek vojtapolasek added this to the 0.1.72 milestone Nov 29, 2023
@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

Rebased to see if we can get a better CI run with the fixes from 11207.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

/test e2e-aws-ocp4-cis
/test e2e-aws-ocp4-cis-node

@xiaojiey
Copy link
Collaborator

@rhmdnd Not sure if I am testing it in a correct way. I create a new profile.compliance manually but the suite using it will stuck at LAUNCHING phase. Could you please help to check? Thanks.
I tried create a new profile.compliance by below steps:

  1. oc get profile.compliance upstream-ocp4-cis -o yaml > ocp4-cis-1.4.0.yaml
  2. Edit the metadata.name to upstream-ocp4-cis -1.4.0 in ocp4-cis-1.4.0.yaml and try to apply the yaml file.
  3. Create a ssb through the oc compliance bind -N test profile/upstream-ocp4-cis -1.4.0
  4. Check the suite status, and find it will stuck at LAUNCHING phase.

@xiaojiey
Copy link
Collaborator

Sorry, above is a naming rule issue. Use a name "upstream-ocp4-cis-1-4-0" instead, it works.
verification pass with 4.15.0-0.nightly-2023-11-28-101923 + code in the PR:

$ oc apply -f upstream-ocp4-cis.yaml
profile.compliance.openshift.io/upstream-ocp4-cis-1-4-0 created
$ oc compliance bind -N testx profile/upstream-ocp4-cis-1-4-0
Creating ScanSettingBinding testx
$ oc get suite -w
NAME    PHASE         RESULT
test    LAUNCHING     NOT-AVAILABLE
test1   RUNNING       NOT-AVAILABLE
test2   RUNNING       NOT-AVAILABLE
testx   AGGREGATING   NOT-AVAILABLE
testx   DONE          NON-COMPLIANT
testx   DONE          NON-COMPLIANT

@xiaojiey
Copy link
Collaborator

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label Nov 29, 2023
We've implement profile versioning for some of the OpenShift and RHCOS
profiles. However, users that have ScanSettingBindings that reference
profiles like `ocp4-cis` can still be affected by rolling updates to
that profile. For example, when we implement support for CIS OpenShift
1.5.0, the `ocp4-cis` profile will automatically update to the rules for
that profile. This is how we've historically maintained profiles.

Now that we have a versioning mechanism and we're using it, we can give
users the ability to pin to a specific version of a profile.

This commit extends the profiles and names them according to their
version. When a user wants to pin to a specific version of a profile,
they can use `ocp4-cis-1-4` to run only the CIS OpenShift 1.4.0
rules, and they won't be affected by changes that implement the 1.5.0
profile when that is supported.

This change doesn't change the functionality of the operator or the
profiles, but gives users more flexibility for pinning to specific
profile versions.
@xiaojiey
Copy link
Collaborator

/hold for retest

@openshift-ci openshift-ci bot added the do-not-merge/hold Used by openshift-ci-robot bot. label Nov 29, 2023
@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

/test e2e-aws-ocp4-cis
/test e2e-aws-ocp4-cis-node

Copy link

codeclimate bot commented Nov 29, 2023

Code Climate has analyzed commit 9296538 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 58.5%.

View more on Code Climate.

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

Green OCP4 CIS run, kicking off the rest of the tests.

/test e2e-aws-ocp4-e8
/test e2e-aws-ocp4-high
/test e2e-aws-ocp4-high-node
/test e2e-aws-ocp4-moderate
/test e2e-aws-ocp4-moderate-node
/test e2e-aws-ocp4-pci-dss
/test e2e-aws-ocp4-pci-dss-node
/test e2e-aws-ocp4-stig
/test e2e-aws-ocp4-stig-node
/test e2e-aws-rhcos4-e8
/test e2e-aws-rhcos4-high
/test e2e-aws-rhcos4-moderate
/test e2e-aws-rhcos4-stig

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 29, 2023

/test e2e-aws-rhcos4-e8

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 30, 2023

/retest

Copy link

openshift-ci bot commented Nov 30, 2023

@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/e2e-aws-rhcos4-e8 9296538 link true /test e2e-aws-rhcos4-e8
ci/prow/e2e-aws-rhcos4-moderate 9296538 link true /test e2e-aws-rhcos4-moderate
ci/prow/e2e-aws-rhcos4-high 9296538 link true /test e2e-aws-rhcos4-high

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

@xiaojiey
Copy link
Collaborator

Verification pass with 4.15.0-0.nightly-2023-11-28-101923 + content in the PR:

$ oc get profile.compliance
NAME                                      AGE   VERSION
ocp4-cis                                  44m   1.4.0
ocp4-cis-node                             44m   1.4.0
ocp4-e8                                   44m   
ocp4-high                                 44m   Revision 4
ocp4-high-node                            44m   Revision 4
ocp4-moderate                             44m   Revision 4
ocp4-moderate-node                        44m   Revision 4
ocp4-nerc-cip                             44m   
ocp4-nerc-cip-node                        44m   
ocp4-pci-dss                              44m   3.2.1
ocp4-pci-dss-node                         44m   3.2.1
ocp4-stig                                 44m   V1R1
ocp4-stig-node                            44m   V1R1
rhcos4-anssi-bp28-enhanced                44m   
rhcos4-anssi-bp28-high                    44m   
rhcos4-anssi-bp28-intermediary            44m   
rhcos4-anssi-bp28-minimal                 44m   
rhcos4-e8                                 44m   
rhcos4-high                               44m   Revision 4
rhcos4-moderate                           44m   Revision 4
rhcos4-nerc-cip                           44m   
rhcos4-stig                               44m   V1R1
upstream-ocp4-cis                         39m   1.4.0
upstream-ocp4-cis-1-4                     39m   1.4.0
upstream-ocp4-cis-node                    39m   1.4.0
upstream-ocp4-cis-node-1-4                39m   1.4.0
upstream-ocp4-e8                          39m   
upstream-ocp4-high                        39m   Revision 4
upstream-ocp4-high-node                   39m   Revision 4
upstream-ocp4-high-node-rev-4             39m   Revision 4
upstream-ocp4-high-rev-4                  39m   Revision 4
upstream-ocp4-moderate                    39m   Revision 4
upstream-ocp4-moderate-node               39m   Revision 4
upstream-ocp4-moderate-node-rev-4         39m   Revision 4
upstream-ocp4-moderate-rev-4              39m   Revision 4
upstream-ocp4-nerc-cip                    39m   
upstream-ocp4-nerc-cip-node               39m   
upstream-ocp4-pci-dss                     39m   3.2.1
upstream-ocp4-pci-dss-3-2                 39m   3.2.1
upstream-ocp4-pci-dss-node                39m   3.2.1
upstream-ocp4-pci-dss-node-3-2            39m   3.2.1
upstream-ocp4-stig                        39m   V1R1
upstream-ocp4-stig-node                   39m   V1R1
upstream-ocp4-stig-node-v1r1              39m   V1R1
upstream-ocp4-stig-v1r1                   39m   V1R1
upstream-rhcos4-anssi-bp28-enhanced       39m   
upstream-rhcos4-anssi-bp28-high           39m   
upstream-rhcos4-anssi-bp28-intermediary   39m   
upstream-rhcos4-anssi-bp28-minimal        39m   
upstream-rhcos4-e8                        39m   
upstream-rhcos4-high                      39m   Revision 4
upstream-rhcos4-high-rev-4                39m   Revision 4
upstream-rhcos4-moderate                  39m   Revision 4
upstream-rhcos4-moderate-rev-4            39m   Revision 4
upstream-rhcos4-nerc-cip                  39m   
upstream-rhcos4-stig                      39m   V1R1
upstream-rhcos4-stig-v1r1                 39m   V1R1
$ oc compliance bind -N test profile/upstream-ocp4-cis-node-1-4
Creating ScanSettingBinding test
$ oc get suite -w
NAME   PHASE     RESULT
test   RUNNING   NOT-AVAILABLE
test   RUNNING   NOT-AVAILABLE
test   AGGREGATING   NOT-AVAILABLE
test   AGGREGATING   NOT-AVAILABLE
test   DONE          NON-COMPLIANT
test   DONE          NON-COMPLIANT

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 30, 2023

Looks like the following rules are failing on the second and third scans:

helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_core_bpf_jit_harden rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_all_accept_ra rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_all_accept_redirects rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_default_accept_ra rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_default_accept_redirects rule didn't match. Expected 'PASS', Got 'FAIL' 

Copy link
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you @rhmdnd

@rhmdnd
Copy link
Collaborator Author

rhmdnd commented Nov 30, 2023

Looks like the following rules are failing on the second and third scans:

helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_core_bpf_jit_harden rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_all_accept_ra rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_all_accept_redirects rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_default_accept_ra rule didn't match. Expected 'PASS', Got 'FAIL'
helpers.go:815: E2E-FAILURE: The expected result for the sysctl_net_ipv6_conf_default_accept_redirects rule didn't match. Expected 'PASS', Got 'FAIL' 

These failures are being tracked in https://issues.redhat.com/browse/OCPBUGS-19690 and are unrelated to the changes in this PR.

@xiaojiey
Copy link
Collaborator

xiaojiey commented Dec 1, 2023

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label Dec 1, 2023
@rhmdnd rhmdnd merged commit 017ab39 into ComplianceAsCode:master Dec 1, 2023
47 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenShift OpenShift product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants