From 6724d37487f5715d992b157e1743d21974d51b7d Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Thu, 18 Mar 2021 10:21:59 -0700 Subject: [PATCH] service/s3: Add documentation for using unseekable body PutObject and UploadPart (#1176) Adds additional documentation to the PutObject and UploadPart operations on how to to use unseekable values to be uploaded to S3. Updates the v4 package with a new helper to swap out the compute payload hash middleware for the unsigned payload middleware. This allows API operations to be updated to not compute the payload hash, and use UNSIGNED-PAYLOAD instead. --- .../sdk-feature-1615946574682127000.json | 9 ++ ...service.s3-bugfix-1615945295096775000.json | 9 ++ aws/signer/v4/middleware.go | 10 ++ aws/signer/v4/middleware_test.go | 101 ++++++++++++++++++ .../S3AddPutObjectUnseekableBodyDoc.java | 88 +++++++++++++++ ...mithy.go.codegen.integration.GoIntegration | 1 + service/s3/api_op_PutObject.go | 3 + service/s3/api_op_UploadPart.go | 3 + 8 files changed, 224 insertions(+) create mode 100644 .changes/next-release/sdk-feature-1615946574682127000.json create mode 100644 .changes/next-release/service.s3-bugfix-1615945295096775000.json create mode 100644 codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3AddPutObjectUnseekableBodyDoc.java diff --git a/.changes/next-release/sdk-feature-1615946574682127000.json b/.changes/next-release/sdk-feature-1615946574682127000.json new file mode 100644 index 00000000000..1e0ae796a3a --- /dev/null +++ b/.changes/next-release/sdk-feature-1615946574682127000.json @@ -0,0 +1,9 @@ +{ + "ID": "sdk-feature-1615946574682127000", + "SchemaVersion": 1, + "Module": "/", + "Type": "feature", + "Description": "Add helper to V4 signer package to swap compute payload hash middleware with unsigned payload middleware", + "MinVersion": "", + "AffectedModules": null +} \ No newline at end of file diff --git a/.changes/next-release/service.s3-bugfix-1615945295096775000.json b/.changes/next-release/service.s3-bugfix-1615945295096775000.json new file mode 100644 index 00000000000..d37bac058e2 --- /dev/null +++ b/.changes/next-release/service.s3-bugfix-1615945295096775000.json @@ -0,0 +1,9 @@ +{ + "ID": "service.s3-bugfix-1615945295096775000", + "SchemaVersion": 1, + "Module": "service/s3", + "Type": "bugfix", + "Description": "Adds documentation to the PutObject and UploadPart operations Body member how to upload unseekable objects to an Amazon S3 Bucket.", + "MinVersion": "", + "AffectedModules": null +} \ No newline at end of file diff --git a/aws/signer/v4/middleware.go b/aws/signer/v4/middleware.go index ffa297668e1..b6e28b4bd46 100644 --- a/aws/signer/v4/middleware.go +++ b/aws/signer/v4/middleware.go @@ -156,6 +156,16 @@ func (m *computePayloadSHA256) HandleBuild( return next.HandleBuild(ctx, in) } +// SwapComputePayloadSHA256ForUnsignedPayloadMiddleware replaces the +// ComputePayloadSHA256 middleware with the UnsignedPayload middleware. +// +// Use this to disable computing the Payload SHA256 checksum and instead use +// UNSIGNED-PAYLOAD for the SHA256 value. +func SwapComputePayloadSHA256ForUnsignedPayloadMiddleware(stack *middleware.Stack) error { + _, err := stack.Build.Swap(computePayloadHashMiddlewareID, &unsignedPayload{}) + return err +} + // contentSHA256Header sets the X-Amz-Content-Sha256 header value to // the Payload hash stored in the context. type contentSHA256Header struct{} diff --git a/aws/signer/v4/middleware_test.go b/aws/signer/v4/middleware_test.go index a56058a86c0..acbc3d827b3 100644 --- a/aws/signer/v4/middleware_test.go +++ b/aws/signer/v4/middleware_test.go @@ -18,6 +18,7 @@ import ( "github.com/aws/smithy-go/logging" "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" + "github.com/google/go-cmp/cmp" ) func TestComputePayloadHashMiddleware(t *testing.T) { @@ -205,6 +206,106 @@ func TestSignHTTPRequestMiddleware(t *testing.T) { } } +func TestSwapComputePayloadSHA256ForUnsignedPayloadMiddleware(t *testing.T) { + cases := map[string]struct { + InitStep func(*middleware.Stack) error + Mutator func(*middleware.Stack) error + ExpectErr string + ExpectIDs []string + }{ + "swap in place": { + InitStep: func(s *middleware.Stack) (err error) { + err = s.Build.Add(middleware.BuildMiddlewareFunc("before", nil), middleware.After) + if err != nil { + return err + } + err = AddComputePayloadSHA256Middleware(s) + if err != nil { + return err + } + err = s.Build.Add(middleware.BuildMiddlewareFunc("after", nil), middleware.After) + if err != nil { + return err + } + return nil + }, + Mutator: SwapComputePayloadSHA256ForUnsignedPayloadMiddleware, + ExpectIDs: []string{ + "before", + computePayloadHashMiddlewareID, + "after", + }, + }, + + "already unsigned payload exists": { + InitStep: func(s *middleware.Stack) (err error) { + err = s.Build.Add(middleware.BuildMiddlewareFunc("before", nil), middleware.After) + if err != nil { + return err + } + err = AddUnsignedPayloadMiddleware(s) + if err != nil { + return err + } + err = s.Build.Add(middleware.BuildMiddlewareFunc("after", nil), middleware.After) + if err != nil { + return err + } + return nil + }, + Mutator: SwapComputePayloadSHA256ForUnsignedPayloadMiddleware, + ExpectIDs: []string{ + "before", + computePayloadHashMiddlewareID, + "after", + }, + }, + + "no compute payload": { + InitStep: func(s *middleware.Stack) (err error) { + err = s.Build.Add(middleware.BuildMiddlewareFunc("before", nil), middleware.After) + if err != nil { + return err + } + err = s.Build.Add(middleware.BuildMiddlewareFunc("after", nil), middleware.After) + if err != nil { + return err + } + return nil + }, + Mutator: SwapComputePayloadSHA256ForUnsignedPayloadMiddleware, + ExpectErr: "not found, " + computePayloadHashMiddlewareID, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + stack := middleware.NewStack(t.Name(), smithyhttp.NewStackRequest) + if err := c.InitStep(stack); err != nil { + t.Fatalf("expect no error, got %v", err) + } + + err := c.Mutator(stack) + if len(c.ExpectErr) != 0 { + if err == nil { + t.Fatalf("expect error, got none") + } + if e, a := c.ExpectErr, err.Error(); !strings.Contains(a, e) { + t.Fatalf("expect error to contain %v, got %v", e, a) + } + return + } + if err != nil { + t.Fatalf("expect no error, got %v", err) + } + + if diff := cmp.Diff(c.ExpectIDs, stack.Build.List()); len(diff) != 0 { + t.Errorf("expect match\n%v", diff) + } + }) + } +} + type nonSeeker struct{} func (nonSeeker) Read(p []byte) (n int, err error) { diff --git a/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3AddPutObjectUnseekableBodyDoc.java b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3AddPutObjectUnseekableBodyDoc.java new file mode 100644 index 00000000000..45afe08c046 --- /dev/null +++ b/codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3AddPutObjectUnseekableBodyDoc.java @@ -0,0 +1,88 @@ +package software.amazon.smithy.aws.go.codegen.customization; + +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.logging.Logger; +import software.amazon.smithy.codegen.core.CodegenException; +import software.amazon.smithy.go.codegen.GoSettings; +import software.amazon.smithy.go.codegen.integration.GoIntegration; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.traits.DocumentationTrait; +import software.amazon.smithy.utils.MapUtils; +import software.amazon.smithy.utils.Pair; +import software.amazon.smithy.utils.SetUtils; + +public class S3AddPutObjectUnseekableBodyDoc implements GoIntegration { + private static final Logger LOGGER = Logger.getLogger(S3AddPutObjectUnseekableBodyDoc.class.getName()); + + private static final Map>> SERVICE_TO_SHAPE_MAP = MapUtils.of( + ShapeId.from("com.amazonaws.s3#AmazonS3"), SetUtils.of( + new Pair(ShapeId.from("com.amazonaws.s3#PutObjectRequest"), "Body"), + new Pair(ShapeId.from("com.amazonaws.s3#UploadPartRequest"), "Body") + ) + ); + + @Override + public byte getOrder() { + // This integration should happen before other integrations that rely on the presence of this trait + return -60; + } + + @Override + public Model preprocessModel( + Model model, GoSettings settings + ) { + ShapeId serviceId = settings.getService(); + if (!SERVICE_TO_SHAPE_MAP.containsKey(serviceId)) { + return model; + } + + Set> shapeIds = SERVICE_TO_SHAPE_MAP.get(serviceId); + + Model.Builder builder = model.toBuilder(); + for (Pair pair : shapeIds) { + ShapeId shapeId = pair.getLeft(); + String memberName = pair.getRight(); + StructureShape parent = model.expectShape(shapeId, StructureShape.class); + + Optional memberOpt = parent.getMember(memberName); + if (!memberOpt.isPresent()) { + // Throw in case member is not present, bad things must of happened. + throw new CodegenException("expect to find " + memberName + " member in shape " + parent.getId()); + } + + MemberShape member = memberOpt.get(); + Shape target = model.expectShape(member.getTarget()); + + Optional docTrait = member.getTrait(DocumentationTrait.class); + String currentDocs = ""; + if (docTrait.isPresent()) { + currentDocs = docTrait.get().getValue(); + } + if (currentDocs.length() != 0) { + currentDocs += "

"; + } + + final String finalCurrentDocs = currentDocs; + StructureShape.Builder parentBuilder = parent.toBuilder(); + parentBuilder.removeMember(memberName); + parentBuilder.addMember(memberName, target.getId(), (memberBuilder) -> { + memberBuilder + .addTraits(member.getAllTraits().values()) + .addTrait(new DocumentationTrait(finalCurrentDocs + + "For using values that are not seekable (io.Seeker) see, " + + "https://aws.github.io/aws-sdk-go-v2/docs/sdk-utilisties/s3/#unseekable-streaming-input")); + }); + + + builder.addShape(parentBuilder.build()); + } + + return builder.build(); + } +} diff --git a/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration b/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration index 2d9f8764c72..bc28b07af84 100644 --- a/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration +++ b/codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration @@ -35,3 +35,4 @@ software.amazon.smithy.aws.go.codegen.AwsHttpPresignURLClientGenerator software.amazon.smithy.aws.go.codegen.ResolveClientConfig software.amazon.smithy.aws.go.codegen.customization.S3GetBucketLocation software.amazon.smithy.aws.go.codegen.RequestResponseLogging +software.amazon.smithy.aws.go.codegen.customization.S3AddPutObjectUnseekableBodyDoc diff --git a/service/s3/api_op_PutObject.go b/service/s3/api_op_PutObject.go index 28badfb4005..291218c91f4 100644 --- a/service/s3/api_op_PutObject.go +++ b/service/s3/api_op_PutObject.go @@ -123,6 +123,9 @@ type PutObjectInput struct { ACL types.ObjectCannedACL // Object data. + // + // For using values that are not seekable (io.Seeker) see, + // https://aws.github.io/aws-sdk-go-v2/docs/sdk-utilisties/s3/#unseekable-streaming-input Body io.Reader // Specifies whether Amazon S3 should use an S3 Bucket Key for object encryption diff --git a/service/s3/api_op_UploadPart.go b/service/s3/api_op_UploadPart.go index bcff02c89b7..0ba51668c17 100644 --- a/service/s3/api_op_UploadPart.go +++ b/service/s3/api_op_UploadPart.go @@ -164,6 +164,9 @@ type UploadPartInput struct { UploadId *string // Object data. + // + // For using values that are not seekable (io.Seeker) see, + // https://aws.github.io/aws-sdk-go-v2/docs/sdk-utilisties/s3/#unseekable-streaming-input Body io.Reader // Size of the body in bytes. This parameter is useful when the size of the body