Skip to content

Conversation

@DZDomi
Copy link

@DZDomi DZDomi commented Jun 9, 2025

Please add a summary of your change

In a previous commit (30728c2) for the function that provides AWS credentials, a crucial if was removed that was responsible for making sure callers of the function GetS3Credentials would not receive expired credentials.

Specifically the removal of the following code caused the problem:

if os.Getenv(awsRoleEnvVar) != "" {
	return nil, nil
}

By not returning nil, nil in any of the code in the specific function and forcing to return some credentials, even if they can expire (for example for IAM roles that are assumed via STS) all functions that would call this function would end up with invalid tokens after some period. This affects specifically long running backup/data mover jobs to cloud storage like S3. This PR addresses this issue, buy checking if the credentials received are able to expire and if yes, will return nil,nil and let the calling functions get their credentials via the default AWS credential provider chain.

Does your change fix a particular issue?

Fixes #8173
Fixes #8454

Please indicate you've done the following:

@blackpiglet blackpiglet force-pushed the fix/aws-credential-chain branch from 542ccc0 to f7a68a7 Compare June 16, 2025 03:07
// For expiring credentials, we want to fall back to use
// the default aws provider chain, which is responsible
// for refreshing the token automatically
if creds.CanExpire {
Copy link
Contributor

@reasonerjt reasonerjt Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By checking the caller of this func:

credValue, err := getS3Credentials(config)

I don't quite understand in unified_repo.go how it is possible to enable refreshing the token automatically by returning nil.

Could you please elaborate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR addresses this issue, buy checking if the credentials received are able to expire and if yes, will return nil,nil and let the calling functions get their credentials via the default AWS credential provider chain.

emphasis mine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants