Introduce aws region into the AWS config cache

Signed-off-by: Maksymilian Boguń <[email protected]>

pkg/scalers/aws/aws_common.go

@@ -137,6 +137,6 @@ func GetAwsAuthorization(uniqueKey string, podIdentity kedav1alpha1.AuthPodIdent
 }
 
 // ClearAwsConfig wraps the removal of the config from the cache
-func ClearAwsConfig(awsAuthorization AuthorizationMetadata) {
-	awsSharedCredentialsCache.RemoveCachedEntry(awsAuthorization)
+func ClearAwsConfig(awsRegion string, awsAuthorization AuthorizationMetadata) {
+	awsSharedCredentialsCache.RemoveCachedEntry(awsRegion, awsAuthorization)
 }

pkg/scalers/aws/aws_config_cache.go

@@ -68,12 +68,12 @@ func newSharedConfigsCache() sharedConfigCache {
 
 // getCacheKey returns a unique key based on given AuthorizationMetadata.
 // As it can contain sensitive data, the key is hashed to not expose secrets
-func (a *sharedConfigCache) getCacheKey(awsAuthorization AuthorizationMetadata) string {
-	key := "keda"
+func (a *sharedConfigCache) getCacheKey(awsRegion string, awsAuthorization AuthorizationMetadata) string {
+	key := "keda-" + awsRegion
 	if awsAuthorization.AwsAccessKeyID != "" {
-		key = fmt.Sprintf("%s-%s-%s", awsAuthorization.AwsAccessKeyID, awsAuthorization.AwsSecretAccessKey, awsAuthorization.AwsSessionToken)
+		key = fmt.Sprintf("%s-%s-%s-%s", awsAuthorization.AwsAccessKeyID, awsAuthorization.AwsSecretAccessKey, awsAuthorization.AwsSessionToken, awsRegion)
 	} else if awsAuthorization.AwsRoleArn != "" {
-		key = awsAuthorization.AwsRoleArn
+		key = fmt.Sprintf("%s-%s", awsAuthorization.AwsRoleArn, awsRegion)
 	}
 	// to avoid sensitive data as key and to use a constant key size,
 	// we hash the key with sha3
@@ -89,7 +89,7 @@ func (a *sharedConfigCache) getCacheKey(awsAuthorization AuthorizationMetadata)
 func (a *sharedConfigCache) GetCredentials(ctx context.Context, awsRegion string, awsAuthorization AuthorizationMetadata) (*aws.Config, error) {
 	a.Lock()
 	defer a.Unlock()
-	key := a.getCacheKey(awsAuthorization)
+	key := a.getCacheKey(awsRegion, awsAuthorization)
 	if cachedEntry, exists := a.items[key]; exists {
 		cachedEntry.usages[awsAuthorization.TriggerUniqueKey] = true
 		a.items[key] = cachedEntry
@@ -125,10 +125,10 @@ func (a *sharedConfigCache) GetCredentials(ctx context.Context, awsRegion string
 // RemoveCachedEntry removes the usage of an AuthorizationMetadata from the cached item.
 // If there isn't any usage of a given cached item (because there isn't any trigger using the aws.Config),
 // we also remove it from the cache
-func (a *sharedConfigCache) RemoveCachedEntry(awsAuthorization AuthorizationMetadata) {
+func (a *sharedConfigCache) RemoveCachedEntry(awsRegion string, awsAuthorization AuthorizationMetadata) {
 	a.Lock()
 	defer a.Unlock()
-	key := a.getCacheKey(awsAuthorization)
+	key := a.getCacheKey(awsRegion, awsAuthorization)
 	if cachedEntry, exists := a.items[key]; exists {
 		// Delete the TriggerUniqueKey from usages
 		delete(cachedEntry.usages, awsAuthorization.TriggerUniqueKey)

pkg/scalers/aws/aws_config_cache_test.go

@@ -34,7 +34,7 @@ func TestGetCredentialsReturnNewItemAndStoreItIfNotExist(t *testing.T) {
 			TriggerUniqueKey: "test-key",
 		},
 	}
-	cacheKey := cache.getCacheKey(config.awsAuthorization)
+	cacheKey := cache.getCacheKey(config.awsRegion, config.awsAuthorization)
 	_, err := cache.GetCredentials(context.Background(), config.awsRegion, config.awsAuthorization)
 	assert.NoError(t, err)
 	assert.Contains(t, cache.items, cacheKey)
@@ -52,7 +52,7 @@ func TestGetCredentialsReturnCachedItemIfExist(t *testing.T) {
 	}
 	cfg := aws.Config{}
 	cfg.AppID = "test1-app"
-	cacheKey := cache.getCacheKey(config.awsAuthorization)
+	cacheKey := cache.getCacheKey(config.awsRegion, config.awsAuthorization)
 	cache.items[cacheKey] = cacheEntry{
 		config: &cfg,
 		usages: map[string]bool{
@@ -76,14 +76,14 @@ func TestRemoveCachedEntryRemovesCachedItemIfNotUsages(t *testing.T) {
 	}
 	cfg := aws.Config{}
 	cfg.AppID = "test2-app"
-	cacheKey := cache.getCacheKey(config.awsAuthorization)
+	cacheKey := cache.getCacheKey(config.awsRegion, config.awsAuthorization)
 	cache.items[cacheKey] = cacheEntry{
 		config: &cfg,
 		usages: map[string]bool{
 			config.awsAuthorization.TriggerUniqueKey: true,
 		},
 	}
-	cache.RemoveCachedEntry(config.awsAuthorization)
+	cache.RemoveCachedEntry(config.awsRegion, config.awsAuthorization)
 	assert.NotContains(t, cache.items, cacheKey)
 }
 
@@ -98,14 +98,37 @@ func TestRemoveCachedEntryNotRemoveCachedItemIfUsages(t *testing.T) {
 	}
 	cfg := aws.Config{}
 	cfg.AppID = "test3-app"
-	cacheKey := cache.getCacheKey(config.awsAuthorization)
+	cacheKey := cache.getCacheKey(config.awsRegion, config.awsAuthorization)
 	cache.items[cacheKey] = cacheEntry{
 		config: &cfg,
 		usages: map[string]bool{
 			config.awsAuthorization.TriggerUniqueKey: true,
 			"other-usage":                            true,
 		},
 	}
-	cache.RemoveCachedEntry(config.awsAuthorization)
+	cache.RemoveCachedEntry(config.awsRegion, config.awsAuthorization)
 	assert.Contains(t, cache.items, cacheKey)
 }
+
+func TestCredentialsShouldBeCachedPerRegion(t *testing.T) {
+	cache := newSharedConfigsCache()
+	cache.logger = logr.Discard()
+	config1 := awsConfigMetadata{
+		awsRegion: "test4-region1",
+		awsAuthorization: AuthorizationMetadata{
+			TriggerUniqueKey: "test4-key1",
+		},
+	}
+	config2 := awsConfigMetadata{
+		awsRegion: "test4-region2",
+		awsAuthorization: AuthorizationMetadata{
+			TriggerUniqueKey: "test4-key2",
+		},
+	}
+	cred1, err1 := cache.GetCredentials(context.Background(), config1.awsRegion, config1.awsAuthorization)
+	cred2, err2 := cache.GetCredentials(context.Background(), config2.awsRegion, config2.awsAuthorization)
+
+	assert.NoError(t, err1)
+	assert.NoError(t, err2)
+	assert.NotEqual(t, cred1, cred2, "Credentials should be stored per region")
+}

pkg/scalers/aws_cloudwatch_scaler.go

@@ -214,7 +214,7 @@ func (s *awsCloudwatchScaler) GetMetricSpecForScaling(context.Context) []v2.Metr
 }
 
 func (s *awsCloudwatchScaler) Close(context.Context) error {
-	awsutils.ClearAwsConfig(s.metadata.awsAuthorization)
+	awsutils.ClearAwsConfig(s.metadata.AwsRegion, s.metadata.awsAuthorization)
 	return nil
 }
 

pkg/scalers/aws_dynamodb_scaler.go

@@ -164,7 +164,7 @@ func (s *awsDynamoDBScaler) GetMetricSpecForScaling(context.Context) []v2.Metric
 }
 
 func (s *awsDynamoDBScaler) Close(context.Context) error {
-	awsutils.ClearAwsConfig(s.metadata.awsAuthorization)
+	awsutils.ClearAwsConfig(s.metadata.AwsRegion, s.metadata.awsAuthorization)
 	return nil
 }
 

pkg/scalers/aws_dynamodb_streams_scaler.go

@@ -169,7 +169,7 @@ func getDynamoDBStreamsArn(ctx context.Context, db dynamodb.DescribeTableAPIClie
 }
 
 func (s *awsDynamoDBStreamsScaler) Close(_ context.Context) error {
-	awsutils.ClearAwsConfig(s.metadata.awsAuthorization)
+	awsutils.ClearAwsConfig(s.metadata.awsRegion, s.metadata.awsAuthorization)
 	return nil
 }
 

pkg/scalers/aws_kinesis_stream_scaler.go

@@ -143,7 +143,7 @@ func createKinesisClient(ctx context.Context, metadata *awsKinesisStreamMetadata
 }
 
 func (s *awsKinesisStreamScaler) Close(context.Context) error {
-	awsutils.ClearAwsConfig(s.metadata.awsAuthorization)
+	awsutils.ClearAwsConfig(s.metadata.awsRegion, s.metadata.awsAuthorization)
 	return nil
 }
 

pkg/scalers/aws_sqs_queue_scaler.go

@@ -202,7 +202,7 @@ func createSqsClient(ctx context.Context, metadata *awsSqsQueueMetadata) (*sqs.C
 }
 
 func (s *awsSqsQueueScaler) Close(context.Context) error {
-	awsutils.ClearAwsConfig(s.metadata.awsAuthorization)
+	awsutils.ClearAwsConfig(s.metadata.awsRegion, s.metadata.awsAuthorization)
 	return nil
 }
 

pkg/scaling/resolver/aws_secretmanager_handler.go

@@ -109,5 +109,5 @@ func (ash *AwsSecretManagerHandler) Initialize(ctx context.Context, client clien
 }
 
 func (ash *AwsSecretManagerHandler) Stop() {
-	awsutils.ClearAwsConfig(ash.awsMetadata)
+	awsutils.ClearAwsConfig(ash.secretManager.Region, ash.awsMetadata)
 }