Merge pull request #128786 from danwinship/bad-ip-warnings

warn on bad IPs in objects
This commit is contained in:
Kubernetes Prow Robot 2025-03-11 00:11:47 -07:00 committed by GitHub
commit 3782b558a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
34 changed files with 1243 additions and 616 deletions

View File

@ -24,6 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
nodeapi "k8s.io/kubernetes/pkg/api/node"
pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
@ -351,6 +352,16 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
}
}
// Deprecated IP address formats
if podSpec.DNSConfig != nil {
for i, ns := range podSpec.DNSConfig.Nameservers {
warnings = append(warnings, validation.GetWarningsForIP(fieldPath.Child("spec", "dnsConfig", "nameservers").Index(i), ns)...)
}
}
for i, hostAlias := range podSpec.HostAliases {
warnings = append(warnings, validation.GetWarningsForIP(fieldPath.Child("spec", "hostAliases").Index(i).Child("ip"), hostAlias.IP)...)
}
return warnings
}

View File

@ -1767,6 +1767,21 @@ func TestWarnings(t *testing.T) {
`spec.affinity.nodeAffinity.preferredDuringSchedulingIgnoredDuringExecution[0].preference.matchExpressions[0].values[0]: -1 is invalid, a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')`,
},
},
{
name: "dubious IP address formats",
template: &api.PodTemplateSpec{Spec: api.PodSpec{
DNSConfig: &api.PodDNSConfig{
Nameservers: []string{"1.2.3.4", "05.06.07.08"},
},
HostAliases: []api.HostAlias{
{IP: "::ffff:1.2.3.4"},
},
}},
expected: []string{
`spec.dnsConfig.nameservers[1]: non-standard IP address "05.06.07.08" will be considered invalid in a future Kubernetes release: use "5.6.7.8"`,
`spec.hostAliases[0].ip: non-standard IP address "::ffff:1.2.3.4" will be considered invalid in a future Kubernetes release: use "1.2.3.4"`,
},
},
}
for _, tc := range testcases {

View File

@ -18,8 +18,8 @@ package service
import (
"fmt"
"net/netip"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/helper"
@ -37,20 +37,20 @@ func GetWarningsForService(service, oldService *api.Service) []string {
if helper.IsServiceIPSet(service) {
for i, clusterIP := range service.Spec.ClusterIPs {
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("clusterIPs").Index(i), clusterIP)...)
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("clusterIPs").Index(i), clusterIP)...)
}
}
for i, externalIP := range service.Spec.ExternalIPs {
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("externalIPs").Index(i), externalIP)...)
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("externalIPs").Index(i), externalIP)...)
}
if len(service.Spec.LoadBalancerIP) > 0 {
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("loadBalancerIP"), service.Spec.LoadBalancerIP)...)
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("loadBalancerIP"), service.Spec.LoadBalancerIP)...)
}
for i, cidr := range service.Spec.LoadBalancerSourceRanges {
warnings = append(warnings, getWarningsForCIDR(field.NewPath("spec").Child("loadBalancerSourceRanges").Index(i), cidr)...)
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(field.NewPath("spec").Child("loadBalancerSourceRanges").Index(i), cidr)...)
}
if service.Spec.Type == api.ServiceTypeExternalName && len(service.Spec.ExternalIPs) > 0 {
@ -62,45 +62,3 @@ func GetWarningsForService(service, oldService *api.Service) []string {
return warnings
}
func getWarningsForIP(fieldPath *field.Path, address string) []string {
// IPv4 addresses with leading zeros CVE-2021-29923 are not valid in golang since 1.17
// This will also warn about possible future changes on the golang std library
// xref: https://issues.k8s.io/108074
ip, err := netip.ParseAddr(address)
if err != nil {
return []string{fmt.Sprintf("%s: IP address was accepted, but will be invalid in a future Kubernetes release: %v", fieldPath, err)}
}
// A Recommendation for IPv6 Address Text Representation
//
// "All of the above examples represent the same IPv6 address. This
// flexibility has caused many problems for operators, systems
// engineers, and customers.
// ..."
// https://datatracker.ietf.org/doc/rfc5952/
if ip.Is6() && ip.String() != address {
return []string{fmt.Sprintf("%s: IPv6 address %q is not in RFC 5952 canonical format (%q), which may cause controller apply-loops", fieldPath, address, ip.String())}
}
return []string{}
}
func getWarningsForCIDR(fieldPath *field.Path, cidr string) []string {
// IPv4 addresses with leading zeros CVE-2021-29923 are not valid in golang since 1.17
// This will also warn about possible future changes on the golang std library
// xref: https://issues.k8s.io/108074
prefix, err := netip.ParsePrefix(cidr)
if err != nil {
return []string{fmt.Sprintf("%s: IP prefix was accepted, but will be invalid in a future Kubernetes release: %v", fieldPath, err)}
}
// A Recommendation for IPv6 Address Text Representation
//
// "All of the above examples represent the same IPv6 address. This
// flexibility has caused many problems for operators, systems
// engineers, and customers.
// ..."
// https://datatracker.ietf.org/doc/rfc5952/
if prefix.Addr().Is6() && prefix.String() != cidr {
return []string{fmt.Sprintf("%s: IPv6 prefix %q is not in RFC 5952 canonical format (%q), which may cause controller apply-loops", fieldPath, cidr, prefix.String())}
}
return []string{}
}

View File

@ -22,7 +22,6 @@ import (
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
api "k8s.io/kubernetes/pkg/apis/core"
)
@ -108,58 +107,58 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
name: "IPv4 with leading zeros",
service: service([]string{"192.012.2.2"}),
want: []string{
`spec.clusterIPs[0]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[0]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
name: "Dual Stack IPv4-IPv6 and IPv4 with leading zeros",
service: service([]string{"192.012.2.2", "2001:db8::2"}),
want: []string{
`spec.clusterIPs[0]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[0]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
name: "Dual Stack IPv6-IPv4 and IPv4 with leading zeros",
service: service([]string{"2001:db8::2", "192.012.2.2"}),
want: []string{
`spec.clusterIPs[1]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[1]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
name: "IPv6 non canonical format",
service: service([]string{"2001:db8:0:0::2"}),
want: []string{
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
},
},
{
name: "Dual Stack IPv4-IPv6 and IPv6 non-canonical format",
service: service([]string{"192.12.2.2", "2001:db8:0:0::2"}),
want: []string{
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
},
},
{
name: "Dual Stack IPv6-IPv4 and IPv6 non-canonical formats",
service: service([]string{"2001:db8:0:0::2", "192.12.2.2"}),
want: []string{
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
},
},
{
name: "Dual Stack IPv4-IPv6 and IPv4 with leading zeros and IPv6 non-canonical format",
service: service([]string{"192.012.2.2", "2001:db8:0:0::2"}),
want: []string{
`spec.clusterIPs[0]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[0]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
},
},
{
name: "Dual Stack IPv6-IPv4 and IPv4 with leading zeros and IPv6 non-canonical format",
service: service([]string{"2001:db8:0:0::2", "192.012.2.2"}),
want: []string{
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[1]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
`spec.clusterIPs[1]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
@ -179,13 +178,13 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
},
},
want: []string{
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
`spec.clusterIPs[1]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
`spec.externalIPs[0]: IPv6 address "2001:db8:1:0::2" is not in RFC 5952 canonical format ("2001:db8:1::2"), which may cause controller apply-loops`,
`spec.externalIPs[1]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("10.012.2.2"): IPv4 field has octet with leading zero`,
`spec.loadBalancerIP: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("10.001.1.1"): IPv4 field has octet with leading zero`,
`spec.loadBalancerSourceRanges[0]: IPv6 prefix "2001:db8:1:0::/64" is not in RFC 5952 canonical format ("2001:db8:1::/64"), which may cause controller apply-loops`,
`spec.loadBalancerSourceRanges[1]: IP prefix was accepted, but will be invalid in a future Kubernetes release: netip.ParsePrefix("10.012.2.0/24"): ParseAddr("10.012.2.0"): IPv4 field has octet with leading zero`,
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
`spec.clusterIPs[1]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
`spec.externalIPs[0]: IPv6 address "2001:db8:1:0::2" should be in RFC 5952 canonical format ("2001:db8:1::2")`,
`spec.externalIPs[1]: non-standard IP address "10.012.2.2" will be considered invalid in a future Kubernetes release: use "10.12.2.2"`,
`spec.loadBalancerIP: non-standard IP address "10.001.1.1" will be considered invalid in a future Kubernetes release: use "10.1.1.1"`,
`spec.loadBalancerSourceRanges[0]: IPv6 CIDR value "2001:db8:1:0::/64" should be in RFC 5952 canonical format ("2001:db8:1::/64")`,
`spec.loadBalancerSourceRanges[1]: non-standard CIDR value "10.012.2.0/24" will be considered invalid in a future Kubernetes release: use "10.12.2.0/24"`,
},
},
}
@ -197,93 +196,3 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
})
}
}
func Test_getWarningsForIP(t *testing.T) {
tests := []struct {
name string
fieldPath *field.Path
address string
want []string
}{
{
name: "IPv4 No failures",
address: "192.12.2.2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: []string{},
},
{
name: "IPv6 No failures",
address: "2001:db8::2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: []string{},
},
{
name: "IPv4 with leading zeros",
address: "192.012.2.2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: []string{
`spec.clusterIPs[0]: IP address was accepted, but will be invalid in a future Kubernetes release: ParseAddr("192.012.2.2"): IPv4 field has octet with leading zero`,
},
},
{
name: "IPv6 non-canonical format",
address: "2001:db8:0:0::2",
fieldPath: field.NewPath("spec").Child("loadBalancerIP"),
want: []string{
`spec.loadBalancerIP: IPv6 address "2001:db8:0:0::2" is not in RFC 5952 canonical format ("2001:db8::2"), which may cause controller apply-loops`,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getWarningsForIP(tt.fieldPath, tt.address); !reflect.DeepEqual(got, tt.want) {
t.Errorf("getWarningsForIP() = %v, want %v", got, tt.want)
}
})
}
}
func Test_getWarningsForCIDR(t *testing.T) {
tests := []struct {
name string
fieldPath *field.Path
cidr string
want []string
}{
{
name: "IPv4 No failures",
cidr: "192.12.2.0/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{},
},
{
name: "IPv6 No failures",
cidr: "2001:db8::/64",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{},
},
{
name: "IPv4 with leading zeros",
cidr: "192.012.2.0/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: IP prefix was accepted, but will be invalid in a future Kubernetes release: netip.ParsePrefix("192.012.2.0/24"): ParseAddr("192.012.2.0"): IPv4 field has octet with leading zero`,
},
},
{
name: "IPv6 non-canonical format",
cidr: "2001:db8:0:0::/64",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: IPv6 prefix "2001:db8:0:0::/64" is not in RFC 5952 canonical format ("2001:db8::/64"), which may cause controller apply-loops`,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getWarningsForCIDR(tt.fieldPath, tt.cidr); !reflect.DeepEqual(got, tt.want) {
t.Errorf("getWarningsForCIDR() = %v, want %v", got, tt.want)
}
})
}
}

View File

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

View File

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

View File

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

View File

@ -67,18 +67,18 @@ const (
// maxCapacity
truncated = "truncated"
// labelManagedBy is a label for recognizing Endpoints managed by this controller.
labelManagedBy = "endpoints.kubernetes.io/managed-by"
// LabelManagedBy is a label for recognizing Endpoints managed by this controller.
LabelManagedBy = "endpoints.kubernetes.io/managed-by"
// controllerName is the name of this controller
controllerName = "endpoint-controller"
// ControllerName is the name of this controller
ControllerName = "endpoint-controller"
)
// NewEndpointController returns a new *Controller.
func NewEndpointController(ctx context.Context, podInformer coreinformers.PodInformer, serviceInformer coreinformers.ServiceInformer,
endpointsInformer coreinformers.EndpointsInformer, client clientset.Interface, endpointUpdatesBatchPeriod time.Duration) *Controller {
broadcaster := record.NewBroadcaster(record.WithContext(ctx))
recorder := broadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: controllerName})
recorder := broadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: ControllerName})
e := &Controller{
client: client,
@ -503,7 +503,7 @@ func (e *Controller) syncService(ctx context.Context, key string) error {
} else {
newEndpoints.Labels = utillabels.CloneAndRemoveLabel(newEndpoints.Labels, v1.IsHeadlessService)
}
newEndpoints.Labels[labelManagedBy] = controllerName
newEndpoints.Labels[LabelManagedBy] = ControllerName
logger.V(4).Info("Update endpoints", "service", klog.KObj(service), "readyEndpoints", totalReadyEps, "notreadyEndpoints", totalNotReadyEps)
var updatedEndpoints *v1.Endpoints
@ -720,16 +720,16 @@ func endpointSubsetsEqualIgnoreResourceVersion(subsets1, subsets2 []v1.EndpointS
// labelsCorrectForEndpoints tests that epLabels is correctly derived from svcLabels
// (ignoring the v1.IsHeadlessService label).
func labelsCorrectForEndpoints(epLabels, svcLabels map[string]string) bool {
if epLabels[labelManagedBy] != controllerName {
if epLabels[LabelManagedBy] != ControllerName {
return false
}
// Every label in epLabels except v1.IsHeadlessService and labelManagedBy should
// Every label in epLabels except v1.IsHeadlessService and LabelManagedBy should
// correspond to a label in svcLabels, and svcLabels should not have any other
// labels that aren't in epLabels.
skipped := 0
for k, v := range epLabels {
if k == v1.IsHeadlessService || k == labelManagedBy {
if k == v1.IsHeadlessService || k == LabelManagedBy {
skipped++
} else if sv, exists := svcLabels[k]; !exists || sv != v {
return false

View File

@ -318,7 +318,7 @@ func TestSyncEndpointsExistingNilSubsets(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
},
},
Subsets: nil,
@ -350,7 +350,7 @@ func TestSyncEndpointsExistingEmptySubsets(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
},
},
Subsets: []v1.EndpointSubset{},
@ -383,7 +383,7 @@ func TestSyncEndpointsWithPodResourceVersionUpdateOnly(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
},
},
Subsets: []v1.EndpointSubset{{
@ -510,7 +510,7 @@ func TestSyncEndpointsProtocolTCP(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -534,7 +534,7 @@ func TestSyncEndpointsHeadlessServiceLabel(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -663,7 +663,7 @@ func TestSyncEndpointsProtocolUDP(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -713,7 +713,7 @@ func TestSyncEndpointsProtocolSCTP(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -759,7 +759,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -806,7 +806,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAllNotReady(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -853,7 +853,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAllMixed(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -878,7 +878,7 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
},
},
Subsets: []v1.EndpointSubset{{
@ -906,7 +906,7 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -930,7 +930,7 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
Name: "foo",
Namespace: ns,
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
},
},
Subsets: []v1.EndpointSubset{{
@ -995,7 +995,7 @@ func TestSyncEndpointsItems(t *testing.T) {
ResourceVersion: "",
Name: "foo",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1046,7 +1046,7 @@ func TestSyncEndpointsItemsWithLabels(t *testing.T) {
}}
serviceLabels[v1.IsHeadlessService] = ""
serviceLabels[labelManagedBy] = controllerName
serviceLabels[LabelManagedBy] = ControllerName
data := runtime.EncodeOrDie(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &v1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
ResourceVersion: "",
@ -1099,7 +1099,7 @@ func TestSyncEndpointsItemsPreexistingLabelsChange(t *testing.T) {
}
serviceLabels[v1.IsHeadlessService] = ""
serviceLabels[labelManagedBy] = controllerName
serviceLabels[LabelManagedBy] = ControllerName
data := runtime.EncodeOrDie(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &v1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
@ -1209,7 +1209,7 @@ func TestSyncEndpointsHeadlessService(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
"a": "b",
v1.IsHeadlessService: "",
},
@ -1239,7 +1239,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyNeverAndPhaseFail
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
"foo": "bar",
},
},
@ -1264,7 +1264,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyNeverAndPhaseFail
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1310,7 +1310,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyNeverAndPhaseSucc
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1357,7 +1357,7 @@ func TestSyncEndpointsItemsExcludeNotReadyPodsWithRestartPolicyOnFailureAndPhase
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1392,7 +1392,7 @@ func TestSyncEndpointsHeadlessWithoutPort(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1612,7 +1612,7 @@ func TestLastTriggerChangeTimeAnnotation(t *testing.T) {
v1.EndpointsLastChangeTriggerTime: triggerTimeString,
},
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1669,7 +1669,7 @@ func TestLastTriggerChangeTimeAnnotation_AnnotationOverridden(t *testing.T) {
v1.EndpointsLastChangeTriggerTime: triggerTimeString,
},
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
},
},
@ -1724,7 +1724,7 @@ func TestLastTriggerChangeTimeAnnotation_AnnotationCleared(t *testing.T) {
Namespace: ns,
ResourceVersion: "1",
Labels: map[string]string{
labelManagedBy: controllerName,
LabelManagedBy: ControllerName,
v1.IsHeadlessService: "",
}, // Annotation not set anymore.
},

View File

@ -72,9 +72,9 @@ const (
// maxSyncBackOff is the max backoff period for syncService calls.
maxSyncBackOff = 1000 * time.Second
// controllerName is a unique value used with LabelManagedBy to indicated
// ControllerName is a unique value used with LabelManagedBy to indicated
// the component managing an EndpointSlice.
controllerName = "endpointslice-controller.k8s.io"
ControllerName = "endpointslice-controller.k8s.io"
// topologyQueueItemKey is the key for all items in the topologyQueue.
topologyQueueItemKey = "topologyQueueItemKey"
@ -185,7 +185,7 @@ func NewController(ctx context.Context, podInformer coreinformers.PodInformer,
c.endpointSliceTracker,
c.topologyCache,
c.eventRecorder,
controllerName,
ControllerName,
endpointslicerec.WithTrafficDistributionEnabled(utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution)),
)

View File

@ -397,7 +397,7 @@ func TestSyncServiceEndpointSlicePendingDeletion(t *testing.T) {
OwnerReferences: []metav1.OwnerReference{*ownerRef},
Labels: map[string]string{
discovery.LabelServiceName: serviceName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
DeletionTimestamp: &deletedTs,
},
@ -442,7 +442,7 @@ func TestSyncServiceEndpointSliceLabelSelection(t *testing.T) {
OwnerReferences: []metav1.OwnerReference{*ownerRef},
Labels: map[string]string{
discovery.LabelServiceName: serviceName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
AddressType: discovery.AddressTypeIPv4,
@ -453,7 +453,7 @@ func TestSyncServiceEndpointSliceLabelSelection(t *testing.T) {
OwnerReferences: []metav1.OwnerReference{*ownerRef},
Labels: map[string]string{
discovery.LabelServiceName: serviceName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
AddressType: discovery.AddressTypeIPv4,
@ -472,7 +472,7 @@ func TestSyncServiceEndpointSliceLabelSelection(t *testing.T) {
Namespace: ns,
Labels: map[string]string{
discovery.LabelServiceName: "something-else",
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
AddressType: discovery.AddressTypeIPv4,
@ -529,7 +529,7 @@ func TestOnEndpointSliceUpdate(t *testing.T) {
Namespace: ns,
Labels: map[string]string{
discovery.LabelServiceName: serviceName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
AddressType: discovery.AddressTypeIPv4,
@ -1750,7 +1750,7 @@ func TestSyncServiceStaleInformer(t *testing.T) {
Generation: testcase.informerGenerationNumber,
Labels: map[string]string{
discovery.LabelServiceName: serviceName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
AddressType: discovery.AddressTypeIPv4,
@ -1977,7 +1977,7 @@ func TestUpdateNode(t *testing.T) {
Namespace: "ns",
Labels: map[string]string{
discovery.LabelServiceName: "svc",
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
Endpoints: []discovery.Endpoint{

View File

@ -62,9 +62,9 @@ const (
// maxSyncBackOff is the max backoff period for syncEndpoints calls.
maxSyncBackOff = 100 * time.Second
// controllerName is a unique value used with LabelManagedBy to indicated
// ControllerName is a unique value used with LabelManagedBy to indicated
// the component managing an EndpointSlice.
controllerName = "endpointslicemirroring-controller.k8s.io"
ControllerName = "endpointslicemirroring-controller.k8s.io"
)
// NewController creates and initializes a new Controller
@ -537,7 +537,7 @@ func (c *Controller) deleteMirroredSlices(namespace, name string) error {
func endpointSlicesMirroredForService(endpointSliceLister discoverylisters.EndpointSliceLister, namespace, name string) ([]*discovery.EndpointSlice, error) {
esLabelSelector := labels.Set(map[string]string{
discovery.LabelServiceName: name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
}).AsSelectorPreValidated()
return endpointSliceLister.EndpointSlices(namespace).List(esLabelSelector)
}

View File

@ -172,7 +172,7 @@ func TestSyncEndpoints(t *testing.T) {
Name: endpointsName + "-1",
Labels: map[string]string{
discovery.LabelServiceName: endpointsName,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
}},
@ -358,7 +358,7 @@ func TestEndpointSlicesMirroredForService(t *testing.T) {
Namespace: "ns1",
Labels: map[string]string{
discovery.LabelServiceName: "svc1",
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
},
@ -373,7 +373,7 @@ func TestEndpointSlicesMirroredForService(t *testing.T) {
Namespace: "ns2",
Labels: map[string]string{
discovery.LabelServiceName: "svc1",
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
},
@ -388,7 +388,7 @@ func TestEndpointSlicesMirroredForService(t *testing.T) {
Namespace: "ns1",
Labels: map[string]string{
discovery.LabelServiceName: "svc2",
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
},
@ -403,7 +403,7 @@ func TestEndpointSlicesMirroredForService(t *testing.T) {
Namespace: "ns1",
Labels: map[string]string{
discovery.LabelServiceName: "svc1",
discovery.LabelManagedBy: controllerName + "foo",
discovery.LabelManagedBy: ControllerName + "foo",
},
},
},
@ -431,7 +431,7 @@ func TestEndpointSlicesMirroredForService(t *testing.T) {
Name: "example-1",
Namespace: "ns1",
Labels: map[string]string{
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
},
},

View File

@ -1078,7 +1078,7 @@ func TestReconcile(t *testing.T) {
for _, epSlice := range tc.existingEndpointSlices {
epSlice.Labels = map[string]string{
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
}
_, err := client.DiscoveryV1().EndpointSlices(namespace).Create(context.TODO(), epSlice, metav1.CreateOptions{})
if err != nil {
@ -1305,7 +1305,7 @@ func expectMatchingAddresses(t *testing.T, epSubset corev1.EndpointSubset, esEnd
func fetchEndpointSlices(t *testing.T, client *fake.Clientset, namespace string) []discovery.EndpointSlice {
t.Helper()
fetchedSlices, err := client.DiscoveryV1().EndpointSlices(namespace).List(context.TODO(), metav1.ListOptions{
LabelSelector: discovery.LabelManagedBy + "=" + controllerName,
LabelSelector: discovery.LabelManagedBy + "=" + ControllerName,
})
if err != nil {
t.Fatalf("Expected no error fetching Endpoint Slices, got: %v", err)

View File

@ -73,7 +73,7 @@ func newEndpointSlice(endpoints *corev1.Endpoints, ports []discovery.EndpointPor
// overwrite specific labels
epSlice.Labels[discovery.LabelServiceName] = endpoints.Name
epSlice.Labels[discovery.LabelManagedBy] = controllerName
epSlice.Labels[discovery.LabelManagedBy] = ControllerName
// clone all annotations but EndpointsLastChangeTriggerTime and LastAppliedConfigAnnotation
for annotation, val := range endpoints.Annotations {
@ -267,5 +267,5 @@ func managedByChanged(endpointSlice1, endpointSlice2 *discovery.EndpointSlice) b
// EndpointSlices is the EndpointSlice controller.
func managedByController(endpointSlice *discovery.EndpointSlice) bool {
managedBy, _ := endpointSlice.Labels[discovery.LabelManagedBy]
return managedBy == controllerName
return managedBy == ControllerName
}

View File

@ -64,7 +64,7 @@ func TestNewEndpointSlice(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
Annotations: map[string]string{},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
@ -86,7 +86,7 @@ func TestNewEndpointSlice(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
Annotations: map[string]string{"foo": "bar"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
@ -109,7 +109,7 @@ func TestNewEndpointSlice(t *testing.T) {
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
Annotations: map[string]string{},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
@ -134,7 +134,7 @@ func TestNewEndpointSlice(t *testing.T) {
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
Annotations: map[string]string{"foo2": "bar2"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
@ -162,7 +162,7 @@ func TestNewEndpointSlice(t *testing.T) {
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
discovery.LabelManagedBy: ControllerName,
},
Annotations: map[string]string{"foo2": "bar2"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),

View File

@ -20,11 +20,13 @@ import (
"context"
"k8s.io/apimachinery/pkg/runtime"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api/legacyscheme"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/validation"
endpointscontroller "k8s.io/kubernetes/pkg/controller/endpoint"
)
// endpointsStrategy implements behavior for Endpoints
@ -57,7 +59,7 @@ func (endpointsStrategy) Validate(ctx context.Context, obj runtime.Object) field
// WarningsOnCreate returns warnings for the creation of the given object.
func (endpointsStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
return nil
return endpointsWarnings(obj.(*api.Endpoints))
}
// Canonicalize normalizes the object after validation.
@ -76,9 +78,33 @@ func (endpointsStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Ob
// WarningsOnUpdate returns warnings for the given update.
func (endpointsStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
return endpointsWarnings(obj.(*api.Endpoints))
}
func (endpointsStrategy) AllowUnconditionalUpdate() bool {
return true
}
func endpointsWarnings(endpoints *api.Endpoints) []string {
// Save time by not checking for bad IPs if the request is coming from the
// Endpoints controller, since we know it fixes up any invalid IPs from its input
// data when outputting the Endpoints. (The "managed-by" label is new, so this
// heuristic may fail in skewed clusters, but that just means we won't get the
// optimization during the skew.)
if endpoints.Labels[endpointscontroller.LabelManagedBy] == endpointscontroller.ControllerName {
return nil
}
var warnings []string
for i := range endpoints.Subsets {
for j := range endpoints.Subsets[i].Addresses {
fldPath := field.NewPath("subsets").Index(i).Child("addresses").Index(j).Child("ip")
warnings = append(warnings, utilvalidation.GetWarningsForIP(fldPath, endpoints.Subsets[i].Addresses[j].IP)...)
}
for j := range endpoints.Subsets[i].NotReadyAddresses {
fldPath := field.NewPath("subsets").Index(i).Child("notReadyAddresses").Index(j).Child("ip")
warnings = append(warnings, utilvalidation.GetWarningsForIP(fldPath, endpoints.Subsets[i].NotReadyAddresses[j].IP)...)
}
}
return warnings
}

View File

@ -0,0 +1,124 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package endpoint
import (
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
api "k8s.io/kubernetes/pkg/apis/core"
)
func Test_endpointsWarning(t *testing.T) {
tests := []struct {
name string
endpoints *api.Endpoints
warnings []string
}{
{
name: "empty Endpoints",
endpoints: &api.Endpoints{},
warnings: nil,
},
{
name: "valid Endpoints",
endpoints: &api.Endpoints{
Subsets: []api.EndpointSubset{
{
Addresses: []api.EndpointAddress{
{IP: "1.2.3.4"},
{IP: "fd00::1234"},
},
},
},
},
warnings: nil,
},
{
name: "bad Endpoints",
endpoints: &api.Endpoints{
Subsets: []api.EndpointSubset{
{
Addresses: []api.EndpointAddress{
{IP: "fd00::1234"},
{IP: "01.02.03.04"},
},
},
{
Addresses: []api.EndpointAddress{
{IP: "::ffff:1.2.3.4"},
},
},
{
Addresses: []api.EndpointAddress{
{IP: "1.2.3.4"},
},
NotReadyAddresses: []api.EndpointAddress{
{IP: "::ffff:1.2.3.4"},
},
},
},
},
warnings: []string{
"subsets[0].addresses[1].ip",
"subsets[1].addresses[0].ip",
"subsets[2].notReadyAddresses[0].ip",
},
},
{
// We don't actually want to let bad IPs through in this case; the
// point here is that we trust the Endpoints controller to not do
// that, and we're testing that the checks correctly get skipped.
name: "bad Endpoints ignored because of label",
endpoints: &api.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"endpoints.kubernetes.io/managed-by": "endpoint-controller",
},
},
Subsets: []api.EndpointSubset{
{
Addresses: []api.EndpointAddress{
{IP: "fd00::1234"},
{IP: "01.02.03.04"},
},
},
},
},
warnings: nil,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
warnings := endpointsWarnings(test.endpoints)
ok := len(warnings) == len(test.warnings)
if ok {
for i := range warnings {
if !strings.HasPrefix(warnings[i], test.warnings[i]) {
ok = false
break
}
}
}
if !ok {
t.Errorf("Expected warnings for %v, got %v", test.warnings, warnings)
}
})
}
}

View File

@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
utilnet "k8s.io/apimachinery/pkg/util/net"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/registry/generic"
pkgstorage "k8s.io/apiserver/pkg/storage"
@ -131,7 +132,7 @@ func (nodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.Erro
// WarningsOnCreate returns warnings for the creation of the given object.
func (nodeStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
return fieldIsDeprecatedWarnings(obj)
return nodeWarnings(obj)
}
// Canonicalize normalizes the object after validation.
@ -146,7 +147,7 @@ func (nodeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object)
// WarningsOnUpdate returns warnings for the given update.
func (nodeStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return fieldIsDeprecatedWarnings(obj)
return nodeWarnings(obj)
}
func (nodeStrategy) AllowUnconditionalUpdate() bool {
@ -287,7 +288,7 @@ func isProxyableHostname(ctx context.Context, hostname string) error {
return nil
}
func fieldIsDeprecatedWarnings(obj runtime.Object) []string {
func nodeWarnings(obj runtime.Object) []string {
newNode := obj.(*api.Node)
var warnings []string
if newNode.Spec.ConfigSource != nil {
@ -297,6 +298,14 @@ func fieldIsDeprecatedWarnings(obj runtime.Object) []string {
if len(newNode.Spec.DoNotUseExternalID) > 0 {
warnings = append(warnings, "spec.externalID: this field is deprecated, and is unused by Kubernetes")
}
if len(newNode.Spec.PodCIDRs) > 0 {
podCIDRsField := field.NewPath("spec", "podCIDRs")
for i, value := range newNode.Spec.PodCIDRs {
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(podCIDRsField.Index(i), value)...)
}
}
return warnings
}

View File

@ -287,25 +287,65 @@ func TestWarningOnUpdateAndCreate(t *testing.T) {
node api.Node
warningText string
}{
{api.Node{},
{
api.Node{},
""},
{api.Node{},
api.Node{},
"",
},
{
api.Node{},
//nolint:staticcheck // ignore deprecation warning
api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
"spec.configSource"},
{api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
"spec.configSource",
},
{
//nolint:staticcheck // ignore deprecation warning
api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
"spec.configSource"},
{api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
api.Node{}, ""},
{api.Node{},
//nolint:staticcheck // ignore deprecation warning
api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
"spec.configSource",
},
{
//nolint:staticcheck // ignore deprecation warning
api.Node{Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{}}},
api.Node{},
"",
},
{
api.Node{},
api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
"spec.externalID"},
{api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
"spec.externalID",
},
{
api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
"spec.externalID"},
{api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
api.Node{}, ""},
api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
"spec.externalID",
},
{
api.Node{Spec: api.NodeSpec{DoNotUseExternalID: "externalID"}},
api.Node{},
"",
},
{
api.Node{},
api.Node{Spec: api.NodeSpec{PodCIDRs: []string{"10.0.1.0/24", "fd00::/64"}}},
"",
},
{
api.Node{},
api.Node{Spec: api.NodeSpec{PodCIDRs: []string{"010.000.001.000/24", "fd00::/64"}}},
"spec.podCIDRs[0]",
},
{
api.Node{},
api.Node{Spec: api.NodeSpec{PodCIDRs: []string{"10.0.1.0/24", "fd00::1234/64"}}},
"spec.podCIDRs[1]",
},
{
api.Node{Spec: api.NodeSpec{PodCIDRs: []string{"010.000.001.000/24", "fd00::1234/64"}}},
api.Node{Spec: api.NodeSpec{PodCIDRs: []string{"10.0.1.0/24", "fd00::/64"}}},
"",
},
}
for i, test := range tests {
warnings := (nodeStrategy{}).WarningsOnUpdate(context.Background(), &test.node, &test.oldNode)

View File

@ -270,7 +270,17 @@ func (podStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Ob
// WarningsOnUpdate returns warnings for the given update.
func (podStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
pod := obj.(*api.Pod)
var warnings []string
for i, podIP := range pod.Status.PodIPs {
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("status", "podIPs").Index(i).Child("ip"), podIP.IP)...)
}
for i, hostIP := range pod.Status.HostIPs {
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("status", "hostIPs").Index(i).Child("ip"), hostIP.IP)...)
}
return warnings
}
type podEphemeralContainersStrategy struct {

View File

@ -3562,3 +3562,68 @@ func TestStatusPrepareForUpdate(t *testing.T) {
})
}
}
func TestWarningsOnUpdate(t *testing.T) {
tests := []struct {
name string
pod *api.Pod
warnings []string
}{
{
name: "no podIPs/hostIPs",
pod: &api.Pod{Status: api.PodStatus{}},
warnings: nil,
},
{
name: "valid podIPs/hostIPs",
pod: &api.Pod{
Status: api.PodStatus{
PodIPs: []api.PodIP{
{IP: "1.2.3.4"},
},
HostIPs: []api.HostIP{
{IP: "5.6.7.8"},
{IP: "fd00::5678"},
},
},
},
warnings: nil,
},
{
name: "bad podIPs/hostIPs",
pod: &api.Pod{
Status: api.PodStatus{
PodIPs: []api.PodIP{
{IP: "01.02.03.04"},
},
HostIPs: []api.HostIP{
{IP: "5.6.7.8"},
{IP: "::ffff:9.10.11.12"},
},
},
},
warnings: []string{
"status.podIPs[0].ip",
"status.hostIPs[1].ip",
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
warnings := StatusStrategy.WarningsOnUpdate(context.Background(), test.pod, test.pod)
ok := len(warnings) == len(test.warnings)
if ok {
for i := range warnings {
if !strings.HasPrefix(warnings[i], test.warnings[i]) {
ok = false
break
}
}
}
if !ok {
t.Errorf("Expected warnings for %v, got %v", test.warnings, warnings)
}
})
}
}

View File

@ -25,6 +25,7 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/registry/generic"
pkgstorage "k8s.io/apiserver/pkg/storage"
@ -168,7 +169,19 @@ func (serviceStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtim
// WarningsOnUpdate returns warnings for the given update.
func (serviceStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
svc := obj.(*api.Service)
var warnings []string
if len(svc.Status.LoadBalancer.Ingress) > 0 {
fieldPath := field.NewPath("status", "loadBalancer", "ingress")
for i, ingress := range svc.Status.LoadBalancer.Ingress {
if len(ingress.IP) > 0 {
warnings = append(warnings, utilvalidation.GetWarningsForIP(fieldPath.Index(i), ingress.IP)...)
}
}
}
return warnings
}
// GetAttrs returns labels and fields of a given object for filtering purposes.

View File

@ -138,6 +138,18 @@ func TestServiceStatusStrategy(t *testing.T) {
if len(errs) != 0 {
t.Errorf("Unexpected error %v", errs)
}
warnings := StatusStrategy.WarningsOnUpdate(ctx, newService, oldService)
if len(warnings) != 0 {
t.Errorf("Unexpected warnings %v", errs)
}
// Bad IP warning (leading zeros)
newService.Status.LoadBalancer.Ingress[0].IP = "127.000.000.002"
warnings = StatusStrategy.WarningsOnUpdate(ctx, newService, oldService)
if len(warnings) != 1 {
t.Errorf("Did not get warning for bad IP")
}
}
func makeServiceWithConditions(conditions []metav1.Condition) *api.Service {

View File

@ -21,12 +21,14 @@ import (
"fmt"
corev1 "k8s.io/api/core/v1"
discoveryv1 "k8s.io/api/discovery/v1"
discoveryv1beta1 "k8s.io/api/discovery/v1beta1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/storage/names"
@ -35,6 +37,8 @@ import (
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/discovery"
"k8s.io/kubernetes/pkg/apis/discovery/validation"
endpointslicecontroller "k8s.io/kubernetes/pkg/controller/endpointslice"
endpointslicemirroringcontroller "k8s.io/kubernetes/pkg/controller/endpointslicemirroring"
"k8s.io/kubernetes/pkg/features"
)
@ -99,6 +103,7 @@ func (endpointSliceStrategy) WarningsOnCreate(ctx context.Context, obj runtime.O
}
var warnings []string
warnings = append(warnings, warnOnDeprecatedAddressType(eps.AddressType)...)
warnings = append(warnings, warnOnBadIPs(eps)...)
return warnings
}
@ -120,7 +125,13 @@ func (endpointSliceStrategy) ValidateUpdate(ctx context.Context, new, old runtim
// WarningsOnUpdate returns warnings for the given update.
func (endpointSliceStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
eps := obj.(*discovery.EndpointSlice)
if eps == nil {
return nil
}
var warnings []string
warnings = append(warnings, warnOnBadIPs(eps)...)
return warnings
}
// AllowUnconditionalUpdate is the default update policy for EndpointSlice objects.
@ -222,3 +233,23 @@ func warnOnDeprecatedAddressType(addressType discovery.AddressType) []string {
}
return nil
}
// warnOnBadIPs returns warnings for bad IP address formats
func warnOnBadIPs(eps *discovery.EndpointSlice) []string {
// Save time by not checking for bad IPs if the request is coming from one of our
// controllers, since we know they fix up any invalid IPs from their input data
// when outputting the EndpointSlices.
if eps.Labels[discoveryv1.LabelManagedBy] == endpointslicecontroller.ControllerName ||
eps.Labels[discoveryv1.LabelManagedBy] == endpointslicemirroringcontroller.ControllerName {
return nil
}
var warnings []string
for i := range eps.Endpoints {
for j, addr := range eps.Endpoints[i].Addresses {
fldPath := field.NewPath("endpoints").Index(i).Child("addresses").Index(j)
warnings = append(warnings, utilvalidation.GetWarningsForIP(fldPath, addr)...)
}
}
return warnings
}

View File

@ -18,6 +18,7 @@ package endpointslice
import (
"context"
"strings"
"testing"
corev1 "k8s.io/api/core/v1"
@ -1014,3 +1015,86 @@ func TestWarningsOnEndpointSliceAddressType(t *testing.T) {
})
}
}
func Test_warnOnBadIPs(t *testing.T) {
tests := []struct {
name string
slice *discovery.EndpointSlice
warnings []string
}{
{
name: "empty EndpointSlice",
slice: &discovery.EndpointSlice{},
warnings: nil,
},
{
name: "valid EndpointSlice",
slice: &discovery.EndpointSlice{
Endpoints: []discovery.Endpoint{
{
Addresses: []string{
"1.2.3.4",
"fd00::1234",
},
},
},
},
warnings: nil,
},
{
name: "bad EndpointSlice",
slice: &discovery.EndpointSlice{
Endpoints: []discovery.Endpoint{
{
Addresses: []string{
"fd00::1234",
"01.02.03.04",
},
},
{
Addresses: []string{
"::ffff:1.2.3.4",
},
},
},
},
warnings: []string{
"endpoints[0].addresses[1]",
"endpoints[1].addresses[0]",
},
},
{
name: "bad EndpointSlice ignored because of label",
slice: &discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"endpointslice.kubernetes.io/managed-by": "endpointslice-controller.k8s.io",
},
},
Endpoints: []discovery.Endpoint{
{
Addresses: []string{
"fd00::1234",
"01.02.03.04",
},
},
},
},
warnings: nil,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
warnings := warnOnBadIPs(test.slice)
if len(warnings) != len(test.warnings) {
t.Fatalf("Expected warnings %v, got %v", test.warnings, warnings)
}
for i := range warnings {
if !strings.HasPrefix(warnings[i], test.warnings[i]) {
t.Fatalf("Expected warnings %v, got %v", test.warnings, warnings)
}
}
})
}
}

View File

@ -22,6 +22,7 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/runtime"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api/legacyscheme"
@ -172,5 +173,17 @@ func (ingressStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtim
// WarningsOnUpdate returns warnings for the given update.
func (ingressStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
newIngress := obj.(*networking.Ingress)
var warnings []string
if len(newIngress.Status.LoadBalancer.Ingress) > 0 {
fieldPath := field.NewPath("status", "loadBalancer", "ingress")
for i, ingress := range newIngress.Status.LoadBalancer.Ingress {
if len(ingress.IP) > 0 {
warnings = append(warnings, utilvalidation.GetWarningsForIP(fieldPath.Index(i), ingress.IP)...)
}
}
}
return warnings
}

View File

@ -137,4 +137,15 @@ func TestIngressStatusStrategy(t *testing.T) {
if len(errs) != 0 {
t.Errorf("Unexpected error %v", errs)
}
warnings := StatusStrategy.WarningsOnUpdate(ctx, &newIngress, &oldIngress)
if len(warnings) != 0 {
t.Errorf("Unexpected warnings %v", errs)
}
newIngress.Status.LoadBalancer.Ingress[0].IP = "127.000.000.002"
warnings = StatusStrategy.WarningsOnUpdate(ctx, &newIngress, &oldIngress)
if len(warnings) != 1 {
t.Errorf("Did not get warning for bad IP")
}
}

View File

@ -21,6 +21,7 @@ import (
"reflect"
"k8s.io/apimachinery/pkg/runtime"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api/legacyscheme"
@ -70,7 +71,7 @@ func (networkPolicyStrategy) Validate(ctx context.Context, obj runtime.Object) f
// WarningsOnCreate returns warnings for the creation of the given object.
func (networkPolicyStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
return nil
return networkPolicyWarnings(obj.(*networking.NetworkPolicy))
}
// Canonicalize normalizes the object after validation.
@ -91,10 +92,41 @@ func (networkPolicyStrategy) ValidateUpdate(ctx context.Context, obj, old runtim
// WarningsOnUpdate returns warnings for the given update.
func (networkPolicyStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
return networkPolicyWarnings(obj.(*networking.NetworkPolicy))
}
// AllowUnconditionalUpdate is the default update policy for NetworkPolicy objects.
func (networkPolicyStrategy) AllowUnconditionalUpdate() bool {
return true
}
func networkPolicyWarnings(networkPolicy *networking.NetworkPolicy) []string {
var warnings []string
for i := range networkPolicy.Spec.Ingress {
for j := range networkPolicy.Spec.Ingress[i].From {
ipBlock := networkPolicy.Spec.Ingress[i].From[j].IPBlock
if ipBlock == nil {
continue
}
fldPath := field.NewPath("spec").Child("ingress").Index(i).Child("from").Index(j).Child("ipBlock")
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(fldPath.Child("cidr"), ipBlock.CIDR)...)
for k, except := range ipBlock.Except {
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(fldPath.Child("except").Index(k), except)...)
}
}
}
for i := range networkPolicy.Spec.Egress {
for j := range networkPolicy.Spec.Egress[i].To {
ipBlock := networkPolicy.Spec.Egress[i].To[j].IPBlock
if ipBlock == nil {
continue
}
fldPath := field.NewPath("spec").Child("egress").Index(i).Child("to").Index(j).Child("ipBlock")
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(fldPath.Child("cidr"), ipBlock.CIDR)...)
for k, except := range ipBlock.Except {
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(fldPath.Child("except").Index(k), except)...)
}
}
}
return warnings
}

View File

@ -18,6 +18,7 @@ package networkpolicy
import (
"context"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -110,4 +111,43 @@ func TestNetworkPolicyStrategy(t *testing.T) {
t.Errorf("Unexpected error from validation for updated Network Policy: %v", errs)
}
warnings := Strategy.WarningsOnUpdate(context.Background(), updatedNetPol, netPol)
if len(warnings) != 0 {
t.Errorf("Unexpected warnings for Network Policy with no IPBlock: %v", warnings)
}
updatedNetPol.Spec.Egress = append(updatedNetPol.Spec.Egress,
networking.NetworkPolicyEgressRule{
To: []networking.NetworkPolicyPeer{
{
IPBlock: &networking.IPBlock{
CIDR: "10.0.0.0/8",
},
},
},
},
)
warnings = Strategy.WarningsOnUpdate(context.Background(), updatedNetPol, netPol)
if len(warnings) != 0 {
t.Errorf("Unexpected warnings for Network Policy with valid IPBlock: %v", warnings)
}
updatedNetPol.Spec.Egress = append(updatedNetPol.Spec.Egress,
networking.NetworkPolicyEgressRule{
To: []networking.NetworkPolicyPeer{
{
IPBlock: &networking.IPBlock{
CIDR: "::ffff:10.0.0.0/104",
Except: []string{
"10.0.0.1/16",
},
},
},
},
},
)
warnings = Strategy.WarningsOnUpdate(context.Background(), updatedNetPol, netPol)
if len(warnings) != 2 || !strings.HasPrefix(warnings[0], "spec.egress[2].to[0].ipBlock.cidr:") || !strings.HasPrefix(warnings[1], "spec.egress[2].to[0].ipBlock.except[0]:") {
t.Errorf("Incorrect warnings for Network Policy with invalid IPBlock: %v", warnings)
}
}

View File

@ -0,0 +1,139 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validation
import (
"fmt"
"net/netip"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
netutils "k8s.io/utils/net"
)
// IsValidIP tests that the argument is a valid IP address.
func IsValidIP(fldPath *field.Path, value string) 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"))
}
return allErrors
}
// GetWarningsForIP returns warnings for IP address values in non-standard forms. This
// should only be used with fields that are validated with IsValidIP().
func GetWarningsForIP(fldPath *field.Path, value string) []string {
ip := netutils.ParseIPSloppy(value)
if ip == nil {
klog.ErrorS(nil, "GetWarningsForIP called on value that was not validated with IsValidIP", "field", fldPath, "value", value)
return nil
}
addr, _ := netip.ParseAddr(value)
if !addr.IsValid() || addr.Is4In6() {
// This catches 2 cases: leading 0s (if ParseIPSloppy() accepted it but
// ParseAddr() doesn't) or IPv4-mapped IPv6 (.Is4In6()). Either way,
// re-stringifying the net.IP value will give the preferred form.
return []string{
fmt.Sprintf("%s: non-standard IP address %q will be considered invalid in a future Kubernetes release: use %q", fldPath, value, ip.String()),
}
}
// If ParseIPSloppy() and ParseAddr() both accept it then it's fully valid, though
// it may be non-canonical.
if addr.Is6() && addr.String() != value {
return []string{
fmt.Sprintf("%s: IPv6 address %q should be in RFC 5952 canonical format (%q)", fldPath, value, addr.String()),
}
}
return nil
}
// IsValidIPv4Address tests that the argument is a valid IPv4 address.
func IsValidIPv4Address(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 IPv4 address"))
}
return allErrors
}
// IsValidIPv6Address tests that the argument is a valid IPv6 address.
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 {
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 allErrors
}
// GetWarningsForCIDR returns warnings for CIDR values in non-standard forms. This should
// only be used with fields that are validated with IsValidCIDR().
func GetWarningsForCIDR(fldPath *field.Path, value string) []string {
ip, ipnet, err := netutils.ParseCIDRSloppy(value)
if err != nil {
klog.ErrorS(err, "GetWarningsForCIDR called on value that was not validated with IsValidCIDR", "field", fldPath, "value", value)
return nil
}
var warnings []string
// Check for bits set after prefix length
if !ip.Equal(ipnet.IP) {
_, addrlen := ipnet.Mask.Size()
singleIPCIDR := fmt.Sprintf("%s/%d", ip.String(), addrlen)
warnings = append(warnings,
fmt.Sprintf("%s: CIDR value %q is ambiguous in this context (should be %q or %q?)", fldPath, value, ipnet.String(), singleIPCIDR),
)
}
prefix, _ := netip.ParsePrefix(value)
addr := prefix.Addr()
if !prefix.IsValid() || addr.Is4In6() {
// This catches 2 cases: leading 0s (if ParseCIDRSloppy() accepted it but
// ParsePrefix() doesn't) or IPv4-mapped IPv6 (.Is4In6()). Either way,
// re-stringifying the net.IPNet value will give the preferred form.
warnings = append(warnings,
fmt.Sprintf("%s: non-standard CIDR value %q will be considered invalid in a future Kubernetes release: use %q", fldPath, value, ipnet.String()),
)
}
// If ParseCIDRSloppy() and ParsePrefix() both accept it then it's fully valid,
// though it may be non-canonical. But only check this if there are no other
// warnings, since either of the other warnings would also cause a round-trip
// failure.
if len(warnings) == 0 && addr.Is6() && prefix.String() != value {
warnings = append(warnings,
fmt.Sprintf("%s: IPv6 CIDR value %q should be in RFC 5952 canonical format (%q)", fldPath, value, prefix.String()),
)
}
return warnings
}

View File

@ -0,0 +1,452 @@
/*
Copyright 2023 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validation
import (
"reflect"
"strings"
"testing"
"k8s.io/apimachinery/pkg/util/validation/field"
)
func TestIsValidIP(t *testing.T) {
for _, tc := range []struct {
name string
in string
family int
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.2.3.4",
family: 4,
},
{
name: "ipv4, all zeros",
in: "0.0.0.0",
family: 4,
},
{
name: "ipv4, max",
in: "255.255.255.255",
family: 4,
},
{
name: "ipv6",
in: "1234::abcd",
family: 6,
},
{
name: "ipv6, all zeros, collapsed",
in: "::",
family: 6,
},
{
name: "ipv6, max",
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
family: 6,
},
// GOOD, THOUGH NON-CANONICAL, VALUES
{
name: "ipv6, all zeros, expanded (non-canonical)",
in: "0:0:0:0:0:0:0:0",
family: 6,
},
{
name: "ipv6, leading 0s (non-canonical)",
in: "0001:002:03:4::",
family: 6,
},
{
name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD",
family: 6,
},
// BAD VALUES WE CURRENTLY CONSIDER GOOD
{
name: "ipv4 with leading 0s",
in: "1.1.1.01",
family: 4,
},
{
name: "ipv4-in-ipv6 value",
in: "::ffff:1.1.1.1",
family: 4,
},
// BAD VALUES
{
name: "empty string",
in: "",
err: "must be a valid IP address",
},
{
name: "junk",
in: "aaaaaaa",
err: "must be a valid IP address",
},
{
name: "domain name",
in: "myhost.mydomain",
err: "must be a valid IP address",
},
{
name: "cidr",
in: "1.2.3.0/24",
err: "must be a valid IP address",
},
{
name: "ipv4 with out-of-range octets",
in: "1.2.3.400",
err: "must be a valid IP address",
},
{
name: "ipv4 with negative octets",
in: "-1.0.0.0",
err: "must be a valid IP address",
},
{
name: "ipv6 with out-of-range segment",
in: "2001:db8::10005",
err: "must be a valid IP address",
},
{
name: "ipv4:port",
in: "1.2.3.4:80",
err: "must be a valid IP address",
},
{
name: "ipv6 with brackets",
in: "[2001:db8::1]",
err: "must be a valid IP address",
},
{
name: "[ipv6]:port",
in: "[2001:db8::1]:80",
err: "must be a valid IP address",
},
{
name: "host:port",
in: "example.com:80",
err: "must be a valid IP address",
},
{
name: "ipv6 with zone",
in: "1234::abcd%eth0",
err: "must be a valid IP address",
},
{
name: "ipv4 with zone",
in: "169.254.0.0%eth0",
err: "must be a valid IP address",
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := IsValidIP(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)
}
}
errs = IsValidIPv4Address(field.NewPath(""), tc.in)
if tc.family == 4 {
if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs)
}
} else {
if len(errs) == 0 {
t.Errorf("expected %q to fail IsValidIPv4Address", tc.in)
}
}
errs = IsValidIPv6Address(field.NewPath(""), tc.in)
if tc.family == 6 {
if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv6Address but got: %v", tc.in, errs)
}
} else {
if len(errs) == 0 {
t.Errorf("expected %q to fail IsValidIPv6Address", tc.in)
}
}
})
}
}
func TestGetWarningsForIP(t *testing.T) {
tests := []struct {
name string
fieldPath *field.Path
address string
want []string
}{
{
name: "IPv4 No failures",
address: "192.12.2.2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: nil,
},
{
name: "IPv6 No failures",
address: "2001:db8::2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: nil,
},
{
name: "IPv4 with leading zeros",
address: "192.012.2.2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: []string{
`spec.clusterIPs[0]: non-standard IP address "192.012.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
name: "IPv4-mapped IPv6",
address: "::ffff:192.12.2.2",
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
want: []string{
`spec.clusterIPs[0]: non-standard IP address "::ffff:192.12.2.2" will be considered invalid in a future Kubernetes release: use "192.12.2.2"`,
},
},
{
name: "IPv6 non-canonical format",
address: "2001:db8:0:0::2",
fieldPath: field.NewPath("spec").Child("loadBalancerIP"),
want: []string{
`spec.loadBalancerIP: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := GetWarningsForIP(tt.fieldPath, tt.address); !reflect.DeepEqual(got, tt.want) {
t.Errorf("getWarningsForIP() = %v, want %v", got, tt.want)
}
})
}
}
func TestIsValidCIDR(t *testing.T) {
for _, tc := range []struct {
name string
in string
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.0.0.0/8",
},
{
name: "ipv4, all IPs",
in: "0.0.0.0/0",
},
{
name: "ipv4, single IP",
in: "1.1.1.1/32",
},
{
name: "ipv6",
in: "2001:4860:4860::/48",
},
{
name: "ipv6, all IPs",
in: "::/0",
},
{
name: "ipv6, single IP",
in: "::1/128",
},
// GOOD, THOUGH NON-CANONICAL, VALUES
{
name: "ipv6, extra 0s (non-canonical)",
in: "2a00:79e0:2:0::/64",
},
{
name: "ipv6, capital letters (non-canonical)",
in: "2001:DB8::/64",
},
// BAD VALUES WE CURRENTLY CONSIDER GOOD
{
name: "ipv4 with leading 0s",
in: "1.1.01.0/24",
},
{
name: "ipv4-in-ipv6 with ipv4-sized prefix",
in: "::ffff:1.1.1.0/24",
},
{
name: "ipv4-in-ipv6 with ipv6-sized prefix",
in: "::ffff:1.1.1.0/120",
},
{
name: "ipv4 with bits past prefix",
in: "1.2.3.4/24",
},
{
name: "ipv6 with bits past prefix",
in: "2001:db8::1/64",
},
{
name: "prefix length with leading 0s",
in: "192.168.0.0/016",
},
// BAD VALUES
{
name: "empty string",
in: "",
err: "must be a valid CIDR value",
},
{
name: "junk",
in: "aaaaaaa",
err: "must be a valid CIDR value",
},
{
name: "IP address",
in: "1.2.3.4",
err: "must be a valid CIDR value",
},
{
name: "partial URL",
in: "192.168.0.1/healthz",
err: "must be a valid CIDR value",
},
{
name: "partial URL 2",
in: "192.168.0.1/0/99",
err: "must be a valid CIDR value",
},
{
name: "negative prefix length",
in: "192.168.0.0/-16",
err: "must be a valid CIDR value",
},
{
name: "prefix length with sign",
in: "192.168.0.0/+16",
err: "must be a valid CIDR value",
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := IsValidCIDR(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)
}
}
})
}
}
func TestGetWarningsForCIDR(t *testing.T) {
tests := []struct {
name string
fieldPath *field.Path
cidr string
want []string
}{
{
name: "IPv4 No failures",
cidr: "192.12.2.0/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: nil,
},
{
name: "IPv6 No failures",
cidr: "2001:db8::/64",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: nil,
},
{
name: "IPv4 with leading zeros",
cidr: "192.012.2.0/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: non-standard CIDR value "192.012.2.0/24" will be considered invalid in a future Kubernetes release: use "192.12.2.0/24"`,
},
},
{
name: "leading zeros in prefix length",
cidr: "192.12.2.0/024",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: non-standard CIDR value "192.12.2.0/024" will be considered invalid in a future Kubernetes release: use "192.12.2.0/24"`,
},
},
{
name: "IPv4-mapped IPv6",
cidr: "::ffff:192.12.2.0/120",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: non-standard CIDR value "::ffff:192.12.2.0/120" will be considered invalid in a future Kubernetes release: use "192.12.2.0/24"`,
},
},
{
name: "bits after prefix length",
cidr: "192.12.2.8/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: CIDR value "192.12.2.8/24" is ambiguous in this context (should be "192.12.2.0/24" or "192.12.2.8/32"?)`,
},
},
{
name: "multiple problems",
cidr: "192.012.2.8/24",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: CIDR value "192.012.2.8/24" is ambiguous in this context (should be "192.12.2.0/24" or "192.12.2.8/32"?)`,
`spec.loadBalancerSourceRanges[0]: non-standard CIDR value "192.012.2.8/24" will be considered invalid in a future Kubernetes release: use "192.12.2.0/24"`,
},
},
{
name: "IPv6 non-canonical format",
cidr: "2001:db8:0:0::/64",
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
want: []string{
`spec.loadBalancerSourceRanges[0]: IPv6 CIDR value "2001:db8:0:0::/64" should be in RFC 5952 canonical format ("2001:db8::/64")`,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := GetWarningsForCIDR(tt.fieldPath, tt.cidr); !reflect.DeepEqual(got, tt.want) {
t.Errorf("getWarningsForCIDR() = %v, want %v", got, tt.want)
}
})
}
}

View File

@ -24,7 +24,6 @@ import (
"unicode"
"k8s.io/apimachinery/pkg/util/validation/field"
netutils "k8s.io/utils/net"
)
const qnameCharFmt string = "[A-Za-z0-9]"
@ -369,45 +368,6 @@ func IsValidPortName(port string) []string {
return errs
}
// IsValidIP tests that the argument is a valid IP address.
func IsValidIP(fldPath *field.Path, value string) 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"))
}
return allErrors
}
// IsValidIPv4Address tests that the argument is a valid IPv4 address.
func IsValidIPv4Address(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 IPv4 address"))
}
return allErrors
}
// IsValidIPv6Address tests that the argument is a valid IPv6 address.
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 {
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 allErrors
}
const percentFmt string = "[0-9]+%"
const percentErrMsg string = "a valid percent string must be a numeric string followed by an ending '%'"

View File

@ -322,302 +322,6 @@ func TestIsValidLabelValue(t *testing.T) {
}
}
func TestIsValidIP(t *testing.T) {
for _, tc := range []struct {
name string
in string
family int
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.2.3.4",
family: 4,
},
{
name: "ipv4, all zeros",
in: "0.0.0.0",
family: 4,
},
{
name: "ipv4, max",
in: "255.255.255.255",
family: 4,
},
{
name: "ipv6",
in: "1234::abcd",
family: 6,
},
{
name: "ipv6, all zeros, collapsed",
in: "::",
family: 6,
},
{
name: "ipv6, max",
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
family: 6,
},
// GOOD, THOUGH NON-CANONICAL, VALUES
{
name: "ipv6, all zeros, expanded (non-canonical)",
in: "0:0:0:0:0:0:0:0",
family: 6,
},
{
name: "ipv6, leading 0s (non-canonical)",
in: "0001:002:03:4::",
family: 6,
},
{
name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD",
family: 6,
},
// BAD VALUES WE CURRENTLY CONSIDER GOOD
{
name: "ipv4 with leading 0s",
in: "1.1.1.01",
family: 4,
},
{
name: "ipv4-in-ipv6 value",
in: "::ffff:1.1.1.1",
family: 4,
},
// BAD VALUES
{
name: "empty string",
in: "",
err: "must be a valid IP address",
},
{
name: "junk",
in: "aaaaaaa",
err: "must be a valid IP address",
},
{
name: "domain name",
in: "myhost.mydomain",
err: "must be a valid IP address",
},
{
name: "cidr",
in: "1.2.3.0/24",
err: "must be a valid IP address",
},
{
name: "ipv4 with out-of-range octets",
in: "1.2.3.400",
err: "must be a valid IP address",
},
{
name: "ipv4 with negative octets",
in: "-1.0.0.0",
err: "must be a valid IP address",
},
{
name: "ipv6 with out-of-range segment",
in: "2001:db8::10005",
err: "must be a valid IP address",
},
{
name: "ipv4:port",
in: "1.2.3.4:80",
err: "must be a valid IP address",
},
{
name: "ipv6 with brackets",
in: "[2001:db8::1]",
err: "must be a valid IP address",
},
{
name: "[ipv6]:port",
in: "[2001:db8::1]:80",
err: "must be a valid IP address",
},
{
name: "host:port",
in: "example.com:80",
err: "must be a valid IP address",
},
{
name: "ipv6 with zone",
in: "1234::abcd%eth0",
err: "must be a valid IP address",
},
{
name: "ipv4 with zone",
in: "169.254.0.0%eth0",
err: "must be a valid IP address",
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := IsValidIP(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)
}
}
errs = IsValidIPv4Address(field.NewPath(""), tc.in)
if tc.family == 4 {
if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs)
}
} else {
if len(errs) == 0 {
t.Errorf("expected %q to fail IsValidIPv4Address", tc.in)
}
}
errs = IsValidIPv6Address(field.NewPath(""), tc.in)
if tc.family == 6 {
if len(errs) != 0 {
t.Errorf("expected %q to pass IsValidIPv6Address but got: %v", tc.in, errs)
}
} else {
if len(errs) == 0 {
t.Errorf("expected %q to fail IsValidIPv6Address", tc.in)
}
}
})
}
}
func TestIsValidCIDR(t *testing.T) {
for _, tc := range []struct {
name string
in string
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.0.0.0/8",
},
{
name: "ipv4, all IPs",
in: "0.0.0.0/0",
},
{
name: "ipv4, single IP",
in: "1.1.1.1/32",
},
{
name: "ipv6",
in: "2001:4860:4860::/48",
},
{
name: "ipv6, all IPs",
in: "::/0",
},
{
name: "ipv6, single IP",
in: "::1/128",
},
// GOOD, THOUGH NON-CANONICAL, VALUES
{
name: "ipv6, extra 0s (non-canonical)",
in: "2a00:79e0:2:0::/64",
},
{
name: "ipv6, capital letters (non-canonical)",
in: "2001:DB8::/64",
},
// BAD VALUES WE CURRENTLY CONSIDER GOOD
{
name: "ipv4 with leading 0s",
in: "1.1.01.0/24",
},
{
name: "ipv4-in-ipv6 with ipv4-sized prefix",
in: "::ffff:1.1.1.0/24",
},
{
name: "ipv4-in-ipv6 with ipv6-sized prefix",
in: "::ffff:1.1.1.0/120",
},
{
name: "ipv4 with bits past prefix",
in: "1.2.3.4/24",
},
{
name: "ipv6 with bits past prefix",
in: "2001:db8::1/64",
},
{
name: "prefix length with leading 0s",
in: "192.168.0.0/016",
},
// BAD VALUES
{
name: "empty string",
in: "",
err: "must be a valid CIDR value",
},
{
name: "junk",
in: "aaaaaaa",
err: "must be a valid CIDR value",
},
{
name: "IP address",
in: "1.2.3.4",
err: "must be a valid CIDR value",
},
{
name: "partial URL",
in: "192.168.0.1/healthz",
err: "must be a valid CIDR value",
},
{
name: "partial URL 2",
in: "192.168.0.1/0/99",
err: "must be a valid CIDR value",
},
{
name: "negative prefix length",
in: "192.168.0.0/-16",
err: "must be a valid CIDR value",
},
{
name: "prefix length with sign",
in: "192.168.0.0/+16",
err: "must be a valid CIDR value",
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := IsValidCIDR(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)
}
}
})
}
}
func TestIsHTTPHeaderName(t *testing.T) {
goodValues := []string{
// Common ones