From b60b76620220386d971ef2ef41023e7c688f0bd7 Mon Sep 17 00:00:00 2001 From: swastik959 Date: Fri, 3 Jul 2026 22:34:51 +0530 Subject: [PATCH 1/2] support for mutiaccount with same name Signed-off-by: swastik959 --- controller/eks-cluster-config-handler.go | 88 +++++++++++++- controller/eks-cluster-config-handler_test.go | 114 ++++++++++++++++++ controller/internal.go | 1 + go.mod | 2 +- pkg/eks/services/mock_services/doc.go | 1 + pkg/eks/services/mock_services/sts_mock.go | 51 ++++++++ pkg/eks/services/sts.go | 26 ++++ 7 files changed, 280 insertions(+), 3 deletions(-) create mode 100644 pkg/eks/services/mock_services/sts_mock.go create mode 100644 pkg/eks/services/sts.go diff --git a/controller/eks-cluster-config-handler.go b/controller/eks-cluster-config-handler.go index 9ef0c5afd..2e1b65f5b 100644 --- a/controller/eks-cluster-config-handler.go +++ b/controller/eks-cluster-config-handler.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/eks" ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types" "github.com/aws/aws-sdk-go-v2/service/iam" + "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/blang/semver" wranglerv1 "github.com/rancher/wrangler/v3/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" @@ -38,6 +39,11 @@ const ( eksConfigUpdatingPhase = "updating" eksConfigImportingPhase = "importing" eksClusterConfigKind = "EKSClusterConfig" + + // stsAccountIDTimeout bounds the STS GetCallerIdentity call used during + // duplicate name validation so a slow or hung STS endpoint cannot stall + // the reconcile loop. + stsAccountIDTimeout = 10 * time.Second ) type Handler struct { @@ -46,6 +52,10 @@ type Handler struct { eksEnqueue func(namespace, name string) secrets wranglerv1.SecretClient secretsCache wranglerv1.SecretCache + // accountIDForSpec resolves the AWS account ID for the credentials + // referenced by an existing cluster spec. It defaults to + // getAWSAccountIDForSpec and is overridable in tests. + accountIDForSpec func(ctx context.Context, spec eksv1.EKSClusterConfigSpec) string } type awsServices struct { @@ -53,6 +63,7 @@ type awsServices struct { eks services.EKSServiceInterface ec2 services.EC2ServiceInterface iam services.IAMServiceInterface + sts services.STSServiceInterface } func Register( @@ -463,13 +474,53 @@ func (h *Handler) validateCreate(ctx context.Context, config *eksv1.EKSClusterCo return fmt.Errorf("aws services not initialized") } - // Check for existing eksclusterconfigs with the same display name + // Check for existing eksclusterconfigs with the same display name. Two EKS + // clusters are only considered the same when they share the same cluster + // name, region and AWS account. This allows importing clusters with the + // same name from different AWS accounts (or regions). eksConfigs, err := h.eksCC.List(config.Namespace, metav1.ListOptions{}) if err != nil { return fmt.Errorf("cannot list eksclusterconfigs for display name check") } + // newAccountID caches the account ID of the new cluster being validated; + // it is resolved lazily and only once (newAccountIDResolved guards against + // re-resolving after a failure). Account IDs for existing clusters are + // cached separately in accountIDCache, keyed by their credential secret. + var newAccountID string + var newAccountIDResolved bool + accountIDForSpec := h.accountIDForSpec + if accountIDForSpec == nil { + accountIDForSpec = h.getAWSAccountIDForSpec + } + accountIDCache := make(map[string]string) for _, c := range eksConfigs.Items { - if c.Spec.DisplayName == config.Spec.DisplayName && c.Name != config.Name { + if c.Name == config.Name || c.Spec.DisplayName != config.Spec.DisplayName { + continue + } + // A different region means a different cluster. + if c.Spec.Region != config.Spec.Region { + continue + } + // Identical credentials necessarily resolve to the same AWS account. + if c.Spec.AmazonCredentialSecret == config.Spec.AmazonCredentialSecret { + return fmt.Errorf("cannot create cluster [%s (id: %s)] because an eksclusterconfig exists with the same name", config.Spec.DisplayName, config.Name) + } + // On a name+region collision with different credentials, compare the + // resolved AWS account IDs. A failure to resolve either account ID + // falls back to treating them as different accounts so a transient + // error does not block cluster creation. Account IDs are cached per + // credential secret to avoid redundant STS calls for clusters that + // share credentials. + if !newAccountIDResolved { + newAccountID = h.getAWSAccountID(ctx, awsSVCs.sts) + newAccountIDResolved = true + } + existingAccountID, cached := accountIDCache[c.Spec.AmazonCredentialSecret] + if !cached { + existingAccountID = accountIDForSpec(ctx, c.Spec) + accountIDCache[c.Spec.AmazonCredentialSecret] = existingAccountID + } + if newAccountID != "" && newAccountID == existingAccountID { return fmt.Errorf("cannot create cluster [%s (id: %s)] because an eksclusterconfig exists with the same name", config.Spec.DisplayName, config.Name) } } @@ -597,6 +648,39 @@ func (h *Handler) validateCreate(ctx context.Context, config *eksv1.EKSClusterCo return nil } +// getAWSAccountID resolves the AWS account ID for the given STS service by +// calling GetCallerIdentity. Any failure (invalid credentials, network error, +// etc.) returns an empty string so callers can fall back to treating the +// credentials as a different account rather than blocking cluster creation on +// a transient issue. +func (h *Handler) getAWSAccountID(ctx context.Context, stsSVC services.STSServiceInterface) string { + if stsSVC == nil { + return "" + } + // Bound the STS call so a slow or unresponsive endpoint cannot stall the + // reconcile loop when the passed-in context has no deadline of its own. + ctx, cancel := context.WithTimeout(ctx, stsAccountIDTimeout) + defer cancel() + out, err := stsSVC.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) + if err != nil { + logrus.Warnf("EKS duplicate name validation: failed to get AWS account ID: %v", err) + return "" + } + return aws.ToString(out.Account) +} + +// getAWSAccountIDForSpec resolves the AWS account ID for the credentials +// referenced by the given cluster spec. It returns an empty string when the +// account cannot be determined. +func (h *Handler) getAWSAccountIDForSpec(ctx context.Context, spec eksv1.EKSClusterConfigSpec) string { + awsSVCs, err := newAWSv2Services(ctx, h.secrets, spec) + if err != nil { + logrus.Warnf("EKS duplicate name validation: failed to create AWS services for credential [%s]: %v", spec.AmazonCredentialSecret, err) + return "" + } + return h.getAWSAccountID(ctx, awsSVCs.sts) +} + func (h *Handler) generateAndSetNetworking(ctx context.Context, config *eksv1.EKSClusterConfig, awsSVCs *awsServices) (*eksv1.EKSClusterConfig, error) { if awsSVCs == nil { return nil, fmt.Errorf("aws services not initialized") diff --git a/controller/eks-cluster-config-handler_test.go b/controller/eks-cluster-config-handler_test.go index 6ce4d4cca..45edf6f71 100644 --- a/controller/eks-cluster-config-handler_test.go +++ b/controller/eks-cluster-config-handler_test.go @@ -2,6 +2,7 @@ package controller import ( "bytes" + "context" "errors" "strings" @@ -12,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/eks/types" awssdkeks "github.com/aws/aws-sdk-go-v2/service/eks" + awssdksts "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -308,6 +310,118 @@ var _ = Describe("updateCluster", func() { }) }) +var _ = Describe("validateCreate display name uniqueness", func() { + var ( + handler *Handler + existingConfig *eksv1.EKSClusterConfig + mockController *gomock.Controller + stsServiceMock *mock_services.MockSTSServiceInterface + ) + + BeforeEach(func() { + mockController = gomock.NewController(GinkgoT()) + stsServiceMock = mock_services.NewMockSTSServiceInterface(mockController) + handler = &Handler{ + eksCC: eksFactory.Eks().V1().EKSClusterConfig(), + secrets: coreFactory.Core().V1().Secret(), + secretsCache: coreFactory.Core().V1().Secret().Cache(), + } + }) + + AfterEach(func() { + if existingConfig != nil { + Expect(test.CleanupAndWait(ctx, cl, existingConfig)).To(Succeed()) + existingConfig = nil + } + mockController.Finish() + }) + + newImportedConfig := func(name, displayName, region, credential string) *eksv1.EKSClusterConfig { + return &eksv1.EKSClusterConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: eksv1.EKSClusterConfigSpec{ + DisplayName: displayName, + Region: region, + AmazonCredentialSecret: credential, + Imported: true, + }, + } + } + + It("should reject a cluster with the same name, region and credentials", func() { + existingConfig = newImportedConfig("existing-dup", "dup", "us-east-1", "default:cred") + Expect(cl.Create(ctx, existingConfig)).To(Succeed()) + + newConfig := newImportedConfig("new-dup", "dup", "us-east-1", "default:cred") + err := handler.validateCreate(ctx, newConfig, &awsServices{sts: stsServiceMock}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("an eksclusterconfig exists with the same name")) + }) + + It("should allow a cluster with the same name in a different region", func() { + existingConfig = newImportedConfig("existing-region", "dup-region", "us-east-1", "default:cred") + Expect(cl.Create(ctx, existingConfig)).To(Succeed()) + + newConfig := newImportedConfig("new-region", "dup-region", "us-west-2", "default:cred") + err := handler.validateCreate(ctx, newConfig, &awsServices{sts: stsServiceMock}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow a cluster with the same name and region but a different AWS account", func() { + existingConfig = newImportedConfig("existing-account", "dup-account", "us-east-1", "default:cred-a") + Expect(cl.Create(ctx, existingConfig)).To(Succeed()) + + // The current cluster resolves to a concrete account ID, while the + // existing cluster's credentials cannot be resolved (its secret does + // not exist), so the two are treated as different accounts. + stsServiceMock.EXPECT().GetCallerIdentity(gomock.Any(), gomock.Any()). + Return(&awssdksts.GetCallerIdentityOutput{Account: aws.String("111111111111")}, nil) + + newConfig := newImportedConfig("new-account", "dup-account", "us-east-1", "default:cred-b") + err := handler.validateCreate(ctx, newConfig, &awsServices{sts: stsServiceMock}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow a cluster when both credentials resolve to different AWS accounts", func() { + existingConfig = newImportedConfig("existing-diff", "dup-diff", "us-east-1", "default:cred-a") + Expect(cl.Create(ctx, existingConfig)).To(Succeed()) + + // The current cluster resolves to one concrete account ID and the + // existing cluster resolves to a different concrete account ID, so + // they are treated as different clusters. + stsServiceMock.EXPECT().GetCallerIdentity(gomock.Any(), gomock.Any()). + Return(&awssdksts.GetCallerIdentityOutput{Account: aws.String("111111111111")}, nil) + handler.accountIDForSpec = func(_ context.Context, _ eksv1.EKSClusterConfigSpec) string { + return "222222222222" + } + + newConfig := newImportedConfig("new-diff", "dup-diff", "us-east-1", "default:cred-b") + err := handler.validateCreate(ctx, newConfig, &awsServices{sts: stsServiceMock}) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should reject a cluster when different credentials resolve to the same AWS account", func() { + existingConfig = newImportedConfig("existing-same", "dup-same", "us-east-1", "default:cred-a") + Expect(cl.Create(ctx, existingConfig)).To(Succeed()) + + // Different credential secrets that both resolve to the same concrete + // account ID identify the same cluster and must be rejected. + stsServiceMock.EXPECT().GetCallerIdentity(gomock.Any(), gomock.Any()). + Return(&awssdksts.GetCallerIdentityOutput{Account: aws.String("333333333333")}, nil) + handler.accountIDForSpec = func(_ context.Context, _ eksv1.EKSClusterConfigSpec) string { + return "333333333333" + } + + newConfig := newImportedConfig("new-same", "dup-same", "us-east-1", "default:cred-b") + err := handler.validateCreate(ctx, newConfig, &awsServices{sts: stsServiceMock}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("an eksclusterconfig exists with the same name")) + }) +}) + var _ = Describe("recordError", func() { var ( eksConfig *eksv1.EKSClusterConfig diff --git a/controller/internal.go b/controller/internal.go index 0bbf4f5bd..f4ba7487f 100644 --- a/controller/internal.go +++ b/controller/internal.go @@ -58,6 +58,7 @@ func newAWSv2Services(ctx context.Context, secretClient wranglerv1.SecretClient, cloudformation: services.NewCloudFormationService(cfg), iam: services.NewIAMService(cfg), ec2: services.NewEC2Service(cfg), + sts: services.NewSTSService(cfg), }, nil } diff --git a/go.mod b/go.mod index 9f7703f54..2409b81ae 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/ec2 v1.303.0 github.com/aws/aws-sdk-go-v2/service/eks v1.83.0 github.com/aws/aws-sdk-go-v2/service/iam v1.53.10 + github.com/aws/aws-sdk-go-v2/service/sts v1.42.1 github.com/blang/semver v3.5.1+incompatible github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 github.com/golang/mock v1.6.0 @@ -49,7 +50,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/signin v1.0.11 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.30.17 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.21 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.42.1 // indirect github.com/aws/smithy-go v1.25.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect diff --git a/pkg/eks/services/mock_services/doc.go b/pkg/eks/services/mock_services/doc.go index 39421371c..acae9ff87 100644 --- a/pkg/eks/services/mock_services/doc.go +++ b/pkg/eks/services/mock_services/doc.go @@ -6,3 +6,4 @@ package mock_services //go:generate ../../../../bin/mockgen -destination eks_mock.go -package mock_services -source ../eks.go EKSServiceInterface //go:generate ../../../../bin/mockgen -destination iam_mock.go -package mock_services -source ../iam.go IAMServiceInterface //go:generate ../../../../bin/mockgen -destination ec2_mock.go -package mock_services -source ../ec2.go EC2ServiceInterface +//go:generate ../../../../bin/mockgen -destination sts_mock.go -package mock_services -source ../sts.go STSServiceInterface diff --git a/pkg/eks/services/mock_services/sts_mock.go b/pkg/eks/services/mock_services/sts_mock.go new file mode 100644 index 000000000..cc3403b5b --- /dev/null +++ b/pkg/eks/services/mock_services/sts_mock.go @@ -0,0 +1,51 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ../sts.go + +// Package mock_services is a generated GoMock package. +package mock_services + +import ( + context "context" + reflect "reflect" + + sts "github.com/aws/aws-sdk-go-v2/service/sts" + gomock "github.com/golang/mock/gomock" +) + +// MockSTSServiceInterface is a mock of STSServiceInterface interface. +type MockSTSServiceInterface struct { + ctrl *gomock.Controller + recorder *MockSTSServiceInterfaceMockRecorder +} + +// MockSTSServiceInterfaceMockRecorder is the mock recorder for MockSTSServiceInterface. +type MockSTSServiceInterfaceMockRecorder struct { + mock *MockSTSServiceInterface +} + +// NewMockSTSServiceInterface creates a new mock instance. +func NewMockSTSServiceInterface(ctrl *gomock.Controller) *MockSTSServiceInterface { + mock := &MockSTSServiceInterface{ctrl: ctrl} + mock.recorder = &MockSTSServiceInterfaceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSTSServiceInterface) EXPECT() *MockSTSServiceInterfaceMockRecorder { + return m.recorder +} + +// GetCallerIdentity mocks base method. +func (m *MockSTSServiceInterface) GetCallerIdentity(ctx context.Context, input *sts.GetCallerIdentityInput) (*sts.GetCallerIdentityOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCallerIdentity", ctx, input) + ret0, _ := ret[0].(*sts.GetCallerIdentityOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCallerIdentity indicates an expected call of GetCallerIdentity. +func (mr *MockSTSServiceInterfaceMockRecorder) GetCallerIdentity(ctx, input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCallerIdentity", reflect.TypeOf((*MockSTSServiceInterface)(nil).GetCallerIdentity), ctx, input) +} diff --git a/pkg/eks/services/sts.go b/pkg/eks/services/sts.go new file mode 100644 index 000000000..2c9409d66 --- /dev/null +++ b/pkg/eks/services/sts.go @@ -0,0 +1,26 @@ +package services + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/sts" +) + +type STSServiceInterface interface { + GetCallerIdentity(ctx context.Context, input *sts.GetCallerIdentityInput) (*sts.GetCallerIdentityOutput, error) +} + +type stsService struct { + svc *sts.Client +} + +func NewSTSService(cfg aws.Config) STSServiceInterface { + return &stsService{ + svc: sts.NewFromConfig(cfg), + } +} + +func (c *stsService) GetCallerIdentity(ctx context.Context, input *sts.GetCallerIdentityInput) (*sts.GetCallerIdentityOutput, error) { + return c.svc.GetCallerIdentity(ctx, input) +} From a543218708a68162507ceb7defadbb1560eec905 Mon Sep 17 00:00:00 2001 From: swastik959 Date: Fri, 3 Jul 2026 22:40:35 +0530 Subject: [PATCH 2/2] go mod tidy Signed-off-by: swastik959 --- go.mod | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 6b7b96e25..ec20f256d 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ replace k8s.io/client-go => k8s.io/client-go v0.36.0 require ( github.com/aws/aws-sdk-go v1.50.38 - github.com/aws/aws-sdk-go-v2/service/sts v1.42.1 github.com/aws/aws-sdk-go-v2 v1.42.0 github.com/aws/aws-sdk-go-v2/config v1.32.23 github.com/aws/aws-sdk-go-v2/credentials v1.19.22 @@ -16,6 +15,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/ec2 v1.305.2 github.com/aws/aws-sdk-go-v2/service/eks v1.84.5 github.com/aws/aws-sdk-go-v2/service/iam v1.54.3 + github.com/aws/aws-sdk-go-v2/service/sts v1.43.2 github.com/blang/semver v3.5.1+incompatible github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 github.com/golang/mock v1.6.0 @@ -50,7 +50,6 @@ require ( github.com/aws/aws-sdk-go-v2/service/signin v1.1.4 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.31.2 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.36.5 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.43.2 // indirect github.com/aws/smithy-go v1.27.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect