Skip to content

Commit 2dd9ebb

Browse files
zicongmeiTiberiuGC
authored andcommitted
Create a seperated CARM map from accounts
1 parent a132c88 commit 2dd9ebb

File tree

5 files changed

+92
-64
lines changed

5 files changed

+92
-64
lines changed

pkg/runtime/adoption_reconciler.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (r *adoptionReconciler) reconcile(ctx context.Context, req ctrlrt.Request)
122122
// annotated with an owner account ID. We need to retrieve the
123123
// roleARN from the ConfigMap and properly requeue if the roleARN
124124
// is not available.
125-
roleARN, err = r.getRoleARN(acctID)
125+
roleARN, err = r.getOwnerAccountRoleARN(acctID)
126126
if err != nil {
127127
ackrtlog.InfoAdoptedResource(r.log, res, fmt.Sprintf("Unable to start adoption reconcilliation %s: %v", acctID, err))
128128
// r.getRoleARN errors are not terminal, we should requeue.
@@ -478,14 +478,20 @@ func (r *adoptionReconciler) getEndpointURL(
478478
return r.cfg.EndpointURL
479479
}
480480

481-
// getRoleARN return the Role ARN that should be assumed in order to manage
481+
// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage
482482
// the resources.
483-
func (r *adoptionReconciler) getRoleARN(
483+
func (r *adoptionReconciler) getOwnerAccountRoleARN(
484484
acctID ackv1alpha1.AWSAccountID,
485485
) (ackv1alpha1.AWSResourceName, error) {
486-
roleARN, err := r.cache.Accounts.GetAccountRoleARN(string(acctID))
487-
if err != nil {
488-
return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err)
486+
roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID))
487+
if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound {
488+
// CARM map v2 not defined. Check v1 map.
489+
roleARN, err = r.cache.Accounts.GetValue(string(acctID))
490+
if err != nil {
491+
return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err)
492+
}
493+
} else if err != nil {
494+
return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err)
489495
}
490496
return ackv1alpha1.AWSResourceName(roleARN), nil
491497
}

pkg/runtime/cache/account.go

+35-30
Original file line numberDiff line numberDiff line change
@@ -28,48 +28,53 @@ var (
2828
// ErrCARMConfigMapNotFound is an error that is returned when the CARM
2929
// configmap is not found.
3030
ErrCARMConfigMapNotFound = errors.New("CARM configmap not found")
31-
// ErrAccountIDNotFound is an error that is returned when the account ID
31+
// ErrKeyNotFound is an error that is returned when the account ID
3232
// is not found in the CARM configmap.
33-
ErrAccountIDNotFound = errors.New("account ID not found in CARM configmap")
34-
// ErrEmptyRoleARN is an error that is returned when the role ARN is empty
33+
ErrKeyNotFound = errors.New("key not found in CARM configmap")
34+
// ErrEmptyValue is an error that is returned when the role ARN is empty
3535
// in the CARM configmap.
36-
ErrEmptyRoleARN = errors.New("role ARN is empty in CARM configmap")
36+
ErrEmptyValue = errors.New("role value is empty in CARM configmap")
3737
)
3838

3939
const (
4040
// ACKRoleAccountMap is the name of the configmap map object storing
4141
// all the AWS Account IDs associated with their AWS Role ARNs.
4242
ACKRoleAccountMap = "ack-role-account-map"
43+
44+
// ACKCARMMapV2 is the name of the v2 CARM map.
45+
// It stores the mapping for:
46+
// - Account ID to the AWS role ARNs.
47+
ACKCARMMapV2 = "ack-carm-map"
4348
)
4449

