From 0c0baa0903a5a7623fb1243924f1abd996196b37 Mon Sep 17 00:00:00 2001 From: Nisan Itzhakov Date: Sun, 8 Mar 2020 16:24:30 +0200 Subject: [PATCH 1/3] Authenticate to AWS with dedicated role without AssumeRole permissions --- pkg/scalers/aws_cloudwatch_scaler.go | 24 +++++++---- pkg/scalers/aws_cloudwatch_test.go | 15 +++++++ pkg/scalers/aws_iam_authorization.go | 55 +++++++++++++----------- pkg/scalers/aws_kinesis_stream_scaler.go | 24 +++++++---- pkg/scalers/aws_kinesis_stream_test.go | 22 +++++++++- pkg/scalers/aws_sqs_queue_scaler.go | 24 +++++++---- pkg/scalers/aws_sqs_queue_test.go | 11 +++++ 7 files changed, 126 insertions(+), 49 deletions(-) diff --git a/pkg/scalers/aws_cloudwatch_scaler.go b/pkg/scalers/aws_cloudwatch_scaler.go index 6634db0c7a0..c0686a68da6 100644 --- a/pkg/scalers/aws_cloudwatch_scaler.go +++ b/pkg/scalers/aws_cloudwatch_scaler.go @@ -199,16 +199,24 @@ func (c *awsCloudwatchScaler) GetCloudwatchMetrics() (float64, error) { sess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(c.metadata.awsRegion), })) - creds := credentials.NewStaticCredentials(c.metadata.awsAuthorization.awsAccessKeyID, c.metadata.awsAuthorization.awsSecretAccessKey, "") - if c.metadata.awsAuthorization.awsRoleArn != "" { - creds = stscreds.NewCredentials(sess, c.metadata.awsAuthorization.awsRoleArn) - } + var cloudwatchClient *cloudwatch.CloudWatch + if c.metadata.awsAuthorization.podIdentity { + creds := credentials.NewStaticCredentials(c.metadata.awsAuthorization.awsAccessKeyID, c.metadata.awsAuthorization.awsSecretAccessKey, "") + + if c.metadata.awsAuthorization.awsRoleArn != "" { + creds = stscreds.NewCredentials(sess, c.metadata.awsAuthorization.awsRoleArn) + } - cloudwatchClient := cloudwatch.New(sess, &aws.Config{ - Region: aws.String(c.metadata.awsRegion), - Credentials: creds, - }) + cloudwatchClient = cloudwatch.New(sess, &aws.Config{ + Region: aws.String(c.metadata.awsRegion), + Credentials: creds, + }) + } else { + cloudwatchClient = cloudwatch.New(sess, &aws.Config{ + Region: aws.String(c.metadata.awsRegion), + }) + } input := cloudwatch.GetMetricDataInput{ StartTime: aws.Time(time.Now().Add(time.Second * -1 * time.Duration(c.metadata.metricCollectionTime))), diff --git a/pkg/scalers/aws_cloudwatch_test.go b/pkg/scalers/aws_cloudwatch_test.go index 7fdad030a05..8790010eb08 100644 --- a/pkg/scalers/aws_cloudwatch_test.go +++ b/pkg/scalers/aws_cloudwatch_test.go @@ -144,6 +144,21 @@ var testAWSCloudwatchMetadata = []parseAWSCloudwatchMetadataTestData{ }, false, "with AWS Role from TriggerAuthentication"}, + {map[string]string{ + "namespace": "AWS/SQS", + "dimensionName": "QueueName", + "dimensionValue": "keda", + "metricName": "ApproximateNumberOfMessagesVisible", + "targetMetricValue": "2", + "minMetricValue": "0", + "metricCollectionTime": "300", + "metricStat": "Average", + "metricStatPeriod": "300", + "awsRegion": "eu-west-1", + "podIdentity": "false"}, + map[string]string{}, + false, + "with AWS Role assigned on KEDA operator itself"}, } func TestCloudwatchParseMetadata(t *testing.T) { diff --git a/pkg/scalers/aws_iam_authorization.go b/pkg/scalers/aws_iam_authorization.go index 624c50700e1..16a9d1723b1 100644 --- a/pkg/scalers/aws_iam_authorization.go +++ b/pkg/scalers/aws_iam_authorization.go @@ -14,37 +14,44 @@ type awsAuthorizationMetadata struct { awsAccessKeyID string awsSecretAccessKey string awsSessionToken string + + podIdentity bool } func getAwsAuthorization(authParams, metadata, resolvedEnv map[string]string) (awsAuthorizationMetadata, error) { meta := awsAuthorizationMetadata{} - if authParams["awsRoleArn"] != "" { - meta.awsRoleArn = authParams["awsRoleArn"] - } else if (authParams["awsAccessKeyID"] != "" || authParams["awsAccessKeyId"] != "") && authParams["awsSecretAccessKey"] != "" { - meta.awsAccessKeyID = authParams["awsAccessKeyID"] - if meta.awsAccessKeyID == "" { - meta.awsAccessKeyID = authParams["awsAccessKeyId"] - } - meta.awsSecretAccessKey = authParams["awsSecretAccessKey"] + if metadata["podIdentity"] == "false" { + meta.podIdentity = false } else { - var keyName string - if keyName = metadata["awsAccessKeyID"]; keyName == "" { - keyName = awsAccessKeyIDEnvVar - } - if val, ok := resolvedEnv[keyName]; ok && val != "" { - meta.awsAccessKeyID = val - } else { - return meta, fmt.Errorf("'%s' doesn't exist in the deployment environment", keyName) - } - - if keyName = metadata["awsSecretAccessKey"]; keyName == "" { - keyName = awsSecretAccessKeyEnvVar - } - if val, ok := resolvedEnv[keyName]; ok && val != "" { - meta.awsSecretAccessKey = val + meta.podIdentity = true + if authParams["awsRoleArn"] != "" { + meta.awsRoleArn = authParams["awsRoleArn"] + } else if (authParams["awsAccessKeyID"] != "" || authParams["awsAccessKeyId"] != "") && authParams["awsSecretAccessKey"] != "" { + meta.awsAccessKeyID = authParams["awsAccessKeyID"] + if meta.awsAccessKeyID == "" { + meta.awsAccessKeyID = authParams["awsAccessKeyId"] + } + meta.awsSecretAccessKey = authParams["awsSecretAccessKey"] } else { - return meta, fmt.Errorf("'%s' doesn't exist in the deployment environment", keyName) + var keyName string + if keyName = metadata["awsAccessKeyID"]; keyName == "" { + keyName = awsAccessKeyIDEnvVar + } + if val, ok := resolvedEnv[keyName]; ok && val != "" { + meta.awsAccessKeyID = val + } else { + return meta, fmt.Errorf("'%s' doesn't exist in the deployment environment", keyName) + } + + if keyName = metadata["awsSecretAccessKey"]; keyName == "" { + keyName = awsSecretAccessKeyEnvVar + } + if val, ok := resolvedEnv[keyName]; ok && val != "" { + meta.awsSecretAccessKey = val + } else { + return meta, fmt.Errorf("'%s' doesn't exist in the deployment environment", keyName) + } } } diff --git a/pkg/scalers/aws_kinesis_stream_scaler.go b/pkg/scalers/aws_kinesis_stream_scaler.go index 846d4ccac1d..fe4c91c25b1 100644 --- a/pkg/scalers/aws_kinesis_stream_scaler.go +++ b/pkg/scalers/aws_kinesis_stream_scaler.go @@ -135,16 +135,24 @@ func (s *awsKinesisStreamScaler) GetAwsKinesisOpenShardCount() (int64, error) { sess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(s.metadata.awsRegion), })) - creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") - if s.metadata.awsAuthorization.awsRoleArn != "" { - creds = stscreds.NewCredentials(sess, s.metadata.awsAuthorization.awsRoleArn) - } + var kinesisClinent *kinesis.Kinesis + if s.metadata.awsAuthorization.podIdentity { + creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") + + if s.metadata.awsAuthorization.awsRoleArn != "" { + creds = stscreds.NewCredentials(sess, s.metadata.awsAuthorization.awsRoleArn) + } - kinesisClinent := kinesis.New(sess, &aws.Config{ - Region: aws.String(s.metadata.awsRegion), - Credentials: creds, - }) + kinesisClinent = kinesis.New(sess, &aws.Config{ + Region: aws.String(s.metadata.awsRegion), + Credentials: creds, + }) + } else { + kinesisClinent = kinesis.New(sess, &aws.Config{ + Region: aws.String(s.metadata.awsRegion), + }) + } output, err := kinesisClinent.DescribeStreamSummary(input) if err != nil { diff --git a/pkg/scalers/aws_kinesis_stream_test.go b/pkg/scalers/aws_kinesis_stream_test.go index eb62bc99c67..7241f00bebb 100644 --- a/pkg/scalers/aws_kinesis_stream_test.go +++ b/pkg/scalers/aws_kinesis_stream_test.go @@ -51,6 +51,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, + podIdentity: true, }, }, isError: false, @@ -86,6 +87,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, + podIdentity: true, }, }, isError: false, @@ -103,6 +105,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, + podIdentity: true, }, }, isError: false, @@ -143,11 +146,28 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ streamName: testAWSKinesisStreamName, awsRegion: testAWSRegion, awsAuthorization: awsAuthorizationMetadata{ - awsRoleArn: testAWSKinesisRoleArn, + awsRoleArn: testAWSKinesisRoleArn, + podIdentity: true, }, }, isError: false, comment: "with AWS Role from TriggerAuthentication"}, + {metadata: map[string]string{ + "streamName": testAWSKinesisStreamName, + "shardCount": "2", + "awsRegion": testAWSRegion, + "podIdentity": "false"}, + authParams: map[string]string{}, + expected: &awsKinesisStreamMetadata{ + targetShardCount: 2, + streamName: testAWSKinesisStreamName, + awsRegion: testAWSRegion, + awsAuthorization: awsAuthorizationMetadata{ + podIdentity: false, + }, + }, + isError: false, + comment: "with AWS Role assigned on KEDA operator itself"}, } func TestKinesisParseMetadata(t *testing.T) { diff --git a/pkg/scalers/aws_sqs_queue_scaler.go b/pkg/scalers/aws_sqs_queue_scaler.go index 9834c8e6266..c0dc590bf1f 100644 --- a/pkg/scalers/aws_sqs_queue_scaler.go +++ b/pkg/scalers/aws_sqs_queue_scaler.go @@ -152,16 +152,24 @@ func (s *awsSqsQueueScaler) GetAwsSqsQueueLength() (int32, error) { sess := session.Must(session.NewSession(&aws.Config{ Region: aws.String(s.metadata.awsRegion), })) - creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") - if s.metadata.awsAuthorization.awsRoleArn != "" { - creds = stscreds.NewCredentials(sess, s.metadata.awsAuthorization.awsRoleArn) - } + var sqsClient *sqs.SQS + if s.metadata.awsAuthorization.podIdentity { + creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") + + if s.metadata.awsAuthorization.awsRoleArn != "" { + creds = stscreds.NewCredentials(sess, s.metadata.awsAuthorization.awsRoleArn) + } - sqsClient := sqs.New(sess, &aws.Config{ - Region: aws.String(s.metadata.awsRegion), - Credentials: creds, - }) + sqsClient = sqs.New(sess, &aws.Config{ + Region: aws.String(s.metadata.awsRegion), + Credentials: creds, + }) + } else { + sqsClient = sqs.New(sess, &aws.Config{ + Region: aws.String(s.metadata.awsRegion), + }) + } output, err := sqsClient.GetQueueAttributes(input) if err != nil { diff --git a/pkg/scalers/aws_sqs_queue_test.go b/pkg/scalers/aws_sqs_queue_test.go index 7c143a5bd61..23ca77765bc 100644 --- a/pkg/scalers/aws_sqs_queue_test.go +++ b/pkg/scalers/aws_sqs_queue_test.go @@ -117,6 +117,17 @@ var testAWSSQSMetadata = []parseAWSSQSMetadataTestData{ }, false, "with AWS Role from TriggerAuthentication"}, + {map[string]string{ + "queueURL": testAWSSQSProperQueueURL, + "queueLength": "1", + "awsRegion": "eu-west-1", + "podIdentity": "false"}, + map[string]string{ + "awsAccessKeyId": "", + "awsSecretAccessKey": "", + }, + false, + "with AWS Role assigned on KEDA operator itself"}, } func TestSQSParseMetadata(t *testing.T) { From 52120b9cde040357d606e3d20bf290d39bfcae93 Mon Sep 17 00:00:00 2001 From: Nisan Itzhakov Date: Tue, 10 Mar 2020 16:11:58 +0200 Subject: [PATCH 2/3] rename podIdentity to identityOwner --- pkg/scalers/aws_cloudwatch_scaler.go | 2 +- pkg/scalers/aws_iam_authorization.go | 10 +++++----- pkg/scalers/aws_kinesis_stream_scaler.go | 2 +- pkg/scalers/aws_kinesis_stream_test.go | 12 ++++++------ pkg/scalers/aws_sqs_queue_scaler.go | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/scalers/aws_cloudwatch_scaler.go b/pkg/scalers/aws_cloudwatch_scaler.go index c0686a68da6..b19012e3acc 100644 --- a/pkg/scalers/aws_cloudwatch_scaler.go +++ b/pkg/scalers/aws_cloudwatch_scaler.go @@ -201,7 +201,7 @@ func (c *awsCloudwatchScaler) GetCloudwatchMetrics() (float64, error) { })) var cloudwatchClient *cloudwatch.CloudWatch - if c.metadata.awsAuthorization.podIdentity { + if c.metadata.awsAuthorization.podIdentityOwner { creds := credentials.NewStaticCredentials(c.metadata.awsAuthorization.awsAccessKeyID, c.metadata.awsAuthorization.awsSecretAccessKey, "") if c.metadata.awsAuthorization.awsRoleArn != "" { diff --git a/pkg/scalers/aws_iam_authorization.go b/pkg/scalers/aws_iam_authorization.go index 16a9d1723b1..5754234b324 100644 --- a/pkg/scalers/aws_iam_authorization.go +++ b/pkg/scalers/aws_iam_authorization.go @@ -15,16 +15,16 @@ type awsAuthorizationMetadata struct { awsSecretAccessKey string awsSessionToken string - podIdentity bool + podIdentityOwner bool } func getAwsAuthorization(authParams, metadata, resolvedEnv map[string]string) (awsAuthorizationMetadata, error) { meta := awsAuthorizationMetadata{} - if metadata["podIdentity"] == "false" { - meta.podIdentity = false - } else { - meta.podIdentity = true + if metadata["identityOwner"] == "operator" { + meta.podIdentityOwner = false + } else if metadata["identityOwner"] == "" || metadata["identityOwner"] == "pod" { + meta.podIdentityOwner = true if authParams["awsRoleArn"] != "" { meta.awsRoleArn = authParams["awsRoleArn"] } else if (authParams["awsAccessKeyID"] != "" || authParams["awsAccessKeyId"] != "") && authParams["awsSecretAccessKey"] != "" { diff --git a/pkg/scalers/aws_kinesis_stream_scaler.go b/pkg/scalers/aws_kinesis_stream_scaler.go index fe4c91c25b1..8740f427e36 100644 --- a/pkg/scalers/aws_kinesis_stream_scaler.go +++ b/pkg/scalers/aws_kinesis_stream_scaler.go @@ -137,7 +137,7 @@ func (s *awsKinesisStreamScaler) GetAwsKinesisOpenShardCount() (int64, error) { })) var kinesisClinent *kinesis.Kinesis - if s.metadata.awsAuthorization.podIdentity { + if s.metadata.awsAuthorization.podIdentityOwner { creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") if s.metadata.awsAuthorization.awsRoleArn != "" { diff --git a/pkg/scalers/aws_kinesis_stream_test.go b/pkg/scalers/aws_kinesis_stream_test.go index 7241f00bebb..7a37cbbf4ba 100644 --- a/pkg/scalers/aws_kinesis_stream_test.go +++ b/pkg/scalers/aws_kinesis_stream_test.go @@ -51,7 +51,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, - podIdentity: true, + podIdentityOwner: true, }, }, isError: false, @@ -87,7 +87,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, - podIdentity: true, + podIdentityOwner: true, }, }, isError: false, @@ -105,7 +105,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ awsAuthorization: awsAuthorizationMetadata{ awsAccessKeyID: testAWSKinesisAccessKeyID, awsSecretAccessKey: testAWSKinesisSecretAccessKey, - podIdentity: true, + podIdentityOwner: true, }, }, isError: false, @@ -146,8 +146,8 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ streamName: testAWSKinesisStreamName, awsRegion: testAWSRegion, awsAuthorization: awsAuthorizationMetadata{ - awsRoleArn: testAWSKinesisRoleArn, - podIdentity: true, + awsRoleArn: testAWSKinesisRoleArn, + podIdentityOwner: true, }, }, isError: false, @@ -163,7 +163,7 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ streamName: testAWSKinesisStreamName, awsRegion: testAWSRegion, awsAuthorization: awsAuthorizationMetadata{ - podIdentity: false, + podIdentityOwner: false, }, }, isError: false, diff --git a/pkg/scalers/aws_sqs_queue_scaler.go b/pkg/scalers/aws_sqs_queue_scaler.go index c0dc590bf1f..69f18b31d0c 100644 --- a/pkg/scalers/aws_sqs_queue_scaler.go +++ b/pkg/scalers/aws_sqs_queue_scaler.go @@ -154,7 +154,7 @@ func (s *awsSqsQueueScaler) GetAwsSqsQueueLength() (int32, error) { })) var sqsClient *sqs.SQS - if s.metadata.awsAuthorization.podIdentity { + if s.metadata.awsAuthorization.podIdentityOwner { creds := credentials.NewStaticCredentials(s.metadata.awsAuthorization.awsAccessKeyID, s.metadata.awsAuthorization.awsSecretAccessKey, "") if s.metadata.awsAuthorization.awsRoleArn != "" { From 04f8e9ec27d4b1616ab9e70d29109ef80ceaf70e Mon Sep 17 00:00:00 2001 From: Nisan Itzhakov Date: Tue, 10 Mar 2020 16:33:30 +0200 Subject: [PATCH 3/3] fix tests --- pkg/scalers/aws_cloudwatch_test.go | 2 +- pkg/scalers/aws_kinesis_stream_test.go | 8 ++++---- pkg/scalers/aws_sqs_queue_test.go | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/scalers/aws_cloudwatch_test.go b/pkg/scalers/aws_cloudwatch_test.go index 8790010eb08..4ad1c10cf10 100644 --- a/pkg/scalers/aws_cloudwatch_test.go +++ b/pkg/scalers/aws_cloudwatch_test.go @@ -155,7 +155,7 @@ var testAWSCloudwatchMetadata = []parseAWSCloudwatchMetadataTestData{ "metricStat": "Average", "metricStatPeriod": "300", "awsRegion": "eu-west-1", - "podIdentity": "false"}, + "identityOwner": "operator"}, map[string]string{}, false, "with AWS Role assigned on KEDA operator itself"}, diff --git a/pkg/scalers/aws_kinesis_stream_test.go b/pkg/scalers/aws_kinesis_stream_test.go index 7a37cbbf4ba..ba55f3df1ff 100644 --- a/pkg/scalers/aws_kinesis_stream_test.go +++ b/pkg/scalers/aws_kinesis_stream_test.go @@ -153,10 +153,10 @@ var testAWSKinesisMetadata = []parseAWSKinesisMetadataTestData{ isError: false, comment: "with AWS Role from TriggerAuthentication"}, {metadata: map[string]string{ - "streamName": testAWSKinesisStreamName, - "shardCount": "2", - "awsRegion": testAWSRegion, - "podIdentity": "false"}, + "streamName": testAWSKinesisStreamName, + "shardCount": "2", + "awsRegion": testAWSRegion, + "identityOwner": "operator"}, authParams: map[string]string{}, expected: &awsKinesisStreamMetadata{ targetShardCount: 2, diff --git a/pkg/scalers/aws_sqs_queue_test.go b/pkg/scalers/aws_sqs_queue_test.go index 23ca77765bc..0b55484e889 100644 --- a/pkg/scalers/aws_sqs_queue_test.go +++ b/pkg/scalers/aws_sqs_queue_test.go @@ -118,10 +118,10 @@ var testAWSSQSMetadata = []parseAWSSQSMetadataTestData{ false, "with AWS Role from TriggerAuthentication"}, {map[string]string{ - "queueURL": testAWSSQSProperQueueURL, - "queueLength": "1", - "awsRegion": "eu-west-1", - "podIdentity": "false"}, + "queueURL": testAWSSQSProperQueueURL, + "queueLength": "1", + "awsRegion": "eu-west-1", + "identityOwner": "operator"}, map[string]string{ "awsAccessKeyId": "", "awsSecretAccessKey": "",