Add loadConfig S3 based on AWS_PROFILE ~/.aws/credentials #5680
Conversation
|
Thanks for your contribution @alban-stourbe-wmx ! :) |
|
Could you please merge the |
alban-stourbe-wmx
force-pushed
the
feature/add-aws-profile-from-aws-credentials
branch
from
1649ff5 to
e35c604
Compare
WalkthroughThe changes enhance AWS S3 credential handling across the application. The validation logic now first checks if the AWS profile is provided before verifying the access key, secret key, and region. An environment variable is read for the AWS profile, and the S3 client creation now supports using a specified profile when loading the configuration. Additionally, the Options struct has been extended to include the AWS profile information. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant S3Client as getS3Client
participant AWSConfig
Caller->>S3Client: call getS3Client(ctx, accessKey, secretKey, region, profile)
alt Profile Provided
S3Client->>AWSConfig: load configuration using profile
else Profile Empty
alt Credentials Provided
S3Client->>AWSConfig: load configuration using accessKey, secretKey and region
else No Credentials
S3Client->>AWSConfig: load default configuration
end
end
S3Client-->>Caller: Return S3 client or error
sequenceDiagram
participant Caller
participant Validator as validateMissingS3Options
Caller->>Validator: call with AWS options
alt AwsProfile is empty
Validator->>Validator: Check AwsAccessKey, AwsSecretKey, AwsRegion
alt Individual Field Missing
Validator->>Validator: Append respective error message(s)
end
alt All Fields Missing
Validator->>Validator: Append error message for AWS_PROFILE
end
else
Validator->>Validator: Skip credential error for provided AwsProfile
end
Validator-->>Caller: Return errors (if any)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/external/customtemplates/s3.go (1)
107-125: Consider documenting credential precedenceThe implementation correctly checks for different authentication methods in order of precedence (profile first, then access/secret keys, then default config). Consider adding a brief comment to clarify this order for future maintenance.
func getS3Client(ctx context.Context, accessKey string, secretKey string, region string, profile string) (*s3.Client, error) { + // Authentication precedence: + // 1. AWS Profile (if provided) + // 2. Static credentials (if access key and secret key are provided) + // 3. Default AWS credential chain var cfg aws.Config var err error if profile != "" {internal/runner/options.go (1)
238-251: Consider improving validation clarityThe validation logic correctly checks for either the AWS profile or the combination of other AWS credentials, but the error messaging could be improved for clarity.
if options.AwsProfile == "" { if options.AwsAccessKey == "" { missing = append(missing, "AWS_ACCESS_KEY") } if options.AwsSecretKey == "" { missing = append(missing, "AWS_SECRET_KEY") } if options.AwsRegion == "" { missing = append(missing, "AWS_REGION") } } - if (options.AwsAccessKey == "" || options.AwsSecretKey == "" || options.AwsRegion == "") && options.AwsProfile == "" { - missing = append(missing, "AWS_PROFILE") + // If we have neither valid profile nor complete credentials, suggest both options + if options.AwsProfile == "" && (options.AwsAccessKey == "" || options.AwsSecretKey == "" || options.AwsRegion == "") { + missing = append(missing, "Either AWS_PROFILE or complete AWS credentials (AWS_ACCESS_KEY, AWS_SECRET_KEY, AWS_REGION)") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/runner/options.go(2 hunks)pkg/external/customtemplates/s3.go(2 hunks)pkg/types/types.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (3)
pkg/types/types.go (1)
348-349: LGTM: New AWS profile field added correctlyThe added
AwsProfilefield with proper documentation allows users to leverage AWS profiles stored in ~/.aws/credentials, which is a standard pattern for AWS authentication.pkg/external/customtemplates/s3.go (1)
65-65: LGTM: Correctly passing the new AwsProfile parameterThe call to
getS3Clienthas been updated to include the new profile parameter.internal/runner/options.go (1)
458-458: LGTM: Environment variable correctly readThe code properly reads the AWS_PROFILE environment variable.
|
@dogancanbakir i have merge into dev can u approve it please ? If it is okay for u |
Proposed changes
To retrieve a S3 bucket based on a loacConfid, i have added a feature allowing to use AWS_PROFILE from ~/.aws/credentials file.
The aim is to be able to use the profile option from aws cli like
aws s3 ls --profile my-profileChecklist
Summary by CodeRabbit