github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 22:46:12 +00:00
Code Issues Projects Releases Wiki Activity

Fix podIP validation

Browse Source
This commit is contained in:
Jordan Liggitt 2020-04-29 22:25:03 -04:00
parent 7814f3aaf7
commit 23e9fb1bb5
7 changed files with 115 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
pkg/apis/core/validation/validation.go
View File

@ -3113,8 +3113,9 @@ func ValidatePodSingleHugePageResources(pod *core.Pod, specPath *field.Path) fie
return allErrs
}
// ValidatePod tests if required fields in the pod are set.
func ValidatePod(pod *core.Pod, opts PodValidationOptions) field.ErrorList {
// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,
// and is called by ValidatePodCreate and ValidatePodUpdate.
func validatePodMetadataAndSpec(pod *core.Pod, opts PodValidationOptions) field.ErrorList {
fldPath := field.NewPath("metadata")
allErrs := ValidateObjectMeta(&pod.ObjectMeta, true, ValidatePodName, fldPath)
allErrs = append(allErrs, ValidatePodSpecificAnnotations(pod.ObjectMeta.Annotations, &pod.Spec, fldPath.Child("annotations"))...)
@ -3145,6 +3146,13 @@ func ValidatePod(pod *core.Pod, opts PodValidationOptions) field.ErrorList {
allErrs = append(allErrs, ValidatePodSingleHugePageResources(pod, specPath)...)
}
return allErrs
}
// validatePodIPs validates IPs in pod status
func validatePodIPs(pod *core.Pod) field.ErrorList {
allErrs := field.ErrorList{}
podIPsField := field.NewPath("status", "podIPs")
// all PodIPs must be valid IPs
@ -3705,7 +3713,7 @@ func ValidateContainerUpdates(newContainers, oldContainers []core.Container, fld
// ValidatePodCreate validates a pod in the context of its initial create
func ValidatePodCreate(pod *core.Pod, opts PodValidationOptions) field.ErrorList {
allErrs := ValidatePod(pod, opts)
allErrs := validatePodMetadataAndSpec(pod, opts)
fldPath := field.NewPath("spec")
// EphemeralContainers can only be set on update using the ephemeralcontainers subresource
@ -3721,6 +3729,7 @@ func ValidatePodCreate(pod *core.Pod, opts PodValidationOptions) field.ErrorList
func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList {
fldPath := field.NewPath("metadata")
allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...)
allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"))...)
specPath := field.NewPath("spec")
@ -3850,6 +3859,14 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod) field.ErrorList {
allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.ContainerStatuses, oldPod.Status.ContainerStatuses, fldPath.Child("containerStatuses"), oldPod.Spec.RestartPolicy)...)
allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.InitContainerStatuses, oldPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), oldPod.Spec.RestartPolicy)...)
if newIPErrs := validatePodIPs(newPod); len(newIPErrs) > 0 {
// Tolerate IP errors if IP errors already existed in the old pod. See http://issue.k8s.io/90625
// TODO(liggitt): Drop the check of oldPod in 1.20
if oldIPErrs := validatePodIPs(oldPod); len(oldIPErrs) == 0 {
allErrs = append(allErrs, newIPErrs...)
}
}
// For status update we ignore changes to pod spec.
newPod.Spec = oldPod.Spec

110
pkg/apis/core/validation/validation_test.go
View File

