github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-14 06:15:45 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #122550 from danwinship/tighten-ip-validation

Browse Source 
Tighten IP/CIDR validation
This commit is contained in:
Kubernetes Prow Robot 2025-03-12 15:57:46 -07:00 committed by GitHub
commit 2261137135
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 1682 additions and 359 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -3790,7 +3790,7 @@ func validatePodDNSConfig(dnsConfig *core.PodDNSConfig, dnsPolicy *core.DNSPolic
allErrs = append(allErrs, field.Invalid(fldPath.Child("nameservers"), dnsConfig.Nameservers, fmt.Sprintf("must not have more than %v nameservers", MaxDNSNameservers))) allErrs = append(allErrs, field.Invalid(fldPath.Child("nameservers"), dnsConfig.Nameservers, fmt.Sprintf("must not have more than %v nameservers", MaxDNSNameservers)))
} }
for i, ns := range dnsConfig.Nameservers { for i, ns := range dnsConfig.Nameservers {
allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("nameservers").Index(i), ns)...) allErrs = append(allErrs, IsValidIPForLegacyField(fldPath.Child("nameservers").Index(i), ns, nil)...)
} }
// Validate searches. // Validate searches.
if len(dnsConfig.Searches) > MaxDNSSearchPaths { if len(dnsConfig.Searches) > MaxDNSSearchPaths {
@ -3966,7 +3966,7 @@ func validateOnlyDeletedSchedulingGates(newGates, oldGates []core.PodSchedulingG
func ValidateHostAliases(hostAliases []core.HostAlias, fldPath *field.Path) field.ErrorList { func ValidateHostAliases(hostAliases []core.HostAlias, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for i, hostAlias := range hostAliases { for i, hostAlias := range hostAliases {
allErrs = append(allErrs, validation.IsValidIP(fldPath.Index(i).Child("ip"), hostAlias.IP)...) allErrs = append(allErrs, IsValidIPForLegacyField(fldPath.Index(i).Child("ip"), hostAlias.IP, nil)...)
for j, hostname := range hostAlias.Hostnames { for j, hostname := range hostAlias.Hostnames {
allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Index(i).Child("hostnames").Index(j))...) allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Index(i).Child("hostnames").Index(j))...)
} }
@ -4108,14 +4108,21 @@ func validatePodMetadataAndSpec(pod *core.Pod, opts PodValidationOptions) field.
} }
// validatePodIPs validates IPs in pod status // validatePodIPs validates IPs in pod status
func validatePodIPs(pod *core.Pod) field.ErrorList { func validatePodIPs(pod, oldPod *core.Pod) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
podIPsField := field.NewPath("status", "podIPs") podIPsField := field.NewPath("status", "podIPs")
// all PodIPs must be valid IPs // all new PodIPs must be valid IPs, but existing invalid ones can be kept.
var existingPodIPs []string
if oldPod != nil {
existingPodIPs = make([]string, len(oldPod.Status.PodIPs))
for i, podIP := range oldPod.Status.PodIPs {
existingPodIPs[i] = podIP.IP
}
}
for i, podIP := range pod.Status.PodIPs { for i, podIP := range pod.Status.PodIPs {
allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...) allErrs = append(allErrs, IsValidIPForLegacyField(podIPsField.Index(i), podIP.IP, existingPodIPs)...)
} }
// if we have more than one Pod.PodIP then we must have a dual-stack pair // if we have more than one Pod.PodIP then we must have a dual-stack pair
@ -4140,7 +4147,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList {
} }
// validateHostIPs validates IPs in pod status // validateHostIPs validates IPs in pod status
func validateHostIPs(pod *core.Pod) field.ErrorList { func validateHostIPs(pod, oldPod *core.Pod) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
if len(pod.Status.HostIPs) == 0 { if len(pod.Status.HostIPs) == 0 {
@ -4154,9 +4161,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 HostIPs must be valid IPs // all new HostIPs must be valid IPs, but existing invalid ones can be kept.
var existingHostIPs []string
if oldPod != nil {
existingHostIPs = make([]string, len(oldPod.Status.HostIPs))
for i, hostIP := range oldPod.Status.HostIPs {
existingHostIPs[i] = hostIP.IP
}
}
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, IsValidIPForLegacyField(hostIPsField.Index(i), hostIP.IP, existingHostIPs)...)
} }
// if we have more than one Pod.HostIP then we must have a dual-stack pair // if we have more than one Pod.HostIP then we must have a dual-stack pair
@ -5462,11 +5476,11 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions
allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.EphemeralContainerStatuses, oldPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"), core.RestartPolicyNever)...) allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.EphemeralContainerStatuses, oldPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"), core.RestartPolicyNever)...)
allErrs = append(allErrs, validatePodResourceClaimStatuses(newPod.Status.ResourceClaimStatuses, newPod.Spec.ResourceClaims, fldPath.Child("resourceClaimStatuses"))...) allErrs = append(allErrs, validatePodResourceClaimStatuses(newPod.Status.ResourceClaimStatuses, newPod.Spec.ResourceClaims, fldPath.Child("resourceClaimStatuses"))...)
if newIPErrs := validatePodIPs(newPod); len(newIPErrs) > 0 { if newIPErrs := validatePodIPs(newPod, oldPod); len(newIPErrs) > 0 {
allErrs = append(allErrs, newIPErrs...) allErrs = append(allErrs, newIPErrs...)
} }
if newIPErrs := validateHostIPs(newPod); len(newIPErrs) > 0 { if newIPErrs := validateHostIPs(newPod, oldPod); len(newIPErrs) > 0 {
allErrs = append(allErrs, newIPErrs...) allErrs = append(allErrs, newIPErrs...)
} }
@ -5860,7 +5874,7 @@ var supportedServiceIPFamilyPolicy = sets.New(
core.IPFamilyPolicyRequireDualStack) core.IPFamilyPolicyRequireDualStack)
// ValidateService tests if required fields/annotations of a Service are valid. // ValidateService tests if required fields/annotations of a Service are valid.
func ValidateService(service *core.Service) field.ErrorList { func ValidateService(service, oldService *core.Service) field.ErrorList {
metaPath := field.NewPath("metadata") metaPath := field.NewPath("metadata")
allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, metaPath) allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, metaPath)
@ -5935,15 +5949,24 @@ func ValidateService(service *core.Service) field.ErrorList {
} }
// dualstack <-> ClusterIPs <-> ipfamilies // dualstack <-> ClusterIPs <-> ipfamilies
allErrs = append(allErrs, ValidateServiceClusterIPsRelatedFields(service)...) allErrs = append(allErrs, ValidateServiceClusterIPsRelatedFields(service, oldService)...)
// All new ExternalIPs must be valid and "non-special". (Existing ExternalIPs may
// have been validated against older rules, but if we allowed them before we can't
// reject them now.)
ipPath := specPath.Child("externalIPs") ipPath := specPath.Child("externalIPs")
var existingExternalIPs []string
if oldService != nil {
existingExternalIPs = oldService.Spec.ExternalIPs // +k8s:verify-mutation:reason=clone
}
for i, ip := range service.Spec.ExternalIPs { for i, ip := range service.Spec.ExternalIPs {
idxPath := ipPath.Index(i) idxPath := ipPath.Index(i)
if errs := validation.IsValidIP(idxPath, ip); len(errs) != 0 { if errs := IsValidIPForLegacyField(idxPath, ip, existingExternalIPs); len(errs) != 0 {
allErrs = append(allErrs, errs...) allErrs = append(allErrs, errs...)
} else { } else {
allErrs = append(allErrs, ValidateNonSpecialIP(ip, idxPath)...) // For historical reasons, this uses ValidateEndpointIP even
// though that is not exactly the appropriate set of checks.
allErrs = append(allErrs, ValidateEndpointIP(ip, idxPath)...)
} }
} }
@ -5995,18 +6018,27 @@ func ValidateService(service *core.Service) field.ErrorList {
ports[key] = true ports[key] = true
} }
// Validate SourceRanges field or annotation. // Validate SourceRanges field or annotation. Existing invalid CIDR values do not
// need to be fixed. Note that even with the tighter CIDR validation we still
// allow excess whitespace, because that is effectively part of the API.
if len(service.Spec.LoadBalancerSourceRanges) > 0 { if len(service.Spec.LoadBalancerSourceRanges) > 0 {
fieldPath := specPath.Child("LoadBalancerSourceRanges") fieldPath := specPath.Child("LoadBalancerSourceRanges")
if service.Spec.Type != core.ServiceTypeLoadBalancer { if service.Spec.Type != core.ServiceTypeLoadBalancer {
allErrs = append(allErrs, field.Forbidden(fieldPath, "may only be used when `type` is 'LoadBalancer'")) allErrs = append(allErrs, field.Forbidden(fieldPath, "may only be used when `type` is 'LoadBalancer'"))
} }
var existingSourceRanges []string
if oldService != nil {
existingSourceRanges = make([]string, len(oldService.Spec.LoadBalancerSourceRanges))
for i, value := range oldService.Spec.LoadBalancerSourceRanges {
existingSourceRanges[i] = strings.TrimSpace(value)
}
}
for idx, value := range service.Spec.LoadBalancerSourceRanges { for idx, value := range service.Spec.LoadBalancerSourceRanges {
// Note: due to a historical accident around transition from the // Note: due to a historical accident around transition from the
// annotation value, these values are allowed to be space-padded. // annotation value, these values are allowed to be space-padded.
value = strings.TrimSpace(value) value = strings.TrimSpace(value)
allErrs = append(allErrs, validation.IsValidCIDR(fieldPath.Index(idx), value)...) allErrs = append(allErrs, IsValidCIDRForLegacyField(fieldPath.Index(idx), value, existingSourceRanges)...)
} }
} else if val, annotationSet := service.Annotations[core.AnnotationLoadBalancerSourceRangesKey]; annotationSet { } else if val, annotationSet := service.Annotations[core.AnnotationLoadBalancerSourceRangesKey]; annotationSet {
fieldPath := field.NewPath("metadata", "annotations").Key(core.AnnotationLoadBalancerSourceRangesKey) fieldPath := field.NewPath("metadata", "annotations").Key(core.AnnotationLoadBalancerSourceRangesKey)
@ -6014,12 +6046,14 @@ func ValidateService(service *core.Service) field.ErrorList {
allErrs = append(allErrs, field.Forbidden(fieldPath, "may only be used when `type` is 'LoadBalancer'")) allErrs = append(allErrs, field.Forbidden(fieldPath, "may only be used when `type` is 'LoadBalancer'"))
} }
val = strings.TrimSpace(val) if oldService == nil || oldService.Annotations[core.AnnotationLoadBalancerSourceRangesKey] != val {
if val != "" { val = strings.TrimSpace(val)
cidrs := strings.Split(val, ",") if val != "" {
for _, value := range cidrs { cidrs := strings.Split(val, ",")
value = strings.TrimSpace(value) for _, value := range cidrs {
allErrs = append(allErrs, validation.IsValidCIDR(fieldPath, value)...) value = strings.TrimSpace(value)
allErrs = append(allErrs, IsValidCIDRForLegacyField(fieldPath, value, nil)...)
}
} }
} }
} }
@ -6179,7 +6213,7 @@ func validateServiceTrafficDistribution(service *core.Service) field.ErrorList {
// ValidateServiceCreate validates Services as they are created. // ValidateServiceCreate validates Services as they are created.
func ValidateServiceCreate(service *core.Service) field.ErrorList { func ValidateServiceCreate(service *core.Service) field.ErrorList {
return ValidateService(service) return ValidateService(service, nil)
} }
// ValidateServiceUpdate tests if required fields in the service are set during an update // ValidateServiceUpdate tests if required fields in the service are set during an update
@ -6202,13 +6236,13 @@ func ValidateServiceUpdate(service, oldService *core.Service) field.ErrorList {
allErrs = append(allErrs, validateServiceExternalTrafficFieldsUpdate(oldService, service)...) allErrs = append(allErrs, validateServiceExternalTrafficFieldsUpdate(oldService, service)...)
return append(allErrs, ValidateService(service)...) return append(allErrs, ValidateService(service, oldService)...)
} }
// ValidateServiceStatusUpdate tests if required fields in the Service are set when updating status. // ValidateServiceStatusUpdate tests if required fields in the Service are set when updating status.
func ValidateServiceStatusUpdate(service, oldService *core.Service) field.ErrorList { func ValidateServiceStatusUpdate(service, oldService *core.Service) field.ErrorList {
allErrs := ValidateObjectMetaUpdate(&service.ObjectMeta, &oldService.ObjectMeta, field.NewPath("metadata")) allErrs := ValidateObjectMetaUpdate(&service.ObjectMeta, &oldService.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateLoadBalancerStatus(&service.Status.LoadBalancer, field.NewPath("status", "loadBalancer"), &service.Spec)...) allErrs = append(allErrs, ValidateLoadBalancerStatus(&service.Status.LoadBalancer, &oldService.Status.LoadBalancer, field.NewPath("status", "loadBalancer"), &service.Spec)...)
return allErrs return allErrs
} }
@ -6403,7 +6437,7 @@ func ValidateNode(node *core.Node) field.ErrorList {
// all PodCIDRs should be valid ones // all PodCIDRs should be valid ones
for idx, value := range node.Spec.PodCIDRs { for idx, value := range node.Spec.PodCIDRs {
allErrs = append(allErrs, validation.IsValidCIDR(podCIDRsField.Index(idx), value)...) allErrs = append(allErrs, IsValidCIDRForLegacyField(podCIDRsField.Index(idx), value, nil)...)
} }
// if more than PodCIDR then it must be a dual-stack pair // if more than PodCIDR then it must be a dual-stack pair
@ -7430,16 +7464,28 @@ func ValidateNamespaceFinalizeUpdate(newNamespace, oldNamespace *core.Namespace)
} }
// ValidateEndpoints validates Endpoints on create and update. // ValidateEndpoints validates Endpoints on create and update.
func ValidateEndpoints(endpoints *core.Endpoints) field.ErrorList { func ValidateEndpoints(endpoints, oldEndpoints *core.Endpoints) field.ErrorList {
allErrs := ValidateObjectMeta(&endpoints.ObjectMeta, true, ValidateEndpointsName, field.NewPath("metadata")) allErrs := ValidateObjectMeta(&endpoints.ObjectMeta, true, ValidateEndpointsName, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(endpoints.Annotations, field.NewPath("annotations"))...) allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(endpoints.Annotations, field.NewPath("annotations"))...)
allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets, field.NewPath("subsets"))...)
subsetErrs := validateEndpointSubsets(endpoints.Subsets, field.NewPath("subsets"))
if len(subsetErrs) != 0 {
// If this is an update, and Subsets was unchanged, then ignore the
// validation errors, since apparently older versions of Kubernetes
// considered the data valid. (We only check this after getting a
// validation error since Endpoints may be large and DeepEqual is slow.)
if oldEndpoints != nil && apiequality.Semantic.DeepEqual(oldEndpoints.Subsets, endpoints.Subsets) {
subsetErrs = nil
}
}
allErrs = append(allErrs, subsetErrs...)
return allErrs return allErrs
} }
// ValidateEndpointsCreate validates Endpoints on create. // ValidateEndpointsCreate validates Endpoints on create.
func ValidateEndpointsCreate(endpoints *core.Endpoints) field.ErrorList { func ValidateEndpointsCreate(endpoints *core.Endpoints) field.ErrorList {
return ValidateEndpoints(endpoints) return ValidateEndpoints(endpoints, nil)
} }
// ValidateEndpointsUpdate validates Endpoints on update. NodeName changes are // ValidateEndpointsUpdate validates Endpoints on update. NodeName changes are
@ -7448,7 +7494,7 @@ func ValidateEndpointsCreate(endpoints *core.Endpoints) field.ErrorList {
// happens. // happens.
func ValidateEndpointsUpdate(newEndpoints, oldEndpoints *core.Endpoints) field.ErrorList { func ValidateEndpointsUpdate(newEndpoints, oldEndpoints *core.Endpoints) field.ErrorList {
allErrs := ValidateObjectMetaUpdate(&newEndpoints.ObjectMeta, &oldEndpoints.ObjectMeta, field.NewPath("metadata")) allErrs := ValidateObjectMetaUpdate(&newEndpoints.ObjectMeta, &oldEndpoints.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateEndpoints(newEndpoints)...) allErrs = append(allErrs, ValidateEndpoints(newEndpoints, oldEndpoints)...)
return allErrs return allErrs
} }
@ -7479,7 +7525,7 @@ func validateEndpointSubsets(subsets []core.EndpointSubset, fldPath *field.Path)
func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList { func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("ip"), address.IP)...) allErrs = append(allErrs, IsValidIPForLegacyField(fldPath.Child("ip"), address.IP, nil)...)
if len(address.Hostname) > 0 { if len(address.Hostname) > 0 {
allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...)
} }
@ -7489,19 +7535,21 @@ func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path)
allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg).WithOrigin("format=dns-label")) allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg).WithOrigin("format=dns-label"))
} }
} }
allErrs = append(allErrs, ValidateNonSpecialIP(address.IP, fldPath.Child("ip"))...) allErrs = append(allErrs, ValidateEndpointIP(address.IP, fldPath.Child("ip"))...)
return allErrs return allErrs
} }
// ValidateNonSpecialIP is used to validate Endpoints, EndpointSlices, and // ValidateEndpointIP is used to validate Endpoints and EndpointSlice addresses, and also
// external IPs. Specifically, this disallows unspecified and loopback addresses // (for historical reasons) external IPs. It disallows certain address types that don't
// are nonsensical and link-local addresses tend to be used for node-centric // make sense in those contexts. Note that this function is _almost_, but not exactly,
// purposes (e.g. metadata service). // equivalent to net.IP.IsGlobalUnicast(). (Unlike IsGlobalUnicast, it allows global
// multicast IPs, which is probably a bug.)
// //
// IPv6 references // This function should not be used for new validations; the exact set of IPs that do and
// - https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml // don't make sense in a particular field is context-dependent (e.g., localhost makes
// - https://www.iana.org/assignments/ipv6-multicast-addresses/ipv6-multicast-addresses.xhtml // sense in some places; unspecified IPs make sense in fields that are used as bind
func ValidateNonSpecialIP(ipAddress string, fldPath *field.Path) field.ErrorList { // addresses rather than destination addresses).
func ValidateEndpointIP(ipAddress string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
ip := netutils.ParseIPSloppy(ipAddress) ip := netutils.ParseIPSloppy(ipAddress)
if ip == nil { if ip == nil {
@ -7520,7 +7568,7 @@ func ValidateNonSpecialIP(ipAddress string, fldPath *field.Path) field.ErrorList
if ip.IsLinkLocalMulticast() { if ip.IsLinkLocalMulticast() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24, ff02::/10)")) allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24, ff02::/10)"))
} }
return allErrs.WithOrigin("format=non-special-ip") return allErrs.WithOrigin("format=endpoint-ip")
} }
func validateEndpointPort(port *core.EndpointPort, requireName bool, fldPath *field.Path) field.ErrorList { func validateEndpointPort(port *core.EndpointPort, requireName bool, fldPath *field.Path) field.ErrorList {
@ -7840,16 +7888,25 @@ var (
) )
// ValidateLoadBalancerStatus validates required fields on a LoadBalancerStatus // ValidateLoadBalancerStatus validates required fields on a LoadBalancerStatus
func ValidateLoadBalancerStatus(status *core.LoadBalancerStatus, fldPath *field.Path, spec *core.ServiceSpec) field.ErrorList { func ValidateLoadBalancerStatus(status, oldStatus *core.LoadBalancerStatus, fldPath *field.Path, spec *core.ServiceSpec) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
ingrPath := fldPath.Child("ingress") ingrPath := fldPath.Child("ingress")
if !utilfeature.DefaultFeatureGate.Enabled(features.AllowServiceLBStatusOnNonLB) && spec.Type != core.ServiceTypeLoadBalancer && len(status.Ingress) != 0 { if !utilfeature.DefaultFeatureGate.Enabled(features.AllowServiceLBStatusOnNonLB) && spec.Type != core.ServiceTypeLoadBalancer && len(status.Ingress) != 0 {
allErrs = append(allErrs, field.Forbidden(ingrPath, "may only be used when `spec.type` is 'LoadBalancer'")) allErrs = append(allErrs, field.Forbidden(ingrPath, "may only be used when `spec.type` is 'LoadBalancer'"))
} else { } else {
var existingIngressIPs []string
if oldStatus != nil {
existingIngressIPs = make([]string, 0, len(oldStatus.Ingress))
for _, ingress := range oldStatus.Ingress {
if len(ingress.IP) > 0 {
existingIngressIPs = append(existingIngressIPs, ingress.IP)
}
}
}
for i, ingress := range status.Ingress { for i, ingress := range status.Ingress {
idxPath := ingrPath.Index(i) idxPath := ingrPath.Index(i)
if len(ingress.IP) > 0 { if len(ingress.IP) > 0 {
allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...) allErrs = append(allErrs, IsValidIPForLegacyField(idxPath.Child("ip"), ingress.IP, existingIngressIPs)...)
} }
if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil { if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil {
@ -8116,7 +8173,7 @@ func validateLabelKeys(fldPath *field.Path, labelKeys []string, labelSelector *m
// ValidateServiceClusterIPsRelatedFields validates .spec.ClusterIPs,, // ValidateServiceClusterIPsRelatedFields validates .spec.ClusterIPs,,
// .spec.IPFamilies, .spec.ipFamilyPolicy. This is exported because it is used // .spec.IPFamilies, .spec.ipFamilyPolicy. This is exported because it is used
// during IP init and allocation. // during IP init and allocation.
func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorList { func ValidateServiceClusterIPsRelatedFields(service, oldService *core.Service) field.ErrorList {
// ClusterIP, ClusterIPs, IPFamilyPolicy and IPFamilies are validated prior (all must be unset) for ExternalName service // ClusterIP, ClusterIPs, IPFamilyPolicy and IPFamilies are validated prior (all must be unset) for ExternalName service
if service.Spec.Type == core.ServiceTypeExternalName { if service.Spec.Type == core.ServiceTypeExternalName {
return field.ErrorList{} return field.ErrorList{}
@ -8170,6 +8227,11 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi
} }
} }
var existingClusterIPs []string
if oldService != nil {
existingClusterIPs = oldService.Spec.ClusterIPs // +k8s:verify-mutation:reason=clone
}
// clusterIPs stand alone validation // clusterIPs stand alone validation
// valid ips with None and empty string handling // valid ips with None and empty string handling
// duplication check is done as part of DualStackvalidation below // duplication check is done as part of DualStackvalidation below
@ -8183,8 +8245,8 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi
continue continue
} }
// is it valid ip? // is it a valid ip? (or was it at least previously considered valid?)
errorMessages := validation.IsValidIP(clusterIPsField.Index(i), clusterIP) errorMessages := IsValidIPForLegacyField(clusterIPsField.Index(i), clusterIP, existingClusterIPs)
hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs
allErrs = append(allErrs, errorMessages...) allErrs = append(allErrs, errorMessages...)
} }
@ -8699,3 +8761,17 @@ func isRestartableInitContainer(initContainer *core.Container) bool {
} }
return *initContainer.RestartPolicy == core.ContainerRestartPolicyAlways return *initContainer.RestartPolicy == core.ContainerRestartPolicyAlways
} }
// IsValidIPForLegacyField is a wrapper around validation.IsValidIPForLegacyField that
// handles setting strictValidation correctly. This is only for fields that use legacy IP
// address validation; use validation.IsValidIP for new fields.
func IsValidIPForLegacyField(fldPath *field.Path, value string, validOldIPs []string) field.ErrorList {
return validation.IsValidIPForLegacyField(fldPath, value, utilfeature.DefaultFeatureGate.Enabled(features.StrictIPCIDRValidation), validOldIPs)
}
// IsValidCIDRForLegacyField is a wrapper around validation.IsValidCIDRForLegacyField that
// handles setting strictValidation correctly. This is only for fields that use legacy CIDR
// value validation; use validation.IsValidCIDR for new fields.
func IsValidCIDRForLegacyField(fldPath *field.Path, value string, validOldCIDRs []string) field.ErrorList {
return validation.IsValidCIDRForLegacyField(fldPath, value, utilfeature.DefaultFeatureGate.Enabled(features.StrictIPCIDRValidation), validOldCIDRs)
}

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

File diff suppressed because it is too large Load Diff

43
pkg/apis/discovery/validation/validation.go
View File

@ -20,6 +20,7 @@ import (
"fmt" "fmt"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
@ -28,6 +29,7 @@ import (
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/discovery" "k8s.io/kubernetes/pkg/apis/discovery"
netutils "k8s.io/utils/net"
) )
var ( var (
@ -54,23 +56,34 @@ var (
var ValidateEndpointSliceName = apimachineryvalidation.NameIsDNSSubdomain var ValidateEndpointSliceName = apimachineryvalidation.NameIsDNSSubdomain
// ValidateEndpointSlice validates an EndpointSlice. // ValidateEndpointSlice validates an EndpointSlice.
func ValidateEndpointSlice(endpointSlice *discovery.EndpointSlice) field.ErrorList { func ValidateEndpointSlice(endpointSlice, oldEndpointSlice *discovery.EndpointSlice) field.ErrorList {
allErrs := apivalidation.ValidateObjectMeta(&endpointSlice.ObjectMeta, true, ValidateEndpointSliceName, field.NewPath("metadata")) allErrs := apivalidation.ValidateObjectMeta(&endpointSlice.ObjectMeta, true, ValidateEndpointSliceName, field.NewPath("metadata"))
allErrs = append(allErrs, validateAddressType(endpointSlice.AddressType)...) allErrs = append(allErrs, validateAddressType(endpointSlice.AddressType)...)
allErrs = append(allErrs, validateEndpoints(endpointSlice.Endpoints, endpointSlice.AddressType, field.NewPath("endpoints"))...)
allErrs = append(allErrs, validatePorts(endpointSlice.Ports, field.NewPath("ports"))...) allErrs = append(allErrs, validatePorts(endpointSlice.Ports, field.NewPath("ports"))...)
endpointsErrs := validateEndpoints(endpointSlice.Endpoints, endpointSlice.AddressType, field.NewPath("endpoints"))
if len(endpointsErrs) != 0 {
// If this is an update, and Endpoints was unchanged, then ignore the
// validation errors, since apparently older versions of Kubernetes
// considered the data valid. (We only check this after getting a
// validation error since Endpoints may be large and DeepEqual is slow.)
if oldEndpointSlice != nil && apiequality.Semantic.DeepEqual(oldEndpointSlice.Endpoints, endpointSlice.Endpoints) {
endpointsErrs = nil
}
}
allErrs = append(allErrs, endpointsErrs...)
return allErrs return allErrs
} }
// ValidateEndpointSliceCreate validates an EndpointSlice when it is created. // ValidateEndpointSliceCreate validates an EndpointSlice when it is created.
func ValidateEndpointSliceCreate(endpointSlice *discovery.EndpointSlice) field.ErrorList { func ValidateEndpointSliceCreate(endpointSlice *discovery.EndpointSlice) field.ErrorList {
return ValidateEndpointSlice(endpointSlice) return ValidateEndpointSlice(endpointSlice, nil)
} }
// ValidateEndpointSliceUpdate validates an EndpointSlice when it is updated. // ValidateEndpointSliceUpdate validates an EndpointSlice when it is updated.
func ValidateEndpointSliceUpdate(newEndpointSlice, oldEndpointSlice *discovery.EndpointSlice) field.ErrorList { func ValidateEndpointSliceUpdate(newEndpointSlice, oldEndpointSlice *discovery.EndpointSlice) field.ErrorList {
allErrs := ValidateEndpointSlice(newEndpointSlice) allErrs := ValidateEndpointSlice(newEndpointSlice, oldEndpointSlice)
allErrs = append(allErrs, apivalidation.ValidateImmutableField(newEndpointSlice.AddressType, oldEndpointSlice.AddressType, field.NewPath("addressType"))...) allErrs = append(allErrs, apivalidation.ValidateImmutableField(newEndpointSlice.AddressType, oldEndpointSlice.AddressType, field.NewPath("addressType"))...)
return allErrs return allErrs
@ -99,11 +112,25 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres
// and do not get validated. // and do not get validated.
switch addrType { switch addrType {
case discovery.AddressTypeIPv4: case discovery.AddressTypeIPv4:
allErrs = append(allErrs, validation.IsValidIPv4Address(addressPath.Index(i), address)...) ipErrs := apivalidation.IsValidIPForLegacyField(addressPath.Index(i), address, nil)
allErrs = append(allErrs, apivalidation.ValidateNonSpecialIP(address, addressPath.Index(i))...) if len(ipErrs) > 0 {
allErrs = append(allErrs, ipErrs...)
} else {
if !netutils.IsIPv4String(address) {
allErrs = append(allErrs, field.Invalid(addressPath, address, "must be an IPv4 address"))
}
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
}
case discovery.AddressTypeIPv6: case discovery.AddressTypeIPv6:
allErrs = append(allErrs, validation.IsValidIPv6Address(addressPath.Index(i), address)...) ipErrs := validation.IsValidIP(addressPath.Index(i), address)
allErrs = append(allErrs, apivalidation.ValidateNonSpecialIP(address, addressPath.Index(i))...) if len(ipErrs) > 0 {
allErrs = append(allErrs, ipErrs...)
} else {
if !netutils.IsIPv6String(address) {
allErrs = append(allErrs, field.Invalid(addressPath, address, "must be an IPv6 address"))
}
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
}
case discovery.AddressTypeFQDN: case discovery.AddressTypeFQDN:
allErrs = append(allErrs, validation.IsFullyQualifiedDomainName(addressPath.Index(i), address)...) allErrs = append(allErrs, validation.IsFullyQualifiedDomainName(addressPath.Index(i), address)...)
} }

80
pkg/apis/discovery/validation/validation_test.go
View File

@ -23,8 +23,11 @@ import (
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/discovery" "k8s.io/kubernetes/pkg/apis/discovery"
"k8s.io/kubernetes/pkg/features"
"k8s.io/utils/ptr" "k8s.io/utils/ptr"
) )
@ -36,6 +39,7 @@ func TestValidateEndpointSlice(t *testing.T) {
testCases := map[string]struct { testCases := map[string]struct {
expectedErrors int expectedErrors int
legacyIPs bool
endpointSlice *discovery.EndpointSlice endpointSlice *discovery.EndpointSlice
}{ }{
"good-slice": { "good-slice": {
@ -235,6 +239,22 @@ func TestValidateEndpointSlice(t *testing.T) {
}}, }},
}, },
}, },
"legacy-ip-with-legacy-validation": {
expectedErrors: 0,
legacyIPs: true,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: ptr.To("http"),
Protocol: ptr.To(api.ProtocolTCP),
}},
Endpoints: []discovery.Endpoint{{
Addresses: []string{"012.034.056.078"},
Hostname: ptr.To("valid-123"),
}},
},
},
// expected failures // expected failures
"duplicate-port-name": { "duplicate-port-name": {
@ -408,7 +428,7 @@ func TestValidateEndpointSlice(t *testing.T) {
}, },
}, },
"bad-ip": { "bad-ip": {
expectedErrors: 2, expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{ endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta, ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4, AddressType: discovery.AddressTypeIPv4,
@ -422,8 +442,23 @@ func TestValidateEndpointSlice(t *testing.T) {
}}, }},
}, },
}, },
"legacy-ip-with-strict-validation": {
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: ptr.To("http"),
Protocol: ptr.To(api.ProtocolTCP),
}},
Endpoints: []discovery.Endpoint{{
Addresses: []string{"012.034.056.078"},
Hostname: ptr.To("valid-123"),
}},
},
},
"bad-ipv4": { "bad-ipv4": {
expectedErrors: 3, expectedErrors: 2,
endpointSlice: &discovery.EndpointSlice{ endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta, ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4, AddressType: discovery.AddressTypeIPv4,
@ -438,7 +473,7 @@ func TestValidateEndpointSlice(t *testing.T) {
}, },
}, },
"bad-ipv6": { "bad-ipv6": {
expectedErrors: 4, expectedErrors: 2,
endpointSlice: &discovery.EndpointSlice{ endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta, ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv6, AddressType: discovery.AddressTypeIPv6,
@ -601,7 +636,8 @@ func TestValidateEndpointSlice(t *testing.T) {
for name, testCase := range testCases { for name, testCase := range testCases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
errs := ValidateEndpointSlice(testCase.endpointSlice) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !testCase.legacyIPs)
errs := ValidateEndpointSlice(testCase.endpointSlice, nil)
if len(errs) != testCase.expectedErrors { if len(errs) != testCase.expectedErrors {
t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs) t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs)
} }
@ -701,6 +737,7 @@ func TestValidateEndpointSliceCreate(t *testing.T) {
for name, testCase := range testCases { for name, testCase := range testCases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateEndpointSliceCreate(testCase.endpointSlice) errs := ValidateEndpointSliceCreate(testCase.endpointSlice)
if len(errs) != testCase.expectedErrors { if len(errs) != testCase.expectedErrors {
t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs) t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs)
@ -729,6 +766,23 @@ func TestValidateEndpointSliceUpdate(t *testing.T) {
}, },
expectedErrors: 0, expectedErrors: 0,
}, },
"unchanged IPs are not revalidated": {
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.04"},
}},
},
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.04"},
}},
},
expectedErrors: 0,
},
// expected errors // expected errors
"invalid node name set": { "invalid node name set": {
@ -787,10 +841,28 @@ func TestValidateEndpointSliceUpdate(t *testing.T) {
}, },
expectedErrors: 1, expectedErrors: 1,
}, },
"changed IPs are revalidated": {
oldEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.3"},
}},
},
newEndpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.1.2.03"},
}},
},
expectedErrors: 1,
},
} }
for name, testCase := range testCases { for name, testCase := range testCases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateEndpointSliceUpdate(testCase.newEndpointSlice, testCase.oldEndpointSlice) errs := ValidateEndpointSliceUpdate(testCase.newEndpointSlice, testCase.oldEndpointSlice)
if len(errs) != testCase.expectedErrors { if len(errs) != testCase.expectedErrors {
t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs) t.Errorf("Expected %d errors, got %d errors: %v", testCase.expectedErrors, len(errs), errs)

84
pkg/apis/networking/validation/validation.go
View File

@ -18,7 +18,6 @@ package validation
import ( import (
"fmt" "fmt"
"net/netip"
"strings" "strings"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
@ -57,6 +56,7 @@ var (
type NetworkPolicyValidationOptions struct { type NetworkPolicyValidationOptions struct {
AllowInvalidLabelValueInSelector bool AllowInvalidLabelValueInSelector bool
AllowCIDRsEvenIfInvalid []string
} }
// ValidateNetworkPolicyName can be used to check whether the given networkpolicy // ValidateNetworkPolicyName can be used to check whether the given networkpolicy
@ -119,7 +119,7 @@ func ValidateNetworkPolicyPeer(peer *networking.NetworkPolicyPeer, opts NetworkP
} }
if peer.IPBlock != nil { if peer.IPBlock != nil {
numPeers++ numPeers++
allErrs = append(allErrs, ValidateIPBlock(peer.IPBlock, peerPath.Child("ipBlock"))...) allErrs = append(allErrs, ValidateIPBlock(peer.IPBlock, peerPath.Child("ipBlock"), opts)...)
} }
if numPeers == 0 { if numPeers == 0 {
@ -207,20 +207,47 @@ func ValidationOptionsForNetworking(new, old *networking.NetworkPolicy) NetworkP
// ValidateNetworkPolicyUpdate tests if an update to a NetworkPolicy is valid. // ValidateNetworkPolicyUpdate tests if an update to a NetworkPolicy is valid.
func ValidateNetworkPolicyUpdate(update, old *networking.NetworkPolicy, opts NetworkPolicyValidationOptions) field.ErrorList { func ValidateNetworkPolicyUpdate(update, old *networking.NetworkPolicy, opts NetworkPolicyValidationOptions) field.ErrorList {
// Record all existing CIDRs in the policy; see ValidateIPBlock.
var existingCIDRs []string
for _, ingress := range old.Spec.Ingress {
for _, peer := range ingress.From {
if peer.IPBlock != nil {
existingCIDRs = append(existingCIDRs, peer.IPBlock.CIDR)
existingCIDRs = append(existingCIDRs, peer.IPBlock.Except...)
}
}
}
for _, egress := range old.Spec.Egress {
for _, peer := range egress.To {
if peer.IPBlock != nil {
existingCIDRs = append(existingCIDRs, peer.IPBlock.CIDR)
existingCIDRs = append(existingCIDRs, peer.IPBlock.Except...)
}
}
}
opts.AllowCIDRsEvenIfInvalid = existingCIDRs
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&update.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))...) allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&update.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))...)
allErrs = append(allErrs, ValidateNetworkPolicySpec(&update.Spec, opts, field.NewPath("spec"))...) allErrs = append(allErrs, ValidateNetworkPolicySpec(&update.Spec, opts, field.NewPath("spec"))...)
return allErrs return allErrs
} }
// ValidateIPBlock validates a cidr and the except fields of an IpBlock NetworkPolicyPeer // ValidateIPBlock validates a cidr and the except fields of an IPBlock NetworkPolicyPeer.
func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path) field.ErrorList { //
// If a pre-existing CIDR is invalid/insecure, but it is not being changed by this update,
// then we have to continue allowing it. But since the user may have changed the policy in
// arbitrary ways (adding/removing rules, adding/removing peers, adding/removing
// ipBlock.except values, etc), we can't reliably determine whether a CIDR value we see
// here is a new value or a pre-existing one. So we just allow any CIDR value that
// appeared in the old NetworkPolicy to be used here without revalidation.
func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path, opts NetworkPolicyValidationOptions) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
if ipb.CIDR == "" { if ipb.CIDR == "" {
allErrs = append(allErrs, field.Required(fldPath.Child("cidr"), "")) allErrs = append(allErrs, field.Required(fldPath.Child("cidr"), ""))
return allErrs return allErrs
} }
allErrs = append(allErrs, validation.IsValidCIDR(fldPath.Child("cidr"), ipb.CIDR)...) allErrs = append(allErrs, apivalidation.IsValidCIDRForLegacyField(fldPath.Child("cidr"), ipb.CIDR, opts.AllowCIDRsEvenIfInvalid)...)
_, cidrIPNet, err := netutils.ParseCIDRSloppy(ipb.CIDR) _, cidrIPNet, err := netutils.ParseCIDRSloppy(ipb.CIDR)
if err != nil { if err != nil {
// Implies validation would have failed so we already added errors for it. // Implies validation would have failed so we already added errors for it.
@ -229,7 +256,7 @@ func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path) field.ErrorLi
for i, exceptCIDRStr := range ipb.Except { for i, exceptCIDRStr := range ipb.Except {
exceptPath := fldPath.Child("except").Index(i) exceptPath := fldPath.Child("except").Index(i)
allErrs = append(allErrs, validation.IsValidCIDR(exceptPath, exceptCIDRStr)...) allErrs = append(allErrs, apivalidation.IsValidCIDRForLegacyField(exceptPath, exceptCIDRStr, opts.AllowCIDRsEvenIfInvalid)...)
_, exceptCIDR, err := netutils.ParseCIDRSloppy(exceptCIDRStr) _, exceptCIDR, err := netutils.ParseCIDRSloppy(exceptCIDRStr)
if err != nil { if err != nil {
// Implies validation would have failed so we already added errors for it. // Implies validation would have failed so we already added errors for it.
@ -347,17 +374,28 @@ func ValidateIngressSpec(spec *networking.IngressSpec, fldPath *field.Path, opts
// ValidateIngressStatusUpdate tests if required fields in the Ingress are set when updating status. // ValidateIngressStatusUpdate tests if required fields in the Ingress are set when updating status.
func ValidateIngressStatusUpdate(ingress, oldIngress *networking.Ingress) field.ErrorList { func ValidateIngressStatusUpdate(ingress, oldIngress *networking.Ingress) field.ErrorList {
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata")) allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
allErrs = append(allErrs, ValidateIngressLoadBalancerStatus(&ingress.Status.LoadBalancer, field.NewPath("status", "loadBalancer"))...) allErrs = append(allErrs, ValidateIngressLoadBalancerStatus(&ingress.Status.LoadBalancer, &oldIngress.Status.LoadBalancer, field.NewPath("status", "loadBalancer"))...)
return allErrs return allErrs
} }
// ValidateIngressLoadBalancerStatus 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, oldStatus *networking.IngressLoadBalancerStatus, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
var existingIngressIPs []string
if oldStatus != nil {
existingIngressIPs = make([]string, 0, len(oldStatus.Ingress))
for _, ingress := range oldStatus.Ingress {
if len(ingress.IP) > 0 {
existingIngressIPs = append(existingIngressIPs, ingress.IP)
}
}
}
for i, ingress := range status.Ingress { for i, ingress := range status.Ingress {
idxPath := fldPath.Child("ingress").Index(i) idxPath := fldPath.Child("ingress").Index(i)
if len(ingress.IP) > 0 { if len(ingress.IP) > 0 {
allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...) allErrs = append(allErrs, apivalidation.IsValidIPForLegacyField(idxPath.Child("ip"), ingress.IP, existingIngressIPs)...)
} }
if len(ingress.Hostname) > 0 { if len(ingress.Hostname) > 0 {
for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) { for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) {
@ -653,12 +691,12 @@ func allowInvalidWildcardHostRule(oldIngress *networking.Ingress) bool {
// IPAddress does not support generating names, prefix is not considered. // IPAddress does not support generating names, prefix is not considered.
func ValidateIPAddressName(name string, prefix bool) []string { func ValidateIPAddressName(name string, prefix bool) []string {
var errs []string var errs []string
ip, err := netip.ParseAddr(name)
if err != nil {
errs = append(errs, err.Error())
} else if ip.String() != name {
errs = append(errs, "must be a canonical format IP address")
allErrs := validation.IsValidIP(&field.Path{}, name)
// Need to unconvert the field.Error from IsValidIP back to a string so our caller
// can convert it back to a field.Error!
for _, err := range allErrs {
errs = append(errs, err.Detail)
} }
return errs return errs
} }
@ -748,28 +786,12 @@ func ValidateServiceCIDR(cidrConfig *networking.ServiceCIDR) field.ErrorList {
} }
for i, cidr := range cidrConfig.Spec.CIDRs { for i, cidr := range cidrConfig.Spec.CIDRs {
allErrs = append(allErrs, validateCIDR(cidr, fieldPath.Index(i))...) allErrs = append(allErrs, validation.IsValidCIDR(fieldPath.Index(i), cidr)...)
} }
return allErrs return allErrs
} }
func validateCIDR(cidr string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
prefix, err := netip.ParsePrefix(cidr)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, cidr, err.Error()))
} else {
if prefix.Addr() != prefix.Masked().Addr() {
allErrs = append(allErrs, field.Invalid(fldPath, cidr, "wrong CIDR format, IP doesn't match network IP address"))
}
if prefix.String() != cidr {
allErrs = append(allErrs, field.Invalid(fldPath, cidr, "CIDR not in RFC 5952 canonical format"))
}
}
return allErrs
}
// ValidateServiceCIDRUpdate tests if an update to a ServiceCIDR is valid. // ValidateServiceCIDRUpdate tests if an update to a ServiceCIDR is valid.
func ValidateServiceCIDRUpdate(update, old *networking.ServiceCIDR) field.ErrorList { func ValidateServiceCIDRUpdate(update, old *networking.ServiceCIDR) field.ErrorList {
var allErrs field.ErrorList var allErrs field.ErrorList

257
pkg/apis/networking/validation/validation_test.go
View File

@ -25,8 +25,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/networking" "k8s.io/kubernetes/pkg/apis/networking"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer" utilpointer "k8s.io/utils/pointer"
) )
@ -248,11 +251,27 @@ func TestValidateNetworkPolicy(t *testing.T) {
} }
// Success cases are expected to pass validation. // Success cases are expected to pass validation.
for _, v := range successCases {
t.Run("", func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
t.Errorf("Expected success, got %v", errs)
}
})
}
for k, v := range successCases { legacyValidationCases := []*networking.NetworkPolicy{
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 { makeNetworkPolicyCustom(setIngressFromIPBlockIPV4, func(networkPolicy *networking.NetworkPolicy) {
t.Errorf("Expected success for the success validation test number %d, got %v", k, errs) networkPolicy.Spec.Ingress[0].From[0].IPBlock.CIDR = "001.002.003.000/0"
} }),
}
for _, v := range legacyValidationCases {
t.Run("", func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, false)
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
t.Errorf("Expected success, got %v", errs)
}
})
} }
invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
@ -299,6 +318,9 @@ func TestValidateNetworkPolicy(t *testing.T) {
"invalid ipv6 cidr format": makeNetworkPolicyCustom(setIngressFromIPBlockIPV6, func(networkPolicy *networking.NetworkPolicy) { "invalid ipv6 cidr format": makeNetworkPolicyCustom(setIngressFromIPBlockIPV6, func(networkPolicy *networking.NetworkPolicy) {
networkPolicy.Spec.Ingress[0].From[0].IPBlock.CIDR = "fd00:192:168::" networkPolicy.Spec.Ingress[0].From[0].IPBlock.CIDR = "fd00:192:168::"
}), }),
"legacy cidr format with strict validation": makeNetworkPolicyCustom(setIngressFromIPBlockIPV4, func(networkPolicy *networking.NetworkPolicy) {
networkPolicy.Spec.Ingress[0].From[0].IPBlock.CIDR = "001.002.003.000/0"
}),
"except field is an empty string": makeNetworkPolicyCustom(setIngressFromIPBlockIPV4, func(networkPolicy *networking.NetworkPolicy) { "except field is an empty string": makeNetworkPolicyCustom(setIngressFromIPBlockIPV4, func(networkPolicy *networking.NetworkPolicy) {
networkPolicy.Spec.Ingress[0].From[0].IPBlock.Except = []string{""} networkPolicy.Spec.Ingress[0].From[0].IPBlock.Except = []string{""}
}), }),
@ -367,9 +389,12 @@ func TestValidateNetworkPolicy(t *testing.T) {
// Error cases are not expected to pass validation. // Error cases are not expected to pass validation.
for testName, networkPolicy := range errorCases { for testName, networkPolicy := range errorCases {
if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 { t.Run(testName, func(t *testing.T) {
t.Errorf("Expected failure for test: %s", testName) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
} if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 {
t.Errorf("Expected failure")
}
})
} }
} }
@ -417,14 +442,98 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
}, },
}, },
}, },
"fix pre-existing invalid CIDR": {
old: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.2.0/24",
"192.168.3.1/24",
},
},
}},
}},
},
},
update: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.2.0/24",
"192.168.3.0/24",
},
},
}},
}},
},
},
},
"fail to fix pre-existing invalid CIDR": {
old: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.2.0/24",
"192.168.3.1/24",
},
},
}},
}},
},
},
update: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.1.0/24",
"192.168.2.0/24",
"192.168.3.1/24",
},
},
}},
}},
},
},
},
} }
for testName, successCase := range successCases { for testName, successCase := range successCases {
successCase.old.ObjectMeta.ResourceVersion = "1" t.Run(testName, func(t *testing.T) {
successCase.update.ObjectMeta.ResourceVersion = "1" featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old, NetworkPolicyValidationOptions{false}); len(errs) != 0 { successCase.old.ObjectMeta.ResourceVersion = "1"
t.Errorf("expected success (%s): %v", testName, errs) successCase.update.ObjectMeta.ResourceVersion = "1"
} if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old, NetworkPolicyValidationOptions{}); len(errs) != 0 {
t.Errorf("expected success, but got %v", errs)
}
})
} }
errorCases := map[string]npUpdateTest{ errorCases := map[string]npUpdateTest{
@ -444,14 +553,58 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
}, },
}, },
}, },
"add new invalid CIDR": {
old: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.2.0/24",
"192.168.3.0/24",
},
},
}},
}},
},
},
update: networking.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: networking.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"a": "b"},
},
Ingress: []networking.NetworkPolicyIngressRule{{
From: []networking.NetworkPolicyPeer{{
IPBlock: &networking.IPBlock{
CIDR: "192.168.0.0/16",
Except: []string{
"192.168.1.1/24",
"192.168.2.0/24",
"192.168.3.0/24",
},
},
}},
}},
},
},
},
} }
for testName, errorCase := range errorCases { for testName, errorCase := range errorCases {
errorCase.old.ObjectMeta.ResourceVersion = "1" t.Run(testName, func(t *testing.T) {
errorCase.update.ObjectMeta.ResourceVersion = "1" featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old, NetworkPolicyValidationOptions{false}); len(errs) == 0 { errorCase.old.ObjectMeta.ResourceVersion = "1"
t.Errorf("expected failure: %s", testName) errorCase.update.ObjectMeta.ResourceVersion = "1"
} if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old, NetworkPolicyValidationOptions{}); len(errs) == 0 {
t.Errorf("expected failure")
}
})
} }
} }
@ -1805,6 +1958,14 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
}, },
}, },
} }
legacyIP := newValid()
legacyIP.Status = networking.IngressStatus{
LoadBalancer: networking.IngressLoadBalancerStatus{
Ingress: []networking.IngressLoadBalancerIngress{
{IP: "001.002.003.004", Hostname: "foo.com"},
},
},
}
invalidHostname := newValid() invalidHostname := newValid()
invalidHostname.Status = networking.IngressStatus{ invalidHostname.Status = networking.IngressStatus{
LoadBalancer: networking.IngressLoadBalancerStatus{ LoadBalancer: networking.IngressLoadBalancerStatus{
@ -1814,26 +1975,56 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
}, },
} }
errs := ValidateIngressStatusUpdate(&newValue, &oldValue) successCases := map[string]struct {
if len(errs) != 0 { oldValue networking.Ingress
t.Errorf("Unexpected error %v", errs) newValue networking.Ingress
legacyIPs bool
}{
"success": {
oldValue: oldValue,
newValue: newValue,
},
"legacy IPs with legacy validation": {
oldValue: oldValue,
newValue: legacyIP,
legacyIPs: true,
},
"legacy IPs unchanged in update": {
oldValue: legacyIP,
newValue: legacyIP,
},
}
for k, tc := range successCases {
t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !tc.legacyIPs)
errs := ValidateIngressStatusUpdate(&tc.newValue, &tc.oldValue)
if len(errs) != 0 {
t.Errorf("Unexpected error %v", errs)
}
})
} }
errorCases := map[string]networking.Ingress{ errorCases := map[string]networking.Ingress{
"status.loadBalancer.ingress[0].ip: Invalid value": invalidIP, "status.loadBalancer.ingress[0].ip: must be a valid IP address": invalidIP,
"status.loadBalancer.ingress[0].hostname: Invalid value": invalidHostname, "status.loadBalancer.ingress[0].ip: must not have leading 0s": legacyIP,
"status.loadBalancer.ingress[0].hostname: must be a DNS name": invalidHostname,
} }
for k, v := range errorCases { for k, v := range errorCases {
errs := ValidateIngressStatusUpdate(&v, &oldValue) t.Run(k, func(t *testing.T) {
if len(errs) == 0 { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
t.Errorf("expected failure for %s", k)
} else { errs := ValidateIngressStatusUpdate(&v, &oldValue)
s := strings.Split(k, ":") if len(errs) == 0 {
err := errs[0] t.Errorf("expected failure")
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) { } else {
t.Errorf("unexpected error: %q, expected: %q", err, k) s := strings.SplitN(k, ":", 2)
err := errs[0]
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
t.Errorf("unexpected error: %q, expected: %q", err, k)
}
} }
} })
} }
} }
@ -2150,13 +2341,13 @@ func TestValidateServiceCIDR(t *testing.T) {
}, },
}, },
"badip-iprange-caps-ipv6": { "badip-iprange-caps-ipv6": {
expectedErrors: 2, expectedErrors: 1,
ipRange: &networking.ServiceCIDR{ ipRange: &networking.ServiceCIDR{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "test-name", Name: "test-name",
}, },
Spec: networking.ServiceCIDRSpec{ Spec: networking.ServiceCIDRSpec{
CIDRs: []string{"FD00:1234::2/64"}, CIDRs: []string{"FD00:1234::0/64"},
}, },
}, },
}, },

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

