Skip to content

Commit 3782b55

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#128786 from danwinship/bad-ip-warnings
warn on bad IPs in objects
2 parents 8f97ac7 + 7316d83 commit 3782b55

34 files changed

+1243
-616
lines changed

‎pkg/api/pod/warnings.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2626
"k8s.io/apimachinery/pkg/util/sets"
27+
"k8s.io/apimachinery/pkg/util/validation"
2728
"k8s.io/apimachinery/pkg/util/validation/field"
2829
nodeapi "k8s.io/kubernetes/pkg/api/node"
2930
pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
@@ -351,6 +352,16 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
351352
}
352353
}
353354

355+
// Deprecated IP address formats
356+
if podSpec.DNSConfig != nil {
357+
for i, ns := range podSpec.DNSConfig.Nameservers {
358+
warnings = append(warnings, validation.GetWarningsForIP(fieldPath.Child("spec", "dnsConfig", "nameservers").Index(i), ns)...)
359+
}
360+
}
361+
for i, hostAlias := range podSpec.HostAliases {
362+
warnings = append(warnings, validation.GetWarningsForIP(fieldPath.Child("spec", "hostAliases").Index(i).Child("ip"), hostAlias.IP)...)
363+
}
364+
354365
return warnings
355366
}
356367

‎pkg/api/pod/warnings_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,6 +1767,21 @@ func TestWarnings(t *testing.T) {
17671767
`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])?')`,
17681768
},
17691769
},
1770+
{
1771+
name: "dubious IP address formats",
1772+
template: &api.PodTemplateSpec{Spec: api.PodSpec{
1773+
DNSConfig: &api.PodDNSConfig{
1774+
Nameservers: []string{"1.2.3.4", "05.06.07.08"},
1775+
},
1776+
HostAliases: []api.HostAlias{
1777+
{IP: "::ffff:1.2.3.4"},
1778+
},
1779+
}},
1780+
expected: []string{
1781+
`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"`,
1782+
`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"`,
1783+
},
1784+
},
17701785
}
17711786

17721787
for _, tc := range testcases {

‎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-
}

0 commit comments

Comments
 (0)