PCP-4205: 🐛 fix: separate control plane logging and vpc config updates (#954)

* fix: separate control plane logging and vpc config updates

* fix: set PublicCIDRs to [] when private only EP

Author: pavansokkenagaraj

pkg/cloud/services/eks/cluster.go

@@ -120,6 +120,10 @@ func (s *Service) reconcileCluster(ctx context.Context) error {
 		return errors.Wrap(err, "failed reconciling cluster config")
 	}
 
+	if err := s.reconcileLogging(cluster.Logging); err != nil {
+		return errors.Wrap(err, "failed reconciling logging")
+	}
+
 	if err := s.reconcileEKSEncryptionConfig(cluster.EncryptionConfig); err != nil {
 		return errors.Wrap(err, "failed reconciling eks encryption config")
 	}
@@ -296,7 +300,9 @@ func makeVpcConfig(subnets infrav1.Subnets, endpointAccess ekscontrolplanev1.End
 		SubnetIds:             subnetIds,
 	}
 
-	if len(cidrs) > 0 {
+	isPrivateOnlyEndPoint := !aws.BoolValue(vpcConfig.EndpointPublicAccess) && aws.BoolValue(vpcConfig.EndpointPrivateAccess)
+
+	if len(cidrs) > 0 || isPrivateOnlyEndPoint {
 		vpcConfig.PublicAccessCidrs = cidrs
 	}
 	sg, ok := securityGroups[infrav1.SecurityGroupEKSNodeAdditional]
@@ -439,11 +445,6 @@ func (s *Service) reconcileClusterConfig(cluster *eks.Cluster) error {
 	var needsUpdate bool
 	input := eks.UpdateClusterConfigInput{Name: aws.String(s.scope.KubernetesClusterName())}
 
-	if updateLogging := s.reconcileLogging(cluster.Logging); updateLogging != nil {
-		needsUpdate = true
-		input.Logging = updateLogging
-	}
-
 	updateVpcConfig, err := s.reconcileVpcConfig(cluster.ResourcesVpcConfig)
 	if err != nil {
 		return errors.Wrap(err, "couldn't create vpc config for cluster")
@@ -475,15 +476,39 @@ func (s *Service) reconcileClusterConfig(cluster *eks.Cluster) error {
 	return nil
 }
 
-func (s *Service) reconcileLogging(logging *eks.Logging) *eks.Logging {
+func (s *Service) reconcileLogging(logging *eks.Logging) error {
+	input := eks.UpdateClusterConfigInput{Name: aws.String(s.scope.KubernetesClusterName())}
+
 	for _, logSetup := range logging.ClusterLogging {
 		for _, l := range logSetup.Types {
 			enabled := s.scope.ControlPlane.Spec.Logging.IsLogEnabled(*l)
 			if enabled != *logSetup.Enabled {
-				return makeEksLogging(s.scope.ControlPlane.Spec.Logging)
+				input.Logging = makeEksLogging(s.scope.ControlPlane.Spec.Logging)
 			}
 		}
 	}
+
+	if input.Logging != nil {
+		if err := input.Validate(); err != nil {
+			return errors.Wrap(err, "created invalid UpdateClusterConfigInput")
+		}
+
+		if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) {
+			if _, err := s.EKSClient.UpdateClusterConfig(&input); err != nil {
+				if aerr, ok := err.(awserr.Error); ok {
+					return false, aerr
+				}
+				return false, err
+			}
+			conditions.MarkTrue(s.scope.ControlPlane, ekscontrolplanev1.EKSControlPlaneUpdatingCondition)
+			record.Eventf(s.scope.ControlPlane, "InitiatedUpdateEKSControlPlane", "Initiated logging update for EKS control plane %s", s.scope.KubernetesClusterName())
+			return true, nil
+		}); err != nil {
+			record.Warnf(s.scope.ControlPlane, "FailedUpdateEKSControlPlane", "Failed to update EKS control plane logging: %v", err)
+			return errors.Wrapf(err, "failed to update EKS cluster")
+		}
+	}
+
 	return nil
 }
 

pkg/cloud/services/eks/cluster_test.go

@@ -234,6 +234,94 @@ func TestMakeVPCConfig(t *testing.T) {
 				PublicAccessCidrs: []*string{aws.String("10.0.0.0/24")},
 			},
 		},
+		{
+			name: "private only endpoint access",
+			input: input{
+				subnets: []infrav1.SubnetSpec{
+					{
+						ID:               idOne,
+						CidrBlock:        "10.0.10.0/24",
+						AvailabilityZone: "us-west-2a",
+						IsPublic:         false,
+					},
+					{
+						ID:               idTwo,
+						CidrBlock:        "10.0.10.1/24",
+						AvailabilityZone: "us-west-2b",
+						IsPublic:         false,
+					},
+				},
+				endpointAccess: ekscontrolplanev1.EndpointAccess{
+					Private:     aws.Bool(true),
+					PublicCIDRs: []*string{},
+				},
+			},
+			expect: &eks.VpcConfigRequest{
+				SubnetIds:             []*string{&idOne, &idTwo},
+				PublicAccessCidrs:     []*string{},
+				EndpointPrivateAccess: aws.Bool(true),
+			},
+		},
+		{
+			name: "public and private endpoint access",
+			input: input{
+				subnets: []infrav1.SubnetSpec{
+					{
+						ID:               idOne,
+						CidrBlock:        "10.0.10.0/24",
+						AvailabilityZone: "us-west-2a",
+						IsPublic:         false,
+					},
+					{
+						ID:               idTwo,
+						CidrBlock:        "10.0.10.1/24",
+						AvailabilityZone: "us-west-2b",
+						IsPublic:         false,
+					},
+				},
+				endpointAccess: ekscontrolplanev1.EndpointAccess{
+					Private:     aws.Bool(true),
+					Public:      aws.Bool(true),
+					PublicCIDRs: []*string{},
+				},
+			},
+			expect: &eks.VpcConfigRequest{
+				SubnetIds:             []*string{&idOne, &idTwo},
+				PublicAccessCidrs:     nil,
+				EndpointPrivateAccess: aws.Bool(true),
+				EndpointPublicAccess:  aws.Bool(true),
+			},
+		},
+		{
+			name: "public only endpoint access",
+			input: input{
+				subnets: []infrav1.SubnetSpec{
+					{
+						ID:               idOne,
+						CidrBlock:        "10.0.10.0/24",
+						AvailabilityZone: "us-west-2a",
+						IsPublic:         false,
+					},
+					{
+						ID:               idTwo,
+						CidrBlock:        "10.0.10.1/24",
+						AvailabilityZone: "us-west-2b",
+						IsPublic:         false,
+					},
+				},
+				endpointAccess: ekscontrolplanev1.EndpointAccess{
+					Private:     aws.Bool(false),
+					Public:      aws.Bool(true),
+					PublicCIDRs: []*string{},
+				},
+			},
+			expect: &eks.VpcConfigRequest{
+				SubnetIds:             []*string{&idOne, &idTwo},
+				PublicAccessCidrs:     nil,
+				EndpointPrivateAccess: aws.Bool(false),
+				EndpointPublicAccess:  aws.Bool(true),
+			},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {