From 2dd9ebb6eb82a591083b77935738c71f339e3be4 Mon Sep 17 00:00:00 2001 From: zicongmei Date: Mon, 18 Mar 2024 15:24:09 -0700 Subject: [PATCH 1/3] Create a seperated CARM map from accounts --- pkg/runtime/adoption_reconciler.go | 18 ++++++--- pkg/runtime/cache/account.go | 65 ++++++++++++++++-------------- pkg/runtime/cache/account_test.go | 38 ++++++++--------- pkg/runtime/cache/cache.go | 16 ++++++-- pkg/runtime/reconciler.go | 19 ++++++--- 5 files changed, 92 insertions(+), 64 deletions(-) diff --git a/pkg/runtime/adoption_reconciler.go b/pkg/runtime/adoption_reconciler.go index 4f0f1cb..c560741 100644 --- a/pkg/runtime/adoption_reconciler.go +++ b/pkg/runtime/adoption_reconciler.go @@ -122,7 +122,7 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request) // annotated with an owner account ID. We need to retrieve the // roleARN from the ConfigMap and properly requeue if the roleARN // is not available. - roleARN, err = r.getRoleARN(acctID) + roleARN, err = r.getOwnerAccountRoleARN(acctID) if err != nil { ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) // r.getRoleARN errors are not terminal, we should requeue. @@ -478,14 +478,20 @@ func (r *adoptionReconciler) getEndpointURL( return r.cfg.EndpointURL } -// getRoleARN return the Role ARN that should be assumed in order to manage +// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage // the resources. -func (r *adoptionReconciler) getRoleARN( +func (r *adoptionReconciler) getOwnerAccountRoleARN( acctID ackv1alpha1.AWSAccountID, ) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.Accounts.GetAccountRoleARN(string(acctID)) - if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) + roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID)) + if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { + // CARM map v2 not defined. Check v1 map. + roleARN, err = r.cache.Accounts.GetValue(string(acctID)) + if err != nil { + return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) + } + } else if err != nil { + return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err) } return ackv1alpha1.AWSResourceName(roleARN), nil } diff --git a/pkg/runtime/cache/account.go b/pkg/runtime/cache/account.go index 8f275e2..e4e131b 100644 --- a/pkg/runtime/cache/account.go +++ b/pkg/runtime/cache/account.go @@ -28,48 +28,53 @@ var ( // ErrCARMConfigMapNotFound is an error that is returned when the CARM // configmap is not found. ErrCARMConfigMapNotFound = errors.New("CARM configmap not found") - // ErrAccountIDNotFound is an error that is returned when the account ID + // ErrKeyNotFound is an error that is returned when the account ID // is not found in the CARM configmap. - ErrAccountIDNotFound = errors.New("account ID not found in CARM configmap") - // ErrEmptyRoleARN is an error that is returned when the role ARN is empty + ErrKeyNotFound = errors.New("key not found in CARM configmap") + // ErrEmptyValue is an error that is returned when the role ARN is empty // in the CARM configmap. - ErrEmptyRoleARN = errors.New("role ARN is empty in CARM configmap") + ErrEmptyValue = errors.New("role value is empty in CARM configmap") ) const ( // ACKRoleAccountMap is the name of the configmap map object storing // all the AWS Account IDs associated with their AWS Role ARNs. ACKRoleAccountMap = "ack-role-account-map" + + // ACKCARMMapV2 is the name of the v2 CARM map. + // It stores the mapping for: + // - Account ID to the AWS role ARNs. + ACKCARMMapV2 = "ack-carm-map" ) -// AccountCache is responsible for caching the CARM configmap +// CARMMap is responsible for caching the CARM configmap // data. It is listening to all the events related to the CARM map and // make the changes accordingly. -type AccountCache struct { +type CARMMap struct { sync.RWMutex log logr.Logger - roleARNs map[string]string + data map[string]string configMapCreated bool } -// NewAccountCache instanciate a new AccountCache. -func NewAccountCache(log logr.Logger) *AccountCache { - return &AccountCache{ +// NewCARMMapCache instanciate a new CARMMap. +func NewCARMMapCache(log logr.Logger) *CARMMap { + return &CARMMap{ log: log.WithName("cache.account"), - roleARNs: make(map[string]string), + data: make(map[string]string), configMapCreated: false, } } -// resourceMatchACKRoleAccountConfigMap verifies if a resource is +// resourceMatchCARMConfigMap verifies if a resource is // the CARM configmap. It verifies the name, namespace and object type. -func resourceMatchACKRoleAccountsConfigMap(raw interface{}) bool { +func resourceMatchCARMConfigMap(raw interface{}, name string) bool { object, ok := raw.(*corev1.ConfigMap) - return ok && object.ObjectMeta.Name == ACKRoleAccountMap + return ok && object.ObjectMeta.Name == name } // Run instantiate a new SharedInformer for ConfigMaps and runs it to begin processing items. -func (c *AccountCache) Run(clientSet kubernetes.Interface, stopCh <-chan struct{}) { +func (c *CARMMap) Run(name string, clientSet kubernetes.Interface, stopCh <-chan struct{}) { c.log.V(1).Info("Starting shared informer for accounts cache", "targetConfigMap", ACKRoleAccountMap) informer := informersv1.NewConfigMapInformer( clientSet, @@ -79,33 +84,33 @@ func (c *AccountCache) Run(clientSet kubernetes.Interface, stopCh <-chan struct{ ) informer.AddEventHandler(k8scache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - if resourceMatchACKRoleAccountsConfigMap(obj) { + if resourceMatchCARMConfigMap(obj, name) { cm := obj.(*corev1.ConfigMap) object := cm.DeepCopy() // To avoid multiple mutex locks, we are updating the cache // and the configmap existence flag in the same function. configMapCreated := true - c.updateAccountRoleData(configMapCreated, object.Data) + c.updateData(configMapCreated, object.Data) c.log.V(1).Info("created account config map", "name", cm.ObjectMeta.Name) } }, UpdateFunc: func(orig, desired interface{}) { - if resourceMatchACKRoleAccountsConfigMap(desired) { + if resourceMatchCARMConfigMap(desired, name) { cm := desired.(*corev1.ConfigMap) object := cm.DeepCopy() //TODO(a-hilaly): compare data checksum before updating the cache - c.updateAccountRoleData(true, object.Data) + c.updateData(true, object.Data) c.log.V(1).Info("updated account config map", "name", cm.ObjectMeta.Name) } }, DeleteFunc: func(obj interface{}) { - if resourceMatchACKRoleAccountsConfigMap(obj) { + if resourceMatchCARMConfigMap(obj, name) { cm := obj.(*corev1.ConfigMap) newMap := make(map[string]string) // To avoid multiple mutex locks, we are updating the cache // and the configmap existence flag in the same function. configMapCreated := false - c.updateAccountRoleData(configMapCreated, newMap) + c.updateData(configMapCreated, newMap) c.log.V(1).Info("deleted account config map", "name", cm.ObjectMeta.Name) } }, @@ -113,33 +118,33 @@ func (c *AccountCache) Run(clientSet kubernetes.Interface, stopCh <-chan struct{ go informer.Run(stopCh) } -// GetAccountRoleARN queries the AWS accountID associated Role ARN +// GetValue queries the value // from the cached CARM configmap. It will return an error if the -// configmap is not found, the accountID is not found or the role ARN +// configmap is not found, the key is not found or the value // is empty. // // This function is thread safe. -func (c *AccountCache) GetAccountRoleARN(accountID string) (string, error) { +func (c *CARMMap) GetValue(key string) (string, error) { c.RLock() defer c.RUnlock() if !c.configMapCreated { return "", ErrCARMConfigMapNotFound } - roleARN, ok := c.roleARNs[accountID] + roleARN, ok := c.data[key] if !ok { - return "", ErrAccountIDNotFound + return "", ErrKeyNotFound } if roleARN == "" { - return "", ErrEmptyRoleARN + return "", ErrEmptyValue } return roleARN, nil } -// updateAccountRoleData updates the CARM map. This function is thread safe. -func (c *AccountCache) updateAccountRoleData(exist bool, data map[string]string) { +// updateData updates the CARM map. This function is thread safe. +func (c *CARMMap) updateData(exist bool, data map[string]string) { c.Lock() defer c.Unlock() - c.roleARNs = data + c.data = data c.configMapCreated = exist } diff --git a/pkg/runtime/cache/account_test.go b/pkg/runtime/cache/account_test.go index 9ae24cb..e0bdd01 100644 --- a/pkg/runtime/cache/account_test.go +++ b/pkg/runtime/cache/account_test.go @@ -64,13 +64,13 @@ func TestAccountCache(t *testing.T) { fakeLogger := ctrlrtzap.New(ctrlrtzap.UseFlagOptions(&zapOptions)) // initlizing account cache - accountCache := ackrtcache.NewAccountCache(fakeLogger) + accountCache := ackrtcache.NewCARMMapCache(fakeLogger) stopCh := make(chan struct{}) - accountCache.Run(k8sClient, stopCh) + accountCache.Run(ackrtcache.ACKRoleAccountMap, k8sClient, stopCh) // Before creating the configmap, the accountCache should error for any // GetAccountRoleARN call. - _, err := accountCache.GetAccountRoleARN(testAccount1) + _, err := accountCache.GetValue(testAccount1) require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) @@ -90,12 +90,12 @@ func TestAccountCache(t *testing.T) { time.Sleep(time.Second) // Test with non existing account - _, err = accountCache.GetAccountRoleARN("random-account-not-exist") + _, err = accountCache.GetValue("random-account-not-exist") require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) // Test with existing account - _, err = accountCache.GetAccountRoleARN(testAccount1) + _, err = accountCache.GetValue(testAccount1) require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) @@ -114,17 +114,17 @@ func TestAccountCache(t *testing.T) { time.Sleep(time.Second) // Test with non existing account - _, err = accountCache.GetAccountRoleARN("random-account-not-exist") + _, err = accountCache.GetValue("random-account-not-exist") require.NotNil(t, err) - require.Equal(t, err, ackrtcache.ErrAccountIDNotFound) + require.Equal(t, err, ackrtcache.ErrKeyNotFound) // Test with existing account - but role ARN is empty - _, err = accountCache.GetAccountRoleARN(testAccount3) + _, err = accountCache.GetValue(testAccount3) require.NotNil(t, err) - require.Equal(t, err, ackrtcache.ErrEmptyRoleARN) + require.Equal(t, err, ackrtcache.ErrEmptyValue) // Test with existing account - roleARN, err := accountCache.GetAccountRoleARN(testAccount1) + roleARN, err := accountCache.GetValue(testAccount1) require.Nil(t, err) require.Equal(t, roleARN, testAccountARN1) @@ -144,21 +144,21 @@ func TestAccountCache(t *testing.T) { time.Sleep(time.Second) // Test with non existing account - _, err = accountCache.GetAccountRoleARN("random-account-not-exist") + _, err = accountCache.GetValue("random-account-not-exist") require.NotNil(t, err) - require.Equal(t, err, ackrtcache.ErrAccountIDNotFound) + require.Equal(t, err, ackrtcache.ErrKeyNotFound) // Test that account was removed - _, err = accountCache.GetAccountRoleARN(testAccount3) + _, err = accountCache.GetValue(testAccount3) require.NotNil(t, err) - require.Equal(t, err, ackrtcache.ErrAccountIDNotFound) + require.Equal(t, err, ackrtcache.ErrKeyNotFound) // Test with existing account - roleARN, err = accountCache.GetAccountRoleARN(testAccount1) + roleARN, err = accountCache.GetValue(testAccount1) require.Nil(t, err) require.Equal(t, roleARN, testAccountARN1) - roleARN, err = accountCache.GetAccountRoleARN(testAccount2) + roleARN, err = accountCache.GetValue(testAccount2) require.Nil(t, err) require.Equal(t, roleARN, testAccountARN2) @@ -172,15 +172,15 @@ func TestAccountCache(t *testing.T) { time.Sleep(time.Second) // Test that accounts ware removed - _, err = accountCache.GetAccountRoleARN(testAccount1) + _, err = accountCache.GetValue(testAccount1) require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) - _, err = accountCache.GetAccountRoleARN(testAccount2) + _, err = accountCache.GetValue(testAccount2) require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) - _, err = accountCache.GetAccountRoleARN(testAccount3) + _, err = accountCache.GetValue(testAccount3) require.NotNil(t, err) require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound) } diff --git a/pkg/runtime/cache/cache.go b/pkg/runtime/cache/cache.go index e77fae3..f631ef7 100644 --- a/pkg/runtime/cache/cache.go +++ b/pkg/runtime/cache/cache.go @@ -43,6 +43,9 @@ const ( // caching system not to set up resyncs with an authoritative state source // (i.e. a Kubernetes API server) on a periodic basis. informerResyncPeriod = 0 * time.Second + + // The prefix for owner account ID in the v2 CARM map. + OwnerAccountIDPrefix = "owner-account-id/" ) // ackSystemNamespace is the namespace in which we look up ACK system @@ -73,7 +76,10 @@ type Caches struct { stopCh chan struct{} // Accounts cache - Accounts *AccountCache + Accounts *CARMMap + + // CARMMaps v2 cache + CARMMaps *CARMMap // Namespaces cache Namespaces *NamespaceCache @@ -82,7 +88,8 @@ type Caches struct { // New instantiate a new Caches object. func New(log logr.Logger, config Config) Caches { return Caches{ - Accounts: NewAccountCache(log), + Accounts: NewCARMMapCache(log), + CARMMaps: NewCARMMapCache(log), Namespaces: NewNamespaceCache(log, config.WatchScope, config.Ignored), } } @@ -91,7 +98,10 @@ func New(log logr.Logger, config Config) Caches { func (c Caches) Run(clientSet kubernetes.Interface) { stopCh := make(chan struct{}) if c.Accounts != nil { - c.Accounts.Run(clientSet, stopCh) + c.Accounts.Run(ACKRoleAccountMap, clientSet, stopCh) + } + if c.CARMMaps != nil { + c.CARMMaps.Run(ACKCARMMapV2, clientSet, stopCh) } if c.Namespaces != nil { c.Namespaces.Run(clientSet, stopCh) diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index a9128b2..b755283 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -239,7 +239,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) // annotated with an owner account ID. We need to retrieve the // roleARN from the ConfigMap and properly requeue if the roleARN // is not available. - roleARN, err = r.getRoleARN(acctID) + roleARN, err = r.getOwnerAccountRoleARN(acctID) if err != nil { // TODO(a-hilaly): Refactor all the reconcile function to make it // easier to understand and maintain. @@ -1089,15 +1089,22 @@ func (r *resourceReconciler) getOwnerAccountID( return controllerAccountID, false } -// getRoleARN return the Role ARN that should be assumed in order to manage +// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage // the resources. -func (r *resourceReconciler) getRoleARN( +func (r *resourceReconciler) getOwnerAccountRoleARN( acctID ackv1alpha1.AWSAccountID, ) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.Accounts.GetAccountRoleARN(string(acctID)) - if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) + roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID)) + if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { + // CARM map v2 not defined. Check v1 map. + roleARN, err = r.cache.Accounts.GetValue(string(acctID)) + if err != nil { + return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) + } + } else if err != nil { + return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err) } + return ackv1alpha1.AWSResourceName(roleARN), nil } From 96a54b3c9e48be21f2fe52edf9b65dda872f1d5f Mon Sep 17 00:00:00 2001 From: zicongmei Date: Thu, 22 Feb 2024 10:26:17 -0800 Subject: [PATCH 2/3] Support role annotation --- apis/core/v1alpha1/annotations.go | 9 +++ apis/core/v1alpha1/common.go | 3 + pkg/runtime/adoption_reconciler.go | 65 +++++++++++++++++--- pkg/runtime/cache/cache.go | 3 + pkg/runtime/cache/namespace.go | 24 ++++++++ pkg/runtime/cache/namespace_test.go | 95 +++++++++++++++++++++++++++++ pkg/runtime/reconciler.go | 84 ++++++++++++++++++++----- 7 files changed, 258 insertions(+), 25 deletions(-) diff --git a/apis/core/v1alpha1/annotations.go b/apis/core/v1alpha1/annotations.go index c2e8ea8..bf50ffd 100644 --- a/apis/core/v1alpha1/annotations.go +++ b/apis/core/v1alpha1/annotations.go @@ -35,6 +35,15 @@ const ( // TODO(jaypipes): Link to documentation on cross-account resource // management AnnotationOwnerAccountID = AnnotationPrefix + "owner-account-id" + // AnnotationTeamID is an annotation whose value is the identifier + // for the AWS team ID to manage the resources. If this annotation + // is set on a CR, the Kubernetes user is indicating that the ACK service + // controller should create/patch/delete the resource in the specified AWS + // role for this team ID. In order for this cross-account resource management + // to succeed, the AWS IAM Role that the ACK service controller runs as needs + // to have the ability to call the AWS STS::AssumeRole API call and assume an + // IAM Role in the target AWS Account. + AnnotationTeamID = AnnotationPrefix + "team-id" // AnnotationRegion is an annotation whose value is the identifier for the // the AWS region in which the resources should be created. If this annotation // is set on a CR metadata, that means the user is indicating to the ACK service diff --git a/apis/core/v1alpha1/common.go b/apis/core/v1alpha1/common.go index cbefbb7..b4519e4 100644 --- a/apis/core/v1alpha1/common.go +++ b/apis/core/v1alpha1/common.go @@ -19,6 +19,9 @@ type AWSRegion string // AWSAccountID represents an AWS account identifier type AWSAccountID string +// TeamID represents a team ID identifier. +type TeamID string + // AWSResourceName represents an AWS Resource Name (ARN) type AWSResourceName string diff --git a/pkg/runtime/adoption_reconciler.go b/pkg/runtime/adoption_reconciler.go index c560741..7ad9497 100644 --- a/pkg/runtime/adoption_reconciler.go +++ b/pkg/runtime/adoption_reconciler.go @@ -17,6 +17,7 @@ import ( "context" "fmt" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -108,6 +109,10 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request) return ackerr.NotAdoptable } + // If a user specified a namespace with role ARN annotation, + // we need to get the role and set the accout ID to that role. + teamID := r.getTeamID(res) + // If a user has specified a namespace that is annotated with the // an owner account ID, we need an appropriate role ARN to assume // in order to perform the reconciliation. The roles ARN are typically @@ -115,20 +120,36 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request) // If the ConfigMap is not created, or not populated with an // accountID to roleARN mapping, we need to properly requeue with a // helpful message to the user. - var roleARN ackv1alpha1.AWSResourceName acctID, needCARMLookup := r.getOwnerAccountID(res) - if needCARMLookup { - // This means that the user is specifying a namespace that is - // annotated with an owner account ID. We need to retrieve the - // roleARN from the ConfigMap and properly requeue if the roleARN - // is not available. - roleARN, err = r.getOwnerAccountRoleARN(acctID) + + var roleARN ackv1alpha1.AWSResourceName + if teamID != "" { + roleARN, err = r.getTeamRoleARN(teamID) if err != nil { ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) // r.getRoleARN errors are not terminal, we should requeue. return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) } + parsedARN, err := arn.Parse(string(roleARN)) + if err != nil { + return fmt.Errorf("failed to parsed role ARN %q from namespace annotation: %v", roleARN, err) + } + acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) + } else { + if needCARMLookup { + // This means that the user is specifying a namespace that is + // annotated with an owner account ID or team ID. We need to retrieve the + // roleARN from the ConfigMap and properly requeue if the roleARN + // is not available. + roleARN, err = r.getOwnerAccountRoleARN(acctID) + if err != nil { + ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) + // r.getRoleARN errors are not terminal, we should requeue. + return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) + } + } } + region := r.getRegion(res) targetDescriptor := rmf.ResourceDescriptor() endpointURL := r.getEndpointURL(res) @@ -460,6 +481,19 @@ func (r *adoptionReconciler) getOwnerAccountID( return ackv1alpha1.AWSAccountID(r.cfg.AccountID), false } +// getTeamID returns the team ID that owns the supplied resource. +func (r *adoptionReconciler) getTeamID( + res *ackv1alpha1.AdoptedResource, +) ackv1alpha1.TeamID { + // look for team id in the namespace annotations + namespace := res.GetNamespace() + teamID, ok := r.cache.Namespaces.GetTeamID(namespace) + if ok { + return ackv1alpha1.TeamID(teamID) + } + return "" +} + // getEndpointURL returns the AWS account that owns the supplied resource. // We look for the namespace associated endpoint url, if that is set we use it. // Otherwise if none of these annotations are set we use the endpoint url specified @@ -478,8 +512,8 @@ func (r *adoptionReconciler) getEndpointURL( return r.cfg.EndpointURL } -// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage -// the resources. +// getRoleARN return the Role ARN that should be assumed for accoutn ID +// in order to manage the resources. func (r *adoptionReconciler) getOwnerAccountRoleARN( acctID ackv1alpha1.AWSAccountID, ) (ackv1alpha1.AWSResourceName, error) { @@ -496,6 +530,19 @@ func (r *adoptionReconciler) getOwnerAccountRoleARN( return ackv1alpha1.AWSResourceName(roleARN), nil } +// getTeamRoleARN return the Role ARN that should be assumed for a team ID +// in order to manage the resources. +func (r *adoptionReconciler) getTeamRoleARN( + teamID ackv1alpha1.TeamID, +) (ackv1alpha1.AWSResourceName, error) { + roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.TeamIDPrefix + string(teamID)) + if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { + return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", teamID, err) + } + + return ackv1alpha1.AWSResourceName(roleARN), nil +} + // getRegion returns the AWS region that the given resource is in or should be // created in. If the CR have a region associated with it, it is used. Otherwise // we look for the namespace associated region, if that is set we use it. Finally diff --git a/pkg/runtime/cache/cache.go b/pkg/runtime/cache/cache.go index f631ef7..a1f4011 100644 --- a/pkg/runtime/cache/cache.go +++ b/pkg/runtime/cache/cache.go @@ -46,6 +46,9 @@ const ( // The prefix for owner account ID in the v2 CARM map. OwnerAccountIDPrefix = "owner-account-id/" + + // The prefix for owner team ID in the v2 CARM map. + TeamIDPrefix = "team-id/" ) // ackSystemNamespace is the namespace in which we look up ACK system diff --git a/pkg/runtime/cache/namespace.go b/pkg/runtime/cache/namespace.go index d2e2326..4f9ccfa 100644 --- a/pkg/runtime/cache/namespace.go +++ b/pkg/runtime/cache/namespace.go @@ -32,6 +32,8 @@ type namespaceInfo struct { defaultRegion string // services.k8s.aws/owner-account-id Annotation ownerAccountID string + // services.k8s.aws/team-id Annotation + teamID string // services.k8s.aws/endpoint-url Annotation endpointURL string // {service}.services.k8s.aws/deletion-policy Annotations (keyed by service) @@ -54,6 +56,14 @@ func (n *namespaceInfo) getOwnerAccountID() string { return n.ownerAccountID } +// getTeamID returns the namespace team-id +func (n *namespaceInfo) getTeamID() string { + if n == nil { + return "" + } + return n.teamID +} + // getEndpointURL returns the namespace Endpoint URL func (n *namespaceInfo) getEndpointURL() string { if n == nil { @@ -182,6 +192,16 @@ func (c *NamespaceCache) GetOwnerAccountID(namespace string) (string, bool) { return "", false } +// GetTeamID returns the team-id if it exists +func (c *NamespaceCache) GetTeamID(namespace string) (string, bool) { + info, ok := c.getNamespaceInfo(namespace) + if ok { + a := info.getTeamID() + return a, a != "" + } + return "", false +} + // GetEndpointURL returns the endpoint URL if it exists func (c *NamespaceCache) GetEndpointURL(namespace string) (string, bool) { info, ok := c.getNamespaceInfo(namespace) @@ -225,6 +245,10 @@ func (c *NamespaceCache) setNamespaceInfoFromK8sObject(ns *corev1.Namespace) { if ok { nsInfo.ownerAccountID = OwnerAccountID } + TeamID, ok := nsa[ackv1alpha1.AnnotationTeamID] + if ok { + nsInfo.teamID = TeamID + } EndpointURL, ok := nsa[ackv1alpha1.AnnotationEndpointURL] if ok { nsInfo.endpointURL = EndpointURL diff --git a/pkg/runtime/cache/namespace_test.go b/pkg/runtime/cache/namespace_test.go index 86fb240..121878d 100644 --- a/pkg/runtime/cache/namespace_test.go +++ b/pkg/runtime/cache/namespace_test.go @@ -131,6 +131,101 @@ func TestNamespaceCache(t *testing.T) { require.False(t, ok) } +func TestNamespaceCacheWithRoleARN(t *testing.T) { + // create a fake k8s client and fake watcher + k8sClient := k8sfake.NewSimpleClientset() + watcher := watch.NewFake() + k8sClient.PrependWatchReactor("production", k8stesting.DefaultWatchReactor(watcher, nil)) + + // New logger writing to specific buffer + zapOptions := ctrlrtzap.Options{ + Development: true, + Level: zapcore.InfoLevel, + } + fakeLogger := ctrlrtzap.New(ctrlrtzap.UseFlagOptions(&zapOptions)) + + // initlizing account cache + namespaceCache := ackrtcache.NewNamespaceCache(fakeLogger, []string{}, []string{}) + stopCh := make(chan struct{}) + + namespaceCache.Run(k8sClient, stopCh) + + // Test create events + _, err := k8sClient.CoreV1().Namespaces().Create( + context.Background(), + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "production", + Annotations: map[string]string{ + ackv1alpha1.AnnotationDefaultRegion: "us-west-2", + ackv1alpha1.AnnotationTeamID: "team-a", + ackv1alpha1.AnnotationEndpointURL: "https://amazon-service.region.amazonaws.com", + }, + }, + }, + metav1.CreateOptions{}, + ) + require.Nil(t, err) + + time.Sleep(time.Second) + + defaultRegion, ok := namespaceCache.GetDefaultRegion("production") + require.True(t, ok) + require.Equal(t, "us-west-2", defaultRegion) + + teamID, ok := namespaceCache.GetTeamID("production") + require.True(t, ok) + require.Equal(t, "team-a", teamID) + + endpointURL, ok := namespaceCache.GetEndpointURL("production") + require.True(t, ok) + require.Equal(t, "https://amazon-service.region.amazonaws.com", endpointURL) + + // Test update events + _, err = k8sClient.CoreV1().Namespaces().Update( + context.Background(), + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "production", + Annotations: map[string]string{ + ackv1alpha1.AnnotationDefaultRegion: "us-est-1", + ackv1alpha1.AnnotationTeamID: "team-b", + ackv1alpha1.AnnotationEndpointURL: "https://amazon-other-service.region.amazonaws.com", + }, + }, + }, + metav1.UpdateOptions{}, + ) + require.Nil(t, err) + + time.Sleep(time.Second) + + defaultRegion, ok = namespaceCache.GetDefaultRegion("production") + require.True(t, ok) + require.Equal(t, "us-est-1", defaultRegion) + + teamID, ok = namespaceCache.GetTeamID("production") + require.True(t, ok) + require.Equal(t, "team-b", teamID) + + endpointURL, ok = namespaceCache.GetEndpointURL("production") + require.True(t, ok) + require.Equal(t, "https://amazon-other-service.region.amazonaws.com", endpointURL) + + // Test delete events + err = k8sClient.CoreV1().Namespaces().Delete( + context.Background(), + "production", + metav1.DeleteOptions{}, + ) + require.Nil(t, err) + + time.Sleep(time.Second) + + _, ok = namespaceCache.GetDefaultRegion(testNamespace1) + require.False(t, ok) +} + func TestScopedNamespaceCache(t *testing.T) { defaultConfig := ackrtcache.Config{ WatchScope: []string{"watch-scope", "watch-scope-2"}, diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index b755283..85344b8 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -20,6 +20,7 @@ import ( "strings" "time" + "github.com/aws/aws-sdk-go/aws/arn" backoff "github.com/cenkalti/backoff/v4" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -225,6 +226,10 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) ctx = context.WithValue(ctx, ackrtlog.ContextKey, rlog) ctx = context.WithValue(ctx, "resourceNamespace", req.Namespace) + // If a user specified a namespace with team-id annotation, + // we need to get the role and set the accout ID to that role. + teamID := r.getTeamID(desired) + // If a user has specified a namespace that is annotated with the // an owner account ID, we need an appropriate role ARN to assume // in order to perform the reconciliation. The roles ARN are typically @@ -232,22 +237,29 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) // If the ConfigMap is not created, or not populated with an // accountID to roleARN mapping, we need to properly requeue with a // helpful message to the user. - var roleARN ackv1alpha1.AWSResourceName acctID, needCARMLookup := r.getOwnerAccountID(desired) - if needCARMLookup { - // This means that the user is specifying a namespace that is - // annotated with an owner account ID. We need to retrieve the - // roleARN from the ConfigMap and properly requeue if the roleARN - // is not available. - roleARN, err = r.getOwnerAccountRoleARN(acctID) + + var roleARN ackv1alpha1.AWSResourceName + if teamID != "" { + roleARN, err = r.getTeamRoleARN(teamID) if err != nil { - // TODO(a-hilaly): Refactor all the reconcile function to make it - // easier to understand and maintain. - reason := err.Error() - latest := desired.DeepCopy() - // set ResourceSynced condition to false with proper error message - condition.SetSynced(latest, corev1.ConditionFalse, &condition.UnavailableIAMRoleMessage, &reason) - return r.HandleReconcileError(ctx, desired, latest, requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay)) + return r.handleCacheError(ctx, err, desired) + } + parsedARN, err := arn.Parse(string(roleARN)) + if err != nil { + return ctrlrt.Result{}, fmt.Errorf("failed to parsed role ARN %q from namespace annotation: %v", roleARN, err) + } + acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) + } else { + if needCARMLookup { + // This means that the user is specifying a namespace that is + // annotated with an owner account ID or team ID. We need to retrieve the + // roleARN from the ConfigMap and properly requeue if the roleARN + // is not available. + roleARN, err = r.getOwnerAccountRoleARN(acctID) + if err != nil { + return r.handleCacheError(ctx, err, desired) + } } } region := r.getRegion(desired) @@ -275,6 +287,20 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) return r.HandleReconcileError(ctx, desired, latest, err) } +func (r *resourceReconciler) handleCacheError( + ctx context.Context, + err error, + desired acktypes.AWSResource, +) (ctrlrt.Result, error) { + // TODO(a-hilaly): Refactor all the reconcile function to make it + // easier to understand and maintain. + reason := err.Error() + latest := desired.DeepCopy() + // set ResourceSynced condition to false with proper error message + condition.SetSynced(latest, corev1.ConditionFalse, &condition.UnavailableIAMRoleMessage, &reason) + return r.HandleReconcileError(ctx, desired, latest, requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay)) +} + // reconcile either cleans up a deleted resource or ensures that the supplied // AWSResource's backing API resource matches the supplied desired state. // @@ -1089,8 +1115,21 @@ func (r *resourceReconciler) getOwnerAccountID( return controllerAccountID, false } -// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage -// the resources. +// getTeamID gets the team-id from the namespace annotation. +func (r *resourceReconciler) getTeamID( + res acktypes.AWSResource, +) ackv1alpha1.TeamID { + // look for team ID in the namespace annotations + namespace := res.MetaObject().GetNamespace() + namespacedTeamID, ok := r.cache.Namespaces.GetTeamID(namespace) + if ok { + return ackv1alpha1.TeamID(namespacedTeamID) + } + return ackv1alpha1.TeamID("") +} + +// getRoleARN return the Role ARN that should be assumed for accoutn ID +// in order to manage the resources. func (r *resourceReconciler) getOwnerAccountRoleARN( acctID ackv1alpha1.AWSAccountID, ) (ackv1alpha1.AWSResourceName, error) { @@ -1108,6 +1147,19 @@ func (r *resourceReconciler) getOwnerAccountRoleARN( return ackv1alpha1.AWSResourceName(roleARN), nil } +// getTeamRoleARN return the Role ARN that should be assumed for a team ID +// in order to manage the resources. +func (r *resourceReconciler) getTeamRoleARN( + teamID ackv1alpha1.TeamID, +) (ackv1alpha1.AWSResourceName, error) { + roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.TeamIDPrefix + string(teamID)) + if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { + return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", teamID, err) + } + + return ackv1alpha1.AWSResourceName(roleARN), nil +} + // getRegion returns the region the resource exists in, or if the resource // has yet to be created, the region the resource *should* be created in. // From 2d1b1925edbc8e38188c309b22f76a9d25484652 Mon Sep 17 00:00:00 2001 From: Tibi <110664232+TiberiuGC@users.noreply.github.com> Date: Fri, 9 Aug 2024 21:16:46 +0300 Subject: [PATCH 3/3] put CARMv2 map behind a feature flag and add service level isolation support --- pkg/featuregate/features.go | 7 ++- pkg/runtime/adoption_reconciler.go | 94 +++++++++++++++--------------- pkg/runtime/cache/account.go | 2 +- pkg/runtime/cache/cache.go | 6 -- pkg/runtime/reconciler.go | 91 +++++++++++++++-------------- 5 files changed, 100 insertions(+), 100 deletions(-) diff --git a/pkg/featuregate/features.go b/pkg/featuregate/features.go index 56275af..4e1bc5a 100644 --- a/pkg/featuregate/features.go +++ b/pkg/featuregate/features.go @@ -16,11 +16,16 @@ // optionally overridden. package featuregate +const ( + // CARMv2 is the name of the CARMv2 feature. + CARMv2 = "CARMv2" +) + // defaultACKFeatureGates is a map of feature names to Feature structs // representing the default feature gates for ACK controllers. var defaultACKFeatureGates = FeatureGates{ // Set feature gates here - // "feature1": {Stage: Alpha, Enabled: false}, + CARMv2: {Stage: Alpha, Enabled: false}, } // FeatureStage represents the development stage of a feature. diff --git a/pkg/runtime/adoption_reconciler.go b/pkg/runtime/adoption_reconciler.go index 7ad9497..4c5f6fd 100644 --- a/pkg/runtime/adoption_reconciler.go +++ b/pkg/runtime/adoption_reconciler.go @@ -33,6 +33,7 @@ import ( ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1" ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config" ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" + "github.com/aws-controllers-k8s/runtime/pkg/featuregate" ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics" "github.com/aws-controllers-k8s/runtime/pkg/requeue" ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache" @@ -109,10 +110,6 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request) return ackerr.NotAdoptable } - // If a user specified a namespace with role ARN annotation, - // we need to get the role and set the accout ID to that role. - teamID := r.getTeamID(res) - // If a user has specified a namespace that is annotated with the // an owner account ID, we need an appropriate role ARN to assume // in order to perform the reconciliation. The roles ARN are typically @@ -123,31 +120,39 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request) acctID, needCARMLookup := r.getOwnerAccountID(res) var roleARN ackv1alpha1.AWSResourceName - if teamID != "" { - roleARN, err = r.getTeamRoleARN(teamID) - if err != nil { - ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) - // r.getRoleARN errors are not terminal, we should requeue. - return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) - } - parsedARN, err := arn.Parse(string(roleARN)) - if err != nil { - return fmt.Errorf("failed to parsed role ARN %q from namespace annotation: %v", roleARN, err) - } - acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) - } else { - if needCARMLookup { - // This means that the user is specifying a namespace that is - // annotated with an owner account ID or team ID. We need to retrieve the - // roleARN from the ConfigMap and properly requeue if the roleARN - // is not available. - roleARN, err = r.getOwnerAccountRoleARN(acctID) + if r.cfg.FeatureGates.IsEnabled(featuregate.CARMv2) { + teamID := r.getTeamID(res) + if teamID != "" { + // The user is specifying a namespace that is annotated with a team ID. + // Requeue if the corresponding roleARN is not available in the CARMv2 configmap. + // Additionally, set the account ID to the role's account ID. + roleARN, err = r.getRoleARNv2(string(teamID)) + if err != nil { + ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) + return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) + } + parsedARN, err := arn.Parse(string(roleARN)) + if err != nil { + return fmt.Errorf("parsing role ARN %q from namespace annotation: %v", roleARN, err) + } + acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) + } else if needCARMLookup { + // The user is specifying a namespace that is annotated with an owner account ID. + // Requeue if the corresponding roleARN is not available in the CARMv2 configmap. + roleARN, err = r.getRoleARNv2(string(acctID)) if err != nil { ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) - // r.getRoleARN errors are not terminal, we should requeue. return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) } } + } else if needCARMLookup { + // The user is specifying a namespace that is annotated with an owner account ID. + // Requeue if the corresponding roleARN is not available in the Accounts (CARMv1) configmap. + roleARN, err = r.getRoleARN(acctID) + if err != nil { + ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err)) + return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay) + } } region := r.getRegion(res) @@ -512,34 +517,29 @@ func (r *adoptionReconciler) getEndpointURL( return r.cfg.EndpointURL } -// getRoleARN return the Role ARN that should be assumed for accoutn ID -// in order to manage the resources. -func (r *adoptionReconciler) getOwnerAccountRoleARN( - acctID ackv1alpha1.AWSAccountID, -) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID)) - if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { - // CARM map v2 not defined. Check v1 map. - roleARN, err = r.cache.Accounts.GetValue(string(acctID)) - if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) - } - } else if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err) +// getRoleARNv2 returns the Role ARN that should be assumed for the given account/team ID, +// from the CARMv2 configmap, in order to manage the resources. +func (r *adoptionReconciler) getRoleARNv2(id string) (ackv1alpha1.AWSResourceName, error) { + // use service level roleARN if present + serviceID := r.sc.GetMetadata().ServiceAlias + "." + id + if roleARN, err := r.cache.CARMMaps.GetValue(serviceID); err == nil { + return ackv1alpha1.AWSResourceName(roleARN), nil + } + // otherwise use account/team level roleARN + roleARN, err := r.cache.CARMMaps.GetValue(id) + if err != nil { + return "", fmt.Errorf("retrieving role ARN for account/team ID %q from %q configmap: %v", id, ackrtcache.ACKCARMMapV2, err) } return ackv1alpha1.AWSResourceName(roleARN), nil } -// getTeamRoleARN return the Role ARN that should be assumed for a team ID -// in order to manage the resources. -func (r *adoptionReconciler) getTeamRoleARN( - teamID ackv1alpha1.TeamID, -) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.TeamIDPrefix + string(teamID)) - if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { - return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", teamID, err) +// getRoleARN returns the Role ARN that should be assumed for the given account ID, +// from the CARMv1 configmap, in order to manage the resources. +func (r *adoptionReconciler) getRoleARN(acctID ackv1alpha1.AWSAccountID) (ackv1alpha1.AWSResourceName, error) { + roleARN, err := r.cache.Accounts.GetValue(string(acctID)) + if err != nil { + return "", fmt.Errorf("retrieving role ARN for account ID %q from %q configMap: %v", acctID, ackrtcache.ACKRoleAccountMap, err) } - return ackv1alpha1.AWSResourceName(roleARN), nil } diff --git a/pkg/runtime/cache/account.go b/pkg/runtime/cache/account.go index e4e131b..c92cc47 100644 --- a/pkg/runtime/cache/account.go +++ b/pkg/runtime/cache/account.go @@ -60,7 +60,7 @@ type CARMMap struct { // NewCARMMapCache instanciate a new CARMMap. func NewCARMMapCache(log logr.Logger) *CARMMap { return &CARMMap{ - log: log.WithName("cache.account"), + log: log.WithName("cache.carm"), data: make(map[string]string), configMapCreated: false, } diff --git a/pkg/runtime/cache/cache.go b/pkg/runtime/cache/cache.go index a1f4011..d3efbc9 100644 --- a/pkg/runtime/cache/cache.go +++ b/pkg/runtime/cache/cache.go @@ -43,12 +43,6 @@ const ( // caching system not to set up resyncs with an authoritative state source // (i.e. a Kubernetes API server) on a periodic basis. informerResyncPeriod = 0 * time.Second - - // The prefix for owner account ID in the v2 CARM map. - OwnerAccountIDPrefix = "owner-account-id/" - - // The prefix for owner team ID in the v2 CARM map. - TeamIDPrefix = "team-id/" ) // ackSystemNamespace is the namespace in which we look up ACK system diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 85344b8..fc18165 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -39,6 +39,7 @@ import ( ackcondition "github.com/aws-controllers-k8s/runtime/pkg/condition" ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config" ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" + "github.com/aws-controllers-k8s/runtime/pkg/featuregate" ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics" "github.com/aws-controllers-k8s/runtime/pkg/requeue" ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache" @@ -226,10 +227,6 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) ctx = context.WithValue(ctx, ackrtlog.ContextKey, rlog) ctx = context.WithValue(ctx, "resourceNamespace", req.Namespace) - // If a user specified a namespace with team-id annotation, - // we need to get the role and set the accout ID to that role. - teamID := r.getTeamID(desired) - // If a user has specified a namespace that is annotated with the // an owner account ID, we need an appropriate role ARN to assume // in order to perform the reconciliation. The roles ARN are typically @@ -240,28 +237,38 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) acctID, needCARMLookup := r.getOwnerAccountID(desired) var roleARN ackv1alpha1.AWSResourceName - if teamID != "" { - roleARN, err = r.getTeamRoleARN(teamID) - if err != nil { - return r.handleCacheError(ctx, err, desired) - } - parsedARN, err := arn.Parse(string(roleARN)) - if err != nil { - return ctrlrt.Result{}, fmt.Errorf("failed to parsed role ARN %q from namespace annotation: %v", roleARN, err) - } - acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) - } else { - if needCARMLookup { - // This means that the user is specifying a namespace that is - // annotated with an owner account ID or team ID. We need to retrieve the - // roleARN from the ConfigMap and properly requeue if the roleARN - // is not available. - roleARN, err = r.getOwnerAccountRoleARN(acctID) + if r.cfg.FeatureGates.IsEnabled(featuregate.CARMv2) { + teamID := r.getTeamID(desired) + if teamID != "" { + // The user is specifying a namespace that is annotated with a team ID. + // Requeue if the corresponding roleARN is not available in the CARMv2 configmap. + // Additionally, set the account ID to the role's account ID. + roleARN, err = r.getRoleARNv2(string(teamID)) + if err != nil { + return r.handleCacheError(ctx, err, desired) + } + parsedARN, err := arn.Parse(string(roleARN)) + if err != nil { + return ctrlrt.Result{}, fmt.Errorf("parsing role ARN %q from namespace annotation: %v", roleARN, err) + } + acctID = ackv1alpha1.AWSAccountID(parsedARN.AccountID) + } else if needCARMLookup { + // The user is specifying a namespace that is annotated with an owner account ID. + // Requeue if the corresponding roleARN is not available in the CARMv2 configmap. + roleARN, err = r.getRoleARNv2(string(acctID)) if err != nil { return r.handleCacheError(ctx, err, desired) } } + } else if needCARMLookup { + // The user is specifying a namespace that is annotated with an owner account ID. + // Requeue if the corresponding roleARN is not available in the Accounts (CARMv1) configmap. + roleARN, err = r.getRoleARN(acctID) + if err != nil { + return r.handleCacheError(ctx, err, desired) + } } + region := r.getRegion(desired) endpointURL := r.getEndpointURL(desired) gvk := r.rd.GroupVersionKind() @@ -1128,35 +1135,29 @@ func (r *resourceReconciler) getTeamID( return ackv1alpha1.TeamID("") } -// getRoleARN return the Role ARN that should be assumed for accoutn ID -// in order to manage the resources. -func (r *resourceReconciler) getOwnerAccountRoleARN( - acctID ackv1alpha1.AWSAccountID, -) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID)) - if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { - // CARM map v2 not defined. Check v1 map. - roleARN, err = r.cache.Accounts.GetValue(string(acctID)) - if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err) - } - } else if err != nil { - return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err) +// getRoleARNv2 returns the Role ARN that should be assumed for the given account/team ID, +// from the CARMv2 configmap, in order to manage the resources. +func (r *resourceReconciler) getRoleARNv2(id string) (ackv1alpha1.AWSResourceName, error) { + // use service level roleARN if present + serviceID := r.sc.GetMetadata().ServiceAlias + "." + id + if roleARN, err := r.cache.CARMMaps.GetValue(serviceID); err == nil { + return ackv1alpha1.AWSResourceName(roleARN), nil + } + // otherwise use account/team level roleARN + roleARN, err := r.cache.CARMMaps.GetValue(id) + if err != nil { + return "", fmt.Errorf("retrieving role ARN for account/team ID %q from %q configmap: %v", id, ackrtcache.ACKCARMMapV2, err) } - return ackv1alpha1.AWSResourceName(roleARN), nil } -// getTeamRoleARN return the Role ARN that should be assumed for a team ID -// in order to manage the resources. -func (r *resourceReconciler) getTeamRoleARN( - teamID ackv1alpha1.TeamID, -) (ackv1alpha1.AWSResourceName, error) { - roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.TeamIDPrefix + string(teamID)) - if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound { - return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", teamID, err) +// getRoleARN returns the Role ARN that should be assumed for the given account ID, +// from the CARMv1 configmap, in order to manage the resources. +func (r *resourceReconciler) getRoleARN(acctID ackv1alpha1.AWSAccountID) (ackv1alpha1.AWSResourceName, error) { + roleARN, err := r.cache.Accounts.GetValue(string(acctID)) + if err != nil { + return "", fmt.Errorf("retrieving role ARN for account ID %q from %q configMap: %v", acctID, ackrtcache.ACKRoleAccountMap, err) } - return ackv1alpha1.AWSResourceName(roleARN), nil }