Apply suggestions from code review

This commit is contained in:
Ciprian Hacman 2021-10-30 19:49:24 +03:00
parent 76898881cb
commit b6565d86a2
3 changed files with 95 additions and 58 deletions

View File

@ -142,7 +142,7 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
if spec.Networking != nil {
allErrs = append(allErrs, validateNetworking(c, spec.Networking, fieldPath.Child("networking"))...)
if spec.Networking.Calico != nil {
allErrs = append(allErrs, validateNetworkingCalico(spec.Networking.Calico, spec.EtcdClusters[0], fieldPath.Child("networking", "calico"))...)
allErrs = append(allErrs, validateNetworkingCalico(&c.Spec, spec.Networking.Calico, fieldPath.Child("networking", "calico"))...)
}
}
@ -1080,7 +1080,7 @@ func validateEtcdMemberSpec(spec kops.EtcdMemberSpec, fieldPath *field.Path) fie
return allErrs
}
func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e kops.EtcdClusterSpec, fldPath *field.Path) field.ErrorList {
func validateNetworkingCalico(c *kops.ClusterSpec, v *kops.CalicoNetworkingSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if v.AWSSrcDstCheck != "" {
@ -1110,8 +1110,18 @@ func validateNetworkingCalico(v *kops.CalicoNetworkingSpec, e kops.EtcdClusterSp
}
if v.EncapsulationMode != "" {
valid := []string{"ipip", "vxlan", "none"}
allErrs = append(allErrs, IsValidValue(fldPath.Child("encapsulationMode"), &v.EncapsulationMode, valid)...)
if c.IsIPv6Only() {
// IPv6 doesn't support encapsulation and kops only uses the "none" networking backend.
// The bird networking backend could also be added in the future if there's any valid use case.
valid := []string{"none"}
allErrs = append(allErrs, IsValidValue(fldPath.Child("encapsulationMode"), &v.EncapsulationMode, valid)...)
} else {
// Don't tolerate "None" for now, which would disable encapsulation in the default IPPool
// object. Note that with no encapsulation, we'd need to select the "bird" networking
// backend in order to allow use of BGP to distribute routes for pod traffic.
valid := []string{"ipip", "vxlan"}
allErrs = append(allErrs, IsValidValue(fldPath.Child("encapsulationMode"), &v.EncapsulationMode, valid)...)
}
}
if v.IPIPMode != "" {

View File

@ -451,8 +451,8 @@ func Test_Validate_AdditionalPolicies(t *testing.T) {
}
type caliInput struct {
Calico *kops.CalicoNetworkingSpec
Etcd kops.EtcdClusterSpec
Cluster *kops.ClusterSpec
Calico *kops.CalicoNetworkingSpec
}
func Test_Validate_Calico(t *testing.T) {
@ -465,7 +465,6 @@ func Test_Validate_Calico(t *testing.T) {
Description: "empty specs",
Input: caliInput{
Calico: &kops.CalicoNetworkingSpec{},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -474,7 +473,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
TyphaReplicas: 3,
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -483,7 +481,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
TyphaReplicas: -1,
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.typhaReplicas"},
},
@ -491,9 +488,6 @@ func Test_Validate_Calico(t *testing.T) {
Description: "with etcd version",
Input: caliInput{
Calico: &kops.CalicoNetworkingSpec{},
Etcd: kops.EtcdClusterSpec{
Version: "3.2.18",
},
},
},
{
@ -502,7 +496,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "first-found",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -511,7 +504,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv6AutoDetectionMethod: "first-found",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -520,7 +512,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "can-reach=8.8.8.8",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -529,7 +520,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv6AutoDetectionMethod: "can-reach=2001:4860:4860::8888",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -538,7 +528,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "bogus",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"},
},
@ -548,7 +537,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv6AutoDetectionMethod: "bogus",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv6AutoDetectionMethod"},
},
@ -558,7 +546,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv6AutoDetectionMethod: "interface=",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv6AutoDetectionMethod"},
},
@ -568,7 +555,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "interface=en.*,eth0",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -577,7 +563,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv6AutoDetectionMethod: "skip-interface=en.*,eth0",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -586,7 +571,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "interface=(,en1",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"},
},
@ -596,7 +580,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "interface=foo=bar",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"},
},
@ -606,7 +589,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPv4AutoDetectionMethod: "=en0,eth.*",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Invalid value::calico.ipv4AutoDetectionMethod"},
},
@ -616,7 +598,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
AWSSrcDstCheck: "off",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Unsupported value::calico.awsSrcDstCheck"},
},
@ -626,7 +607,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
AWSSrcDstCheck: "Enable",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -635,7 +615,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
AWSSrcDstCheck: "Disable",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -644,16 +623,41 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
AWSSrcDstCheck: "DoNothing",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "unknown Calico encapsulation mode",
Input: caliInput{
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "None",
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "none",
},
},
ExpectedErrors: []string{"Unsupported value::calico.encapsulationMode"},
},
{
Description: "unknown Calico encapsulation mode IPIP for IPv6",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "::/0",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "ipip",
},
},
ExpectedErrors: []string{"Unsupported value::calico.encapsulationMode"},
},
{
Description: "unknown Calico encapsulation mode VXLAN for IPv6",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "::/0",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "vxlan",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Unsupported value::calico.encapsulationMode"},
},
@ -663,7 +667,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPIPMode: "unknown",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{"Unsupported value::calico.ipipMode"},
},
@ -675,7 +678,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPIPMode: "Always",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -684,7 +686,6 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPIPMode: "CrossSubnet",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
@ -693,87 +694,113 @@ func Test_Validate_Calico(t *testing.T) {
Calico: &kops.CalicoNetworkingSpec{
IPIPMode: "Never",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "Calico IPIP encapsulation mode (explicit) with IPIP IPPool mode (always)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "ipip",
IPIPMode: "Always",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "Calico IPIP encapsulation mode (explicit) with IPIP IPPool mode (cross-subnet)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "ipip",
IPIPMode: "CrossSubnet",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "Calico IPIP encapsulation mode (explicit) with IPIP IPPool mode (never)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "ipip",
IPIPMode: "Never",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "Calico VXLAN encapsulation mode with IPIP IPPool mode",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "vxlan",
IPIPMode: "Always",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{`Forbidden::calico.ipipMode`},
},
{
Description: "Calico VXLAN encapsulation mode with IPIP IPPool mode (always)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "vxlan",
IPIPMode: "Always",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{`Forbidden::calico.ipipMode`},
},
{
Description: "Calico VXLAN encapsulation mode with IPIP IPPool mode (cross-subnet)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "vxlan",
IPIPMode: "CrossSubnet",
},
Etcd: kops.EtcdClusterSpec{},
},
ExpectedErrors: []string{`Forbidden::calico.ipipMode`},
},
{
Description: "Calico VXLAN encapsulation mode with IPIP IPPool mode (never)",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "100.64.0.0/10",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "vxlan",
IPIPMode: "Never",
},
Etcd: kops.EtcdClusterSpec{},
},
},
{
Description: "Calico IPv6 without encapsulation",
Input: caliInput{
Cluster: &kops.ClusterSpec{
NonMasqueradeCIDR: "::/0",
},
Calico: &kops.CalicoNetworkingSpec{
EncapsulationMode: "none",
IPIPMode: "Never",
VXLANMode: "Never",
},
},
},
}
rootFieldPath := field.NewPath("calico")
for _, g := range grid {
t.Run(g.Description, func(t *testing.T) {
errs := validateNetworkingCalico(g.Input.Calico, g.Input.Etcd, rootFieldPath)
errs := validateNetworkingCalico(g.Input.Cluster, g.Input.Calico, rootFieldPath)
testErrors(t, g.Input, errs, g.ExpectedErrors)
})
}

