Skip to content

WIP: CNTRLPLANE-3386: Required scc annotation checker updates #31325

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.

Sign up for GitHub

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

Open
liouk wants to merge 2 commits into
base: main
Choose a base branch
from
+17 −49
Open

WIP: CNTRLPLANE-3386: Required scc annotation checker updates #31325

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6bff2d
required-scc-annotation-checker: Ignore run-level namespaces from the…
liouk Jun 23, 2026
51eaa69
required-scc-annotation-checker: Remove pending namespaces, and ignor…
liouk Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 17 additions & 49 deletions pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,38 +37,7 @@ var nonStandardSCCNamespaces = map[string]sets.Set[string]{
"machine-api-termination-handler": sets.New("openshift-machine-api"),
}

// namespacesWithPendingSCCPinning includes namespaces with workloads that have pending SCC pinning.
var namespacesWithPendingSCCPinning = sets.NewString(
"openshift-cluster-csi-drivers",
"openshift-image-registry",
"openshift-ingress",
"openshift-insights",
"openshift-machine-api",
// run-level namespaces
"openshift-cloud-controller-manager",
"openshift-cloud-controller-manager-operator",
"openshift-cluster-api",
"openshift-cluster-machine-approver",
"openshift-dns",
"openshift-dns-operator",
"openshift-etcd",
"openshift-etcd-operator",
"openshift-kube-apiserver",
"openshift-kube-apiserver-operator",
"openshift-kube-controller-manager",
"openshift-kube-controller-manager-operator",
"openshift-kube-proxy",
"openshift-kube-scheduler",
"openshift-kube-scheduler-operator",
"openshift-multus",
"openshift-network-operator",
"openshift-ovn-kubernetes",
"openshift-sdn",
"openshift-storage",
)

// systemNamespaces includes namespaces that should be treated as flaking.
// these namespaces are included because we don't control their creation or labeling on their creation.
// systemNamespaces are skipped because we don't control their creation or labeling.
var systemNamespaces = sets.NewString(
"default",
"kube-system",
Expand Down Expand Up @@ -112,15 +81,7 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD

junits := []*junitapi.JUnitTestCase{}
for _, ns := range namespaces.Items {
// skip managed service namespaces
if exutil.ManagedServiceNamespaces.Has(ns.Name) {
continue
}

// require that all workloads in openshift, kube-*, or default namespaces must have the required-scc annotation
// ignore openshift-must-gather-* namespaces which are generated dynamically
isPermanentOpenShiftNamespace := (ns.Name == "openshift" || strings.HasPrefix(ns.Name, "openshift-")) && !strings.HasPrefix(ns.Name, "openshift-must-gather-")
if !strings.HasPrefix(ns.Name, "kube-") && ns.Name != "default" && !isPermanentOpenShiftNamespace {
if shouldSkipNamespace(&ns) {

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the required-scc monitor has an explicit CatalogSource pod exclusion.
# Expected: a match that skips/continues CatalogSource-owned pods before required SCC validation.
rg -n -C4 --type go 'CatalogSource|catalogsource|OwnerReference|ownerReferences|RequiredSCCAnnotation' pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go

Repository: openshift/origin

Length of output: 1589

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the monitor implementation around the validation loop and namespace skip logic.
sed -n '1,260p' pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go

Repository: openshift/origin

Length of output: 6351

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the monitor implementation around the validation loop and namespace skip logic.
sed -n '1,260p' pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go

Repository: openshift/origin

Length of output: 6351

Add a pod-level CatalogSource skip. CollectData only filters namespaces here; pods in included namespaces still go through the SCC checks, so CatalogSource-owned pods are not excluded. Add an explicit pod-owner check before validating RequiredSCCAnnotation.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go` at
line 84, The pod filtering in CollectData only skips entire namespaces, so
CatalogSource-owned pods in allowed namespaces still reach the SCC validation
path. Update the pod-processing logic in CollectData to add an explicit owner
check for CatalogSource before calling the RequiredSCCAnnotation validation,
using the existing namespace skip helper and the pod iteration flow to keep the
exclusion at the pod level.

continue
}

Expand Down Expand Up @@ -188,14 +149,6 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
SystemOut: failureMsg,
FailureOutput: &junitapi.FailureOutput{Output: failureMsg},
})

// add a successful test with the same name to cause a flake if the namespace should be flaking
if namespacesWithPendingSCCPinning.Has(ns.Name) || systemNamespaces.Has(ns.Name) {
junits = append(junits,
&junitapi.JUnitTestCase{
Name: testName,
})
}
}

return nil, junits, nil
Expand All @@ -217,6 +170,21 @@ func (w *requiredSCCAnnotationChecker) Cleanup(ctx context.Context) error {
return nil
}

func shouldSkipNamespace(ns *v1.Namespace) bool {
if exutil.ManagedServiceNamespaces.Has(ns.Name) {
return true
}
if systemNamespaces.Has(ns.Name) {
return true
}
if _, hasRunLevel := ns.Labels["openshift.io/run-level"]; hasRunLevel {
return true
}
if !strings.HasPrefix(ns.Name, "openshift-") || strings.HasPrefix(ns.Name, "openshift-must-gather-") {
return true
}
return false
}
func ownerReferences(pod *v1.Pod) string {
ownerRefs := make([]string, len(pod.OwnerReferences))
for i, or := range pod.OwnerReferences {
Expand Down