Merge pull request #105959 from liggitt/podsecurity-details

PodSecurity: return namespace validation errors in standard field.ErrorList format

staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go

@@ -289,7 +289,7 @@ func NewInvalid(qualifiedKind schema.GroupKind, name string, errs field.ErrorLis
 			Field:   err.Field,
 		})
 	}
-	return &StatusError{metav1.Status{
+	err := &StatusError{metav1.Status{
 		Status: metav1.StatusFailure,
 		Code:   http.StatusUnprocessableEntity,
 		Reason: metav1.StatusReasonInvalid,
@@ -299,8 +299,14 @@ func NewInvalid(qualifiedKind schema.GroupKind, name string, errs field.ErrorLis
 			Name:   name,
 			Causes: causes,
 		},
-		Message: fmt.Sprintf("%s %q is invalid: %v", qualifiedKind.String(), name, errs.ToAggregate()),
 	}}
+	aggregatedErrs := errs.ToAggregate()
+	if aggregatedErrs == nil {
+		err.ErrStatus.Message = fmt.Sprintf("%s %q is invalid", qualifiedKind.String(), name)
+	} else {
+		err.ErrStatus.Message = fmt.Sprintf("%s %q is invalid: %v", qualifiedKind.String(), name, aggregatedErrs)
+	}
+	return err
 }
 
 // NewBadRequest creates an error that indicates that the request is invalid and can not be processed.

staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go

@@ -125,6 +125,7 @@ func TestNewInvalid(t *testing.T) {
 	testCases := []struct {
 		Err     *field.Error
 		Details *metav1.StatusDetails
+		Msg     string
 	}{
 		{
 			field.Duplicate(field.NewPath("field[0].name"), "bar"),
@@ -136,6 +137,7 @@ func TestNewInvalid(t *testing.T) {
 					Field: "field[0].name",
 				}},
 			},
+			`Kind "name" is invalid: field[0].name: Duplicate value: "bar"`,
 		},
 		{
 			field.Invalid(field.NewPath("field[0].name"), "bar", "detail"),
@@ -147,6 +149,7 @@ func TestNewInvalid(t *testing.T) {
 					Field: "field[0].name",
 				}},
 			},
+			`Kind "name" is invalid: field[0].name: Invalid value: "bar": detail`,
 		},
 		{
 			field.NotFound(field.NewPath("field[0].name"), "bar"),
@@ -158,6 +161,7 @@ func TestNewInvalid(t *testing.T) {
 					Field: "field[0].name",
 				}},
 			},
+			`Kind "name" is invalid: field[0].name: Not found: "bar"`,
 		},
 		{
 			field.NotSupported(field.NewPath("field[0].name"), "bar", nil),
@@ -169,6 +173,7 @@ func TestNewInvalid(t *testing.T) {
 					Field: "field[0].name",
 				}},
 			},
+			`Kind "name" is invalid: field[0].name: Unsupported value: "bar"`,
 		},
 		{
 			field.Required(field.NewPath("field[0].name"), ""),
@@ -180,12 +185,28 @@ func TestNewInvalid(t *testing.T) {
 					Field: "field[0].name",
 				}},
 			},
