Add legacy versions of IsValidIP/IsValidCIDR

Add validation.IsValidIPForLegacyField and
validation.IsValidCIDRForLegacyField, which validate "legacy" IP/CIDR
fields correctly. Use them for all such fields (indirectly, via a
wrapper in pkg/apis/core/validation that handles the
StrictIPCIDRValidation feature gate correctly).

Change IsValidIP and IsValidCIDR to require strict parsing and
canonical form, and update the IPAddr, ServiceCIDR, and
NetworkDeviceData validation to make use of them.
This commit is contained in:
Dan Winship 2025-02-28 17:41:10 -05:00
parent ba189de78f
commit 692785d25b
11 changed files with 709 additions and 88 deletions

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)...)
} }
// 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)...)
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))...)
} }
@ -4115,7 +4115,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList {
// all PodIPs must be valid IPs // all PodIPs must be valid IPs
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)...)
} }
// 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
@ -4156,7 +4156,7 @@ func validateHostIPs(pod *core.Pod) field.ErrorList {
// all HostIPs must be valid IPs // all HostIPs must be valid IPs
for i, hostIP := range pod.Status.HostIPs { for i, hostIP := range pod.Status.HostIPs {
allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...) allErrs = append(allErrs, IsValidIPForLegacyField(hostIPsField.Index(i), hostIP.IP)...)
} }
// 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
@ -5940,7 +5940,7 @@ func ValidateService(service *core.Service) field.ErrorList {
ipPath := specPath.Child("externalIPs") ipPath := specPath.Child("externalIPs")
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); len(errs) != 0 {
allErrs = append(allErrs, errs...) allErrs = append(allErrs, errs...)
} else { } else {
// For historical reasons, this uses ValidateEndpointIP even // For historical reasons, this uses ValidateEndpointIP even
@ -6008,7 +6008,7 @@ func ValidateService(service *core.Service) field.ErrorList {
// 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)...)
} }
} 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)
@ -6021,7 +6021,7 @@ func ValidateService(service *core.Service) field.ErrorList {
cidrs := strings.Split(val, ",") cidrs := strings.Split(val, ",")
for _, value := range cidrs { for _, value := range cidrs {
value = strings.TrimSpace(value) value = strings.TrimSpace(value)
allErrs = append(allErrs, validation.IsValidCIDR(fieldPath, value)...) allErrs = append(allErrs, IsValidCIDRForLegacyField(fieldPath, value)...)
} }
} }
} }
@ -6405,7 +6405,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)...)
} }
// if more than PodCIDR then it must be a dual-stack pair // if more than PodCIDR then it must be a dual-stack pair
@ -7481,7 +7481,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)...)
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"))...)
} }
@ -7853,7 +7853,7 @@ func ValidateLoadBalancerStatus(status *core.LoadBalancerStatus, fldPath *field.
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)...)
} }
if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil { if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil {
@ -8188,7 +8188,7 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi
} }
// is it valid ip? // is it valid ip?
errorMessages := validation.IsValidIP(clusterIPsField.Index(i), clusterIP) errorMessages := IsValidIPForLegacyField(clusterIPsField.Index(i), clusterIP)
hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs
allErrs = append(allErrs, errorMessages...) allErrs = append(allErrs, errorMessages...)
} }
@ -8703,3 +8703,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) field.ErrorList {
return validation.IsValidIPForLegacyField(fldPath, value, utilfeature.DefaultFeatureGate.Enabled(features.StrictIPCIDRValidation))
}
// 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) field.ErrorList {
return validation.IsValidCIDRForLegacyField(fldPath, value, utilfeature.DefaultFeatureGate.Enabled(features.StrictIPCIDRValidation))
}

View File

