github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 03:11:40 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #114505 from aojea/service_warnings

Browse Source 
Services API: warnings on IP addresses
This commit is contained in:
Kubernetes Prow Robot 2022-12-16 18:07:52 -08:00 committed by GitHub
commit 038d983769
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 345 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
pkg/api/service/warnings.go Normal file
View File

@ -0,0 +1,95 @@
/*
Copyright 2022 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 service
import (
"fmt"
"net/netip"
"k8s.io/apimachinery/pkg/util/validation/field"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/helper"
)
func GetWarningsForService(service, oldService *api.Service) []string {
if service == nil {
return nil
}
var warnings []string
if helper.IsServiceIPSet(service) {
for i, clusterIP := range service.Spec.ClusterIPs {
warnings = append(warnings, 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)...)
}
if len(service.Spec.LoadBalancerIP) > 0 {
warnings = append(warnings, 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)...)
}
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{}
}

244
pkg/api/service/warnings_test.go Normal file
View File

@ -0,0 +1,244 @@
/*
Copyright 2022 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 service
import (
"reflect"
"testing"
"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"
)
func TestGetWarningsForServiceClusterIPs(t *testing.T) {
service := func(clusterIPs []string) *api.Service {
svc := api.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "svc-test",
Namespace: "ns",
},
Spec: api.ServiceSpec{
Type: api.ServiceTypeClusterIP,
},
}
if len(clusterIPs) > 0 {
svc.Spec.ClusterIP = clusterIPs[0]
svc.Spec.ClusterIPs = clusterIPs
}
return &svc
}
tests := []struct {
name string
service *api.Service
oldService *api.Service
want []string
}{
{
name: "IPv4 No failures",
service: service([]string{"192.12.2.2"}),
},
{
name: "IPv6 No failures",
service: service([]string{"2001:db8::2"}),
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
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`,
},
},
{
name: "Service with all IPs fields with errors",
service: &api.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "svc-test",
Namespace: "ns",
},
Spec: api.ServiceSpec{
ClusterIP: "2001:db8:0:0::2",
ClusterIPs: []string{"2001:db8:0:0::2", "192.012.2.2"},
ExternalIPs: []string{"2001:db8:1:0::2", "10.012.2.2"},
LoadBalancerIP: "10.001.1.1",
LoadBalancerSourceRanges: []string{"2001:db8:1:0::/64", "10.012.2.0/24"},
Type: api.ServiceTypeClusterIP,
},
},
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`,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := GetWarningsForService(tt.service, tt.oldService); !reflect.DeepEqual(got, tt.want) {
t.Errorf("GetWarningsForService() = %v", cmp.Diff(got, tt.want))
}
})
}
}
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)
}
})
}
}

8
pkg/registry/core/service/strategy.go
View File

@ -25,8 +25,10 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api/legacyscheme"
serviceapi "k8s.io/kubernetes/pkg/api/service"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/validation"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
@ -83,7 +85,9 @@ func (svcStrategy) Validate(ctx context.Context, obj runtime.Object) field.Error
}
// WarningsOnCreate returns warnings for the creation of the given object.
func (svcStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil }
func (svcStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
return serviceapi.GetWarningsForService(obj.(*api.Service), nil)
}
// Canonicalize normalizes the object after validation.
func (svcStrategy) Canonicalize(obj runtime.Object) {
@ -100,7 +104,7 @@ func (strategy svcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime
// WarningsOnUpdate returns warnings for the given update.
func (svcStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
return serviceapi.GetWarningsForService(obj.(*api.Service), old.(*api.Service))
}
func (svcStrategy) AllowUnconditionalUpdate() bool {