diff --git a/pkg/apis/kops/validation/legacy.go b/pkg/apis/kops/validation/legacy.go index 03646675b3..172ef61af9 100644 --- a/pkg/apis/kops/validation/legacy.go +++ b/pkg/apis/kops/validation/legacy.go @@ -73,82 +73,15 @@ func ValidateCluster(c *kops.Cluster, strict bool) field.ErrorList { allErrs = append(allErrs, field.Required(fieldSpec.Child("docker"), "docker not configured")) } - var networkCIDR *net.IPNet + var nonMasqueradeCIDR *net.IPNet + var serviceClusterIPRange *net.IPNet var err error - if c.Spec.Networking.NetworkCIDR != "" { - _, networkCIDR, _ = net.ParseCIDR(c.Spec.Networking.NetworkCIDR) + if c.Spec.Networking.NonMasqueradeCIDR != "" { + _, nonMasqueradeCIDR, _ = net.ParseCIDR(c.Spec.Networking.NonMasqueradeCIDR) } - - // nonMasqueradeCIDR is essentially deprecated, and we're moving to cluster-cidr instead (which is better named pod-cidr) - nonMasqueradeCIDRRequired := true - serviceClusterMustBeSubnetOfNonMasqueradeCIDR := true - if c.Spec.Networking.GCE != nil { - nonMasqueradeCIDRRequired = false - serviceClusterMustBeSubnetOfNonMasqueradeCIDR = false - } - - // Check NonMasqueradeCIDR - var nonMasqueradeCIDR *net.IPNet - { - nonMasqueradeCIDRString := c.Spec.Networking.NonMasqueradeCIDR - if nonMasqueradeCIDRString == "" { - if nonMasqueradeCIDRRequired { - allErrs = append(allErrs, field.Required(fieldSpec.Child("networking", "nonMasqueradeCIDR"), "Cluster did not have nonMasqueradeCIDR set")) - } - } else { - _, nonMasqueradeCIDR, err = net.ParseCIDR(nonMasqueradeCIDRString) - if err != nil { - allErrs = append(allErrs, field.Invalid(fieldSpec.Child("networking", "nonMasqueradeCIDR"), nonMasqueradeCIDRString, "Cluster had an invalid nonMasqueradeCIDR")) - } else { - if strings.Contains(nonMasqueradeCIDRString, ":") && nonMasqueradeCIDRString != "::/0" { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("networking", "nonMasqueradeCIDR"), "IPv6 clusters must have a nonMasqueradeCIDR of \"::/0\"")) - } - - if networkCIDR != nil && subnet.Overlap(nonMasqueradeCIDR, networkCIDR) && c.Spec.Networking.AmazonVPC == nil && (c.Spec.Networking.Cilium == nil || c.Spec.Networking.Cilium.IPAM != kops.CiliumIpamEni) { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("networking", "nonMasqueradeCIDR"), fmt.Sprintf("nonMasqueradeCIDR %q cannot overlap with networkCIDR %q", nonMasqueradeCIDRString, c.Spec.Networking.NetworkCIDR))) - } - - if c.Spec.ContainerRuntime == "docker" && c.Spec.Kubelet != nil && fi.ValueOf(c.Spec.Kubelet.NetworkPluginName) == "kubenet" { - if fi.ValueOf(c.Spec.Kubelet.NonMasqueradeCIDR) != nonMasqueradeCIDRString { - if strict || c.Spec.Kubelet.NonMasqueradeCIDR != nil { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubelet", "networking", "nonMasqueradeCIDR"), "kubelet nonMasqueradeCIDR did not match cluster nonMasqueradeCIDR")) - } - } - if fi.ValueOf(c.Spec.ControlPlaneKubelet.NonMasqueradeCIDR) != nonMasqueradeCIDRString { - if strict || c.Spec.ControlPlaneKubelet.NonMasqueradeCIDR != nil { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("controlPlaneKubelet", "nonMasqueradeCIDR"), "controlPlaneKubelet nonMasqueradeCIDR did not match cluster nonMasqueradeCIDR")) - } - } - } - } - } - } - - // Check ServiceClusterIPRange - var serviceClusterIPRange *net.IPNet - { - serviceClusterIPRangeString := c.Spec.Networking.ServiceClusterIPRange - if serviceClusterIPRangeString == "" { - if strict { - allErrs = append(allErrs, field.Required(fieldSpec.Child("networking", "serviceClusterIPRange"), "Cluster did not have serviceClusterIPRange set")) - } - } else { - _, serviceClusterIPRange, err = net.ParseCIDR(serviceClusterIPRangeString) - if err != nil { - allErrs = append(allErrs, field.Invalid(fieldSpec.Child("networking", "serviceClusterIPRange"), serviceClusterIPRangeString, "Cluster had an invalid serviceClusterIPRange")) - } else { - if nonMasqueradeCIDR != nil && serviceClusterMustBeSubnetOfNonMasqueradeCIDR && !subnet.BelongsTo(nonMasqueradeCIDR, serviceClusterIPRange) { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("networking", "serviceClusterIPRange"), fmt.Sprintf("serviceClusterIPRange %q must be a subnet of nonMasqueradeCIDR %q", serviceClusterIPRangeString, c.Spec.Networking.NonMasqueradeCIDR))) - } - - if c.Spec.KubeAPIServer != nil && c.Spec.KubeAPIServer.ServiceClusterIPRange != serviceClusterIPRangeString { - if strict || c.Spec.KubeAPIServer.ServiceClusterIPRange != "" { - allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubeAPIServer", "serviceClusterIPRange"), "kubeAPIServer serviceClusterIPRange did not match cluster serviceClusterIPRange")) - } - } - } - } + if c.Spec.Networking.ServiceClusterIPRange != "" { + _, serviceClusterIPRange, _ = net.ParseCIDR(c.Spec.Networking.ServiceClusterIPRange) } // Check ClusterCIDR diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 24bb45da9c..28071d1b1e 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kops/pkg/util/subnet" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/featureflag" @@ -127,7 +128,7 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie } if spec.KubeAPIServer != nil { - allErrs = append(allErrs, validateKubeAPIServer(spec.KubeAPIServer, c, fieldPath.Child("kubeAPIServer"))...) + allErrs = append(allErrs, validateKubeAPIServer(spec.KubeAPIServer, c, fieldPath.Child("kubeAPIServer"), strict)...) } if spec.ExternalCloudControllerManager == nil && spec.IsIPv6Only() { @@ -290,17 +291,21 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie } type cloudProviderConstraints struct { - requiresSubnets bool - requiresNetworkCIDR bool - prohibitsNetworkCIDR bool - requiresSubnetCIDR bool + requiresSubnets bool + requiresNetworkCIDR bool + prohibitsNetworkCIDR bool + requiresNonMasqueradeCIDR bool + requiresServiceClusterSubnetOfNonMasqueradeCIDR bool + requiresSubnetCIDR bool } func validateCloudProvider(c *kops.Cluster, provider *kops.CloudProviderSpec, fieldSpec *field.Path) (allErrs field.ErrorList, constraints *cloudProviderConstraints) { constraints = &cloudProviderConstraints{ - requiresSubnets: true, - requiresNetworkCIDR: true, - requiresSubnetCIDR: true, + requiresSubnets: true, + requiresNetworkCIDR: true, + requiresNonMasqueradeCIDR: true, + requiresServiceClusterSubnetOfNonMasqueradeCIDR: true, + requiresSubnetCIDR: true, } optionTaken := false @@ -335,6 +340,8 @@ func validateCloudProvider(c *kops.Cluster, provider *kops.CloudProviderSpec, fi if c.Spec.Networking.NetworkCIDR != "" { allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("networking", "networkCIDR"), "networkCIDR should not be set on GCE")) } + constraints.requiresNonMasqueradeCIDR = false + constraints.requiresServiceClusterSubnetOfNonMasqueradeCIDR = false } if c.Spec.CloudProvider.Hetzner != nil { if optionTaken { @@ -679,7 +686,7 @@ func validateExecContainerAction(v *kops.ExecContainerAction, fldPath *field.Pat return allErrs } -func validateKubeAPIServer(v *kops.KubeAPIServerConfig, c *kops.Cluster, fldPath *field.Path) field.ErrorList { +func validateKubeAPIServer(v *kops.KubeAPIServerConfig, c *kops.Cluster, fldPath *field.Path, strict bool) field.ErrorList { allErrs := field.ErrorList{} if len(v.AdmissionControl) > 0 { @@ -774,6 +781,12 @@ func validateKubeAPIServer(v *kops.KubeAPIServerConfig, c *kops.Cluster, fldPath } } + if v.ServiceClusterIPRange != c.Spec.Networking.ServiceClusterIPRange { + if strict || v.ServiceClusterIPRange != "" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceClusterIPRange"), "kubeAPIServer serviceClusterIPRange did not match cluster serviceClusterIPRange")) + } + } + return allErrs } @@ -865,6 +878,10 @@ func validateKubelet(k *kops.KubeletConfigSpec, c *kops.Cluster, kubeletPath *fi if k.NonMasqueradeCIDR != nil { allErrs = append(allErrs, field.Forbidden(kubeletPath.Child("nonMasqueradeCIDR"), "nonMasqueradeCIDR has been removed on Kubernetes >=1.24")) } + } else { + if c.Spec.ContainerRuntime == "docker" && fi.ValueOf(c.Spec.Kubelet.NetworkPluginName) == "kubenet" && fi.ValueOf(k.NonMasqueradeCIDR) != c.Spec.Networking.NonMasqueradeCIDR { + allErrs = append(allErrs, field.Forbidden(kubeletPath.Child("nonMasqueradeCIDR"), "kubelet nonMasqueradeCIDR does not match cluster nonMasqueradeCIDR")) + } } if k.ShutdownGracePeriodCriticalPods != nil { @@ -907,6 +924,47 @@ func validateNetworking(cluster *kops.Cluster, v *kops.NetworkingSpec, fldPath * allErrs = append(allErrs, validateCIDR(cidr, fldPath.Child("additionalNetworkCIDRs").Index(i), &networkCIDRs)...) } + var nonMasqueradeCIDRs []*net.IPNet + { + if v.NonMasqueradeCIDR == "" { + if providerConstraints.requiresNonMasqueradeCIDR { + allErrs = append(allErrs, field.Required(fldPath.Child("nonMasqueradeCIDR"), "Cluster does not have nonMasqueradeCIDR set")) + } + } else { + allErrs = append(allErrs, validateCIDR(v.NonMasqueradeCIDR, fldPath.Child("nonMasqueradeCIDR"), &nonMasqueradeCIDRs)...) + + if strings.Contains(v.NonMasqueradeCIDR, ":") && v.NonMasqueradeCIDR != "::/0" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("nonMasqueradeCIDR"), "IPv6 clusters must have a nonMasqueradeCIDR of \"::/0\"")) + } + + if len(networkCIDRs) > 0 && v.AmazonVPC == nil && (v.Cilium == nil || v.Cilium.IPAM != kops.CiliumIpamEni) { + if subnet.Overlap(nonMasqueradeCIDRs[0], networkCIDRs[0]) { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("nonMasqueradeCIDR"), fmt.Sprintf("nonMasqueradeCIDR %q cannot overlap with networkCIDR %q", v.NonMasqueradeCIDR, v.NetworkCIDR))) + } + for i, cidr := range networkCIDRs[1:] { + if subnet.Overlap(nonMasqueradeCIDRs[0], cidr) { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("nonMasqueradeCIDR"), fmt.Sprintf("nonMasqueradeCIDR %q cannot overlap with additionalNetworkCIDRs[%d] %q", v.NonMasqueradeCIDR, i, cidr))) + } + } + } + } + } + + var serviceClusterIPRanges []*net.IPNet + { + if v.ServiceClusterIPRange == "" { + if strict { + allErrs = append(allErrs, field.Required(fldPath.Child("serviceClusterIPRange"), "Cluster did not have serviceClusterIPRange set")) + } + } else { + allErrs = append(allErrs, validateCIDR(v.ServiceClusterIPRange, fldPath.Child("serviceClusterIPRange"), &serviceClusterIPRanges)...) + + if len(nonMasqueradeCIDRs) > 0 && providerConstraints.requiresServiceClusterSubnetOfNonMasqueradeCIDR && !subnet.BelongsTo(nonMasqueradeCIDRs[0], serviceClusterIPRanges[0]) { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceClusterIPRange"), fmt.Sprintf("serviceClusterIPRange %q must be a subnet of nonMasqueradeCIDR %q", v.ServiceClusterIPRange, v.NonMasqueradeCIDR))) + } + } + } + allErrs = append(allErrs, validateSubnets(&cluster.Spec, v.Subnets, fldPath.Child("subnets"), strict, providerConstraints, networkCIDRs)...) if v.Topology != nil { diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go index b9e82d53be..2c56d46f48 100644 --- a/pkg/apis/kops/validation/validation_test.go +++ b/pkg/apis/kops/validation/validation_test.go @@ -317,7 +317,7 @@ func TestValidateKubeAPIServer(t *testing.T) { }, } } - errs := validateKubeAPIServer(&g.Input, g.Cluster, field.NewPath("KubeAPIServer")) + errs := validateKubeAPIServer(&g.Input, g.Cluster, field.NewPath("KubeAPIServer"), true) testErrors(t, g.Input, errs, g.ExpectedErrors) @@ -392,7 +392,9 @@ func Test_Validate_Networking_Flannel(t *testing.T) { cluster := &kops.Cluster{ Spec: kops.ClusterSpec{ Networking: kops.NetworkingSpec{ - NetworkCIDR: "10.0.0.0/8", + NetworkCIDR: "10.0.0.0/8", + NonMasqueradeCIDR: "100.64.0.0/10", + ServiceClusterIPRange: "100.64.0.0/13", Subnets: []kops.ClusterSubnetSpec{ { Name: "sg-test", @@ -456,7 +458,9 @@ func Test_Validate_AdditionalPolicies(t *testing.T) { AWS: &kops.AWSSpec{}, }, Networking: kops.NetworkingSpec{ - NetworkCIDR: "10.10.0.0/16", + NetworkCIDR: "10.10.0.0/16", + NonMasqueradeCIDR: "100.64.0.0/10", + ServiceClusterIPRange: "100.64.0.0/13", Subnets: []kops.ClusterSubnetSpec{ { Name: "subnet1",