From 904ed61df29967b0dda5502d0c3d05e21551c7e3 Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Tue, 27 Aug 2024 17:15:02 +1000 Subject: [PATCH 01/19] feat: support setting EKS AuthenticationMode --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 17 +++++- .../api/v1beta1/zz_generated.conversion.go | 1 + .../v1beta2/awsmanagedcontrolplane_types.go | 15 +++++- controlplane/eks/api/v1beta2/types.go | 15 ++++++ .../eks/api/v1beta2/zz_generated.deepcopy.go | 20 +++++++ pkg/cloud/services/eks/cluster.go | 54 +++++++++++++++++++ 6 files changed, 120 insertions(+), 2 deletions(-) diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 345b3f4379..4a25036cac 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -2102,6 +2102,21 @@ spec: description: AWSManagedControlPlaneSpec defines the desired state of an Amazon EKS Cluster. properties: + accessConfig: + description: AccessConfig specifies the access configuration information + for the cluster + properties: + authenticationMode: + default: CONFIG_MAP + description: |- + AuthenticationMode specifies the desired authentication mode for the cluster + Defaults to CONFIG_MAP + enum: + - CONFIG_MAP + - API + - API_AND_CONFIG_MAP + type: string + type: object additionalTags: additionalProperties: type: string @@ -2825,7 +2840,7 @@ spec: type: object oidcIdentityProviderConfig: description: |- - IdentityProviderconfig is used to specify the oidc provider config + OIDCIdentityProviderConfig is used to specify the oidc provider config to be attached with this eks cluster properties: clientId: diff --git a/controlplane/eks/api/v1beta1/zz_generated.conversion.go b/controlplane/eks/api/v1beta1/zz_generated.conversion.go index 151772f75b..ba4acbbfd7 100644 --- a/controlplane/eks/api/v1beta1/zz_generated.conversion.go +++ b/controlplane/eks/api/v1beta1/zz_generated.conversion.go @@ -390,6 +390,7 @@ func autoConvert_v1beta2_AWSManagedControlPlaneSpec_To_v1beta1_AWSManagedControl out.AssociateOIDCProvider = in.AssociateOIDCProvider out.Addons = (*[]Addon)(unsafe.Pointer(in.Addons)) out.OIDCIdentityProviderConfig = (*OIDCIdentityProviderConfig)(unsafe.Pointer(in.OIDCIdentityProviderConfig)) + // WARNING: in.AccessConfig requires manual conversion: does not exist in peer-type if err := Convert_v1beta2_VpcCni_To_v1beta1_VpcCni(&in.VpcCni, &out.VpcCni, s); err != nil { return err } diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go index 109752e573..cf114fb230 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go @@ -164,11 +164,15 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned // +optional Addons *[]Addon `json:"addons,omitempty"` - // IdentityProviderconfig is used to specify the oidc provider config + // OIDCIdentityProviderConfig is used to specify the oidc provider config // to be attached with this eks cluster // +optional OIDCIdentityProviderConfig *OIDCIdentityProviderConfig `json:"oidcIdentityProviderConfig,omitempty"` + // AccessConfig specifies the access configuration information for the cluster + // +optional + AccessConfig *AccessConfig `json:"accessConfig,omitempty"` + // VpcCni is used to set configuration options for the VPC CNI plugin // +optional VpcCni VpcCni `json:"vpcCni,omitempty"` @@ -219,6 +223,15 @@ type EndpointAccess struct { Private *bool `json:"private,omitempty"` } +// AccessConfig represents the access configuration information for the cluster +type AccessConfig struct { + // AuthenticationMode specifies the desired authentication mode for the cluster + // Defaults to CONFIG_MAP + // +kubebuilder:default=CONFIG_MAP + // +kubebuilder:validation:Enum=CONFIG_MAP;API;API_AND_CONFIG_MAP + AuthenticationMode EKSAuthenticationMode `json:"authenticationMode,omitempty"` +} + // EncryptionConfig specifies the encryption configuration for the EKS clsuter. type EncryptionConfig struct { // Provider specifies the ARN or alias of the CMK (in AWS KMS) diff --git a/controlplane/eks/api/v1beta2/types.go b/controlplane/eks/api/v1beta2/types.go index 1ef47215ce..6f385f7d40 100644 --- a/controlplane/eks/api/v1beta2/types.go +++ b/controlplane/eks/api/v1beta2/types.go @@ -79,6 +79,21 @@ var ( EKSTokenMethodAWSCli = EKSTokenMethod("aws-cli") ) +// EKSAuthenticationMode defines the authentication mode for the cluster +type EKSAuthenticationMode string + +var ( + // EKSAuthenticationModeConfigMap indicates that only `aws-auth` ConfigMap will be used for authentication + EKSAuthenticationModeConfigMap = EKSAuthenticationMode("CONFIG_MAP") + + // EKSAuthenticationModeApi indicates that only AWS Access Entries will be used for authentication + EKSAuthenticationModeApi = EKSAuthenticationMode("API") + + // EKSAuthenticationModeApiAndConfigMap indicates that both `aws-auth` ConfigMap and AWS Access Entries will + // be used for authentication + EKSAuthenticationModeApiAndConfigMap = EKSAuthenticationMode("API_AND_CONFIG_MAP") +) + var ( // DefaultEKSControlPlaneRole is the name of the default IAM role to use for the EKS control plane // if no other role is supplied in the spec and if iam role creation is not enabled. The default diff --git a/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go b/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go index 160f556db9..71c30d3e9e 100644 --- a/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go +++ b/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go @@ -170,6 +170,11 @@ func (in *AWSManagedControlPlaneSpec) DeepCopyInto(out *AWSManagedControlPlaneSp *out = new(OIDCIdentityProviderConfig) (*in).DeepCopyInto(*out) } + if in.AccessConfig != nil { + in, out := &in.AccessConfig, &out.AccessConfig + *out = new(AccessConfig) + **out = **in + } in.VpcCni.DeepCopyInto(&out.VpcCni) out.KubeProxy = in.KubeProxy } @@ -238,6 +243,21 @@ func (in *AWSManagedControlPlaneStatus) DeepCopy() *AWSManagedControlPlaneStatus return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessConfig) DeepCopyInto(out *AccessConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessConfig. +func (in *AccessConfig) DeepCopy() *AccessConfig { + if in == nil { + return nil + } + out := new(AccessConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Addon) DeepCopyInto(out *Addon) { *out = *in diff --git a/pkg/cloud/services/eks/cluster.go b/pkg/cloud/services/eks/cluster.go index 62c990bd36..82c3a2f925 100644 --- a/pkg/cloud/services/eks/cluster.go +++ b/pkg/cloud/services/eks/cluster.go @@ -121,6 +121,10 @@ func (s *Service) reconcileCluster(ctx context.Context) error { return errors.Wrap(err, "failed reconciling cluster config") } + if err := s.reconcileAccessConfig(cluster.AccessConfig); err != nil { + return errors.Wrap(err, "failed reconciling access config") + } + if err := s.reconcileLogging(cluster.Logging); err != nil { return errors.Wrap(err, "failed reconciling logging") } @@ -375,6 +379,13 @@ func (s *Service) createCluster(eksClusterName string) (*eks.Cluster, error) { return nil, errors.Wrap(err, "couldn't create vpc config for cluster") } + var accessConfig *eks.CreateAccessConfigRequest + if s.scope.ControlPlane.Spec.AccessConfig != nil && s.scope.ControlPlane.Spec.AccessConfig.AuthenticationMode != "" { + accessConfig = &eks.CreateAccessConfigRequest{ + AuthenticationMode: aws.String(string(s.scope.ControlPlane.Spec.AccessConfig.AuthenticationMode)), + } + } + var netConfig *eks.KubernetesNetworkConfigRequest if s.scope.VPC().IsIPv6Enabled() { netConfig = &eks.KubernetesNetworkConfigRequest{ @@ -416,6 +427,7 @@ func (s *Service) createCluster(eksClusterName string) (*eks.Cluster, error) { Name: aws.String(eksClusterName), Version: eksVersion, Logging: logging, + AccessConfig: accessConfig, EncryptionConfig: encryptionConfigs, ResourcesVpcConfig: vpcConfig, RoleArn: role.Arn, @@ -423,6 +435,10 @@ func (s *Service) createCluster(eksClusterName string) (*eks.Cluster, error) { KubernetesNetworkConfig: netConfig, } + if err := input.Validate(); err != nil { + return nil, errors.Wrap(err, "created invalid CreateClusterInput") + } + var out *eks.CreateClusterOutput if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { if out, err = s.EKSClient.CreateCluster(input); err != nil { @@ -501,6 +517,44 @@ func (s *Service) reconcileClusterConfig(cluster *eks.Cluster) error { return nil } +func (s *Service) reconcileAccessConfig(accessConfig *eks.AccessConfigResponse) error { + input := eks.UpdateClusterConfigInput{Name: aws.String(s.scope.KubernetesClusterName())} + + if s.scope.ControlPlane.Spec.AccessConfig == nil || s.scope.ControlPlane.Spec.AccessConfig.AuthenticationMode == "" { + return nil + } + + expectedAuthenticationMode := string(s.scope.ControlPlane.Spec.AccessConfig.AuthenticationMode) + if expectedAuthenticationMode != aws.StringValue(accessConfig.AuthenticationMode) { + input.AccessConfig = &eks.UpdateAccessConfigRequest{ + AuthenticationMode: aws.String(expectedAuthenticationMode), + } + } + + if input.AccessConfig != 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 auth config 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 auth config: %v", err) + return errors.Wrapf(err, "failed to update EKS cluster") + } + } + + return nil +} + func (s *Service) reconcileLogging(logging *eks.Logging) error { input := eks.UpdateClusterConfigInput{Name: aws.String(s.scope.KubernetesClusterName())} From fe7b9a94379fed35c906159293144660243f862d Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Fri, 6 Sep 2024 15:33:03 +1000 Subject: [PATCH 02/19] Add GCFLAGS --- Dockerfile | 3 ++- Makefile | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 155b9519a7..b28186a6c4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -41,10 +41,11 @@ COPY ./ ./ ARG package=. ARG ARCH ARG LDFLAGS +ARG GCFLAGS RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.local/share/golang \ - CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags "${LDFLAGS} -extldflags '-static'" -o manager ${package} + CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -gcflags "${GCFLAGS}" -ldflags "${LDFLAGS} -extldflags '-static'" -o manager ${package} ENTRYPOINT [ "/start.sh", "/workspace/manager" ] # Copy the controller-manager into a thin image diff --git a/Makefile b/Makefile index ef0bb24e20..5a9e82000c 100644 --- a/Makefile +++ b/Makefile @@ -134,6 +134,9 @@ RBAC_ROOT ?= $(MANIFEST_ROOT)/rbac # Allow overriding the imagePullPolicy PULL_POLICY ?= Always +# Allow overriding the GCFLAGS +GCFLAGS ?= + # Set build time variables including version details LDFLAGS := $(shell source ./hack/version.sh; version::ldflags) @@ -371,12 +374,12 @@ binaries: managers clusterawsadm ## Builds and installs all binaries .PHONY: clusterawsadm clusterawsadm: ## Build clusterawsadm binary - go build -ldflags "$(LDFLAGS)" -o $(BIN_DIR)/clusterawsadm ./cmd/clusterawsadm + go build -gcflags "$(GCFLAGS)" -ldflags "$(LDFLAGS)" -o $(BIN_DIR)/clusterawsadm ./cmd/clusterawsadm .PHONY: docker-build docker-build: docker-pull-prerequisites ## Build the docker image for controller-manager - docker build --build-arg ARCH=$(ARCH) --build-arg builder_image=$(GO_CONTAINER_IMAGE) --build-arg LDFLAGS="$(LDFLAGS)" . -t $(CORE_CONTROLLER_IMG)-$(ARCH):$(TAG) + docker build --build-arg ARCH=$(ARCH) --build-arg builder_image=$(GO_CONTAINER_IMAGE) --build-arg GCFLAGS="$(GCFLAGS)" --build-arg LDFLAGS="$(LDFLAGS)" . -t $(CORE_CONTROLLER_IMG)-$(ARCH):$(TAG) .PHONY: docker-build-all ## Build all the architecture docker images docker-build-all: $(addprefix docker-build-,$(ALL_ARCH)) @@ -395,7 +398,7 @@ managers: ## Alias for manager-aws-infrastructure .PHONY: manager-aws-infrastructure manager-aws-infrastructure: ## Build manager binary - CGO_ENABLED=0 GOARCH=${ARCH} go build -ldflags "${LDFLAGS} -extldflags '-static'" -o $(BIN_DIR)/manager . + CGO_ENABLED=0 GOARCH=${ARCH} go build -gcflags "${GCFLAGS}" -ldflags "${LDFLAGS} -extldflags '-static'" -o $(BIN_DIR)/manager . ##@ test: From f36e389000fe363bb75b2ae66c22efa4b95dc433 Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Fri, 6 Sep 2024 15:33:21 +1000 Subject: [PATCH 03/19] Add debug log line --- pkg/cloud/services/eks/cluster.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cloud/services/eks/cluster.go b/pkg/cloud/services/eks/cluster.go index 82c3a2f925..1a0c2f96c1 100644 --- a/pkg/cloud/services/eks/cluster.go +++ b/pkg/cloud/services/eks/cluster.go @@ -525,6 +525,7 @@ func (s *Service) reconcileAccessConfig(accessConfig *eks.AccessConfigResponse) } expectedAuthenticationMode := string(s.scope.ControlPlane.Spec.AccessConfig.AuthenticationMode) + s.scope.Debug("Reconciling EKS Access Config for cluster", "cluster-name", s.scope.KubernetesClusterName(), "expected", expectedAuthenticationMode, "current", accessConfig.AuthenticationMode) if expectedAuthenticationMode != aws.StringValue(accessConfig.AuthenticationMode) { input.AccessConfig = &eks.UpdateAccessConfigRequest{ AuthenticationMode: aws.String(expectedAuthenticationMode), From 49e7a7b3d3de41ca44d0340024fa896a16cb04df Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Thu, 12 Sep 2024 12:18:46 +1000 Subject: [PATCH 04/19] Add webhook validation for access config --- .../v1beta2/awsmanagedcontrolplane_webhook.go | 23 +++++ .../awsmanagedcontrolplane_webhook_test.go | 90 +++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go index abda129f92..13e325ddaa 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go @@ -123,6 +123,7 @@ func (r *AWSManagedControlPlane) ValidateUpdate(old runtime.Object) (admission.W allErrs = append(allErrs, r.validateEKSClusterNameSame(oldAWSManagedControlplane)...) allErrs = append(allErrs, r.validateEKSVersion(oldAWSManagedControlplane)...) allErrs = append(allErrs, r.Spec.Bastion.Validate()...) + allErrs = append(allErrs, r.validateAccessConfig(oldAWSManagedControlplane)...) allErrs = append(allErrs, r.validateIAMAuthConfig()...) allErrs = append(allErrs, r.validateSecondaryCIDR()...) allErrs = append(allErrs, r.validateEKSAddons()...) @@ -289,6 +290,28 @@ func (r *AWSManagedControlPlane) validateEKSAddons() field.ErrorList { return allErrs } +func (r *AWSManagedControlPlane) validateAccessConfig(old *AWSManagedControlPlane) field.ErrorList { + var allErrs field.ErrorList + + // If accessConfig is already set, do not allow removal of it. + if old.Spec.AccessConfig != nil && r.Spec.AccessConfig == nil { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "accessConfig"), r.Spec.AccessConfig, "removing AccessConfig is not allowed after it has been enabled"), + ) + } + + // AuthenticationMode is ratcheting - do not allow downgrades + if old.Spec.AccessConfig != nil && old.Spec.AccessConfig.AuthenticationMode != r.Spec.AccessConfig.AuthenticationMode && + ((old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeApiAndConfigMap && r.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeConfigMap) || + old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeApi) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "accessConfig", "authenticationMode"), r.Spec.AccessConfig.AuthenticationMode, "downgrading authentication mode is not allowed after it has been enabled"), + ) + } + + return allErrs +} + func (r *AWSManagedControlPlane) validateIAMAuthConfig() field.ErrorList { var allErrs field.ErrorList diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go index 7441040b8e..13d987adb9 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go @@ -603,6 +603,96 @@ func TestWebhookUpdate(t *testing.T) { }, expectError: false, }, + { + name: "no change in access config", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + expectError: false, + }, + { + name: "change in access config to nil", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + }, + expectError: true, + }, + { + name: "change in access config from nil to valid", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + expectError: false, + }, + { + name: "change in access config auth mode from ApiAndConfigMap to API is allowed", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeApiAndConfigMap, + }, + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeApi, + }, + }, + expectError: false, + }, + { + name: "change in access config auth mode from API to Config Map is denied", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeApi, + }, + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + expectError: true, + }, + { + name: "change in access config auth mode from APIAndConfigMap to Config Map is denied", + oldClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeApiAndConfigMap, + }, + }, + newClusterSpec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + }, + }, + expectError: true, + }, { name: "change in encryption config to nil", oldClusterSpec: AWSManagedControlPlaneSpec{ From 3e5754c7a951d488161ff133a5ad02cd3858a5dd Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Thu, 12 Sep 2024 12:52:44 +1000 Subject: [PATCH 05/19] add unit tests --- pkg/cloud/services/eks/cluster.go | 11 +++ pkg/cloud/services/eks/cluster_test.go | 116 +++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/pkg/cloud/services/eks/cluster.go b/pkg/cloud/services/eks/cluster.go index 1a0c2f96c1..a6f3022923 100644 --- a/pkg/cloud/services/eks/cluster.go +++ b/pkg/cloud/services/eks/cluster.go @@ -544,6 +544,17 @@ func (s *Service) reconcileAccessConfig(accessConfig *eks.AccessConfigResponse) } return false, err } + + // Wait until status transitions to UPDATING because there's a short + // window after UpdateClusterVersion returns where the cluster + // status is ACTIVE and the update would be tried again + if err := s.EKSClient.WaitUntilClusterUpdating( + &eks.DescribeClusterInput{Name: aws.String(s.scope.KubernetesClusterName())}, + request.WithWaiterLogger(&awslog{s.GetLogger()}), + ); err != nil { + return false, err + } + conditions.MarkTrue(s.scope.ControlPlane, ekscontrolplanev1.EKSControlPlaneUpdatingCondition) record.Eventf(s.scope.ControlPlane, "InitiatedUpdateEKSControlPlane", "Initiated auth config update for EKS control plane %s", s.scope.KubernetesClusterName()) return true, nil diff --git a/pkg/cloud/services/eks/cluster_test.go b/pkg/cloud/services/eks/cluster_test.go index 7079c62de5..bebc8fe23b 100644 --- a/pkg/cloud/services/eks/cluster_test.go +++ b/pkg/cloud/services/eks/cluster_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/eks" "github.com/aws/aws-sdk-go/service/iam" "github.com/golang/mock/gomock" @@ -463,6 +464,121 @@ func TestReconcileClusterVersion(t *testing.T) { } } +func TestReconcileAccessConfig(t *testing.T) { + clusterName := "default.cluster" + tests := []struct { + name string + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "no upgrade necessary", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m. + DescribeCluster(gomock.AssignableToTypeOf(&eks.DescribeClusterInput{})). + Return(&eks.DescribeClusterOutput{ + Cluster: &eks.Cluster{ + Name: aws.String("default.cluster"), + AccessConfig: &eks.AccessConfigResponse{ + AuthenticationMode: aws.String(eks.AuthenticationModeApiAndConfigMap), + }, + }, + }, nil) + }, + expectError: false, + }, + { + name: "needs upgrade", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m. + DescribeCluster(gomock.AssignableToTypeOf(&eks.DescribeClusterInput{})). + Return(&eks.DescribeClusterOutput{ + Cluster: &eks.Cluster{ + Name: aws.String("default.cluster"), + AccessConfig: &eks.AccessConfigResponse{ + AuthenticationMode: aws.String(eks.AuthenticationModeConfigMap), + }, + }, + }, nil) + m.WaitUntilClusterUpdating( + gomock.AssignableToTypeOf(&eks.DescribeClusterInput{}), gomock.Any(), + ).Return(nil) + m. + UpdateClusterConfig(gomock.AssignableToTypeOf(&eks.UpdateClusterConfigInput{})). + Return(&eks.UpdateClusterConfigOutput{}, nil) + }, + expectError: false, + }, + { + name: "api error", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m. + DescribeCluster(gomock.AssignableToTypeOf(&eks.DescribeClusterInput{})). + Return(&eks.DescribeClusterOutput{ + Cluster: &eks.Cluster{ + Name: aws.String("default.cluster"), + AccessConfig: &eks.AccessConfigResponse{ + AuthenticationMode: aws.String(eks.AuthenticationModeApi), + }, + }, + }, nil) + m. + UpdateClusterConfig(gomock.AssignableToTypeOf(&eks.UpdateClusterConfigInput{})). + Return(&eks.UpdateClusterConfigOutput{}, awserr.New(eks.ErrCodeInvalidParameterException, "Unsupported authentication mode update", nil)) + }, + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{ + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + AccessConfig: &ekscontrolplanev1.AccessConfig{ + AuthenticationMode: eks.AuthenticationModeApiAndConfigMap, + }, + }, + }, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + cluster, err := s.describeEKSCluster(clusterName) + g.Expect(err).To(BeNil()) + + err = s.reconcileAccessConfig(cluster.AccessConfig) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + func TestCreateCluster(t *testing.T) { clusterName := "cluster.default" version := aws.String("1.24") From c192252319cf887e0fda98cf2b2bebb43df31b62 Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Tue, 17 Sep 2024 16:04:22 +1000 Subject: [PATCH 06/19] Make revive happy on naming --- .../eks/api/v1beta2/awsmanagedcontrolplane_webhook.go | 4 ++-- .../api/v1beta2/awsmanagedcontrolplane_webhook_test.go | 8 ++++---- controlplane/eks/api/v1beta2/types.go | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go index 13e325ddaa..431b9d4382 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go @@ -302,8 +302,8 @@ func (r *AWSManagedControlPlane) validateAccessConfig(old *AWSManagedControlPlan // AuthenticationMode is ratcheting - do not allow downgrades if old.Spec.AccessConfig != nil && old.Spec.AccessConfig.AuthenticationMode != r.Spec.AccessConfig.AuthenticationMode && - ((old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeApiAndConfigMap && r.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeConfigMap) || - old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeApi) { + ((old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeAPIAndConfigMap && r.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeConfigMap) || + old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeAPI) { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "accessConfig", "authenticationMode"), r.Spec.AccessConfig.AuthenticationMode, "downgrading authentication mode is not allowed after it has been enabled"), ) diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go index 13d987adb9..b5a2f7b722 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go @@ -650,13 +650,13 @@ func TestWebhookUpdate(t *testing.T) { oldClusterSpec: AWSManagedControlPlaneSpec{ EKSClusterName: "default_cluster1", AccessConfig: &AccessConfig{ - AuthenticationMode: EKSAuthenticationModeApiAndConfigMap, + AuthenticationMode: EKSAuthenticationModeAPIAndConfigMap, }, }, newClusterSpec: AWSManagedControlPlaneSpec{ EKSClusterName: "default_cluster1", AccessConfig: &AccessConfig{ - AuthenticationMode: EKSAuthenticationModeApi, + AuthenticationMode: EKSAuthenticationModeAPI, }, }, expectError: false, @@ -666,7 +666,7 @@ func TestWebhookUpdate(t *testing.T) { oldClusterSpec: AWSManagedControlPlaneSpec{ EKSClusterName: "default_cluster1", AccessConfig: &AccessConfig{ - AuthenticationMode: EKSAuthenticationModeApi, + AuthenticationMode: EKSAuthenticationModeAPI, }, }, newClusterSpec: AWSManagedControlPlaneSpec{ @@ -682,7 +682,7 @@ func TestWebhookUpdate(t *testing.T) { oldClusterSpec: AWSManagedControlPlaneSpec{ EKSClusterName: "default_cluster1", AccessConfig: &AccessConfig{ - AuthenticationMode: EKSAuthenticationModeApiAndConfigMap, + AuthenticationMode: EKSAuthenticationModeAPIAndConfigMap, }, }, newClusterSpec: AWSManagedControlPlaneSpec{ diff --git a/controlplane/eks/api/v1beta2/types.go b/controlplane/eks/api/v1beta2/types.go index 6f385f7d40..0e8381f841 100644 --- a/controlplane/eks/api/v1beta2/types.go +++ b/controlplane/eks/api/v1beta2/types.go @@ -86,12 +86,12 @@ var ( // EKSAuthenticationModeConfigMap indicates that only `aws-auth` ConfigMap will be used for authentication EKSAuthenticationModeConfigMap = EKSAuthenticationMode("CONFIG_MAP") - // EKSAuthenticationModeApi indicates that only AWS Access Entries will be used for authentication - EKSAuthenticationModeApi = EKSAuthenticationMode("API") + // EKSAuthenticationModeAPI indicates that only AWS Access Entries will be used for authentication + EKSAuthenticationModeAPI = EKSAuthenticationMode("API") - // EKSAuthenticationModeApiAndConfigMap indicates that both `aws-auth` ConfigMap and AWS Access Entries will + // EKSAuthenticationModeAPIAndConfigMap indicates that both `aws-auth` ConfigMap and AWS Access Entries will // be used for authentication - EKSAuthenticationModeApiAndConfigMap = EKSAuthenticationMode("API_AND_CONFIG_MAP") + EKSAuthenticationModeAPIAndConfigMap = EKSAuthenticationMode("API_AND_CONFIG_MAP") ) var ( From e2353cd779b34828898a17c941c075a2f91627b1 Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Tue, 17 Sep 2024 16:20:25 +1000 Subject: [PATCH 07/19] fix failing test with invalid mocked input --- pkg/cloud/services/eks/cluster_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cloud/services/eks/cluster_test.go b/pkg/cloud/services/eks/cluster_test.go index bebc8fe23b..b9c95c6946 100644 --- a/pkg/cloud/services/eks/cluster_test.go +++ b/pkg/cloud/services/eks/cluster_test.go @@ -853,6 +853,7 @@ func TestCreateIPv6Cluster(t *testing.T) { eksMock.EXPECT().CreateCluster(&eks.CreateClusterInput{ Name: aws.String("cluster-name"), Version: aws.String("1.22"), + RoleArn: aws.String("arn:role"), EncryptionConfig: []*eks.EncryptionConfig{ { Provider: &eks.Provider{ @@ -875,6 +876,7 @@ func TestCreateIPv6Cluster(t *testing.T) { RoleName: aws.String("arn-role"), }).Return(&iam.GetRoleOutput{ Role: &iam.Role{ + Arn: aws.String("arn:role"), RoleName: ptr.To[string]("arn-role"), }, }, nil) From 62e41373c9d84b8e52ada5b92dd6c711de070d3b Mon Sep 17 00:00:00 2001 From: Adam Malcontenti-Wilson Date: Tue, 17 Sep 2024 16:49:59 +1000 Subject: [PATCH 08/19] add accessConfig to v1beta1 type as well --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 15 +++++++++ .../v1beta1/awsmanagedcontrolplane_types.go | 13 ++++++++ controlplane/eks/api/v1beta1/types.go | 15 +++++++++ .../api/v1beta1/zz_generated.conversion.go | 33 ++++++++++++++++++- .../eks/api/v1beta1/zz_generated.deepcopy.go | 20 +++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 4a25036cac..aaa39720c0 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -67,6 +67,21 @@ spec: description: AWSManagedControlPlaneSpec defines the desired state of an Amazon EKS Cluster. properties: + accessConfig: + description: AccessConfig specifies the access configuration information + for the cluster + properties: + authenticationMode: + default: CONFIG_MAP + description: |- + AuthenticationMode specifies the desired authentication mode for the cluster + Defaults to CONFIG_MAP + enum: + - CONFIG_MAP + - API + - API_AND_CONFIG_MAP + type: string + type: object additionalTags: additionalProperties: type: string diff --git a/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go index a965bef381..97398e4e03 100644 --- a/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go @@ -165,6 +165,10 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned // +optional OIDCIdentityProviderConfig *OIDCIdentityProviderConfig `json:"oidcIdentityProviderConfig,omitempty"` + // AccessConfig specifies the access configuration information for the cluster + // +optional + AccessConfig *AccessConfig `json:"accessConfig,omitempty"` + // DisableVPCCNI indicates that the Amazon VPC CNI should be disabled. With EKS clusters the // Amazon VPC CNI is automatically installed into the cluster. For clusters where you want // to use an alternate CNI this option provides a way to specify that the Amazon VPC CNI @@ -212,6 +216,15 @@ type EndpointAccess struct { Private *bool `json:"private,omitempty"` } +// AccessConfig represents the access configuration information for the cluster +type AccessConfig struct { + // AuthenticationMode specifies the desired authentication mode for the cluster + // Defaults to CONFIG_MAP + // +kubebuilder:default=CONFIG_MAP + // +kubebuilder:validation:Enum=CONFIG_MAP;API;API_AND_CONFIG_MAP + AuthenticationMode EKSAuthenticationMode `json:"authenticationMode,omitempty"` +} + // EncryptionConfig specifies the encryption configuration for the EKS clsuter. type EncryptionConfig struct { // Provider specifies the ARN or alias of the CMK (in AWS KMS) diff --git a/controlplane/eks/api/v1beta1/types.go b/controlplane/eks/api/v1beta1/types.go index 0ca9a64ebe..73370445ad 100644 --- a/controlplane/eks/api/v1beta1/types.go +++ b/controlplane/eks/api/v1beta1/types.go @@ -79,6 +79,21 @@ var ( EKSTokenMethodAWSCli = EKSTokenMethod("aws-cli") ) +// EKSAuthenticationMode defines the authentication mode for the cluster +type EKSAuthenticationMode string + +var ( + // EKSAuthenticationModeConfigMap indicates that only `aws-auth` ConfigMap will be used for authentication + EKSAuthenticationModeConfigMap = EKSAuthenticationMode("CONFIG_MAP") + + // EKSAuthenticationModeAPI indicates that only AWS Access Entries will be used for authentication + EKSAuthenticationModeAPI = EKSAuthenticationMode("API") + + // EKSAuthenticationModeAPIAndConfigMap indicates that both `aws-auth` ConfigMap and AWS Access Entries will + // be used for authentication + EKSAuthenticationModeAPIAndConfigMap = EKSAuthenticationMode("API_AND_CONFIG_MAP") +) + var ( // DefaultEKSControlPlaneRole is the name of the default IAM role to use for the EKS control plane // if no other role is supplied in the spec and if iam role creation is not enabled. The default diff --git a/controlplane/eks/api/v1beta1/zz_generated.conversion.go b/controlplane/eks/api/v1beta1/zz_generated.conversion.go index ba4acbbfd7..9229a8120c 100644 --- a/controlplane/eks/api/v1beta1/zz_generated.conversion.go +++ b/controlplane/eks/api/v1beta1/zz_generated.conversion.go @@ -70,6 +70,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*AccessConfig)(nil), (*v1beta2.AccessConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(a.(*AccessConfig), b.(*v1beta2.AccessConfig), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*v1beta2.AccessConfig)(nil), (*AccessConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(a.(*v1beta2.AccessConfig), b.(*AccessConfig), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*Addon)(nil), (*v1beta2.Addon)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_Addon_To_v1beta2_Addon(a.(*Addon), b.(*v1beta2.Addon), scope) }); err != nil { @@ -353,6 +363,7 @@ func autoConvert_v1beta1_AWSManagedControlPlaneSpec_To_v1beta2_AWSManagedControl out.AssociateOIDCProvider = in.AssociateOIDCProvider out.Addons = (*[]v1beta2.Addon)(unsafe.Pointer(in.Addons)) out.OIDCIdentityProviderConfig = (*v1beta2.OIDCIdentityProviderConfig)(unsafe.Pointer(in.OIDCIdentityProviderConfig)) + out.AccessConfig = (*v1beta2.AccessConfig)(unsafe.Pointer(in.AccessConfig)) // WARNING: in.DisableVPCCNI requires manual conversion: does not exist in peer-type if err := Convert_v1beta1_VpcCni_To_v1beta2_VpcCni(&in.VpcCni, &out.VpcCni, s); err != nil { return err @@ -390,7 +401,7 @@ func autoConvert_v1beta2_AWSManagedControlPlaneSpec_To_v1beta1_AWSManagedControl out.AssociateOIDCProvider = in.AssociateOIDCProvider out.Addons = (*[]Addon)(unsafe.Pointer(in.Addons)) out.OIDCIdentityProviderConfig = (*OIDCIdentityProviderConfig)(unsafe.Pointer(in.OIDCIdentityProviderConfig)) - // WARNING: in.AccessConfig requires manual conversion: does not exist in peer-type + out.AccessConfig = (*AccessConfig)(unsafe.Pointer(in.AccessConfig)) if err := Convert_v1beta2_VpcCni_To_v1beta1_VpcCni(&in.VpcCni, &out.VpcCni, s); err != nil { return err } @@ -449,6 +460,26 @@ func Convert_v1beta2_AWSManagedControlPlaneStatus_To_v1beta1_AWSManagedControlPl return autoConvert_v1beta2_AWSManagedControlPlaneStatus_To_v1beta1_AWSManagedControlPlaneStatus(in, out, s) } +func autoConvert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in *AccessConfig, out *v1beta2.AccessConfig, s conversion.Scope) error { + out.AuthenticationMode = v1beta2.EKSAuthenticationMode(in.AuthenticationMode) + return nil +} + +// Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig is an autogenerated conversion function. +func Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in *AccessConfig, out *v1beta2.AccessConfig, s conversion.Scope) error { + return autoConvert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in, out, s) +} + +func autoConvert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in *v1beta2.AccessConfig, out *AccessConfig, s conversion.Scope) error { + out.AuthenticationMode = EKSAuthenticationMode(in.AuthenticationMode) + return nil +} + +// Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig is an autogenerated conversion function. +func Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in *v1beta2.AccessConfig, out *AccessConfig, s conversion.Scope) error { + return autoConvert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in, out, s) +} + func autoConvert_v1beta1_Addon_To_v1beta2_Addon(in *Addon, out *v1beta2.Addon, s conversion.Scope) error { out.Name = in.Name out.Version = in.Version diff --git a/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go b/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go index e2372492a6..118c717645 100644 --- a/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go +++ b/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go @@ -170,6 +170,11 @@ func (in *AWSManagedControlPlaneSpec) DeepCopyInto(out *AWSManagedControlPlaneSp *out = new(OIDCIdentityProviderConfig) (*in).DeepCopyInto(*out) } + if in.AccessConfig != nil { + in, out := &in.AccessConfig, &out.AccessConfig + *out = new(AccessConfig) + **out = **in + } in.VpcCni.DeepCopyInto(&out.VpcCni) out.KubeProxy = in.KubeProxy } @@ -238,6 +243,21 @@ func (in *AWSManagedControlPlaneStatus) DeepCopy() *AWSManagedControlPlaneStatus return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessConfig) DeepCopyInto(out *AccessConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessConfig. +func (in *AccessConfig) DeepCopy() *AccessConfig { + if in == nil { + return nil + } + out := new(AccessConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Addon) DeepCopyInto(out *Addon) { *out = *in From 39b209c22d5c30c4da79f940ac052a5418da5f74 Mon Sep 17 00:00:00 2001 From: Josh French Date: Tue, 1 Jul 2025 18:03:07 -0400 Subject: [PATCH 09/19] Revert "add accessConfig to v1beta1 type as well" This reverts commit 62e41373c9d84b8e52ada5b92dd6c711de070d3b. --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 15 --------- .../v1beta1/awsmanagedcontrolplane_types.go | 13 -------- controlplane/eks/api/v1beta1/types.go | 15 --------- .../api/v1beta1/zz_generated.conversion.go | 33 +------------------ .../eks/api/v1beta1/zz_generated.deepcopy.go | 20 ----------- 5 files changed, 1 insertion(+), 95 deletions(-) diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index aaa39720c0..4a25036cac 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -67,21 +67,6 @@ spec: description: AWSManagedControlPlaneSpec defines the desired state of an Amazon EKS Cluster. properties: - accessConfig: - description: AccessConfig specifies the access configuration information - for the cluster - properties: - authenticationMode: - default: CONFIG_MAP - description: |- - AuthenticationMode specifies the desired authentication mode for the cluster - Defaults to CONFIG_MAP - enum: - - CONFIG_MAP - - API - - API_AND_CONFIG_MAP - type: string - type: object additionalTags: additionalProperties: type: string diff --git a/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go index 97398e4e03..a965bef381 100644 --- a/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta1/awsmanagedcontrolplane_types.go @@ -165,10 +165,6 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned // +optional OIDCIdentityProviderConfig *OIDCIdentityProviderConfig `json:"oidcIdentityProviderConfig,omitempty"` - // AccessConfig specifies the access configuration information for the cluster - // +optional - AccessConfig *AccessConfig `json:"accessConfig,omitempty"` - // DisableVPCCNI indicates that the Amazon VPC CNI should be disabled. With EKS clusters the // Amazon VPC CNI is automatically installed into the cluster. For clusters where you want // to use an alternate CNI this option provides a way to specify that the Amazon VPC CNI @@ -216,15 +212,6 @@ type EndpointAccess struct { Private *bool `json:"private,omitempty"` } -// AccessConfig represents the access configuration information for the cluster -type AccessConfig struct { - // AuthenticationMode specifies the desired authentication mode for the cluster - // Defaults to CONFIG_MAP - // +kubebuilder:default=CONFIG_MAP - // +kubebuilder:validation:Enum=CONFIG_MAP;API;API_AND_CONFIG_MAP - AuthenticationMode EKSAuthenticationMode `json:"authenticationMode,omitempty"` -} - // EncryptionConfig specifies the encryption configuration for the EKS clsuter. type EncryptionConfig struct { // Provider specifies the ARN or alias of the CMK (in AWS KMS) diff --git a/controlplane/eks/api/v1beta1/types.go b/controlplane/eks/api/v1beta1/types.go index 73370445ad..0ca9a64ebe 100644 --- a/controlplane/eks/api/v1beta1/types.go +++ b/controlplane/eks/api/v1beta1/types.go @@ -79,21 +79,6 @@ var ( EKSTokenMethodAWSCli = EKSTokenMethod("aws-cli") ) -// EKSAuthenticationMode defines the authentication mode for the cluster -type EKSAuthenticationMode string - -var ( - // EKSAuthenticationModeConfigMap indicates that only `aws-auth` ConfigMap will be used for authentication - EKSAuthenticationModeConfigMap = EKSAuthenticationMode("CONFIG_MAP") - - // EKSAuthenticationModeAPI indicates that only AWS Access Entries will be used for authentication - EKSAuthenticationModeAPI = EKSAuthenticationMode("API") - - // EKSAuthenticationModeAPIAndConfigMap indicates that both `aws-auth` ConfigMap and AWS Access Entries will - // be used for authentication - EKSAuthenticationModeAPIAndConfigMap = EKSAuthenticationMode("API_AND_CONFIG_MAP") -) - var ( // DefaultEKSControlPlaneRole is the name of the default IAM role to use for the EKS control plane // if no other role is supplied in the spec and if iam role creation is not enabled. The default diff --git a/controlplane/eks/api/v1beta1/zz_generated.conversion.go b/controlplane/eks/api/v1beta1/zz_generated.conversion.go index 9229a8120c..ba4acbbfd7 100644 --- a/controlplane/eks/api/v1beta1/zz_generated.conversion.go +++ b/controlplane/eks/api/v1beta1/zz_generated.conversion.go @@ -70,16 +70,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*AccessConfig)(nil), (*v1beta2.AccessConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(a.(*AccessConfig), b.(*v1beta2.AccessConfig), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1beta2.AccessConfig)(nil), (*AccessConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(a.(*v1beta2.AccessConfig), b.(*AccessConfig), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Addon)(nil), (*v1beta2.Addon)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_Addon_To_v1beta2_Addon(a.(*Addon), b.(*v1beta2.Addon), scope) }); err != nil { @@ -363,7 +353,6 @@ func autoConvert_v1beta1_AWSManagedControlPlaneSpec_To_v1beta2_AWSManagedControl out.AssociateOIDCProvider = in.AssociateOIDCProvider out.Addons = (*[]v1beta2.Addon)(unsafe.Pointer(in.Addons)) out.OIDCIdentityProviderConfig = (*v1beta2.OIDCIdentityProviderConfig)(unsafe.Pointer(in.OIDCIdentityProviderConfig)) - out.AccessConfig = (*v1beta2.AccessConfig)(unsafe.Pointer(in.AccessConfig)) // WARNING: in.DisableVPCCNI requires manual conversion: does not exist in peer-type if err := Convert_v1beta1_VpcCni_To_v1beta2_VpcCni(&in.VpcCni, &out.VpcCni, s); err != nil { return err @@ -401,7 +390,7 @@ func autoConvert_v1beta2_AWSManagedControlPlaneSpec_To_v1beta1_AWSManagedControl out.AssociateOIDCProvider = in.AssociateOIDCProvider out.Addons = (*[]Addon)(unsafe.Pointer(in.Addons)) out.OIDCIdentityProviderConfig = (*OIDCIdentityProviderConfig)(unsafe.Pointer(in.OIDCIdentityProviderConfig)) - out.AccessConfig = (*AccessConfig)(unsafe.Pointer(in.AccessConfig)) + // WARNING: in.AccessConfig requires manual conversion: does not exist in peer-type if err := Convert_v1beta2_VpcCni_To_v1beta1_VpcCni(&in.VpcCni, &out.VpcCni, s); err != nil { return err } @@ -460,26 +449,6 @@ func Convert_v1beta2_AWSManagedControlPlaneStatus_To_v1beta1_AWSManagedControlPl return autoConvert_v1beta2_AWSManagedControlPlaneStatus_To_v1beta1_AWSManagedControlPlaneStatus(in, out, s) } -func autoConvert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in *AccessConfig, out *v1beta2.AccessConfig, s conversion.Scope) error { - out.AuthenticationMode = v1beta2.EKSAuthenticationMode(in.AuthenticationMode) - return nil -} - -// Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig is an autogenerated conversion function. -func Convert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in *AccessConfig, out *v1beta2.AccessConfig, s conversion.Scope) error { - return autoConvert_v1beta1_AccessConfig_To_v1beta2_AccessConfig(in, out, s) -} - -func autoConvert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in *v1beta2.AccessConfig, out *AccessConfig, s conversion.Scope) error { - out.AuthenticationMode = EKSAuthenticationMode(in.AuthenticationMode) - return nil -} - -// Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig is an autogenerated conversion function. -func Convert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in *v1beta2.AccessConfig, out *AccessConfig, s conversion.Scope) error { - return autoConvert_v1beta2_AccessConfig_To_v1beta1_AccessConfig(in, out, s) -} - func autoConvert_v1beta1_Addon_To_v1beta2_Addon(in *Addon, out *v1beta2.Addon, s conversion.Scope) error { out.Name = in.Name out.Version = in.Version diff --git a/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go b/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go index 118c717645..e2372492a6 100644 --- a/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go +++ b/controlplane/eks/api/v1beta1/zz_generated.deepcopy.go @@ -170,11 +170,6 @@ func (in *AWSManagedControlPlaneSpec) DeepCopyInto(out *AWSManagedControlPlaneSp *out = new(OIDCIdentityProviderConfig) (*in).DeepCopyInto(*out) } - if in.AccessConfig != nil { - in, out := &in.AccessConfig, &out.AccessConfig - *out = new(AccessConfig) - **out = **in - } in.VpcCni.DeepCopyInto(&out.VpcCni) out.KubeProxy = in.KubeProxy } @@ -243,21 +238,6 @@ func (in *AWSManagedControlPlaneStatus) DeepCopy() *AWSManagedControlPlaneStatus return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AccessConfig) DeepCopyInto(out *AccessConfig) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessConfig. -func (in *AccessConfig) DeepCopy() *AccessConfig { - if in == nil { - return nil - } - out := new(AccessConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Addon) DeepCopyInto(out *Addon) { *out = *in From 4fdb6c282823bec515f181d4e9ff935965fcec3f Mon Sep 17 00:00:00 2001 From: Josh French Date: Tue, 1 Jul 2025 16:54:38 -0400 Subject: [PATCH 10/19] preserve AccessConfig during AWSManagedControlPlane API conversions --- controlplane/eks/api/v1beta1/conversion.go | 1 + 1 file changed, 1 insertion(+) diff --git a/controlplane/eks/api/v1beta1/conversion.go b/controlplane/eks/api/v1beta1/conversion.go index e137e7dede..415e25f1d4 100644 --- a/controlplane/eks/api/v1beta1/conversion.go +++ b/controlplane/eks/api/v1beta1/conversion.go @@ -41,6 +41,7 @@ func (r *AWSManagedControlPlane) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.VpcCni.Disable = r.Spec.DisableVPCCNI dst.Spec.Partition = restored.Spec.Partition dst.Spec.RestrictPrivateSubnets = restored.Spec.RestrictPrivateSubnets + dst.Spec.AccessConfig = restored.Spec.AccessConfig return nil } From 7a73ff780c9614fbc5da933a4d9b4341af008da0 Mon Sep 17 00:00:00 2001 From: Josh French Date: Wed, 2 Jul 2025 09:15:44 -0400 Subject: [PATCH 11/19] cruft --- Dockerfile | 3 +-- Makefile | 9 +++------ ...olplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml | 2 +- .../eks/api/v1beta2/awsmanagedcontrolplane_types.go | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Dockerfile b/Dockerfile index b28186a6c4..155b9519a7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -41,11 +41,10 @@ COPY ./ ./ ARG package=. ARG ARCH ARG LDFLAGS -ARG GCFLAGS RUN --mount=type=cache,target=/root/.cache/go-build \ --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.local/share/golang \ - CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -gcflags "${GCFLAGS}" -ldflags "${LDFLAGS} -extldflags '-static'" -o manager ${package} + CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags "${LDFLAGS} -extldflags '-static'" -o manager ${package} ENTRYPOINT [ "/start.sh", "/workspace/manager" ] # Copy the controller-manager into a thin image diff --git a/Makefile b/Makefile index 4efc4b49f8..84e7b00d8f 100644 --- a/Makefile +++ b/Makefile @@ -137,9 +137,6 @@ RBAC_ROOT ?= $(MANIFEST_ROOT)/rbac # Allow overriding the imagePullPolicy PULL_POLICY ?= Always -# Allow overriding the GCFLAGS -GCFLAGS ?= - # Set build time variables including version details LDFLAGS := $(shell source ./hack/version.sh; version::ldflags) @@ -384,12 +381,12 @@ binaries: managers clusterawsadm ## Builds and installs all binaries .PHONY: clusterawsadm clusterawsadm: ## Build clusterawsadm binary - go build -gcflags "$(GCFLAGS)" -ldflags "$(LDFLAGS)" -o $(BIN_DIR)/clusterawsadm ./cmd/clusterawsadm + go build -ldflags "$(LDFLAGS)" -o $(BIN_DIR)/clusterawsadm ./cmd/clusterawsadm .PHONY: docker-build docker-build: docker-pull-prerequisites ## Build the docker image for controller-manager - docker build --build-arg ARCH=$(ARCH) --build-arg builder_image=$(GO_CONTAINER_IMAGE) --build-arg GCFLAGS="$(GCFLAGS)" --build-arg LDFLAGS="$(LDFLAGS)" . -t $(CORE_CONTROLLER_IMG)-$(ARCH):$(TAG) + docker build --build-arg ARCH=$(ARCH) --build-arg builder_image=$(GO_CONTAINER_IMAGE) --build-arg LDFLAGS="$(LDFLAGS)" . -t $(CORE_CONTROLLER_IMG)-$(ARCH):$(TAG) .PHONY: docker-build-all ## Build all the architecture docker images docker-build-all: $(addprefix docker-build-,$(ALL_ARCH)) @@ -408,7 +405,7 @@ managers: ## Alias for manager-aws-infrastructure .PHONY: manager-aws-infrastructure manager-aws-infrastructure: ## Build manager binary - CGO_ENABLED=0 GOARCH=${ARCH} go build -gcflags "${GCFLAGS}" -ldflags "${LDFLAGS} -extldflags '-static'" -o $(BIN_DIR)/manager . + CGO_ENABLED=0 GOARCH=${ARCH} go build -ldflags "${LDFLAGS} -extldflags '-static'" -o $(BIN_DIR)/manager . ##@ test: diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 06ed84dd99..10a3b96c39 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -3025,7 +3025,7 @@ spec: type: object oidcIdentityProviderConfig: description: |- - OIDCIdentityProviderConfig is used to specify the oidc provider config + IdentityProviderconfig is used to specify the oidc provider config to be attached with this eks cluster properties: clientId: diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go index 4144f654e5..406d9b9921 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go @@ -188,7 +188,7 @@ type AWSManagedControlPlaneSpec struct { //nolint: maligned // +optional Addons *[]Addon `json:"addons,omitempty"` - // OIDCIdentityProviderConfig is used to specify the oidc provider config + // IdentityProviderconfig is used to specify the oidc provider config // to be attached with this eks cluster // +optional OIDCIdentityProviderConfig *OIDCIdentityProviderConfig `json:"oidcIdentityProviderConfig,omitempty"` From c8573f728fe5bd44b050655b1e0e89236d02fed4 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Fri, 27 Jun 2025 11:07:18 +0100 Subject: [PATCH 12/19] Create multiple control plane loadbalancers concurrently --- pkg/cloud/services/elb/loadbalancer.go | 131 +++++++++----- pkg/cloud/services/elb/loadbalancer_test.go | 184 ++++++++++++++++---- 2 files changed, 243 insertions(+), 72 deletions(-) diff --git a/pkg/cloud/services/elb/loadbalancer.go b/pkg/cloud/services/elb/loadbalancer.go index 5af91b26c5..c2265012a6 100644 --- a/pkg/cloud/services/elb/loadbalancer.go +++ b/pkg/cloud/services/elb/loadbalancer.go @@ -67,11 +67,21 @@ const additionalTargetGroupPrefix = "additional-listener-" // cantAttachSGToNLBRegions is a set of regions that do not support Security Groups in NLBs. var cantAttachSGToNLBRegions = sets.New("us-iso-east-1", "us-iso-west-1", "us-isob-east-1") +type lbReconciler func() error + // ReconcileLoadbalancers reconciles the load balancers for the given cluster. func (s *Service) ReconcileLoadbalancers() error { s.scope.Debug("Reconciling load balancers") var errs []error + var lbReconcilers []lbReconciler + + // The following splits load balancer reconciliation into 2 phases: + // 1. Get or create the load balancer + // 2. Reconcile the load balancer + // We ensure that we only wait for the load balancer to become available in + // the reconcile phase. This is useful when creating multiple load + // balancers, as they can take several minutes to become available. for _, lbSpec := range s.scope.ControlPlaneLoadBalancers() { if lbSpec == nil { @@ -79,48 +89,72 @@ func (s *Service) ReconcileLoadbalancers() error { } switch lbSpec.LoadBalancerType { case infrav1.LoadBalancerTypeClassic: - errs = append(errs, s.reconcileClassicLoadBalancer()) + reconciler, err := s.getOrCreateClassicLoadBalancer() + if err != nil { + errs = append(errs, err) + } else { + lbReconcilers = append(lbReconcilers, reconciler) + } case infrav1.LoadBalancerTypeNLB, infrav1.LoadBalancerTypeALB, infrav1.LoadBalancerTypeELB: - errs = append(errs, s.reconcileV2LB(lbSpec)) + reconciler, err := s.getOrCreateV2LB(lbSpec) + if err != nil { + errs = append(errs, err) + } else { + lbReconcilers = append(lbReconcilers, reconciler) + } default: errs = append(errs, fmt.Errorf("unknown or unsupported load balancer type on primary load balancer: %s", lbSpec.LoadBalancerType)) } } + // Reconcile all load balancers + for _, reconciler := range lbReconcilers { + if err := reconciler(); err != nil { + errs = append(errs, err) + } + } + return kerrors.NewAggregate(errs) } -// reconcileV2LB creates a load balancer. It also takes care of generating unique names across -// namespaces by appending the namespace to the name. -func (s *Service) reconcileV2LB(lbSpec *infrav1.AWSLoadBalancerSpec) error { +// getOrCreateV2LB gets an existing load balancer, or creates a new one if it does not exist. +// It also takes care of generating unique names across namespaces by appending the namespace to the name. +// It returns a function that reconciles the load balancer. +func (s *Service) getOrCreateV2LB(lbSpec *infrav1.AWSLoadBalancerSpec) (lbReconciler, error) { name, err := LBName(s.scope, lbSpec) if err != nil { - return errors.Wrap(err, "failed to get control plane load balancer name") + return nil, errors.Wrap(err, "failed to get control plane load balancer name") } // Get default api server spec. desiredLB, err := s.getAPIServerLBSpec(name, lbSpec) if err != nil { - return err + return nil, err } lb, err := s.describeLB(name, lbSpec) switch { case IsNotFound(err) && s.scope.ControlPlaneEndpoint().IsValid(): // if elb is not found and owner cluster ControlPlaneEndpoint is already populated, then we should not recreate the elb. - return errors.Wrapf(err, "no loadbalancer exists for the AWSCluster %s, the cluster has become unrecoverable and should be deleted manually", s.scope.InfraClusterName()) + return nil, errors.Wrapf(err, "no loadbalancer exists for the AWSCluster %s, the cluster has become unrecoverable and should be deleted manually", s.scope.InfraClusterName()) case IsNotFound(err): lb, err = s.createLB(desiredLB, lbSpec) if err != nil { s.scope.Error(err, "failed to create LB") - return err + return nil, err } s.scope.Debug("Created new network load balancer for apiserver", "api-server-lb-name", lb.Name) case err != nil: // Failed to describe the classic ELB - return err + return nil, err } + return func() error { + return s.reconcileV2LB(lb, desiredLB, lbSpec) + }, nil +} + +func (s *Service) reconcileV2LB(lb *infrav1.LoadBalancer, desiredLB *infrav1.LoadBalancer, lbSpec *infrav1.AWSLoadBalancerSpec) error { wReq := &elbv2.DescribeLoadBalancersInput{ LoadBalancerArns: aws.StringSlice([]string{lb.ARN}), } @@ -442,12 +476,21 @@ func (s *Service) createLB(spec *infrav1.LoadBalancer, lbSpec *infrav1.AWSLoadBa // Target Groups and listeners will be reconciled separately - s.scope.Info("Created network load balancer", "dns-name", *out.LoadBalancers[0].DNSName) + if out.LoadBalancers[0].DNSName == nil { + return nil, fmt.Errorf("CreateLoadBalancer did not return a DNS name for %s", spec.Name) + } + dnsName := *out.LoadBalancers[0].DNSName + if out.LoadBalancers[0].LoadBalancerArn == nil { + return nil, fmt.Errorf("CreateLoadBalancer did not return an ARN for %s", spec.Name) + } + arn := *out.LoadBalancers[0].LoadBalancerArn + + s.scope.Info("Created network load balancer", "dns-name", dnsName) res := spec.DeepCopy() - s.scope.Debug("applying load balancer DNS to result", "dns", *out.LoadBalancers[0].DNSName) - res.DNSName = *out.LoadBalancers[0].DNSName - res.ARN = *out.LoadBalancers[0].LoadBalancerArn + s.scope.Debug("applying load balancer DNS to result", "dns", dnsName) + res.DNSName = dnsName + res.ARN = arn return res, nil } @@ -507,37 +550,46 @@ func (s *Service) describeLB(name string, lbSpec *infrav1.AWSLoadBalancerSpec) ( return fromSDKTypeToLB(out.LoadBalancers[0], outAtt.Attributes, tags), nil } -func (s *Service) reconcileClassicLoadBalancer() error { +// getOrCreateClassicLoadBalancer gets an existing classic load balancer, or creates a new one if it does not exist. +// It also takes care of generating unique names across namespaces by appending the namespace to the name. +// It returns a function that reconciles the load balancer. +func (s *Service) getOrCreateClassicLoadBalancer() (lbReconciler, error) { // Generate a default control plane load balancer name. The load balancer name cannot be // generated by the defaulting webhook, because it is derived from the cluster name, and that // name is undefined at defaulting time when generateName is used. name, err := ELBName(s.scope) if err != nil { - return errors.Wrap(err, "failed to get control plane load balancer name") + return nil, errors.Wrap(err, "failed to get control plane load balancer name") } // Get default api server spec. spec, err := s.getAPIServerClassicELBSpec(name) if err != nil { - return err + return nil, err } apiELB, err := s.describeClassicELB(spec.Name) switch { case IsNotFound(err) && s.scope.ControlPlaneEndpoint().IsValid(): // if elb is not found and owner cluster ControlPlaneEndpoint is already populated, then we should not recreate the elb. - return errors.Wrapf(err, "no loadbalancer exists for the AWSCluster %s, the cluster has become unrecoverable and should be deleted manually", s.scope.InfraClusterName()) + return nil, errors.Wrapf(err, "no loadbalancer exists for the AWSCluster %s, the cluster has become unrecoverable and should be deleted manually", s.scope.InfraClusterName()) case IsNotFound(err): apiELB, err = s.createClassicELB(spec) if err != nil { - return err + return nil, err } s.scope.Debug("Created new classic load balancer for apiserver", "api-server-elb-name", apiELB.Name) case err != nil: // Failed to describe the classic ELB - return err + return nil, err } + return func() error { + return s.reconcileClassicLoadBalancer(apiELB, spec) + }, nil +} + +func (s *Service) reconcileClassicLoadBalancer(apiELB *infrav1.LoadBalancer, spec *infrav1.LoadBalancer) error { if apiELB.IsManaged(s.scope.Name()) { if !cmp.Equal(spec.ClassicElbAttributes, apiELB.ClassicElbAttributes) { err := s.configureAttributes(apiELB.Name, spec.ClassicElbAttributes) @@ -546,6 +598,9 @@ func (s *Service) reconcileClassicLoadBalancer() error { } } + // BUG: note that describeClassicELB doesn't set HealthCheck in its output, + // so we're configuring the health check on every reconcile whether it's + // needed or not. if !cmp.Equal(spec.HealthCheck, apiELB.HealthCheck) { s.scope.Debug("Reconciling health check for apiserver load balancer", "health-check", spec.HealthCheck) err := s.configureHealthCheck(apiELB.Name, spec.HealthCheck) @@ -597,7 +652,7 @@ func (s *Service) reconcileClassicLoadBalancer() error { } func (s *Service) configureHealthCheck(name string, healthCheck *infrav1.ClassicELBHealthCheck) error { - if _, err := s.ELBClient.ConfigureHealthCheck(&elb.ConfigureHealthCheckInput{ + healthCheckInput := &elb.ConfigureHealthCheckInput{ LoadBalancerName: aws.String(name), HealthCheck: &elb.HealthCheck{ Target: aws.String(healthCheck.Target), @@ -606,7 +661,14 @@ func (s *Service) configureHealthCheck(name string, healthCheck *infrav1.Classic HealthyThreshold: aws.Int64(healthCheck.HealthyThreshold), UnhealthyThreshold: aws.Int64(healthCheck.UnhealthyThreshold), }, - }); err != nil { + } + + if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { + if _, err := s.ELBClient.ConfigureHealthCheck(healthCheckInput); err != nil { + return false, err + } + return true, nil + }, awserrors.LoadBalancerNotFound); err != nil { return errors.Wrapf(err, "failed to configure health check for classic load balancer: %s", name) } return nil @@ -1193,30 +1255,15 @@ func (s *Service) createClassicELB(spec *infrav1.LoadBalancer) (*infrav1.LoadBal return nil, errors.Wrapf(err, "failed to create classic load balancer: %v", spec) } - if spec.HealthCheck != nil { - if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) { - if _, err := s.ELBClient.ConfigureHealthCheck(&elb.ConfigureHealthCheckInput{ - LoadBalancerName: aws.String(spec.Name), - HealthCheck: &elb.HealthCheck{ - Target: aws.String(spec.HealthCheck.Target), - Interval: aws.Int64(int64(spec.HealthCheck.Interval.Seconds())), - Timeout: aws.Int64(int64(spec.HealthCheck.Timeout.Seconds())), - HealthyThreshold: aws.Int64(spec.HealthCheck.HealthyThreshold), - UnhealthyThreshold: aws.Int64(spec.HealthCheck.UnhealthyThreshold), - }, - }); err != nil { - return false, err - } - return true, nil - }, awserrors.LoadBalancerNotFound); err != nil { - return nil, errors.Wrapf(err, "failed to configure health check for classic load balancer: %v", spec) - } - } - s.scope.Info("Created classic load balancer", "dns-name", *out.DNSName) res := spec.DeepCopy() res.DNSName = *out.DNSName + + // We haven't configured any health check yet. Don't report it here so it + // will be set later during reconciliation. + res.HealthCheck = nil + return res, nil } diff --git a/pkg/cloud/services/elb/loadbalancer_test.go b/pkg/cloud/services/elb/loadbalancer_test.go index 073c64e0b2..7a578fd7dd 100644 --- a/pkg/cloud/services/elb/loadbalancer_test.go +++ b/pkg/cloud/services/elb/loadbalancer_test.go @@ -2546,7 +2546,10 @@ func TestReconcileV2LB(t *testing.T) { scope: clusterScope, ELBV2Client: elbV2APIMocks, } - err = s.reconcileV2LB(clusterScope.ControlPlaneLoadBalancer()) + reconciler, err := s.getOrCreateV2LB(clusterScope.ControlPlaneLoadBalancer()) + if err == nil { + err = reconciler() + } lb := s.scope.Network().APIServerELB tc.check(t, &lb, err) @@ -2567,9 +2570,41 @@ func TestReconcileLoadbalancers(t *testing.T) { az = "us-west-1a" ) + primaryELB := func() *elbv2.LoadBalancer { + return &elbv2.LoadBalancer{ + LoadBalancerArn: aws.String(elbArn), + LoadBalancerName: aws.String(elbName), + DNSName: aws.String(elbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternetFacing)), + AvailabilityZones: []*elbv2.AvailabilityZone{ + { + SubnetId: aws.String(clusterSubnetID), + ZoneName: aws.String(az), + }, + }, + VpcId: aws.String(vpcID), + } + } + + secondaryELB := func() *elbv2.LoadBalancer { + return &elbv2.LoadBalancer{ + LoadBalancerArn: aws.String(secondElbArn), + LoadBalancerName: aws.String(secondElbName), + DNSName: aws.String(secondElbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternal)), + AvailabilityZones: []*elbv2.AvailabilityZone{ + { + SubnetId: aws.String(clusterSubnetID), + ZoneName: aws.String(az), + }, + }, + VpcId: aws.String(vpcID), + } + } + tests := []struct { name string - elbV2APIMocks func(m *mocks.MockELBV2APIMockRecorder) + elbV2APIMocks func(*mocks.MockELBV2APIMockRecorder) check func(t *testing.T, firstLB, secondLB *infrav1.LoadBalancer, err error) awsCluster func(acl infrav1.AWSCluster) infrav1.AWSCluster spec func(spec infrav1.LoadBalancer) infrav1.LoadBalancer @@ -2590,20 +2625,7 @@ func TestReconcileLoadbalancers(t *testing.T) { Names: aws.StringSlice([]string{elbName}), })). Return(&elbv2.DescribeLoadBalancersOutput{ - LoadBalancers: []*elbv2.LoadBalancer{ - { - LoadBalancerArn: aws.String(elbArn), - LoadBalancerName: aws.String(elbName), - Scheme: aws.String(string(infrav1.ELBSchemeInternetFacing)), - AvailabilityZones: []*elbv2.AvailabilityZone{ - { - SubnetId: aws.String(clusterSubnetID), - ZoneName: aws.String(az), - }, - }, - VpcId: aws.String(vpcID), - }, - }, + LoadBalancers: []*elbv2.LoadBalancer{primaryELB()}, }, nil) m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(elbArn)}).Return( &elbv2.DescribeLoadBalancerAttributesOutput{ @@ -2632,20 +2654,7 @@ func TestReconcileLoadbalancers(t *testing.T) { Names: aws.StringSlice([]string{secondElbName}), })). Return(&elbv2.DescribeLoadBalancersOutput{ - LoadBalancers: []*elbv2.LoadBalancer{ - { - LoadBalancerArn: aws.String(secondElbArn), - LoadBalancerName: aws.String(secondElbName), - Scheme: aws.String(string(infrav1.ELBSchemeInternal)), - AvailabilityZones: []*elbv2.AvailabilityZone{ - { - SubnetId: aws.String(clusterSubnetID), - ZoneName: aws.String(az), - }, - }, - VpcId: aws.String(vpcID), - }, - }, + LoadBalancers: []*elbv2.LoadBalancer{secondaryELB()}, }, nil) m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(secondElbArn)}).Return( &elbv2.DescribeLoadBalancerAttributesOutput{ @@ -2690,6 +2699,121 @@ func TestReconcileLoadbalancers(t *testing.T) { } }, }, + { + name: "ensure two load balancers are created concurrently", + awsCluster: func(acl infrav1.AWSCluster) infrav1.AWSCluster { + acl.Spec.ControlPlaneLoadBalancer.Name = aws.String(elbName) + acl.Spec.SecondaryControlPlaneLoadBalancer = &infrav1.AWSLoadBalancerSpec{ + Name: aws.String(secondElbName), + Scheme: &infrav1.ELBSchemeInternal, + LoadBalancerType: infrav1.LoadBalancerTypeNLB, + } + return acl + }, + elbV2APIMocks: func(m *mocks.MockELBV2APIMockRecorder) { + // Initial DescribeLoadBalancers return empty + m.DescribeLoadBalancers(gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + Names: aws.StringSlice([]string{elbName}), + })).Return(&elbv2.DescribeLoadBalancersOutput{}, nil) + m.DescribeLoadBalancers(gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + Names: aws.StringSlice([]string{secondElbName}), + })).Return(&elbv2.DescribeLoadBalancersOutput{}, nil) + + // Create both load balancers + createLB1 := m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ + Name: aws.String(elbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternetFacing)), + SecurityGroups: []*string{aws.String("")}, + Subnets: []*string{}, + Tags: []*elbv2.Tag{ + { + Key: aws.String("Name"), + Value: aws.String(elbName), + }, + { + Key: aws.String(infrav1.ClusterTagKey(clusterName)), + Value: aws.String(string(infrav1.ResourceLifecycleOwned)), + }, + { + Key: aws.String(infrav1.NameAWSClusterAPIRole), + Value: aws.String(infrav1.APIServerRoleTagValue), + }, + }, + Type: aws.String("network"), + })).Return(&elbv2.CreateLoadBalancerOutput{ + LoadBalancers: []*elbv2.LoadBalancer{primaryELB()}, + }, nil) + + createLB2 := m.CreateLoadBalancer(gomock.Eq(&elbv2.CreateLoadBalancerInput{ + Name: aws.String(secondElbName), + Scheme: aws.String(string(infrav1.ELBSchemeInternal)), + SecurityGroups: []*string{aws.String("")}, + Subnets: []*string{}, + Tags: []*elbv2.Tag{ + { + Key: aws.String("Name"), + Value: aws.String(secondElbName), + }, + { + Key: aws.String(infrav1.ClusterTagKey(clusterName)), + Value: aws.String(string(infrav1.ResourceLifecycleOwned)), + }, + { + Key: aws.String(infrav1.NameAWSClusterAPIRole), + Value: aws.String(infrav1.APIServerRoleTagValue), + }, + }, + Type: aws.String("network"), + })).Return(&elbv2.CreateLoadBalancerOutput{ + LoadBalancers: []*elbv2.LoadBalancer{secondaryELB()}, + }, nil) + + // Assert that we don't wait for either load balancer to be + // available until both load balancers have been created + m.WaitUntilLoadBalancerAvailableWithContext(gomock.Any(), gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + LoadBalancerArns: aws.StringSlice([]string{elbArn}), + })).Return(nil).After(createLB1).After(createLB2) + m.WaitUntilLoadBalancerAvailableWithContext(gomock.Any(), gomock.Eq(&elbv2.DescribeLoadBalancersInput{ + LoadBalancerArns: aws.StringSlice([]string{secondElbArn}), + })).Return(nil).After(createLB1).After(createLB2) + + // Make minimal assertions on other calls not under test + m.DescribeTargetGroups(gomock.Any()).Return(&elbv2.DescribeTargetGroupsOutput{}, nil).AnyTimes() + m.DescribeListeners(gomock.Any()).Return(&elbv2.DescribeListenersOutput{}, nil).AnyTimes() + m.ModifyTargetGroupAttributes(gomock.Any()).Return(nil, nil).AnyTimes() + m.ModifyListener(gomock.Any()).Return(nil, nil).AnyTimes() + m.SetSecurityGroups(gomock.Any()).Return(nil, nil).AnyTimes() + + // These calls return the same info for both load balancers, but it's not important to this test + m.CreateTargetGroup(gomock.Any()).Return(&elbv2.CreateTargetGroupOutput{ + TargetGroups: []*elbv2.TargetGroup{ + { + TargetGroupName: aws.String(elbName), + TargetGroupArn: aws.String(elbArn), + }, + }, + }, nil).AnyTimes() + m.CreateListener(gomock.Any()).Return(&elbv2.CreateListenerOutput{ + Listeners: []*elbv2.Listener{ + { + ListenerArn: aws.String(elbArn), + }, + }, + }, nil).AnyTimes() + }, + check: func(t *testing.T, firstLB, secondLB *infrav1.LoadBalancer, err error) { + t.Helper() + if err != nil { + t.Fatalf("did not expect error: %v", err) + } + if firstLB == nil { + t.Errorf("Expected first LB to be populated, was nil") + } + if secondLB == nil { + t.Errorf("Expected second LB to be populated, was nil") + } + }, + }, } for _, tc := range tests { From a1c815a73dea8cd2eea14d8ec155954eac217a28 Mon Sep 17 00:00:00 2001 From: Pankaj Walke Date: Wed, 2 Jul 2025 03:51:27 -0700 Subject: [PATCH 13/19] =?UTF-8?q?=F0=9F=8C=B1=20=20Migrate=20ServiceLimite?= =?UTF-8?q?rs=20to=20AWS=20SDK=20V2=20(#5574)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Migrate ServiceLimiters to AWS SDK V2 Signed-off-by: Pankaj Walke * fix lint errors Signed-off-by: Pankaj Walke * makefile: bump release-binaries's GOMAXPROCS=2 it was hanging otherwise --------- Signed-off-by: Pankaj Walke Co-authored-by: Damiano Donati --- Makefile | 2 +- pkg/cloud/throttle/throttle.go | 72 ++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 84e7b00d8f..07bf590963 100644 --- a/Makefile +++ b/Makefile @@ -606,7 +606,7 @@ promote-images: $(KPROMO) $(YQ) .PHONY: release-binaries release-binaries: $(GORELEASER) ## Builds only the binaries, not a release. - $(GORELEASER) build --config $(GORELEASER_CONFIG) --snapshot --clean + GOMAXPROCS=2 $(GORELEASER) build --config $(GORELEASER_CONFIG) --snapshot --clean .PHONY: release-staging release-staging: ## Builds and push container images and manifests to the staging bucket. diff --git a/pkg/cloud/throttle/throttle.go b/pkg/cloud/throttle/throttle.go index 77511952b7..128bb121a6 100644 --- a/pkg/cloud/throttle/throttle.go +++ b/pkg/cloud/throttle/throttle.go @@ -18,10 +18,13 @@ limitations under the License. package throttle import ( + "context" "regexp" "strings" + awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware" "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/smithy-go/middleware" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/internal/rate" @@ -52,6 +55,11 @@ func (o *OperationLimiter) Wait(r *request.Request) error { return o.getLimiter().Wait(r.Context()) } +// WaitV2 will wait on a request for AWS SDK V2. +func (o *OperationLimiter) WaitV2(ctx context.Context) error { + return o.getLimiter().Wait(ctx) +} + // Match will match a request. func (o *OperationLimiter) Match(r *request.Request) (bool, error) { if o.regexp == nil { @@ -64,6 +72,19 @@ func (o *OperationLimiter) Match(r *request.Request) (bool, error) { return o.regexp.MatchString(r.Operation.Name), nil } +// MatchV2 will match a request for AWS SDK V2. +func (o *OperationLimiter) MatchV2(ctx context.Context) (bool, error) { + if o.regexp == nil { + var err error + o.regexp, err = regexp.Compile("^" + o.Operation) + if err != nil { + return false, err + } + } + opName := awsmiddleware.GetOperationName(ctx) + return o.regexp.MatchString(opName), nil +} + // LimitRequest will limit a request. func (s ServiceLimiter) LimitRequest(r *request.Request) { if ol, ok := s.matchRequest(r); ok { @@ -71,6 +92,13 @@ func (s ServiceLimiter) LimitRequest(r *request.Request) { } } +// LimitRequestV2 will limit a request for AWS SDK V2. +func (s ServiceLimiter) LimitRequestV2(ctx context.Context) { + if ol, ok := s.matchRequestV2(ctx); ok { + _ = ol.WaitV2(ctx) + } +} + func (o *OperationLimiter) getLimiter() *rate.Limiter { if o.limiter == nil { o.limiter = rate.NewLimiter(o.RefillRate, o.Burst) @@ -92,6 +120,16 @@ func (s ServiceLimiter) ReviewResponse(r *request.Request) { } } +// ReviewResponseV2 will review the limits of a Request's response for AWS SDK V2. +func (s ServiceLimiter) ReviewResponseV2(ctx context.Context, errorCode string) { + switch errorCode { + case "Throttling", "RequestLimitExceeded": + if ol, ok := s.matchRequestV2(ctx); ok { + ol.limiter.ResetTokens() + } + } +} + func (s ServiceLimiter) matchRequest(r *request.Request) (*OperationLimiter, bool) { for _, ol := range s { match, err := ol.Match(r) @@ -104,3 +142,37 @@ func (s ServiceLimiter) matchRequest(r *request.Request) (*OperationLimiter, boo } return nil, false } + +// matchRequestV2 is used for matching request for AWS SDK V2. +func (s ServiceLimiter) matchRequestV2(ctx context.Context) (*OperationLimiter, bool) { + for _, ol := range s { + match, err := ol.MatchV2(ctx) + if err != nil { + return nil, false + } + if match { + return ol, true + } + } + return nil, false +} + +// WithServiceLimiterMiddleware returns ServiceLimiter middleware stack for specified service name. +func WithServiceLimiterMiddleware(limiter *ServiceLimiter) func(stack *middleware.Stack) error { + return func(stack *middleware.Stack) error { + // Inserts service Limiter middleware after RequestContext initialization. + return stack.Finalize.Insert(getServiceLimiterMiddleware(limiter), "capa/RequestMetricContextMiddleware", middleware.After) + } +} + +// getServiceLimiterMiddleware implements serviceLimiter middleware. +func getServiceLimiterMiddleware(limiter *ServiceLimiter) middleware.FinalizeMiddleware { + return middleware.FinalizeMiddlewareFunc("capa/ServiceLimiterMiddleware", func(ctx context.Context, input middleware.FinalizeInput, handler middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) { + limiter.LimitRequestV2(ctx) + + out, metadata, err := handler.HandleFinalize(ctx, input) + smithyErr := awserrors.ParseSmithyError(err) + limiter.ReviewResponseV2(ctx, smithyErr.ErrorCode()) + return out, metadata, err + }) +} From bbfc6de6ce0a733a879f16019f85a20a302279df Mon Sep 17 00:00:00 2001 From: Josh French Date: Wed, 2 Jul 2025 12:25:11 -0400 Subject: [PATCH 14/19] Add AccessEntry to AWSManagedControlPlane API --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 78 +++++++++++++++++++ .../v1beta2/awsmanagedcontrolplane_types.go | 56 +++++++++++++ .../eks/api/v1beta2/zz_generated.deepcopy.go | 72 ++++++++++++++++- 3 files changed, 205 insertions(+), 1 deletion(-) diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 10a3b96c39..145ff7a00c 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -2212,6 +2212,84 @@ spec: description: AccessConfig specifies the access configuration information for the cluster properties: + accessEntries: + description: |- + AccessEntries specifies the access entries for the cluster + Access entries require AuthenticationMode to be either API or API_AND_CONFIG_MAP + items: + description: AccessEntry represents an AWS EKS access entry + for IAM principals + properties: + accessPolicies: + description: |- + AccessPolicies specifies the policies to associate with this access entry + Cannot be specified if Type is EC2_LINUX or EC2_WINDOWS + items: + description: AccessPolicyReference represents a reference + to an AWS EKS access policy + properties: + accessScope: + description: AccessScope specifies the scope for the + policy + properties: + namespaces: + description: |- + Namespaces are the namespaces for the access scope + Only valid when Type is namespace + items: + type: string + type: array + type: + default: cluster + description: Type is the type of access scope. + Defaults to "cluster". + enum: + - cluster + - namespace + type: string + required: + - type + type: object + policyARN: + description: PolicyARN is the Amazon Resource Name + (ARN) of the access policy + type: string + required: + - accessScope + - policyARN + type: object + type: array + kubernetesGroups: + description: |- + KubernetesGroups represents the Kubernetes groups for the access entry + Cannot be specified if Type is EC2_LINUX or EC2_WINDOWS + items: + type: string + type: array + principalARN: + description: PrincipalARN is the Amazon Resource Name (ARN) + of the IAM principal + type: string + type: + default: STANDARD + description: Type is the type of access entry. Defaults + to STANDARD if not specified. + enum: + - STANDARD + - EC2_LINUX + - EC2_WINDOWS + - FARGATE_LINUX + - EC2 + - HYBRID_LINUX + - HYPERPOD_LINUX + type: string + username: + description: Username is the username for the access entry + type: string + required: + - principalARN + type: object + type: array authenticationMode: default: CONFIG_MAP description: |- diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go index 406d9b9921..af0fa4d826 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go @@ -253,6 +253,57 @@ type EndpointAccess struct { Private *bool `json:"private,omitempty"` } +// AccessEntry represents an AWS EKS access entry for IAM principals +type AccessEntry struct { + // PrincipalARN is the Amazon Resource Name (ARN) of the IAM principal + // +kubebuilder:validation:Required + PrincipalARN string `json:"principalARN"` + + // Type is the type of access entry. Defaults to STANDARD if not specified. + // +kubebuilder:default=STANDARD + // +kubebuilder:validation:Enum=STANDARD;EC2_LINUX;EC2_WINDOWS;FARGATE_LINUX;EC2;HYBRID_LINUX;HYPERPOD_LINUX + // +optional + Type string `json:"type,omitempty"` + + // KubernetesGroups represents the Kubernetes groups for the access entry + // Cannot be specified if Type is EC2_LINUX or EC2_WINDOWS + // +optional + KubernetesGroups []string `json:"kubernetesGroups,omitempty"` + + // Username is the username for the access entry + // +optional + Username string `json:"username,omitempty"` + + // AccessPolicies specifies the policies to associate with this access entry + // Cannot be specified if Type is EC2_LINUX or EC2_WINDOWS + // +optional + AccessPolicies []AccessPolicyReference `json:"accessPolicies,omitempty"` +} + +// AccessPolicyReference represents a reference to an AWS EKS access policy +type AccessPolicyReference struct { + // PolicyARN is the Amazon Resource Name (ARN) of the access policy + // +kubebuilder:validation:Required + PolicyARN string `json:"policyARN"` + + // AccessScope specifies the scope for the policy + // +kubebuilder:validation:Required + AccessScope AccessScope `json:"accessScope"` +} + +// AccessScope represents the scope for an access policy +type AccessScope struct { + // Type is the type of access scope. Defaults to "cluster". + // +kubebuilder:validation:Enum=cluster;namespace + // +kubebuilder:default=cluster + Type string `json:"type"` + + // Namespaces are the namespaces for the access scope + // Only valid when Type is namespace + // +optional + Namespaces []string `json:"namespaces,omitempty"` +} + // AccessConfig represents the access configuration information for the cluster type AccessConfig struct { // AuthenticationMode specifies the desired authentication mode for the cluster @@ -260,6 +311,11 @@ type AccessConfig struct { // +kubebuilder:default=CONFIG_MAP // +kubebuilder:validation:Enum=CONFIG_MAP;API;API_AND_CONFIG_MAP AuthenticationMode EKSAuthenticationMode `json:"authenticationMode,omitempty"` + + // AccessEntries specifies the access entries for the cluster + // Access entries require AuthenticationMode to be either API or API_AND_CONFIG_MAP + // +optional + AccessEntries []AccessEntry `json:"accessEntries,omitempty"` } // EncryptionConfig specifies the encryption configuration for the EKS clsuter. diff --git a/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go b/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go index c615dfb731..71ee81d349 100644 --- a/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go +++ b/controlplane/eks/api/v1beta2/zz_generated.deepcopy.go @@ -173,7 +173,7 @@ func (in *AWSManagedControlPlaneSpec) DeepCopyInto(out *AWSManagedControlPlaneSp if in.AccessConfig != nil { in, out := &in.AccessConfig, &out.AccessConfig *out = new(AccessConfig) - **out = **in + (*in).DeepCopyInto(*out) } in.VpcCni.DeepCopyInto(&out.VpcCni) out.KubeProxy = in.KubeProxy @@ -251,6 +251,13 @@ func (in *AWSManagedControlPlaneStatus) DeepCopy() *AWSManagedControlPlaneStatus // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AccessConfig) DeepCopyInto(out *AccessConfig) { *out = *in + if in.AccessEntries != nil { + in, out := &in.AccessEntries, &out.AccessEntries + *out = make([]AccessEntry, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessConfig. @@ -263,6 +270,69 @@ func (in *AccessConfig) DeepCopy() *AccessConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessEntry) DeepCopyInto(out *AccessEntry) { + *out = *in + if in.KubernetesGroups != nil { + in, out := &in.KubernetesGroups, &out.KubernetesGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.AccessPolicies != nil { + in, out := &in.AccessPolicies, &out.AccessPolicies + *out = make([]AccessPolicyReference, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessEntry. +func (in *AccessEntry) DeepCopy() *AccessEntry { + if in == nil { + return nil + } + out := new(AccessEntry) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessPolicyReference) DeepCopyInto(out *AccessPolicyReference) { + *out = *in + in.AccessScope.DeepCopyInto(&out.AccessScope) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessPolicyReference. +func (in *AccessPolicyReference) DeepCopy() *AccessPolicyReference { + if in == nil { + return nil + } + out := new(AccessPolicyReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessScope) DeepCopyInto(out *AccessScope) { + *out = *in + if in.Namespaces != nil { + in, out := &in.Namespaces, &out.Namespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessScope. +func (in *AccessScope) DeepCopy() *AccessScope { + if in == nil { + return nil + } + out := new(AccessScope) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Addon) DeepCopyInto(out *Addon) { *out = *in From 2db5348e4fdbade8a84321bb1a3edb81c9b86a80 Mon Sep 17 00:00:00 2001 From: Josh French Date: Wed, 2 Jul 2025 12:25:55 -0400 Subject: [PATCH 15/19] validations for awsmanagedcontrolplane accessentries --- .../v1beta2/awsmanagedcontrolplane_webhook.go | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go index 261eb8efbe..41bb35ffac 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go @@ -324,6 +324,57 @@ func (r *AWSManagedControlPlane) validateAccessConfig(old *AWSManagedControlPlan ) } + // AccessEntries require AuthenticationMode to be API or API_AND_CONFIG_MAP + if r.Spec.AccessConfig != nil && len(r.Spec.AccessConfig.AccessEntries) > 0 { + if r.Spec.AccessConfig.AuthenticationMode != EKSAuthenticationModeAPI && + r.Spec.AccessConfig.AuthenticationMode != EKSAuthenticationModeAPIAndConfigMap { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("spec", "accessConfig", "accessEntries"), + r.Spec.AccessConfig.AccessEntries, + "accessEntries can only be used when authenticationMode is set to API or API_AND_CONFIG_MAP", + ), + ) + } + + // Validate that EC2 types don't have kubernetes groups or access policies + for i, entry := range r.Spec.AccessConfig.AccessEntries { + if entry.Type == "EC2_LINUX" || entry.Type == "EC2_WINDOWS" { + if len(entry.KubernetesGroups) > 0 { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("spec", "accessConfig", "accessEntries").Index(i).Child("kubernetesGroups"), + entry.KubernetesGroups, + "kubernetesGroups cannot be specified when type is EC2_LINUX or EC2_WINDOWS", + ), + ) + } + if len(entry.AccessPolicies) > 0 { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("spec", "accessConfig", "accessEntries").Index(i).Child("accessPolicies"), + entry.AccessPolicies, + "accessPolicies cannot be specified when type is EC2_LINUX or EC2_WINDOWS", + ), + ) + } + } + + // Validate namespace scopes + for j, policy := range entry.AccessPolicies { + if policy.AccessScope.Type == "namespace" && len(policy.AccessScope.Namespaces) == 0 { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("spec", "accessConfig", "accessEntries").Index(i).Child("accessPolicies").Index(j).Child("accessScope", "namespaces"), + policy.AccessScope.Namespaces, + "at least one value must be specified when accessScope type is namespace", + ), + ) + } + } + } + } + return allErrs } From 75adc154c41e6cf7b4c6d0323c602e0ae2325d8e Mon Sep 17 00:00:00 2001 From: Josh French Date: Wed, 2 Jul 2025 13:22:54 -0400 Subject: [PATCH 16/19] Tests for AccessEntry validations --- ...ster.x-k8s.io_awsmanagedcontrolplanes.yaml | 2 + .../v1beta2/awsmanagedcontrolplane_types.go | 2 + .../v1beta2/awsmanagedcontrolplane_webhook.go | 7 +- .../awsmanagedcontrolplane_webhook_test.go | 160 ++++++++++++++++++ 4 files changed, 168 insertions(+), 3 deletions(-) diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 145ff7a00c..7af31039ed 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -2238,6 +2238,7 @@ spec: Only valid when Type is namespace items: type: string + minItems: 1 type: array type: default: cluster @@ -2258,6 +2259,7 @@ spec: - accessScope - policyARN type: object + maxItems: 20 type: array kubernetesGroups: description: |- diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go index af0fa4d826..6e44c5d0b6 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_types.go @@ -277,6 +277,7 @@ type AccessEntry struct { // AccessPolicies specifies the policies to associate with this access entry // Cannot be specified if Type is EC2_LINUX or EC2_WINDOWS // +optional + // +kubebuilder:validation:MaxItems=20 AccessPolicies []AccessPolicyReference `json:"accessPolicies,omitempty"` } @@ -301,6 +302,7 @@ type AccessScope struct { // Namespaces are the namespaces for the access scope // Only valid when Type is namespace // +optional + // +kubebuilder:validation:MinItems=1 Namespaces []string `json:"namespaces,omitempty"` } diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go index 41bb35ffac..f6a315211d 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go @@ -98,6 +98,7 @@ func (*awsManagedControlPlaneWebhook) ValidateCreate(_ context.Context, obj runt // TODO: Add ipv6 validation things in these validations. allErrs = append(allErrs, r.validateEKSVersion(nil)...) allErrs = append(allErrs, r.Spec.Bastion.Validate()...) + allErrs = append(allErrs, r.validateAccessConfig(nil)...) allErrs = append(allErrs, r.validateIAMAuthConfig()...) allErrs = append(allErrs, r.validateSecondaryCIDR()...) allErrs = append(allErrs, r.validateEKSAddons()...) @@ -309,14 +310,14 @@ func (r *AWSManagedControlPlane) validateAccessConfig(old *AWSManagedControlPlan var allErrs field.ErrorList // If accessConfig is already set, do not allow removal of it. - if old.Spec.AccessConfig != nil && r.Spec.AccessConfig == nil { + if old != nil && old.Spec.AccessConfig != nil && r.Spec.AccessConfig == nil { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "accessConfig"), r.Spec.AccessConfig, "removing AccessConfig is not allowed after it has been enabled"), ) } // AuthenticationMode is ratcheting - do not allow downgrades - if old.Spec.AccessConfig != nil && old.Spec.AccessConfig.AuthenticationMode != r.Spec.AccessConfig.AuthenticationMode && + if old != nil && old.Spec.AccessConfig != nil && r.Spec.AccessConfig != nil && old.Spec.AccessConfig.AuthenticationMode != r.Spec.AccessConfig.AuthenticationMode && ((old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeAPIAndConfigMap && r.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeConfigMap) || old.Spec.AccessConfig.AuthenticationMode == EKSAuthenticationModeAPI) { allErrs = append(allErrs, @@ -367,7 +368,7 @@ func (r *AWSManagedControlPlane) validateAccessConfig(old *AWSManagedControlPlan field.Invalid( field.NewPath("spec", "accessConfig", "accessEntries").Index(i).Child("accessPolicies").Index(j).Child("accessScope", "namespaces"), policy.AccessScope.Namespaces, - "at least one value must be specified when accessScope type is namespace", + "at least one value must be provided when accessScope type is namespace", ), ) } diff --git a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go index e1722fb99e..7729355219 100644 --- a/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go +++ b/controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go @@ -526,6 +526,166 @@ func TestWebhookCreateIPv6Details(t *testing.T) { } } +func TestWebhookValidateAccessEntries(t *testing.T) { + tests := []struct { + name string + accessConfig *AccessConfig + expectError bool + errorSubstr string + }{ + { + name: "valid access entries with API auth mode", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPI, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "STANDARD", + KubernetesGroups: []string{"system:masters"}, + }, + }, + }, + expectError: false, + }, + { + name: "valid access entries with API_AND_CONFIG_MAP auth mode", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPIAndConfigMap, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "STANDARD", + KubernetesGroups: []string{"system:masters"}, + }, + }, + }, + expectError: false, + }, + { + name: "invalid access entries with CONFIG_MAP auth mode", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeConfigMap, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "STANDARD", + KubernetesGroups: []string{"system:masters"}, + }, + }, + }, + expectError: true, + errorSubstr: "accessEntries can only be used when authenticationMode is set to API or API_AND_CONFIG_MAP", + }, + { + name: "invalid EC2_LINUX access entry with kubernetes groups", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPI, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "EC2_LINUX", + KubernetesGroups: []string{"system:masters"}, + }, + }, + }, + expectError: true, + errorSubstr: "kubernetesGroups cannot be specified when type is EC2_LINUX or EC2_WINDOWS", + }, + { + name: "invalid EC2_WINDOWS access entry with access policies", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPI, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "EC2_WINDOWS", + AccessPolicies: []AccessPolicyReference{ + { + PolicyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy", + AccessScope: AccessScope{ + Type: "cluster", + }, + }, + }, + }, + }, + }, + expectError: true, + errorSubstr: "accessPolicies cannot be specified when type is EC2_LINUX or EC2_WINDOWS", + }, + { + name: "invalid access policy with namespace type and no namespaces", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPI, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "STANDARD", + AccessPolicies: []AccessPolicyReference{ + { + PolicyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy", + AccessScope: AccessScope{ + Type: "namespace", + }, + }, + }, + }, + }, + }, + expectError: true, + errorSubstr: "at least one value must be provided when accessScope type is namespace", + }, + { + name: "valid access policy with namespace type and namespaces", + accessConfig: &AccessConfig{ + AuthenticationMode: EKSAuthenticationModeAPI, + AccessEntries: []AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/EKSAdmin", + Type: "STANDARD", + AccessPolicies: []AccessPolicyReference{ + { + PolicyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy", + AccessScope: AccessScope{ + Type: "namespace", + Namespaces: []string{"default", "kube-system"}, + }, + }, + }, + }, + }, + }, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mcp := &AWSManagedControlPlane{ + Spec: AWSManagedControlPlaneSpec{ + EKSClusterName: "default_cluster1", + AccessConfig: tc.accessConfig, + }, + } + + warn, err := (&awsManagedControlPlaneWebhook{}).ValidateCreate(context.Background(), mcp) + + if tc.expectError { + g.Expect(err).ToNot(BeNil()) + if tc.errorSubstr != "" { + g.Expect(err.Error()).To(ContainSubstring(tc.errorSubstr)) + } + } else { + g.Expect(err).To(BeNil()) + } + // Nothing emits warnings yet + g.Expect(warn).To(BeEmpty()) + }) + } +} + func TestWebhookUpdate(t *testing.T) { tests := []struct { name string From c081d316f0411410b33016692da6e77aefc926ae Mon Sep 17 00:00:00 2001 From: Josh French Date: Wed, 2 Jul 2025 16:58:34 -0400 Subject: [PATCH 17/19] Adds support for reconciling EKS AccessEntries --- pkg/cloud/services/eks/accessentry.go | 271 ++++++++++++++++++++++++++ pkg/cloud/services/eks/cluster.go | 8 +- pkg/cloud/services/eks/service.go | 8 + 3 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 pkg/cloud/services/eks/accessentry.go diff --git a/pkg/cloud/services/eks/accessentry.go b/pkg/cloud/services/eks/accessentry.go new file mode 100644 index 0000000000..5646f90c93 --- /dev/null +++ b/pkg/cloud/services/eks/accessentry.go @@ -0,0 +1,271 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eks + +import ( + "context" + "slices" + + "github.com/aws/aws-sdk-go-v2/service/eks" + ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + "github.com/pkg/errors" + + ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/record" +) + +func (s *Service) reconcileAccessEntries(ctx context.Context) error { + if s.scope.ControlPlane.Spec.AccessConfig == nil || len(s.scope.ControlPlane.Spec.AccessConfig.AccessEntries) == 0 { + s.scope.Info("no access entries defined, skipping reconcile") + return nil + } + + existingAccessEntries, err := s.getExistingAccessEntries(ctx) + if err != nil { + return errors.Wrap(err, "failed to list existing access entries") + } + + for _, accessEntry := range s.scope.ControlPlane.Spec.AccessConfig.AccessEntries { + if _, exists := existingAccessEntries[accessEntry.PrincipalARN]; exists { + if err := s.updateAccessEntry(ctx, accessEntry); err != nil { + return errors.Wrapf(err, "failed to update access entry for principal %s", accessEntry.PrincipalARN) + } + delete(existingAccessEntries, accessEntry.PrincipalARN) + } else { + if err := s.createAccessEntry(ctx, accessEntry); err != nil { + return errors.Wrapf(err, "failed to create access entry for principal %s", accessEntry.PrincipalARN) + } + } + } + + for principalArn := range existingAccessEntries { + if err := s.deleteAccessEntry(ctx, principalArn); err != nil { + return errors.Wrapf(err, "failed to delete access entry for principal %s", principalArn) + } + } + + record.Event(s.scope.ControlPlane, "SuccessfulReconcileAccessEntries", "Reconciled access entries") + return nil +} + +func (s *Service) getExistingAccessEntries(ctx context.Context) (map[string]bool, error) { + existingAccessEntries := make(map[string]bool) + var nextToken *string + + clusterName := s.scope.KubernetesClusterName() + for { + input := &eks.ListAccessEntriesInput{ + ClusterName: &clusterName, + NextToken: nextToken, + } + + output, err := s.EKSClient.ListAccessEntries(ctx, input) + if err != nil { + return nil, errors.Wrap(err, "failed to list access entries") + } + + for _, principalArn := range output.AccessEntries { + existingAccessEntries[principalArn] = true + } + + if output.NextToken == nil { + break + } + + nextToken = output.NextToken + } + + return existingAccessEntries, nil +} + +func (s *Service) createAccessEntry(ctx context.Context, accessEntry ekscontrolplanev1.AccessEntry) error { + clusterName := s.scope.KubernetesClusterName() + createInput := &eks.CreateAccessEntryInput{ + ClusterName: &clusterName, + PrincipalArn: &accessEntry.PrincipalARN, + } + + if len(accessEntry.KubernetesGroups) > 0 { + createInput.KubernetesGroups = accessEntry.KubernetesGroups + } + + if accessEntry.Type != "" { + createInput.Type = &accessEntry.Type + } + + if accessEntry.Username != "" { + createInput.Username = &accessEntry.Username + } + + if _, err := s.EKSClient.CreateAccessEntry(ctx, createInput); err != nil { + return errors.Wrapf(err, "failed to create access entry for principal %s", accessEntry.PrincipalARN) + } + + if err := s.reconcileAccessPolicies(ctx, accessEntry); err != nil { + return errors.Wrapf(err, "failed to reconcile access policies for principal %s", accessEntry.PrincipalARN) + } + + return nil +} + +func (s *Service) updateAccessEntry(ctx context.Context, accessEntry ekscontrolplanev1.AccessEntry) error { + clusterName := s.scope.KubernetesClusterName() + describeInput := &eks.DescribeAccessEntryInput{ + ClusterName: &clusterName, + PrincipalArn: &accessEntry.PrincipalARN, + } + + describeOutput, err := s.EKSClient.DescribeAccessEntry(ctx, describeInput) + if err != nil { + return errors.Wrapf(err, "failed to describe access entry for principal %s", accessEntry.PrincipalARN) + } + + // EKS requires recreate when changing type + if accessEntry.Type != *describeOutput.AccessEntry.Type { + if err = s.deleteAccessEntry(ctx, accessEntry.PrincipalARN); err != nil { + return errors.Wrapf(err, "failed to delete access entry for principal %s during recreation", accessEntry.PrincipalARN) + } + + if err = s.createAccessEntry(ctx, accessEntry); err != nil { + return errors.Wrapf(err, "failed to recreate access entry for principal %s", accessEntry.PrincipalARN) + } + return nil + } + + slices.Sort(accessEntry.KubernetesGroups) + slices.Sort(describeOutput.AccessEntry.KubernetesGroups) + + updateInput := &eks.UpdateAccessEntryInput{ + ClusterName: &clusterName, + PrincipalArn: &accessEntry.PrincipalARN, + } + + needsUpdate := false + + if accessEntry.Username != *describeOutput.AccessEntry.Username { + updateInput.Username = &accessEntry.Username + needsUpdate = true + } + + if !slices.Equal(accessEntry.KubernetesGroups, describeOutput.AccessEntry.KubernetesGroups) { + updateInput.KubernetesGroups = accessEntry.KubernetesGroups + needsUpdate = true + } + + if needsUpdate { + if _, err := s.EKSClient.UpdateAccessEntry(ctx, updateInput); err != nil { + return errors.Wrapf(err, "failed to update access entry for principal %s", accessEntry.PrincipalARN) + } + } + + if err := s.reconcileAccessPolicies(ctx, accessEntry); err != nil { + return errors.Wrapf(err, "failed to reconcile access policies for principal %s", accessEntry.PrincipalARN) + } + + return nil +} + +func (s *Service) deleteAccessEntry(ctx context.Context, principalArn string) error { + clusterName := s.scope.KubernetesClusterName() + + if _, err := s.EKSClient.DeleteAccessEntry(ctx, &eks.DeleteAccessEntryInput{ + ClusterName: &clusterName, + PrincipalArn: &principalArn, + }); err != nil { + return errors.Wrapf(err, "failed to delete access entry for principal %s", principalArn) + } + + return nil +} + +func (s *Service) reconcileAccessPolicies(ctx context.Context, accessEntry ekscontrolplanev1.AccessEntry) error { + if accessEntry.Type == "EC2_LINUX" || accessEntry.Type == "EC2_WINDOWS" { + s.scope.Info("Skipping access policy reconciliation for EC2 access type", "principalARN", accessEntry.PrincipalARN) + return nil + } + + existingPolicies, err := s.getExistingAccessPolicies(ctx, accessEntry.PrincipalARN) + if err != nil { + return errors.Wrapf(err, "failed to get existing access policies for principal %s", accessEntry.PrincipalARN) + } + + clusterName := s.scope.KubernetesClusterName() + + for _, policy := range accessEntry.AccessPolicies { + input := &eks.AssociateAccessPolicyInput{ + ClusterName: &clusterName, + PrincipalArn: &accessEntry.PrincipalARN, + PolicyArn: &policy.PolicyARN, + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeType(policy.AccessScope.Type), + }, + } + + if policy.AccessScope.Type == "namespace" && len(policy.AccessScope.Namespaces) > 0 { + input.AccessScope.Namespaces = policy.AccessScope.Namespaces + } + + if _, err := s.EKSClient.AssociateAccessPolicy(ctx, input); err != nil { + return errors.Wrapf(err, "failed to associate access policy %s", policy.PolicyARN) + } + + delete(existingPolicies, policy.PolicyARN) + } + + for policyARN := range existingPolicies { + if _, err := s.EKSClient.DisassociateAccessPolicy(ctx, &eks.DisassociateAccessPolicyInput{ + ClusterName: &clusterName, + PrincipalArn: &accessEntry.PrincipalARN, + PolicyArn: &policyARN, + }); err != nil { + return errors.Wrapf(err, "failed to disassociate access policy %s", policyARN) + } + } + + return nil +} + +func (s *Service) getExistingAccessPolicies(ctx context.Context, principalARN string) (map[string]ekstypes.AssociatedAccessPolicy, error) { + existingPolicies := map[string]ekstypes.AssociatedAccessPolicy{} + var nextToken *string + clusterName := s.scope.KubernetesClusterName() + + for { + input := &eks.ListAssociatedAccessPoliciesInput{ + ClusterName: &clusterName, + PrincipalArn: &principalARN, + NextToken: nextToken, + } + + output, err := s.EKSClient.ListAssociatedAccessPolicies(ctx, input) + if err != nil { + return nil, errors.Wrapf(err, "failed to list associated access policies for principal %s", principalARN) + } + + for _, policy := range output.AssociatedAccessPolicies { + existingPolicies[*policy.PolicyArn] = policy + } + + if output.NextToken == nil { + break + } + + nextToken = output.NextToken + } + + return existingPolicies, nil +} diff --git a/pkg/cloud/services/eks/cluster.go b/pkg/cloud/services/eks/cluster.go index 1e95c2eab8..1cf66b225c 100644 --- a/pkg/cloud/services/eks/cluster.go +++ b/pkg/cloud/services/eks/cluster.go @@ -363,7 +363,6 @@ func makeVpcConfig(subnets infrav1.Subnets, endpointAccess ekscontrolplanev1.End return vpcConfig, nil } - func makeEksLogging(loggingSpec *ekscontrolplanev1.ControlPlaneLoggingSpec) *ekstypes.Logging { if loggingSpec == nil { return nil @@ -597,6 +596,13 @@ func (s *Service) reconcileAccessConfig(ctx context.Context, accessConfig *eksty } } + if expectedAuthenticationMode == string(ekscontrolplanev1.EKSAuthenticationModeAPI) || + expectedAuthenticationMode == string(ekscontrolplanev1.EKSAuthenticationModeAPIAndConfigMap) { + if err := s.reconcileAccessEntries(ctx); err != nil { + return errors.Wrap(err, "failed to reconcile access entries") + } + } + return nil } diff --git a/pkg/cloud/services/eks/service.go b/pkg/cloud/services/eks/service.go index c4c274a123..976e978243 100644 --- a/pkg/cloud/services/eks/service.go +++ b/pkg/cloud/services/eks/service.go @@ -62,6 +62,14 @@ type EKSAPI interface { TagResource(ctx context.Context, params *eks.TagResourceInput, optFns ...func(*eks.Options)) (*eks.TagResourceOutput, error) UntagResource(ctx context.Context, params *eks.UntagResourceInput, optFns ...func(*eks.Options)) (*eks.UntagResourceOutput, error) DisassociateIdentityProviderConfig(ctx context.Context, params *eks.DisassociateIdentityProviderConfigInput, optFns ...func(*eks.Options)) (*eks.DisassociateIdentityProviderConfigOutput, error) + ListAccessEntries(ctx context.Context, params *eks.ListAccessEntriesInput, optFns ...func(*eks.Options)) (*eks.ListAccessEntriesOutput, error) + DescribeAccessEntry(ctx context.Context, params *eks.DescribeAccessEntryInput, optFns ...func(*eks.Options)) (*eks.DescribeAccessEntryOutput, error) + CreateAccessEntry(ctx context.Context, params *eks.CreateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.CreateAccessEntryOutput, error) + UpdateAccessEntry(ctx context.Context, params *eks.UpdateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.UpdateAccessEntryOutput, error) + DeleteAccessEntry(ctx context.Context, params *eks.DeleteAccessEntryInput, optFns ...func(*eks.Options)) (*eks.DeleteAccessEntryOutput, error) + ListAssociatedAccessPolicies(ctx context.Context, params *eks.ListAssociatedAccessPoliciesInput, optFns ...func(*eks.Options)) (*eks.ListAssociatedAccessPoliciesOutput, error) + AssociateAccessPolicy(ctx context.Context, params *eks.AssociateAccessPolicyInput, optFns ...func(*eks.Options)) (*eks.AssociateAccessPolicyOutput, error) + DisassociateAccessPolicy(ctx context.Context, params *eks.DisassociateAccessPolicyInput, optFns ...func(*eks.Options)) (*eks.DisassociateAccessPolicyOutput, error) // Waiters for EKS Cluster WaitUntilClusterActive(ctx context.Context, params *eks.DescribeClusterInput, maxWait time.Duration) error From 19a93edd0fa8c07b6f9f9ccb2cfe3b2785b69973 Mon Sep 17 00:00:00 2001 From: Josh French Date: Thu, 3 Jul 2025 09:56:07 -0400 Subject: [PATCH 18/19] tests for access entry reconciliation --- pkg/cloud/services/eks/accessentry_test.go | 813 ++++++++++++++++++ .../services/eks/mock_eksiface/eksapi_mock.go | 160 ++++ 2 files changed, 973 insertions(+) create mode 100644 pkg/cloud/services/eks/accessentry_test.go diff --git a/pkg/cloud/services/eks/accessentry_test.go b/pkg/cloud/services/eks/accessentry_test.go new file mode 100644 index 0000000000..5399f8cb99 --- /dev/null +++ b/pkg/cloud/services/eks/accessentry_test.go @@ -0,0 +1,813 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package eks + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/eks" + ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" + "github.com/golang/mock/gomock" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" + ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/eks/mock_eksiface" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" +) + +const ( + clusterName = "test-cluster" + principalARN = "arn:aws:iam::123456789012:role/my-role" + secondPrincipalARN = "arn:aws:iam::123456789012:role/second-role" + policyARN = "arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy" +) + +func TestReconcileAccessEntries(t *testing.T) { + tests := []struct { + name string + accessConfig *ekscontrolplanev1.AccessConfig + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "access config nil", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {}, + expectError: false, + }, + { + name: "no access entries", + accessConfig: &ekscontrolplanev1.AccessConfig{ + AuthenticationMode: ekscontrolplanev1.EKSAuthenticationModeAPIAndConfigMap, + AccessEntries: []ekscontrolplanev1.AccessEntry{}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {}, + expectError: false, + }, + { + name: "create new access entry", + accessConfig: &ekscontrolplanev1.AccessConfig{ + AuthenticationMode: ekscontrolplanev1.EKSAuthenticationModeAPIAndConfigMap, + AccessEntries: []ekscontrolplanev1.AccessEntry{ + { + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"system:masters"}, + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: policyARN, + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "cluster", + }, + }, + }, + }, + }, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAccessEntries(gomock.Any(), gomock.Any()).Return(&eks.ListAccessEntriesOutput{ + AccessEntries: []string{}, + }, nil) + + m.CreateAccessEntry(gomock.Any(), &eks.CreateAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters"}, + }).Return(&eks.CreateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + + m.AssociateAccessPolicy(gomock.Any(), &eks.AssociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }).Return(&eks.AssociateAccessPolicyOutput{}, nil) + }, + expectError: false, + }, + { + name: "update existing access entry", + accessConfig: &ekscontrolplanev1.AccessConfig{ + AuthenticationMode: ekscontrolplanev1.EKSAuthenticationModeAPIAndConfigMap, + AccessEntries: []ekscontrolplanev1.AccessEntry{ + { + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin-updated", + KubernetesGroups: []string{"system:masters", "developers"}, + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: policyARN, + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "cluster", + }, + }, + }, + }, + }, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAccessEntries(gomock.Any(), gomock.Any()).Return(&eks.ListAccessEntriesOutput{ + AccessEntries: []string{principalARN}, + }, nil) + + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Username: aws.String("admin"), + Type: aws.String("STANDARD"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.UpdateAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.UpdateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{ + { + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }, + }, + }, nil) + + m.AssociateAccessPolicy(gomock.Any(), &eks.AssociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }).Return(&eks.AssociateAccessPolicyOutput{}, nil) + }, + expectError: false, + }, + { + name: "delete access entry", + accessConfig: &ekscontrolplanev1.AccessConfig{ + AuthenticationMode: ekscontrolplanev1.EKSAuthenticationModeAPIAndConfigMap, + AccessEntries: []ekscontrolplanev1.AccessEntry{ + { + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"system:masters"}, + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: policyARN, + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "cluster", + }, + }, + }, + }, + }, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAccessEntries(gomock.Any(), gomock.Any()).Return(&eks.ListAccessEntriesOutput{ + AccessEntries: []string{principalARN, secondPrincipalARN}, + }, nil) + + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Username: aws.String("admin"), + Type: aws.String("STANDARD"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{ + { + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }, + }, + }, nil) + + m.AssociateAccessPolicy(gomock.Any(), &eks.AssociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }).Return(&eks.AssociateAccessPolicyOutput{}, nil) + + m.DeleteAccessEntry(gomock.Any(), &eks.DeleteAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(secondPrincipalARN), + }).Return(&eks.DeleteAccessEntryOutput{}, nil) + }, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + controlPlane := &ekscontrolplanev1.AWSManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + AccessConfig: tc.accessConfig, + }, + } + + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: controlPlane, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + err = s.reconcileAccessEntries(context.TODO()) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + +func TestReconcileAccessPolicies(t *testing.T) { + tests := []struct { + name string + accessEntry ekscontrolplanev1.AccessEntry + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "EC2_LINUX type skips policy reconciliation", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "EC2_LINUX", + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {}, + expectError: false, + }, + { + name: "EC2_WINDOWS type skips policy reconciliation", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "EC2_WINDOWS", + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) {}, + expectError: false, + }, + { + name: "associate new policy", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: policyARN, + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "cluster", + }, + }, + }, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + + m.AssociateAccessPolicy(gomock.Any(), &eks.AssociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }).Return(&eks.AssociateAccessPolicyOutput{}, nil) + }, + expectError: false, + }, + { + name: "disassociate policy", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{ + { + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeCluster, + }, + }, + }, + }, nil) + + m.DisassociateAccessPolicy(gomock.Any(), &eks.DisassociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + }).Return(&eks.DisassociateAccessPolicyOutput{}, nil) + }, + expectError: false, + }, + { + name: "namespace scoped policy", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: policyARN, + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "namespace", + Namespaces: []string{"kube-system", "default"}, + }, + }, + }, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + + m.AssociateAccessPolicy(gomock.Any(), &eks.AssociateAccessPolicyInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + PolicyArn: aws.String(policyARN), + AccessScope: &ekstypes.AccessScope{ + Type: ekstypes.AccessScopeTypeNamespace, + Namespaces: []string{"kube-system", "default"}, + }, + }).Return(&eks.AssociateAccessPolicyOutput{}, nil) + }, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + }, + }, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + err = s.reconcileAccessPolicies(context.TODO(), tc.accessEntry) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + +func TestCreateAccessEntry(t *testing.T) { + tests := []struct { + name string + accessEntry ekscontrolplanev1.AccessEntry + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "basic access entry", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.CreateAccessEntry(gomock.Any(), &eks.CreateAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + }).Return(&eks.CreateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + { + name: "access entry with groups", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"system:masters", "developers"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.CreateAccessEntry(gomock.Any(), &eks.CreateAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters", "developers"}, + }).Return(&eks.CreateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + { + name: "api error", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"system:masters"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.CreateAccessEntry(gomock.Any(), gomock.Any()).Return(nil, &ekstypes.InvalidParameterException{ + Message: aws.String("error creating access entry"), + }) + }, + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + }, + }, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + err = s.createAccessEntry(context.TODO(), tc.accessEntry) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + +func TestUpdateAccessEntry(t *testing.T) { + tests := []struct { + name string + accessEntry ekscontrolplanev1.AccessEntry + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "no updates needed", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"system:masters"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + { + name: "type change requires recreate", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "FARGATE_LINUX", + Username: "admin", + KubernetesGroups: []string{"system:masters"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.DeleteAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DeleteAccessEntryOutput{}, nil) + + m.CreateAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.CreateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + { + name: "username update", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "new-admin", + KubernetesGroups: []string{"system:masters"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.UpdateAccessEntry(gomock.Any(), &eks.UpdateAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + Username: aws.String("new-admin"), + }).Return(&eks.UpdateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + { + name: "kubernetes groups update", + accessEntry: ekscontrolplanev1.AccessEntry{ + PrincipalARN: principalARN, + Type: "STANDARD", + Username: "admin", + KubernetesGroups: []string{"developers"}, + }, + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DescribeAccessEntry(gomock.Any(), gomock.Any()).Return(&eks.DescribeAccessEntryOutput{ + AccessEntry: &ekstypes.AccessEntry{ + PrincipalArn: aws.String(principalARN), + Type: aws.String("STANDARD"), + Username: aws.String("admin"), + KubernetesGroups: []string{"system:masters"}, + }, + }, nil) + + m.UpdateAccessEntry(gomock.Any(), &eks.UpdateAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + KubernetesGroups: []string{"developers"}, + }).Return(&eks.UpdateAccessEntryOutput{}, nil) + + m.ListAssociatedAccessPolicies(gomock.Any(), gomock.Any()).Return(&eks.ListAssociatedAccessPoliciesOutput{ + AssociatedAccessPolicies: []ekstypes.AssociatedAccessPolicy{}, + }, nil) + }, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + }, + }, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + err = s.updateAccessEntry(context.TODO(), tc.accessEntry) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + +func TestDeleteAccessEntry(t *testing.T) { + tests := []struct { + name string + expect func(m *mock_eksiface.MockEKSAPIMockRecorder) + expectError bool + }{ + { + name: "successful delete", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DeleteAccessEntry(gomock.Any(), &eks.DeleteAccessEntryInput{ + ClusterName: aws.String(clusterName), + PrincipalArn: aws.String(principalARN), + }).Return(&eks.DeleteAccessEntryOutput{}, nil) + }, + expectError: false, + }, + { + name: "api error", + expect: func(m *mock_eksiface.MockEKSAPIMockRecorder) { + m.DeleteAccessEntry(gomock.Any(), gomock.Any()).Return(nil, &ekstypes.ResourceNotFoundException{ + Message: aws.String("access entry not found"), + }) + }, + expectError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + mockControl := gomock.NewController(t) + defer mockControl.Finish() + + eksMock := mock_eksiface.NewMockEKSAPI(mockControl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + _ = ekscontrolplanev1.AddToScheme(scheme) + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + scope, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + }, + ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: clusterName, + }, + Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{ + EKSClusterName: clusterName, + }, + }, + }) + g.Expect(err).To(BeNil()) + + tc.expect(eksMock.EXPECT()) + s := NewService(scope) + s.EKSClient = eksMock + + err = s.deleteAccessEntry(context.TODO(), principalARN) + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(BeNil()) + }) + } +} + diff --git a/pkg/cloud/services/eks/mock_eksiface/eksapi_mock.go b/pkg/cloud/services/eks/mock_eksiface/eksapi_mock.go index cbc119e961..20e7898593 100644 --- a/pkg/cloud/services/eks/mock_eksiface/eksapi_mock.go +++ b/pkg/cloud/services/eks/mock_eksiface/eksapi_mock.go @@ -52,6 +52,26 @@ func (m *MockEKSAPI) EXPECT() *MockEKSAPIMockRecorder { return m.recorder } +// AssociateAccessPolicy mocks base method. +func (m *MockEKSAPI) AssociateAccessPolicy(arg0 context.Context, arg1 *eks.AssociateAccessPolicyInput, arg2 ...func(*eks.Options)) (*eks.AssociateAccessPolicyOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "AssociateAccessPolicy", varargs...) + ret0, _ := ret[0].(*eks.AssociateAccessPolicyOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AssociateAccessPolicy indicates an expected call of AssociateAccessPolicy. +func (mr *MockEKSAPIMockRecorder) AssociateAccessPolicy(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssociateAccessPolicy", reflect.TypeOf((*MockEKSAPI)(nil).AssociateAccessPolicy), varargs...) +} + // AssociateEncryptionConfig mocks base method. func (m *MockEKSAPI) AssociateEncryptionConfig(arg0 context.Context, arg1 *eks.AssociateEncryptionConfigInput, arg2 ...func(*eks.Options)) (*eks.AssociateEncryptionConfigOutput, error) { m.ctrl.T.Helper() @@ -92,6 +112,26 @@ func (mr *MockEKSAPIMockRecorder) AssociateIdentityProviderConfig(arg0, arg1 int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssociateIdentityProviderConfig", reflect.TypeOf((*MockEKSAPI)(nil).AssociateIdentityProviderConfig), varargs...) } +// CreateAccessEntry mocks base method. +func (m *MockEKSAPI) CreateAccessEntry(arg0 context.Context, arg1 *eks.CreateAccessEntryInput, arg2 ...func(*eks.Options)) (*eks.CreateAccessEntryOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CreateAccessEntry", varargs...) + ret0, _ := ret[0].(*eks.CreateAccessEntryOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateAccessEntry indicates an expected call of CreateAccessEntry. +func (mr *MockEKSAPIMockRecorder) CreateAccessEntry(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateAccessEntry", reflect.TypeOf((*MockEKSAPI)(nil).CreateAccessEntry), varargs...) +} + // CreateAddon mocks base method. func (m *MockEKSAPI) CreateAddon(arg0 context.Context, arg1 *eks.CreateAddonInput, arg2 ...func(*eks.Options)) (*eks.CreateAddonOutput, error) { m.ctrl.T.Helper() @@ -172,6 +212,26 @@ func (mr *MockEKSAPIMockRecorder) CreateNodegroup(arg0, arg1 interface{}, arg2 . return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNodegroup", reflect.TypeOf((*MockEKSAPI)(nil).CreateNodegroup), varargs...) } +// DeleteAccessEntry mocks base method. +func (m *MockEKSAPI) DeleteAccessEntry(arg0 context.Context, arg1 *eks.DeleteAccessEntryInput, arg2 ...func(*eks.Options)) (*eks.DeleteAccessEntryOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteAccessEntry", varargs...) + ret0, _ := ret[0].(*eks.DeleteAccessEntryOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteAccessEntry indicates an expected call of DeleteAccessEntry. +func (mr *MockEKSAPIMockRecorder) DeleteAccessEntry(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAccessEntry", reflect.TypeOf((*MockEKSAPI)(nil).DeleteAccessEntry), varargs...) +} + // DeleteAddon mocks base method. func (m *MockEKSAPI) DeleteAddon(arg0 context.Context, arg1 *eks.DeleteAddonInput, arg2 ...func(*eks.Options)) (*eks.DeleteAddonOutput, error) { m.ctrl.T.Helper() @@ -252,6 +312,26 @@ func (mr *MockEKSAPIMockRecorder) DeleteNodegroup(arg0, arg1 interface{}, arg2 . return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNodegroup", reflect.TypeOf((*MockEKSAPI)(nil).DeleteNodegroup), varargs...) } +// DescribeAccessEntry mocks base method. +func (m *MockEKSAPI) DescribeAccessEntry(arg0 context.Context, arg1 *eks.DescribeAccessEntryInput, arg2 ...func(*eks.Options)) (*eks.DescribeAccessEntryOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DescribeAccessEntry", varargs...) + ret0, _ := ret[0].(*eks.DescribeAccessEntryOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DescribeAccessEntry indicates an expected call of DescribeAccessEntry. +func (mr *MockEKSAPIMockRecorder) DescribeAccessEntry(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeAccessEntry", reflect.TypeOf((*MockEKSAPI)(nil).DescribeAccessEntry), varargs...) +} + // DescribeAddon mocks base method. func (m *MockEKSAPI) DescribeAddon(arg0 context.Context, arg1 *eks.DescribeAddonInput, arg2 ...func(*eks.Options)) (*eks.DescribeAddonOutput, error) { m.ctrl.T.Helper() @@ -412,6 +492,26 @@ func (mr *MockEKSAPIMockRecorder) DescribeUpdate(arg0, arg1 interface{}, arg2 .. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeUpdate", reflect.TypeOf((*MockEKSAPI)(nil).DescribeUpdate), varargs...) } +// DisassociateAccessPolicy mocks base method. +func (m *MockEKSAPI) DisassociateAccessPolicy(arg0 context.Context, arg1 *eks.DisassociateAccessPolicyInput, arg2 ...func(*eks.Options)) (*eks.DisassociateAccessPolicyOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DisassociateAccessPolicy", varargs...) + ret0, _ := ret[0].(*eks.DisassociateAccessPolicyOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DisassociateAccessPolicy indicates an expected call of DisassociateAccessPolicy. +func (mr *MockEKSAPIMockRecorder) DisassociateAccessPolicy(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DisassociateAccessPolicy", reflect.TypeOf((*MockEKSAPI)(nil).DisassociateAccessPolicy), varargs...) +} + // DisassociateIdentityProviderConfig mocks base method. func (m *MockEKSAPI) DisassociateIdentityProviderConfig(arg0 context.Context, arg1 *eks.DisassociateIdentityProviderConfigInput, arg2 ...func(*eks.Options)) (*eks.DisassociateIdentityProviderConfigOutput, error) { m.ctrl.T.Helper() @@ -432,6 +532,26 @@ func (mr *MockEKSAPIMockRecorder) DisassociateIdentityProviderConfig(arg0, arg1 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DisassociateIdentityProviderConfig", reflect.TypeOf((*MockEKSAPI)(nil).DisassociateIdentityProviderConfig), varargs...) } +// ListAccessEntries mocks base method. +func (m *MockEKSAPI) ListAccessEntries(arg0 context.Context, arg1 *eks.ListAccessEntriesInput, arg2 ...func(*eks.Options)) (*eks.ListAccessEntriesOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "ListAccessEntries", varargs...) + ret0, _ := ret[0].(*eks.ListAccessEntriesOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListAccessEntries indicates an expected call of ListAccessEntries. +func (mr *MockEKSAPIMockRecorder) ListAccessEntries(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAccessEntries", reflect.TypeOf((*MockEKSAPI)(nil).ListAccessEntries), varargs...) +} + // ListAddons mocks base method. func (m *MockEKSAPI) ListAddons(arg0 context.Context, arg1 *eks.ListAddonsInput, arg2 ...func(*eks.Options)) (*eks.ListAddonsOutput, error) { m.ctrl.T.Helper() @@ -452,6 +572,26 @@ func (mr *MockEKSAPIMockRecorder) ListAddons(arg0, arg1 interface{}, arg2 ...int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAddons", reflect.TypeOf((*MockEKSAPI)(nil).ListAddons), varargs...) } +// ListAssociatedAccessPolicies mocks base method. +func (m *MockEKSAPI) ListAssociatedAccessPolicies(arg0 context.Context, arg1 *eks.ListAssociatedAccessPoliciesInput, arg2 ...func(*eks.Options)) (*eks.ListAssociatedAccessPoliciesOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "ListAssociatedAccessPolicies", varargs...) + ret0, _ := ret[0].(*eks.ListAssociatedAccessPoliciesOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListAssociatedAccessPolicies indicates an expected call of ListAssociatedAccessPolicies. +func (mr *MockEKSAPIMockRecorder) ListAssociatedAccessPolicies(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAssociatedAccessPolicies", reflect.TypeOf((*MockEKSAPI)(nil).ListAssociatedAccessPolicies), varargs...) +} + // ListClusters mocks base method. func (m *MockEKSAPI) ListClusters(arg0 context.Context, arg1 *eks.ListClustersInput, arg2 ...func(*eks.Options)) (*eks.ListClustersOutput, error) { m.ctrl.T.Helper() @@ -532,6 +672,26 @@ func (mr *MockEKSAPIMockRecorder) UntagResource(arg0, arg1 interface{}, arg2 ... return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UntagResource", reflect.TypeOf((*MockEKSAPI)(nil).UntagResource), varargs...) } +// UpdateAccessEntry mocks base method. +func (m *MockEKSAPI) UpdateAccessEntry(arg0 context.Context, arg1 *eks.UpdateAccessEntryInput, arg2 ...func(*eks.Options)) (*eks.UpdateAccessEntryOutput, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "UpdateAccessEntry", varargs...) + ret0, _ := ret[0].(*eks.UpdateAccessEntryOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateAccessEntry indicates an expected call of UpdateAccessEntry. +func (mr *MockEKSAPIMockRecorder) UpdateAccessEntry(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateAccessEntry", reflect.TypeOf((*MockEKSAPI)(nil).UpdateAccessEntry), varargs...) +} + // UpdateAddon mocks base method. func (m *MockEKSAPI) UpdateAddon(arg0 context.Context, arg1 *eks.UpdateAddonInput, arg2 ...func(*eks.Options)) (*eks.UpdateAddonOutput, error) { m.ctrl.T.Helper() From d7cf41b7b6cea833a26b974b940dedf7bc0cc2af Mon Sep 17 00:00:00 2001 From: Josh French Date: Thu, 3 Jul 2025 12:17:02 -0400 Subject: [PATCH 19/19] e2e tests for access entries --- ...control-plane-only-with-accessentries.yaml | 51 ++++++++++++++ test/e2e/suites/managed/eks_test.go | 50 ++++++++++++++ test/e2e/suites/managed/helpers.go | 68 +++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 test/e2e/data/eks/cluster-template-eks-control-plane-only-with-accessentries.yaml diff --git a/test/e2e/data/eks/cluster-template-eks-control-plane-only-with-accessentries.yaml b/test/e2e/data/eks/cluster-template-eks-control-plane-only-with-accessentries.yaml new file mode 100644 index 0000000000..158b1cc62c --- /dev/null +++ b/test/e2e/data/eks/cluster-template-eks-control-plane-only-with-accessentries.yaml @@ -0,0 +1,51 @@ +--- +apiVersion: cluster.x-k8s.io/v1beta1 +kind: Cluster +metadata: + name: "${CLUSTER_NAME}" +spec: + clusterNetwork: + pods: + cidrBlocks: ["192.168.0.0/16"] + infrastructureRef: + kind: AWSManagedControlPlane + apiVersion: controlplane.cluster.x-k8s.io/v1beta2 + name: "${CLUSTER_NAME}-control-plane" + controlPlaneRef: + kind: AWSManagedControlPlane + apiVersion: controlplane.cluster.x-k8s.io/v1beta2 + name: "${CLUSTER_NAME}-control-plane" +--- +kind: AWSManagedControlPlane +apiVersion: controlplane.cluster.x-k8s.io/v1beta2 +metadata: + name: "${CLUSTER_NAME}-control-plane" +spec: + region: "${AWS_REGION}" + sshKeyName: "${AWS_SSH_KEY_NAME}" + version: "${KUBERNETES_VERSION}" + accessConfig: + authenticationMode: API + accessEntries: + - principalARN: "arn:aws:iam::123456789012:role/KubernetesAdmin" + type: STANDARD + username: kubernetes-admin + kubernetesGroups: + - system:masters + accessPolicies: + - policyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy" + accessScope: + type: "cluster" + - principalARN: "arn:aws:iam::123456789012:role/DeveloperRole" + type: STANDARD + username: developer + kubernetesGroups: + - developers + accessPolicies: + - policyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy" + accessScope: + type: "namespace" + namespaces: ["default"] + identityRef: + kind: AWSClusterStaticIdentity + name: e2e-account diff --git a/test/e2e/suites/managed/eks_test.go b/test/e2e/suites/managed/eks_test.go index 3ee6a575c0..45ae3b6621 100644 --- a/test/e2e/suites/managed/eks_test.go +++ b/test/e2e/suites/managed/eks_test.go @@ -165,6 +165,56 @@ var _ = ginkgo.Describe("[managed] [general] EKS cluster tests", func() { } }) + ginkgo.By("should create a cluster with access entries") + ManagedClusterSpec(ctx, func() ManagedClusterSpecInput { + return ManagedClusterSpecInput{ + E2EConfig: e2eCtx.E2EConfig, + ConfigClusterFn: defaultConfigCluster, + BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy, + AWSSession: e2eCtx.BootstrapUserAWSSession, + AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2, + Namespace: namespace, + ClusterName: clusterName, + Flavour: EKSControlPlaneOnlyWithAccessEntriesFlavor, + ControlPlaneMachineCount: 1, // NOTE: this cannot be zero as clusterctl returns an error + WorkerMachineCount: 0, + } + }) + + ginkgo.By("should have created the expected access entries") + expectedEntries := []ekscontrolplanev1.AccessEntry{ + { + PrincipalARN: "arn:aws:iam::123456789012:role/KubernetesAdmin", + Type: "STANDARD", + Username: "kubernetes-admin", + KubernetesGroups: []string{"system:masters"}, + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSClusterAdminPolicy", + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "cluster", + }, + }, + }, + }, + { + PrincipalARN: "arn:aws:iam::123456789012:role/DeveloperRole", + Type: "STANDARD", + Username: "developer", + KubernetesGroups: []string{"developers"}, + AccessPolicies: []ekscontrolplanev1.AccessPolicyReference{ + { + PolicyARN: "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy", + AccessScope: ekscontrolplanev1.AccessScope{ + Type: "namespace", + Namespaces: []string{"default"}, + }, + }, + }, + }, + } + verifyAccessEntries(ctx, eksClusterName, expectedEntries, e2eCtx.BootstrapUserAWSSessionV2) + ginkgo.By(fmt.Sprintf("getting cluster with name %s", clusterName)) cluster := framework.GetClusterByName(ctx, framework.GetClusterByNameInput{ Getter: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), diff --git a/test/e2e/suites/managed/helpers.go b/test/e2e/suites/managed/helpers.go index f6c5a942a0..9e48c98341 100644 --- a/test/e2e/suites/managed/helpers.go +++ b/test/e2e/suites/managed/helpers.go @@ -22,6 +22,7 @@ package managed import ( "context" "fmt" + "slices" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -35,6 +36,7 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" + ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2" "sigs.k8s.io/cluster-api/test/framework/clusterctl" ) @@ -49,6 +51,7 @@ const ( EKSMachinePoolOnlyFlavor = "eks-machinepool-only" EKSIPv6ClusterFlavor = "eks-ipv6-cluster" EKSControlPlaneOnlyLegacyFlavor = "eks-control-plane-only-legacy" + EKSControlPlaneOnlyWithAccessEntriesFlavor = "eks-control-plane-only-with-accessentries" ) const ( @@ -231,3 +234,68 @@ func verifyASG(eksClusterName, asgName string, checkOwned bool, cfg *aws.Config) Expect(found).To(BeTrue(), "expecting the cluster owned tag to exist") } } + +func verifyAccessEntries(ctx context.Context, eksClusterName string, expectedEntries []ekscontrolplanev1.AccessEntry, cfg *aws.Config) { + eksClient := eks.NewFromConfig(*cfg) + + listOutput, err := eksClient.ListAccessEntries(ctx, &eks.ListAccessEntriesInput{ + ClusterName: &eksClusterName, + }) + Expect(err).ToNot(HaveOccurred(), "failed to list access entries") + + expectedEntriesMap := make(map[string]ekscontrolplanev1.AccessEntry, len(expectedEntries)) + for _, entry := range expectedEntries { + expectedEntriesMap[entry.PrincipalARN] = entry + } + + for _, principalARN := range listOutput.AccessEntries { + expectedEntry, exists := expectedEntriesMap[principalARN] + Expect(exists).To(BeTrue(), fmt.Sprintf("unexpected access entry: %s", principalARN)) + + describeOutput, err := eksClient.DescribeAccessEntry(ctx, &eks.DescribeAccessEntryInput{ + ClusterName: &eksClusterName, + PrincipalArn: &principalARN, + }) + Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to describe access entry: %s", principalARN)) + + Expect(describeOutput.AccessEntry.Type).To(Equal(expectedEntry.Type), "access entry type does not match") + Expect(describeOutput.AccessEntry.Username).To(Equal(expectedEntry.Username), "access entry username does not match") + + if len(expectedEntry.KubernetesGroups) > 0 { + slices.Sort(expectedEntry.KubernetesGroups) + slices.Sort(describeOutput.AccessEntry.KubernetesGroups) + Expect(describeOutput.AccessEntry.KubernetesGroups).To(Equal(expectedEntry.KubernetesGroups), "access entry kubernetes groups do not match") + } + + if len(expectedEntry.AccessPolicies) > 0 { + listOutput, err := eksClient.ListAssociatedAccessPolicies(ctx, &eks.ListAssociatedAccessPoliciesInput{ + ClusterName: &eksClusterName, + PrincipalArn: &principalARN, + }) + Expect(err).ToNot(HaveOccurred(), "failed to list access policies") + + expectedPolicies := make(map[string]ekscontrolplanev1.AccessPolicyReference, len(expectedEntry.AccessPolicies)) + for _, policy := range expectedEntry.AccessPolicies { + expectedPolicies[policy.PolicyARN] = policy + } + + for _, policy := range listOutput.AssociatedAccessPolicies { + expectedPolicy, exists := expectedPolicies[*policy.PolicyArn] + Expect(exists).To(BeTrue(), fmt.Sprintf("unexpected access policy: %s", *policy.PolicyArn)) + + Expect(policy.AccessScope.Type).To(Equal(expectedPolicy.AccessScope.Type), "access policy scope type does not match") + + if expectedPolicy.AccessScope.Type == "namespace" { + slices.Sort(expectedPolicy.AccessScope.Namespaces) + slices.Sort(policy.AccessScope.Namespaces) + Expect(policy.AccessScope.Namespaces).To(Equal(expectedPolicy.AccessScope.Namespaces), "access policy scope namespaces do not match") + } + + delete(expectedPolicies, *policy.PolicyArn) + } + Expect(expectedPolicies).To(BeEmpty(), "not all expected access policies were found") + } + delete(expectedEntriesMap, principalARN) + } + Expect(expectedEntriesMap).To(BeEmpty(), "not all expected access entries were found") +}