Split CARMv2 functionality into Team Level Role and Service Level Role (#158)

Issue #, if available:

Description of changes:

This PR aims to resolve a concern where a user migrating from CARMv1 to v2 (i.e. to teamIDs and service level isolation support) might end up with their resources re-created into incorrect accounts just by enabling the feature flag, due to lack of v2 configuration.

The PR splits CARMv2 feature into 2 different features, each behind its own feature flag:

1. team level role - `TeamLevelCARM` , the mappings are being stored in a new configmap `ack-role-team-map`
2. service level role - `ServiceLevelCARM` , the mappings can be stored in both the existing configmap `ack-role-account-map` and the new configmap `ack-role-team-map`

When both feature flags are **ENABLED**, the configmap setup may look like below (this is currently all squeezed into the CARMv2 map i.e. `ack-carm-map`):

`ack-role-team-map` 👇
```
data:
  team-a: "arn:aws:iam::111111111111:role/team-a-global-role"
  s3.team-a: "arn:aws:iam::111111111111:role/team-a-s3-role"
  dynamodb.team-a: "arn:aws:iam::111111111111:role/team-a-dynamodb-role"
```
`ack-role-account-map` 👇
```
data:
  111111111111: arn:aws:iam::111111111111:role/global-role
  s3.111111111111: arn:aws:iam::111111111111:role/s3-role
  dynamodb.111111111111: arn:aws:iam::111111111111:role/dynamodb-role 
```

When both feature flags are **DISABLED**, or neither teamID annotation or service level roles are setup, runtime continues to use the existing CARMv1 setup:

 `ack-role-account-map` :point_down:
```
data:
  111111111111: arn:aws:iam::111111111111:role/global-role
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: TiberiuGC

pkg/featuregate/features.go

@@ -17,15 +17,19 @@
 package featuregate
 
 const (
-	// CARMv2 is the name of the CARMv2 feature.
-	CARMv2 = "CARMv2"
+	// TeamLevelCARM is a feature gate for enabling CARM for team-level resources.
+	TeamLevelCARM = "TeamLevelCARM"
+
+	// ServiceLevelCARM is a feature gate for enabling CARM for service-level resources.
+	ServiceLevelCARM = "ServiceLevelCARM"
 )
 
 // 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
-	CARMv2: {Stage: Alpha, Enabled: false},
+	TeamLevelCARM:    {Stage: Alpha, Enabled: false},
+	ServiceLevelCARM: {Stage: Alpha, Enabled: false},
 }
 
 // FeatureStage represents the development stage of a feature.

pkg/runtime/adoption_reconciler.go

@@ -113,42 +113,31 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request)
 	// 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
-	// stored in a ConfigMap in the ACK system namespace.
+	// stored in the `ack-role-account-map` ConfigMap in the ACK system namespace.
 	// 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.
 	acctID, needCARMLookup := r.getOwnerAccountID(res)
 
 	var roleARN ackv1alpha1.AWSResourceName
-	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))
-				return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay)
-			}
+	if teamID := r.getTeamID(res); teamID != "" && r.cfg.FeatureGates.IsEnabled(featuregate.TeamLevelCARM) {
+		// The user is specifying a namespace that is annotated with a team ID.
+		// Requeue if the corresponding roleARN is not available in the Teams configmap.
+		// Additionally, set the account ID to the role's account ID.
+		roleARN, err = r.getRoleARN(string(teamID), ackrtcache.ACKRoleTeamMap)
+		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 %q configmap: %v", roleARN, ackrtcache.ACKRoleTeamMap, 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 Accounts (CARMv1) configmap.
-		roleARN, err = r.getRoleARN(acctID)
+		// Requeue if the corresponding roleARN is not available in the Accounts configmap.
+		roleARN, err = r.getRoleARN(string(acctID), ackrtcache.ACKRoleAccountMap)
 		if err != nil {
 			ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err))
 			return requeue.NeededAfter(err, roleARNNotAvailableRequeueDelay)
@@ -517,28 +506,31 @@ func (r *adoptionReconciler) getEndpointURL(
 	return r.cfg.EndpointURL
 }
 
-// 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)
+// getRoleARN returns the Role ARN that should be assumed for the given accountID or teamID,
+// from the appropriate configmap, in order to manage the resources.
+func (r *adoptionReconciler) getRoleARN(id string, cacheName string) (ackv1alpha1.AWSResourceName, error) {
+	var cache *ackrtcache.CARMMap
+	switch cacheName {
+	case ackrtcache.ACKRoleTeamMap:
+		cache = r.cache.Teams
+	case ackrtcache.ACKRoleAccountMap:
+		cache = r.cache.Accounts
+	default:
+		return "", fmt.Errorf("invalid cache name: %s", cacheName)
+	}
+
+	if r.cfg.FeatureGates.IsEnabled(featuregate.ServiceLevelCARM) {
+		// use service level roleARN if present
+		serviceID := r.sc.GetMetadata().ServiceAlias + "." + id
+		if roleARN, err := cache.GetValue(serviceID); err == nil {
+			return ackv1alpha1.AWSResourceName(roleARN), nil
+		}
 	}
-	return ackv1alpha1.AWSResourceName(roleARN), nil
-}
 
-// 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))
+	// otherwise use account/team level roleARN
+	roleARN, err := cache.GetValue(id)
 	if err != nil {
-		return "", fmt.Errorf("retrieving role ARN for account ID %q from %q configMap: %v", acctID, ackrtcache.ACKRoleAccountMap, err)
+		return "", fmt.Errorf("retrieving role ARN for account/team ID %q from %q configmap: %v", id, cacheName, err)
 	}
 	return ackv1alpha1.AWSResourceName(roleARN), nil
 }

pkg/runtime/cache/account.go

@@ -41,10 +41,9 @@ const (
 	// 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"
+	// ACKRoleTeamMap is the name of the config map object storing
+	// all the AWS Team IDs associated with their AWS Role ARNs.
+	ACKRoleTeamMap = "ack-role-team-map"
 )
 
 // CARMMap is responsible for caching the CARM configmap

pkg/runtime/cache/cache.go

@@ -79,24 +79,22 @@ type Caches struct {
 	// Accounts cache
 	Accounts *CARMMap
 
-	// CARMMaps v2 cache
-	CARMMaps *CARMMap
+	// Teams cache
+	Teams *CARMMap
 
 	// Namespaces cache
 	Namespaces *NamespaceCache
 }
 
 // New instantiate a new Caches object.
 func New(log logr.Logger, config Config, features featuregate.FeatureGates) Caches {
-	var carmMaps, accounts *CARMMap
-	if features.IsEnabled(featuregate.CARMv2) {
-		carmMaps = NewCARMMapCache(log)
-	} else {
-		accounts = NewCARMMapCache(log)
+	var teams *CARMMap
+	if features.IsEnabled(featuregate.TeamLevelCARM) {
+		teams = NewCARMMapCache(log)
 	}
 	return Caches{
-		Accounts:   accounts,
-		CARMMaps:   carmMaps,
+		Accounts:   NewCARMMapCache(log),
+		Teams:      teams,
 		Namespaces: NewNamespaceCache(log, config.WatchScope, config.Ignored),
 	}
 }
@@ -107,8 +105,8 @@ func (c Caches) Run(clientSet kubernetes.Interface) {
 	if c.Accounts != nil {
 		c.Accounts.Run(ACKRoleAccountMap, clientSet, stopCh)
 	}
-	if c.CARMMaps != nil {
-		c.CARMMaps.Run(ACKCARMMapV2, clientSet, stopCh)
+	if c.Teams != nil {
+		c.Teams.Run(ACKRoleTeamMap, clientSet, stopCh)
 	}
 	if c.Namespaces != nil {
 		c.Namespaces.Run(clientSet, stopCh)
@@ -127,8 +125,8 @@ func (c Caches) WaitForCachesToSync(ctx context.Context) bool {
 	if c.Accounts != nil {
 		accountSynced = cache.WaitForCacheSync(ctx.Done(), c.Accounts.hasSynced)
 	}
-	if c.CARMMaps != nil {
-		carmSynced = cache.WaitForCacheSync(ctx.Done(), c.CARMMaps.hasSynced)
+	if c.Teams != nil {
+		carmSynced = cache.WaitForCacheSync(ctx.Done(), c.Teams.hasSynced)
 	}
 	return namespaceSynced && accountSynced && carmSynced
 }

pkg/runtime/reconciler.go

@@ -230,40 +230,30 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
 	// 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
-	// stored in a ConfigMap in the ACK system namespace.
+	// stored in the `ack-role-account-map` ConfigMap in the ACK system namespace.
 	// 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.
 	acctID, needCARMLookup := r.getOwnerAccountID(desired)
 
 	var roleARN ackv1alpha1.AWSResourceName
-	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)
-			}
+	if teamID := r.getTeamID(desired); teamID != "" && r.cfg.FeatureGates.IsEnabled(featuregate.TeamLevelCARM) {
+		// The user is specifying a namespace that is annotated with a team ID.
+		// Requeue if the corresponding roleARN is not available in the Teams configmap.
+		// Additionally, set the account ID to the role's account ID.
+		roleARN, err = r.getRoleARN(string(teamID), ackrtcache.ACKRoleTeamMap)
+		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 %q configmap: %v", roleARN, ackrtcache.ACKRoleTeamMap, 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 Accounts (CARMv1) configmap.
-		roleARN, err = r.getRoleARN(acctID)
+		// Requeue if the corresponding roleARN is not available in the Accounts configmap.
+		roleARN, err = r.getRoleARN(string(acctID), ackrtcache.ACKRoleAccountMap)
 		if err != nil {
 			return r.handleCacheError(ctx, err, desired)
 		}
@@ -1134,28 +1124,31 @@ func (r *resourceReconciler) getTeamID(
 	return ackv1alpha1.TeamID("")
 }
 
-// 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)
+// getRoleARN returns the Role ARN that should be assumed for the given accountID or teamID,
+// from the appropriate configmap, in order to manage the resources.
+func (r *resourceReconciler) getRoleARN(id string, cacheName string) (ackv1alpha1.AWSResourceName, error) {
+	var cache *ackrtcache.CARMMap
+	switch cacheName {
+	case ackrtcache.ACKRoleTeamMap:
+		cache = r.cache.Teams
+	case ackrtcache.ACKRoleAccountMap:
+		cache = r.cache.Accounts
+	default:
+		return "", fmt.Errorf("invalid cache name: %s", cacheName)
+	}
+
+	if r.cfg.FeatureGates.IsEnabled(featuregate.ServiceLevelCARM) {
+		// use service level roleARN if present
+		serviceID := r.sc.GetMetadata().ServiceAlias + "." + id
+		if roleARN, err := cache.GetValue(serviceID); err == nil {
+			return ackv1alpha1.AWSResourceName(roleARN), nil
+		}
 	}
-	return ackv1alpha1.AWSResourceName(roleARN), nil
-}
 
-// 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))
+	// otherwise use account/team level roleARN
+	roleARN, err := cache.GetValue(id)
 	if err != nil {
-		return "", fmt.Errorf("retrieving role ARN for account ID %q from %q configMap: %v", acctID, ackrtcache.ACKRoleAccountMap, err)
+		return "", fmt.Errorf("retrieving role ARN for account/team ID %q from %q configmap: %v", id, cacheName, err)
 	}
 	return ackv1alpha1.AWSResourceName(roleARN), nil
 }