Add warnings to all IP/CIDR-valued fields

This commit is contained in:
Dan Winship 2024-02-16 17:56:31 -05:00
parent d4c55d06cf
commit 7316d83137
16 changed files with 561 additions and 25 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

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

@ -241,7 +241,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

@ -3407,3 +3407,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)
}
}