Skip to content

Commit 7cd32ab

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request #45775 from liggitt/mirror-pod-validation
Automatic merge from submit-queue (batch tested with PRs 44337, 45775, 45832, 45574, 45758) Tighten validation of mirror pod annotations Tightens validation for pods with a mirror pod annotation: 1. spec.nodeName must be set 2. makes the mirror pod annotation immutable 3. starts validating pod-specific annotations during pod status update None of these changes affect usage of the mirror pod annotation by kubelets, which only set it on pod creation (verified this is true back to 1.5.x) the second commit updates the pod validation tests to look for specific error messages (best reviewed ignoring whitespace changes) This is the validation portion of https://github.com/kubernetes/community/blob/master/contributors/design-proposals/kubelet-authorizer.md and kubernetes/enhancements#279 ```release-note Mirror pods must now indicate the nodeName they are bound to on creation. The mirror pod annotation is now treated as immutable and cannot be added to an existing pod, removed from a pod, or modified. ```
2 parents eee8598 + eb0e4fa commit 7cd32ab

File tree

7 files changed

+596
-413
lines changed

7 files changed

+596
-413
lines changed

Diff for: pkg/api/annotation_key_constants.go

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ limitations under the License.
1717
package api
1818

1919
const (
20+
// MirrorAnnotationKey represents the annotation key set by kubelets when creating mirror pods
21+
MirrorPodAnnotationKey string = "kubernetes.io/config.mirror"
22+
2023
// TolerationsAnnotationKey represents the key of tolerations data (json serialized)
2124
// in the Annotations of a Pod.
2225
TolerationsAnnotationKey string = "scheduler.alpha.kubernetes.io/tolerations"

Diff for: pkg/api/validation/validation.go

+19-6
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ func ValidatePodSpecificAnnotations(annotations map[string]string, spec *api.Pod
111111
allErrs = append(allErrs, ValidateAffinityInPodAnnotations(annotations, fldPath)...)
112112
}
113113

114+
if value, isMirror := annotations[api.MirrorPodAnnotationKey]; isMirror {
115+
if len(spec.NodeName) == 0 {
116+
allErrs = append(allErrs, field.Invalid(fldPath.Key(api.MirrorPodAnnotationKey), value, "must set spec.nodeName if mirror pod annotation is set"))
117+
}
118+
}
119+
114120
if annotations[api.TolerationsAnnotationKey] != "" {
115121
allErrs = append(allErrs, ValidateTolerationsInPodAnnotations(annotations, fldPath)...)
116122
}
@@ -177,20 +183,26 @@ func ValidatePodSpecificAnnotationUpdates(newPod, oldPod *api.Pod, fldPath *fiel
177183
newAnnotations := newPod.Annotations
178184
oldAnnotations := oldPod.Annotations
179185
for k, oldVal := range oldAnnotations {
180-
if newAnnotations[k] == oldVal {
186+
if newVal, exists := newAnnotations[k]; exists && newVal == oldVal {
181187
continue // No change.
182188
}
183189
if strings.HasPrefix(k, apparmor.ContainerAnnotationKeyPrefix) {
184-
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not update AppArmor annotations"))
190+
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove or update AppArmor annotations"))
191+
}
192+
if k == api.MirrorPodAnnotationKey {
193+
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove or update mirror pod annotation"))
185194
}
186195
}
187-
// Check for removals.
196+
// Check for additions
188197
for k := range newAnnotations {
189198
if _, ok := oldAnnotations[k]; ok {
190199
continue // No change.
191200
}
192201
if strings.HasPrefix(k, apparmor.ContainerAnnotationKeyPrefix) {
193-
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not remove AppArmor annotations"))
202+
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not add AppArmor annotations"))
203+
}
204+
if k == api.MirrorPodAnnotationKey {
205+
allErrs = append(allErrs, field.Forbidden(fldPath.Key(k), "may not add mirror pod annotation"))
194206
}
195207
}
196208
allErrs = append(allErrs, ValidatePodSpecificAnnotations(newAnnotations, &newPod.Spec, fldPath)...)
@@ -2548,9 +2560,10 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) field.ErrorList {
25482560
// ValidatePodStatusUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields
25492561
// that cannot be changed.
25502562
func ValidatePodStatusUpdate(newPod, oldPod *api.Pod) field.ErrorList {
2551-
allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, field.NewPath("metadata"))
2563+
fldPath := field.NewPath("metadata")
2564+
allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
2565+
allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"))...)
25522566

2553-
// TODO: allow change when bindings are properly decoupled from pods
25542567
if newPod.Spec.NodeName != oldPod.Spec.NodeName {
25552568
allErrs = append(allErrs, field.Forbidden(field.NewPath("status", "nodeName"), "may not be changed directly"))
25562569
}

0 commit comments

Comments
 (0)