+			`Kind "name" is invalid: field[0].name: Required value`,
+		},
+		{
+			nil,
+			&metav1.StatusDetails{
+				Kind:   "Kind",
+				Name:   "name",
+				Causes: []metav1.StatusCause{},
+			},
+			`Kind "name" is invalid`,
 		},
 	}
 	for i, testCase := range testCases {
 		vErr, expected := testCase.Err, testCase.Details
-		expected.Causes[0].Message = vErr.ErrorBody()
+		if vErr != nil && expected != nil {
-		err := NewInvalid(kind("Kind"), "name", field.ErrorList{vErr})
+			expected.Causes[0].Message = vErr.ErrorBody()
+		}
+		var errList field.ErrorList
+		if vErr != nil {
+			errList = append(errList, vErr)
+		}
+		err := NewInvalid(kind("Kind"), "name", errList)
 		status := err.ErrStatus
 		if status.Code != 422 || status.Reason != metav1.StatusReasonInvalid {
 			t.Errorf("%d: unexpected status: %#v", i, status)
@@ -193,6 +214,9 @@ func TestNewInvalid(t *testing.T) {
 		if !reflect.DeepEqual(expected, status.Details) {
 			t.Errorf("%d: expected %#v, got %#v", i, expected, status.Details)
 		}
+		if testCase.Msg != status.Message {
+			t.Errorf("%d: expected\n%s\ngot\n%s", i, testCase.Msg, status.Message)
+		}
 	}
 }
 

staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go

@@ -239,6 +239,9 @@ func NewErrorTypeMatcher(t ErrorType) utilerrors.Matcher {
 
 // ToAggregate converts the ErrorList into an errors.Aggregate.
 func (list ErrorList) ToAggregate() utilerrors.Aggregate {
+	if len(list) == 0 {
+		return nil
+	}
 	errs := make([]error, 0, len(list))
 	errorMsgs := sets.NewString()
 	for _, err := range list {

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/admission/admission_test.go

@@ -460,7 +460,9 @@ func TestValidateNamespace(t *testing.T) {
 			attrs := &AttributesRecord{
 				Object:      newObject,
 				OldObject:   oldObject,
+				Name:        newObject.Name,
 				Namespace:   newObject.Name,
+				Kind:        schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"},
 				Resource:    schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"},
 				Subresource: tc.subresource,
 				Operation:   operation,
@@ -592,6 +594,7 @@ func TestValidatePodController(t *testing.T) {
 		newObject runtime.Object
 		// for update
 		oldObject runtime.Object
+		gvk       schema.GroupVersionKind
 		gvr       schema.GroupVersionResource
 
 		expectWarnings         []string
@@ -602,40 +605,47 @@ func TestValidatePodController(t *testing.T) {
 			subresource: "status",
 			newObject:   &badDeploy,
 			oldObject:   &goodDeploy,
+			gvk:         schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:         schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 		},
 		{
 			desc:             "namespace in exemptNamespaces will be exempted",
 			newObject:        &badDeploy,
+			gvk:              schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:              schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			exemptNamespaces: []string{testNamespace},
 		},
 		{
 			desc:                 "runtimeClass in exemptRuntimeClasses will be exempted",
 			newObject:            &badDeploy,
+			gvk:                  schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:                  schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			exemptRuntimeClasses: []string{"containerd"},
 		},
 		{
 			desc:        "user in exemptUsers will be exempted",
 			newObject:   &badDeploy,
+			gvk:         schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:         schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			exemptUsers: []string{"testuser"},
 		},
 		{
 			desc:      "podMetadata == nil && podSpec == nil will skip verification",
 			newObject: &corev1.ReplicationController{ObjectMeta: metav1.ObjectMeta{Name: "foo-rc"}},
+			gvk:       schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ReplicationController"},
 			gvr:       schema.GroupVersionResource{Group: "", Version: "v1", Resource: "replicationcontrollers"},
 		},
 		{
 			desc:                   "good deploy creates and produce nothing",
 			newObject:              &goodDeploy,
+			gvk:                    schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:                    schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			expectAuditAnnotations: map[string]string{},
 		},
 		{
 			desc:                   "bad deploy creates produce correct user-visible warnings and correct auditAnnotations",
 			newObject:              &badDeploy,
+			gvk:                    schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:                    schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			expectAuditAnnotations: map[string]string{"audit": "would violate PodSecurity \"baseline:latest\": forbidden sysctls (unknown)"},
 			expectWarnings:         []string{"would violate PodSecurity \"baseline:latest\": forbidden sysctls (unknown)"},
@@ -644,6 +654,7 @@ func TestValidatePodController(t *testing.T) {
 			desc:                   "bad spec updates don't block on enforce failures and returns correct information",
 			newObject:              &badDeploy,
 			oldObject:              &goodDeploy,
+			gvk:                    schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
 			gvr:                    schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			expectAuditAnnotations: map[string]string{"audit": "would violate PodSecurity \"baseline:latest\": forbidden sysctls (unknown)"},
 			expectWarnings:         []string{"would violate PodSecurity \"baseline:latest\": forbidden sysctls (unknown)"},
@@ -660,6 +671,7 @@ func TestValidatePodController(t *testing.T) {
 			attrs := &AttributesRecord{
 				testName,
 				testNamespace,
+				tc.gvk,
 				tc.gvr,
 				tc.subresource,
 				operation,

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

@@ -34,45 +34,45 @@ func ToPolicy(defaults PodSecurityDefaults) (policyapi.Policy, error) {
 	)
 
 	if len(defaults.Enforce) == 0 {
-		errs = appendErr(errs, requiredErr, "Enforce.Level")
+		errs = appendErr(errs, requiredErr, "enforce")
 	} else {
 		p.Enforce.Level, err = policyapi.ParseLevel(defaults.Enforce)
-		errs = appendErr(errs, err, "Enforce.Level")
+		errs = appendErr(errs, err, "enforce")
 	}
 
 	if len(defaults.EnforceVersion) == 0 {
-		errs = appendErr(errs, requiredErr, "Enforce.Version")
+		errs = appendErr(errs, requiredErr, "enforce-version")
 	} else {
 		p.Enforce.Version, err = policyapi.ParseVersion(defaults.EnforceVersion)
-		errs = appendErr(errs, err, "Enforce.Version")
+		errs = appendErr(errs, err, "enforce-version")
 	}
 
 	if len(defaults.Audit) == 0 {
-		errs = appendErr(errs, requiredErr, "Audit.Level")
+		errs = appendErr(errs, requiredErr, "audit")
 	} else {
 		p.Audit.Level, err = policyapi.ParseLevel(defaults.Audit)
-		errs = appendErr(errs, err, "Audit.Level")
+		errs = appendErr(errs, err, "audit")
 	}
 
 	if len(defaults.AuditVersion) == 0 {
-		errs = appendErr(errs, requiredErr, "Audit.Version")
+		errs = appendErr(errs, requiredErr, "audit-version")
 	} else {
 		p.Audit.Version, err = policyapi.ParseVersion(defaults.AuditVersion)
-		errs = appendErr(errs, err, "Audit.Version")
+		errs = appendErr(errs, err, "audit-version")
 	}
 
 	if len(defaults.Warn) == 0 {
-		errs = appendErr(errs, requiredErr, "Warn.Level")
+		errs = appendErr(errs, requiredErr, "warn")
 	} else {
 		p.Warn.Level, err = policyapi.ParseLevel(defaults.Warn)
-		errs = appendErr(errs, err, "Warn.Level")
+		errs = appendErr(errs, err, "warn")
 	}
 
 	if len(defaults.WarnVersion) == 0 {
-		errs = appendErr(errs, requiredErr, "Warn.Version")
+		errs = appendErr(errs, requiredErr, "warn-version")
 	} else {
 		p.Warn.Version, err = policyapi.ParseVersion(defaults.WarnVersion)
-		errs = appendErr(errs, err, "Warn.Version")
+		errs = appendErr(errs, err, "warn-version")
 	}
 
 	return p, errors.NewAggregate(errs)

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

@@ -27,6 +27,7 @@ import (
 type AttributesRecord struct {
 	Name        string
 	Namespace   string
+	Kind        schema.GroupVersionKind
 	Resource    schema.GroupVersionResource
 	Subresource string
 	Operation   admissionv1.Operation
@@ -41,6 +42,9 @@ func (a *AttributesRecord) GetName() string {
 func (a *AttributesRecord) GetNamespace() string {
 	return a.Namespace
 }
+func (a *AttributesRecord) GetKind() schema.GroupVersionKind {
+	return a.Kind
+}
 func (a *AttributesRecord) GetResource() schema.GroupVersionResource {
 	return a.Resource
 }
@@ -81,6 +85,9 @@ func (a *attributes) GetName() string {
 func (a *attributes) GetNamespace() string {
 	return a.r.Namespace
 }
+func (a *attributes) GetKind() schema.GroupVersionKind {
+	return schema.GroupVersionKind(a.r.Kind)
+}
 func (a *attributes) GetResource() schema.GroupVersionResource {
 	return schema.GroupVersionResource(a.r.Resource)
 }

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, "Enforce.Level")
+		errs = appendErr(errs, err, EnforceLevelLabel, level)
 	}
 	if version, ok := labels[EnforceVersionLabel]; ok {
 		p.Enforce.Version, err = ParseVersion(version)
-		errs = appendErr(errs, err, "Enforce.Version")
+		errs = appendErr(errs, err, EnforceVersionLabel, version)
 	}
 	if level, ok := labels[AuditLevelLabel]; ok {
 		p.Audit.Level, err = ParseLevel(level)
-		errs = appendErr(errs, err, "Audit.Level")
+		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, "Audit.Version")
+		errs = appendErr(errs, err, AuditVersionLabel, version)
 	}
 	if level, ok := labels[WarnLevelLabel]; ok {
 		p.Warn.Level, err = ParseLevel(level)
-		errs = appendErr(errs, err, "Warn.Level")
+		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, "Warn.Version")
+		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
 }

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

@@ -30,6 +30,8 @@ type Attributes interface {
 	GetNamespace() string
 	// GetResource is the name of the resource being requested.  This is not the kind.  For example: pods
 	GetResource() schema.GroupVersionResource
+	// GetKind is the name of the kind being requested.  For example: Pod
+	GetKind() schema.GroupVersionKind
 	// GetSubresource is the name of the subresource being requested.  This is a different resource, scoped to the parent resource, but it may have a different kind.
 	// For instance, /pods has the resource "pods" and the kind "Pod", while /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod"
 	// (because status operates on pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource "binding", and kind "Binding".