diff --git a/pkg/kubeapiserver/admission/BUILD b/pkg/kubeapiserver/admission/BUILD index c1a833dcd13..95a3eb4c306 100644 --- a/pkg/kubeapiserver/admission/BUILD +++ b/pkg/kubeapiserver/admission/BUILD @@ -42,6 +42,7 @@ filegroup( srcs = [ ":package-srcs", "//pkg/kubeapiserver/admission/configuration:all-srcs", + "//pkg/kubeapiserver/admission/util:all-srcs", ], tags = ["automanaged"], ) diff --git a/pkg/kubeapiserver/admission/util/BUILD b/pkg/kubeapiserver/admission/util/BUILD new file mode 100644 index 00000000000..401e3a39a58 --- /dev/null +++ b/pkg/kubeapiserver/admission/util/BUILD @@ -0,0 +1,31 @@ +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) + +load( + "@io_bazel_rules_go//go:def.bzl", + "go_library", +) + +go_library( + name = "go_default_library", + srcs = ["initializer.go"], + tags = ["automanaged"], + deps = [ + "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], +) diff --git a/pkg/kubeapiserver/admission/util/initializer.go b/pkg/kubeapiserver/admission/util/initializer.go new file mode 100644 index 00000000000..165c4ee9ba8 --- /dev/null +++ b/pkg/kubeapiserver/admission/util/initializer.go @@ -0,0 +1,56 @@ +/* +Copyright 2017 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 util + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apiserver/pkg/admission" +) + +// IsUpdatingInitializedObject returns if the operation is trying to update an +// already initialized object. +func IsUpdatingInitializedObject(a admission.Attributes) (bool, error) { + if a.GetOperation() != admission.Update { + return false, nil + } + oldObj := a.GetOldObject() + accessor, err := meta.Accessor(oldObj) + if err != nil { + return false, err + } + if accessor.GetInitializers() == nil { + return true, nil + } + return false, nil +} + +// IsUpdatingUninitializedObject returns if the operation is trying to update an +// object that is not initialized yet. +func IsUpdatingUninitializedObject(a admission.Attributes) (bool, error) { + if a.GetOperation() != admission.Update { + return false, nil + } + oldObj := a.GetOldObject() + accessor, err := meta.Accessor(oldObj) + if err != nil { + return false, err + } + if accessor.GetInitializers() == nil { + return false, nil + } + return true, nil +} diff --git a/plugin/pkg/admission/podnodeselector/BUILD b/plugin/pkg/admission/podnodeselector/BUILD index e89110ebb08..3d589c9332d 100644 --- a/plugin/pkg/admission/podnodeselector/BUILD +++ b/plugin/pkg/admission/podnodeselector/BUILD @@ -15,6 +15,7 @@ go_library( "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/client/listers/core/internalversion:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", + "//pkg/kubeapiserver/admission/util:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/plugin/pkg/admission/podnodeselector/admission.go b/plugin/pkg/admission/podnodeselector/admission.go index 8e00f0b825d..469286f1e9b 100644 --- a/plugin/pkg/admission/podnodeselector/admission.go +++ b/plugin/pkg/admission/podnodeselector/admission.go @@ -33,6 +33,7 @@ import ( informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" + "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" ) // The annotation key scheduler.alpha.kubernetes.io/node-selector is for assigning @@ -112,11 +113,20 @@ func (p *podNodeSelector) Admit(a admission.Attributes) error { return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request")) } + updateInitialized, err := util.IsUpdatingInitializedObject(a) + if err != nil { + return err + } + if updateInitialized { + // node selector of an initialized pod is immutable + return nil + } + name := pod.Name nsName := a.GetNamespace() var namespace *api.Namespace - namespace, err := p.namespaceLister.Get(nsName) + namespace, err = p.namespaceLister.Get(nsName) if errors.IsNotFound(err) { namespace, err = p.defaultGetNamespace(nsName) if err != nil { @@ -158,7 +168,7 @@ func (p *podNodeSelector) Admit(a admission.Attributes) error { func NewPodNodeSelector(clusterNodeSelectors map[string]string) *podNodeSelector { return &podNodeSelector{ - Handler: admission.NewHandler(admission.Create), + Handler: admission.NewHandler(admission.Create, admission.Update), clusterNodeSelectors: clusterNodeSelectors, } } diff --git a/plugin/pkg/admission/podnodeselector/admission_test.go b/plugin/pkg/admission/podnodeselector/admission_test.go index f5bc4ac969e..0075468a89a 100644 --- a/plugin/pkg/admission/podnodeselector/admission_test.go +++ b/plugin/pkg/admission/podnodeselector/admission_test.go @@ -52,6 +52,12 @@ func TestPodAdmission(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"}, } + oldPod := *pod + oldPod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init"}}} + oldPod.Spec.NodeSelector = map[string]string{ + "old": "true", + } + tests := []struct { defaultNodeSelector string namespaceNodeSelector string @@ -166,7 +172,17 @@ func TestPodAdmission(t *testing.T) { } else if !test.admit && err == nil { t.Errorf("Test: %s, expected an error", test.testName) } + if test.admit && !labels.Equals(test.mergedNodeSelector, labels.Set(pod.Spec.NodeSelector)) { + t.Errorf("Test: %s, expected: %s but got: %s", test.testName, test.mergedNodeSelector, pod.Spec.NodeSelector) + } + // handles update of uninitialized pod like it's newly created. + err = handler.Admit(admission.NewAttributesRecord(pod, &oldPod, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil)) + if test.admit && err != nil { + t.Errorf("Test: %s, expected no error but got: %s", test.testName, err) + } else if !test.admit && err == nil { + t.Errorf("Test: %s, expected an error", test.testName) + } if test.admit && !labels.Equals(test.mergedNodeSelector, labels.Set(pod.Spec.NodeSelector)) { t.Errorf("Test: %s, expected: %s but got: %s", test.testName, test.mergedNodeSelector, pod.Spec.NodeSelector) } @@ -176,7 +192,7 @@ func TestPodAdmission(t *testing.T) { func TestHandles(t *testing.T) { for op, shouldHandle := range map[admission.Operation]bool{ admission.Create: true, - admission.Update: false, + admission.Update: true, admission.Connect: false, admission.Delete: false, } { @@ -187,6 +203,40 @@ func TestHandles(t *testing.T) { } } +func TestIgnoreUpdatingInitializedPod(t *testing.T) { + mockClient := &fake.Clientset{} + handler, informerFactory, err := newHandlerForTest(mockClient) + if err != nil { + t.Errorf("unexpected error initializing handler: %v", err) + } + handler.SetReadyFunc(func() bool { return true }) + + podNodeSelector := map[string]string{"infra": "false"} + pod := &api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"}, + Spec: api.PodSpec{NodeSelector: podNodeSelector}, + } + // this conflicts with podNodeSelector + namespaceNodeSelector := "infra=true" + namespace := &api.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testNamespace", + Namespace: "", + Annotations: map[string]string{"scheduler.alpha.kubernetes.io/node-selector": namespaceNodeSelector}, + }, + } + err = informerFactory.Core().InternalVersion().Namespaces().Informer().GetStore().Update(namespace) + if err != nil { + t.Fatal(err) + } + + // if the update of initialized pod is not ignored, an error will be returned because the pod's nodeSelector conflicts with namespace's nodeSelector. + err = handler.Admit(admission.NewAttributesRecord(pod, pod, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil)) + if err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + // newHandlerForTest returns the admission controller configured for testing. func newHandlerForTest(c clientset.Interface) (*podNodeSelector, informers.SharedInformerFactory, error) { f := informers.NewSharedInformerFactory(c, 5*time.Minute) diff --git a/plugin/pkg/admission/podtolerationrestriction/BUILD b/plugin/pkg/admission/podtolerationrestriction/BUILD index f8df1e2786b..929165c92bf 100644 --- a/plugin/pkg/admission/podtolerationrestriction/BUILD +++ b/plugin/pkg/admission/podtolerationrestriction/BUILD @@ -40,6 +40,7 @@ go_library( "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/client/listers/core/internalversion:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", + "//pkg/kubeapiserver/admission/util:go_default_library", "//pkg/util/tolerations:go_default_library", "//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction:go_default_library", "//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/install:go_default_library", diff --git a/plugin/pkg/admission/podtolerationrestriction/admission.go b/plugin/pkg/admission/podtolerationrestriction/admission.go index a71f92ba4a1..655a2e365a0 100644 --- a/plugin/pkg/admission/podtolerationrestriction/admission.go +++ b/plugin/pkg/admission/podtolerationrestriction/admission.go @@ -34,6 +34,7 @@ import ( informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" + "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/util/tolerations" pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" @@ -112,7 +113,11 @@ func (p *podTolerationsPlugin) Admit(a admission.Attributes) error { } var finalTolerations []api.Toleration - if a.GetOperation() == admission.Create { + updateUninitialized, err := util.IsUpdatingUninitializedObject(a) + if err != nil { + return err + } + if a.GetOperation() == admission.Create || updateUninitialized { ts, err := p.getNamespaceDefaultTolerations(namespace) if err != nil { return err diff --git a/plugin/pkg/admission/podtolerationrestriction/admission_test.go b/plugin/pkg/admission/podtolerationrestriction/admission_test.go index 76faaa71894..4673aa0692c 100644 --- a/plugin/pkg/admission/podtolerationrestriction/admission_test.go +++ b/plugin/pkg/admission/podtolerationrestriction/admission_test.go @@ -233,6 +233,11 @@ func TestPodAdmission(t *testing.T) { pod := test.pod pod.Spec.Tolerations = test.podTolerations + // copy the original pod for tests of uninitialized pod updates. + oldPod := *pod + oldPod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init"}}} + oldPod.Spec.Tolerations = []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil}} + err := handler.Admit(admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)) if test.admit && err != nil { t.Errorf("Test: %s, expected no error but got: %s", test.testName, err) @@ -244,6 +249,19 @@ func TestPodAdmission(t *testing.T) { if test.admit && !tolerations.EqualTolerations(updatedPodTolerations, test.mergedTolerations) { t.Errorf("Test: %s, expected: %#v but got: %#v", test.testName, test.mergedTolerations, updatedPodTolerations) } + + // handles update of uninitialized pod like it's newly created. + err = handler.Admit(admission.NewAttributesRecord(pod, &oldPod, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil)) + if test.admit && err != nil { + t.Errorf("Test: %s, expected no error but got: %s", test.testName, err) + } else if !test.admit && err == nil { + t.Errorf("Test: %s, expected an error", test.testName) + } + + updatedPodTolerations = pod.Spec.Tolerations + if test.admit && !tolerations.EqualTolerations(updatedPodTolerations, test.mergedTolerations) { + t.Errorf("Test: %s, expected: %#v but got: %#v", test.testName, test.mergedTolerations, updatedPodTolerations) + } } } @@ -267,6 +285,54 @@ func TestHandles(t *testing.T) { } } +func TestIgnoreUpdatingInitializedPod(t *testing.T) { + mockClient := &fake.Clientset{} + handler, informerFactory, err := newHandlerForTest(mockClient) + if err != nil { + t.Errorf("unexpected error initializing handler: %v", err) + } + handler.SetReadyFunc(func() bool { return true }) + + pod := &api.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"}, + Spec: api.PodSpec{}, + } + podToleration := api.Toleration{ + Key: "testKey", + Operator: "Equal", + Value: "testValue1", + Effect: "NoSchedule", + TolerationSeconds: nil, + } + pod.Spec.Tolerations = []api.Toleration{podToleration} + + // this conflicts with pod's Tolerations + namespaceToleration := podToleration + namespaceToleration.Value = "testValue2" + namespaceTolerations := []api.Toleration{namespaceToleration} + tolerationsStr, err := json.Marshal(namespaceTolerations) + if err != nil { + t.Errorf("error in marshalling namespace tolerations %v", namespaceTolerations) + } + namespace := &api.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testNamespace", + Namespace: "", + }, + } + namespace.Annotations = map[string]string{NSDefaultTolerations: string(tolerationsStr)} + err = informerFactory.Core().InternalVersion().Namespaces().Informer().GetStore().Update(namespace) + if err != nil { + t.Fatal(err) + } + + // if the update of initialized pod is not ignored, an error will be returned because the pod's Tolerations conflicts with namespace's Tolerations. + err = handler.Admit(admission.NewAttributesRecord(pod, pod, api.Kind("Pod").WithVersion("version"), "testNamespace", pod.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Update, nil)) + if err != nil { + t.Errorf("expected no error, got: %v", err) + } +} + // newHandlerForTest returns the admission controller configured for testing. func newHandlerForTest(c clientset.Interface) (*podTolerationsPlugin, informers.SharedInformerFactory, error) { f := informers.NewSharedInformerFactory(c, 5*time.Minute) diff --git a/plugin/pkg/admission/serviceaccount/BUILD b/plugin/pkg/admission/serviceaccount/BUILD index dc51db0a95a..0d9bf3a764d 100644 --- a/plugin/pkg/admission/serviceaccount/BUILD +++ b/plugin/pkg/admission/serviceaccount/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/client/listers/core/internalversion:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", + "//pkg/kubeapiserver/admission/util:go_default_library", "//pkg/serviceaccount:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 2b7c4abfb8f..9e319d26d6c 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -36,6 +36,7 @@ import ( informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" + "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/serviceaccount" ) @@ -90,7 +91,7 @@ var _ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&serviceAccount{ // 5. If MountServiceAccountToken is true, it adds a VolumeMount with the pod's ServiceAccount's api token secret to containers func NewServiceAccount() *serviceAccount { return &serviceAccount{ - Handler: admission.NewHandler(admission.Create), + Handler: admission.NewHandler(admission.Create, admission.Update), // TODO: enable this once we've swept secret usage to account for adding secret references to service accounts LimitSecretReferences: false, // Auto mount service account API token secrets @@ -143,6 +144,15 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) { return nil } + updateInitialized, err := util.IsUpdatingInitializedObject(a) + if err != nil { + return err + } + if updateInitialized { + // related pod spec fields are immutable after the pod is initialized + return nil + } + // Don't modify the spec of mirror pods. // That makes the kubelet very angry and confused, and it immediately deletes the pod (because the spec doesn't match) // That said, don't allow mirror pods to reference ServiceAccounts or SecretVolumeSources either diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index f5b86315566..80337481010 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -36,7 +36,7 @@ import ( func TestIgnoresNonCreate(t *testing.T) { pod := &api.Pod{} - for _, op := range []admission.Operation{admission.Update, admission.Delete, admission.Connect} { + for _, op := range []admission.Operation{admission.Delete, admission.Connect} { attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "myns", "myname", api.Resource("pods").WithVersion("version"), "", op, nil) handler := admission.NewChainHandler(NewServiceAccount()) err := handler.Admit(attrs) @@ -46,6 +46,17 @@ func TestIgnoresNonCreate(t *testing.T) { } } +func TestIgnoresUpdateOfInitializedPod(t *testing.T) { + pod := &api.Pod{} + oldPod := &api.Pod{} + attrs := admission.NewAttributesRecord(pod, oldPod, api.Kind("Pod").WithVersion("version"), "myns", "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, nil) + handler := admission.NewChainHandler(NewServiceAccount()) + err := handler.Admit(attrs) + if err != nil { + t.Errorf("Expected update of initialized pod allowed, got err: %v", err) + } +} + func TestIgnoresNonPodResource(t *testing.T) { pod := &api.Pod{} attrs := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "myns", "myname", api.Resource("CustomResource").WithVersion("version"), "", admission.Create, nil) @@ -309,6 +320,55 @@ func TestAutomountsAPIToken(t *testing.T) { t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) } + // Test ServiceAccount admission plugin applies the same changes if the + // operation is an update to an uninitialized pod. + oldPod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + // the volumeMount in the oldPod shouldn't affect the result. + VolumeMounts: []api.VolumeMount{ + { + Name: "wrong-" + tokenName, + ReadOnly: true, + MountPath: DefaultAPITokenMountPath, + }, + }, + }, + }, + }, + } + // oldPod is not intialized. + oldPod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "init"}}} + pod = &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + {}, + }, + }, + } + attrs = admission.NewAttributesRecord(pod, oldPod, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, nil) + err = admit.Admit(attrs) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pod.Spec.ServiceAccountName != DefaultServiceAccountName { + t.Errorf("Expected service account %s assigned, got %s", DefaultServiceAccountName, pod.Spec.ServiceAccountName) + } + if len(pod.Spec.Volumes) != 1 { + t.Fatalf("Expected 1 volume, got %d", len(pod.Spec.Volumes)) + } + if !reflect.DeepEqual(expectedVolume, pod.Spec.Volumes[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolume, pod.Spec.Volumes[0]) + } + if len(pod.Spec.Containers[0].VolumeMounts) != 1 { + t.Fatalf("Expected 1 volume mount, got %d", len(pod.Spec.Containers[0].VolumeMounts)) + } + if !reflect.DeepEqual(expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) { + t.Fatalf("Expected\n\t%#v\ngot\n\t%#v", expectedVolumeMount, pod.Spec.Containers[0].VolumeMounts[0]) + } + + // testing InitContainers pod = &api.Pod{ Spec: api.PodSpec{ InitContainers: []api.Container{