Add AWS config loader and update S3 client setup #526
Conversation
Introduced awsx/config.go to provide a unified AWS configuration loader supporting both confx and environment variables. Added comprehensive tests in awsx/config_test.go. Updated s3x/s3.go to use AWS SDK v2 config directly in S3 client setup, removing direct access key fields from S3 config.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Eliminated the accessID and accessKey fields from the example.yaml S3 configuration, likely to rely on AWS default credentials or environment configuration.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a unified AWS configuration loader and modernizes S3 client setup by migrating from direct access key fields to AWS SDK v2 configuration patterns.
- Adds a new
awsx/config.gomodule that provides centralized AWS configuration loading supporting both confx and environment variables - Updates S3 client setup to accept AWS SDK v2 config directly instead of separate access key fields
- Removes direct access key configuration from S3 config structure
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| awsx/config.go | New AWS configuration loader with support for confx and environment variables |
| awsx/config_test.go | Comprehensive test suite for the AWS configuration loader |
| s3x/s3.go | Updated S3 client setup to use AWS SDK v2 config parameter |
| s3x/example/example.yaml | Removed direct access key fields from example configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Replaces the SetupAWSConfigFactory with a direct SetupAWSConfig function, simplifying the API and removing unnecessary indirection.
Eliminated the unused lifecycle import and updated the SetupAWSConfig function signature to remove the lifecycle parameter, simplifying AWS configuration setup.
Refactored awsx/config.go to eliminate the LoadConfig function and move its logic into SetupAWSConfig. Updated tests in awsx/config_test.go to use SetupAWSConfig instead of LoadConfig for consistency and simplification.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
awsx/config.go:1
- The os.Setenv calls modify global environment state without synchronization, which could cause race conditions in concurrent usage. Consider using config.LoadDefaultConfig with config.WithCredentialsProvider to avoid global state modification.
package awsx
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| func SetupAWSConfig(ctx context.Context, conf *Config) (*aws.Config, error) { | ||
| // Set environment variables from confx config if provided | ||
| if conf.Region != "" { | ||
| os.Setenv("AWS_REGION", conf.Region) | ||
| } | ||
| if conf.AccessKeyID != "" { | ||
| os.Setenv("AWS_ACCESS_KEY_ID", conf.AccessKeyID) | ||
| } | ||
| if conf.SecretAccessKey != "" { | ||
| os.Setenv("AWS_SECRET_ACCESS_KEY", conf.SecretAccessKey) | ||
| } | ||
| if conf.SessionToken != "" { | ||
| os.Setenv("AWS_SESSION_TOKEN", conf.SessionToken) | ||
| } |
There was a problem hiding this comment.
Setting credentials in environment variables globally affects the entire process and could expose credentials to other parts of the application. Consider using aws.NewCredentialsCache with credentials.NewStaticCredentialsProvider instead to avoid modifying global environment state.
Copilot uses AI. Check for mistakes.
3 participants
Introduced awsx/config.go to provide a unified AWS configuration loader supporting both confx and environment variables. Added comprehensive tests in awsx/config_test.go. Updated s3x/s3.go to use AWS SDK v2 config directly in S3 client setup, removing direct access key fields from S3 config.