Skip to content

Commit 610adeb

Browse files
danwinshipdanwinship
committed
Add utilvalidation.GetWarningsForIP and .GetWarningsForCIDR
(And port the existing Service warnings to use them.)
1 parent 3471700 commit 610adeb

File tree

4 files changed

+232
-155
lines changed

4 files changed

+232
-155
lines changed

‎pkg/api/service/warnings.go‎

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ package service
1818

1919
import (
2020
"fmt"
21-
"net/netip"
2221

22+
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
2323
"k8s.io/apimachinery/pkg/util/validation/field"
2424
api "k8s.io/kubernetes/pkg/apis/core"
2525
"k8s.io/kubernetes/pkg/apis/core/helper"
@@ -37,20 +37,20 @@ func GetWarningsForService(service, oldService *api.Service) []string {
3737

3838
if helper.IsServiceIPSet(service) {
3939
for i, clusterIP := range service.Spec.ClusterIPs {
40-
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("clusterIPs").Index(i), clusterIP)...)
40+
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("clusterIPs").Index(i), clusterIP)...)
4141
}
4242
}
4343

4444
for i, externalIP := range service.Spec.ExternalIPs {
45-
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("externalIPs").Index(i), externalIP)...)
45+
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("externalIPs").Index(i), externalIP)...)
4646
}
4747

4848
if len(service.Spec.LoadBalancerIP) > 0 {
49-
warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("loadBalancerIP"), service.Spec.LoadBalancerIP)...)
49+
warnings = append(warnings, utilvalidation.GetWarningsForIP(field.NewPath("spec").Child("loadBalancerIP"), service.Spec.LoadBalancerIP)...)
5050
}
5151

5252
for i, cidr := range service.Spec.LoadBalancerSourceRanges {
53-
warnings = append(warnings, getWarningsForCIDR(field.NewPath("spec").Child("loadBalancerSourceRanges").Index(i), cidr)...)
53+
warnings = append(warnings, utilvalidation.GetWarningsForCIDR(field.NewPath("spec").Child("loadBalancerSourceRanges").Index(i), cidr)...)
5454
}
5555

5656
if service.Spec.Type == api.ServiceTypeExternalName && len(service.Spec.ExternalIPs) > 0 {
@@ -62,45 +62,3 @@ func GetWarningsForService(service, oldService *api.Service) []string {
6262

6363
return warnings
6464
}
65-
66-
func getWarningsForIP(fieldPath *field.Path, address string) []string {
67-
// IPv4 addresses with leading zeros CVE-2021-29923 are not valid in golang since 1.17
68-
// This will also warn about possible future changes on the golang std library
69-
// xref: https://issues.k8s.io/108074
70-
ip, err := netip.ParseAddr(address)
71-
if err != nil {
72-
return []string{fmt.Sprintf("%s: IP address was accepted, but will be invalid in a future Kubernetes release: %v", fieldPath, err)}
73-
}
74-
// A Recommendation for IPv6 Address Text Representation
75-
//
76-
// "All of the above examples represent the same IPv6 address. This
77-
// flexibility has caused many problems for operators, systems
78-
// engineers, and customers.
79-
// ..."
80-
// https://datatracker.ietf.org/doc/rfc5952/
81-
if ip.Is6() && ip.String() != address {
82-
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())}
83-
}
84-
return []string{}
85-
}
86-
87-
func getWarningsForCIDR(fieldPath *field.Path, cidr string) []string {
88-
// IPv4 addresses with leading zeros CVE-2021-29923 are not valid in golang since 1.17
89-
// This will also warn about possible future changes on the golang std library
90-
// xref: https://issues.k8s.io/108074
91-
prefix, err := netip.ParsePrefix(cidr)
92-
if err != nil {
93-
return []string{fmt.Sprintf("%s: IP prefix was accepted, but will be invalid in a future Kubernetes release: %v", fieldPath, err)}
94-
}
95-
// A Recommendation for IPv6 Address Text Representation
96-
//
97-
// "All of the above examples represent the same IPv6 address. This
98-
// flexibility has caused many problems for operators, systems
99-
// engineers, and customers.
100-
// ..."
101-
// https://datatracker.ietf.org/doc/rfc5952/
102-
if prefix.Addr().Is6() && prefix.String() != cidr {
103-
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())}
104-
}
105-
return []string{}
106-
}

‎pkg/api/service/warnings_test.go‎

Lines changed: 17 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
"github.com/google/go-cmp/cmp"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25-
"k8s.io/apimachinery/pkg/util/validation/field"
2625
api "k8s.io/kubernetes/pkg/apis/core"
2726
)
2827

