Warn when adding PSA labels to exempt namespaces (#109680)

staging/src/k8s.io/pod-security-admission/admission/admission.go

@@ -249,6 +249,13 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 		if len(newErrs) > 0 {
 			return invalidResponse(attrs, newErrs)
 		}
+		if a.exemptNamespace(attrs.GetNamespace()) {
+			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" {
+				response := allowedResponse()
+				response.Warnings = append(response.Warnings, warning)
+				return response
+			}
+		}
 		return sharedAllowedResponse
 
 	case admissionv1.Update:
@@ -286,7 +293,12 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 			return sharedAllowedResponse
 		}
 		if a.exemptNamespace(attrs.GetNamespace()) {
-			return sharedAllowedByNamespaceExemptionResponse
+			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" {
+				response := allowedResponse()
+				response.Warnings = append(response.Warnings, warning)
+				return response
+			}
+			return sharedAllowedResponse
 		}
 		response := allowedResponse()
 		response.Warnings = a.EvaluatePodsInNamespace(ctx, namespace.Name, newPolicy.Enforce)
@@ -334,10 +346,10 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs api.Attributes) *admi
 	if err != nil {
 		klog.ErrorS(err, "failed to fetch pod namespace", "namespace", attrs.GetNamespace())
 		a.Metrics.RecordError(true, attrs)
-		return errorResponse(err, &apierrors.NewInternalError(fmt.Errorf("failed to lookup namespace %s", attrs.GetNamespace())).ErrStatus)
+		return errorResponse(err, &apierrors.NewInternalError(fmt.Errorf("failed to lookup namespace %q", attrs.GetNamespace())).ErrStatus)
 	}
 	nsPolicy, nsPolicyErrs := a.PolicyToEvaluate(namespace.Labels)
-	if len(nsPolicyErrs) == 0 && nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged {
+	if len(nsPolicyErrs) == 0 && nsPolicy.FullyPrivileged() {
 		a.Metrics.RecordEvaluation(metrics.DecisionAllow, nsPolicy.Enforce, metrics.ModeEnforce, attrs)
 		return sharedAllowedPrivilegedResponse
 	}
@@ -400,7 +412,7 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs api.Attribu
 		a.Metrics.RecordError(true, attrs)
 		response := allowedResponse()
 		response.AuditAnnotations = map[string]string{
-			"error": fmt.Sprintf("failed to lookup namespace %s: %v", attrs.GetNamespace(), err),
+			"error": fmt.Sprintf("failed to lookup namespace %q: %v", attrs.GetNamespace(), err),
 		}
 		return response
 	}
@@ -723,3 +735,14 @@ func containsString(needle string, haystack []string) bool {
 	}
 	return false
 }
+
+// exemptNamespaceWarning returns a non-empty warning message if the exempt namespace has a
+// non-privileged policy and sets pod security labels.
+func (a *Admission) exemptNamespaceWarning(exemptNamespace string, policy api.Policy) string {
+	if policy.FullyPrivileged() || policy.Equivalent(&a.defaultPolicy) {
+		return ""
+	}
+
+	return fmt.Sprintf("namespace %q is exempt from Pod Security, and the policy (%s) will be ignored",
+		exemptNamespace, policy.CompactString())
+}

staging/src/k8s.io/pod-security-admission/admission/admission_test.go

@@ -267,10 +267,20 @@ func TestValidateNamespace(t *testing.T) {
 		},
 		{
 			name:           "create restricted",
-			newLabels:      map[string]string{api.EnforceLevelLabel: string(api.LevelBaseline)},
+			newLabels:      map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted)},
 			expectAllowed:  true,
 			expectListPods: false,
 		},
