Add presignable trait to Smithy #897

kggilmer · 2021-08-24T21:46:43Z

Issue #, if available: N/A

Description of changes:

Adds an annotation trait that provides context about which specific service operations may have client-side presigners. This state is currently modeled in customizations in various SDKs. Some samples: Java v2 (in code, eg S3), Go v2, and Kotlin SDKs.

NOTE: this trait only models a specific use case for presigners: generators. It does not model "presign url autofill parameters" of some services due to current migration away from this to other approaches.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

JordonPhillips

Sorry for the delay on this, looks like it got a bit lost between oncalls

JordonPhillips · 2021-09-02T13:55:12Z

docs/source/1.0/spec/aws/aws-auth.rst

 .. smithy-trait:: aws.auth#cognitoUserPools
 .. _aws.auth#cognitoUserPools-trait:

+------------------------
+``presignable`` trait
+------------------------


You'll need to add a smithy-trait directive here and make sure that the cognito user pools directive is directly above its trait. Also the title here should be fully qualified, e.g. aws.auth#presignable (to be fair, we're inconsistent on this currently)

Ah, I didn't notice the significance of these directives. Fixed, thanks.

JordonPhillips · 2021-09-02T13:57:02Z

docs/source/1.0/spec/aws/aws-auth.rst

+Trait value
+    Annotation trait.
+
+The following example defines a service operation for which presigned requests can be generated for.


You should include a directions on how to pre-sign a request, or at least link to any existing AWS docs that do the same.

not: no need for the last "for" in the above sentence.

fixed in #1077

JordonPhillips · 2021-09-02T13:58:41Z

smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.json

+                "smithy.api#trait": {
+                    "selector": "operation"
+                },
+                "smithy.api#documentation": "indicates that a client presigner could be generated for the given operation."


Suggested change

"smithy.api#documentation": "indicates that a client presigner could be generated for the given operation."

"smithy.api#documentation": "Indicates that a client presigner can be generated for the given operation."

JordonPhillips · 2021-09-02T13:59:19Z

docs/source/1.0/spec/aws/aws-auth.rst

+    difficult or impossible.  This trait indicates that a client presigner could be
+    generated for the given operation.


Suggested change

difficult or impossible. This trait indicates that a client presigner could be

generated for the given operation.

difficult or impossible. This trait indicates that a client presigner can be

generated for the given operation.

kggilmer · 2021-09-23T22:51:27Z

bump @JordonPhillips

JordonPhillips

Looks good! Lets get some more feedback from the other sdk teams before we merge though

brainstorm · 2021-09-27T12:49:32Z

Does this trait account for the new Sigv4-A or just Sigv4? See awslabs/aws-sdk-rust#139 (comment) for more details.

kggilmer · 2021-09-28T18:27:22Z

@brainstorm the trait is not auth implementation specific. Rather, the trait references a specific operation. Details such as auth scheme can be determined elsewhere in the model associated with the tagged operation.

mtdowling · 2021-10-15T20:43:40Z

docs/source/1.0/spec/aws/aws-auth.rst

+.. smithy-trait:: aws.auth#presignable
+.. _aws.auth#presignable-trait:
+
+------------------------


This needs to span the entire width of the text. So add more "-" characters.

fixed in #1077

mtdowling · 2021-10-15T20:43:52Z

docs/source/1.0/spec/aws/aws-auth.rst

+    operation invocation that may be used in contexts where direct calls may be
+    difficult or impossible. This trait indicates that a client presigner can be
+    generated for the given operation.
+


I think this line needs to be removed.

fixed in #1077

mtdowling · 2021-10-15T20:45:08Z

docs/source/1.0/spec/aws/aws-auth.rst

+Trait value
+    Annotation trait.
+
+The following example defines a service operation for which presigned requests can be generated for.


not: no need for the last "for" in the above sentence.

mtdowling · 2021-10-15T20:45:33Z

docs/source/1.0/spec/aws/aws-auth.rst

