From 3f1b0cc511f8f29ece230aa17561e46c65bac026 Mon Sep 17 00:00:00 2001 From: jennybuckley Date: Tue, 18 Sep 2018 14:49:45 -0700 Subject: [PATCH] Don't run limitranger admission plugin on pod update requests --- plugin/pkg/admission/limitranger/admission.go | 7 +++++++ .../admission/limitranger/admission_test.go | 20 +++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/plugin/pkg/admission/limitranger/admission.go b/plugin/pkg/admission/limitranger/admission.go index 3659cbb738..14a5f4099d 100644 --- a/plugin/pkg/admission/limitranger/admission.go +++ b/plugin/pkg/admission/limitranger/admission.go @@ -463,6 +463,13 @@ func (d *DefaultLimitRangerActions) SupportsAttributes(a admission.Attributes) b return false } + // Since containers and initContainers cannot currently be added, removed, or updated, it is unnecessary + // to mutate and validate limitrange on pod updates. Trying to mutate containers or initContainers on a pod + // update request will always fail pod validation because those fields are immutable once the object is created. + if a.GetKind().GroupKind() == api.Kind("Pod") && a.GetOperation() == admission.Update { + return false + } + return a.GetKind().GroupKind() == api.Kind("Pod") || a.GetKind().GroupKind() == api.Kind("PersistentVolumeClaim") } diff --git a/plugin/pkg/admission/limitranger/admission_test.go b/plugin/pkg/admission/limitranger/admission_test.go index 0504614310..dde3373c63 100644 --- a/plugin/pkg/admission/limitranger/admission_test.go +++ b/plugin/pkg/admission/limitranger/admission_test.go @@ -694,13 +694,17 @@ func TestLimitRangerIgnoresSubresource(t *testing.T) { informerFactory.Start(wait.NeverStop) testPod := validPod("testPod", 1, api.ResourceRequirements{}) - err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil)) if err != nil { t.Fatal(err) } - err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil)) if err == nil { - t.Errorf("Expected an error since the pod did not specify resource limits in its update call") + t.Errorf("Expected an error since the pod did not specify resource limits in its create call") + } + err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + if err != nil { + t.Errorf("Expected not to call limitranger actions on pod updates") } err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, false, nil)) @@ -720,13 +724,17 @@ func TestLimitRangerAdmitPod(t *testing.T) { informerFactory.Start(wait.NeverStop) testPod := validPod("testPod", 1, api.ResourceRequirements{}) - err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil)) if err != nil { t.Fatal(err) } - err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil)) if err == nil { - t.Errorf("Expected an error since the pod did not specify resource limits in its update call") + t.Errorf("Expected an error since the pod did not specify resource limits in its create call") + } + err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil)) + if err != nil { + t.Errorf("Expected not to call limitranger actions on pod updates") } err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, false, nil))