-
Notifications
You must be signed in to change notification settings - Fork 638
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
Added fallback behavior to custom v1 resolvers #2206
Conversation
e0b81a4
to
9a803b4
Compare
service/s3/endpoints.go
Outdated
if nf := (&aws.EndpointNotFoundError{}); !errors.As(err, &nf) { | ||
return endpoint, err | ||
if nf := (&aws.EndpointNotFoundError{}); errors.As(err, &nf) { | ||
return endpoint, nil |
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.
question: Does downstream code handle this properly? i.e. if the v1 middleware gets a zero-value endpoint, it does nothing and lets v2 pick up the rest?
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.
it didnt. pushed new commits with a fix. and tested via debugger with sample program and unit tests. heres my sample program:
package main
import (
"context"
"log"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/s3"
)
func main() {
customResolver := aws.EndpointResolverWithOptionsFunc(func(service, region string, options ...interface{}) (aws.Endpoint, error) {
return aws.Endpoint{}, &aws.EndpointNotFoundError{}
})
cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithEndpointResolverWithOptions(customResolver), config.WithRegion("us-west-2"))
client := s3.NewFromConfig(cfg)
_, err = client.GetObject(context.TODO(), &s3.GetObjectInput{
Bucket: aws.String("test-bucket"),
Key: aws.String("test-key"),
})
if err != nil {
log.Fatal(err)
}
}
…flag can be appropriately set
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.
Would be nice to see a unit test added to cover this behavior if possible.
When a custom legacy endpoint resolver (pre-EP20) was set via one of the config setter helper functions, it was wrapped with a fallback endpoint resolver
https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/endpoints/
This alters that previous fallback behavior to fallback to the endpoint resolver v2