Minor IP/CIDR validation cleanups/fixups

Remove unnecessary duplicate checks for pod.spec.podIPs /
pod.spec.hostIPs / node.spec.podCIDRs. (A list that is known to
contain exactly 2 values, where one is IPv4 and the other is IPv6,
cannot possibly contain duplicates.)

Fix a bad CIDR in the NetworkPolicy validation tests.

Fix some comment typos.
This commit is contained in:
Dan Winship 2023-12-27 09:44:45 -05:00
parent ea49618a74
commit 5e067b6781
3 changed files with 7 additions and 38 deletions

View File

@ -4118,9 +4118,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList {
allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...) allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...)
} }
// if we have more than one Pod.PodIP then // if we have more than one Pod.PodIP then we must have a dual-stack pair
// - validate for dual stack
// - validate for duplication
if len(pod.Status.PodIPs) > 1 { if len(pod.Status.PodIPs) > 1 {
podIPs := make([]string, 0, len(pod.Status.PodIPs)) podIPs := make([]string, 0, len(pod.Status.PodIPs))
for _, podIP := range pod.Status.PodIPs { for _, podIP := range pod.Status.PodIPs {
@ -4136,15 +4134,6 @@ func validatePodIPs(pod *core.Pod) field.ErrorList {
if !dualStack || len(podIPs) > 2 { if !dualStack || len(podIPs) > 2 {
allErrs = append(allErrs, field.Invalid(podIPsField, pod.Status.PodIPs, "may specify no more than one IP for each IP family")) allErrs = append(allErrs, field.Invalid(podIPsField, pod.Status.PodIPs, "may specify no more than one IP for each IP family"))
} }
// There should be no duplicates in list of Pod.PodIPs
seen := sets.Set[string]{} // := make(map[string]int)
for i, podIP := range pod.Status.PodIPs {
if seen.Has(podIP.IP) {
allErrs = append(allErrs, field.Duplicate(podIPsField.Index(i), podIP))
}
seen.Insert(podIP.IP)
}
} }
return allErrs return allErrs
@ -4165,25 +4154,16 @@ func validateHostIPs(pod *core.Pod) field.ErrorList {
allErrs = append(allErrs, field.Invalid(hostIPsField.Index(0).Child("ip"), pod.Status.HostIPs[0].IP, "must be equal to `hostIP`")) 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 // all HostIPs must be valid IPs
for i, hostIP := range pod.Status.HostIPs { for i, hostIP := range pod.Status.HostIPs {
allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...) allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...)
} }
// if we have more than one Pod.HostIP then // if we have more than one Pod.HostIP then we must have a dual-stack pair
// - validate for dual stack
// - validate for duplication
if len(pod.Status.HostIPs) > 1 { if len(pod.Status.HostIPs) > 1 {
seen := sets.Set[string]{}
hostIPs := make([]string, 0, len(pod.Status.HostIPs)) hostIPs := make([]string, 0, len(pod.Status.HostIPs))
for _, hostIP := range 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) 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) dualStack, err := netutils.IsDualStackIPStrings(hostIPs)
@ -6417,9 +6397,7 @@ func ValidateNode(node *core.Node) field.ErrorList {
allErrs = append(allErrs, validation.IsValidCIDR(podCIDRsField.Index(idx), value)...) allErrs = append(allErrs, validation.IsValidCIDR(podCIDRsField.Index(idx), value)...)
} }
// if more than PodCIDR then // if more than PodCIDR then it must be a dual-stack pair
// - validate for dual stack
// - validate for duplication
if len(node.Spec.PodCIDRs) > 1 { if len(node.Spec.PodCIDRs) > 1 {
dualStack, err := netutils.IsDualStackCIDRStrings(node.Spec.PodCIDRs) dualStack, err := netutils.IsDualStackCIDRStrings(node.Spec.PodCIDRs)
if err != nil { if err != nil {
@ -6428,15 +6406,6 @@ func ValidateNode(node *core.Node) field.ErrorList {
if !dualStack || len(node.Spec.PodCIDRs) > 2 { if !dualStack || len(node.Spec.PodCIDRs) > 2 {
allErrs = append(allErrs, field.Invalid(podCIDRsField, node.Spec.PodCIDRs, "may specify no more than one CIDR for each IP family")) allErrs = append(allErrs, field.Invalid(podCIDRsField, node.Spec.PodCIDRs, "may specify no more than one CIDR for each IP family"))
} }
// PodCIDRs must not contain duplicates
seen := sets.Set[string]{}
for i, value := range node.Spec.PodCIDRs {
if seen.Has(value) {
allErrs = append(allErrs, field.Duplicate(podCIDRsField.Index(i), value))
}
seen.Insert(value)
}
} }
} }

View File

@ -351,7 +351,7 @@ func ValidateIngressStatusUpdate(ingress, oldIngress *networking.Ingress) field.
return allErrs return allErrs
} }
// ValidateLIngressoadBalancerStatus validates required fields on an IngressLoadBalancerStatus // ValidateIngressLoadBalancerStatus validates required fields on an IngressLoadBalancerStatus
func ValidateIngressLoadBalancerStatus(status *networking.IngressLoadBalancerStatus, fldPath *field.Path) field.ErrorList { func ValidateIngressLoadBalancerStatus(status *networking.IngressLoadBalancerStatus, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for i, ingress := range status.Ingress { for i, ingress := range status.Ingress {

View File

@ -311,7 +311,7 @@ func TestValidateNetworkPolicy(t *testing.T) {
"except IP is outside of CIDR range": makeNetworkPolicyCustom(setIngressFromEmptyFirstElement, func(networkPolicy *networking.NetworkPolicy) { "except IP is outside of CIDR range": makeNetworkPolicyCustom(setIngressFromEmptyFirstElement, func(networkPolicy *networking.NetworkPolicy) {
networkPolicy.Spec.Ingress[0].From[0].IPBlock = &networking.IPBlock{ networkPolicy.Spec.Ingress[0].From[0].IPBlock = &networking.IPBlock{
CIDR: "192.168.8.0/24", CIDR: "192.168.8.0/24",
Except: []string{"192.168.9.1/24"}, Except: []string{"192.168.9.1/32"},
} }
}), }),
"except IP is not strictly within CIDR range": makeNetworkPolicyCustom(setIngressFromEmptyFirstElement, func(networkPolicy *networking.NetworkPolicy) { "except IP is not strictly within CIDR range": makeNetworkPolicyCustom(setIngressFromEmptyFirstElement, func(networkPolicy *networking.NetworkPolicy) {