PodSecurity: return field errors for invalid namespace labels

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

@@ -30,9 +30,11 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	admissionapi "k8s.io/pod-security-admission/admission/api"
 	"k8s.io/pod-security-admission/admission/api/validation"
 	"k8s.io/pod-security-admission/api"
@@ -239,13 +241,13 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 		return badRequestResponse("failed to decode namespace")
 	}
 
-	newPolicy, newErr := a.PolicyToEvaluate(namespace.Labels)
+	newPolicy, newErrs := a.PolicyToEvaluate(namespace.Labels)
 
 	switch attrs.GetOperation() {
 	case admissionv1.Create:
 		// require valid labels on create
-		if newErr != nil {
+		if len(newErrs) > 0 {
-			return invalidResponse(newErr.Error())
+			return invalidResponse(attrs, newErrs)
 		}
 		return sharedAllowedResponse()
 
@@ -261,11 +263,11 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs api.Attributes)
 			klog.InfoS("failed to assert old namespace type", "type", reflect.TypeOf(oldObj))
 			return badRequestResponse("failed to decode old namespace")
 		}
-		oldPolicy, oldErr := a.PolicyToEvaluate(oldNamespace.Labels)
+		oldPolicy, oldErrs := a.PolicyToEvaluate(oldNamespace.Labels)
 
 		// require valid labels on update if they have changed
-		if newErr != nil && (oldErr == nil || newErr.Error() != oldErr.Error()) {
+		if len(newErrs) > 0 && (len(oldErrs) == 0 || !reflect.DeepEqual(newErrs, oldErrs)) {
-			return invalidResponse(newErr.Error())
+			return invalidResponse(attrs, newErrs)
 		}
 
 		// Skip dry-running pods:
@@ -327,8 +329,8 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs api.Attributes) *admi
 		klog.ErrorS(err, "failed to fetch pod namespace", "namespace", attrs.GetNamespace())
 		return internalErrorResponse(fmt.Sprintf("failed to lookup namespace %s", attrs.GetNamespace()))
 	}
-	nsPolicy, nsPolicyErr := a.PolicyToEvaluate(namespace.Labels)
+	nsPolicy, nsPolicyErrs := a.PolicyToEvaluate(namespace.Labels)
-	if nsPolicyErr == nil && nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged {
+	if len(nsPolicyErrs) == 0 && nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged {
 		return sharedAllowedResponse()
 	}
 
@@ -358,7 +360,7 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs api.Attributes) *admi
 			return sharedAllowedResponse()
 		}
 	}
-	return a.EvaluatePod(ctx, nsPolicy, nsPolicyErr, &pod.ObjectMeta, &pod.Spec, attrs, true)
+	return a.EvaluatePod(ctx, nsPolicy, nsPolicyErrs.ToAggregate(), &pod.ObjectMeta, &pod.Spec, attrs, true)
 }
 
 // ValidatePodController evaluates a pod controller create or update request against the effective policy for the namespace.
@@ -379,8 +381,8 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs api.Attribu
 		klog.ErrorS(err, "failed to fetch pod namespace", "namespace", attrs.GetNamespace())
 		return internalErrorResponse(fmt.Sprintf("failed to lookup namespace %s", attrs.GetNamespace()))
 	}
-	nsPolicy, nsPolicyErr := a.PolicyToEvaluate(namespace.Labels)
+	nsPolicy, nsPolicyErrs := a.PolicyToEvaluate(namespace.Labels)
-	if nsPolicyErr == nil && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged {
+	if len(nsPolicyErrs) == 0 && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged {
 		return sharedAllowedResponse()
 	}
 
@@ -398,7 +400,7 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs api.Attribu
 		// if a controller with an optional pod spec does not contain a pod spec, skip validation
 		return sharedAllowedResponse()
 	}
-	return a.EvaluatePod(ctx, nsPolicy, nsPolicyErr, podMetadata, podSpec, attrs, false)
+	return a.EvaluatePod(ctx, nsPolicy, nsPolicyErrs.ToAggregate(), podMetadata, podSpec, attrs, false)
 }
 
 // EvaluatePod evaluates the given policy against the given pod(-like) object.
