Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Sigv4 Endpoint config parameter #494

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Sigv4 Endpoint config parameter #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sigv4/sigv4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func NewSigV4RoundTripper(cfg *SigV4Config, next http.RoundTripper) (http.RoundT
sess, err := session.NewSessionWithOptions(session.Options{
Config: aws.Config{
Region: aws.String(cfg.Region),
Endpoint: aws.String(cfg.Endpoint),
Copy link

Choose a reason for hiding this comment

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

I think what you want to change is the property STSRegionalEndpoint and set it to RegionalSTSEndpoint

References:
https://docs.aws.amazon.com/sdk-for-go/api/aws/#Config
https://github.com/aws/aws-sdk-go/blob/main/aws/endpoints/endpoints.go#L186C2-L186C21

Then we don't need to add a configuration because it is fine to always use the regional STS endpoint.

Copy link

Choose a reason for hiding this comment

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

On the other hand, I think it is safer to add a configuration parameter whose default value is the current behaviour (global endpoint - value LegacySTSEndpoint). In fact you the zero value will be the legacy endpoint so it is fine to not set if the configuration is not setting.

https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_enable-regions.html#id_credentials_region-endpoints

Copy link

Choose a reason for hiding this comment

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

still we have the benefit of not needing to provide the sts endpoint explicitly and we leave the SDK to do that for us.

Credentials: creds,
},
Profile: cfg.Profile,
Expand Down
1 change: 1 addition & 0 deletions sigv4/sigv4_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type SigV4Config struct {
SecretKey config.Secret `yaml:"secret_key,omitempty"`
Profile string `yaml:"profile,omitempty"`
RoleARN string `yaml:"role_arn,omitempty"`
Endpoint string `yaml:"endpoint,omitempty"`
}

func (c *SigV4Config) Validate() error {
Expand Down
1 change: 1 addition & 0 deletions sigv4/testdata/sigv4_good.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ access_key: AccessKey
secret_key: SecretKey
profile: profile
role_arn: blah:role/arn
endpoint: https://sts.us-west-2.amazonaws.com
Loading