From bf030fd68ab20720dd5b9180b0093540798cf361 Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Sat, 27 May 2023 23:35:13 +0800 Subject: [PATCH] Add validate HostIPs --- pkg/apis/core/validation/validation.go | 56 ++++++++++ pkg/apis/core/validation/validation_test.go | 107 ++++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index c6ec53521cb..859fa3fa988 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3817,6 +3817,58 @@ func validatePodIPs(pod *core.Pod) field.ErrorList { return allErrs } +// validateHostIPs validates IPs in pod status +func validateHostIPs(pod *core.Pod) field.ErrorList { + allErrs := field.ErrorList{} + + if len(pod.Status.HostIPs) == 0 { + return allErrs + } + + hostIPsField := field.NewPath("status", "hostIPs") + + // hostIP must be equal to hostIPs[0].IP + if pod.Status.HostIP != pod.Status.HostIPs[0].IP { + allErrs = append(allErrs, field.Invalid(hostIPsField.Index(0).Child("ip"), pod.Status.HostIPs[0].IP, "must be equal to `hostIP`")) + } + + // all HostPs must be valid IPs + for i, hostIP := range pod.Status.HostIPs { + for _, msg := range validation.IsValidIP(hostIP.IP) { + allErrs = append(allErrs, field.Invalid(hostIPsField.Index(i), hostIP.IP, msg)) + } + } + + // if we have more than one Pod.HostIP then + // - validate for dual stack + // - validate for duplication + if len(pod.Status.HostIPs) > 1 { + seen := sets.String{} + hostIPs := make([]string, 0, len(pod.Status.HostIPs)) + + // There should be no duplicates in list of Pod.HostIPs + for i, hostIP := range pod.Status.HostIPs { + hostIPs = append(hostIPs, hostIP.IP) + if seen.Has(hostIP.IP) { + allErrs = append(allErrs, field.Duplicate(hostIPsField.Index(i), hostIP)) + } + seen.Insert(hostIP.IP) + } + + dualStack, err := netutils.IsDualStackIPStrings(hostIPs) + if err != nil { + allErrs = append(allErrs, field.InternalError(hostIPsField, fmt.Errorf("failed to check for dual stack with error:%v", err))) + } + + // We only support one from each IP family (i.e. max two IPs in this list). + if !dualStack || len(hostIPs) > 2 { + allErrs = append(allErrs, field.Invalid(hostIPsField, pod.Status.HostIPs, "may specify no more than one IP for each IP family")) + } + } + + return allErrs +} + // ValidatePodSpec tests that the specified PodSpec has valid data. // This includes checking formatting and uniqueness. It also canonicalizes the // structure by setting default values and implementing any backwards-compatibility @@ -4853,6 +4905,10 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions allErrs = append(allErrs, newIPErrs...) } + if newIPErrs := validateHostIPs(newPod); len(newIPErrs) > 0 { + allErrs = append(allErrs, newIPErrs...) + } + return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index bfa33c179ce..7b87ba7722b 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -21352,6 +21352,113 @@ func TestPodIPsValidation(t *testing.T) { } } +func makePodWithHostIPs(podName string, podNamespace string, hostIPs []core.HostIP) core.Pod { + hostIP := "" + if len(hostIPs) > 0 { + hostIP = hostIPs[0].IP + } + return core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: podNamespace}, + Spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + Status: core.PodStatus{ + HostIP: hostIP, + HostIPs: hostIPs, + }, + } +} + +func TestHostIPsValidation(t *testing.T) { + testCases := []struct { + pod core.Pod + expectError bool + }{ + { + expectError: false, + pod: makePodWithHostIPs("nil-ips", "ns", nil), + }, + { + expectError: false, + pod: makePodWithHostIPs("empty-HostIPs-list", "ns", []core.HostIP{}), + }, + { + expectError: false, + pod: makePodWithHostIPs("single-ip-family-6", "ns", []core.HostIP{{IP: "::1"}}), + }, + { + expectError: false, + pod: makePodWithHostIPs("single-ip-family-4", "ns", []core.HostIP{{IP: "1.1.1.1"}}), + }, + { + expectError: false, + pod: makePodWithHostIPs("dual-stack-4-6", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "::1"}}), + }, + { + expectError: false, + pod: makePodWithHostIPs("dual-stack-6-4", "ns", []core.HostIP{{IP: "::1"}, {IP: "1.1.1.1"}}), + }, + /* failure cases start here */ + { + expectError: true, + pod: makePodWithHostIPs("invalid-pod-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}), + }, + { + expectError: true, + pod: makePodWithHostIPs("dualstack-same-ip-family-6", "ns", []core.HostIP{{IP: "::1"}, {IP: "::2"}}), + }, + { + expectError: true, + pod: makePodWithHostIPs("dualstack-same-ip-family-4", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "2.2.2.2"}}), + }, + { + expectError: true, + pod: makePodWithHostIPs("dualstack-repeated-ip-family-6", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "::1"}, {IP: "::2"}}), + }, + { + expectError: true, + pod: makePodWithHostIPs("dualstack-repeated-ip-family-4", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "::1"}, {IP: "2.2.2.2"}}), + }, + + { + expectError: true, + pod: makePodWithHostIPs("dualstack-duplicate-ip-family-4", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "1.1.1.1"}, {IP: "::1"}}), + }, + { + expectError: true, + pod: makePodWithHostIPs("dualstack-duplicate-ip-family-6", "ns", []core.HostIP{{IP: "1.1.1.1"}, {IP: "::1"}, {IP: "::1"}}), + }, + } + + for _, testCase := range testCases { + 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, PodValidationOptions{}) + + 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) + } + } + }) + } +} + // makes a node with pod cidr and a name func makeNode(nodeName string, podCIDRs []string) core.Node { return core.Node{