From 244c86b08d83b42725c8e9155c7160c7ca81760d 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 | 30 +++++++++++++++++--------
 pkg/cargo/files_test.go                 |  4 ++--
 pkg/cargo/kilnfile.go                   |  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/internal/component/s3_release_source.go b/internal/component/s3_release_source.go
index fcbf49448..205f9f2b5 100644
--- a/internal/component/s3_release_source.go
+++ b/internal/component/s3_release_source.go
@@ -68,25 +68,37 @@ 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
-	}
-
 	if config.Endpoint != "" { // for acceptance testing
 		awsConfig = awsConfig.WithEndpoint(config.Endpoint)
 		awsConfig = awsConfig.WithS3ForcePathStyle(true)
 	}
 
-	sess := session.Must(session.NewSession(awsConfig, &assumedRoleAwsConfig))
+	sess, err := session.NewSession(awsConfig)
+	if err != nil {
+		// TODO: add test coverage for this block
+		panic(err)
+	}
+
+	if config.RoleARN != "" {
+		// TODO: add test coverage for this block
+		assumeRoleConfig := &aws.Config{
+			Credentials: stscreds.NewCredentials(sess, config.RoleARN),
+
+			// Note we don't set the region or endpoint here (as we did in the other aws config).
+			// The config might be populated by the other awsConfig used to create sess.
+		}
+		sess, err = session.NewSession(assumeRoleConfig)
+		if err != nil {
+			// TODO: add test coverage for this block
+			panic(err)
+		}
+	}
+
 	client := s3.New(sess)
 
 	return NewS3ReleaseSource(
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"`