-
Notifications
You must be signed in to change notification settings - Fork 36
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.
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
Add credentials adapter from v2 to v1 #125
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow 🎉
% make test
go test -timeout=30s -parallel=4 ./...
ok github.com/hashicorp/aws-sdk-go-base/v2 1.661s
? github.com/hashicorp/aws-sdk-go-base/v2/internal/awsconfig [no test files]
? github.com/hashicorp/aws-sdk-go-base/v2/internal/config [no test files]
? github.com/hashicorp/aws-sdk-go-base/v2/internal/constants [no test files]
? github.com/hashicorp/aws-sdk-go-base/v2/internal/endpoints [no test files]
ok github.com/hashicorp/aws-sdk-go-base/v2/internal/expand 0.452s
? github.com/hashicorp/aws-sdk-go-base/v2/internal/test [no test files]
? github.com/hashicorp/aws-sdk-go-base/v2/mockdata [no test files]
? github.com/hashicorp/aws-sdk-go-base/v2/servicemocks [no test files]
cd v2/awsv1shim && go test -timeout=30s -parallel=4 ./...
ok github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2 1.170s
? github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/mockdata [no test files]
ok github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr (cached)
v2/awsv1shim/credentials.go
Outdated
return p.RetrieveWithContext(context.Background()) | ||
} | ||
|
||
func (p *v2CredentialsProvider) getCreds() *awsv2.Credentials { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pedantic: Rename to getCredentials()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better yet, credentials()
There's nothing wrong with providing getters and setters yourself, and it's often appropriate to do so, but it's neither idiomatic nor necessary to put Get into the getter's name. If you have a field called owner (lower case, unexported), the getter method should be called Owner (upper case, exported), not GetOwner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was copied from inspired by the AWS SDK, so full of Java-isms
Adds and adapter between the AWS SDK for Go v2 and v1 credentials models. The previous approach did not work for credentials with an expiry.
Relates hashicorp/terraform-provider-aws#23176