Helper function to create aws configs, add unit tests

agent/config/config.go

@@ -665,3 +665,9 @@ func (cfg *Config) String() string {
 func IsFIPSEnabled() bool {
 	return isFIPSEnabled
 }
+
+// SetFIPSEnabled sets the isFIPSEnabled variable for testing purposes
+// that is used in s3/factory/factory_test.go
+func SetFIPSEnabled(enabled bool) {
+	isFIPSEnabled = enabled
+}

agent/s3/factory/factory.go

@@ -50,49 +50,43 @@ func NewS3ClientCreator() S3ClientCreator {
 
 type s3ClientCreator struct{}
 
-// NewS3ManagerClient returns a new S3 client based on the region of the bucket.
-func (*s3ClientCreator) NewS3ManagerClient(bucket, region string,
-	creds credentials.IAMRoleCredentials) (s3client.S3ManagerClient, error) {
+func createAWSConfig(region string, creds credentials.IAMRoleCredentials) *aws.Config {
 	cfg := aws.NewConfig().
 		WithHTTPClient(httpclient.New(roundtripTimeout, false, agentversion.String(), config.OSType)).
 		WithCredentials(
-			awscreds.NewStaticCredentials(creds.AccessKeyID, creds.SecretAccessKey,
-				creds.SessionToken)).WithRegion(region)
+			awscreds.NewStaticCredentials(creds.AccessKeyID, creds.SecretAccessKey, creds.SessionToken)).
+		WithRegion(region)
 	if config.IsFIPSEnabled() {
 		logger.Debug("FIPS mode detected, using FIPS-compliant S3 endpoint")
 		cfg.UseFIPSEndpoint = endpoints.FIPSEndpointStateEnabled
+		cfg.S3ForcePathStyle = aws.Bool(false)
 	}
+	return cfg
+}
+
+// NewS3ManagerClient returns a new S3 client based on the region of the bucket.
+func (*s3ClientCreator) NewS3ManagerClient(bucket, region string, creds credentials.IAMRoleCredentials) (s3client.S3ManagerClient, error) {
+	cfg := createAWSConfig(region, creds)
 	sess := session.Must(session.NewSession(cfg))
 	svc := s3.New(sess)
 	bucketRegion, err := getRegionFromBucket(svc, bucket)
 	if err != nil {
 		return nil, err
 	}
-	cfg.WithRegion(bucketRegion)
-	sessWithRegion := session.Must(session.NewSession(cfg))
+	sessWithRegion := session.Must(session.NewSession(cfg.WithRegion(bucketRegion)))
 	return s3manager.NewDownloaderWithClient(s3.New(sessWithRegion)), nil
 }
 
 // NewS3Client returns a new S3 client to support S3 operations which are not provided by s3manager.
-func (*s3ClientCreator) NewS3Client(bucket, region string,
-	creds credentials.IAMRoleCredentials) (s3client.S3Client, error) {
-	cfg := aws.NewConfig().
-		WithHTTPClient(httpclient.New(roundtripTimeout, false, agentversion.String(), config.OSType)).
-		WithCredentials(
-			awscreds.NewStaticCredentials(creds.AccessKeyID, creds.SecretAccessKey,
-				creds.SessionToken)).WithRegion(region)
-	if config.IsFIPSEnabled() {
-		logger.Debug("FIPS mode detected, using FIPS-compliant S3 endpoint")
-		cfg.UseFIPSEndpoint = endpoints.FIPSEndpointStateEnabled
-	}
+func (*s3ClientCreator) NewS3Client(bucket, region string, creds credentials.IAMRoleCredentials) (s3client.S3Client, error) {
+	cfg := createAWSConfig(region, creds)
 	sess := session.Must(session.NewSession(cfg))
 	svc := s3.New(sess)
 	bucketRegion, err := getRegionFromBucket(svc, bucket)
 	if err != nil {
 		return nil, err
 	}
-	cfg.WithRegion(bucketRegion)
-	sessWithRegion := session.Must(session.NewSession(cfg))
+	sessWithRegion := session.Must(session.NewSession(cfg.WithRegion(bucketRegion)))
 	return s3.New(sessWithRegion), nil
 }
 func getRegionFromBucket(svc *s3.S3, bucket string) (string, error) {

agent/s3/factory/factory_test.go

@@ -0,0 +1,61 @@
+//go:build unit
+// +build unit
+
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package factory
+
+import (
+	"testing"
+
+	"github.com/aws/amazon-ecs-agent/agent/config"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/credentials"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCreateAWSConfig(t *testing.T) {
+	creds := credentials.IAMRoleCredentials{
+		AccessKeyID:     "dummyAccessKeyID",
+		SecretAccessKey: "dummySecretAccessKey",
+		SessionToken:    "dummySessionToken",
+	}
+	region := "us-west-2"
+	// Test without FIPS enabled
+	config.SetFIPSEnabled(false)
+	cfg := createAWSConfig(region, creds)
+	assert.Equal(t, roundtripTimeout, cfg.HTTPClient.Timeout, "HTTPClient timeout should be set")
+	assert.Equal(t, region, aws.StringValue(cfg.Region), "Region should be set")
+	credsValue, err := cfg.Credentials.Get()
+	assert.NoError(err)
+	assert.Equal(t, "dummyAccessKeyID", credsValue.AccessKeyID, "AccessKeyID should be set")
+	assert.Equal(t, "dummySecretAccessKey", credsValue.SecretAccessKey, "SecretAccessKey should be set")
+	assert.Equal(t, "dummySessionToken", credsValue.SessionToken, "SessionToken should be set")
+	assert.Equal(t, endpoints.FIPSEndpointStateUnset, cfg.UseFIPSEndpoint, "UseFIPSEndpoint should not be set")
+	assert.Nil(t, cfg.S3ForcePathStyle, "S3ForcePathStyle should not be set")
+
+	// Test with FIPS enabled
+	config.SetFIPSEnabled(true)
+	cfg = createAWSConfig(region, creds)
+	assert.Equal(t, roundtripTimeout, cfg.HTTPClient.Timeout, "HTTPClient timeout should be set")
+	assert.Equal(t, region, aws.StringValue(cfg.Region), "Region should be set")
+	credsValue, err = cfg.Credentials.Get()
+	assert.NoError(t, err)
+	assert.Equal(t, "dummyAccessKeyID", credsValue.AccessKeyID, "AccessKeyID should be set")
+	assert.Equal(t, "dummySecretAccessKey", credsValue.SecretAccessKey, "SecretAccessKey should be set")
+	assert.Equal(t, "dummySessionToken", credsValue.SessionToken, "SessionToken should be set")
+	assert.Equal(t, endpoints.FIPSEndpointStateEnabled, cfg.UseFIPSEndpoint, "UseFIPSEndpoint should be set to FIPSEndpointStateEnabled")
+	assert.False(t, aws.BoolValue(cfg.S3ForcePathStyle), "S3ForcePathStyle should be set to false")
+}