From 4844b382dc90f64909dfdf8ae117274c4dc4768d Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Sat, 18 Jan 2020 11:40:18 +0100 Subject: [PATCH] kube-proxy: validate dual-stack cidrs kube-proxy was not validating correctly the clusterCIDRs, if dual-stack it MAY have 1 or more clusterCIDRs. If it has 2 cidrs and at least one of each IP family. It also fixes a bug where validation was not taking into account the feature gates global state. --- pkg/proxy/apis/config/validation/BUILD | 2 + .../apis/config/validation/validation.go | 33 +++- .../apis/config/validation/validation_test.go | 177 +++++++++++++++++- 3 files changed, 203 insertions(+), 9 deletions(-) diff --git a/pkg/proxy/apis/config/validation/BUILD b/pkg/proxy/apis/config/validation/BUILD index 8c5c7ab447e..ac34aeb4939 100644 --- a/pkg/proxy/apis/config/validation/BUILD +++ b/pkg/proxy/apis/config/validation/BUILD @@ -17,8 +17,10 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/component-base/config:go_default_library", "//staging/src/k8s.io/component-base/metrics:go_default_library", + "//vendor/k8s.io/utils/net:go_default_library", ], ) diff --git a/pkg/proxy/apis/config/validation/validation.go b/pkg/proxy/apis/config/validation/validation.go index 773dd542dc2..63557542a8b 100644 --- a/pkg/proxy/apis/config/validation/validation.go +++ b/pkg/proxy/apis/config/validation/validation.go @@ -26,11 +26,13 @@ import ( utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" componentbaseconfig "k8s.io/component-base/config" "k8s.io/component-base/metrics" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" kubefeatures "k8s.io/kubernetes/pkg/features" kubeproxyconfig "k8s.io/kubernetes/pkg/proxy/apis/config" + netutils "k8s.io/utils/net" ) // Validate validates the configuration of kube-proxy @@ -39,6 +41,11 @@ func Validate(config *kubeproxyconfig.KubeProxyConfiguration) field.ErrorList { newPath := field.NewPath("KubeProxyConfiguration") + effectiveFeatures := utilfeature.DefaultFeatureGate.DeepCopy() + if err := effectiveFeatures.SetFromMap(config.FeatureGates); err != nil { + allErrs = append(allErrs, field.Invalid(newPath.Child("featureGates"), config.FeatureGates, err.Error())) + } + allErrs = append(allErrs, validateKubeProxyIPTablesConfiguration(config.IPTables, newPath.Child("KubeProxyIPTablesConfiguration"))...) if config.Mode == kubeproxyconfig.ProxyModeIPVS { allErrs = append(allErrs, validateKubeProxyIPVSConfiguration(config.IPVS, newPath.Child("KubeProxyIPVSConfiguration"))...) @@ -69,16 +76,26 @@ func Validate(config *kubeproxyconfig.KubeProxyConfiguration) field.ErrorList { allErrs = append(allErrs, validateHostPort(config.MetricsBindAddress, newPath.Child("MetricsBindAddress"))...) if config.ClusterCIDR != "" { - if config.FeatureGates[string(kubefeatures.IPv6DualStack)] { - cidrs := strings.Split(config.ClusterCIDR, ",") - for _, cidr := range cidrs { - if _, _, err := net.ParseCIDR(cidr); err != nil { - allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), cidr, "must be a valid CIDR block (e.g. 10.100.0.0/16 or FD02::0:0:0/96)")) - } + cidrs := strings.Split(config.ClusterCIDR, ",") + dualStackEnabled := effectiveFeatures.Enabled(kubefeatures.IPv6DualStack) + + switch { + // if DualStack only valid one cidr or two cidrs with one of each IP family + case dualStackEnabled && len(cidrs) > 2: + allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), config.ClusterCIDR, "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)")) + // if DualStack and two cidrs validate if there is at least one of each IP family + case dualStackEnabled && len(cidrs) == 2: + isDual, err := netutils.IsDualStackCIDRStrings(cidrs) + if err != nil || !isDual { + allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), config.ClusterCIDR, "must be a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)")) } - } else { + // if not DualStack only one CIDR allowed + case !dualStackEnabled && len(cidrs) > 1: + allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), config.ClusterCIDR, "only one CIDR allowed (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)")) + // if we are here means that len(cidrs) == 1, we need to validate it + default: if _, _, err := net.ParseCIDR(config.ClusterCIDR); err != nil { - allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), config.ClusterCIDR, "must be a valid CIDR block (e.g. 10.100.0.0/16 or FD02::0:0:0/96)")) + allErrs = append(allErrs, field.Invalid(newPath.Child("ClusterCIDR"), config.ClusterCIDR, "must be a valid CIDR block (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)")) } } } diff --git a/pkg/proxy/apis/config/validation/validation_test.go b/pkg/proxy/apis/config/validation/validation_test.go index 28da03dac00..4ff23262847 100644 --- a/pkg/proxy/apis/config/validation/validation_test.go +++ b/pkg/proxy/apis/config/validation/validation_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" componentbaseconfig "k8s.io/component-base/config" kubeproxyconfig "k8s.io/kubernetes/pkg/proxy/apis/config" + "k8s.io/utils/pointer" ) @@ -100,6 +101,85 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, + { + BindAddress: "fd00:192:168:59::103", + HealthzBindAddress: "", + MetricsBindAddress: "[::1]:10249", + ClusterCIDR: "fd00:192:168:59::/64", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + { + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "192.168.59.0/24", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + { + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "fd00:192:168::/64", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + { + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "192.168.59.0/24,fd00:192:168::/64", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, } for _, successCase := range successCases { @@ -202,7 +282,102 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be a valid CIDR block (e.g. 10.100.0.0/16 or FD02::0:0:0/96)", + msg: "must be a valid CIDR block (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)", + }, + { + config: kubeproxyconfig.KubeProxyConfiguration{ + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + // DualStack ClusterCIDR without feature flag enabled + FeatureGates: map[string]bool{"IPv6DualStack": false}, + ClusterCIDR: "192.168.59.0/24,fd00:192:168::/64", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + msg: "only one CIDR allowed (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)", + }, + { + config: kubeproxyconfig.KubeProxyConfiguration{ + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + // DualStack with multiple CIDRs but only one IP family + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "192.168.59.0/24,10.0.0.0/16", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + msg: "must be a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", + }, + { + config: kubeproxyconfig.KubeProxyConfiguration{ + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + // DualStack with an invalid subnet + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "192.168.59.0/24,fd00:192:168::/64,a.b.c.d/f", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + msg: "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", + }, + { + config: kubeproxyconfig.KubeProxyConfiguration{ + BindAddress: "10.10.12.11", + HealthzBindAddress: "0.0.0.0:12345", + MetricsBindAddress: "127.0.0.1:10249", + FeatureGates: map[string]bool{"IPv6DualStack": true}, + ClusterCIDR: "192.168.59.0/24,fd00:192:168::/64,10.0.0.0/16", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + }, + msg: "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", }, { config: kubeproxyconfig.KubeProxyConfiguration{