@ -9738,6 +9738,7 @@ func TestValidatePodDNSConfig(t *testing.T) {
dnsConfig *core.PodDNSConfig dnsConfig *core.PodDNSConfig
dnsPolicy *core.DNSPolicy dnsPolicy *core.DNSPolicy
opts PodValidationOptions opts PodValidationOptions
legacyIPs bool
expectedError bool expectedError bool
}{{ }{{
desc: "valid: empty DNSConfig", desc: "valid: empty DNSConfig",
@ -9932,6 +9933,19 @@ func TestValidatePodDNSConfig(t *testing.T) {
Nameservers: []string{"invalid"}, Nameservers: []string{"invalid"},
}, },
expectedError: true, expectedError: true,
}, {
desc: "valid legacy IP nameserver with legacy IP validation",
dnsConfig: &core.PodDNSConfig{
Nameservers: []string{"001.002.003.004"},
},
legacyIPs: true,
expectedError: false,
}, {
desc: "invalid legacy IP nameserver with strict IP validation",
dnsConfig: &core.PodDNSConfig{
Nameservers: []string{"001.002.003.004"},
},
expectedError: true,
}, { }, {
desc: "invalid empty option name", desc: "invalid empty option name",
dnsConfig: &core.PodDNSConfig{ dnsConfig: &core.PodDNSConfig{
@ -9952,6 +9966,8 @@ func TestValidatePodDNSConfig(t *testing.T) {
for _, tc := range testCases { for _, tc := range testCases {
t.Run("", func(t *testing.T) { t.Run("", func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !tc.legacyIPs)
if tc.dnsPolicy == nil { if tc.dnsPolicy == nil {
tc.dnsPolicy = &testDNSClusterFirst tc.dnsPolicy = &testDNSClusterFirst
} }
@ -10192,6 +10208,24 @@ func TestValidatePodSpec(t *testing.T) {
} }
for k, v := range successCases { for k, v := range successCases {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
opts := PodValidationOptions{
ResourceIsPod: true,
}
if errs := ValidatePodSpec(&v.Spec, nil, field.NewPath("field"), opts); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
})
}
legacyValidationCases := map[string]*core.Pod{
"populate HostAliases with legacy IP with legacy validation": podtest.MakePod("",
podtest.SetHostAliases(core.HostAlias{IP: "012.034.056.078", Hostnames: []string{"host1", "host2"}}),
),
}
for k, v := range legacyValidationCases {
t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, false)
opts := PodValidationOptions{ opts := PodValidationOptions{
ResourceIsPod: true, ResourceIsPod: true,
} }
@ -10237,6 +10271,12 @@ func TestValidatePodSpec(t *testing.T) {
}), }),
podtest.SetHostAliases(core.HostAlias{IP: "999.999.999.999", Hostnames: []string{"host1", "host2"}}), podtest.SetHostAliases(core.HostAlias{IP: "999.999.999.999", Hostnames: []string{"host1", "host2"}}),
), ),
"with hostAliases with invalid legacy IP with strict IP validation": *podtest.MakePod("",
podtest.SetSecurityContext(&core.PodSecurityContext{
HostNetwork: false,
}),
podtest.SetHostAliases(core.HostAlias{IP: "001.002.003.004", Hostnames: []string{"host1", "host2"}}),
),
"with hostAliases with invalid hostname": *podtest.MakePod("", "with hostAliases with invalid hostname": *podtest.MakePod("",
podtest.SetSecurityContext(&core.PodSecurityContext{ podtest.SetSecurityContext(&core.PodSecurityContext{
HostNetwork: false, HostNetwork: false,
@ -10307,6 +10347,7 @@ func TestValidatePodSpec(t *testing.T) {
} }
for k, v := range failureCases { for k, v := range failureCases {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
opts := PodValidationOptions{ opts := PodValidationOptions{
ResourceIsPod: true, ResourceIsPod: true,
} }
@ -15118,7 +15159,17 @@ func TestValidateServiceCreate(t *testing.T) {
tweakSvc func(svc *core.Service) // given a basic valid service, each test case can customize it tweakSvc func(svc *core.Service) // given a basic valid service, each test case can customize it
numErrs int numErrs int
featureGates []featuregate.Feature featureGates []featuregate.Feature
legacyIPs bool
}{{ }{{
name: "default",
tweakSvc: func(s *core.Service) {},
numErrs: 0,
}, {
name: "default, with legacy IP validation",
tweakSvc: func(s *core.Service) {},
legacyIPs: true,
numErrs: 0,
}, {
name: "missing namespace", name: "missing namespace",
tweakSvc: func(s *core.Service) { tweakSvc: func(s *core.Service) {
s.Namespace = "" s.Namespace = ""
@ -15254,6 +15305,21 @@ func TestValidateServiceCreate(t *testing.T) {
s.Spec.ClusterIPs = []string{"invalid"} s.Spec.ClusterIPs = []string{"invalid"}
}, },
numErrs: 1, numErrs: 1,
}, {
name: "valid legacy cluster ip with legacy validation",
tweakSvc: func(s *core.Service) {
s.Spec.ClusterIP = "001.002.003.004"
s.Spec.ClusterIPs = []string{"001.002.003.004"}
},
legacyIPs: true,
numErrs: 0,
}, {
name: "invalid legacy cluster ip with strict validation",
tweakSvc: func(s *core.Service) {
s.Spec.ClusterIP = "001.002.003.004"
s.Spec.ClusterIPs = []string{"001.002.003.004"}
},
numErrs: 1,
}, { }, {
name: "missing port", name: "missing port",
tweakSvc: func(s *core.Service) { tweakSvc: func(s *core.Service) {
@ -15333,6 +15399,21 @@ func TestValidateServiceCreate(t *testing.T) {
s.Spec.ExternalIPs = []string{"myhost.mydomain"} s.Spec.ExternalIPs = []string{"myhost.mydomain"}
}, },
numErrs: 1, numErrs: 1,
}, {
name: "valid legacy externalIPs with legacy validation",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"001.002.003.004"}
},
legacyIPs: true,
numErrs: 0,
}, {
name: "invalid legacy externalIPs with strict validation",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"001.002.003.004"}
},
numErrs: 1,
}, { }, {
name: "valid externalIPs", name: "valid externalIPs",
tweakSvc: func(s *core.Service) { tweakSvc: func(s *core.Service) {
@ -15644,6 +15725,34 @@ func TestValidateServiceCreate(t *testing.T) {
s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "" s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = ""
}, },
numErrs: 1, numErrs: 1,
}, {
name: "valid legacy LoadBalancer source range with legacy validation",
tweakSvc: func(s *core.Service) {
s.Spec.Type = core.ServiceTypeLoadBalancer
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.AllocateLoadBalancerNodePorts = ptr.To(true)
s.Spec.LoadBalancerSourceRanges = []string{"001.002.003.000/24"}
},
legacyIPs: true,
numErrs: 0,
}, {
name: "invalid legacy LoadBalancer source range with strict validation",
tweakSvc: func(s *core.Service) {
s.Spec.Type = core.ServiceTypeLoadBalancer
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.AllocateLoadBalancerNodePorts = ptr.To(true)
s.Spec.LoadBalancerSourceRanges = []string{"001.002.003.000/24"}
},
numErrs: 1,
}, {
name: "invalid legacy LoadBalancer source range annotation with strict validation",
tweakSvc: func(s *core.Service) {
s.Spec.Type = core.ServiceTypeLoadBalancer
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.AllocateLoadBalancerNodePorts = ptr.To(true)
s.Annotations[core.AnnotationLoadBalancerSourceRangesKey] = "001.002.003.000/24"
},
numErrs: 1,
}, { }, {
name: "valid LoadBalancer source range", name: "valid LoadBalancer source range",
tweakSvc: func(s *core.Service) { tweakSvc: func(s *core.Service) {
@ -16318,6 +16427,7 @@ func TestValidateServiceCreate(t *testing.T) {
for i := range tc.featureGates { for i := range tc.featureGates {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, tc.featureGates[i], true) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, tc.featureGates[i], true)
} }
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !tc.legacyIPs)
svc := makeValidService() svc := makeValidService()
tc.tweakSvc(&svc) tc.tweakSvc(&svc)
errs := ValidateServiceCreate(&svc) errs := ValidateServiceCreate(&svc)
@ -16994,12 +17104,41 @@ func TestValidateNode(t *testing.T) {
} }
for _, successCase := range successCases { for _, successCase := range successCases {
t.Run("", func(t *testing.T) { t.Run("", func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
if errs := ValidateNode(&successCase); len(errs) != 0 { if errs := ValidateNode(&successCase); len(errs) != 0 {
t.Errorf("expected success: %v", errs) t.Errorf("expected success: %v", errs)
} }
}) })
} }
legacyValidationCases := map[string]core.Node{
"valid-legacy-pod-cidr-with-legacy-validation": {
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
},
Status: core.NodeStatus{
Addresses: []core.NodeAddress{
{Type: core.NodeExternalIP, Address: "something"},
},
Capacity: core.ResourceList{
core.ResourceName(core.ResourceCPU): resource.MustParse("10"),
core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
},
},
Spec: core.NodeSpec{
PodCIDRs: []string{"192.168.000.000/16"},
},
},
}
for name, legacyCase := range legacyValidationCases {
t.Run(name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, false)
if errs := ValidateNode(&legacyCase); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
})
}
errorCases := map[string]core.Node{ errorCases := map[string]core.Node{
"zero-length Name": { "zero-length Name": {
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
@ -17182,6 +17321,23 @@ func TestValidateNode(t *testing.T) {
PodCIDRs: []string{"192.168.0.0"}, PodCIDRs: []string{"192.168.0.0"},
}, },
}, },
"invalid-legacy-pod-cidr-with-strict-validation": {
ObjectMeta: metav1.ObjectMeta{
Name: "abc",
},
Status: core.NodeStatus{
Addresses: []core.NodeAddress{
{Type: core.NodeExternalIP, Address: "something"},
},
Capacity: core.ResourceList{
core.ResourceName(core.ResourceCPU): resource.MustParse("10"),
core.ResourceName(core.ResourceMemory): resource.MustParse("0"),
},
},
Spec: core.NodeSpec{
PodCIDRs: []string{"192.168.000.000/16"},
},
},
"duplicate-pod-cidr": { "duplicate-pod-cidr": {
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: "abc", Name: "abc",
@ -17202,6 +17358,8 @@ func TestValidateNode(t *testing.T) {
} }
for k, v := range errorCases { for k, v := range errorCases {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateNode(&v) errs := ValidateNode(&v)
if len(errs) == 0 { if len(errs) == 0 {
t.Errorf("expected failure") t.Errorf("expected failure")
@ -20590,9 +20748,36 @@ func TestValidateEndpointsCreate(t *testing.T) {
}, },
}, },
} }
for name, tc := range successCases { for name, tc := range successCases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateEndpointsCreate(&tc.endpoints)
if len(errs) != 0 {
t.Errorf("Expected no validation errors, got %v", errs)
}
})
}
legacyValidationCases := map[string]struct {
endpoints core.Endpoints
}{
"legacy IPs with legacy validation": {
endpoints: core.Endpoints{
ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
Subsets: []core.EndpointSubset{{
Addresses: []core.EndpointAddress{{IP: "010.010.001.001"}, {IP: "10.10.2.2"}},
Ports: []core.EndpointPort{{Name: "a", Port: 8675, Protocol: "TCP"}, {Name: "b", Port: 309, Protocol: "TCP"}},
}, {
Addresses: []core.EndpointAddress{{IP: "::ffff:10.10.3.3"}},
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}, {Name: "b", Port: 76, Protocol: "TCP"}},
}},
},
},
}
for name, tc := range legacyValidationCases {
t.Run(name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, false)
errs := ValidateEndpointsCreate(&tc.endpoints) errs := ValidateEndpointsCreate(&tc.endpoints)
if len(errs) != 0 { if len(errs) != 0 {
t.Errorf("Expected no validation errors, got %v", errs) t.Errorf("Expected no validation errors, got %v", errs)
@ -20652,6 +20837,18 @@ func TestValidateEndpointsCreate(t *testing.T) {
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=ip-sloppy"), field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=ip-sloppy"),
}, },
}, },
"invalid legacy IP with strict validation": {
endpoints: core.Endpoints{
ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
Subsets: []core.EndpointSubset{{
Addresses: []core.EndpointAddress{{IP: "001.002.003.004"}},
Ports: []core.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}},
}},
},
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=ip-sloppy"),
},
},
"Multiple ports, one without name": { "Multiple ports, one without name": {
endpoints: core.Endpoints{ endpoints: core.Endpoints{
ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
@ -20776,6 +20973,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
for k, v := range errorCases { for k, v := range errorCases {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateEndpointsCreate(&v.endpoints) errs := ValidateEndpointsCreate(&v.endpoints)
// TODO: set .RequireOriginWhenInvalid() once metadata is done // TODO: set .RequireOriginWhenInvalid() once metadata is done
matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin() matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin()
@ -22838,6 +23036,7 @@ func TestPodIPsValidation(t *testing.T) {
testCases := []struct { testCases := []struct {
pod core.Pod pod core.Pod
legacyIPs bool
expectError bool expectError bool
}{ }{
{ {
@ -22858,11 +23057,18 @@ func TestPodIPsValidation(t *testing.T) {
}, { }, {
expectError: false, expectError: false,
pod: makePod("dual-stack-6-4", "ns", []core.PodIP{{IP: "::1"}, {IP: "1.1.1.1"}}), pod: makePod("dual-stack-6-4", "ns", []core.PodIP{{IP: "::1"}, {IP: "1.1.1.1"}}),
}, {
expectError: false,
legacyIPs: true,
pod: makePod("legacy-pod-ip-with-legacy-validation", "ns", []core.PodIP{{IP: "001.002.003.004"}}),
}, },
/* failure cases start here */ /* failure cases start here */
{ {
expectError: true, expectError: true,
pod: makePod("invalid-pod-ip", "ns", []core.PodIP{{IP: "this-is-not-an-ip"}}), pod: makePod("invalid-pod-ip", "ns", []core.PodIP{{IP: "this-is-not-an-ip"}}),
}, {
expectError: true,
pod: makePod("legacy-pod-ip-with-strict-validation", "ns", []core.PodIP{{IP: "001.002.003.004"}}),
}, { }, {
expectError: true, expectError: true,
pod: makePod("dualstack-same-ip-family-6", "ns", []core.PodIP{{IP: "::1"}, {IP: "::2"}}), pod: makePod("dualstack-same-ip-family-6", "ns", []core.PodIP{{IP: "::1"}, {IP: "::2"}}),
@ -22888,6 +23094,7 @@ func TestPodIPsValidation(t *testing.T) {
for _, testCase := range testCases { for _, testCase := range testCases {
t.Run(testCase.pod.Name, func(t *testing.T) { t.Run(testCase.pod.Name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !testCase.legacyIPs)
for _, oldTestCase := range testCases { for _, oldTestCase := range testCases {
newPod := testCase.pod.DeepCopy() newPod := testCase.pod.DeepCopy()
newPod.ResourceVersion = "1" newPod.ResourceVersion = "1"
@ -22938,6 +23145,7 @@ func TestHostIPsValidation(t *testing.T) {
testCases := []struct { testCases := []struct {
pod core.Pod pod core.Pod
legacyIPs bool
expectError bool expectError bool
}{ }{
{ {
@ -22964,11 +23172,20 @@ func TestHostIPsValidation(t *testing.T) {
expectError: false, expectError: false,
pod: makePodWithHostIPs("dual-stack-6-4", "ns", []core.HostIP{{IP: "::1"}, {IP: "1.1.1.1"}}), pod: makePodWithHostIPs("dual-stack-6-4", "ns", []core.HostIP{{IP: "::1"}, {IP: "1.1.1.1"}}),
}, },
{
expectError: false,
legacyIPs: true,
pod: makePodWithHostIPs("legacy-host-ip-with-legacy-validation", "ns", []core.HostIP{{IP: "001.002.003.004"}}),
},
/* failure cases start here */ /* failure cases start here */
{ {
expectError: true, expectError: true,
pod: makePodWithHostIPs("invalid-host-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}), pod: makePodWithHostIPs("invalid-host-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}),
}, },
{
expectError: true,
pod: makePodWithHostIPs("invalid-legacy-host-ip-with-strict-validation", "ns", []core.HostIP{{IP: "001.002.003.004"}}),
},
{ {
expectError: true, expectError: true,
pod: makePodWithHostIPs("dualstack-same-ip-family-6", "ns", []core.HostIP{{IP: "::1"}, {IP: "::2"}}), pod: makePodWithHostIPs("dualstack-same-ip-family-6", "ns", []core.HostIP{{IP: "::1"}, {IP: "::2"}}),
@ -22998,6 +23215,7 @@ func TestHostIPsValidation(t *testing.T) {
for _, testCase := range testCases { for _, testCase := range testCases {
t.Run(testCase.pod.Name, func(t *testing.T) { t.Run(testCase.pod.Name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !testCase.legacyIPs)
for _, oldTestCase := range testCases { for _, oldTestCase := range testCases {
newPod := testCase.pod.DeepCopy() newPod := testCase.pod.DeepCopy()
newPod.ResourceVersion = "1" newPod.ResourceVersion = "1"
@ -24566,6 +24784,7 @@ func TestValidateLoadBalancerStatus(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
ipModeEnabled bool ipModeEnabled bool
legacyIPs bool
nonLBAllowed bool nonLBAllowed bool
tweakLBStatus func(s *core.LoadBalancerStatus) tweakLBStatus func(s *core.LoadBalancerStatus)
tweakSvcSpec func(s *core.ServiceSpec) tweakSvcSpec func(s *core.ServiceSpec)
@ -24652,12 +24871,37 @@ func TestValidateLoadBalancerStatus(t *testing.T) {
}} }}
}, },
numErrs: 1, numErrs: 1,
}, {
name: "legacy IP with legacy validation",
ipModeEnabled: true,
legacyIPs: true,
tweakLBStatus: func(s *core.LoadBalancerStatus) {
s.Ingress = []core.LoadBalancerIngress{{
IP: "001.002.003.004",
IPMode: &ipModeVIP,
}}
},
numErrs: 0,
}, {
name: "legacy IP with strict validation",
ipModeEnabled: true,
tweakLBStatus: func(s *core.LoadBalancerStatus) {
s.Ingress = []core.LoadBalancerIngress{{
IP: "001.002.003.004",
IPMode: &ipModeVIP,
}}
},
numErrs: 1,
}, },
} }
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
if !tc.ipModeEnabled { if !tc.ipModeEnabled {
featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31")) featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31"))
} else {
// (This feature gate doesn't exist in 1.31 so we can't set it
// when testing !ipModeEnabled.)
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !tc.legacyIPs)
} }
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LoadBalancerIPMode, tc.ipModeEnabled) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LoadBalancerIPMode, tc.ipModeEnabled)
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AllowServiceLBStatusOnNonLB, tc.nonLBAllowed) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AllowServiceLBStatusOnNonLB, tc.nonLBAllowed)

View File

@ -100,7 +100,7 @@ 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:
ipErrs := validation.IsValidIP(addressPath.Index(i), address) ipErrs := apivalidation.IsValidIPForLegacyField(addressPath.Index(i), address)
if len(ipErrs) > 0 { if len(ipErrs) > 0 {
allErrs = append(allErrs, ipErrs...) allErrs = append(allErrs, ipErrs...)
} else { } else {

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": {
@ -422,6 +442,21 @@ 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: 2, expectedErrors: 2,
endpointSlice: &discovery.EndpointSlice{ endpointSlice: &discovery.EndpointSlice{
@ -601,6 +636,7 @@ 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) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, !testCase.legacyIPs)
errs := ValidateEndpointSlice(testCase.endpointSlice) errs := ValidateEndpointSlice(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)

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"
@ -220,7 +219,7 @@ func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path) field.ErrorLi
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)...)
_, 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 +228,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)...)
_, 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.
@ -357,7 +356,7 @@ func ValidateIngressLoadBalancerStatus(status *networking.IngressLoadBalancerSta
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)...)
} }
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 +652,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 +747,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

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,9 +251,23 @@ func TestValidateNetworkPolicy(t *testing.T) {
} }
// Success cases are expected to pass validation. // Success cases are expected to pass validation.
for _, v := range successCases { for _, v := range successCases {
t.Run("", func(t *testing.T) { 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)
}
})
}
legacyValidationCases := []*networking.NetworkPolicy{
makeNetworkPolicyCustom(setIngressFromIPBlockIPV4, func(networkPolicy *networking.NetworkPolicy) {
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 { if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
t.Errorf("Expected success, got %v", errs) t.Errorf("Expected success, got %v", errs)
} }
@ -301,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{""}
}), }),
@ -370,6 +390,7 @@ 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 {
t.Run(testName, func(t *testing.T) { t.Run(testName, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 { if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 {
t.Errorf("Expected failure") t.Errorf("Expected failure")
} }
@ -1813,6 +1834,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{
@ -1822,22 +1851,46 @@ 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,
},
}
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 {
t.Run(k, func(t *testing.T) { t.Run(k, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StrictIPCIDRValidation, true)
errs := ValidateIngressStatusUpdate(&v, &oldValue) errs := ValidateIngressStatusUpdate(&v, &oldValue)
if len(errs) == 0 { if len(errs) == 0 {
t.Errorf("expected failure") t.Errorf("expected failure")
} else { } else {
s := strings.Split(k, ":") s := strings.SplitN(k, ":", 2)
err := errs[0] err := errs[0]
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) { if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
t.Errorf("unexpected error: %q, expected: %q", err, k) t.Errorf("unexpected error: %q, expected: %q", err, k)
@ -2160,13 +2213,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"},
}, },
}, },
}, },

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
// //

View File

@ -763,6 +763,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},

View File

@ -26,21 +26,78 @@ import (
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.)
//
// 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) field.ErrorList {
_, 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
} }
@ -65,22 +122,80 @@ func GetWarningsForIP(fldPath *field.Path, value string) []string {
return nil return nil
} }
// IsValidCIDR tests that the argument is a valid CIDR value. func parseCIDR(fldPath *field.Path, value string, strictValidation bool) (*net.IPNet, field.ErrorList) {
func IsValidCIDR(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList var allErrors field.ErrorList
_, _, err := netutils.ParseCIDRSloppy(value)
_, ipnet, 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.
//
// 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) field.ErrorList {
_, 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
} }

View File

@ -28,7 +28,10 @@ func TestIsValidIP(t *testing.T) {
for _, tc := range []struct { for _, tc := range []struct {
name string name string
in string in string
err string
err string
legacyErr string
legacyStrictErr string
}{ }{
// GOOD VALUES // GOOD VALUES
{ {
@ -56,95 +59,148 @@ func TestIsValidIP(t *testing.T) {
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
}, },
// 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",
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::",
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",
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",
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",
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",
}, },
} { } {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
@ -160,6 +216,32 @@ func TestIsValidIP(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 = IsValidIPForLegacyField(field.NewPath(""), tc.in, false)
if tc.legacyErr == "" {
if len(errs) != 0 {
t.Errorf("expected %q to be valid according to IsValidIPForLegacyField but got: %v", tc.in, errs)
}
} else {
if len(errs) != 1 {
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 = IsValidIPForLegacyField(field.NewPath(""), tc.in, true)
if tc.legacyStrictErr == "" {
if len(errs) != 0 {
t.Errorf("expected %q to be valid according to IsValidIPForLegacyField with strict validation, but got: %v", tc.in, errs)
}
} else {
if len(errs) != 1 {
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)
}
}
}) })
} }
} }
@ -221,7 +303,10 @@ func TestIsValidCIDR(t *testing.T) {
for _, tc := range []struct { for _, tc := range []struct {
name string name string
in string in string
err string
err string
legacyErr string
legacyStrictErr string
}{ }{
// GOOD VALUES // GOOD VALUES
{ {
@ -249,77 +334,126 @@ 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",
}, },
} { } {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
@ -335,6 +469,32 @@ 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)
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)
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)
}
}
}) })
} }
} }

View File

@ -1376,6 +1376,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