From 506b1fdd6e76f0a998524f2670e24a49117ba826 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Fri, 25 Oct 2019 14:33:30 -0700 Subject: [PATCH] aws/endpoints: Fix resolve endpoint with empty region (#2911) Fixes the SDK's behavior when attempting to resolve a service's endpoint when no region was provided. Adds legacy support for services that were able to resolve a valid endpoint. No new service will support resolving an endpoint without an region. Fixes #2909 --- CHANGELOG_PENDING.md | 3 ++ aws/endpoints/sts_legacy_regions.go | 1 - aws/endpoints/v3model.go | 40 ++++++++++++---- aws/endpoints/v3model_test.go | 54 +++++++++++++++++++++ aws/session/session.go | 74 +++++++++++++++-------------- aws/session/session_test.go | 34 +++++++++++++ 6 files changed, 161 insertions(+), 45 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8290acd8f0..273bc267f3 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -6,3 +6,6 @@ * `aws/endpoints`: Add PartitionID to ResolvedEndpoint ([#2902](https://github.com/aws/aws-sdk-go/pull/2902)) ### SDK Bugs +* `aws/endpoints`: Fix resolve endpoint with empty region ([#2911](https://github.com/aws/aws-sdk-go/pull/2911)) + * Fixes the SDK's behavior when attempting to resolve a service's endpoint when no region was provided. Adds legacy support for services that were able to resolve a valid endpoint. No new service will support resolving an endpoint without an region. + * Fixes [#2909](https://github.com/aws/aws-sdk-go/issues/2909) diff --git a/aws/endpoints/sts_legacy_regions.go b/aws/endpoints/sts_legacy_regions.go index 68ed2d9e25..2613962197 100644 --- a/aws/endpoints/sts_legacy_regions.go +++ b/aws/endpoints/sts_legacy_regions.go @@ -5,7 +5,6 @@ var stsLegacyGlobalRegions = map[string]struct{}{ "ap-south-1": {}, "ap-southeast-1": {}, "ap-southeast-2": {}, - "aws-global": {}, "ca-central-1": {}, "eu-central-1": {}, "eu-north-1": {}, diff --git a/aws/endpoints/v3model.go b/aws/endpoints/v3model.go index 009f00bc32..7b09adff63 100644 --- a/aws/endpoints/v3model.go +++ b/aws/endpoints/v3model.go @@ -75,25 +75,49 @@ func (p partition) canResolveEndpoint(service, region string, strictMatch bool) return p.RegionRegex.MatchString(region) } +func allowLegacyEmptyRegion(service string) bool { + legacy := map[string]struct{}{ + "budgets": {}, + "ce": {}, + "chime": {}, + "cloudfront": {}, + "ec2metadata": {}, + "iam": {}, + "importexport": {}, + "organizations": {}, + "route53": {}, + "sts": {}, + "support": {}, + "waf": {}, + } + + _, allowed := legacy[service] + return allowed +} + func (p partition) EndpointFor(service, region string, opts ...func(*Options)) (resolved ResolvedEndpoint, err error) { var opt Options opt.Set(opts...) - if service == "sts" && opt.STSRegionalEndpoint != RegionalSTSEndpoint { - if _, ok := stsLegacyGlobalRegions[region]; ok { - region = "aws-global" - } - } - s, hasService := p.Services[service] - if !(hasService || opt.ResolveUnknownService) { + if len(service) == 0 || !(hasService || opt.ResolveUnknownService) { // 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 && allowLegacyEmptyRegion(service) && len(s.PartitionEndpoint) != 0 { + region = s.PartitionEndpoint + } + + if service == "sts" && opt.STSRegionalEndpoint != RegionalSTSEndpoint { + if _, ok := stsLegacyGlobalRegions[region]; ok { + region = "aws-global" + } + } + e, hasEndpoint := s.endpointForRegion(region) - if !hasEndpoint && opt.StrictMatching { + if len(region) == 0 || (!hasEndpoint && opt.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 f0443d3758..ab2e43668c 100644 --- a/aws/endpoints/v3model_test.go +++ b/aws/endpoints/v3model_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "reflect" "regexp" + "strings" "testing" ) @@ -539,3 +540,56 @@ func TestEndpointFor_RegionalFlag(t *testing.T) { }) } } + +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 + "budgets": {Service: "budgets", RealRegion: "aws-global"}, + "ce": {Service: "ce", RealRegion: "aws-global"}, + "chime": {Service: "chime", RealRegion: "aws-global"}, + "ec2metadata": {Service: "ec2metadata", RealRegion: "aws-global"}, + "iam": {Service: "iam", RealRegion: "aws-global"}, + "importexport": {Service: "importexport", RealRegion: "aws-global"}, + "organizations": {Service: "organizations", RealRegion: "aws-global"}, + "route53": {Service: "route53", RealRegion: "aws-global"}, + "sts": {Service: "sts", RealRegion: "aws-global"}, + "support": {Service: "support", RealRegion: "aws-global"}, + "waf": {Service: "waf", 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 := DefaultResolver().EndpointFor(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 := DefaultResolver().EndpointFor(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/aws/session/session.go b/aws/session/session.go index 245a5fb1ba..15fa647699 100644 --- a/aws/session/session.go +++ b/aws/session/session.go @@ -614,40 +614,15 @@ func (s *Session) Copy(cfgs ...*aws.Config) *Session { // ClientConfig satisfies the client.ConfigProvider interface and is used to // configure the service client instances. Passing the Session to the service // client's constructor (New) will use this method to configure the client. -func (s *Session) ClientConfig(serviceName string, cfgs ...*aws.Config) client.Config { - // Backwards compatibility, the error will be eaten if user calls ClientConfig - // directly. All SDK services will use ClientconfigWithError. - cfg, _ := s.clientConfigWithErr(serviceName, cfgs...) - - return cfg -} - -func (s *Session) clientConfigWithErr(serviceName string, cfgs ...*aws.Config) (client.Config, error) { +func (s *Session) ClientConfig(service string, cfgs ...*aws.Config) client.Config { s = s.Copy(cfgs...) - var resolved endpoints.ResolvedEndpoint - var err error - region := aws.StringValue(s.Config.Region) - - if endpoint := aws.StringValue(s.Config.Endpoint); len(endpoint) != 0 { - resolved.URL = endpoints.AddScheme(endpoint, aws.BoolValue(s.Config.DisableSSL)) - resolved.SigningRegion = region - } else { - resolved, err = s.Config.EndpointResolver.EndpointFor( - serviceName, region, - func(opt *endpoints.Options) { - opt.DisableSSL = aws.BoolValue(s.Config.DisableSSL) - opt.UseDualStack = aws.BoolValue(s.Config.UseDualStack) - // Support for STSRegionalEndpoint where the STSRegionalEndpoint is - // provided in envconfig or sharedconfig with envconfig getting precedence. - opt.STSRegionalEndpoint = s.Config.STSRegionalEndpoint - - // Support the condition where the service is modeled but its - // endpoint metadata is not available. - opt.ResolveUnknownService = true - }, - ) + resolved, err := s.resolveEndpoint(service, region, s.Config) + if err != nil && s.Config.Logger != nil { + s.Config.Logger.Log(fmt.Sprintf( + "ERROR: unable to resolve endpoint for service %q, region %q, err: %v", + service, region, err)) } return client.Config{ @@ -657,7 +632,37 @@ func (s *Session) clientConfigWithErr(serviceName string, cfgs ...*aws.Config) ( SigningRegion: resolved.SigningRegion, SigningNameDerived: resolved.SigningNameDerived, SigningName: resolved.SigningName, - }, err + } +} + +func (s *Session) resolveEndpoint(service, region string, cfg *aws.Config) (endpoints.ResolvedEndpoint, error) { + + if ep := aws.StringValue(cfg.Endpoint); len(ep) != 0 { + return endpoints.ResolvedEndpoint{ + URL: endpoints.AddScheme(ep, aws.BoolValue(cfg.DisableSSL)), + SigningRegion: region, + }, nil + } + + resolved, err := cfg.EndpointResolver.EndpointFor(service, region, + func(opt *endpoints.Options) { + opt.DisableSSL = aws.BoolValue(cfg.DisableSSL) + opt.UseDualStack = aws.BoolValue(cfg.UseDualStack) + // Support for STSRegionalEndpoint where the STSRegionalEndpoint is + // provided in envConfig or sharedConfig with envConfig getting + // precedence. + opt.STSRegionalEndpoint = cfg.STSRegionalEndpoint + + // Support the condition where the service is modeled but its + // endpoint metadata is not available. + opt.ResolveUnknownService = true + }, + ) + if err != nil { + return endpoints.ResolvedEndpoint{}, err + } + + return resolved, nil } // ClientConfigNoResolveEndpoint is the same as ClientConfig with the exception @@ -667,12 +672,9 @@ func (s *Session) ClientConfigNoResolveEndpoint(cfgs ...*aws.Config) client.Conf s = s.Copy(cfgs...) var resolved endpoints.ResolvedEndpoint - - region := aws.StringValue(s.Config.Region) - if ep := aws.StringValue(s.Config.Endpoint); len(ep) > 0 { resolved.URL = endpoints.AddScheme(ep, aws.BoolValue(s.Config.DisableSSL)) - resolved.SigningRegion = region + resolved.SigningRegion = aws.StringValue(s.Config.Region) } return client.Config{ diff --git a/aws/session/session_test.go b/aws/session/session_test.go index 5d7a28f08b..3cf7c72efc 100644 --- a/aws/session/session_test.go +++ b/aws/session/session_test.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/defaults" "github.com/aws/aws-sdk-go/aws/endpoints" + "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/s3" ) @@ -149,6 +150,39 @@ func TestSessionClientConfig(t *testing.T) { } } +func TestNewSession_ResolveEndpointError(t *testing.T) { + logger := mockLogger{Buffer: bytes.NewBuffer(nil)} + sess, err := NewSession(defaults.Config(), &aws.Config{ + Region: aws.String(""), + Logger: logger, + EndpointResolver: endpoints.ResolverFunc( + func(service, region string, opts ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { + return endpoints.ResolvedEndpoint{}, fmt.Errorf("mock error") + }, + ), + }) + if err != nil { + t.Fatalf("expect no error got %v", err) + } + + cfg := sess.ClientConfig("mock service") + + var r request.Request + cfg.Handlers.Validate.Run(&r) + + if r.Error == nil { + t.Fatalf("expect validation error, got none") + } + + if e, a := aws.ErrMissingRegion.Error(), r.Error.Error(); !strings.Contains(a, e) { + t.Errorf("expect %v validation error, got %v", e, a) + } + + if e, a := "unable to resolve", logger.Buffer.String(); !strings.Contains(a, e) { + t.Errorf("expect %v logged, got %v", e, a) + } +} + func TestNewSession_NoCredentials(t *testing.T) { restoreEnvFn := initSessionTestEnv() defer restoreEnvFn()