@ -4041,7 +4041,7 @@ func TestHugePagesIsolation(t *testing.T) {
for tcName, tc := range testCases {
t.Run(tcName, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePageStorageMediumSize, tc.enableHugePageStorageMediumSize)()
errs := ValidatePod(tc.pod, PodValidationOptions{tc.enableHugePageStorageMediumSize})
errs := ValidatePodCreate(tc.pod, PodValidationOptions{tc.enableHugePageStorageMediumSize})
if tc.expectError && len(errs) == 0 {
t.Errorf("Unexpected success")
}
@ -7426,7 +7426,7 @@ func TestValidatePod(t *testing.T) {
},
}
for _, pod := range successCases {
if errs := ValidatePod(&pod, PodValidationOptions{}); len(errs) != 0 {
if errs := ValidatePodCreate(&pod, PodValidationOptions{}); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
}
@ -8276,7 +8276,7 @@ func TestValidatePod(t *testing.T) {
},
}
for k, v := range errorCases {
if errs := ValidatePod(&v.spec, PodValidationOptions{}); len(errs) == 0 {
if errs := ValidatePodCreate(&v.spec, PodValidationOptions{}); len(errs) == 0 {
t.Errorf("expected failure for %q", k)
} else if v.expectedError == "" {
t.Errorf("missing expectedError for %q, got %q", k, errs.ToAggregate().Error())
@ -8485,7 +8485,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
Containers: []core.Container{
{
Image: "foo:V1",
Name: "container",
Image: "foo:V1",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8495,7 +8498,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
Containers: []core.Container{
{
Image: "foo:V2",
Name: "container",
Image: "foo:V2",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8509,7 +8515,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
InitContainers: []core.Container{
{
Image: "foo:V1",
Name: "container",
Image: "foo:V1",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8519,7 +8528,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
InitContainers: []core.Container{
{
Image: "foo:V2",
Name: "container",
Image: "foo:V2",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8532,7 +8544,11 @@ func TestValidatePodUpdate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: core.PodSpec{
Containers: []core.Container{
{},
{
Name: "container",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
},
@ -8541,7 +8557,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
Containers: []core.Container{
{
Image: "foo:V2",
Name: "container",
Image: "foo:V2",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8554,7 +8573,11 @@ func TestValidatePodUpdate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: core.PodSpec{
InitContainers: []core.Container{
{},
{
Name: "container",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
},
@ -8563,7 +8586,10 @@ func TestValidatePodUpdate(t *testing.T) {
Spec: core.PodSpec{
InitContainers: []core.Container{
{
Image: "foo:V2",
Name: "container",
Image: "foo:V2",
TerminationMessagePolicy: "File",
ImagePullPolicy: "Always",
},
},
},
@ -8690,7 +8716,7 @@ func TestValidatePodUpdate(t *testing.T) {
ActiveDeadlineSeconds: &activeDeadlineSecondsPositive,
},
},
"",
"spec.activeDeadlineSeconds",
"activeDeadlineSeconds change to zero from positive",
},
{
@ -8700,7 +8726,7 @@ func TestValidatePodUpdate(t *testing.T) {
},
},
core.Pod{},
"",
"spec.activeDeadlineSeconds",
"activeDeadlineSeconds change to zero from nil",
},
{
@ -9047,6 +9073,29 @@ func TestValidatePodUpdate(t *testing.T) {
for _, test := range tests {
test.new.ObjectMeta.ResourceVersion = "1"
test.old.ObjectMeta.ResourceVersion = "1"
// set required fields if old and new match and have no opinion on the value
if test.new.Name == "" && test.old.Name == "" {
test.new.Name = "name"
test.old.Name = "name"
}
if test.new.Namespace == "" && test.old.Namespace == "" {
test.new.Namespace = "namespace"
test.old.Namespace = "namespace"
}
if test.new.Spec.Containers == nil && test.old.Spec.Containers == nil {
test.new.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
test.old.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
}
if len(test.new.Spec.DNSPolicy) == 0 && len(test.old.Spec.DNSPolicy) == 0 {
test.new.Spec.DNSPolicy = core.DNSClusterFirst
test.old.Spec.DNSPolicy = core.DNSClusterFirst
}
if len(test.new.Spec.RestartPolicy) == 0 && len(test.old.Spec.RestartPolicy) == 0 {
test.new.Spec.RestartPolicy = "Always"
test.old.Spec.RestartPolicy = "Always"
}
errs := ValidatePodUpdate(&test.new, &test.old, PodValidationOptions{})
if test.err == "" {
if len(errs) != 0 {
@ -15031,15 +15080,32 @@ func TestPodIPsValidation(t *testing.T) {
}
for _, testCase := range testCases {
errs := ValidatePod(&testCase.pod, PodValidationOptions{})
if len(errs) == 0 && testCase.expectError {
t.Errorf("expected failure for %s, but there were none", testCase.pod.Name)
return
}
if len(errs) != 0 && !testCase.expectError {
t.Errorf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
return
}
t.Run(testCase.pod.Name, func(t *testing.T) {
for _, oldTestCase := range testCases {
newPod := testCase.pod.DeepCopy()
newPod.ResourceVersion = "1"
oldPod := oldTestCase.pod.DeepCopy()
oldPod.ResourceVersion = "1"
oldPod.Name = newPod.Name
errs := ValidatePodStatusUpdate(newPod, oldPod)
if oldTestCase.expectError {
// The old pod was invalid, tolerate invalid IPs in the new pod as well
if len(errs) > 0 {
t.Fatalf("expected success for update to pod with already-invalid IPs, got errors: %v", errs)
}
continue
}
if len(errs) == 0 && testCase.expectError {
t.Fatalf("expected failure for %s, but there were none", testCase.pod.Name)
}
if len(errs) != 0 && !testCase.expectError {
t.Fatalf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
}
}
})
}
}

4
pkg/kubelet/config/common.go
View File

@ -138,7 +138,7 @@ func tryDecodeSinglePod(data []byte, defaultFn defaultFunc) (parsed bool, pod *v
opts := validation.PodValidationOptions{
AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize),
}
if errs := validation.ValidatePod(newPod, opts); len(errs) > 0 {
if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 {
return true, pod, fmt.Errorf("invalid pod: %v", errs)
}
v1Pod := &v1.Pod{}
@ -172,7 +172,7 @@ func tryDecodePodList(data []byte, defaultFn defaultFunc) (parsed bool, pods v1.
if err = defaultFn(newPod); err != nil {
return true, pods, err
}
if errs := validation.ValidatePod(newPod, opts); len(errs) > 0 {
if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 {
err = fmt.Errorf("invalid pod: %v", errs)
return true, pods, err
}

2
pkg/kubelet/config/common_test.go
View File

@ -251,7 +251,7 @@ func TestStaticPodNameGenerate(t *testing.T) {
if c.overwrite != "" {
pod.Name = c.overwrite
}
errs := validation.ValidatePod(pod, validation.PodValidationOptions{})
errs := validation.ValidatePodCreate(pod, validation.PodValidationOptions{})
if c.shouldErr {
specNameErrored := false
for _, err := range errs {

4
pkg/kubelet/config/file_linux_test.go
View File

@ -90,7 +90,7 @@ func TestReadPodsFromFileExistAlready(t *testing.T) {
if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil {
t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err)
}
if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) > 0 {
if errs := validation.ValidatePodCreate(internalPod, validation.PodValidationOptions{}); len(errs) > 0 {
t.Fatalf("%s: Invalid pod %#v, %#v", testCase.desc, internalPod, errs)
}
}
@ -369,7 +369,7 @@ func expectUpdate(t *testing.T, ch chan interface{}, testCase *testCase) {
if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil {
t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err)
}
if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) > 0 {
if errs := validation.ValidatePodCreate(internalPod, validation.PodValidationOptions{}); len(errs) > 0 {
t.Fatalf("%s: Invalid pod %#v, %#v", testCase.desc, internalPod, errs)
}
}

2
pkg/kubelet/config/http_test.go
View File

@ -319,7 +319,7 @@ func TestExtractPodsFromHTTP(t *testing.T) {
if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil {
t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err)
}
if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) != 0 {
if errs := validation.ValidatePodCreate(internalPod, validation.PodValidationOptions{}); len(errs) != 0 {
t.Errorf("%s: Expected no validation errors on %#v, Got %v", testCase.desc, pod, errs.ToAggregate())
}
}

3
pkg/registry/core/pod/strategy.go
View File

@ -113,8 +113,7 @@ func (podStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object)
// Allow multiple huge pages on pod create if feature is enabled or if the old pod already has multiple hugepages specified
AllowMultipleHugePageResources: oldFailsSingleHugepagesValidation || utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize),
}
errorList := validation.ValidatePod(obj.(*api.Pod), opts)
errorList = append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod), opts)...)
errorList := validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod), opts)
errorList = append(errorList, validation.ValidateConditionalPod(obj.(*api.Pod), old.(*api.Pod), field.NewPath(""))...)
return errorList
}