@@ -108,58 +107,58 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
108107
name: "IPv4 with leading zeros",
109108
service: service([]string{"192.012.2.2"}),
110109
want: []string{
111-
`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`,
110+
`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"`,
112111
},
113112
},
114113
{
115114
name: "Dual Stack IPv4-IPv6 and IPv4 with leading zeros",
116115
service: service([]string{"192.012.2.2", "2001:db8::2"}),
117116
want: []string{
118-
`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`,
117+
`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"`,
119118
},
120119
},
121120
{
122121
name: "Dual Stack IPv6-IPv4 and IPv4 with leading zeros",
123122
service: service([]string{"2001:db8::2", "192.012.2.2"}),
124123
want: []string{
125-
`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`,
124+
`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"`,
126125
},
127126
},
128127
{
129128
name: "IPv6 non canonical format",
130129
service: service([]string{"2001:db8:0:0::2"}),
131130
want: []string{
132-
`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`,
131+
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
133132
},
134133
},
135134
{
136135
name: "Dual Stack IPv4-IPv6 and IPv6 non-canonical format",
137136
service: service([]string{"192.12.2.2", "2001:db8:0:0::2"}),
138137
want: []string{
139-
`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`,
138+
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
140139
},
141140
},
142141
{
143142
name: "Dual Stack IPv6-IPv4 and IPv6 non-canonical formats",
144143
service: service([]string{"2001:db8:0:0::2", "192.12.2.2"}),
145144
want: []string{
146-
`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`,
145+
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
147146
},
148147
},
149148
{
150149
name: "Dual Stack IPv4-IPv6 and IPv4 with leading zeros and IPv6 non-canonical format",
151150
service: service([]string{"192.012.2.2", "2001:db8:0:0::2"}),
152151
want: []string{
153-
`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`,
154-
`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`,
152+
`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"`,
153+
`spec.clusterIPs[1]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
155154
},
156155
},
157156
{
158157
name: "Dual Stack IPv6-IPv4 and IPv4 with leading zeros and IPv6 non-canonical format",
159158
service: service([]string{"2001:db8:0:0::2", "192.012.2.2"}),
160159
want: []string{
161-
`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`,
162-
`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`,
160+
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
161+
`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"`,
163162
},
164163
},
165164
{
@@ -179,13 +178,13 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
179178
},
180179
},
181180
want: []string{
182-
`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`,
183-
`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`,
184-
`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`,
185-
`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`,
186-
`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`,
187-
`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`,
188-
`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`,
181+
`spec.clusterIPs[0]: IPv6 address "2001:db8:0:0::2" should be in RFC 5952 canonical format ("2001:db8::2")`,
182+
`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"`,
183+
`spec.externalIPs[0]: IPv6 address "2001:db8:1:0::2" should be in RFC 5952 canonical format ("2001:db8:1::2")`,
184+
`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"`,
185+
`spec.loadBalancerIP: non-standard IP address "10.001.1.1" will be considered invalid in a future Kubernetes release: use "10.1.1.1"`,
186+
`spec.loadBalancerSourceRanges[0]: IPv6 CIDR value "2001:db8:1:0::/64" should be in RFC 5952 canonical format ("2001:db8:1::/64")`,
187+
`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"`,
189188
},
190189
},
191190
}
@@ -197,93 +196,3 @@ func TestGetWarningsForServiceClusterIPs(t *testing.T) {
197196
})
198197
}
199198
}
200-
201-
func Test_getWarningsForIP(t *testing.T) {
202-
tests := []struct {
203-
name string
204-
fieldPath *field.Path
205-
address string
206-
want []string
207-
}{
208-
{
209-
name: "IPv4 No failures",
210-
address: "192.12.2.2",
211-
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
212-
want: []string{},
213-
},
214-
{
215-
name: "IPv6 No failures",
216-
address: "2001:db8::2",
217-
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
218-
want: []string{},
219-
},
220-
{
221-
name: "IPv4 with leading zeros",
222-
address: "192.012.2.2",
223-
fieldPath: field.NewPath("spec").Child("clusterIPs").Index(0),
224-
want: []string{
225-
`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`,
226-
},
227-
},
228-
{
229-
name: "IPv6 non-canonical format",
230-
address: "2001:db8:0:0::2",
231-
fieldPath: field.NewPath("spec").Child("loadBalancerIP"),
232-
want: []string{
233-
`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`,
234-
},
235-
},
236-
}
237-
for _, tt := range tests {
238-
t.Run(tt.name, func(t *testing.T) {
239-
if got := getWarningsForIP(tt.fieldPath, tt.address); !reflect.DeepEqual(got, tt.want) {
240-
t.Errorf("getWarningsForIP() = %v, want %v", got, tt.want)
241-
}
242-
})
243-
}
244-
}
245-
246-
func Test_getWarningsForCIDR(t *testing.T) {
247-
tests := []struct {
248-
name string
249-
fieldPath *field.Path
250-
cidr string
251-
want []string
252-
}{
253-
{
254-
name: "IPv4 No failures",
255-
cidr: "192.12.2.0/24",
256-
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
257-
want: []string{},
258-
},
259-
{
260-
name: "IPv6 No failures",
261-
cidr: "2001:db8::/64",
262-
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
263-
want: []string{},
264-
},
265-
{
266-
name: "IPv4 with leading zeros",
267-
cidr: "192.012.2.0/24",
268-
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
269-
want: []string{
270-
`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`,
271-
},
272-
},
273-
{
274-
name: "IPv6 non-canonical format",
275-
cidr: "2001:db8:0:0::/64",
276-
fieldPath: field.NewPath("spec").Child("loadBalancerSourceRanges").Index(0),
277-
want: []string{
278-
`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`,
279-
},
280-
},
281-
}
282-
for _, tt := range tests {
283-
t.Run(tt.name, func(t *testing.T) {
284-
if got := getWarningsForCIDR(tt.fieldPath, tt.cidr); !reflect.DeepEqual(got, tt.want) {
285-
t.Errorf("getWarningsForCIDR() = %v, want %v", got, tt.want)
286-
}
287-
})
288-
}
289-
}

