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-44966: Revert "Enforce the required-scc monitor test and validate usage of non-standard OCP SCCs #29321

Merged
Merged
Changes from all commits
Commits
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
Revert "OCPBUGS-42435: Enforce the required-scc monitor test and vali…
…date usage of non-standard OCP SCCs"
  • Loading branch information
neisw authored Nov 24, 2024
commit 0536dfd4c692fd8da60f4dd4054d51d33caa5a5f
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,6 @@ var defaultSCCs = sets.NewString(
"restricted-v2",
)

var nonStandardSCCNamespaces = map[string]sets.Set[string]{
"node-exporter": sets.New("openshift-monitoring"),
"machine-api-termination-handler": sets.New("openshift-machine-api"),
}

var namespacesWithPendingSCCPinning = sets.NewString(
"openshift-cluster-csi-drivers",
"openshift-cluster-version",
"openshift-image-registry",
"openshift-ingress",
"openshift-ingress-canary",
"openshift-ingress-operator",
"openshift-insights",
"openshift-machine-api",
"openshift-marketplace",
"openshift-monitoring",
)

type requiredSCCAnnotationChecker struct {
kubeClient kubernetes.Interface
}
Expand Down Expand Up @@ -86,59 +68,20 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
continue
}

// check if the namespace should be treated as flaking when failed
flakeWhenFailed := ns.Labels["openshift.io/run-level"] == "0" ||
ns.Labels["openshift.io/run-level"] == "1" ||
namespacesWithPendingSCCPinning.Has(ns.Name)

pods, err := w.kubeClient.CoreV1().Pods(ns.Name).List(ctx, metav1.ListOptions{})
if err != nil {
return nil, nil, err
}

failures := make([]string, 0)
for _, pod := range pods.Items {
validatedSCC := pod.Annotations[securityv1.ValidatedSCCAnnotation]
allowedNamespaces, isNonStandard := nonStandardSCCNamespaces[validatedSCC]

if _, exists := pod.Annotations[securityv1.RequiredSCCAnnotation]; exists {
if isNonStandard && !allowedNamespaces.Has(ns.Name) {
failures = append(failures, fmt.Sprintf(
"pod '%s' has a non-standard SCC '%s' not allowed in namespace '%s'; allowed namespaces are: %s",
pod.Name, validatedSCC, ns.Name, strings.Join(allowedNamespaces.UnsortedList(), ", ")))
}
continue
}

suggestedSCC := suggestSCC(&pod)
owners := ownerReferences(&pod)

switch {
case len(validatedSCC) == 0:
failures = append(failures, fmt.Sprintf(
"annotation missing from pod '%s'%s; cannot suggest required-scc, no validated SCC on pod",
pod.Name, owners))

case defaultSCCs.Has(validatedSCC):
failures = append(failures, fmt.Sprintf(
"annotation missing from pod '%s'%s; suggested required-scc: '%s'",
pod.Name, owners, validatedSCC))

case isNonStandard:
if allowedNamespaces.Has(ns.Name) {
failures = append(failures, fmt.Sprintf(
"annotation missing from pod '%s'%s; suggested required-scc: '%s', this is a non-standard SCC",
pod.Name, owners, validatedSCC))
} else {
failures = append(failures, fmt.Sprintf(
"annotation missing from pod '%s'%s; pod is using non-standard SCC '%s' not allowed in namespace '%s'; allowed namespaces are: %s",
pod.Name, owners, validatedSCC, ns.Name, strings.Join(allowedNamespaces.UnsortedList(), ", ")))
}

default:
failures = append(failures, fmt.Sprintf(
"annotation missing from pod '%s'%s; cannot suggest required-scc, validated SCC '%s' is a custom SCC",
pod.Name, owners, validatedSCC))
}
failures = append(failures, fmt.Sprintf("annotation missing from pod '%s'%s; %s", pod.Name, owners, suggestedSCC))
}

testName := fmt.Sprintf("[sig-auth] all workloads in ns/%s must set the '%s' annotation", ns.Name, securityv1.RequiredSCCAnnotation)
Expand All @@ -148,21 +91,18 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
}

failureMsg := strings.Join(failures, "\n")

junits = append(junits,
&junitapi.JUnitTestCase{
Name: testName,
SystemOut: failureMsg,
FailureOutput: &junitapi.FailureOutput{Output: failureMsg},
})

// add a successful test with the same name to cause a flake
if flakeWhenFailed {
junits = append(junits,
&junitapi.JUnitTestCase{
Name: testName,
})
}
},

// add a successful test with the same name to cause a flake
&junitapi.JUnitTestCase{
Name: testName,
},
)
}

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

// suggestSCC suggests the assigned SCC only if it belongs to the default set of SCCs
// pods in runlevel 0/1 namespaces won't have any assigned SCC as SCC admission is disabled
func suggestSCC(pod *v1.Pod) string {
if len(pod.Annotations[securityv1.ValidatedSCCAnnotation]) == 0 {
return "cannot suggest required-scc, no validated SCC on pod"
}

if defaultSCCs.Has(pod.Annotations[securityv1.ValidatedSCCAnnotation]) {
return fmt.Sprintf("suggested required-scc: '%s'", pod.Annotations[securityv1.ValidatedSCCAnnotation])
}

return "cannot suggest required-scc, validated SCC is custom"
}

func ownerReferences(pod *v1.Pod) string {
ownerRefs := make([]string, len(pod.OwnerReferences))
for i, or := range pod.OwnerReferences {
Expand Down