+    operation PingServer {}
+
+
+Given this model, a client-side code generator may generate a function that presigns a `PingServer` request.


nit: we typically try to wrap .rst files at 80 chars

fixed in #1077

mtdowling · 2021-10-15T20:46:51Z

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/PresignableTrait.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.


fixed in #1077 (2022)

mtdowling · 2021-10-15T20:47:52Z

docs/source/1.0/spec/aws/aws-auth.rst

+------------------------
+
+Trait Summary
+    A presigner is a client-side utility that generates a presigned request for a given


Can this start with a short sentence like: "Indicates that an operation can be presigned"?

fixed in #1077

mtdowling · 2021-10-15T20:48:04Z

smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.json

+                "smithy.api#trait": {
+                    "selector": "operation"
+                },
+                "smithy.api#documentation": "Indicates that a client presigner can be generated for the given operation."


Suggestion: "Indicates that an operation can be presigned."

fixed in #1077

kggilmer · 2022-02-03T00:46:11Z

Closing in favor of #1077

Add presignable trait to Smithy

73a633b

kggilmer requested review from mtdowling, jasdel and kstich August 24, 2021 21:46

kggilmer requested review from DavidOgunsAWS and JordonPhillips as code owners August 24, 2021 21:46

kggilmer mentioned this pull request Sep 1, 2021

RFC: API for Pre-signed URLs smithy-lang/smithy-rs#681

Merged

JordonPhillips requested changes Sep 2, 2021

View reviewed changes

Fixes and tweaks from PR feedback

abc7556

JordonPhillips reviewed Sep 27, 2021

View reviewed changes

jdisanti mentioned this pull request Oct 1, 2021

Add presigned request support for Polly smithy-lang/smithy-rs#735

Merged

2 tasks

mtdowling requested changes Oct 15, 2021

View reviewed changes

kggilmer added a commit to kggilmer/smithy that referenced this pull request Feb 3, 2022

Rewrite smithy-lang#897 in forked copy of repo

9005f43

kggilmer mentioned this pull request Feb 3, 2022

Add presignable trait to Smithy #1077

Closed

kggilmer closed this Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add presignable trait to Smithy #897

Add presignable trait to Smithy #897

kggilmer commented Aug 24, 2021 •

edited

JordonPhillips left a comment

JordonPhillips Sep 2, 2021

kggilmer Sep 9, 2021

JordonPhillips Sep 2, 2021

kggilmer Sep 9, 2021

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

JordonPhillips Sep 2, 2021

kggilmer Sep 9, 2021

JordonPhillips Sep 2, 2021

kggilmer Sep 9, 2021

kggilmer commented Sep 23, 2021

JordonPhillips left a comment

brainstorm commented Sep 27, 2021

kggilmer commented Sep 28, 2021

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022 •

edited

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

mtdowling Oct 15, 2021

mtdowling Oct 15, 2021

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

mtdowling Oct 15, 2021

kggilmer Feb 3, 2022

kggilmer commented Feb 3, 2022

	"smithy.api#documentation": "indicates that a client presigner could be generated for the given operation."
	"smithy.api#documentation": "Indicates that a client presigner can be generated for the given operation."

		difficult or impossible. This trait indicates that a client presigner could be
		generated for the given operation.

		operation PingServer {}


		Given this model, a client-side code generator may generate a function that presigns a `PingServer` request.

		@@ -0,0 +1,39 @@
		/*
		* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.

Add presignable trait to Smithy #897

Add presignable trait to Smithy #897

Conversation

kggilmer commented Aug 24, 2021 • edited

JordonPhillips left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kggilmer commented Sep 23, 2021

JordonPhillips left a comment

Choose a reason for hiding this comment

brainstorm commented Sep 27, 2021

kggilmer commented Sep 28, 2021

Choose a reason for hiding this comment

kggilmer Feb 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kggilmer commented Feb 3, 2022

kggilmer commented Aug 24, 2021 •

edited

kggilmer Feb 3, 2022 •

edited