From 636c769fb8d715cdfbb0382761447b0652d86e7b Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 25 Aug 2021 14:38:45 -0400 Subject: [PATCH] PodSecurity: preconstruct reused values benchmark old ns/op new ns/op delta BenchmarkVerifyPod/enforce-implicit_pod-12 370 228 -38.49% BenchmarkVerifyPod/enforce-implicit_deployment-12 408 241 -40.86% BenchmarkVerifyPod/enforce-privileged_pod-12 420 242 -42.27% BenchmarkVerifyPod/enforce-privileged_deployment-12 426 256 -39.84% BenchmarkVerifyPod/enforce-baseline_pod-12 4259 3006 -29.42% BenchmarkVerifyPod/enforce-baseline_deployment-12 341 266 -22.12% BenchmarkVerifyPod/enforce-restricted_pod-12 3322 3282 -1.20% BenchmarkVerifyPod/enforce-restricted_deployment-12 327 260 -20.59% BenchmarkVerifyPod/warn-baseline_pod-12 2964 3020 +1.89% BenchmarkVerifyPod/warn-baseline_deployment-12 3069 3127 +1.89% BenchmarkVerifyPod/warn-restricted_pod-12 3223 3330 +3.32% BenchmarkVerifyPod/warn-restricted_deployment-12 3443 3533 +2.61% BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-12 5193 5405 +4.08% BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-12 4295 4358 +1.47% BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-12 4363 4513 +3.44% BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-12 4482 4588 +2.37% benchmark old allocs new allocs delta BenchmarkVerifyPod/enforce-implicit_pod-12 2 1 -50.00% BenchmarkVerifyPod/enforce-implicit_deployment-12 2 1 -50.00% BenchmarkVerifyPod/enforce-privileged_pod-12 2 1 -50.00% BenchmarkVerifyPod/enforce-privileged_deployment-12 2 1 -50.00% BenchmarkVerifyPod/enforce-baseline_pod-12 17 17 +0.00% BenchmarkVerifyPod/enforce-baseline_deployment-12 2 1 -50.00% BenchmarkVerifyPod/enforce-restricted_pod-12 17 17 +0.00% BenchmarkVerifyPod/enforce-restricted_deployment-12 2 1 -50.00% BenchmarkVerifyPod/warn-baseline_pod-12 17 17 +0.00% BenchmarkVerifyPod/warn-baseline_deployment-12 19 19 +0.00% BenchmarkVerifyPod/warn-restricted_pod-12 17 17 +0.00% BenchmarkVerifyPod/warn-restricted_deployment-12 19 19 +0.00% BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-12 27 27 +0.00% BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-12 24 24 +0.00% BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-12 22 22 +0.00% BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-12 24 24 +0.00% benchmark old bytes new bytes delta BenchmarkVerifyPod/enforce-implicit_pod-12 208 112 -46.15% BenchmarkVerifyPod/enforce-implicit_deployment-12 208 112 -46.15% BenchmarkVerifyPod/enforce-privileged_pod-12 208 112 -46.15% BenchmarkVerifyPod/enforce-privileged_deployment-12 208 112 -46.15% BenchmarkVerifyPod/enforce-baseline_pod-12 3368 3368 +0.00% BenchmarkVerifyPod/enforce-baseline_deployment-12 208 112 -46.15% BenchmarkVerifyPod/enforce-restricted_pod-12 3368 3368 +0.00% BenchmarkVerifyPod/enforce-restricted_deployment-12 208 112 -46.15% BenchmarkVerifyPod/warn-baseline_pod-12 3368 3368 +0.00% BenchmarkVerifyPod/warn-baseline_deployment-12 3552 3552 +0.00% BenchmarkVerifyPod/warn-restricted_pod-12 3368 3368 +0.00% BenchmarkVerifyPod/warn-restricted_deployment-12 3552 3552 +0.00% BenchmarkVerifyPod/enforce-warn-audit-baseline_pod-12 5864 5864 +0.00% BenchmarkVerifyPod/enforce-warn-audit-baseline_deployment-12 4800 4800 +0.00% BenchmarkVerifyPod/warn-baseline-audit-restricted_pod-12 4616 4616 +0.00% BenchmarkVerifyPod/warn-baseline-audit-restricted_deployment-12 4800 4800 +0.00% --- .../admission/admission.go | 56 +++++++++++++------ .../admission/main_test.go | 36 ++++++++++++ 2 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 staging/src/k8s.io/pod-security-admission/admission/main_test.go 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 4269b740383..9f942e39c90 100644 --- a/staging/src/k8s.io/pod-security-admission/admission/admission.go +++ b/staging/src/k8s.io/pod-security-admission/admission/admission.go @@ -188,14 +188,20 @@ func (a *Admission) ValidateConfiguration() error { return nil } +var ( + namespacesResource = corev1.Resource("namespaces") + podsResource = corev1.Resource("pods") +) + // Validate admits an API request. // The objects in admission attributes are expected to be external v1 objects that we care about. +// The returned response may be shared and must not be mutated. func (a *Admission) Validate(ctx context.Context, attrs Attributes) *admissionv1.AdmissionResponse { var response *admissionv1.AdmissionResponse switch attrs.GetResource().GroupResource() { - case corev1.Resource("namespaces"): + case namespacesResource: response = a.ValidateNamespace(ctx, attrs) - case corev1.Resource("pods"): + case podsResource: response = a.ValidatePod(ctx, attrs) default: response = a.ValidatePodController(ctx, attrs) @@ -206,10 +212,13 @@ func (a *Admission) Validate(ctx context.Context, attrs Attributes) *admissionv1 return response } +// ValidateNamespace evaluates a namespace create or update request to ensure the pod security labels are valid, +// and checks existing pods in the namespace for violations of the new policy when updating the enforce level on a namespace. +// The returned response may be shared between evaluations and must not be mutated. func (a *Admission) ValidateNamespace(ctx context.Context, attrs Attributes) *admissionv1.AdmissionResponse { // short-circuit on subresources if attrs.GetSubresource() != "" { - return allowedResponse() + return sharedAllowedResponse() } obj, err := attrs.GetObject() if err != nil { @@ -230,7 +239,7 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs Attributes) *ad if newErr != nil { return invalidResponse(newErr.Error()) } - return allowedResponse() + return sharedAllowedResponse() case admissionv1.Update: // if update, check if policy labels changed @@ -257,24 +266,24 @@ func (a *Admission) ValidateNamespace(ctx context.Context, attrs Attributes) *ad // * if the new enforce is the same version and level was relaxed // * for exempt namespaces if newPolicy.Enforce == oldPolicy.Enforce { - return allowedResponse() + return sharedAllowedResponse() } if newPolicy.Enforce.Level == api.LevelPrivileged { - return allowedResponse() + return sharedAllowedResponse() } if newPolicy.Enforce.Version == oldPolicy.Enforce.Version && api.CompareLevels(newPolicy.Enforce.Level, oldPolicy.Enforce.Level) < 1 { - return allowedResponse() + return sharedAllowedResponse() } if a.exemptNamespace(attrs.GetNamespace()) { - return allowedResponse() + return sharedAllowedResponse() } response := allowedResponse() response.Warnings = a.EvaluatePodsInNamespace(ctx, namespace.Name, newPolicy.Enforce) return response default: - return allowedResponse() + return sharedAllowedResponse() } } @@ -292,14 +301,16 @@ var ignoredPodSubresources = map[string]bool{ "status": true, } +// ValidatePod evaluates a pod create or update request against the effective policy for the namespace. +// The returned response may be shared between evaluations and must not be mutated. func (a *Admission) ValidatePod(ctx context.Context, attrs Attributes) *admissionv1.AdmissionResponse { // short-circuit on ignored subresources if ignoredPodSubresources[attrs.GetSubresource()] { - return allowedResponse() + return sharedAllowedResponse() } // short-circuit on exempt namespaces and users if a.exemptNamespace(attrs.GetNamespace()) || a.exemptUser(attrs.GetUserName()) { - return allowedResponse() + return sharedAllowedResponse() } // short-circuit on privileged enforce+audit+warn namespaces @@ -310,7 +321,7 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs Attributes) *admissio } nsPolicy, _ := a.PolicyToEvaluate(namespace.Labels) if nsPolicy.Enforce.Level == api.LevelPrivileged && nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged { - return allowedResponse() + return sharedAllowedResponse() } obj, err := attrs.GetObject() @@ -336,20 +347,22 @@ func (a *Admission) ValidatePod(ctx context.Context, attrs Attributes) *admissio } if !isSignificantPodUpdate(pod, oldPod) { // Nothing we care about changed, so always allow the update. - return allowedResponse() + return sharedAllowedResponse() } } return a.EvaluatePod(ctx, attrs.GetNamespace(), &pod.ObjectMeta, &pod.Spec, true) } +// ValidatePodController evaluates a pod controller create or update request against the effective policy for the namespace. +// The returned response may be shared between evaluations and must not be mutated. func (a *Admission) ValidatePodController(ctx context.Context, attrs Attributes) *admissionv1.AdmissionResponse { // short-circuit on subresources if attrs.GetSubresource() != "" { - return allowedResponse() + return sharedAllowedResponse() } // short-circuit on exempt namespaces and users if a.exemptNamespace(attrs.GetNamespace()) || a.exemptUser(attrs.GetUserName()) { - return allowedResponse() + return sharedAllowedResponse() } // short-circuit on privileged audit+warn namespaces @@ -360,7 +373,7 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs Attributes) } nsPolicy, _ := a.PolicyToEvaluate(namespace.Labels) if nsPolicy.Warn.Level == api.LevelPrivileged && nsPolicy.Audit.Level == api.LevelPrivileged { - return allowedResponse() + return sharedAllowedResponse() } obj, err := attrs.GetObject() @@ -375,17 +388,18 @@ func (a *Admission) ValidatePodController(ctx context.Context, attrs Attributes) } if podMetadata == nil && podSpec == nil { // if a controller with an optional pod spec does not contain a pod spec, skip validation - return allowedResponse() + return sharedAllowedResponse() } return a.EvaluatePod(ctx, attrs.GetNamespace(), podMetadata, podSpec, false) } // EvaluatePod looks up the policy for the pods namespace, and checks it against the given pod(-like) object. // The enforce policy is only checked if enforce=true. +// The returned response may be shared between evaluations and must not be mutated. func (a *Admission) EvaluatePod(ctx context.Context, namespaceName string, podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec, enforce bool) *admissionv1.AdmissionResponse { // short-circuit on exempt runtimeclass if a.exemptRuntimeClass(podSpec.RuntimeClassName) { - return allowedResponse() + return sharedAllowedResponse() } namespace, err := a.NamespaceGetter.GetNamespace(ctx, namespaceName) @@ -478,6 +492,12 @@ func (a *Admission) PolicyToEvaluate(labels map[string]string) (api.Policy, erro return api.PolicyToEvaluate(labels, a.defaultPolicy) } +var _sharedAllowedResponse = allowedResponse() + +func sharedAllowedResponse() *admissionv1.AdmissionResponse { + return _sharedAllowedResponse +} + // allowedResponse is the response used when the admission decision is allow. func allowedResponse() *admissionv1.AdmissionResponse { return &admissionv1.AdmissionResponse{Allowed: true} diff --git a/staging/src/k8s.io/pod-security-admission/admission/main_test.go b/staging/src/k8s.io/pod-security-admission/admission/main_test.go new file mode 100644 index 00000000000..4d0e5851bf2 --- /dev/null +++ b/staging/src/k8s.io/pod-security-admission/admission/main_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package admission + +import ( + "fmt" + "os" + "reflect" + "testing" +) + +func TestMain(m *testing.M) { + sharedCopy := sharedAllowedResponse().DeepCopy() + rc := m.Run() + + if !reflect.DeepEqual(sharedCopy, sharedAllowedResponse()) { + fmt.Println("sharedAllowedReponse mutated") + rc = 1 + } + + os.Exit(rc) +}