45-
// AccountCache is responsible for caching the CARM configmap
50+
// CARMMap is responsible for caching the CARM configmap
4651
// data. It is listening to all the events related to the CARM map and
4752
// make the changes accordingly.
48-
type AccountCache struct {
53+
type CARMMap struct {
4954
sync.RWMutex
5055
log logr.Logger
51-
roleARNs map[string]string
56+
data map[string]string
5257
configMapCreated bool
5358
}
5459

55-
// NewAccountCache instanciate a new AccountCache.
56-
func NewAccountCache(log logr.Logger) *AccountCache {
57-
return &AccountCache{
60+
// NewCARMMapCache instanciate a new CARMMap.
61+
func NewCARMMapCache(log logr.Logger) *CARMMap {
62+
return &CARMMap{
5863
log: log.WithName("cache.account"),
59-
roleARNs: make(map[string]string),
64+
data: make(map[string]string),
6065
configMapCreated: false,
6166
}
6267
}
6368

64-
// resourceMatchACKRoleAccountConfigMap verifies if a resource is
69+
// resourceMatchCARMConfigMap verifies if a resource is
6570
// the CARM configmap. It verifies the name, namespace and object type.
66-
func resourceMatchACKRoleAccountsConfigMap(raw interface{}) bool {
71+
func resourceMatchCARMConfigMap(raw interface{}, name string) bool {
6772
object, ok := raw.(*corev1.ConfigMap)
68-
return ok && object.ObjectMeta.Name == ACKRoleAccountMap
73+
return ok && object.ObjectMeta.Name == name
6974
}
7075

7176
// Run instantiate a new SharedInformer for ConfigMaps and runs it to begin processing items.
72-
func (c *AccountCache) Run(clientSet kubernetes.Interface, stopCh <-chan struct{}) {
77+
func (c *CARMMap) Run(name string, clientSet kubernetes.Interface, stopCh <-chan struct{}) {
7378
c.log.V(1).Info("Starting shared informer for accounts cache", "targetConfigMap", ACKRoleAccountMap)
7479
informer := informersv1.NewConfigMapInformer(
7580
clientSet,
@@ -79,67 +84,67 @@ func (c *AccountCache) Run(clientSet kubernetes.Interface, stopCh <-chan struct{
7984
)
8085
informer.AddEventHandler(k8scache.ResourceEventHandlerFuncs{
8186
AddFunc: func(obj interface{}) {
82-
if resourceMatchACKRoleAccountsConfigMap(obj) {
87+
if resourceMatchCARMConfigMap(obj, name) {
8388
cm := obj.(*corev1.ConfigMap)
8489
object := cm.DeepCopy()
8590
// To avoid multiple mutex locks, we are updating the cache
8691
// and the configmap existence flag in the same function.
8792
configMapCreated := true
88-
c.updateAccountRoleData(configMapCreated, object.Data)
93+
c.updateData(configMapCreated, object.Data)
8994
c.log.V(1).Info("created account config map", "name", cm.ObjectMeta.Name)
9095
}
9196
},
9297
UpdateFunc: func(orig, desired interface{}) {
93-
if resourceMatchACKRoleAccountsConfigMap(desired) {
98+
if resourceMatchCARMConfigMap(desired, name) {
9499
cm := desired.(*corev1.ConfigMap)
95100
object := cm.DeepCopy()
96101
//TODO(a-hilaly): compare data checksum before updating the cache
97-
c.updateAccountRoleData(true, object.Data)
102+
c.updateData(true, object.Data)
98103
c.log.V(1).Info("updated account config map", "name", cm.ObjectMeta.Name)
99104
}
100105
},
101106
DeleteFunc: func(obj interface{}) {
102-
if resourceMatchACKRoleAccountsConfigMap(obj) {
107+
if resourceMatchCARMConfigMap(obj, name) {
103108
cm := obj.(*corev1.ConfigMap)
104109
newMap := make(map[string]string)
105110
// To avoid multiple mutex locks, we are updating the cache
106111
// and the configmap existence flag in the same function.
107112
configMapCreated := false
108-
c.updateAccountRoleData(configMapCreated, newMap)
113+
c.updateData(configMapCreated, newMap)
109114
c.log.V(1).Info("deleted account config map", "name", cm.ObjectMeta.Name)
110115
}
111116
},
112117
})
113118
go informer.Run(stopCh)
114119
}
115120

116-
// GetAccountRoleARN queries the AWS accountID associated Role ARN
121+
// GetValue queries the value
117122
// from the cached CARM configmap. It will return an error if the
118-
// configmap is not found, the accountID is not found or the role ARN
123+
// configmap is not found, the key is not found or the value
119124
// is empty.
120125
//
121126
// This function is thread safe.
122-
func (c *AccountCache) GetAccountRoleARN(accountID string) (string, error) {
127+
func (c *CARMMap) GetValue(key string) (string, error) {
123128
c.RLock()
124129
defer c.RUnlock()
125130

126131
if !c.configMapCreated {
127132
return "", ErrCARMConfigMapNotFound
128133
}
129-
roleARN, ok := c.roleARNs[accountID]
134+
roleARN, ok := c.data[key]
130135
if !ok {
131-
return "", ErrAccountIDNotFound
136+
return "", ErrKeyNotFound
132137
}
133138
if roleARN == "" {
134-
return "", ErrEmptyRoleARN
139+
return "", ErrEmptyValue
135140
}
136141
return roleARN, nil
137142
}
138143

139-
// updateAccountRoleData updates the CARM map. This function is thread safe.
140-
func (c *AccountCache) updateAccountRoleData(exist bool, data map[string]string) {
144+
// updateData updates the CARM map. This function is thread safe.
145+
func (c *CARMMap) updateData(exist bool, data map[string]string) {
141146
c.Lock()
142147
defer c.Unlock()
143-
c.roleARNs = data
148+
c.data = data
144149
c.configMapCreated = exist
145150
}

pkg/runtime/cache/account_test.go

+19-19
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ func TestAccountCache(t *testing.T) {
6464
fakeLogger := ctrlrtzap.New(ctrlrtzap.UseFlagOptions(&zapOptions))
6565

6666
// initlizing account cache
67-
accountCache := ackrtcache.NewAccountCache(fakeLogger)
67+
accountCache := ackrtcache.NewCARMMapCache(fakeLogger)
6868
stopCh := make(chan struct{})
69-
accountCache.Run(k8sClient, stopCh)
69+
accountCache.Run(ackrtcache.ACKRoleAccountMap, k8sClient, stopCh)
7070

7171
// Before creating the configmap, the accountCache should error for any
7272
// GetAccountRoleARN call.
73-
_, err := accountCache.GetAccountRoleARN(testAccount1)
73+
_, err := accountCache.GetValue(testAccount1)
7474
require.NotNil(t, err)
7575
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
7676

@@ -90,12 +90,12 @@ func TestAccountCache(t *testing.T) {
9090
time.Sleep(time.Second)
9191

9292
// Test with non existing account
93-
_, err = accountCache.GetAccountRoleARN("random-account-not-exist")
93+
_, err = accountCache.GetValue("random-account-not-exist")
9494
require.NotNil(t, err)
9595
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
9696

9797
// Test with existing account
98-
_, err = accountCache.GetAccountRoleARN(testAccount1)
98+
_, err = accountCache.GetValue(testAccount1)
9999
require.NotNil(t, err)
100100
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
101101

@@ -114,17 +114,17 @@ func TestAccountCache(t *testing.T) {
114114
time.Sleep(time.Second)
115115

116116
// Test with non existing account
117-
_, err = accountCache.GetAccountRoleARN("random-account-not-exist")
117+
_, err = accountCache.GetValue("random-account-not-exist")
118118
require.NotNil(t, err)
119-
require.Equal(t, err, ackrtcache.ErrAccountIDNotFound)
119+
require.Equal(t, err, ackrtcache.ErrKeyNotFound)
120120

121121
// Test with existing account - but role ARN is empty
122-
_, err = accountCache.GetAccountRoleARN(testAccount3)
122+
_, err = accountCache.GetValue(testAccount3)
123123
require.NotNil(t, err)
124-
require.Equal(t, err, ackrtcache.ErrEmptyRoleARN)
124+
require.Equal(t, err, ackrtcache.ErrEmptyValue)
125125

126126
// Test with existing account
127-
roleARN, err := accountCache.GetAccountRoleARN(testAccount1)
127+
roleARN, err := accountCache.GetValue(testAccount1)
128128
require.Nil(t, err)
129129
require.Equal(t, roleARN, testAccountARN1)
130130

@@ -144,21 +144,21 @@ func TestAccountCache(t *testing.T) {
144144
time.Sleep(time.Second)
145145

146146
// Test with non existing account
147-
_, err = accountCache.GetAccountRoleARN("random-account-not-exist")
147+
_, err = accountCache.GetValue("random-account-not-exist")
148148
require.NotNil(t, err)
149-
require.Equal(t, err, ackrtcache.ErrAccountIDNotFound)
149+
require.Equal(t, err, ackrtcache.ErrKeyNotFound)
150150

151151
// Test that account was removed
152-
_, err = accountCache.GetAccountRoleARN(testAccount3)
152+
_, err = accountCache.GetValue(testAccount3)
153153
require.NotNil(t, err)
154-
require.Equal(t, err, ackrtcache.ErrAccountIDNotFound)
154+
require.Equal(t, err, ackrtcache.ErrKeyNotFound)
155155

156156
// Test with existing account
157-
roleARN, err = accountCache.GetAccountRoleARN(testAccount1)
157+
roleARN, err = accountCache.GetValue(testAccount1)
158158
require.Nil(t, err)
159159
require.Equal(t, roleARN, testAccountARN1)
160160

161-
roleARN, err = accountCache.GetAccountRoleARN(testAccount2)
161+
roleARN, err = accountCache.GetValue(testAccount2)
162162
require.Nil(t, err)
163163
require.Equal(t, roleARN, testAccountARN2)
164164

@@ -172,15 +172,15 @@ func TestAccountCache(t *testing.T) {
172172
time.Sleep(time.Second)
173173

174174
// Test that accounts ware removed
175-
_, err = accountCache.GetAccountRoleARN(testAccount1)
175+
_, err = accountCache.GetValue(testAccount1)
176176
require.NotNil(t, err)
177177
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
178178

179-
_, err = accountCache.GetAccountRoleARN(testAccount2)
179+
_, err = accountCache.GetValue(testAccount2)
180180
require.NotNil(t, err)
181181
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
182182

183-
_, err = accountCache.GetAccountRoleARN(testAccount3)
183+
_, err = accountCache.GetValue(testAccount3)
184184
require.NotNil(t, err)
185185
require.Equal(t, err, ackrtcache.ErrCARMConfigMapNotFound)
186186
}

pkg/runtime/cache/cache.go

+13-3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ const (
4343
// caching system not to set up resyncs with an authoritative state source
4444
// (i.e. a Kubernetes API server) on a periodic basis.
4545
informerResyncPeriod = 0 * time.Second
46+
47+
// The prefix for owner account ID in the v2 CARM map.
48+
OwnerAccountIDPrefix = "owner-account-id/"
4649
)
4750

4851
// ackSystemNamespace is the namespace in which we look up ACK system
@@ -73,7 +76,10 @@ type Caches struct {
7376
stopCh chan struct{}
7477

7578
// Accounts cache
76-
Accounts *AccountCache
79+
Accounts *CARMMap
80+
81+
// CARMMaps v2 cache
82+
CARMMaps *CARMMap
7783

7884
// Namespaces cache
7985
Namespaces *NamespaceCache
@@ -82,7 +88,8 @@ type Caches struct {
8288
// New instantiate a new Caches object.
8389
func New(log logr.Logger, config Config) Caches {
8490
return Caches{
85-
Accounts: NewAccountCache(log),
91+
Accounts: NewCARMMapCache(log),
92+
CARMMaps: NewCARMMapCache(log),
8693
Namespaces: NewNamespaceCache(log, config.WatchScope, config.Ignored),
8794
}
8895
}
@@ -91,7 +98,10 @@ func New(log logr.Logger, config Config) Caches {
9198
func (c Caches) Run(clientSet kubernetes.Interface) {
9299
stopCh := make(chan struct{})
93100
if c.Accounts != nil {
94-
c.Accounts.Run(clientSet, stopCh)
101+
c.Accounts.Run(ACKRoleAccountMap, clientSet, stopCh)
102+
}
103+
if c.CARMMaps != nil {
104+
c.CARMMaps.Run(ACKCARMMapV2, clientSet, stopCh)
95105
}
96106
if c.Namespaces != nil {
97107
c.Namespaces.Run(clientSet, stopCh)

pkg/runtime/reconciler.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request)
239239
// annotated with an owner account ID. We need to retrieve the
240240
// roleARN from the ConfigMap and properly requeue if the roleARN
241241
// is not available.
242-
roleARN, err = r.getRoleARN(acctID)
242+
roleARN, err = r.getOwnerAccountRoleARN(acctID)
243243
if err != nil {
244244
// TODO(a-hilaly): Refactor all the reconcile function to make it
245245
// easier to understand and maintain.
@@ -1089,15 +1089,22 @@ func (r *resourceReconciler) getOwnerAccountID(
10891089
return controllerAccountID, false
10901090
}
10911091

1092-
// getRoleARN return the Role ARN that should be assumed in order to manage
1092+
// getOwnerAccountRoleARN return the Role ARN that should be assumed in order to manage
10931093
// the resources.
1094-
func (r *resourceReconciler) getRoleARN(
1094+
func (r *resourceReconciler) getOwnerAccountRoleARN(
10951095
acctID ackv1alpha1.AWSAccountID,
10961096
) (ackv1alpha1.AWSResourceName, error) {
1097-
roleARN, err := r.cache.Accounts.GetAccountRoleARN(string(acctID))
1098-
if err != nil {
1099-
return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err)
1097+
roleARN, err := r.cache.CARMMaps.GetValue(ackrtcache.OwnerAccountIDPrefix + string(acctID))
1098+
if err == ackrtcache.ErrCARMConfigMapNotFound || err == ackrtcache.ErrKeyNotFound {
1099+
// CARM map v2 not defined. Check v1 map.
1100+
roleARN, err = r.cache.Accounts.GetValue(string(acctID))
1101+
if err != nil {
1102+
return "", fmt.Errorf("unable to retrieve role ARN for account %s: %v", acctID, err)
1103+
}
1104+
} else if err != nil {
1105+
return "", fmt.Errorf("unable to retrieve role ARN from CARM v2 for account %s: %v", acctID, err)
11001106
}
1107+
11011108
return ackv1alpha1.AWSResourceName(roleARN), nil
11021109
}
11031110

0 commit comments

Comments
 (0)