diff --git a/pkg/api/annotation_key_constants.go b/pkg/api/annotation_key_constants.go index 30331f982d3..e63417fcd88 100644 --- a/pkg/api/annotation_key_constants.go +++ b/pkg/api/annotation_key_constants.go @@ -17,6 +17,9 @@ limitations under the License. package api const ( + // MirrorAnnotationKey represents the annotation key set by kubelets when creating mirror pods + MirrorPodAnnotationKey string = "kubernetes.io/config.mirror" + // TolerationsAnnotationKey represents the key of tolerations data (json serialized) // in the Annotations of a Pod. TolerationsAnnotationKey string = "scheduler.alpha.kubernetes.io/tolerations" diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index bf69f1fa47e..350b424b97a 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -111,6 +111,12 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, spec *api.Pod allErrs = append(allErrs, ValidateAffinityInPodAnnotations(annotations, fldPath)...) } + if value, isMirror := annotations[api.MirrorPodAnnotationKey]; isMirror { + if len(spec.NodeName) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Key(api.MirrorPodAnnotationKey), value, "must set spec.nodeName if mirror pod annotation is set")) + } + } + if annotations[api.TolerationsAnnotationKey] != "" { allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath)...) } @@ -177,20 +183,26 @@ func ValidatePodSpecificAnnotationUpdates(newPod, oldPod *api.Pod, fldPath *fiel newAnnotations := newPod.Annotations oldAnnotations := oldPod.Annotations for k, oldVal := range oldAnnotations { - if newAnnotations[k] == oldVal { + if newVal, exists := newAnnotations[k]; exists && newVal == oldVal { continue // No change. } if strings.HasPrefix(k, apparmor.ContainerAnnotationKeyPrefix) { - allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not update AppArmor annotations")) + allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove or update AppArmor annotations")) + } + if k == api.MirrorPodAnnotationKey { + allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove or update mirror pod annotation")) } } - // Check for removals. + // Check for additions for k := range newAnnotations { if _, ok := oldAnnotations[k]; ok { continue // No change. } if strings.HasPrefix(k, apparmor.ContainerAnnotationKeyPrefix) { - allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove AppArmor annotations")) + allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not add AppArmor annotations")) + } + if k == api.MirrorPodAnnotationKey { + allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not add mirror pod annotation")) } } allErrs = append(allErrs, ValidatePodSpecificAnnotations(newAnnotations, &newPod.Spec, fldPath)...) @@ -2548,9 +2560,10 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList { // ValidatePodStatusUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields // that cannot be changed. func ValidatePodStatusUpdate(newPod, oldPod *api.Pod) field.ErrorList { - allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, field.NewPath("metadata")) + fldPath := field.NewPath("metadata") + allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) + allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"))...) - // TODO: allow change when bindings are properly decoupled from pods if newPod.Spec.NodeName != oldPod.Spec.NodeName { allErrs = append(allErrs, field.Forbidden(field.NewPath("status", "nodeName"), "may not be changed directly")) } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 1cb675a938e..21bf37d0001 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -4335,6 +4335,22 @@ func TestValidatePod(t *testing.T) { DNSPolicy: api.DNSClusterFirst, }, }, + "mirror-pod present without nodeName": { + ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns", Annotations: map[string]string{api.MirrorPodAnnotationKey: ""}}, + Spec: api.PodSpec{ + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + }, + }, + "mirror-pod populated without nodeName": { + ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns", Annotations: map[string]string{api.MirrorPodAnnotationKey: "foo"}}, + Spec: api.PodSpec{ + Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + }, + }, } for k, v := range errorCases { if errs := ValidatePod(&v); len(errs) == 0 { @@ -5056,6 +5072,30 @@ func TestValidatePodUpdate(t *testing.T) { false, "added invalid new toleration to existing tolerations in pod spec updates", }, + { + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: api.PodSpec{NodeName: "foo"}}, + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + false, + "removed nodeName from pod spec", + }, + { + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{api.MirrorPodAnnotationKey: ""}}, Spec: api.PodSpec{NodeName: "foo"}}, + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: api.PodSpec{NodeName: "foo"}}, + false, + "removed mirror pod annotation", + }, + { + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: api.PodSpec{NodeName: "foo"}}, + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{api.MirrorPodAnnotationKey: ""}}, Spec: api.PodSpec{NodeName: "foo"}}, + false, + "added mirror pod annotation", + }, + { + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{api.MirrorPodAnnotationKey: "foo"}}, Spec: api.PodSpec{NodeName: "foo"}}, + api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{api.MirrorPodAnnotationKey: "bar"}}, Spec: api.PodSpec{NodeName: "foo"}}, + false, + "changed mirror pod annotation", + }, } for _, test := range tests { diff --git a/pkg/kubelet/types/pod_update.go b/pkg/kubelet/types/pod_update.go index 72e1b14d65b..a5b4de85f88 100644 --- a/pkg/kubelet/types/pod_update.go +++ b/pkg/kubelet/types/pod_update.go @@ -26,7 +26,7 @@ import ( const ( ConfigSourceAnnotationKey = "kubernetes.io/config.source" - ConfigMirrorAnnotationKey = "kubernetes.io/config.mirror" + ConfigMirrorAnnotationKey = kubeapi.MirrorPodAnnotationKey ConfigFirstSeenAnnotationKey = "kubernetes.io/config.seen" ConfigHashAnnotationKey = "kubernetes.io/config.hash" CriticalPodAnnotationKey = "scheduler.alpha.kubernetes.io/critical-pod" diff --git a/plugin/pkg/admission/serviceaccount/BUILD b/plugin/pkg/admission/serviceaccount/BUILD index f3e2925e7aa..7faad45f80d 100644 --- a/plugin/pkg/admission/serviceaccount/BUILD +++ b/plugin/pkg/admission/serviceaccount/BUILD @@ -22,7 +22,6 @@ 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/kubelet/types: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 0da39c486ed..2c21f5d0105 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -36,7 +36,6 @@ 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" - kubelet "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/serviceaccount" ) @@ -146,7 +145,7 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) { // 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 - if _, isMirrorPod := pod.Annotations[kubelet.ConfigMirrorAnnotationKey]; isMirrorPod { + if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; isMirrorPod { if len(pod.Spec.ServiceAccountName) != 0 { return admission.NewForbidden(a, fmt.Errorf("a mirror pod may not reference service accounts")) } diff --git a/staging/src/k8s.io/client-go/pkg/api/annotation_key_constants.go b/staging/src/k8s.io/client-go/pkg/api/annotation_key_constants.go index 30331f982d3..e63417fcd88 100644 --- a/staging/src/k8s.io/client-go/pkg/api/annotation_key_constants.go +++ b/staging/src/k8s.io/client-go/pkg/api/annotation_key_constants.go @@ -17,6 +17,9 @@ limitations under the License. package api const ( + // MirrorAnnotationKey represents the annotation key set by kubelets when creating mirror pods + MirrorPodAnnotationKey string = "kubernetes.io/config.mirror" + // TolerationsAnnotationKey represents the key of tolerations data (json serialized) // in the Annotations of a Pod. TolerationsAnnotationKey string = "scheduler.alpha.kubernetes.io/tolerations"