From 57fdd167e4ecdf2af8d297919d87b56b7a5adcad Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 27 Oct 2021 20:24:02 -0400 Subject: [PATCH 1/5] apierrors: Avoid spurious in invalid error message --- .../apimachinery/pkg/api/errors/errors.go | 10 +++++-- .../pkg/api/errors/errors_test.go | 28 +++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go index 253fda22816..97e17be3941 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/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. diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go index 057f548dbb0..f57d21ff780 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go +++ b/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() - err := NewInvalid(kind("Kind"), "name", field.ErrorList{vErr}) + if vErr != nil && expected != nil { + 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) + } } } From 091724a6d86eb8ce86ffd4aaca4e8d4fb07785ef Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 28 Oct 2021 00:16:19 -0400 Subject: [PATCH 2/5] apierrors: optimize ToAggregate() for zero-length lists --- .../k8s.io/apimachinery/pkg/util/validation/field/errors.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go index c283ad189a0..2ed368f5693 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go +++ b/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 { From 7cd905e897f44b049c71ff284ced02ab147f3e8f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 28 Oct 2021 00:16:45 -0400 Subject: [PATCH 3/5] PodSecurity: plumb kind in attributes --- .../admission/admission_test.go | 12 ++++++++++++ .../pod-security-admission/admission/attributes.go | 7 +++++++ .../k8s.io/pod-security-admission/api/interfaces.go | 2 ++ 3 files changed, 21 insertions(+) diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go b/staging/src/k8s.io/pod-security-admission/admission/admission_test.go index da198f8391e..49fd3b7f8fa 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission_test.go +++ b/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, diff --git a/staging/src/k8s.io/pod-security-admission/admission/attributes.go b/staging/src/k8s.io/pod-security-admission/admission/attributes.go index f1821f70fae..f55c5a92505 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/attributes.go +++ b/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) } diff --git a/staging/src/k8s.io/pod-security-admission/api/interfaces.go b/staging/src/k8s.io/pod-security-admission/api/interfaces.go index 585a5ad6ce9..30fd002eaec 100644 --- a/staging/src/k8s.io/pod-security-admission/api/interfaces.go +++ b/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". From c0f33ddf087e93720710e9509c1c1d6bb47c51d1 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 27 Oct 2021 23:44:26 -0400 Subject: [PATCH 4/5] PodSecurity: fix level/version validation fieldpaths --- .../admission/api/helpers.go | 24 +++++++++---------- .../pod-security-admission/api/helpers.go | 12 +++++----- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/staging/src/k8s.io/pod-security-admission/admission/api/helpers.go b/staging/src/k8s.io/pod-security-admission/admission/api/helpers.go index 85313b5c262..86591615d15 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/api/helpers.go +++ b/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) diff --git a/staging/src/k8s.io/pod-security-admission/api/helpers.go b/staging/src/k8s.io/pod-security-admission/api/helpers.go index e3dafe6d0f0..4f946a92aa9 100644 --- a/staging/src/k8s.io/pod-security-admission/api/helpers.go +++ b/staging/src/k8s.io/pod-security-admission/api/helpers.go @@ -158,33 +158,33 @@ func PolicyToEvaluate(labels map[string]string, defaults Policy) (Policy, error) ) if level, ok := labels[EnforceLevelLabel]; ok { p.Enforce.Level, err = ParseLevel(level) - errs = appendErr(errs, err, "Enforce.Level") + errs = appendErr(errs, err, EnforceLevelLabel) } if version, ok := labels[EnforceVersionLabel]; ok { p.Enforce.Version, err = ParseVersion(version) - errs = appendErr(errs, err, "Enforce.Version") + errs = appendErr(errs, err, EnforceVersionLabel) } if level, ok := labels[AuditLevelLabel]; ok { p.Audit.Level, err = ParseLevel(level) - errs = appendErr(errs, err, "Audit.Level") + errs = appendErr(errs, err, AuditLevelLabel) 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) } if level, ok := labels[WarnLevelLabel]; ok { p.Warn.Level, err = ParseLevel(level) - errs = appendErr(errs, err, "Warn.Level") + errs = appendErr(errs, err, WarnLevelLabel) 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) } return p, errors.NewAggregate(errs) } From 3aa656b63f9e0fc0d0750a159b3bbd91185998a6 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 28 Oct 2021 00:23:44 -0400 Subject: [PATCH 5/5] PodSecurity: return field errors for invalid namespace labels --- .../admission/admission.go | 35 +++++++++++-------- .../pod-security-admission/api/helpers.go | 28 ++++++++------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/staging/src/k8s.io/pod-security-admission/admission/admission.go b/staging/src/k8s.io/pod-security-admission/admission/admission.go index a3a698d8d92..0a75435103d 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission.go +++ b/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 { - return invalidResponse(newErr.Error()) + if len(newErrs) > 0 { + 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()) { - return invalidResponse(newErr.Error()) + if len(newErrs) > 0 && (len(oldErrs) == 0 || !reflect.DeepEqual(newErrs, oldErrs)) { + 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) - if nsPolicyErr == nil && nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged { + 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 { 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) - if nsPolicyErr == nil && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged { + nsPolicy, nsPolicyErrs := a.PolicyToEvaluate(namespace.Labels) + 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 { - return failureResponse(msg, metav1.StatusReasonInvalid, 422) +func invalidResponse(attrs api.Attributes, fieldErrors field.ErrorList) *admissionv1.AdmissionResponse { + 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. diff --git a/staging/src/k8s.io/pod-security-admission/api/helpers.go b/staging/src/k8s.io/pod-security-admission/api/helpers.go index 4f946a92aa9..ee175993d9f 100644 --- a/staging/src/k8s.io/pod-security-admission/api/helpers.go +++ b/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. -func appendErr(errs []error, err error, field string) []error { +var labelsPath = field.NewPath("metadata", "labels") + +// 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 }