aws: Endpoints Usage Refactor for aws.Request and aws.Metadata (#473)

Author: skmcgrail

CHANGELOG_PENDING.md

@@ -8,12 +8,17 @@ Breaking Change
 * `private/model`: Add utility for validating shape names for structs and enums for the service packages ([#471](https://github.com/aws/aws-sdk-go-v2/pull/471))
   * Fixes bug which allowed service package structs, enums to start with non alphabetic character 
   * Fixes the incorrect enum types in mediapackage service package, changing enum types __AdTriggersElement, __PeriodTriggersElement to AdTriggersElement, PeriodTriggersElement respectively.
+* `aws`: Client, Metadata, and Request structures have been refactored to simplify the usage of resolved endpoints ([#473](https://github.com/aws/aws-sdk-go-v2/pull/473))
+  * `aws.Client.Endpoint` struct member has been removed, and `aws.Request.Endpoint` struct member has been added of type `aws.Endpoint`
+  * `aws.Client.Region` structure member has been removed
 
 Services
 ---
 
 SDK Features
 ---
+* `aws`: `PartitionID` has been added to `aws.Endpoint` structure, and is used by the endpoint resolver to indicate which AWS partition an endpoint was resolved for ([#473](https://github.com/aws/aws-sdk-go-v2/pull/473))
+* `aws/endpoints`: Updated resolvers to populate `PartitionID` for a resolved `aws.Endpoint` ([#473](https://github.com/aws/aws-sdk-go-v2/pull/473))
 
 SDK Enhancements
 ---

aws/client.go

@@ -11,7 +11,6 @@ type Metadata struct {
 	EndpointsID string
 	APIVersion  string
 
-	Endpoint      string
 	SigningName   string
 	SigningRegion string
 
@@ -24,7 +23,6 @@ type Metadata struct {
 type Client struct {
 	Metadata         Metadata
 	Config           Config
-	Region           string
 	Credentials      CredentialsProvider
 	EndpointResolver EndpointResolver
 	Handlers         Handlers
@@ -42,7 +40,6 @@ func NewClient(cfg Config, metadata Metadata) *Client {
 		// TODO remove config when request refactored
 		Config: cfg,
 
-		Region:           cfg.Region,
 		Credentials:      cfg.Credentials,
 		EndpointResolver: cfg.EndpointResolver,
 		Handlers:         cfg.Handlers.Copy(),

aws/defaults/handlers.go

@@ -216,9 +216,9 @@ var AfterRetryHandler = aws.NamedHandler{
 // appropriate Region and Endpoint set. Will set r.Error if the endpoint or
 // region is not valid.
 var ValidateEndpointHandler = aws.NamedHandler{Name: "core.ValidateEndpointHandler", Fn: func(r *aws.Request) {
-	if r.Metadata.SigningRegion == "" && r.Config.Region == "" {
+	if r.Endpoint.SigningRegion == "" && r.Config.Region == "" {
 		r.Error = aws.ErrMissingRegion
-	} else if r.Metadata.Endpoint == "" {
+	} else if len(r.Endpoint.URL) == 0 {
 		r.Error = aws.ErrMissingEndpoint
 	}
 }}

aws/ec2metadata/api_client.go

@@ -227,7 +227,7 @@ func unmarshalError(r *aws.Request) {
 }
 
 func validateEndpointHandler(r *aws.Request) {
-	if r.Metadata.Endpoint == "" {
+	if len(r.Endpoint.URL) == 0 {
 		r.Error = aws.ErrMissingEndpoint
 	}
 }

aws/endpointcreds/provider.go

@@ -134,7 +134,7 @@ func (p *Provider) getCredentials(ctx context.Context) (*getCredentialsOutput, e
 }
 
 func validateEndpointHandler(r *aws.Request) {
-	if len(r.Metadata.Endpoint) == 0 {
+	if len(r.Endpoint.URL) == 0 {
 		r.Error = aws.ErrMissingEndpoint
 	}
 }

aws/endpoints.go

@@ -35,6 +35,9 @@ type Endpoint struct {
 	// The URL of the endpoint.
 	URL string
 
+	// The endpoint partition
+	PartitionID string
+
 	// The service name that should be used for signing the requests to the
 	// endpoint.
 	SigningName string

aws/endpoints/v3model.go

@@ -101,7 +101,7 @@ func (p partition) EndpointFor(service, region string, opts ResolveOptions) (res
 	}
 
 	defs := []endpoint{p.Defaults, s.Defaults}
-	return e.resolve(service, region, p.DNSSuffix, defs, opts), nil
+	return e.resolve(service, p.ID, region, p.DNSSuffix, defs, opts), nil
 }
 
 func serviceList(ss services) []string {
@@ -213,7 +213,7 @@ func getByPriority(s []string, p []string, def string) string {
 	return s[0]
 }
 
-func (e endpoint) resolve(service, region, dnsSuffix string, defs []endpoint, opts ResolveOptions) aws.Endpoint {
+func (e endpoint) resolve(service, parittionID, region, dnsSuffix string, defs []endpoint, opts ResolveOptions) aws.Endpoint {
 	var merged endpoint
 	for _, def := range defs {
 		merged.mergeIn(def)
@@ -253,6 +253,7 @@ func (e endpoint) resolve(service, region, dnsSuffix string, defs []endpoint, op
 
 	return aws.Endpoint{
 		URL:                u,
+		PartitionID:        parittionID,
 		SigningRegion:      signingRegion,
 		SigningName:        signingName,
 		SigningNameDerived: signingNameDerived,

aws/endpoints/v3model_test.go

@@ -207,13 +207,14 @@ func TestEndpointResolve(t *testing.T) {
 		SSLCommonName:     "new sslCommonName",
 	}
 
-	resolved := e.resolve("service", "region", "dnsSuffix",
-		defs, ResolveOptions{},
-	)
+	resolved := e.resolve("service", "partition", "region", "dnsSuffix", defs, ResolveOptions{})
 
 	if e, a := "https://service.region.dnsSuffix", resolved.URL; e != a {
 		t.Errorf("expect %v, got %v", e, a)
 	}
+	if e, a := "partition", resolved.PartitionID; e != a {
+		t.Errorf("expect %v, got %v", e, a)
+	}
 	if e, a := "signing_service", resolved.SigningName; e != a {
 		t.Errorf("expect %v, got %v", e, a)
 	}

aws/request.go

@@ -36,6 +36,7 @@ const (
 type Request struct {
 	Config           Config
 	Metadata         Metadata
+	Endpoint         Endpoint
 	Handlers         Handlers
 	Retryer          Retryer
 	AttemptTime      time.Time
@@ -111,12 +112,12 @@ func New(cfg Config, metadata Metadata, handlers Handlers,
 
 	if err == nil {
 		// TODO so ugly
-		metadata.Endpoint = endpoint.URL
-		if len(endpoint.SigningName) > 0 && !endpoint.SigningNameDerived {
-			metadata.SigningName = endpoint.SigningName
+		if len(endpoint.SigningRegion) == 0 {
+			endpoint.SigningRegion = metadata.SigningRegion
 		}
-		if len(endpoint.SigningRegion) > 0 {
-			metadata.SigningRegion = endpoint.SigningRegion
+
+		if len(endpoint.SigningName) == 0 || (len(endpoint.SigningName) > 0 && endpoint.SigningNameDerived) {
+			endpoint.SigningName = metadata.SigningName
 		}
 
 		httpReq.URL, err = url.Parse(endpoint.URL + operation.HTTPPath)
@@ -127,10 +128,10 @@ func New(cfg Config, metadata Metadata, handlers Handlers,
 	}
 
 	r := &Request{
-		Config:   cfg,
-		Metadata: metadata,
-		Handlers: handlers.Copy(),
-
+		Config:      cfg,
+		Metadata:    metadata,
+		Endpoint:    endpoint,
+		Handlers:    handlers.Copy(),
 		Retryer:     retryer,
 		Time:        time.Now(),
 		ExpireTime:  0,

aws/request_test.go

@@ -754,7 +754,6 @@ func TestSerializationErrConnectionReset(t *testing.T) {
 		ServiceName:   "fooService",
 		SigningName:   "foo",
 		SigningRegion: "foo",
-		Endpoint:      "localhost",
 		APIVersion:    "2001-01-01",
 		JSONVersion:   "1.1",
 		TargetPrefix:  "Foo",
@@ -1151,3 +1150,97 @@ func (f *stubSeekFail) ReadAt(b []byte, offset int64) (int, error) {
 func (f *stubSeekFail) Seek(offset int64, mode int) (int64, error) {
 	return 0, f.Err
 }
+
+func TestRequestEndpointConstruction(t *testing.T) {
+	cases := map[string]struct {
+		EndpointResolver aws.EndpointResolverFunc
+		ExpectedEndpoint aws.Endpoint
+	}{
+		"resolved modeled endpoint": {
+			EndpointResolver: func(_, _ string) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					URL:           "https://localhost",
+					SigningName:   "foo-service",
+					SigningRegion: "bar-region",
+				}, nil
+			},
+			ExpectedEndpoint: aws.Endpoint{
+				URL:           "https://localhost",
+				SigningName:   "foo-service",
+				SigningRegion: "bar-region",
+			},
+		},
+		"resolved endpoint missing signing region": {
+			EndpointResolver: func(_, _ string) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					URL:         "https://localhost",
+					SigningName: "foo-service",
+				}, nil
+			},
+			ExpectedEndpoint: aws.Endpoint{
+				URL:           "https://localhost",
+				SigningName:   "foo-service",
+				SigningRegion: "meta-bar-region",
+			},
+		},
+		"resolved endpoint missing signing name": {
+			EndpointResolver: func(_, _ string) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					URL:           "https://localhost",
+					SigningRegion: "bar-region",
+				}, nil
+			},
+			ExpectedEndpoint: aws.Endpoint{
+				URL:           "https://localhost",
+				SigningName:   "meta-foo-service",
+				SigningRegion: "bar-region",
+			},
+		},
+		"resolved endpoint signing name derived": {
+			EndpointResolver: func(_, _ string) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					URL:                "https://localhost",
+					SigningRegion:      "bar-region",
+					SigningName:        "derived-signing-name",
+					SigningNameDerived: true,
+				}, nil
+			},
+			ExpectedEndpoint: aws.Endpoint{
+				URL:                "https://localhost",
+				SigningName:        "meta-foo-service",
+				SigningRegion:      "bar-region",
+				SigningNameDerived: true,
+			},
+		},
+		"resolved endpoint missing signing region and signing name": {
+			EndpointResolver: func(_, _ string) (aws.Endpoint, error) {
+				return aws.Endpoint{
+					URL: "https://localhost",
+				}, nil
+			},
+			ExpectedEndpoint: aws.Endpoint{
+				URL:           "https://localhost",
+				SigningName:   "meta-foo-service",
+				SigningRegion: "meta-bar-region",
+			},
+		},
+	}
+
+	meta := aws.Metadata{
+		ServiceName:   "FooService",
+		SigningName:   "meta-foo-service",
+		SigningRegion: "meta-bar-region",
+	}
+
+	for name, tt := range cases {
+		t.Run(name, func(t *testing.T) {
+			client := aws.NewClient(aws.Config{EndpointResolver: tt.EndpointResolver}, meta)
+
+			request := client.NewRequest(&aws.Operation{Name: "ZapOperation", HTTPMethod: "PUT", HTTPPath: "/"}, nil, nil)
+
+			if e, a := tt.ExpectedEndpoint, request.Endpoint; !reflect.DeepEqual(e, a) {
+				t.Errorf("expected %v, got %v", e, a)
+			}
+		})
+	}
+}