From 7ede46e0932b4d8e0eef99fd871add8e193df22b Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Thu, 12 Dec 2019 10:35:49 -0800 Subject: [PATCH] aws/endpoints: Fix SDK resolving endpoint without region (#420) Fixes the SDK's endpoint resolve incorrectly resolving endpoints for a service when the region is empty. Also fixes the SDK attempting to resolve a service when the service value is empty. Related to: aws/aws-sdk-go#2911 --- CHANGELOG_PENDING.md | 4 +- aws/endpoints/v3model.go | 17 ++++++- aws/endpoints/v3model_test.go | 44 +++++++++++++++++++ .../service/s3/mockPaginator/mockPaginator.go | 14 ++++-- .../s3/mockPaginator/mockPaginator_test.go | 10 ++++- 5 files changed, 81 insertions(+), 8 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index cb7c688ed3e..a16b6c03cef 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -9,4 +9,6 @@ SDK Enhancements SDK Bugs --- - +* `aws/endpoints`: aws/endpoints: Fix SDK resolving endpoint without region ([#420](https://github.com/aws/aws-sdk-go-v2/pull/420)) + * Fixes the SDK's endpoint resolve incorrectly resolving endpoints for a service when the region is empty. Also fixes the SDK attempting to resolve a service when the service value is empty. + * Related to [aws/aws-sdk-go#2909](https://github.com/aws/aws-sdk-go/issues/2909) diff --git a/aws/endpoints/v3model.go b/aws/endpoints/v3model.go index 610dc1e1d39..620c1dccee6 100644 --- a/aws/endpoints/v3model.go +++ b/aws/endpoints/v3model.go @@ -74,16 +74,29 @@ func (p partition) canResolveEndpoint(service, region string, strictMatch bool) return p.RegionRegex.MatchString(region) } +var allowEmptyRegion = map[string]struct{}{ + "ec2metadata": {}, +} + +func serviceRequiresRegion(service string) bool { + _, allowed := allowEmptyRegion[service] + return !allowed +} + func (p partition) EndpointFor(service, region string, opts ResolveOptions) (resolved aws.Endpoint, err error) { s, hasService := p.Services[service] - if !hasService && opts.StrictMatching { + if len(service) == 0 || (!hasService && opts.StrictMatching) { // Only return error if the resolver will not fallback to creating // endpoint based on service endpoint ID passed in. return resolved, NewUnknownServiceError(p.ID, service, serviceList(p.Services)) } + if len(region) == 0 && !serviceRequiresRegion(service) && len(s.PartitionEndpoint) != 0 { + region = s.PartitionEndpoint + } + e, hasEndpoint := s.endpointForRegion(region) - if !hasEndpoint && opts.StrictMatching { + if len(region) == 0 || (!hasEndpoint && opts.StrictMatching) { return resolved, NewUnknownEndpointError(p.ID, service, region, endpointList(s.Endpoints)) } diff --git a/aws/endpoints/v3model_test.go b/aws/endpoints/v3model_test.go index 9f6641498f5..5779c5596d5 100644 --- a/aws/endpoints/v3model_test.go +++ b/aws/endpoints/v3model_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "reflect" "regexp" + "strings" "testing" ) @@ -522,3 +523,46 @@ func TestResolveEndpoint_AwsGlobal(t *testing.T) { t.Errorf("expect the signing name to be derived") } } + +func TestEndpointFor_EmptyRegion(t *testing.T) { + cases := map[string]struct { + Service string + Region string + RealRegion string + ExpectErr string + }{ + // Legacy services that previous accepted empty region + "ec2metadata": {Service: "ec2metadata", RealRegion: "aws-global"}, + + // Other services + "s3": {Service: "s3", Region: "us-east-1", RealRegion: "us-east-1"}, + "s3 no region": {Service: "s3", ExpectErr: "could not resolve endpoint"}, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + actual, err := NewDefaultResolver().ResolveEndpoint(c.Service, c.Region) + if len(c.ExpectErr) != 0 { + if e, a := c.ExpectErr, err.Error(); !strings.Contains(a, e) { + t.Errorf("expect %q error in %q", e, a) + } + return + } + if err != nil { + t.Fatalf("expect no error got, %v", err) + } + + expect, err := NewDefaultResolver().ResolveEndpoint(c.Service, c.RealRegion) + if err != nil { + t.Fatalf("failed to get endpoint for default resolver") + } + if e, a := expect.URL, actual.URL; e != a { + t.Errorf("expect %v URL, got %v", e, a) + } + if e, a := expect.SigningRegion, actual.SigningRegion; e != a { + t.Errorf("expect %v signing region, got %v", e, a) + } + + }) + } +} diff --git a/example/service/s3/mockPaginator/mockPaginator.go b/example/service/s3/mockPaginator/mockPaginator.go index 0055ee0bca6..a616620662e 100644 --- a/example/service/s3/mockPaginator/mockPaginator.go +++ b/example/service/s3/mockPaginator/mockPaginator.go @@ -5,6 +5,7 @@ package main import ( "context" "fmt" + "log" "os" "github.com/aws/aws-sdk-go-v2/aws/external" @@ -24,12 +25,15 @@ func main() { bucket := os.Args[1] svc := s3.New(cfg) - keys := getKeys(svc, bucket) + keys, err := getKeys(svc, bucket) + if err != nil { + log.Fatalf("failed to get keys, %v", err) + } fmt.Printf("keys for bucket %q,\n%v\n", bucket, keys) } -func getKeys(svc s3iface.ClientAPI, bucket string) []string { +func getKeys(svc s3iface.ClientAPI, bucket string) ([]string, error) { req := svc.ListObjectsRequest(&s3.ListObjectsInput{ Bucket: &bucket, }) @@ -41,5 +45,9 @@ func getKeys(svc s3iface.ClientAPI, bucket string) []string { keys = append(keys, *obj.Key) } } - return keys + if err := p.Err(); err != nil { + return nil, err + } + + return keys, nil } diff --git a/example/service/s3/mockPaginator/mockPaginator_test.go b/example/service/s3/mockPaginator/mockPaginator_test.go index f7331d34eb3..5f6fbebf086 100644 --- a/example/service/s3/mockPaginator/mockPaginator_test.go +++ b/example/service/s3/mockPaginator/mockPaginator_test.go @@ -72,10 +72,16 @@ func TestListObjectsPagination(t *testing.T) { }, } - svc.Client = s3.New(defaults.Config()) + cfg := defaults.Config() + cfg.Region = "us-west-2" + + svc.Client = s3.New(cfg) svc.objects = objects - keys := getKeys(svc, "foo") + keys, err := getKeys(svc, "foo") + if err != nil { + t.Fatalf("expect no error, got %v", err) + } expected := []string{"1", "2", "3"} if e, a := 3, len(keys); e != a {