Merge pull request #15872 from johngmyers/aws-ccm-always

Remove dead code for in-tree CCM
kubernetes · Sep 5, 2023 · 92c0f48 · 92c0f48
2 parents efab9bc + edd3d0b
commit 92c0f48
Show file tree

Hide file tree

Showing 555 changed files with 384 additions and 790 deletions.
diff --git a/cmd/kops-controller/pkg/config/options.go b/cmd/kops-controller/pkg/config/options.go
@@ -62,9 +62,6 @@ type ServerOptions struct {
 	SigningCAs []string `json:"signingCAs"`
 	// CertNames is the list of active certificate names.
 	CertNames []string `json:"certNames"`
-
-	// UseInstanceIDForNodeName uses the instance ID instead of the hostname for the node name.
-	UseInstanceIDForNodeName bool `json:"useInstanceIDForNodeName,omitempty"`
 }
 
 type ServerProviderOptions struct {

diff --git a/cmd/kops-controller/pkg/server/server.go b/cmd/kops-controller/pkg/server/server.go
@@ -157,7 +157,7 @@ func (s *Server) bootstrap(w http.ResponseWriter, r *http.Request) {
 
 	ctx := r.Context()
 
-	id, err := s.verifier.VerifyToken(ctx, r, r.Header.Get("Authorization"), body, s.opt.Server.UseInstanceIDForNodeName)
+	id, err := s.verifier.VerifyToken(ctx, r, r.Header.Get("Authorization"), body)
 	if err != nil {
 		// means that we should exit nodeup gracefully
 		if err == bootstrap.ErrAlreadyExists {

diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml
@@ -325,8 +325,8 @@ spec:
                       driver
                     properties:
                       enabled:
-                        description: 'Enabled enables the AWS EBS CSI driver Default:
-                          false'
+                        description: 'Enabled enables the AWS EBS CSI driver. Can
+                          only be set to true. Default: true'
                         type: boolean
                       managed:
                         description: Managed controls if aws-ebs-csi-driver is manged

diff --git a/nodeup/pkg/model/kubelet.go b/nodeup/pkg/model/kubelet.go
@@ -726,7 +726,7 @@ func (b *KubeletBuilder) kubeletNames() ([]string, error) {
 		return nil, fmt.Errorf("error describing instances: %v", err)
 	}
 
-	return awsup.GetInstanceCertificateNames(result, b.NodeupConfig.UseInstanceIDForNodeName)
+	return awsup.GetInstanceCertificateNames(result)
 }
 
 func (b *KubeletBuilder) buildCgroupService(name string) *nodetasks.Service {

diff --git a/pkg/apis/kops/componentconfig.go b/pkg/apis/kops/componentconfig.go
@@ -883,9 +883,9 @@ type CloudConfiguration struct {
 
 // EBSCSIDriverSpec is the config for the AWS EBS CSI driver
 type EBSCSIDriverSpec struct {
-	// Enabled enables the AWS EBS CSI driver
-	// Default: false
-	Enabled *bool `json:"enabled,omitempty"`
+	// Enabled enables the AWS EBS CSI driver. Can only be set to true.
+	// Default: true
+	Enabled *bool `json:"-"`
 
 	// Managed controls if aws-ebs-csi-driver is manged and deployed by kOps.
 	// The deployment of aws-ebs-csi-driver is skipped if this is set to false.

diff --git a/pkg/apis/kops/v1alpha2/componentconfig.go b/pkg/apis/kops/v1alpha2/componentconfig.go
@@ -950,8 +950,8 @@ type CloudConfiguration struct {
 
 // EBSCSIDriverSpec is the config for the AWS EBS CSI driver
 type EBSCSIDriverSpec struct {
-	// Enabled enables the AWS EBS CSI driver
-	// Default: false
+	// Enabled enables the AWS EBS CSI driver. Can only be set to true.
+	// Default: true
 	Enabled *bool `json:"enabled,omitempty"`
 
 	// Managed controls if aws-ebs-csi-driver is manged and deployed by kOps.

diff --git a/pkg/apis/kops/v1alpha3/componentconfig.go b/pkg/apis/kops/v1alpha3/componentconfig.go
@@ -880,9 +880,8 @@ type CloudConfiguration struct {
 
 // EBSCSIDriverSpec is the config for the AWS EBS CSI driver
 type EBSCSIDriverSpec struct {
-	// Enabled enables the AWS EBS CSI driver
-	// Default: false
-	Enabled *bool `json:"enabled,omitempty"`
+	// Enabled enables the AWS EBS CSI driver.
+	Enabled *bool `json:"-"`
 
 	// Managed controls if aws-ebs-csi-driver is manged and deployed by kOps.
 	// The deployment of aws-ebs-csi-driver is skipped if this is set to false.

diff --git a/pkg/apis/kops/validation/aws.go b/pkg/apis/kops/validation/aws.go
@@ -54,7 +54,7 @@ func awsValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
 		allErrs = append(allErrs, awsValidateLoadBalancerSubnets(lbPath.Child("subnets"), c.Spec)...)
 	}
 
-	allErrs = append(allErrs, awsValidateExternalCloudControllerManager(c)...)
+	allErrs = append(allErrs, awsValidateEBSCSIDriver(c)...)
 
 	if c.Spec.Authentication != nil && c.Spec.Authentication.AWS != nil {
 		allErrs = append(allErrs, awsValidateIAMAuthenticator(field.NewPath("spec", "authentication", "aws"), c.Spec.Authentication.AWS)...)
@@ -63,16 +63,12 @@ func awsValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
 	return allErrs
 }
 
-func awsValidateExternalCloudControllerManager(cluster *kops.Cluster) (allErrs field.ErrorList) {
+func awsValidateEBSCSIDriver(cluster *kops.Cluster) (allErrs field.ErrorList) {
 	c := cluster.Spec
 
-	if c.ExternalCloudControllerManager == nil {
-		return allErrs
-	}
-	fldPath := field.NewPath("spec", "externalCloudControllerManager")
-	if !hasAWSEBSCSIDriver(c) {
-		allErrs = append(allErrs, field.Forbidden(fldPath,
-			"AWS external CCM cannot be used without enabling spec.cloudProvider.aws.ebsCSIDriverSpec."))
+	fldPath := field.NewPath("spec", "cloudProvider", "aws", "ebsCSIDriver", "enabled")
+	if c.CloudProvider.AWS.EBSCSIDriver != nil && c.CloudProvider.AWS.EBSCSIDriver.Enabled != nil && !*c.CloudProvider.AWS.EBSCSIDriver.Enabled {
+		allErrs = append(allErrs, field.Forbidden(fldPath, "must not be disabled"))
 	}
 	return allErrs
 }
@@ -397,15 +393,6 @@ func awsValidateIAMAuthenticator(fieldPath *field.Path, spec *kops.AWSAuthentica
 	return allErrs
 }
 
-func hasAWSEBSCSIDriver(c kops.ClusterSpec) bool {
-	// EBSCSIDriverSpec will have a default value, so if this is all false, it will be populated on next pass
-	if c.CloudProvider.AWS.EBSCSIDriver == nil || c.CloudProvider.AWS.EBSCSIDriver.Enabled == nil {
-		return true
-	}
-
-	return *c.CloudProvider.AWS.EBSCSIDriver.Enabled
-}
-
 func awsValidateAdditionalRoutes(fieldPath *field.Path, routes []kops.RouteSpec, networkCIDRs []*net.IPNet) field.ErrorList {
 	allErrs := field.ErrorList{}
 

diff --git a/pkg/apis/kops/validation/aws_test.go b/pkg/apis/kops/validation/aws_test.go
@@ -33,7 +33,7 @@ import (
 	"k8s.io/kops/pkg/apis/kops"
 )
 
-func TestAWSValidateExternalCloudConfig(t *testing.T) {
+func TestAWSValidateEBSCSIDriver(t *testing.T) {
 	grid := []struct {
 		Input          kops.ClusterSpec
 		ExpectedErrors []string
@@ -49,7 +49,7 @@ func TestAWSValidateExternalCloudConfig(t *testing.T) {
 					},
 				},
 			},
-			ExpectedErrors: []string{"Forbidden::spec.externalCloudControllerManager"},
+			ExpectedErrors: []string{"Forbidden::spec.cloudProvider.aws.ebsCSIDriver.enabled"},
 		},
 		{
 			Input: kops.ClusterSpec{
@@ -80,7 +80,7 @@ func TestAWSValidateExternalCloudConfig(t *testing.T) {
 		cluster := &kops.Cluster{
 			Spec: g.Input,
 		}
-		errs := awsValidateExternalCloudControllerManager(cluster)
+		errs := awsValidateEBSCSIDriver(cluster)
 
 		testErrors(t, g.Input, errs, g.ExpectedErrors)
 	}
@@ -585,6 +585,9 @@ func TestLoadBalancerSubnets(t *testing.T) {
 						Type:  kops.LoadBalancerTypeInternal,
 					},
 				},
+				CloudProvider: kops.CloudProviderSpec{
+					AWS: &kops.AWSSpec{},
+				},
 			},
 		}
 		if test.class != nil {
@@ -668,6 +671,9 @@ func TestAWSAuthentication(t *testing.T) {
 						IdentityMappings: test.identityMappings,
 					},
 				},
+				CloudProvider: kops.CloudProviderSpec{
+					AWS: &kops.AWSSpec{},
+				},
 			},
 		}
 		errs := awsValidateCluster(&cluster, true)

diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go
@@ -130,10 +130,6 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 		allErrs = append(allErrs, validateKubeAPIServer(spec.KubeAPIServer, c, fieldPath.Child("kubeAPIServer"), strict)...)
 	}
 
-	if spec.ExternalCloudControllerManager == nil && spec.IsIPv6Only() {
-		allErrs = append(allErrs, field.Required(fieldPath.Child("cloudControllerManager"), "IPv6 requires external Cloud Controller Manager"))
-	}
-
 	if spec.KubeProxy != nil {
 		allErrs = append(allErrs, validateKubeProxy(spec.KubeProxy, fieldPath.Child("kubeProxy"))...)
 	}

diff --git a/pkg/apis/nodeup/config.go b/pkg/apis/nodeup/config.go
@@ -116,8 +116,6 @@ type Config struct {
 	ElbSecurityGroup *string `json:"elbSecurityGroup,omitempty"`
 	// NodeIPFamilies controls the IP families reported for each node.
 	NodeIPFamilies []string `json:"nodeIPFamilies,omitempty"`
-	// UseInstanceIDForNodeName uses the instance ID instead of the hostname for the node name.
-	UseInstanceIDForNodeName bool `json:"useInstanceIDForNodeName,omitempty"`
 	// WarmPoolImages are the container images to pre-pull during instance pre-initialization
 	WarmPoolImages []string `json:"warmPoolImages,omitempty"`
 
@@ -336,10 +334,6 @@ func NewConfig(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (*Confi
 		config.Networking.KubeRouter = &kops.KuberouterNetworkingSpec{}
 	}
 
-	if UsesInstanceIDForNodeName(cluster) {
-		config.UseInstanceIDForNodeName = true
-	}
-
 	if instanceGroup.Spec.Kubelet != nil {
 		config.KubeletConfig = *instanceGroup.Spec.Kubelet
 	}
@@ -454,10 +448,6 @@ func buildKubeProxy(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) *k
 	return config
 }
 
-func UsesInstanceIDForNodeName(cluster *kops.Cluster) bool {
-	return cluster.Spec.ExternalCloudControllerManager != nil && cluster.Spec.GetCloudProvider() == kops.CloudProviderAWS
-}
-
 func filterFileAssets(f []kops.FileAssetSpec, role kops.InstanceGroupRole) []kops.FileAssetSpec {
 	var fileAssets []kops.FileAssetSpec
 	for _, fileAsset := range f {

diff --git a/pkg/bootstrap/authenticate.go b/pkg/bootstrap/authenticate.go
@@ -49,5 +49,5 @@ type VerifyResult struct {
 
 // Verifier verifies authentication credentials for requests.
 type Verifier interface {
-	VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*VerifyResult, error)
+	VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte) (*VerifyResult, error)
 }
diff --git a/pkg/model/components/apiserver.go b/pkg/model/components/apiserver.go
@@ -181,7 +181,7 @@ func (b *KubeAPIServerOptionsBuilder) BuildOptions(o interface{}) error {
 		c.FeatureGates = make(map[string]string)
 	}
 
-	if clusterSpec.CloudProvider.AWS != nil && clusterSpec.CloudProvider.AWS.EBSCSIDriver != nil && fi.ValueOf(clusterSpec.CloudProvider.AWS.EBSCSIDriver.Enabled) {
+	if clusterSpec.CloudProvider.AWS != nil {
 
 		if _, found := c.FeatureGates["InTreePluginAWSUnregister"]; !found {
 			c.FeatureGates["InTreePluginAWSUnregister"] = "true"

diff --git a/pkg/model/components/awscloudcontrollermanager.go b/pkg/model/components/awscloudcontrollermanager.go
@@ -45,10 +45,6 @@ func (b *AWSCloudControllerManagerOptionsBuilder) BuildOptions(o interface{}) er
 
 	eccm := clusterSpec.ExternalCloudControllerManager
 
-	if eccm == nil {
-		return nil
-	}
-
 	// No significant downside to always doing a leader election.
 	// Also, having multiple control plane nodes requires leader election.
 	eccm.LeaderElection = &kops.LeaderElectionConfiguration{LeaderElect: fi.PtrTo(true)}

diff --git a/pkg/model/components/awsebscsidriver.go b/pkg/model/components/awsebscsidriver.go
@@ -42,10 +42,6 @@ func (b *AWSEBSCSIDriverOptionsBuilder) BuildOptions(o interface{}) error {
 	}
 	c := aws.EBSCSIDriver
 
-	if !fi.ValueOf(c.Enabled) {
-		return nil
-	}
-
 	if c.Version == nil {
 		version := "v1.14.1"
 		c.Version = &version

diff --git a/pkg/model/components/gcpcloudcontrollermanager.go b/pkg/model/components/gcpcloudcontrollermanager.go
@@ -42,10 +42,6 @@ func (b *GCPCloudControllerManagerOptionsBuilder) BuildOptions(options interface
 
 	ccmConfig := clusterSpec.ExternalCloudControllerManager
 
-	if ccmConfig == nil {
-		return nil
-	}
-
 	// No significant downside to always doing a leader election.
 	// Also, having multiple control plane nodes requires leader election.
 	ccmConfig.LeaderElection = &kops.LeaderElectionConfiguration{LeaderElect: fi.PtrTo(true)}

diff --git a/pkg/model/components/kubecontrollermanager.go b/pkg/model/components/kubecontrollermanager.go
@@ -77,14 +77,6 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o interface{}) error
 	kcm.ClusterName = b.ClusterName
 	kcm.CloudProvider = "external"
 
-	if clusterSpec.ExternalCloudControllerManager == nil {
-		if kcm.CloudProvider == "aws" || kcm.CloudProvider == "gce" {
-			kcm.EnableLeaderMigration = fi.PtrTo(true)
-		}
-	} else {
-		kcm.CloudProvider = "external"
-	}
-
 	if kcm.LogLevel == 0 {
 		kcm.LogLevel = 2
 	}
@@ -151,7 +143,7 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o interface{}) error
 		}
 	}
 
-	if clusterSpec.CloudProvider.AWS != nil && clusterSpec.CloudProvider.AWS.EBSCSIDriver != nil && fi.ValueOf(clusterSpec.CloudProvider.AWS.EBSCSIDriver.Enabled) {
+	if clusterSpec.CloudProvider.AWS != nil {
 
 		if kcm.FeatureGates == nil {
 			kcm.FeatureGates = make(map[string]string)

diff --git a/pkg/model/components/kubelet.go b/pkg/model/components/kubelet.go
@@ -171,7 +171,7 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 		clusterSpec.Kubelet.FeatureGates = make(map[string]string)
 	}
 
-	if clusterSpec.CloudProvider.AWS != nil && clusterSpec.CloudProvider.AWS.EBSCSIDriver != nil && fi.ValueOf(clusterSpec.CloudProvider.AWS.EBSCSIDriver.Enabled) {
+	if clusterSpec.CloudProvider.AWS != nil {
 		if _, found := clusterSpec.Kubelet.FeatureGates["CSIMigrationAWS"]; !found && b.IsKubernetesLT("1.27") {
 			clusterSpec.Kubelet.FeatureGates["CSIMigrationAWS"] = "true"
 		}

diff --git a/pkg/model/components/kubescheduler.go b/pkg/model/components/kubescheduler.go
@@ -57,7 +57,7 @@ func (b *KubeSchedulerOptionsBuilder) BuildOptions(o interface{}) error {
 		}
 	}
 
-	if clusterSpec.CloudProvider.AWS != nil && clusterSpec.CloudProvider.AWS.EBSCSIDriver != nil && fi.ValueOf(clusterSpec.CloudProvider.AWS.EBSCSIDriver.Enabled) {
+	if clusterSpec.CloudProvider.AWS != nil {
 
 		if config.FeatureGates == nil {
 			config.FeatureGates = make(map[string]string)

diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -405,20 +405,12 @@ func (r *NodeRoleMaster) BuildAWSPolicy(b *PolicyBuilder) (*Policy, error) {
 	// Protokube needs dns-controller permissions in instance role even if UseServiceAccountExternalPermissions.
 	AddDNSControllerPermissions(b, p)
 
-	// If cluster does not use external CCM, the master IAM Role needs CCM permissions
-	if b.Cluster.Spec.ExternalCloudControllerManager == nil {
-		AddCCMPermissions(p, b.Cluster.Spec.Networking.Kubenet != nil)
-		AddLegacyCCMPermissions(p)
-	}
-
 	if !b.UseServiceAccountExternalPermisssions {
 		esc := b.Cluster.Spec.SnapshotController != nil &&
 			fi.ValueOf(b.Cluster.Spec.SnapshotController.Enabled)
 		AddAWSEBSCSIDriverPermissions(p, esc)
 
-		if b.Cluster.Spec.ExternalCloudControllerManager != nil {
-			AddCCMPermissions(p, b.Cluster.Spec.Networking.Kubenet != nil)
-		}
+		AddCCMPermissions(p, b.Cluster.Spec.Networking.Kubenet != nil)
 
 		if c := b.Cluster.Spec.CloudProvider.AWS.LoadBalancerController; c != nil && fi.ValueOf(b.Cluster.Spec.CloudProvider.AWS.LoadBalancerController.Enabled) {
 			AddAWSLoadbalancerControllerPermissions(p, c.EnableWAF, c.EnableWAFv2, c.EnableShield)
@@ -826,28 +818,6 @@ func addEtcdManagerPermissions(p *Policy) {
 	)
 }
 
-func AddLegacyCCMPermissions(p *Policy) {
-	p.unconditionalAction.Insert(
-		"ec2:CreateSecurityGroup",
-		"ec2:CreateTags",
-		"elasticloadbalancing:CreateTargetGroup",
-		"elasticloadbalancing:AddTags",
-		"elasticloadbalancing:RegisterTargets",
-		"elasticloadbalancing:CreateListener",
-		"elasticloadbalancing:DeleteListener",
-		"elasticloadbalancing:ModifyListener",
-		"ec2:DescribeVolumes",
-		"ec2:ModifyInstanceAttribute",
-		"ec2:ModifyVolume",
-		"ec2:AttachVolume",
-		"ec2:AuthorizeSecurityGroupIngress",
-		"ec2:DeleteRoute",
-		"ec2:DeleteSecurityGroup",
-		"ec2:DeleteVolume",
-		"ec2:DetachVolume",
-	)
-}
-
 func AddCCMPermissions(p *Policy, cloudRoutes bool) {
 	p.unconditionalAction.Insert(
 		"autoscaling:DescribeAutoScalingGroups",

diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go
@@ -209,6 +209,7 @@ func TestPolicyGeneration(t *testing.T) {
 							},
 						},
 					},
+					ExternalCloudControllerManager: &kops.CloudControllerManagerConfig{},
 					Networking: kops.NetworkingSpec{
 						Kubenet: &kops.KubenetNetworkingSpec{},
 					},