Skip to content

Commit

Permalink
service/s3/internal/customization: Remove dual-stack endpoint customi…
Browse files Browse the repository at this point in the history 
…zation
  • Loading branch information
@skmcgrail
skmcgrail committed May 25, 2021
1 parent 07f550d commit 361b6eb
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 42 deletions.
10 changes: 9 additions & 1 deletion service/s3/internal/customizations/process_arn_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"github.com/aws/aws-sdk-go-v2/aws"
internalendpoints "github.com/aws/aws-sdk-go-v2/service/s3/internal/endpoints"
"net/url"
"strings"

Expand Down Expand Up @@ -250,8 +251,15 @@ func buildAccessPointRequest(ctx context.Context, options accesspointOptions) (c

resolveService := tv.Service

eo := options.EndpointResolverOptions
if options.UseDualstack {
eo.UseDualStack = internalendpoints.EnableDualStackEndpoint
} else {
eo.UseDualStack = internalendpoints.DisableDualStackEndpoint
}

// resolve endpoint
endpoint, err := options.EndpointResolver.ResolveEndpoint(resolveRegion, options.EndpointResolverOptions)
endpoint, err := options.EndpointResolver.ResolveEndpoint(resolveRegion, eo)
if err != nil {
return ctx, s3shared.NewFailedToResolveEndpointError(
tv,
Expand Down
39 changes: 19 additions & 20 deletions service/s3/internal/customizations/update_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@ package customizations
import (
"context"
"fmt"
"github.com/aws/smithy-go/encoding/httpbinding"
"log"
"net/url"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/smithy-go/middleware"
smithyhttp "github.com/aws/smithy-go/transport/http"

awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
"github.com/aws/aws-sdk-go-v2/service/internal/s3shared"

internalendpoints "github.com/aws/aws-sdk-go-v2/service/s3/internal/endpoints"
"github.com/aws/smithy-go/encoding/httpbinding"
"github.com/aws/smithy-go/middleware"
smithyhttp "github.com/aws/smithy-go/transport/http"
)

// EndpointResolver interface for resolving service endpoints.
Expand Down Expand Up @@ -67,8 +65,18 @@ type UpdateEndpointOptions struct {
EndpointResolverOptions EndpointResolverOptions
}

// IsUseDualStack returns whether dual-stack endpoint resolution is enabled
func (o UpdateEndpointOptions) IsUseDualStack() bool {
if o.EndpointResolverOptions.UseDualStack != internalendpoints.UnsetDualStackEndpoint {
return o.EndpointResolverOptions.UseDualStack == internalendpoints.EnableDualStackEndpoint
}
return o.UseDualstack
}

// UpdateEndpoint adds the middleware to the middleware stack based on the UpdateEndpointOptions.
func UpdateEndpoint(stack *middleware.Stack, options UpdateEndpointOptions) (err error) {
const serializerID = "OperationSerializer"

// initial arn look up middleware
err = stack.Initialize.Add(&s3shared.ARNLookup{
GetARNValue: options.Accessor.GetBucketFromInput,
Expand All @@ -81,10 +89,10 @@ func UpdateEndpoint(stack *middleware.Stack, options UpdateEndpointOptions) (err
err = stack.Serialize.Insert(&processARNResource{
UseARNRegion: options.UseARNRegion,
UseAccelerate: options.UseAccelerate,
UseDualstack: options.UseDualstack,
UseDualstack: options.IsUseDualStack(),
EndpointResolver: options.EndpointResolver,
EndpointResolverOptions: options.EndpointResolverOptions,
}, "OperationSerializer", middleware.Before)
}, serializerID, middleware.Before)
if err != nil {
return err
}
Expand All @@ -95,25 +103,16 @@ func UpdateEndpoint(stack *middleware.Stack, options UpdateEndpointOptions) (err
err = stack.Serialize.Insert(&s3ObjectLambdaEndpoint{
UseEndpoint: options.TargetS3ObjectLambda,
UseAccelerate: options.UseAccelerate,
UseDualstack: options.UseDualstack,
UseDualstack: options.IsUseDualStack(),
EndpointResolver: options.EndpointResolver,
EndpointResolverOptions: options.EndpointResolverOptions,
}, "OperationSerializer", middleware.Before)
}, serializerID, middleware.Before)
if err != nil {
return err
}

// remove bucket arn middleware
err = stack.Serialize.Insert(&removeBucketFromPathMiddleware{}, "OperationSerializer", middleware.After)
if err != nil {
return err
}

// enable dual stack support
err = stack.Serialize.Insert(&s3shared.EnableDualstack{
UseDualstack: options.UseDualstack,
DefaultServiceID: "s3",
}, "OperationSerializer", middleware.After)
err = stack.Serialize.Insert(&removeBucketFromPathMiddleware{}, serializerID, middleware.After)
if err != nil {
return err
}
Expand All @@ -124,7 +123,7 @@ func UpdateEndpoint(stack *middleware.Stack, options UpdateEndpointOptions) (err
getBucketFromInput: options.Accessor.GetBucketFromInput,
useAccelerate: options.UseAccelerate,
supportsAccelerate: options.SupportsAccelerate,
}, (*s3shared.EnableDualstack)(nil).ID(), middleware.After)
}, serializerID, middleware.After)
if err != nil {
return err
}
Expand Down
101 changes: 80 additions & 21 deletions service/s3/internal/customizations/update_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,27 +74,27 @@ func TestUpdateEndpointBuild(t *testing.T) {
"DualStack": {
useDualstack: true,
tests: []s3BucketTest{
{"abc", "key", "https://abc.s3.dualstack.mock-region.amazonaws.com/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a.b.c/key?x-id=GetObject", ""},
{"a$b$c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a%24b%24c/key?x-id=GetObject", ""},
{"abc", "key", "https://abc.s3.dualstack.mock-region.aws/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.aws/a.b.c/key?x-id=GetObject", ""},
{"a$b$c", "key", "https://s3.dualstack.mock-region.aws/a%24b%24c/key?x-id=GetObject", ""},
},
},
"DualStackWithPathStyle": {
useDualstack: true,
usePathStyle: true,
tests: []s3BucketTest{
{"abc", "key", "https://s3.dualstack.mock-region.amazonaws.com/abc/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a.b.c/key?x-id=GetObject", ""},
{"a$b$c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a%24b%24c/key?x-id=GetObject", ""},
{"abc", "key", "https://s3.dualstack.mock-region.aws/abc/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.aws/a.b.c/key?x-id=GetObject", ""},
{"a$b$c", "key", "https://s3.dualstack.mock-region.aws/a%24b%24c/key?x-id=GetObject", ""},
},
},
"AccelerateWithDualStack": {
useAccelerate: true,
useDualstack: true,
tests: []s3BucketTest{
{"abc", "key", "https://abc.s3-accelerate.dualstack.amazonaws.com/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a.b.c/key?x-id=GetObject", "not compatible"},
{"a$b$c", "key", "https://s3.dualstack.mock-region.amazonaws.com/a%24b%24c/key?x-id=GetObject", "not compatible"},
{"abc", "key", "https://abc.s3-accelerate.dualstack.aws/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://s3.dualstack.mock-region.aws/a.b.c/key?x-id=GetObject", "not compatible"},
{"a$b$c", "key", "https://s3.dualstack.mock-region.aws/a%24b%24c/key?x-id=GetObject", "not compatible"},
},
},
},
Expand Down Expand Up @@ -153,18 +153,6 @@ func TestUpdateEndpointBuild(t *testing.T) {
{"a$b$c", "key", "https://example.region.amazonaws.com/a%24b%24c/key?x-id=GetObject", ""},
},
},
"DualStack": {
useDualstack: true,
customEndpoint: &aws.Endpoint{
URL: "https://example.region.amazonaws.com",
HostnameImmutable: true,
},
tests: []s3BucketTest{
{"abc", "key", "https://example.region.amazonaws.com/abc/key?x-id=GetObject", ""},
{"a.b.c", "key", "https://example.region.amazonaws.com/a.b.c/key?x-id=GetObject", ""},
{"a$b$c", "key", "https://example.region.amazonaws.com/a%24b%24c/key?x-id=GetObject", ""},
},
},
},
}

