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 <[email protected]>
pivotal-cf · Jul 11, 2023 · 244c86b · 244c86b
1 parent e865aee
commit 244c86b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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"`