@ -37,7 +37,6 @@ import (
"k8s.io/dynamic-resource-allocation/structured" "k8s.io/dynamic-resource-allocation/structured"
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/resource" "k8s.io/kubernetes/pkg/apis/resource"
netutils "k8s.io/utils/net"
) )
var ( var (
@ -890,25 +889,7 @@ func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fl
allErrs = append(allErrs, validateSet(networkDeviceData.IPs, resource.NetworkDeviceDataMaxIPs, allErrs = append(allErrs, validateSet(networkDeviceData.IPs, resource.NetworkDeviceDataMaxIPs,
func(address string, fldPath *field.Path) field.ErrorList { func(address string, fldPath *field.Path) field.ErrorList {
// reformat CIDR to handle different ways IPs can be written return validation.IsValidInterfaceAddress(fldPath, address)
// (e.g. 2001:db8::1/64 == 2001:0db8::1/64) }, stringKey, fldPath.Child("ips"))...)
ip, ipNet, err := netutils.ParseCIDRSloppy(address)
if err != nil {
// must fail
return validation.IsValidCIDR(fldPath, address)
}
maskSize, _ := ipNet.Mask.Size()
canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize)
if address != canonical {
return field.ErrorList{
field.Invalid(fldPath, address, fmt.Sprintf("must be in canonical form (%s)", canonical)),
}
}
return nil
},
func(address string) (string, string) {
return address, ""
},
fldPath.Child("ips"))...)
return allErrs return allErrs
} }

8
pkg/apis/resource/validation/validation_resourceclaim_test.go
View File

@ -1257,9 +1257,9 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
wantFailures: field.ErrorList{ wantFailures: field.ErrorList{
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength),
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength),
field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"), field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/64)"),
field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(1), "010.009.008.000/24", "must be in canonical form (10.9.8.0/24)"), field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(1), "010.009.008.000/24", "must be in canonical form (\"10.9.8.0/24\")"),
field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(2), "2001:0db8::1/64", "must be in canonical form (2001:db8::1/64)"), field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(2), "2001:0db8::1/64", "must be in canonical form (\"2001:db8::1/64\")"),
}, },
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
@ -1390,7 +1390,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
wantFailures: field.ErrorList{ wantFailures: field.ErrorList{
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength),
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength),
field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"), field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/64)"),
}, },
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {

6
pkg/features/kube_features.go
View File

@ -715,6 +715,12 @@ const (
// Allow API server Protobuf encoder to encode collections item by item, instead of all at once. // Allow API server Protobuf encoder to encode collections item by item, instead of all at once.
StreamingCollectionEncodingToProtobuf featuregate.Feature = "StreamingCollectionEncodingToProtobuf" StreamingCollectionEncodingToProtobuf featuregate.Feature = "StreamingCollectionEncodingToProtobuf"
// owner: @danwinship
// kep: https://kep.k8s.io/4858
//
// Requires stricter validation of IP addresses and CIDR values in API objects.
StrictIPCIDRValidation featuregate.Feature = "StrictIPCIDRValidation"
// owner: @robscott // owner: @robscott
// kep: https://kep.k8s.io/2433 // kep: https://kep.k8s.io/2433
// //

4
pkg/features/versioned_kube_features.go
View File

@ -766,6 +766,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta}, {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta},
}, },
StrictIPCIDRValidation: {
{Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Alpha},
},
TopologyAwareHints: { TopologyAwareHints: {
{Version: version.MustParse("1.21"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.21"), Default: false, PreRelease: featuregate.Alpha},
{Version: version.MustParse("1.23"), Default: false, PreRelease: featuregate.Beta}, {Version: version.MustParse("1.23"), Default: false, PreRelease: featuregate.Beta},

4
pkg/registry/core/service/storage/alloc.go
View File

@ -151,9 +151,7 @@ func (al *Allocators) initIPFamilyFields(after After, before Before) error {
// Do some loose pre-validation of the input. This makes it easier in the // Do some loose pre-validation of the input. This makes it easier in the
// rest of allocation code to not have to consider corner cases. // rest of allocation code to not have to consider corner cases.
// TODO(thockin): when we tighten validation (e.g. to require IPs) we will if el := validation.ValidateServiceClusterIPsRelatedFields(service, oldService); len(el) != 0 {
// need a "strict" and a "loose" form of this.
if el := validation.ValidateServiceClusterIPsRelatedFields(service); len(el) != 0 {
return errors.NewInvalid(api.Kind("Service"), service.Name, el) return errors.NewInvalid(api.Kind("Service"), service.Name, el)
} }

201
staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go
View File

@ -18,28 +18,94 @@ package validation
import ( import (
"fmt" "fmt"
"net"
"net/netip" "net/netip"
"slices"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2" "k8s.io/klog/v2"
netutils "k8s.io/utils/net" netutils "k8s.io/utils/net"
) )
// IsValidIP tests that the argument is a valid IP address. func parseIP(fldPath *field.Path, value string, strictValidation bool) (net.IP, field.ErrorList) {
func IsValidIP(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList var allErrors field.ErrorList
if netutils.ParseIPSloppy(value) == nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)").WithOrigin("format=ip-sloppy")) ip := netutils.ParseIPSloppy(value)
if ip == nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)"))
return nil, allErrors
} }
return allErrors
if strictValidation {
addr, err := netip.ParseAddr(value)
if err != nil {
// If netutils.ParseIPSloppy parsed it, but netip.ParseAddr
// doesn't, then it must have illegal leading 0s.
allErrors = append(allErrors, field.Invalid(fldPath, value, "must not have leading 0s"))
}
if addr.Is4In6() {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must not be an IPv4-mapped IPv6 address"))
}
}
return ip, allErrors
}
// IsValidIPForLegacyField tests that the argument is a valid IP address for a "legacy"
// API field that predates strict IP validation. In particular, this allows IPs that are
// not in canonical form (e.g., "FE80:0:0:0:0:0:0:0abc" instead of "fe80::abc").
//
// If strictValidation is false, this also allows IPs in certain invalid or ambiguous
// formats:
//
// 1. IPv4 IPs are allowed to have leading "0"s in octets (e.g. "010.002.003.004").
// Historically, net.ParseIP (and later netutils.ParseIPSloppy) simply ignored leading
// "0"s in IPv4 addresses, but most libc-based software treats 0-prefixed IPv4 octets
// as octal, meaning different software might interpret the same string as two
// different IPs, potentially leading to security issues. (Current net.ParseIP and
// netip.ParseAddr simply reject inputs with leading "0"s.)
//
// 2. IPv4-mapped IPv6 IPs (e.g. "::ffff:1.2.3.4") are allowed. These can also lead to
// different software interpreting the value in different ways, because they may be
// treated as IPv4 by some software and IPv6 by other software. (net.ParseIP and
// netip.ParseAddr both allow these, but there are no use cases for representing IPv4
// addresses as IPv4-mapped IPv6 addresses in Kubernetes.)
//
// Alternatively, when validating an update to an existing field, you can pass a list of
// IP values from the old object that should be accepted if they appear in the new object
// even if they are not valid.
//
// This function should only be used to validate the existing fields that were
// historically validated in this way, and strictValidation should be true unless the
// StrictIPCIDRValidation feature gate is disabled. Use IsValidIP for parsing new fields.
func IsValidIPForLegacyField(fldPath *field.Path, value string, strictValidation bool, validOldIPs []string) field.ErrorList {
if slices.Contains(validOldIPs, value) {
return nil
}
_, allErrors := parseIP(fldPath, value, strictValidation)
return allErrors.WithOrigin("format=ip-sloppy")
}
// IsValidIP tests that the argument is a valid IP address, according to current
// Kubernetes standards for IP address validation.
func IsValidIP(fldPath *field.Path, value string) field.ErrorList {
ip, allErrors := parseIP(fldPath, value, true)
if len(allErrors) != 0 {
return allErrors.WithOrigin("format=ip-strict")
}
if value != ip.String() {
allErrors = append(allErrors, field.Invalid(fldPath, value, fmt.Sprintf("must be in canonical form (%q)", ip.String())))
}
return allErrors.WithOrigin("format=ip-strict")
} }
// GetWarningsForIP returns warnings for IP address values in non-standard forms. This // GetWarningsForIP returns warnings for IP address values in non-standard forms. This
// should only be used with fields that are validated with IsValidIP(). // should only be used with fields that are validated with IsValidIPForLegacyField().
func GetWarningsForIP(fldPath *field.Path, value string) []string { func GetWarningsForIP(fldPath *field.Path, value string) []string {
ip := netutils.ParseIPSloppy(value) ip := netutils.ParseIPSloppy(value)
if ip == nil { if ip == nil {
klog.ErrorS(nil, "GetWarningsForIP called on value that was not validated with IsValidIP", "field", fldPath, "value", value) klog.ErrorS(nil, "GetWarningsForIP called on value that was not validated with IsValidIPForLegacyField", "field", fldPath, "value", value)
return nil return nil
} }
@ -64,42 +130,88 @@ func GetWarningsForIP(fldPath *field.Path, value string) []string {
return nil return nil
} }
// IsValidIPv4Address tests that the argument is a valid IPv4 address. func parseCIDR(fldPath *field.Path, value string, strictValidation bool) (*net.IPNet, field.ErrorList) {
func IsValidIPv4Address(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList var allErrors field.ErrorList
ip := netutils.ParseIPSloppy(value)
if ip == nil || ip.To4() == nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IPv4 address"))
}
return allErrors
}
// IsValidIPv6Address tests that the argument is a valid IPv6 address. _, ipnet, err := netutils.ParseCIDRSloppy(value)
func IsValidIPv6Address(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList
ip := netutils.ParseIPSloppy(value)
if ip == nil || ip.To4() != nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IPv6 address"))
}
return allErrors
}
// IsValidCIDR tests that the argument is a valid CIDR value.
func IsValidCIDR(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList
_, _, err := netutils.ParseCIDRSloppy(value)
if err != nil { if err != nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)")) allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"))
return nil, allErrors
}
if strictValidation {
prefix, err := netip.ParsePrefix(value)
if err != nil {
// If netutils.ParseCIDRSloppy parsed it, but netip.ParsePrefix
// doesn't, then it must have illegal leading 0s (either in the
// IP part or the prefix).
allErrors = append(allErrors, field.Invalid(fldPath, value, "must not have leading 0s in IP or prefix length"))
} else if prefix.Addr().Is4In6() {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must not have an IPv4-mapped IPv6 address"))
} else if prefix.Addr() != prefix.Masked().Addr() {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must not have bits set beyond the prefix length"))
}
}
return ipnet, allErrors
}
// IsValidCIDRForLegacyField tests that the argument is a valid CIDR value for a "legacy"
// API field that predates strict IP validation. In particular, this allows IPs that are
// not in canonical form (e.g., "FE80:0abc:0:0:0:0:0:0/64" instead of "fe80:abc::/64").
//
// If strictValidation is false, this also allows CIDR values in certain invalid or
// ambiguous formats:
//
// 1. The IP part of the CIDR value is parsed as with IsValidIPForLegacyField with
// strictValidation=false.
//
// 2. The CIDR value is allowed to be either a "subnet"/"mask" (with the lower bits after
// the prefix length all being 0), or an "interface address" as with `ip addr` (with a
// complete IP address and associated subnet length). With strict validation, the
// value is required to be in "subnet"/"mask" form.
//
// 3. The prefix length is allowed to have leading 0s.
//
// Alternatively, when validating an update to an existing field, you can pass a list of
// CIDR values from the old object that should be accepted if they appear in the new
// object even if they are not valid.
//
// This function should only be used to validate the existing fields that were
// historically validated in this way, and strictValidation should be true unless the
// StrictIPCIDRValidation feature gate is disabled. Use IsValidCIDR or
// IsValidInterfaceAddress for parsing new fields.
func IsValidCIDRForLegacyField(fldPath *field.Path, value string, strictValidation bool, validOldCIDRs []string) field.ErrorList {
if slices.Contains(validOldCIDRs, value) {
return nil
}
_, allErrors := parseCIDR(fldPath, value, strictValidation)
return allErrors
}
// IsValidCIDR tests that the argument is a valid CIDR value, according to current
// Kubernetes standards for CIDR validation. This function is only for
// "subnet"/"mask"-style CIDR values (e.g., "192.168.1.0/24", with no bits set beyond the
// prefix length). Use IsValidInterfaceAddress for "ifaddr"-style CIDR values.
func IsValidCIDR(fldPath *field.Path, value string) field.ErrorList {
ipnet, allErrors := parseCIDR(fldPath, value, true)
if len(allErrors) != 0 {
return allErrors
}
if value != ipnet.String() {
allErrors = append(allErrors, field.Invalid(fldPath, value, fmt.Sprintf("must be in canonical form (%q)", ipnet.String())))
} }
return allErrors return allErrors
} }
// GetWarningsForCIDR returns warnings for CIDR values in non-standard forms. This should // GetWarningsForCIDR returns warnings for CIDR values in non-standard forms. This should
// only be used with fields that are validated with IsValidCIDR(). // only be used with fields that are validated with IsValidCIDRForLegacyField().
func GetWarningsForCIDR(fldPath *field.Path, value string) []string { func GetWarningsForCIDR(fldPath *field.Path, value string) []string {
ip, ipnet, err := netutils.ParseCIDRSloppy(value) ip, ipnet, err := netutils.ParseCIDRSloppy(value)
if err != nil { if err != nil {
klog.ErrorS(err, "GetWarningsForCIDR called on value that was not validated with IsValidCIDR", "field", fldPath, "value", value) klog.ErrorS(err, "GetWarningsForCIDR called on value that was not validated with IsValidCIDRForLegacyField", "field", fldPath, "value", value)
return nil return nil
} }
@ -137,3 +249,30 @@ func GetWarningsForCIDR(fldPath *field.Path, value string) []string {
return warnings return warnings
} }
// IsValidInterfaceAddress tests that the argument is a valid "ifaddr"-style CIDR value in
// canonical form (e.g., "192.168.1.5/24", with a complete IP address and associated
// subnet length). Use IsValidCIDR for "subnet"/"mask"-style CIDR values (e.g.,
// "192.168.1.0/24").
func IsValidInterfaceAddress(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList
ip, ipnet, err := netutils.ParseCIDRSloppy(value)
if err != nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/64)"))
return allErrors
}
// The canonical form of `value` is not `ipnet.String()`, because `ipnet` doesn't
// include the bits after the prefix. We need to construct the canonical form
// ourselves from `ip` and `ipnet.Mask`.
maskSize, _ := ipnet.Mask.Size()
if netutils.IsIPv4(ip) && maskSize > net.IPv4len*8 {
// "::ffff:192.168.0.1/120" -> "192.168.0.1/24"
maskSize -= (net.IPv6len - net.IPv4len) * 8
}
canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize)
if value != canonical {
allErrors = append(allErrors, field.Invalid(fldPath, value, fmt.Sprintf("must be in canonical form (%q)", canonical)))
}
return allErrors
}

