Skip to content

Commit

Permalink
codegen: Add flag to direct if SDK is allowed to modify a custom endp…
Browse files Browse the repository at this point in the history
…oint or not

Adds a new member to the `aws.Endpoint` struct, `HostnameImmutable`. This member
directs if the SDK is allowed to modify the resolved endpoint to meet the
requirements of the API.  If `HostnameImmutable` is true, the SDK will not
attempt to modify the hostname via any customizations, or prefix behavior. Flag
 defaults to false.

- Fixes #827 (SDK requiring region for endpoint resolution)
- Fixes #328
- Fixes #364
- Related to #836 When Amazon S3 Accesspoint support is implemented.
- Related to #479

Depends on aws/smithy-go#222
  • Loading branch information
jasdel committed Oct 27, 2020
1 parent 1465859 commit 2592e95
Show file tree
Hide file tree
Showing 14 changed files with 412 additions and 188 deletions.
58 changes: 47 additions & 11 deletions aws/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,44 @@ import (
"fmt"
)

// Endpoint represents the endpoint a service client should make requests to.
// Endpoint represents the endpoint a service client should make API operation
// calls to.
//
// The SDK will automatically resolve these endpoints per API client using an
// internal endpoint resolvers. If you'd like to provide custom endpoint
// resolving behavior you can implement the EndpointResolver interface.
type Endpoint struct {
// The URL of the endpoint.
// The base URL endpoint the SDK API clients will use to make API calls to.
// The SDK will suffix URI path and query elements to this endpoint.
URL string

// The endpoint partition
// Specifies if the endpoint's hostname can be modified by the SDK's API
// client.
//
// If the hostname is mutable the SDK API clients may modify any part of
// the hostname based on the requirements of the API, (e.g. adding, or
// removing content in the hostname). Such as, Amazon S3 API client
// prefixing "bucketname" to the hostname, or changing the
// hostname service name component from "s3." to "s3-accesspoint.dualstack."
// for the dualstack endpoint of an S3 Accesspoint resource.
//
// Care should be taken when providing a custom endpoint for an API. If the
// endpoint hostname is mutable, and the client cannot modify the endpoint
// correctly, the operation call will most likely fail, or have undefined
// behavior.
//
// If hostname is immutable, the SDK API clients will not modify the
// hostname of the URL. This may cause the API client not to function
// correctly if the API requires the operation specific hostname values
// to be used by the client.
//
// This flag does not modify the API client's behavior if this endpoint
// will be used instead of Endpoint Discovery, or if the endpoint will be
// used to perform Endpoint Discovery. That behavior is configured via the
// API Client's Options.
HostnameImmutable bool

// The AWS partition the endpoint belongs to.
PartitionID string

// The service name that should be used for signing the requests to the
Expand All @@ -24,9 +56,11 @@ type Endpoint struct {
SigningMethod string
}

// EndpointNotFoundError is a sentinel error to indicate that the EndpointResolver implementation was unable
// to resolve an endpoint for the given service and region. Resolvers should use this to indicate that
// a client should fallback and attempt to use it's default resolver to resolve the endpoint.
// EndpointNotFoundError is a sentinel error to indicate that the
// EndpointResolver implementation was unable to resolve an endpoint for the
// given service and region. Resolvers should use this to indicate that an API
// client should fallback and attempt to use it's internal default resolver to
// resolve the endpoint.
type EndpointNotFoundError struct {
Err error
}
Expand All @@ -41,18 +75,20 @@ func (e *EndpointNotFoundError) Unwrap() error {
return e.Err
}

// EndpointResolver is an endpoint resolver that can be used to provide or override an endpoint for the given
// service and region. Clients will attempt to use the EndpointResolver first to resolve an endpoint if available.
// If the EndpointResolver returns an EndpointNotFoundError error, clients will fallback to attempting to resolve the endpoint
// using their default endpoint resolver.
// EndpointResolver is an endpoint resolver that can be used to provide or
// override an endpoint for the given service and region. API clients will
// attempt to use the EndpointResolver first to resolve an endpoint if
// available. If the EndpointResolver returns an EndpointNotFoundError error,
// API clients will fallback to attempting to resolve the endpoint using its
// internal default endpoint resolver.
type EndpointResolver interface {
ResolveEndpoint(service, region string) (Endpoint, error)
}

// EndpointResolverFunc wraps a function to satisfy the EndpointResolver interface.
type EndpointResolverFunc func(service, region string) (Endpoint, error)

// ResolveEndpoint calls the wrapped function and returns the results
// ResolveEndpoint calls the wrapped function and returns the results.
func (e EndpointResolverFunc) ResolveEndpoint(service, region string) (Endpoint, error) {
return e(service, region)
}
12 changes: 3 additions & 9 deletions aws/errors.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
package aws

// MissingRegionError is an error that is returned if region configuration is not found.
// MissingRegionError is an error that is returned if region configuration
// value was not found.
type MissingRegionError struct{}

func (*MissingRegionError) Error() string {
return "could not find region configuration"
}

// MissingEndpointError is an error that is returned if an endpoint cannot be resolved for a service.
type MissingEndpointError struct{}

func (*MissingEndpointError) Error() string {
return "'Endpoint' configuration is required for this service"
return "an AWS region is required, but was not found"
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ final class EndpointGenerator implements Runnable {
public static final String ADD_MIDDLEWARE_HELPER_NAME = String.format("add%sMiddleware", MIDDLEWARE_NAME);
public static final String RESOLVER_INTERFACE_NAME = "EndpointResolver";
public static final String RESOLVER_FUNC_NAME = "EndpointResolverFunc";
public static final String RESOLVER_OPTIONS = "ResolverOptions";
public static final String RESOLVER_OPTIONS = "EndpointResolverOptions";
public static final String CLIENT_CONFIG_RESOLVER = "resolveDefaultEndpointConfiguration";
public static final String RESOLVER_CONSTRUCTOR_NAME = "NewDefaultEndpointResolver";
public static final String AWS_ENDPOINT_RESOLVER_HELPER = "WithEndpointResolver";
Expand Down Expand Up @@ -217,38 +217,47 @@ private void generateMiddlewareResolverBody(GoStackStepMiddlewareGenerator g, Go
w.addUseImports(SmithyGoDependency.FMT);
w.addUseImports(SmithyGoDependency.NET_URL);
w.addUseImports(AwsGoDependency.AWS_MIDDLEWARE);
w.addUseImports(SmithyGoDependency.SMITHY_MIDDLEWARE);
w.addUseImports(SmithyGoDependency.SMITHY_HTTP_TRANSPORT);

w.write("req, ok := in.Request.(*smithyhttp.Request)");
w.openBlock("if !ok {", "}", () -> {
w.write("return out, metadata, fmt.Errorf(\"unknown transport type %T\", in.Request)");
});
w.write("");

w.openBlock("if m.Resolver == nil {", "}", () -> {
w.write("return out, metadata, fmt.Errorf(\"expected endpoint resolver to not be nil\")");
});
w.write("");

w.write("var endpoint $T", SymbolUtils.createValueSymbolBuilder("Endpoint", AwsGoDependency.AWS_CORE)
.build());
w.write("endpoint, err = m.Resolver.ResolveEndpoint(awsmiddleware.GetRegion(ctx), m.Options)");
w.openBlock("if err != nil {", "}", () -> {
w.write("return out, metadata, fmt.Errorf(\"failed to resolve service endpoint\")");
w.write("return out, metadata, fmt.Errorf(\"failed to resolve service endpoint, %w\", err)");
});
w.write("");

w.write("req.URL, err = url.Parse(endpoint.URL)");
w.openBlock("if err != nil {", "}", () -> {
w.write("return out, metadata, fmt.Errorf(\"failed to parse endpoint URL: %w\", err)");
});
w.write("");

w.openBlock("if len(awsmiddleware.GetSigningName(ctx)) == 0 {", "}", () -> {
w.write("signingName := endpoint.SigningName");
w.openBlock("if len(signingName) == 0 {", "}", () -> {
w.write("signingName = $S", serviceShape.expectTrait(ServiceTrait.class).getArnNamespace());
});
w.write("ctx = awsmiddleware.SetSigningName(ctx, signingName)");
});
w.write("");

w.write("ctx = awsmiddleware.SetSigningRegion(ctx, endpoint.SigningRegion)");
w.write("ctx = smithyhttp.SetHostnameImmutable(ctx, endpoint.HostnameImmutable)");
w.write("");

w.write("return next.HandleSerialize(ctx, in)");
}

Expand Down Expand Up @@ -387,6 +396,12 @@ private void generateInternalResolverImplementation(GoWriter writer) {
writer.write("");
writer.writeDocs("ResolveEndpoint resolves the service endpoint for the given region and options");
writeInternalResolveEndpointImplementation(writer, resolverImplSymbol, "r", () -> {
// Currently all APIs require a region to derive the endpoint for that API. If there are ever a truly
// region-less API then this should be gated at codegen.
writer.addUseImports(AwsGoDependency.AWS_CORE);
writer.write("if len(region) == 0 { return endpoint, &aws.MissingRegionError{} }");
writer.write("");

Symbol sharedOptions = SymbolUtils.createPointableSymbolBuilder("Options",
AwsGoDependency.AWS_ENDPOINTS).build();
writer.openBlock("opt := $T{", "}", sharedOptions, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestClient${operation}_presignURLCustomization(t *testing.T) {
return smithyhttp.NopClient{}.Do(r)
}),
EndpointResolver: EndpointResolverFunc(
func(region string, options ResolverOptions) (aws.Endpoint, error) {
func(region string, options EndpointResolverOptions) (aws.Endpoint, error) {
return aws.Endpoint{
URL: "https://service." + region + ".amazonaws.com",
SigningRegion: c.ClientRegion,
Expand Down
4 changes: 2 additions & 2 deletions feature/s3/manager/bucket_region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestGetBucketRegion_Exists(t *testing.T) {
server := testSetupGetBucketRegionServer(c.RespRegion, c.StatusCode, true)

client := s3.New(s3.Options{
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.ResolverOptions) (aws.Endpoint, error) {
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error) {
return aws.Endpoint{
URL: server.URL,
}, nil
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestGetBucketRegion_NotExists(t *testing.T) {
defer server.Close()

client := s3.New(s3.Options{
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.ResolverOptions) (aws.Endpoint, error) {
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error) {
return aws.Endpoint{
URL: server.URL,
}, nil
Expand Down
4 changes: 3 additions & 1 deletion feature/s3/manager/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,9 @@ func TestDownloadPartBodyRetry_FailRetry(t *testing.T) {
}

func TestDownloadWithContextCanceled(t *testing.T) {
d := manager.NewDownloader(s3.New(s3.Options{}))
d := manager.NewDownloader(s3.New(s3.Options{
Region: "mock-region",
}))

params := s3.GetObjectInput{
Bucket: aws.String("bucket"),
Expand Down
4 changes: 2 additions & 2 deletions feature/s3/manager/internal/testing/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (
)

// EndpointResolverFunc is a mock s3 endpoint resolver that wraps the given function
type EndpointResolverFunc func(region string, options s3.ResolverOptions) (aws.Endpoint, error)
type EndpointResolverFunc func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error)

// ResolveEndpoint returns the results from the wrapped function.
func (m EndpointResolverFunc) ResolveEndpoint(region string, options s3.ResolverOptions) (aws.Endpoint, error) {
func (m EndpointResolverFunc) ResolveEndpoint(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error) {
return m(region, options)
}
3 changes: 2 additions & 1 deletion feature/s3/manager/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ func TestSSE(t *testing.T) {
func TestUploadWithContextCanceled(t *testing.T) {
u := manager.NewUploader(s3.New(s3.Options{
UsePathStyle: true,
Region: "mock-region",
}))

params := s3.PutObjectInput{
Expand Down Expand Up @@ -878,7 +879,7 @@ func TestUploadRetry(t *testing.T) {
defer server.Close()

client := s3.New(s3.Options{
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.ResolverOptions) (aws.Endpoint, error) {
EndpointResolver: s3testing.EndpointResolverFunc(func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error) {
return aws.Endpoint{
URL: server.URL,
}, nil
Expand Down
8 changes: 6 additions & 2 deletions service/internal/s3shared/update_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"strings"

"github.com/awslabs/smithy-go/middleware"
"github.com/awslabs/smithy-go/transport/http"
smithyhttp "github.com/awslabs/smithy-go/transport/http"
)

// EnableDualstackMiddleware represents middleware struct for enabling dualstack support
Expand All @@ -28,7 +28,11 @@ func (u *EnableDualstackMiddleware) HandleSerialize(
) (
out middleware.SerializeOutput, metadata middleware.Metadata, err error,
) {
req, ok := in.Request.(*http.Request)
if smithyhttp.GetHostnameImmutable(ctx) {
return next.HandleSerialize(ctx, in)
}

req, ok := in.Request.(*smithyhttp.Request)
if !ok {
return out, metadata, fmt.Errorf("unknown request type %T", req)
}
Expand Down
6 changes: 6 additions & 0 deletions service/s3/internal/customizations/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ Since serializers serialize by default as path style url, we use customization
to modify the endpoint url when `UsePathStyle` option on S3Client is unset or
false. This flag will be ignored if `UseAccelerate` option is set to true.
If UseAccelerate is not enabled, and the bucket name is not a valid hostname
label, they SDK will fallback to forcing the request to be made as if
UsePathStyle was enabled. This behavior is also used if UseDualStack is enabled.
https://docs.aws.amazon.com/AmazonS3/latest/dev/dual-stack-endpoints.html#dual-stack-endpoints-description
Transfer acceleration
Expand Down
6 changes: 3 additions & 3 deletions service/s3/internal/customizations/handle_200_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import (
"github.com/aws/aws-sdk-go-v2/service/s3/types"
)

type EndpointResolverFunc func(region string, options s3.ResolverOptions) (aws.Endpoint, error)
type EndpointResolverFunc func(region string, options s3.EndpointResolverOptions) (aws.Endpoint, error)

func (fn EndpointResolverFunc) ResolveEndpoint(region string, options s3.ResolverOptions) (endpoint aws.Endpoint, err error) {
func (fn EndpointResolverFunc) ResolveEndpoint(region string, options s3.EndpointResolverOptions) (endpoint aws.Endpoint, err error) {
return fn(region, options)
}

Expand Down Expand Up @@ -73,7 +73,7 @@ func TestErrorResponseWith200StatusCode(t *testing.T) {
Credentials: unit.StubCredentialsProvider{},
Retryer: aws.NoOpRetryer{},
Region: "mock-region",
EndpointResolver: EndpointResolverFunc(func(region string, options s3.ResolverOptions) (e aws.Endpoint, err error) {
EndpointResolver: EndpointResolverFunc(func(region string, options s3.EndpointResolverOptions) (e aws.Endpoint, err error) {
e.URL = server.URL
e.SigningRegion = "us-west-2"
return e, err
Expand Down
22 changes: 17 additions & 5 deletions service/s3/internal/customizations/update_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"strings"

"github.com/awslabs/smithy-go/middleware"
"github.com/awslabs/smithy-go/transport/http"
smithyhttp "github.com/awslabs/smithy-go/transport/http"

"github.com/aws/aws-sdk-go-v2/service/internal/s3shared"
)
Expand Down Expand Up @@ -78,7 +78,11 @@ func (u *updateEndpointMiddleware) HandleSerialize(
) (
out middleware.SerializeOutput, metadata middleware.Metadata, err error,
) {
req, ok := in.Request.(*http.Request)
if smithyhttp.GetHostnameImmutable(ctx) {
return next.HandleSerialize(ctx, in)
}

req, ok := in.Request.(*smithyhttp.Request)
if !ok {
return out, metadata, fmt.Errorf("unknown request type %T", req)
}
Expand Down Expand Up @@ -109,15 +113,23 @@ func (u *updateEndpointMiddleware) HandleSerialize(
return next.HandleSerialize(ctx, in)
}

func (u updateEndpointMiddleware) updateEndpointFromConfig(req *http.Request, bucket string) error {
func (u updateEndpointMiddleware) updateEndpointFromConfig(req *smithyhttp.Request, bucket string) error {
// do nothing if path style is enforced
if u.usePathStyle {
return nil
}

if !hostCompatibleBucketName(req.URL, bucket) {
// bucket name must be valid to put into the host
return fmt.Errorf("bucket name %s is not compatible with S3", bucket)
// bucket name must be valid to put into the host for accelerate operations.
// For non-accelerate operations the bucket name can stay in the path if
// not valid hostname.
var err error
if u.useAccelerate {
err = fmt.Errorf("bucket name %s is not compatible with S3", bucket)
}

// No-Op if not using accelerate.
return err
}

// accelerate is only supported if use path style is disabled
Expand Down
Loading

0 comments on commit 2592e95

Please sign in to comment.