+		{
+			name:             "create restricted exempt",
+			newLabels:        map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted)},
+			exemptNamespaces: []string{"test"},
+			expectAllowed:    true,
+			expectListPods:   false,
+			expectWarnings: []string{
+				`namespace "test" is exempt from Pod Security, and the policy (enforce=restricted:latest) will be ignored`,
+			},
+		},
 		{
 			name:           "create malformed level",
 			newLabels:      map[string]string{api.EnforceLevelLabel: "unknown"},
@@ -346,10 +356,18 @@ func TestValidateNamespace(t *testing.T) {
 		{
 			name:             "update exempt to restricted",
 			exemptNamespaces: []string{"test"},
-			newLabels:        map[string]string{api.EnforceLevelLabel: string(api.LevelRestricted), api.EnforceVersionLabel: "v1.0"},
+			newLabels: map[string]string{
-			oldLabels:        map[string]string{},
+				api.EnforceLevelLabel:   string(api.LevelRestricted),
-			expectAllowed:    true,
+				api.EnforceVersionLabel: "v1.0",
-			expectListPods:   false,
+				api.AuditLevelLabel:     string(api.LevelRestricted),
+				api.WarnLevelLabel:      string(api.LevelBaseline),
+			},
+			oldLabels:      map[string]string{},
+			expectAllowed:  true,
+			expectListPods: false,
+			expectWarnings: []string{
+				`namespace "test" is exempt from Pod Security, and the policy (enforce=restricted:v1.0, audit=restricted:latest, warn=baseline:latest) will be ignored`,
+			},
 		},
 
 		// update tests that introduce labels errors
@@ -520,6 +538,14 @@ func TestValidateNamespace(t *testing.T) {
 					Level:   api.LevelPrivileged,
 					Version: api.LatestVersion(),
 				},
+				Audit: api.LevelVersion{
+					Level:   api.LevelPrivileged,
+					Version: api.LatestVersion(),
+				},
+				Warn: api.LevelVersion{
+					Level:   api.LevelPrivileged,
+					Version: api.LatestVersion(),
+				},
 			}
 			if tc.defaultPolicy != nil {
 				defaultPolicy = *tc.defaultPolicy
@@ -1112,6 +1138,111 @@ func TestPrioritizePods(t *testing.T) {
 	}
 }
 
+func TestExemptNamespaceWarning(t *testing.T) {
+	privileged := api.LevelVersion{
+		Level:   api.LevelPrivileged,
+		Version: api.LatestVersion(),
+	}
+	privilegedPolicy := api.Policy{
+		Enforce: privileged,
+		Audit:   privileged,
+		Warn:    privileged,
+	}
+	baseline := api.LevelVersion{
+		Level:   api.LevelBaseline,
+		Version: api.MajorMinorVersion(1, 23),
+	}
+	baselinePolicy := api.Policy{
+		Enforce: baseline,
+		Audit:   baseline,
+		Warn:    baseline,
+	}
+	tests := []struct {
+		name                  string
+		labels                map[string]string
+		defaultPolicy         api.Policy // Defaults to privilegedPolicy if empty.
+		expectWarning         bool
+		expectWarningContains string
+	}{{
+		name:          "empty-case",
+		expectWarning: false,
+	}, {
+		name: "ignore-privileged",
+		labels: map[string]string{
+			api.EnforceLevelLabel:   string(api.LevelPrivileged),
+			api.EnforceVersionLabel: "v1.24",
+			api.WarnVersionLabel:    "v1.25",
+		},
+		expectWarning: false,
+	}, {
+		name: "warn-on-enforce",
+		labels: map[string]string{
+			api.EnforceLevelLabel: string(api.LevelBaseline),
+		},
+		expectWarning:         true,
+		expectWarningContains: "(enforce=baseline:latest)",
+	}, {
+		name: "warn-on-warn",
+		labels: map[string]string{
+			api.WarnLevelLabel: string(api.LevelBaseline),
+		},
+		expectWarning:         true,
+		expectWarningContains: "(warn=baseline:latest)",
+	}, {
+		name: "warn-on-audit",
+		labels: map[string]string{
+			api.AuditLevelLabel: string(api.LevelRestricted),
+		},
+		expectWarning:         true,
+		expectWarningContains: "(audit=restricted:latest)",
+	}, {
+		name:          "ignore-default-policy",
+		defaultPolicy: baselinePolicy,
+		expectWarning: false,
+	}, {
+		name: "warn-versions-default-policy",
+		labels: map[string]string{
+			api.WarnVersionLabel: "latest",
+		},
+		defaultPolicy:         baselinePolicy,
+		expectWarning:         true,
+		expectWarningContains: "(enforce=baseline:v1.23, audit=baseline:v1.23, warn=baseline:latest)",
+	}}
+
+	const (
+		sentinelLabelKey   = "qqincegidneocgu"
+		sentinelLabelValue = "vpmxkpcjphxrcpx"
+	)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			defaultPolicy := test.defaultPolicy
+			if defaultPolicy == (api.Policy{}) {
+				defaultPolicy = privilegedPolicy
+			}
+			a := &Admission{defaultPolicy: defaultPolicy}
+			labels := test.labels
+			if labels == nil {
+				labels = map[string]string{}
+			}
+			labels[sentinelLabelKey] = sentinelLabelValue
+			policy, err := api.PolicyToEvaluate(labels, defaultPolicy)
+			require.NoError(t, err.ToAggregate())
+
+			warning := a.exemptNamespaceWarning(test.name, policy)
+			if !test.expectWarning {
+				assert.Empty(t, warning)
+				return
+			}
+			require.NotEmpty(t, warning)
+
+			assert.NotContains(t, warning, sentinelLabelKey, "non-podsecurity label key included")
+			assert.NotContains(t, warning, sentinelLabelValue, "non-podsecurity label value included")
+
+			assert.Contains(t, warning, test.expectWarningContains)
+		})
+	}
+}
+
 type testAttributes struct {
 	api.AttributesRecord
 

staging/src/k8s.io/pod-security-admission/api/helpers.go

@@ -144,12 +144,65 @@ func (lv LevelVersion) String() string {
 	return fmt.Sprintf("%s:%s", lv.Level, lv.Version)
 }
 
+// Equivalent determines whether two LevelVersions are functionally equivalent. LevelVersions are
+// considered equivalent if both are privileged, or both levels & versions are equal.
+func (lv *LevelVersion) Equivalent(other *LevelVersion) bool {
+	return (lv.Level == LevelPrivileged && other.Level == LevelPrivileged) ||
+		(lv.Level == other.Level && lv.Version == other.Version)
+}
+
 type Policy struct {
 	Enforce LevelVersion
 	Audit   LevelVersion
 	Warn    LevelVersion
 }
 
+func (p *Policy) String() string {
+	return fmt.Sprintf("enforce=%#v, audit=%#v, warn=%#v", p.Enforce, p.Audit, p.Warn)
+}
+
+// CompactString prints a minimalist representation of the policy that excludes any privileged
+// levels.
+func (p *Policy) CompactString() string {
+	sb := strings.Builder{}
+	if p.Enforce.Level != LevelPrivileged {
+		sb.WriteString("enforce=")
+		sb.WriteString(p.Enforce.String())
+	}
+	if p.Audit.Level != LevelPrivileged {
+		if sb.Len() > 0 {
+			sb.WriteString(", ")
+		}
+		sb.WriteString("audit=")
+		sb.WriteString(p.Audit.String())
+	}
+	if p.Warn.Level != LevelPrivileged {
+		if sb.Len() > 0 {
+			sb.WriteString(", ")
+		}
+		sb.WriteString("warn=")
+		sb.WriteString(p.Warn.String())
+	}
+	if sb.Len() == 0 {
+		// All modes were privileged, just output "privileged".
+		return string(LevelPrivileged)
+	}
+	return sb.String()
+}
+
+// Equivalent determines whether two policies are functionally equivalent. Policies are considered
+// equivalent if all 3 modes are considered equivalent.
+func (p *Policy) Equivalent(other *Policy) bool {
+	return p.Enforce.Equivalent(&other.Enforce) && p.Audit.Equivalent(&other.Audit) && p.Warn.Equivalent(&other.Warn)
+}
+
+// FullyPrivileged returns true if all 3 policy modes are privileged.
+func (p *Policy) FullyPrivileged() bool {
+	return p.Enforce.Level == LevelPrivileged &&
+		p.Audit.Level == LevelPrivileged &&
+		p.Warn.Level == LevelPrivileged
+}
+
 // PolicyToEvaluate resolves the PodSecurity namespace labels to the policy for that namespace,
 // falling back to the provided defaults when a label is unspecified. A valid policy is always
 // returned, even when an error is returned. If labels cannot be parsed correctly, the values of

staging/src/k8s.io/pod-security-admission/api/helpers_test.go

@@ -53,3 +53,67 @@ func TestParseVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestLevelVersionEquals(t *testing.T) {
+	t.Run("a LevelVersion should be equal to itself", func(t *testing.T) {
+		for _, l := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} {
+			for _, v := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} {
+				lv := LevelVersion{l, v}
+				other := lv
+				assert.True(t, lv.Equivalent(&other), lv.String())
+			}
+		}
+	})
+	t.Run("different levels should not be equal", func(t *testing.T) {
+		for _, l1 := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} {
+			for _, l2 := range []Level{LevelPrivileged, LevelBaseline, LevelRestricted} {
+				if l1 != l2 {
+					lv1 := LevelVersion{l1, LatestVersion()}
+					lv2 := LevelVersion{l2, LatestVersion()}
+					assert.False(t, lv1.Equivalent(&lv2), "%#v != %#v", lv1, lv2)
+				}
+			}
+		}
+	})
+	t.Run("different non-privileged versions should not be equal", func(t *testing.T) {
+		for _, l := range []Level{LevelBaseline, LevelRestricted} {
+			for _, v1 := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} {
+				for _, v2 := range []Version{MajorMinorVersion(1, 16), MajorMinorVersion(1, 13)} {
+					lv1 := LevelVersion{l, v1}
+					lv2 := LevelVersion{l, v2}
+					assert.False(t, lv1.Equivalent(&lv2), "%#v != %#v", lv1, lv2)
+				}
+			}
+		}
+	})
+	t.Run("different privileged versions should be equal", func(t *testing.T) {
+		for _, v1 := range []Version{LatestVersion(), MajorMinorVersion(1, 18), MajorMinorVersion(1, 30)} {
+			for _, v2 := range []Version{MajorMinorVersion(1, 16), MajorMinorVersion(1, 13)} {
+				lv1 := LevelVersion{LevelPrivileged, v1}
+				lv2 := LevelVersion{LevelPrivileged, v2}
+				assert.True(t, lv1.Equivalent(&lv2), "%#v == %#v", lv1, lv2)
+			}
+		}
+	})
+}
+
+func TestPolicyEquals(t *testing.T) {
+	privileged := Policy{
+		Enforce: LevelVersion{LevelPrivileged, LatestVersion()},
+		Audit:   LevelVersion{LevelPrivileged, LatestVersion()},
+		Warn:    LevelVersion{LevelPrivileged, LatestVersion()},
+	}
+	require.True(t, privileged.FullyPrivileged())
+
+	privileged2 := privileged
+	privileged2.Enforce.Version = MajorMinorVersion(1, 20)
+	require.True(t, privileged2.FullyPrivileged())
+
+	baseline := privileged
+	baseline.Audit.Level = LevelBaseline
+	require.False(t, baseline.FullyPrivileged())
+
+	assert.True(t, privileged.Equivalent(&privileged2), "ignore privileged versions")
+	assert.True(t, baseline.Equivalent(&baseline), "baseline policy equals itself")
+	assert.False(t, privileged.Equivalent(&baseline), "privileged != baseline")
+}