From 574442dd7b15617883a123226a33110044021079 Mon Sep 17 00:00:00 2001 From: Matt Long Date: Mon, 25 May 2020 13:02:17 +0200 Subject: [PATCH 1/8] Enable configuration of the calico IP_AUTODETECTION_METHOD --- pkg/apis/kops/networking.go | 12 ++++++++++++ upup/models/bindata.go | 8 ++++++++ .../k8s-1.12.yaml.template | 4 ++++ .../k8s-1.16.yaml.template | 4 ++++ 4 files changed, 28 insertions(+) diff --git a/pkg/apis/kops/networking.go b/pkg/apis/kops/networking.go index 644feaba8fe0e..b66ff43df3c86 100644 --- a/pkg/apis/kops/networking.go +++ b/pkg/apis/kops/networking.go @@ -118,6 +118,18 @@ type CalicoNetworkingSpec struct { PrometheusProcessMetricsEnabled bool `json:"prometheusProcessMetricsEnabled,omitempty"` // MajorVersion is the version of Calico to use MajorVersion string `json:"majorVersion,omitempty"` + // IPAutoDetectionMethod configures how Calico chooses the IP address used to route + // between nodes. This should be set when the host has multiple interfaces + // and it is important to select the interface used. + // Options: "first-found" (default), "can-reach=DESTINATION", + // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" + IPAutoDetectionMethod string `json:"ipAutoDetectionMethod,omitempty"` + // IP6AutoDetectionMethod configures how Calico chooses the IP address used to route + // between nodes. This should be set when the host has multiple interfaces + // and it is important to select the interface used. + // Options: "first-found" (default), "can-reach=DESTINATION", + // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" + IP6AutoDetectionMethod string `json:"ip6AutoDetectionMethod,omitempty"` // IptablesBackend controls which variant of iptables binary Felix uses // Default: Auto (other options: Legacy, NFT) IptablesBackend string `json:"iptablesBackend,omitempty"` diff --git a/upup/models/bindata.go b/upup/models/bindata.go index fa6de4c40df03..f697879ce4d43 100644 --- a/upup/models/bindata.go +++ b/upup/models/bindata.go @@ -7589,6 +7589,10 @@ spec: # Auto-detect the BGP IP address. - name: IP value: "autodetect" + - name: IP_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + - name: IP6_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" @@ -8704,6 +8708,10 @@ spec: # Auto-detect the BGP IP address. - name: IP value: "autodetect" + - name: IP_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + - name: IP6_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" diff --git a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template index 1440d7957c8ee..75e6d18564161 100644 --- a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template @@ -789,6 +789,10 @@ spec: # Auto-detect the BGP IP address. - name: IP value: "autodetect" + - name: IP_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + - name: IP6_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" diff --git a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template index 722bf53cb8bb0..24c41196bc071 100644 --- a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template @@ -800,6 +800,10 @@ spec: # Auto-detect the BGP IP address. - name: IP value: "autodetect" + - name: IP_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + - name: IP6_AUTODETECTION_METHOD + value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" From c00464f11d410a6e64ee2904ddf02c89095fd789 Mon Sep 17 00:00:00 2001 From: Matt Long Date: Wed, 27 May 2020 15:24:01 +0200 Subject: [PATCH 2/8] Update crds, apis, models. Add limited field validation --- k8s/crds/kops.k8s.io_clusters.yaml | 16 ++++ pkg/apis/kops/networking.go | 8 +- pkg/apis/kops/v1alpha2/networking.go | 12 +++ .../kops/v1alpha2/zz_generated.conversion.go | 4 + pkg/apis/kops/validation/validation.go | 53 +++++++++++++ pkg/apis/kops/validation/validation_test.go | 75 +++++++++++++++++++ upup/models/bindata.go | 8 +- .../k8s-1.12.yaml.template | 4 +- .../k8s-1.16.yaml.template | 4 +- .../pkg/fi/cloudup/bootstrapchannelbuilder.go | 4 +- 10 files changed, 174 insertions(+), 14 deletions(-) diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index d84214e8e0b75..6711a9e487a4f 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -2655,6 +2655,22 @@ spec: binary Felix uses Default: Auto (other options: Legacy, NFT)' type: string + ipv4AutoDetectionMethod: + description: 'IPv4AutoDetectionMethod configures how Calico + chooses the IP address used to route between nodes. This + should be set when the host has multiple interfaces and + it is important to select the interface used. Options: "first-found" + (default), "can-reach=DESTINATION", "interface=INTERFACE-REGEX", + or "skip-interface=INTERFACE-REGEX"' + type: string + ipv6AutoDetectionMethod: + description: 'IPv6AutoDetectionMethod configures how Calico + chooses the IP address used to route between nodes. This + should be set when the host has multiple interfaces and + it is important to select the interface used. Options: "first-found" + (default), "can-reach=DESTINATION", "interface=INTERFACE-REGEX", + or "skip-interface=INTERFACE-REGEX"' + type: string logSeverityScreen: description: 'LogSeverityScreen lets us set the desired log level. (Default: info)' diff --git a/pkg/apis/kops/networking.go b/pkg/apis/kops/networking.go index b66ff43df3c86..52e506d47b7ff 100644 --- a/pkg/apis/kops/networking.go +++ b/pkg/apis/kops/networking.go @@ -118,18 +118,18 @@ type CalicoNetworkingSpec struct { PrometheusProcessMetricsEnabled bool `json:"prometheusProcessMetricsEnabled,omitempty"` // MajorVersion is the version of Calico to use MajorVersion string `json:"majorVersion,omitempty"` - // IPAutoDetectionMethod configures how Calico chooses the IP address used to route + // IPv4AutoDetectionMethod configures how Calico chooses the IP address used to route // between nodes. This should be set when the host has multiple interfaces // and it is important to select the interface used. // Options: "first-found" (default), "can-reach=DESTINATION", // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" - IPAutoDetectionMethod string `json:"ipAutoDetectionMethod,omitempty"` - // IP6AutoDetectionMethod configures how Calico chooses the IP address used to route + IPv4AutoDetectionMethod string `json:"ipv4AutoDetectionMethod,omitempty"` + // IPv6AutoDetectionMethod configures how Calico chooses the IP address used to route // between nodes. This should be set when the host has multiple interfaces // and it is important to select the interface used. // Options: "first-found" (default), "can-reach=DESTINATION", // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" - IP6AutoDetectionMethod string `json:"ip6AutoDetectionMethod,omitempty"` + IPv6AutoDetectionMethod string `json:"ipv6AutoDetectionMethod,omitempty"` // IptablesBackend controls which variant of iptables binary Felix uses // Default: Auto (other options: Legacy, NFT) IptablesBackend string `json:"iptablesBackend,omitempty"` diff --git a/pkg/apis/kops/v1alpha2/networking.go b/pkg/apis/kops/v1alpha2/networking.go index 84bac3c0a283c..fe9761f3363c8 100644 --- a/pkg/apis/kops/v1alpha2/networking.go +++ b/pkg/apis/kops/v1alpha2/networking.go @@ -123,6 +123,18 @@ type CalicoNetworkingSpec struct { IptablesBackend string `json:"iptablesBackend,omitempty"` // IPIPMode is mode for CALICO_IPV4POOL_IPIP IPIPMode string `json:"ipipMode,omitempty"` + // IPv4AutoDetectionMethod configures how Calico chooses the IP address used to route + // between nodes. This should be set when the host has multiple interfaces + // and it is important to select the interface used. + // Options: "first-found" (default), "can-reach=DESTINATION", + // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" + IPv4AutoDetectionMethod string `json:"ipv4AutoDetectionMethod,omitempty"` + // IPv6AutoDetectionMethod configures how Calico chooses the IP address used to route + // between nodes. This should be set when the host has multiple interfaces + // and it is important to select the interface used. + // Options: "first-found" (default), "can-reach=DESTINATION", + // "interface=INTERFACE-REGEX", or "skip-interface=INTERFACE-REGEX" + IPv6AutoDetectionMethod string `json:"ipv6AutoDetectionMethod,omitempty"` // TyphaPrometheusMetricsEnabled enables Prometheus metrics collection from Typha // (default: false) TyphaPrometheusMetricsEnabled bool `json:"typhaPrometheusMetricsEnabled,omitempty"` diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index d3f72e7f94d0a..1ede7f5c07fce 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go @@ -1292,6 +1292,8 @@ func autoConvert_v1alpha2_CalicoNetworkingSpec_To_kops_CalicoNetworkingSpec(in * out.MajorVersion = in.MajorVersion out.IptablesBackend = in.IptablesBackend out.IPIPMode = in.IPIPMode + out.IPv4AutoDetectionMethod = in.IPv4AutoDetectionMethod + out.IPv6AutoDetectionMethod = in.IPv6AutoDetectionMethod out.TyphaPrometheusMetricsEnabled = in.TyphaPrometheusMetricsEnabled out.TyphaPrometheusMetricsPort = in.TyphaPrometheusMetricsPort out.TyphaReplicas = in.TyphaReplicas @@ -1313,6 +1315,8 @@ func autoConvert_kops_CalicoNetworkingSpec_To_v1alpha2_CalicoNetworkingSpec(in * out.PrometheusGoMetricsEnabled = in.PrometheusGoMetricsEnabled out.PrometheusProcessMetricsEnabled = in.PrometheusProcessMetricsEnabled out.MajorVersion = in.MajorVersion + out.IPv4AutoDetectionMethod = in.IPv4AutoDetectionMethod + out.IPv6AutoDetectionMethod = in.IPv6AutoDetectionMethod out.IptablesBackend = in.IptablesBackend out.IPIPMode = in.IPIPMode out.TyphaPrometheusMetricsEnabled = in.TyphaPrometheusMetricsEnabled diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 4efe43df8d149..e3e30019acd14 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -19,6 +19,7 @@ package validation import ( "fmt" "net" + "regexp" "strings" "github.com/blang/semver" @@ -599,9 +600,61 @@ func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e *kops.EtcdClusterS allErrs = append(allErrs, IsValidValue(fldPath.Child("iptablesBackend"), &v.IptablesBackend, valid)...) } + if v.IPv4AutoDetectionMethod != "" { + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv4AutoDetectionMethod"), &v.IPv4AutoDetectionMethod)...) + } + + if v.IPv6AutoDetectionMethod != "" { + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv6AutoDetectionMethod"), &v.IPv6AutoDetectionMethod)...) + } + return allErrs } +func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string) field.ErrorList { + methodFirstFound := "first-found" + methodCanReach := "can-reach=" + methodInterface := "interface=" + methodSkipInterface := "skip-interface=" + validationError := field.ErrorList{} + + // validation code is based on the checks in calico/node startup code + // valid formats are "first-found", "can-reach=DEST", or + // "(skip-)interface=" + // + // We won't do deep validation of the values in this check, since they can + // be actual interface names or regexes + if *runtime == methodFirstFound { + return field.ErrorList{} + + } else if strings.HasPrefix(*runtime, methodCanReach) { + destStr := strings.TrimPrefix(*runtime, methodCanReach) + regex := regexp.MustCompile("\\s*").MatchString(destStr) + if !regex { + validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'can-reach='")) + } + return validationError + + } else if strings.HasPrefix(*runtime, methodInterface) { + ifStr := strings.TrimPrefix(*runtime, methodInterface) + ifRegexes := regexp.MustCompile("\\s*,\\s*").Split(ifStr, -1) + fmt.Printf("%+v %d\n", ifRegexes, len(ifRegexes)) + if len(ifRegexes) == 0 || ifRegexes[0] == "" { + validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'interface='")) + } + return validationError + + } else if strings.HasPrefix(*runtime, methodSkipInterface) { + ifStr := strings.TrimPrefix(*runtime, methodSkipInterface) + ifRegexes := regexp.MustCompile("\\s*,\\s*").Split(ifStr, -1) + if len(ifRegexes) == 0 || ifRegexes[0] == "" { + validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'skip-interface='")) + } + return validationError + } + return field.ErrorList{field.Invalid(fldPath, runtime, "Invalid autodetection method")} +} + func validateContainerRuntime(runtime *string, fldPath *field.Path) field.ErrorList { valid := []string{"containerd", "docker"} diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index 9071f5a9d3d75..71bacec0ff483 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -396,6 +396,81 @@ func Test_Validate_Calico(t *testing.T) { }, ExpectedErrors: []string{"Forbidden::calico.majorVersion"}, }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "first-found", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv6AutoDetectionMethod: "first-found", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "can-reach=8.8.8.8", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv6AutoDetectionMethod: "can-reach=2001:4860:4860::8888", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "bogus", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"}, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv6AutoDetectionMethod: "bogus", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv6AutoDetectionMethod"}, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv6AutoDetectionMethod: "interface=", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv6AutoDetectionMethod"}, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "interface=en*,eth0", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv6AutoDetectionMethod: "skip-interface=en*,eth0", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + }, } for _, g := range grid { errs := validateNetworkingCalico(g.Input.Calico, g.Input.Etcd, field.NewPath("calico")) diff --git a/upup/models/bindata.go b/upup/models/bindata.go index f697879ce4d43..2343118eb0777 100644 --- a/upup/models/bindata.go +++ b/upup/models/bindata.go @@ -7590,9 +7590,9 @@ spec: - name: IP value: "autodetect" - name: IP_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}" - name: IP6_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" @@ -8709,9 +8709,9 @@ spec: - name: IP value: "autodetect" - name: IP_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}" - name: IP6_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" diff --git a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template index 75e6d18564161..6a7cfbf738d1a 100644 --- a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template @@ -790,9 +790,9 @@ spec: - name: IP value: "autodetect" - name: IP_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}" - name: IP6_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" diff --git a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template index 24c41196bc071..aae5bc6625331 100644 --- a/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template +++ b/upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.16.yaml.template @@ -801,9 +801,9 @@ spec: - name: IP value: "autodetect" - name: IP_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IPAutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}" - name: IP6_AUTODETECTION_METHOD - value: "{{- or .Networking.Calico.IP6AutoDetectionMethod "first-found" }}" + value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "first-found" }}" # Enable IPIP - name: CALICO_IPV4POOL_IPIP value: "{{- if and (eq .CloudProvider "aws") (.Networking.Calico.CrossSubnet) -}}CrossSubnet{{- else -}} {{- or .Networking.Calico.IPIPMode "Always" -}} {{- end -}}" diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go index 20bce40d7513b..446c8b7542b4b 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder.go @@ -725,8 +725,8 @@ func (b *BootstrapChannelBuilder) buildAddons() *channelsapi.Addons { versions := map[string]string{ "k8s-1.7": "2.6.12-kops.1", "k8s-1.7-v3": "3.8.0-kops.2", - "k8s-1.12": "3.9.5-kops.3", - "k8s-1.16": "3.13.3-kops.2", + "k8s-1.12": "3.9.5-kops.4", + "k8s-1.16": "3.13.3-kops.3", } { From 79275f9ea8d5605f0245fae05a21eb9cfd5602da Mon Sep 17 00:00:00 2001 From: Matt Long Date: Wed, 27 May 2020 15:43:08 +0200 Subject: [PATCH 3/8] Add additional tighter validation --- go.mod | 1 + pkg/apis/kops/validation/BUILD.bazel | 2 ++ pkg/apis/kops/validation/validation.go | 33 ++++++++++++++++----- pkg/apis/kops/validation/validation_test.go | 9 ++++++ 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 7a805e4183e7d..4de55aa7b46fa 100644 --- a/go.mod +++ b/go.mod @@ -100,6 +100,7 @@ require ( github.com/zclconf/go-cty v1.3.1 go.uber.org/zap v1.10.0 golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975 + golang.org/x/net v0.0.0-20200202094626-16171245cfb2 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 golang.org/x/sys v0.0.0-20191128015809-6d18c012aee9 golang.org/x/tools v0.0.0-20191203134012-c197fd4bf371 diff --git a/pkg/apis/kops/validation/BUILD.bazel b/pkg/apis/kops/validation/BUILD.bazel index d8a29e0a30133..9b0e03ca84efd 100644 --- a/pkg/apis/kops/validation/BUILD.bazel +++ b/pkg/apis/kops/validation/BUILD.bazel @@ -24,6 +24,8 @@ go_library( "//upup/pkg/fi/cloudup/awsup:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws/arn:go_default_library", "//vendor/github.com/blang/semver:go_default_library", + "//vendor/golang.org/x/net/ipv4:go_default_library", + "//vendor/golang.org/x/net/ipv6:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library", diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index e3e30019acd14..e4a207c8f76ae 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -23,11 +23,14 @@ import ( "strings" "github.com/blang/semver" + "golang.org/x/net/ipv4" + "golang.org/x/net/ipv6" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/api/validation" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/model/components" @@ -601,17 +604,17 @@ func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e *kops.EtcdClusterS } if v.IPv4AutoDetectionMethod != "" { - allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv4AutoDetectionMethod"), &v.IPv4AutoDetectionMethod)...) + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv4AutoDetectionMethod"), &v.IPv4AutoDetectionMethod, ipv4.Version)...) } if v.IPv6AutoDetectionMethod != "" { - allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv6AutoDetectionMethod"), &v.IPv6AutoDetectionMethod)...) + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv6AutoDetectionMethod"), &v.IPv6AutoDetectionMethod, ipv6.Version)...) } return allErrs } -func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string) field.ErrorList { +func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, version int) field.ErrorList { methodFirstFound := "first-found" methodCanReach := "can-reach=" methodInterface := "interface=" @@ -629,19 +632,26 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string) fie } else if strings.HasPrefix(*runtime, methodCanReach) { destStr := strings.TrimPrefix(*runtime, methodCanReach) - regex := regexp.MustCompile("\\s*").MatchString(destStr) - if !regex { - validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'can-reach='")) + if version == ipv4.Version { + return utilvalidation.IsValidIPv4Address(fldPath, destStr) + } else if version == ipv6.Version { + return utilvalidation.IsValidIPv6Address(fldPath, destStr) } - return validationError + + return field.ErrorList{field.Invalid(fldPath, runtime, "Invalid; IP version in incorrect")} } else if strings.HasPrefix(*runtime, methodInterface) { ifStr := strings.TrimPrefix(*runtime, methodInterface) ifRegexes := regexp.MustCompile("\\s*,\\s*").Split(ifStr, -1) - fmt.Printf("%+v %d\n", ifRegexes, len(ifRegexes)) if len(ifRegexes) == 0 || ifRegexes[0] == "" { validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'interface='")) } + for _, r := range ifRegexes { + _, e := regexp.Compile(r) + if e != nil { + validationError = append(validationError, field.Invalid(fldPath, runtime, "Invalid regexp: "+e.Error())) + } + } return validationError } else if strings.HasPrefix(*runtime, methodSkipInterface) { @@ -650,6 +660,13 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string) fie if len(ifRegexes) == 0 || ifRegexes[0] == "" { validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'skip-interface='")) } + for _, r := range ifRegexes { + _, e := regexp.Compile(r) + if e != nil { + validationError = append(validationError, field.Invalid(fldPath, runtime, "Invalid regexp: "+e.Error())) + } + } + return validationError } return field.ErrorList{field.Invalid(fldPath, runtime, "Invalid autodetection method")} diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index 71bacec0ff483..dc40aa06daf2a 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -471,6 +471,15 @@ func Test_Validate_Calico(t *testing.T) { Etcd: &kops.EtcdClusterSpec{}, }, }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "interface=(,en1", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"}, + }, } for _, g := range grid { errs := validateNetworkingCalico(g.Input.Calico, g.Input.Etcd, field.NewPath("calico")) From 1fea54bc3c56c0770d018c1f999ea81cd0b6b12e Mon Sep 17 00:00:00 2001 From: Matt Long Date: Wed, 27 May 2020 15:44:57 +0200 Subject: [PATCH 4/8] Update regex in test --- pkg/apis/kops/validation/validation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index dc40aa06daf2a..37675f6a9ae95 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -458,7 +458,7 @@ func Test_Validate_Calico(t *testing.T) { { Input: caliInput{ Calico: &kops.CalicoNetworkingSpec{ - IPv4AutoDetectionMethod: "interface=en*,eth0", + IPv4AutoDetectionMethod: "interface=en.*,eth0", }, Etcd: &kops.EtcdClusterSpec{}, }, @@ -466,7 +466,7 @@ func Test_Validate_Calico(t *testing.T) { { Input: caliInput{ Calico: &kops.CalicoNetworkingSpec{ - IPv6AutoDetectionMethod: "skip-interface=en*,eth0", + IPv6AutoDetectionMethod: "skip-interface=en.*,eth0", }, Etcd: &kops.EtcdClusterSpec{}, }, From 2317b77ba59bc3bddbacf698a832b9888420d61c Mon Sep 17 00:00:00 2001 From: Matt Long Date: Wed, 27 May 2020 15:51:55 +0200 Subject: [PATCH 5/8] Update with static check suggestions --- pkg/apis/kops/validation/validation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index e4a207c8f76ae..51612df5b0edc 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -642,7 +642,7 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, ver } else if strings.HasPrefix(*runtime, methodInterface) { ifStr := strings.TrimPrefix(*runtime, methodInterface) - ifRegexes := regexp.MustCompile("\\s*,\\s*").Split(ifStr, -1) + ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(ifStr, -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" { validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'interface='")) } @@ -656,7 +656,7 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, ver } else if strings.HasPrefix(*runtime, methodSkipInterface) { ifStr := strings.TrimPrefix(*runtime, methodSkipInterface) - ifRegexes := regexp.MustCompile("\\s*,\\s*").Split(ifStr, -1) + ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(ifStr, -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" { validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'skip-interface='")) } From 9385b1adf98dc6d91e5226bab0e73746810ec8a0 Mon Sep 17 00:00:00 2001 From: Matt Long Date: Thu, 28 May 2020 08:47:52 +0200 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: John Gardiner Myers --- pkg/apis/kops/validation/validation.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 51612df5b0edc..97cd6acbbd548 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -638,13 +638,13 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, ver return utilvalidation.IsValidIPv6Address(fldPath, destStr) } - return field.ErrorList{field.Invalid(fldPath, runtime, "Invalid; IP version in incorrect")} + return field.ErrorList{field.InternalError(fldPath, runtime, "IP version is incorrect")} } else if strings.HasPrefix(*runtime, methodInterface) { ifStr := strings.TrimPrefix(*runtime, methodInterface) ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(ifStr, -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" { - validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'interface='")) + validationError = append(validationError, field.Invalid(fldPath, runtime, "'interface=' must be followed by a comma separated list of interface regular expressions")) } for _, r := range ifRegexes { _, e := regexp.Compile(r) @@ -669,7 +669,7 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, ver return validationError } - return field.ErrorList{field.Invalid(fldPath, runtime, "Invalid autodetection method")} + return field.ErrorList{field.Invalid(fldPath, runtime, "unsupported autodetection method")} } func validateContainerRuntime(runtime *string, fldPath *field.Path) field.ErrorList { From b983af231e85e54e673b8302f042329a4740a510 Mon Sep 17 00:00:00 2001 From: Matt Long Date: Thu, 28 May 2020 11:13:56 +0200 Subject: [PATCH 7/8] Update validation per code review comments --- pkg/apis/kops/validation/validation.go | 51 +++++++++++---------- pkg/apis/kops/validation/validation_test.go | 18 ++++++++ 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 97cd6acbbd548..2b4c6fb3a5901 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -604,21 +604,17 @@ func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e *kops.EtcdClusterS } if v.IPv4AutoDetectionMethod != "" { - allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv4AutoDetectionMethod"), &v.IPv4AutoDetectionMethod, ipv4.Version)...) + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv4AutoDetectionMethod"), v.IPv4AutoDetectionMethod, ipv4.Version)...) } if v.IPv6AutoDetectionMethod != "" { - allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv6AutoDetectionMethod"), &v.IPv6AutoDetectionMethod, ipv6.Version)...) + allErrs = append(allErrs, validateCalicoAutoDetectionMethod(fldPath.Child("ipv6AutoDetectionMethod"), v.IPv6AutoDetectionMethod, ipv6.Version)...) } return allErrs } -func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, version int) field.ErrorList { - methodFirstFound := "first-found" - methodCanReach := "can-reach=" - methodInterface := "interface=" - methodSkipInterface := "skip-interface=" +func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime string, version int) field.ErrorList { validationError := field.ErrorList{} // validation code is based on the checks in calico/node startup code @@ -627,49 +623,54 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime *string, ver // // We won't do deep validation of the values in this check, since they can // be actual interface names or regexes - if *runtime == methodFirstFound { - return field.ErrorList{} + method := strings.Split(runtime, "=") + if len(method) == 0 { + return field.ErrorList{field.Invalid(fldPath, runtime, "missing autodetection method")} + } + if len(method) > 2 { + return field.ErrorList{field.Invalid(fldPath, runtime, "malformed autodetection method")} + } - } else if strings.HasPrefix(*runtime, methodCanReach) { - destStr := strings.TrimPrefix(*runtime, methodCanReach) + // 'method' should contain something like "[interface eth0,en.*]" or "[first-found]" + switch method[0] { + case "first-found": + return nil + case "can-reach": + destStr := method[1] if version == ipv4.Version { return utilvalidation.IsValidIPv4Address(fldPath, destStr) } else if version == ipv6.Version { return utilvalidation.IsValidIPv6Address(fldPath, destStr) } - return field.ErrorList{field.InternalError(fldPath, runtime, "IP version is incorrect")} - - } else if strings.HasPrefix(*runtime, methodInterface) { - ifStr := strings.TrimPrefix(*runtime, methodInterface) - ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(ifStr, -1) + return field.ErrorList{field.Invalid(fldPath, runtime, "IP version is incorrect")} + case "interface": + ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(method[1], -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" { validationError = append(validationError, field.Invalid(fldPath, runtime, "'interface=' must be followed by a comma separated list of interface regular expressions")) } for _, r := range ifRegexes { _, e := regexp.Compile(r) if e != nil { - validationError = append(validationError, field.Invalid(fldPath, runtime, "Invalid regexp: "+e.Error())) + validationError = append(validationError, field.Invalid(fldPath, runtime, fmt.Sprintf("regexp %s does not compile: %s", r, e.Error()))) } } return validationError - - } else if strings.HasPrefix(*runtime, methodSkipInterface) { - ifStr := strings.TrimPrefix(*runtime, methodSkipInterface) - ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(ifStr, -1) + case "skip-interface": + ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(method[1], -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" { - validationError = append(validationError, field.Invalid(fldPath, runtime, "Expected 'skip-interface='")) + validationError = append(validationError, field.Invalid(fldPath, runtime, "'skip-interface=' must be followed by a comma separated list of interface regular expressions")) } for _, r := range ifRegexes { _, e := regexp.Compile(r) if e != nil { - validationError = append(validationError, field.Invalid(fldPath, runtime, "Invalid regexp: "+e.Error())) + validationError = append(validationError, field.Invalid(fldPath, runtime, fmt.Sprintf("regexp %s does not compile: %s", r, e.Error()))) } } - return validationError + default: + return field.ErrorList{field.Invalid(fldPath, runtime, "unsupported autodetection method")} } - return field.ErrorList{field.Invalid(fldPath, runtime, "unsupported autodetection method")} } func validateContainerRuntime(runtime *string, fldPath *field.Path) field.ErrorList { diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index 37675f6a9ae95..fe7f36c174382 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -480,6 +480,24 @@ func Test_Validate_Calico(t *testing.T) { }, ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"}, }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "interface=foo=bar", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"}, + }, + { + Input: caliInput{ + Calico: &kops.CalicoNetworkingSpec{ + IPv4AutoDetectionMethod: "=en0,eth.*", + }, + Etcd: &kops.EtcdClusterSpec{}, + }, + ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"}, + }, } for _, g := range grid { errs := validateNetworkingCalico(g.Input.Calico, g.Input.Etcd, field.NewPath("calico")) From de1d082bc5373e18907833801c51952f22e2cfd2 Mon Sep 17 00:00:00 2001 From: Matt Long Date: Fri, 29 May 2020 09:55:46 +0200 Subject: [PATCH 8/8] Change error to InernalError --- pkg/apis/kops/validation/validation.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 2b4c6fb3a5901..221a1063630f0 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -17,6 +17,7 @@ limitations under the License. package validation import ( + "errors" "fmt" "net" "regexp" @@ -643,7 +644,7 @@ func validateCalicoAutoDetectionMethod(fldPath *field.Path, runtime string, vers return utilvalidation.IsValidIPv6Address(fldPath, destStr) } - return field.ErrorList{field.Invalid(fldPath, runtime, "IP version is incorrect")} + return field.ErrorList{field.InternalError(fldPath, errors.New("IP version is incorrect"))} case "interface": ifRegexes := regexp.MustCompile(`\s*,\s*`).Split(method[1], -1) if len(ifRegexes) == 0 || ifRegexes[0] == "" {