support for mutiaccount with same name

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,13 +52,18 @@ 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 {
 	cloudformation services.CloudFormationServiceInterface
 	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")

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

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
 }
 

go.mod

@@ -15,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
@@ -49,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

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

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)
+}