From 574923afc66ae6bd8bbac57584a7391cbdb39f72 Mon Sep 17 00:00:00 2001
From: Christopher Hunter <hunterch@vmware.com>
Date: Tue, 11 Jul 2023 13:43:41 -0400
Subject: [PATCH] rename Kilnfile key to "role_arn" from "aws_role_arn"

we don't specify the iaas implementation in other config keys
so we decided to remove the prefix here too

we also changed the control flow in NewS3ReleaseSourceFromConfig
to miror the implementation in leftovers more closely: https://github.com/pivotal/leftovers/commit/34fcf991bb381011f8ead1acb9beba866da94025

Co-authored-by: Ramkumar Vengadakrishnan <ramkumarv@vmware.com>
---
 internal/component/s3_release_source.go | 41 ++++++++++++++++---------
 pkg/cargo/files_test.go                 |  4 +--
 pkg/cargo/kilnfile.go                   |  2 +-
 3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/internal/component/s3_release_source.go b/internal/component/s3_release_source.go
index fcbf49448..5f9737572 100644
--- a/internal/component/s3_release_source.go
+++ b/internal/component/s3_release_source.go
@@ -68,25 +68,23 @@ func NewS3ReleaseSource(c cargo.ReleaseSourceConfig, client S3Client, downloader
 func NewS3ReleaseSourceFromConfig(config cargo.ReleaseSourceConfig, logger *log.Logger) S3ReleaseSource {
 	validateConfig(config)
 
-	// https://docs.aws.amazon.com/sdk-for-go/api/service/s3/
-	awsConfig := &aws.Config{
-		Region:      aws.String(config.Region),
-		Credentials: credentials.NewStaticCredentials(config.AccessKeyId, config.SecretAccessKey, ""),
-	}
-
-	var assumedRoleAwsConfig aws.Config
-	if config.AwsRoleARN != "" {
-		stsSession := session.Must(session.NewSession(awsConfig))
-		roleCredentials := stscreds.NewCredentials(stsSession, config.AwsRoleARN)
-		assumedRoleAwsConfig.Credentials = roleCredentials
+	awsConfig := awsRegionAndEndpointConfiguration(config).WithCredentials(credentials.NewStaticCredentials(config.AccessKeyId, config.SecretAccessKey, ""))
+	sess, err := session.NewSession(awsConfig)
+	if err != nil {
+		// TODO: add test coverage for this block
+		panic(err)
 	}
 
-	if config.Endpoint != "" { // for acceptance testing
-		awsConfig = awsConfig.WithEndpoint(config.Endpoint)
-		awsConfig = awsConfig.WithS3ForcePathStyle(true)
+	if config.RoleARN != "" {
+		// TODO: add test coverage for this block
+		awsConfigWithARN := awsRegionAndEndpointConfiguration(config).WithCredentials(stscreds.NewCredentials(sess, config.RoleARN))
+		sess, err = session.NewSession(awsConfigWithARN)
+		if err != nil {
+			// TODO: add test coverage for this block
+			panic(err)
+		}
 	}
 
-	sess := session.Must(session.NewSession(awsConfig, &assumedRoleAwsConfig))
 	client := s3.New(sess)
 
 	return NewS3ReleaseSource(
@@ -98,6 +96,19 @@ func NewS3ReleaseSourceFromConfig(config cargo.ReleaseSourceConfig, logger *log.
 	)
 }
 
+func awsRegionAndEndpointConfiguration(config cargo.ReleaseSourceConfig) *aws.Config {
+	awsConfig := &aws.Config{
+		Region: aws.String(config.Region),
+	}
+
+	if config.Endpoint != "" { // for acceptance testing
+		awsConfig = awsConfig.WithEndpoint(config.Endpoint)
+		awsConfig = awsConfig.WithS3ForcePathStyle(true)
+	}
+
+	return awsConfig
+}
+
 func validateConfig(config cargo.ReleaseSourceConfig) {
 	if config.PathTemplate == "" {
 		panic(`Missing required field "path_template" in release source config. Is your Kilnfile out of date?`)
diff --git a/pkg/cargo/files_test.go b/pkg/cargo/files_test.go
index ac8370439..3e87281c6 100644
--- a/pkg/cargo/files_test.go
+++ b/pkg/cargo/files_test.go
@@ -64,7 +64,7 @@ release_sources:
     region: $( variable "region" )
     access_key_id: $( variable "access_key" )
     secret_access_key: $( variable "secret_key" )
-    aws_role_arn: $( variable "role_arn" )
+    role_arn: $( variable "role_arn" )
     path_template: $( variable "path_template" )
 `
 
@@ -91,7 +91,7 @@ release_sources:
 				Bucket:          "my-bucket",
 				Region:          "middle-earth",
 				AccessKeyId:     "id",
-				AwsRoleARN:      "role-arn",
+				RoleARN:         "role-arn",
 				SecretAccessKey: "key",
 				PathTemplate:    "not-used",
 			},
diff --git a/pkg/cargo/kilnfile.go b/pkg/cargo/kilnfile.go
index 852da19fd..d309c6e61 100644
--- a/pkg/cargo/kilnfile.go
+++ b/pkg/cargo/kilnfile.go
@@ -109,7 +109,7 @@ type ReleaseSourceConfig struct {
 	Region          string `yaml:"region,omitempty"`
 	AccessKeyId     string `yaml:"access_key_id,omitempty"`
 	SecretAccessKey string `yaml:"secret_access_key,omitempty"`
-	AwsRoleARN      string `yaml:"aws_role_arn,omitempty"`
+	RoleARN         string `yaml:"role_arn,omitempty"`
 	PathTemplate    string `yaml:"path_template,omitempty"`
 	Endpoint        string `yaml:"endpoint,omitempty"`
 	Org             string `yaml:"org,omitempty"`