credentialspec: initialize s3 client with bucket's region (#3886)

* Fix: gMSA s3 test

* Update NewS3Client

* Updated mocks

* Updated unit tests

* Create mocks for modified S3ClientCreator interface

* Modify mocks for S3ClientCreator interface

* Add error handling on creating a new S3 client

---------

Co-authored-by: EC2 Default User <[email protected]>

Author: as14692

agent/s3/factory/factory.go

@@ -34,7 +34,7 @@ const (
 
 type S3ClientCreator interface {
 	NewS3ManagerClient(bucket, region string, creds credentials.IAMRoleCredentials) (s3client.S3ManagerClient, error)
-	NewS3Client(region string, creds credentials.IAMRoleCredentials) s3client.S3Client
+	NewS3Client(bucket, region string, creds credentials.IAMRoleCredentials) (s3client.S3Client, error)
 }
 
 // NewS3ClientCreator provide 2 implementations
@@ -65,15 +65,21 @@ func (*s3ClientCreator) NewS3ManagerClient(bucket, region string,
 }
 
 // NewS3Client returns a new S3 client to support s3 operations which are not provided by s3manager.
-func (*s3ClientCreator) NewS3Client(region string,
-	creds credentials.IAMRoleCredentials) s3client.S3Client {
+func (*s3ClientCreator) NewS3Client(bucket, region string,
+	creds credentials.IAMRoleCredentials) (s3client.S3Client, error) {
 	cfg := aws.NewConfig().
 		WithHTTPClient(httpclient.New(roundtripTimeout, false)).
 		WithCredentials(
 			awscreds.NewStaticCredentials(creds.AccessKeyID, creds.SecretAccessKey,
 				creds.SessionToken)).WithRegion(region)
 	sess := session.Must(session.NewSession(cfg))
-	return s3.New(sess)
+	svc := s3.New(sess)
+	bucketRegion, err := getRegionFromBucket(svc, bucket)
+	if err != nil {
+		return nil, err
+	}
+	sessWithRegion := session.Must(session.NewSession(cfg.WithRegion(bucketRegion)))
+	return s3.New(sessWithRegion), nil
 }
 func getRegionFromBucket(svc *s3.S3, bucket string) (string, error) {
 	input := &s3.GetBucketLocationInput{

agent/s3/factory/mocks/factory_mocks.go

@@ -435,7 +435,12 @@ func (cs *CredentialSpecResource) handleS3CredentialspecFile(originalCredentialS
 		return
 	}
 
-	s3Client := cs.s3ClientCreator.NewS3Client(cs.region, iamCredentials)
+	s3Client, err := cs.s3ClientCreator.NewS3Client(bucket, cs.region, iamCredentials)
+	if err != nil {
+		cs.setTerminalReason(err.Error())
+		errorEvents <- err
+		return
+	}
 
 	credSpecJsonStringUnformatted, err := s3.GetObject(bucket, key, s3Client)
 

agent/taskresource/credentialspec/credentialspec_linux.go

@@ -374,7 +374,7 @@ func TestHandleS3CredentialSpecFileGetS3SecretValue(t *testing.T) {
 		Body: io.NopCloser(strings.NewReader(testData)),
 	}
 	gomock.InOrder(
-		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any()).Return(mockS3Client),
+		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockS3Client, nil),
 		mockS3Client.EXPECT().GetObject(gomock.Any()).Return(s3GetObjectResponse, nil).Times(1),
 	)
 
@@ -439,7 +439,7 @@ func TestHandleS3DomainlessCredentialSpecFileGetS3SecretValue(t *testing.T) {
 		Body: io.NopCloser(strings.NewReader(testData)),
 	}
 	gomock.InOrder(
-		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any()).Return(mockS3Client),
+		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockS3Client, nil),
 		mockS3Client.EXPECT().GetObject(gomock.Any()).Return(s3GetObjectResponse, nil).Times(1),
 	)
 
@@ -501,7 +501,7 @@ func TestHandleS3CredentialSpecFileGetS3SecretValueErr(t *testing.T) {
 	}, apitaskstatus.TaskStatusNone, apitaskstatus.TaskRunning)
 
 	gomock.InOrder(
-		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any()).Return(mockS3Client),
+		s3ClientCreator.EXPECT().NewS3Client(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockS3Client, nil),
 		mockS3Client.EXPECT().GetObject(gomock.Any()).Return(nil, errors.New("test-error")).Times(1),
 	)