View File

@ -54,11 +54,11 @@ data:
"ipam": {
"assign_ipv4": "{{ not IsIPv6Only }}",
"assign_ipv6": "{{ IsIPv6Only }}",
{{- if not IsIPv6Only }}
"type": "calico-ipam"
{{- else }}
{{- if IsIPv6Only }}
"type": "host-local",
"ranges": [[{ "subnet": "usePodCidr" }]]
{{- else }}
"type": "calico-ipam"
{{- end }}
},
"policy": {
@ -4037,16 +4037,16 @@ spec:
value: "{{- if not IsIPv6Only -}}autodetect{{- else -}}none{{- end -}}"
- name: IP6
value: "{{- if IsIPv6Only -}}autodetect{{- else -}}none{{- end -}}"
{{- if not IsIPv6Only }}
- name: IP_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}"
- name: IP6_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "none" }}"
{{- else }}
{{- if IsIPv6Only }}
- name: IP_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "none" }}"
- name: IP6_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "first-found" }}"
{{- else }}
- name: IP_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv4AutoDetectionMethod "first-found" }}"
- name: IP6_AUTODETECTION_METHOD
value: "{{- or .Networking.Calico.IPv6AutoDetectionMethod "none" }}"
{{- end }}
# Enable IPIP
- name: CALICO_IPV4POOL_IPIP
@ -4151,10 +4151,10 @@ spec:
- /bin/calico-node
- -felix-live
{{- if eq .Networking.Calico.EncapsulationMode "ipip" }}
{{- if not IsIPv6Only }}
- -bird-live
{{- else }}
{{- if IsIPv6Only }}
- -bird6-live
{{- else }}
- -bird-live
{{- end }}
{{- end }}
periodSeconds: 10
@ -4167,10 +4167,10 @@ spec:
- /bin/calico-node
- -felix-ready
{{- if eq .Networking.Calico.EncapsulationMode "ipip" }}
{{- if not IsIPv6Only }}
- -bird-ready
{{- else }}
{{- if IsIPv6Only }}
- -bird6-ready
{{- else }}
- -bird-ready
{{- end }}
{{- end }}
periodSeconds: 10