Merge pull request #113491 from tallclair/psa-warn

[PSA] default warn to enforce level

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

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
+	"strings"
 	"time"
 
 	"k8s.io/klog/v2"
@@ -250,7 +251,7 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 			return invalidResponse(attrs, newErrs)
 		}
 		if a.exemptNamespace(attrs.GetNamespace()) {
-			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" {
+			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy, namespace.Labels); warning != "" {
 				response := allowedResponse()
 				response.Warnings = append(response.Warnings, warning)
 				return response
@@ -293,7 +294,7 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 			return sharedAllowedResponse
 		}
 		if a.exemptNamespace(attrs.GetNamespace()) {
-			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy); warning != "" {
+			if warning := a.exemptNamespaceWarning(namespace.Name, newPolicy, namespace.Labels); warning != "" {
 				response := allowedResponse()
 				response.Warnings = append(response.Warnings, warning)
 				return response
@@ -736,11 +737,39 @@ func containsString(needle string, haystack []string) bool {
 
 // 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 {
+func (a *Admission) exemptNamespaceWarning(exemptNamespace string, policy api.Policy, nsLabels map[string]string) string {
 	if policy.FullyPrivileged() || policy.Equivalent(&a.defaultPolicy) {
 		return ""
 	}
 
+	// Build a compact representation of the policy, only printing non-privileged modes that have
+	// been explicitly set.
+	sb := strings.Builder{}
+	_, hasEnforceLevel := nsLabels[api.EnforceLevelLabel]
+	_, hasEnforceVersion := nsLabels[api.EnforceVersionLabel]
+	if policy.Enforce.Level != api.LevelPrivileged && (hasEnforceLevel || hasEnforceVersion) {
+		sb.WriteString("enforce=")
+		sb.WriteString(policy.Enforce.String())
+	}
+	_, hasAuditLevel := nsLabels[api.AuditLevelLabel]
+	_, hasAuditVersion := nsLabels[api.AuditVersionLabel]
+	if policy.Audit.Level != api.LevelPrivileged && (hasAuditLevel || hasAuditVersion) {
+		if sb.Len() > 0 {
+			sb.WriteString(", ")
+		}
+		sb.WriteString("audit=")
+		sb.WriteString(policy.Audit.String())
+	}
+	_, hasWarnLevel := nsLabels[api.WarnLevelLabel]
+	_, hasWarnVersion := nsLabels[api.WarnVersionLabel]
+	if policy.Warn.Level != api.LevelPrivileged && (hasWarnLevel || hasWarnVersion) {
+		if sb.Len() > 0 {
+			sb.WriteString(", ")
+		}
+		sb.WriteString("warn=")
+		sb.WriteString(policy.Warn.String())
+	}
+
 	return fmt.Sprintf("namespace %q is exempt from Pod Security, and the policy (%s) will be ignored",
-		exemptNamespace, policy.CompactString())
+		exemptNamespace, sb.String())
 }

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

@@ -1206,7 +1206,7 @@ func TestExemptNamespaceWarning(t *testing.T) {
 		},
 		defaultPolicy:         baselinePolicy,
 		expectWarning:         true,
-		expectWarningContains: "(enforce=baseline:v1.23, audit=baseline:v1.23, warn=baseline:latest)",
+		expectWarningContains: "(warn=baseline:latest)",
 	}}
 
 	const (
@@ -1228,7 +1228,7 @@ func TestExemptNamespaceWarning(t *testing.T) {
 			policy, err := api.PolicyToEvaluate(labels, defaultPolicy)
 			require.NoError(t, err.ToAggregate())
 
-			warning := a.exemptNamespaceWarning(test.name, policy)
+			warning := a.exemptNamespaceWarning(test.name, policy, labels)
 			if !test.expectWarning {
 				assert.Empty(t, warning)
 				return

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

@@ -161,35 +161,6 @@ 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 {
@@ -213,12 +184,16 @@ func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, field.
 		errs field.ErrorList
 
 		p = defaults
+
+		hasEnforceLevel              bool
+		hasWarnLevel, hasWarnVersion bool
 	)
 	if len(labels) == 0 {
 		return p, nil
 	}
 	if level, ok := labels[EnforceLevelLabel]; ok {
 		p.Enforce.Level, err = ParseLevel(level)
+		hasEnforceLevel = (err == nil) // Don't default warn in case of error
 		errs = appendErr(errs, err, EnforceLevelLabel, level)
 	}
 	if version, ok := labels[EnforceVersionLabel]; ok {
@@ -237,6 +212,7 @@ func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, field.
 		errs = appendErr(errs, err, AuditVersionLabel, version)
 	}
 	if level, ok := labels[WarnLevelLabel]; ok {
+		hasWarnLevel = true
 		p.Warn.Level, err = ParseLevel(level)
 		errs = appendErr(errs, err, WarnLevelLabel, level)
 		if err != nil {
@@ -244,9 +220,19 @@ func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, field.
 		}
 	}
 	if version, ok := labels[WarnVersionLabel]; ok {
+		hasWarnVersion = true
 		p.Warn.Version, err = ParseVersion(version)
 		errs = appendErr(errs, err, WarnVersionLabel, version)
 	}
+
+	// Default warn to the enforce level when explicitly set to a more restrictive level.
+	if !hasWarnLevel && hasEnforceLevel && CompareLevels(p.Enforce.Level, p.Warn.Level) > 0 {
+		p.Warn.Level = p.Enforce.Level
+		if !hasWarnVersion {
+			p.Warn.Version = p.Enforce.Version
+		}
+	}
+
 	return p, errs
 }
 

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

@@ -117,3 +117,151 @@ func TestPolicyEquals(t *testing.T) {
 	assert.True(t, baseline.Equivalent(&baseline), "baseline policy equals itself")
 	assert.False(t, privileged.Equivalent(&baseline), "privileged != baseline")
 }
+
+func TestPolicyToEvaluate(t *testing.T) {
+	privilegedLV := LevelVersion{
+		Level:   LevelPrivileged,
+		Version: LatestVersion(),
+	}
+	privilegedPolicy := Policy{
+		Enforce: privilegedLV,
+		Warn:    privilegedLV,
+		Audit:   privilegedLV,
+	}
+
+	type testcase struct {
+		desc      string
+		labels    map[string]string
+		defaults  Policy
+		expect    Policy
+		expectErr bool
+	}
+
+	tests := []testcase{{
+		desc:   "simple enforce",
+		labels: makeLabels("enforce", "baseline"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, LatestVersion()},
+			Warn:    LevelVersion{LevelBaseline, LatestVersion()},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "simple warn",
+		labels: makeLabels("warn", "restricted"),
+		expect: Policy{
+			Enforce: privilegedLV,
+			Warn:    LevelVersion{LevelRestricted, LatestVersion()},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "simple audit",
+		labels: makeLabels("audit", "baseline"),
+		expect: Policy{
+			Enforce: privilegedLV,
+			Warn:    privilegedLV,
+			Audit:   LevelVersion{LevelBaseline, LatestVersion()},
+		},
+	}, {
+		desc:   "enforce & warn",
+		labels: makeLabels("enforce", "baseline", "warn", "restricted"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, LatestVersion()},
+			Warn:    LevelVersion{LevelRestricted, LatestVersion()},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "enforce version",
+		labels: makeLabels("enforce", "baseline", "enforce-version", "v1.22"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, MajorMinorVersion(1, 22)},
+			Warn:    LevelVersion{LevelBaseline, MajorMinorVersion(1, 22)},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "enforce version & warn-version",
+		labels: makeLabels("enforce", "baseline", "enforce-version", "v1.22", "warn-version", "latest"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, MajorMinorVersion(1, 22)},
+			Warn:    LevelVersion{LevelBaseline, LatestVersion()},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "enforce & warn-version",
+		labels: makeLabels("enforce", "baseline", "warn-version", "v1.23"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, LatestVersion()},
+			Warn:    LevelVersion{LevelBaseline, MajorMinorVersion(1, 23)},
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc: "fully specd",
+		labels: makeLabels(
+			"enforce", "baseline", "enforce-version", "v1.20",
+			"warn", "restricted", "warn-version", "v1.21",
+			"audit", "restricted", "audit-version", "v1.22"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, MajorMinorVersion(1, 20)},
+			Warn:    LevelVersion{LevelRestricted, MajorMinorVersion(1, 21)},
+			Audit:   LevelVersion{LevelRestricted, MajorMinorVersion(1, 22)},
+		},
+	}, {
+		desc:   "enforce no warn",
+		labels: makeLabels("enforce", "baseline", "warn", "privileged"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, LatestVersion()},
+			Warn:    privilegedLV,
+			Audit:   privilegedLV,
+		},
+	}, {
+		desc:   "enforce warn error",
+		labels: makeLabels("enforce", "baseline", "warn", "foo"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelBaseline, LatestVersion()},
+			Warn:    privilegedLV,
+			Audit:   privilegedLV,
+		},
+		expectErr: true,
+	}, {
+		desc:   "enforce error",
+		labels: makeLabels("enforce", "foo"),
+		expect: Policy{
+			Enforce: LevelVersion{LevelRestricted, LatestVersion()},
+			Warn:    privilegedLV,
+			Audit:   privilegedLV,
+		},
+		expectErr: true,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			if test.defaults == (Policy{}) {
+				test.defaults = privilegedPolicy
+			}
+
+			actual, errs := PolicyToEvaluate(test.labels, test.defaults)
+			if test.expectErr {
+				assert.Error(t, errs.ToAggregate())
+			} else {
+				assert.NoError(t, errs.ToAggregate())
+			}
+
+			assert.Equal(t, test.expect, actual)
+		})
+	}
+}
+
+// makeLabels turns the kev-value pairs into a labels map[string]string.
+func makeLabels(kvs ...string) map[string]string {
+	if len(kvs)%2 != 0 {
+		panic("makeLabels called with mismatched key-values")
+	}
+	labels := map[string]string{
+		"other-label": "foo-bar",
+	}
+	for i := 0; i < len(kvs); i += 2 {
+		key, value := kvs[i], kvs[i+1]
+		key = labelPrefix + key
+		labels[key] = value
+	}
+	return labels
+}