‎staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ limitations under the License.
1717
package validation
1818

1919
import (
20+
"fmt"
21+
"net/netip"
22+
2023
"k8s.io/apimachinery/pkg/util/validation/field"
24+
"k8s.io/klog/v2"
2125
netutils "k8s.io/utils/net"
2226
)
2327

@@ -30,6 +34,36 @@ func IsValidIP(fldPath *field.Path, value string) field.ErrorList {
3034
return allErrors
3135
}
3236

37+
// GetWarningsForIP returns warnings for IP address values in non-standard forms. This
38+
// should only be used with fields that are validated with IsValidIP().
39+
func GetWarningsForIP(fldPath *field.Path, value string) []string {
40+
ip := netutils.ParseIPSloppy(value)
41+
if ip == nil {
42+
klog.ErrorS(nil, "GetWarningsForIP called on value that was not validated with IsValidIP", "field", fldPath, "value", value)
43+
return nil
44+
}
45+
46+
addr, _ := netip.ParseAddr(value)
47+
if !addr.IsValid() || addr.Is4In6() {
48+
// This catches 2 cases: leading 0s (if ParseIPSloppy() accepted it but
49+
// ParseAddr() doesn't) or IPv4-mapped IPv6 (.Is4In6()). Either way,
50+
// re-stringifying the net.IP value will give the preferred form.
51+
return []string{
52+
fmt.Sprintf("%s: non-standard IP address %q will be considered invalid in a future Kubernetes release: use %q", fldPath, value, ip.String()),
53+
}
54+
}
55+
56+
// If ParseIPSloppy() and ParseAddr() both accept it then it's fully valid, though
57+
// it may be non-canonical.
58+
if addr.Is6() && addr.String() != value {
59+
return []string{
60+
fmt.Sprintf("%s: IPv6 address %q should be in RFC 5952 canonical format (%q)", fldPath, value, addr.String()),
61+
}
62+
}
63+
64+
return nil
65+
}
66+
3367
// IsValidIPv4Address tests that the argument is a valid IPv4 address.
3468
func IsValidIPv4Address(fldPath *field.Path, value string) field.ErrorList {
3569
var allErrors field.ErrorList
@@ -59,3 +93,47 @@ func IsValidCIDR(fldPath *field.Path, value string) field.ErrorList {
5993
}
6094
return allErrors
6195
}
96+
97+
// GetWarningsForCIDR returns warnings for CIDR values in non-standard forms. This should
98+
// only be used with fields that are validated with IsValidCIDR().
99+
func GetWarningsForCIDR(fldPath *field.Path, value string) []string {
100+
ip, ipnet, err := netutils.ParseCIDRSloppy(value)
101+
if err != nil {
102+
klog.ErrorS(err, "GetWarningsForCIDR called on value that was not validated with IsValidCIDR", "field", fldPath, "value", value)
103+
return nil
104+
}
105+
106+
var warnings []string
107+
108+
// Check for bits set after prefix length
109+
if !ip.Equal(ipnet.IP) {
110+
_, addrlen := ipnet.Mask.Size()
111+
singleIPCIDR := fmt.Sprintf("%s/%d", ip.String(), addrlen)
112+
warnings = append(warnings,
113+
fmt.Sprintf("%s: CIDR value %q is ambiguous in this context (should be %q or %q?)", fldPath, value, ipnet.String(), singleIPCIDR),
114+
)
115+
}
116+
117+
prefix, _ := netip.ParsePrefix(value)
118+
addr := prefix.Addr()
119+
if !prefix.IsValid() || addr.Is4In6() {
120+
// This catches 2 cases: leading 0s (if ParseCIDRSloppy() accepted it but
121+
// ParsePrefix() doesn't) or IPv4-mapped IPv6 (.Is4In6()). Either way,
122+
// re-stringifying the net.IPNet value will give the preferred form.
123+
warnings = append(warnings,
124+
fmt.Sprintf("%s: non-standard CIDR value %q will be considered invalid in a future Kubernetes release: use %q", fldPath, value, ipnet.String()),
125+
)
126+
}
127+
128+
// If ParseCIDRSloppy() and ParsePrefix() both accept it then it's fully valid,
129+
// though it may be non-canonical. But only check this if there are no other
130+
// warnings, since either of the other warnings would also cause a round-trip
131+
// failure.
132+
if len(warnings) == 0 && addr.Is6() && prefix.String() != value {
133+
warnings = append(warnings,
134+
fmt.Sprintf("%s: IPv6 CIDR value %q should be in RFC 5952 canonical format (%q)", fldPath, value, prefix.String()),
135+
)
136+
}
137+
138+
return warnings
139+
}

0 commit comments

Comments
 (0)