413
staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go
View File

@ -25,140 +25,193 @@ import (
) )
func TestIsValidIP(t *testing.T) { func TestIsValidIP(t *testing.T) {
for _, tc := range []struct { testCases := []struct {
name string name string
in string in string
family int
err string err string
legacyErr string
legacyStrictErr string
}{ }{
// GOOD VALUES // GOOD VALUES
{ {
name: "ipv4", name: "ipv4",
in: "1.2.3.4", in: "1.2.3.4",
family: 4,
}, },
{ {
name: "ipv4, all zeros", name: "ipv4, all zeros",
in: "0.0.0.0", in: "0.0.0.0",
family: 4,
}, },
{ {
name: "ipv4, max", name: "ipv4, max",
in: "255.255.255.255", in: "255.255.255.255",
family: 4,
}, },
{ {
name: "ipv6", name: "ipv6",
in: "1234::abcd", in: "1234::abcd",
family: 6,
}, },
{ {
name: "ipv6, all zeros, collapsed", name: "ipv6, all zeros, collapsed",
in: "::", in: "::",
family: 6,
}, },
{ {
name: "ipv6, max", name: "ipv6, max",
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
family: 6,
}, },
// GOOD, THOUGH NON-CANONICAL, VALUES // NON-CANONICAL VALUES
{ {
name: "ipv6, all zeros, expanded (non-canonical)", name: "ipv6, all zeros, expanded (non-canonical)",
in: "0:0:0:0:0:0:0:0", in: "0:0:0:0:0:0:0:0",
family: 6,
err: `must be in canonical form ("::")`,
}, },
{ {
name: "ipv6, leading 0s (non-canonical)", name: "ipv6, leading 0s (non-canonical)",
in: "0001:002:03:4::", in: "0001:002:03:4::",
family: 6,
err: `must be in canonical form ("1:2:3:4::")`,
}, },
{ {
name: "ipv6, capital letters (non-canonical)", name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD", in: "1234::ABCD",
family: 6,
err: `must be in canonical form ("1234::abcd")`,
}, },
// BAD VALUES WE CURRENTLY CONSIDER GOOD // GOOD WITH LEGACY VALIDATION, BAD WITH STRICT VALIDATION
{ {
name: "ipv4 with leading 0s", name: "ipv4 with leading 0s",
in: "1.1.1.01", in: "1.1.1.01",
family: 4,
err: "must not have leading 0s",
legacyErr: "",
legacyStrictErr: "must not have leading 0s",
}, },
{ {
name: "ipv4-in-ipv6 value", name: "ipv4-in-ipv6 value",
in: "::ffff:1.1.1.1", in: "::ffff:1.1.1.1",
family: 4,
err: "must not be an IPv4-mapped IPv6 address",
legacyErr: "",
legacyStrictErr: "must not be an IPv4-mapped IPv6 address",
}, },
// BAD VALUES // BAD VALUES
{ {
name: "empty string", name: "empty string",
in: "", in: "",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "junk", name: "junk",
in: "aaaaaaa", in: "aaaaaaa",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "domain name", name: "domain name",
in: "myhost.mydomain", in: "myhost.mydomain",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "cidr", name: "cidr",
in: "1.2.3.0/24", in: "1.2.3.0/24",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv4 with out-of-range octets", name: "ipv4 with out-of-range octets",
in: "1.2.3.400", in: "1.2.3.400",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv4 with negative octets", name: "ipv4 with negative octets",
in: "-1.0.0.0", in: "-1.0.0.0",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv6 with out-of-range segment", name: "ipv6 with out-of-range segment",
in: "2001:db8::10005", in: "2001:db8::10005",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv4:port", name: "ipv4:port",
in: "1.2.3.4:80", in: "1.2.3.4:80",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv6 with brackets", name: "ipv6 with brackets",
in: "[2001:db8::1]", in: "[2001:db8::1]",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "[ipv6]:port", name: "[ipv6]:port",
in: "[2001:db8::1]:80", in: "[2001:db8::1]:80",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "host:port", name: "host:port",
in: "example.com:80", in: "example.com:80",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv6 with zone", name: "ipv6 with zone",
in: "1234::abcd%eth0", in: "1234::abcd%eth0",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
{ {
name: "ipv4 with zone", name: "ipv4 with zone",
in: "169.254.0.0%eth0", in: "169.254.0.0%eth0",
err: "must be a valid IP address",
err: "must be a valid IP address",
legacyErr: "must be a valid IP address",
legacyStrictErr: "must be a valid IP address",
}, },
} { }
var badIPs []string
for _, tc := range testCases {
if tc.legacyStrictErr != "" {
badIPs = append(badIPs, tc.in)
}
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
errs := IsValidIP(field.NewPath(""), tc.in) errs := IsValidIP(field.NewPath(""), tc.in)
if tc.err == "" { if tc.err == "" {
@ -173,27 +226,36 @@ func TestIsValidIP(t *testing.T) {
} }
} }
errs = IsValidIPv4Address(field.NewPath(""), tc.in) errs = IsValidIPForLegacyField(field.NewPath(""), tc.in, false, nil)
if tc.family == 4 { if tc.legacyErr == "" {
if len(errs) != 0 { if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs) t.Errorf("expected %q to be valid according to IsValidIPForLegacyField but got: %v", tc.in, errs)
} }
} else { } else {
if len(errs) == 0 { if len(errs) != 1 {
t.Errorf("expected %q to fail IsValidIPv4Address", tc.in) t.Errorf("expected %q to have 1 error from IsValidIPForLegacyField but got: %v", tc.in, errs)
} else if !strings.Contains(errs[0].Detail, tc.legacyErr) {
t.Errorf("expected error from IsValidIPForLegacyField for %q to contain %q but got: %q", tc.in, tc.legacyErr, errs[0].Detail)
} }
} }
errs = IsValidIPv6Address(field.NewPath(""), tc.in) errs = IsValidIPForLegacyField(field.NewPath(""), tc.in, true, nil)
if tc.family == 6 { if tc.legacyStrictErr == "" {
if len(errs) != 0 { if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv6Address but got: %v", tc.in, errs) t.Errorf("expected %q to be valid according to IsValidIPForLegacyField with strict validation, but got: %v", tc.in, errs)
} }
} else { } else {
if len(errs) == 0 { if len(errs) != 1 {
t.Errorf("expected %q to fail IsValidIPv6Address", tc.in) t.Errorf("expected %q to have 1 error from IsValidIPForLegacyField with strict validation, but got: %v", tc.in, errs)
} else if !strings.Contains(errs[0].Detail, tc.legacyStrictErr) {
t.Errorf("expected error from IsValidIPForLegacyField with strict validation for %q to contain %q but got: %q", tc.in, tc.legacyStrictErr, errs[0].Detail)
} }
} }
errs = IsValidIPForLegacyField(field.NewPath(""), tc.in, true, badIPs)
if len(errs) != 0 {
t.Errorf("expected %q to be accepted when using validOldIPs, but got: %v", tc.in, errs)
}
}) })
} }
} }
@ -252,10 +314,13 @@ func TestGetWarningsForIP(t *testing.T) {
} }
func TestIsValidCIDR(t *testing.T) { func TestIsValidCIDR(t *testing.T) {
for _, tc := range []struct { testCases := []struct {
name string name string
in string in string
err string
err string
legacyErr string
legacyStrictErr string
}{ }{
// GOOD VALUES // GOOD VALUES
{ {
@ -283,79 +348,137 @@ func TestIsValidCIDR(t *testing.T) {
in: "::1/128", in: "::1/128",
}, },
// GOOD, THOUGH NON-CANONICAL, VALUES // NON-CANONICAL VALUES
{ {
name: "ipv6, extra 0s (non-canonical)", name: "ipv6, extra 0s (non-canonical)",
in: "2a00:79e0:2:0::/64", in: "2a00:79e0:2:0::/64",
err: `must be in canonical form ("2a00:79e0:2::/64")`,
}, },
{ {
name: "ipv6, capital letters (non-canonical)", name: "ipv6, capital letters (non-canonical)",
in: "2001:DB8::/64", in: "2001:DB8::/64",
err: `must be in canonical form ("2001:db8::/64")`,
}, },
// BAD VALUES WE CURRENTLY CONSIDER GOOD // GOOD WITH LEGACY VALIDATION, BAD WITH STRICT VALIDATION
{ {
name: "ipv4 with leading 0s", name: "ipv4 with leading 0s",
in: "1.1.01.0/24", in: "1.1.01.0/24",
err: "must not have leading 0s in IP",
legacyErr: "",
legacyStrictErr: "must not have leading 0s in IP",
}, },
{ {
name: "ipv4-in-ipv6 with ipv4-sized prefix", name: "ipv4-in-ipv6 with ipv4-sized prefix",
in: "::ffff:1.1.1.0/24", in: "::ffff:1.1.1.0/24",
err: "must not have an IPv4-mapped IPv6 address",
legacyErr: "",
legacyStrictErr: "must not have an IPv4-mapped IPv6 address",
}, },
{ {
name: "ipv4-in-ipv6 with ipv6-sized prefix", name: "ipv4-in-ipv6 with ipv6-sized prefix",
in: "::ffff:1.1.1.0/120", in: "::ffff:1.1.1.0/120",
err: "must not have an IPv4-mapped IPv6 address",
legacyErr: "",
legacyStrictErr: "must not have an IPv4-mapped IPv6 address",
}, },
{ {
name: "ipv4 with bits past prefix", name: "ipv4 ifaddr",
in: "1.2.3.4/24", in: "1.2.3.4/24",
err: "must not have bits set beyond the prefix length",
legacyErr: "",
legacyStrictErr: "must not have bits set beyond the prefix length",
}, },
{ {
name: "ipv6 with bits past prefix", name: "ipv6 ifaddr",
in: "2001:db8::1/64", in: "2001:db8::1/64",
err: "must not have bits set beyond the prefix length",
legacyErr: "",
legacyStrictErr: "must not have bits set beyond the prefix length",
}, },
{ {
name: "prefix length with leading 0s", name: "prefix length with leading 0s",
in: "192.168.0.0/016", in: "192.168.0.0/016",
err: "must not have leading 0s",
legacyErr: "",
legacyStrictErr: "must not have leading 0s",
}, },
// BAD VALUES // BAD VALUES
{ {
name: "empty string", name: "empty string",
in: "", in: "",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "junk", name: "junk",
in: "aaaaaaa", in: "aaaaaaa",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "IP address", name: "IP address",
in: "1.2.3.4", in: "1.2.3.4",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "partial URL", name: "partial URL",
in: "192.168.0.1/healthz", in: "192.168.0.1/healthz",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "partial URL 2", name: "partial URL 2",
in: "192.168.0.1/0/99", in: "192.168.0.1/0/99",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "negative prefix length", name: "negative prefix length",
in: "192.168.0.0/-16", in: "192.168.0.0/-16",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
{ {
name: "prefix length with sign", name: "prefix length with sign",
in: "192.168.0.0/+16", in: "192.168.0.0/+16",
err: "must be a valid CIDR value",
err: "must be a valid CIDR value",
legacyErr: "must be a valid CIDR value",
legacyStrictErr: "must be a valid CIDR value",
}, },
} { }
var badCIDRs []string
for _, tc := range testCases {
if tc.legacyStrictErr != "" {
badCIDRs = append(badCIDRs, tc.in)
}
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
errs := IsValidCIDR(field.NewPath(""), tc.in) errs := IsValidCIDR(field.NewPath(""), tc.in)
if tc.err == "" { if tc.err == "" {
@ -369,6 +492,37 @@ func TestIsValidCIDR(t *testing.T) {
t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail) t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail)
} }
} }
errs = IsValidCIDRForLegacyField(field.NewPath(""), tc.in, false, nil)
if tc.legacyErr == "" {
if len(errs) != 0 {
t.Errorf("expected %q to be valid according to IsValidCIDRForLegacyField but got: %v", tc.in, errs)
}
} else {
if len(errs) != 1 {
t.Errorf("expected %q to have 1 error from IsValidCIDRForLegacyField but got: %v", tc.in, errs)
} else if !strings.Contains(errs[0].Detail, tc.legacyErr) {
t.Errorf("expected error for %q from IsValidCIDRForLegacyField to contain %q but got: %q", tc.in, tc.legacyErr, errs[0].Detail)
}
}
errs = IsValidCIDRForLegacyField(field.NewPath(""), tc.in, true, nil)
if tc.legacyStrictErr == "" {
if len(errs) != 0 {
t.Errorf("expected %q to be valid according to IsValidCIDRForLegacyField with strict validation but got: %v", tc.in, errs)
}
} else {
if len(errs) != 1 {
t.Errorf("expected %q to have 1 error from IsValidCIDRForLegacyField with strict validation but got: %v", tc.in, errs)
} else if !strings.Contains(errs[0].Detail, tc.legacyStrictErr) {
t.Errorf("expected error for %q from IsValidCIDRForLegacyField with strict validation to contain %q but got: %q", tc.in, tc.legacyStrictErr, errs[0].Detail)
}
}
errs = IsValidCIDRForLegacyField(field.NewPath(""), tc.in, true, badCIDRs)
if len(errs) != 0 {
t.Errorf("expected %q to be accepted when using validOldCIDRs, but got: %v", tc.in, errs)
}
}) })
} }
} }
@ -450,3 +604,106 @@ func TestGetWarningsForCIDR(t *testing.T) {
}) })
} }
} }
func TestIsValidInterfaceAddress(t *testing.T) {
for _, tc := range []struct {
name string
in string
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.2.3.4/24",
},
{
name: "ipv4, single IP",
in: "1.1.1.1/32",
},
{
name: "ipv6",
in: "2001:4860:4860::1/48",
},
{
name: "ipv6, single IP",
in: "::1/128",
},
// BAD VALUES
{
name: "empty string",
in: "",
err: "must be a valid address in CIDR form",
},
{
name: "junk",
in: "aaaaaaa",
err: "must be a valid address in CIDR form",
},
{
name: "IP address",
in: "1.2.3.4",
err: "must be a valid address in CIDR form",
},
{
name: "partial URL",
in: "192.168.0.1/healthz",
err: "must be a valid address in CIDR form",
},
{
name: "partial URL 2",
in: "192.168.0.1/0/99",
err: "must be a valid address in CIDR form",
},
{
name: "negative prefix length",
in: "192.168.0.0/-16",
err: "must be a valid address in CIDR form",
},
{
name: "prefix length with sign",
in: "192.168.0.0/+16",
err: "must be a valid address in CIDR form",
},
{
name: "ipv6 non-canonical",
in: "2001:0:0:0::0BCD/64",
err: `must be in canonical form ("2001::bcd/64")`,
},
{
name: "ipv4 with leading 0s",
in: "1.1.01.002/24",
err: `must be in canonical form ("1.1.1.2/24")`,
},
{
name: "ipv4-in-ipv6 with ipv4-sized prefix",
in: "::ffff:1.1.1.1/24",
err: `must be in canonical form ("1.1.1.1/24")`,
},
{
name: "ipv4-in-ipv6 with ipv6-sized prefix",
in: "::ffff:1.1.1.1/120",
err: `must be in canonical form ("1.1.1.1/24")`,
},
{
name: "prefix length with leading 0s",
in: "192.168.0.5/016",
err: `must be in canonical form ("192.168.0.5/16")`,
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := IsValidInterfaceAddress(field.NewPath(""), tc.in)
if tc.err == "" {
if len(errs) != 0 {
t.Errorf("expected %q to be valid but got: %v", tc.in, errs)
}
} else {
if len(errs) != 1 {
t.Errorf("expected %q to have 1 error but got: %v", tc.in, errs)
} else if !strings.Contains(errs[0].Detail, tc.err) {
t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail)
}
}
})
}
}

6
test/featuregates_linter/test_data/versioned_feature_list.yaml
View File

@ -1388,6 +1388,12 @@
lockToDefault: true lockToDefault: true
preRelease: GA preRelease: GA
version: "1.32" version: "1.32"
- name: StrictIPCIDRValidation
versionedSpecs:
- default: false
lockToDefault: false
preRelease: Alpha
version: "1.33"
- name: StructuredAuthenticationConfiguration - name: StructuredAuthenticationConfiguration
versionedSpecs: versionedSpecs:
- default: false - default: false