Expand Down Expand Up @@ -814,6 +802,9 @@ func TestEndpointWithARN(t *testing.T) {
for name, c := range cases {
t.Run(name, func(t *testing.T) {
runValidations(t, c, func(ctx context.Context, svc *s3.Client, fm *requestRetrieverMiddleware) (interface{}, error) {
if c.operation != nil {
return c.operation(ctx, svc, fm)
}
return svc.GetObject(ctx, &s3.GetObjectInput{
Bucket: ptr.String(c.bucket),
Key: ptr.String("testkey"),
Expand Down Expand Up @@ -1067,6 +1058,74 @@ func TestWriteGetObjectResponse_UpdateEndpoint(t *testing.T) {
}
}

func TestUseDualStackClientBehavior(t *testing.T) {
cases := map[string]testCaseForEndpointCustomization{
"client options dual-stack false, endpoint resolver dual-stack unset": {
options: s3.Options{
Region: "us-west-2",
UseDualstack: false,
},
expectedReqURL: "https://test-bucket.s3.us-west-2.amazonaws.com/test-key?x-id=GetObject",
expectedSigningRegion: "us-west-2",
expectedSigningName: "s3",
},
"client options dual-stack true, endpoint resolver dual-stack unset": {
options: s3.Options{
Region: "us-west-2",
UseDualstack: true,
},
expectedReqURL: "https://test-bucket.s3.dualstack.us-west-2.amazonaws.com/test-key?x-id=GetObject",
expectedSigningRegion: "us-west-2",
expectedSigningName: "s3",
},
"client options dual-stack off, endpoint resolver dual-stack disabled": {
options: s3.Options{
Region: "us-west-2",
EndpointOptions: s3.EndpointResolverOptions{
UseDualStack: s3.DisableDualStackEndpoint,
},
},
expectedReqURL: "https://test-bucket.s3.us-west-2.amazonaws.com/test-key?x-id=GetObject",
expectedSigningRegion: "us-west-2",
expectedSigningName: "s3",
},
"client options dual-stack off, endpoint resolver dual-stack enabled": {
options: s3.Options{
Region: "us-west-2",
EndpointOptions: s3.EndpointResolverOptions{
UseDualStack: s3.EnableDualStackEndpoint,
},
},
expectedReqURL: "https://test-bucket.s3.dualstack.us-west-2.amazonaws.com/test-key?x-id=GetObject",
expectedSigningRegion: "us-west-2",
expectedSigningName: "s3",
},
"client options dual-stack on, endpoint resolver dual-stack disabled": {
options: s3.Options{
Region: "us-west-2",
UseDualstack: true,
EndpointOptions: s3.EndpointResolverOptions{
UseDualStack: s3.DisableDualStackEndpoint,
},
},
expectedReqURL: "https://test-bucket.s3.us-west-2.amazonaws.com/test-key?x-id=GetObject",
expectedSigningRegion: "us-west-2",
expectedSigningName: "s3",
},
}
for name, tt := range cases {
t.Run(name, func(t *testing.T) {
runValidations(t, tt, func(ctx context.Context, client *s3.Client, retrieverMiddleware *requestRetrieverMiddleware) (interface{}, error) {
return client.GetObject(context.Background(),
&s3.GetObjectInput{
Bucket: aws.String("test-bucket"),
Key: aws.String("test-key"),
}, addRequestRetriever(retrieverMiddleware))
})
})
}
}

func runValidations(t *testing.T, c testCaseForEndpointCustomization, operation func(
context.Context, *s3.Client, *requestRetrieverMiddleware) (interface{}, error)) {
// options
Expand Down

0 comments on commit 361b6eb

Please sign in to comment.