@@ -559,7 +561,7 @@ func decoratePodWarnings(podWarningsToCount map[string]podCount, warnings []stri
 	}
 }
 
-func (a *Admission) PolicyToEvaluate(labels map[string]string) (api.Policy, error) {
+func (a *Admission) PolicyToEvaluate(labels map[string]string) (api.Policy, field.ErrorList) {
 	return api.PolicyToEvaluate(labels, a.defaultPolicy)
 }
 
@@ -592,8 +594,11 @@ func forbiddenResponse(msg string) *admissionv1.AdmissionResponse {
 }
 
 // invalidResponse is the response used for namespace requests when namespace labels are invalid.
-func invalidResponse(msg string) *admissionv1.AdmissionResponse {
+func invalidResponse(attrs api.Attributes, fieldErrors field.ErrorList) *admissionv1.AdmissionResponse {
-	return failureResponse(msg, metav1.StatusReasonInvalid, 422)
+	return &admissionv1.AdmissionResponse{
+		Allowed: false,
+		Result:  &apierrors.NewInvalid(attrs.GetKind().GroupKind(), attrs.GetName(), fieldErrors).ErrStatus,
+	}
 }
 
 // badRequestResponse is the response used when a request cannot be processed.

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

@@ -22,7 +22,7 @@ import (
 	"strconv"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/component-base/version"
 )
 
@@ -149,44 +149,44 @@ type Policy struct {
 // 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
 // "restricted" and "latest" are used for level and version respectively.
-func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, error) {
+func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, field.ErrorList) {
 	var (
 		err  error
-		errs []error
+		errs field.ErrorList
 
 		p = defaults
 	)
 	if level, ok := labels[EnforceLevelLabel]; ok {
 		p.Enforce.Level, err = ParseLevel(level)
-		errs = appendErr(errs, err, EnforceLevelLabel)
+		errs = appendErr(errs, err, EnforceLevelLabel, level)
 	}
 	if version, ok := labels[EnforceVersionLabel]; ok {
 		p.Enforce.Version, err = ParseVersion(version)
-		errs = appendErr(errs, err, EnforceVersionLabel)
+		errs = appendErr(errs, err, EnforceVersionLabel, version)
 	}
 	if level, ok := labels[AuditLevelLabel]; ok {
 		p.Audit.Level, err = ParseLevel(level)
-		errs = appendErr(errs, err, AuditLevelLabel)
+		errs = appendErr(errs, err, AuditLevelLabel, level)
 		if err != nil {
 			p.Audit.Level = LevelPrivileged // Fail open for audit.
 		}
 	}
 	if version, ok := labels[AuditVersionLabel]; ok {
 		p.Audit.Version, err = ParseVersion(version)
-		errs = appendErr(errs, err, AuditVersionLabel)
+		errs = appendErr(errs, err, AuditVersionLabel, version)
 	}
 	if level, ok := labels[WarnLevelLabel]; ok {
 		p.Warn.Level, err = ParseLevel(level)
-		errs = appendErr(errs, err, WarnLevelLabel)
+		errs = appendErr(errs, err, WarnLevelLabel, level)
 		if err != nil {
 			p.Warn.Level = LevelPrivileged // Fail open for warn.
 		}
 	}
 	if version, ok := labels[WarnVersionLabel]; ok {
 		p.Warn.Version, err = ParseVersion(version)
-		errs = appendErr(errs, err, WarnVersionLabel)
+		errs = appendErr(errs, err, WarnVersionLabel, version)
 	}
-	return p, errors.NewAggregate(errs)
+	return p, errs
 }
 
 // CompareLevels returns an integer comparing two levels by strictness. The result will be 0 if
@@ -211,10 +211,12 @@ func CompareLevels(a, b Level) int {
 	return 0
 }
 
-// appendErr is a helper function to collect field-specific errors.
+var labelsPath = field.NewPath("metadata", "labels")
-func appendErr(errs []error, err error, field string) []error {
+
+// appendErr is a helper function to collect label-specific errors.
+func appendErr(errs field.ErrorList, err error, label, value string) field.ErrorList {
 	if err != nil {
-		return append(errs, fmt.Errorf("%s: %s", field, err.Error()))
+		return append(errs, field.Invalid(labelsPath.Key(label), value, err.Error()))
 	}
 	return errs
 }