CMP-3385: Refactor test suite to group rules #54

rhmdnd · 2025-07-17T21:36:20Z

With the amount of profiles we're testing across OpenShift versions, we
experience a lot of duplicate failures if one rule breaks because it
will affect multiple profiles across multiple OpenShift versions.

To make testing easier to manage, we want to try building a single
tailored profile for all platform rules and another for node rules. Then
run all rules of the same type together. After that, we'll check the
outcome of each rule like we did.

The result should be that we only need to run a single test to exercise
all platform rules on a single OpenShift version. Instead of running
multiple platform profiles on each OpenShift version.

rhmdnd · 2025-07-17T21:38:16Z

e2e_test.go

-		var scanN int
-
-		for scanN = 2; scanN < 5; scanN++ {
-			var needsMoreRemediations bool


We'll need to add these back in since we definitely still want to test remediations. I only pulled them out here to simplify the overall test suite during the initial refactor.

Vincent056 · 2025-07-17T22:26:57Z

helpers.go

+	var platformRules []cmpv1alpha1.Rule
+	var nodeRules []cmpv1alpha1.Rule
+
+	for _, rule := range ruleList.Items {


I wonder if we also want to go through each profile and find all unique rules to categorize them, besides finding them directly from the profile bundle

Would that catch a different set of rules? I assumed (perhaps incorrectly) that looping through all available rules would result in a super set from the rules from each profile, since I think we ship rules that aren't included in any profile.

I think looping through all rules in the Bundle should be enough.

helpers.go

With the amount of profiles we're testing across OpenShift versions, we experience a lot of duplicate failures if one rule breaks because it will affect multiple profiles across multiple OpenShift versions. To make testing easier to manage, we want to try building a single tailored profile for all platform rules and another for node rules. Then run all rules of the same type together. After that, we'll check the outcome of each rule like we did. The result should be that we only need to run a single test to exercise all platform rules on a single OpenShift version. Instead of running multiple platform profiles on each OpenShift version.

rhmdnd · 2025-07-18T17:26:48Z

helpers.go

+		case cmpv1alpha1.CheckTypeNode:
+			nodeRules = append(nodeRules, rule)
+		default:
+			t.Logf("Skipping rule %s with unknown check type: %s", rule.Name, rule.CheckType)


Testing picked up a bunch of rules that don't have a type:

helpers.go:1360: Skipping rule e2e-accounts-restrict-service-account-tokens with unknown check type: helpers.go:1360: Skipping rule e2e-accounts-unique-service-account with unknown check type: helpers.go:1360: Skipping rule e2e-alert-receiver-configured with unknown check type: helpers.go:1360: Skipping rule e2e-configure-appropriate-network-policies with unknown check type: helpers.go:1360: Skipping rule e2e-configure-network-bandwidth with unknown check type: helpers.go:1360: Skipping rule e2e-etcd-backup with unknown check type: helpers.go:1360: Skipping rule e2e-file-groupowner-kubeconfig with unknown check type: helpers.go:1360: Skipping rule e2e-file-groupowner-proxy-kubeconfig with unknown check type: helpers.go:1360: Skipping rule e2e-file-owner-kubeconfig with unknown check type: helpers.go:1360: Skipping rule e2e-file-owner-proxy-kubeconfig with unknown check type: helpers.go:1360: Skipping rule e2e-file-permissions-kube-scheduler with unknown check type: helpers.go:1360: Skipping rule e2e-file-permissions-kubeconfig with unknown check type: helpers.go:1360: Skipping rule e2e-general-apply-scc with unknown check type: helpers.go:1360: Skipping rule e2e-general-configure-imagepolicywebhook with unknown check type: helpers.go:1360: Skipping rule e2e-general-default-namespace-use with unknown check type: helpers.go:1360: Skipping rule e2e-general-default-seccomp-profile with unknown check type: helpers.go:1360: Skipping rule e2e-general-namespace-separation with unknown check type: helpers.go:1360: Skipping rule e2e-general-namespaces-in-use with unknown check type: helpers.go:1360: Skipping rule e2e-general-network-separation with unknown check type: helpers.go:1360: Skipping rule e2e-general-node-separation with unknown check type: helpers.go:1360: Skipping rule e2e-kube-descheduler-podlifetime with unknown check type: helpers.go:1360: Skipping rule e2e-kubelet-disable-hostname-override with unknown check type: helpers.go:1360: Skipping rule e2e-liveness-readiness-probe-in-workload with unknown check type: helpers.go:1360: Skipping rule e2e-partition-for-var-log-kube-apiserver with unknown check type: helpers.go:1360: Skipping rule e2e-partition-for-var-log-oauth-apiserver with unknown check type: helpers.go:1360: Skipping rule e2e-partition-for-var-log-openshift-apiserver with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-least-privilege with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-limit-cluster-admin with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-limit-secrets-access with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-logging-del with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-logging-mod with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-logging-view with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-pod-creation-access with unknown check type: helpers.go:1360: Skipping rule e2e-rbac-wildcard-use with unknown check type: helpers.go:1360: Skipping rule e2e-scc-drop-container-capabilities with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-host-dir-volume-plugin with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-host-ports with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-ipc-namespace with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-net-raw-capability with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-network-namespace with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-privilege-escalation with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-privileged-containers with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-process-id-namespace with unknown check type: helpers.go:1360: Skipping rule e2e-scc-limit-root-containers with unknown check type: helpers.go:1360: Skipping rule e2e-secrets-consider-external-storage with unknown check type: helpers.go:1360: Skipping rule e2e-secrets-no-environment-variables with unknown check type: helpers.go:1360: Skipping rule e2e-version-detect-in-hypershift with unknown check type: helpers.go:1360: Skipping rule e2e-version-detect-in-ocp with unknown check type: helpers.go:1360: Skipping rule ocp4-accounts-restrict-service-account-tokens with unknown check type: helpers.go:1360: Skipping rule ocp4-accounts-unique-service-account with unknown check type: helpers.go:1360: Skipping rule ocp4-alert-receiver-configured with unknown check type: helpers.go:1360: Skipping rule ocp4-configure-appropriate-network-policies with unknown check type: helpers.go:1360: Skipping rule ocp4-configure-network-bandwidth with unknown check type: helpers.go:1360: Skipping rule ocp4-etcd-backup with unknown check type: helpers.go:1360: Skipping rule ocp4-file-groupowner-kubeconfig with unknown check type: helpers.go:1360: Skipping rule ocp4-file-groupowner-proxy-kubeconfig with unknown check type: helpers.go:1360: Skipping rule ocp4-file-owner-kubeconfig with unknown check type: helpers.go:1360: Skipping rule ocp4-file-owner-proxy-kubeconfig with unknown check type: helpers.go:1360: Skipping rule ocp4-file-permissions-kube-scheduler with unknown check type: helpers.go:1360: Skipping rule ocp4-file-permissions-kubeconfig with unknown check type: helpers.go:1360: Skipping rule ocp4-general-apply-scc with unknown check type: helpers.go:1360: Skipping rule ocp4-general-configure-imagepolicywebhook with unknown check type: helpers.go:1360: Skipping rule ocp4-general-default-namespace-use with unknown check type: helpers.go:1360: Skipping rule ocp4-general-default-seccomp-profile with unknown check type: helpers.go:1360: Skipping rule ocp4-general-namespace-separation with unknown check type: helpers.go:1360: Skipping rule ocp4-general-namespaces-in-use with unknown check type: helpers.go:1360: Skipping rule ocp4-general-network-separation with unknown check type: helpers.go:1360: Skipping rule ocp4-general-node-separation with unknown check type: helpers.go:1360: Skipping rule ocp4-kube-descheduler-podlifetime with unknown check type: helpers.go:1360: Skipping rule ocp4-kubelet-disable-hostname-override with unknown check type: helpers.go:1360: Skipping rule ocp4-liveness-readiness-probe-in-workload with unknown check type: helpers.go:1360: Skipping rule ocp4-partition-for-var-log-kube-apiserver with unknown check type: helpers.go:1360: Skipping rule ocp4-partition-for-var-log-oauth-apiserver with unknown check type: helpers.go:1360: Skipping rule ocp4-partition-for-var-log-openshift-apiserver with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-least-privilege with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-limit-cluster-admin with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-limit-secrets-access with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-logging-del with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-logging-mod with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-logging-view with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-pod-creation-access with unknown check type: helpers.go:1360: Skipping rule ocp4-rbac-wildcard-use with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-drop-container-capabilities with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-host-dir-volume-plugin with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-host-ports with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-ipc-namespace with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-net-raw-capability with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-network-namespace with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-privilege-escalation with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-privileged-containers with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-process-id-namespace with unknown check type: helpers.go:1360: Skipping rule ocp4-scc-limit-root-containers with unknown check type: helpers.go:1360: Skipping rule ocp4-secrets-consider-external-storage with unknown check type: helpers.go:1360: Skipping rule ocp4-secrets-no-environment-variables with unknown check type: helpers.go:1360: Skipping rule ocp4-version-detect-in-hypershift with unknown check type: helpers.go:1360: Skipping rule ocp4-version-detect-in-ocp with unknown check type: helpers.go:1360: Skipping rule rhcos4-account-passwords-pam-faillock-dir with unknown check type: helpers.go:1360: Skipping rule rhcos4-account-use-centralized-automated-auth with unknown check type: helpers.go:1360: Skipping rule rhcos4-all-apparmor-profiles-enforced with unknown check type: helpers.go:1360: Skipping rule rhcos4-configure-user-data-backups with unknown check type: helpers.go:1360: Skipping rule rhcos4-encrypt-partitions with unknown check type: helpers.go:1360: Skipping rule rhcos4-kernel-disable-entropy-contribution-for-solid-state-drives with unknown check type: helpers.go:1360: Skipping rule rhcos4-partition-for-var-log with unknown check type: helpers.go:1360: Skipping rule rhcos4-partition-for-var-log-audit with unknown check type: helpers.go:1360: Skipping rule rhcos4-rsyslog-accept-remote-messages-tcp with unknown check type: helpers.go:1360: Skipping rule rhcos4-rsyslog-accept-remote-messages-udp with unknown check type: helpers.go:1360: Skipping rule rhcos4-set-ip6tables-default-rule with unknown check type: helpers.go:1360: Skipping rule rhcos4-set-iptables-default-rule with unknown check type: helpers.go:1360: Skipping rule rhcos4-set-iptables-default-rule-forward with unknown check type: helpers.go:1360: Skipping rule rhcos4-wireless-disable-in-bios with unknown check type: helpers.go:1360: Skipping rule rhcos4-zipl-enable-selinux with unknown check type:

rhmdnd · 2025-07-18T17:27:27Z

e2e_test.go

-	var numberOfRemediations int
+	t.Run("Find and categorize rules", func(t *testing.T) {
+		platformRules, nodeRules := ctx.findAndCategorizeRules(t)
+		t.Logf("Found %d platform rules and %d node rules", len(platformRules), len(nodeRules))


For awareness:

e2e_test.go:41: Found 336 platform rules and 608 node rules

Make it easier to figure out what the suite is installing by printing the git sha of the content it cloned.

We're changing the test strategy so that we don't need to test each profile individually. Instead, we want to stitch together rules into a single profile and run them all at once so we reduce the amount of churn when a single rule fails across multiple profiles. This commit removes the assertProfile assertion so that it's not required.

Adding a dedicated testing package with a function to return a TestConfig struct so we can grab the configuration whenever we need it. This can be useful when we need the config for setup or in the tests directly.

Currently, all the utility methods for setting up the tests are in the test cases themselves. We can make this cleaner by having a dedicated setup and teardown for our tests. This commit adds in a new package called `helpers` that contains utility functions for setting up and tearing down the test suite. Subsequent changes will include setup and teardown functions that eventually get worked into the e2e_test.go file.

This is based on linting recommendations from gostaticcheck.

Add a separate function to helpers that sets up the entire test suite before running the tests.

Just like setup, we can add a placeholder for tearing down test suites. While this might not be useful for CI environments that get thrown away, it can be useful for development environments where we need to run tests multiple times.

These are useful bits of information we use across the client and tests. Let's add them to the config so we can share them.

This is a useful method that makes it easier to instantiate clients in the tests. Let's make it public so we can generate clients and share them when needed.

These functions are similar to what we already have in helpers.go, but uses a different interface so that we can continue using the old ones while we move things over. Eventually, we can remove the versions of these functions in helpers.go and just rely on the ones in the helpers package.

This commit updates the e2e suite to use the setup and tear down approach, and also refactors the suite to use tailored profiles, so that we can test all node and platform rules in a single scan, instead of spinning up multiple scans for each profile which may contain redundant rules.

rhmdnd · 2025-07-22T17:19:52Z

helpers/setup.go

+	"github.com/ComplianceAsCode/ocp4e2e/config"
+)
+
+// setup performs all the necessary setup for the test suite


Capitalize this doc string.

rhmdnd · 2025-07-22T17:20:16Z

helpers/setup.go

+		return fmt.Errorf("failed to generate kube config: %w", kubeConfigErr)
+	}
+
+	// setVersionInformation? Not sure if we need this in setup or not


Pull the version information functionality into the test config.

openshift-ci · 2025-07-22T18:52:50Z

@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-ocp4-stig	`dc1f8fa`	link	true	`/test e2e-aws-ocp4-stig`
ci/prow/e2e-aws-ocp4-moderate	`dc1f8fa`	link	true	`/test e2e-aws-ocp4-moderate`
ci/prow/e2e-aws-ocp4-cis	`dc1f8fa`	link	true	`/test e2e-aws-ocp4-cis`
ci/prow/e2e-aws-rhcos4-moderate	`dc1f8fa`	link	true	`/test e2e-aws-rhcos4-moderate`

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.

rhmdnd requested review from Vincent056 and yuumasato July 17, 2025 21:36

rhmdnd commented Jul 17, 2025

View reviewed changes

Vincent056 reviewed Jul 17, 2025

View reviewed changes

rhmdnd commented Jul 17, 2025

View reviewed changes

helpers.go Outdated Show resolved Hide resolved

rhmdnd force-pushed the CMP-3385 branch 2 times, most recently from 02066bc to f712ca9 Compare July 17, 2025 23:48

rhmdnd force-pushed the CMP-3385 branch from f712ca9 to 22511ea Compare July 18, 2025 13:28

rhmdnd commented Jul 18, 2025

View reviewed changes

rhmdnd added 13 commits July 18, 2025 12:53

Add debug logging during operator install

6cc7002

Make it easier to figure out what the suite is installing by printing the git sha of the content it cloned.

Fix assertion file naming

f345707

Add a config package for testing configuration

84f5b22

Adding a dedicated testing package with a function to return a TestConfig struct so we can grab the configuration whenever we need it. This can be useful when we need the config for setup or in the tests directly.

Use fmt.Errorf instead of errors.New

a9b907b

This is based on linting recommendations from gostaticcheck.

Add a setup function

f5ca9e7

Add a separate function to helpers that sets up the entire test suite before running the tests.

Add a placeholder for teardown

9ab31c4

Just like setup, we can add a placeholder for tearing down test suites. While this might not be useful for CI environments that get thrown away, it can be useful for development environments where we need to run tests multiple times.

Error strings shouldn't be capitalized

f73a6bc

Add operator namespace and cluster version to test config

1b4e0f1

These are useful bits of information we use across the client and tests. Let's add them to the config so we can share them.

Make GenerateKubeConfig accessible outside the package

ee8155f

This is a useful method that makes it easier to instantiate clients in the tests. Let's make it public so we can generate clients and share them when needed.

rhmdnd commented Jul 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CMP-3385: Refactor test suite to group rules #54

CMP-3385: Refactor test suite to group rules #54

rhmdnd commented Jul 17, 2025

Uh oh!

rhmdnd Jul 17, 2025

Uh oh!

Vincent056 Jul 17, 2025

Uh oh!

rhmdnd Jul 17, 2025

Uh oh!

yuumasato Jul 18, 2025

Uh oh!

Uh oh!

rhmdnd Jul 18, 2025

Uh oh!

rhmdnd Jul 18, 2025

Uh oh!

rhmdnd Jul 22, 2025

Uh oh!

rhmdnd Jul 22, 2025

Uh oh!

openshift-ci bot commented Jul 22, 2025

Uh oh!

Uh oh!

CMP-3385: Refactor test suite to group rules #54

Are you sure you want to change the base?

CMP-3385: Refactor test suite to group rules #54

Conversation

rhmdnd commented Jul 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jul 22, 2025

Uh oh!

Uh oh!