generate auth params and bindings #453

lucix-aws · 2023-09-26T15:21:38Z

Add code generation for modeled auth resolver parameters.
- The only "universal" param is the operation name. This is resolved by implementing an operationName() interface on input structures.
- An additional "region" field is generated for sigv4x services.
Add codegen for auth resolver param bindings.
Add GoIntegration support for loading both auth params and resolvers.

syall · 2023-09-26T20:40:06Z

...go-codegen/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersGenerator.java

+    }
+
+    private void loadFields() {
+        if (context.getService().hasTrait(SigV4Trait.class)) {


Instead of bringing in a new dependency for smithy-aws-traits, you can check trait application by ShapeId.

Suggested change

if (context.getService().hasTrait(SigV4Trait.class)) {

if (context.getService().hasTrait(ShapeId.from("aws.auth#sigv4"))) {

Overall though, it would be nice to add REGION for Sigv4x through a RuntimeClientPlugin rather than special cased here.

syall · 2023-09-26T20:42:05Z

codegen/smithy-go-codegen/build.gradle.kts

@@ -21,6 +21,7 @@ extra["moduleName"] = "software.amazon.smithy.go.codegen"

 dependencies {
    api("software.amazon.smithy:smithy-codegen-core:$smithyVersion")
+    api("software.amazon.smithy:smithy-aws-traits:$smithyVersion")


We should avoid bringing in AWS dependencies.

syall · 2023-09-26T20:42:45Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoUniverseTypes.java

-    public static Symbol tString = universe("string");
+    public static final Symbol STRING = universe("string");


Any reason for this change, but not to the other Symbols?

I put these GoUniverseTypes into a PR first but using them ended up triggering spotbugs/checkstyle errors. I only changed the ones I needed but I'll update them all.

syall · 2023-09-26T20:55:20Z

...en/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/OperationGenerator.java

+        writer.write("""
+                            func (*$T) operationName() string {
+                                return $S
+                            }
+                            """,


Fix indentation here

syall · 2023-09-26T20:56:19Z

...en/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/auth/AuthGenerator.java

+/**
+ * Entry point into smithy client auth generation.
+ */
+public class AuthGenerator {


nit: would annotate this with @SmithyInternalApi

syall · 2023-09-26T20:58:51Z

...go-codegen/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersGenerator.java

+ * SigV4[A] will also have a field for the region.
+ * Additional parameters can be loaded via GoIntegration.
+ */
+public class AuthParametersGenerator {


nit: would annotate this with @SmithyInternalApi

syall · 2023-09-26T21:04:31Z

...-go-codegen/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersResolver.java

+
+import software.amazon.smithy.codegen.core.Symbol;
+
+public record AuthParametersResolver(Symbol resolver) { }


nit: would annotate this with @SmithyInternalApi

syall · 2023-09-26T21:06:43Z

...en/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersResolverGenerator.java

+        };
+    }
+
+    private void loadBindings() {


nit: maybe a better name would be loadResolvers()?

syall · 2023-09-26T21:08:09Z

...en/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersResolverGenerator.java

+ * The only value bound by default is the operation name. Generators must load additional bindings through
+ * GoIntegration.
+ */
+public class AuthParametersResolverGenerator {


nit: would annotate this with @SmithyInternalApi

syall · 2023-09-26T21:08:50Z

...en/src/main/java/software/amazon/smithy/go/codegen/auth/AuthParametersResolverGenerator.java

+    public GoWriter.Writable generate() {
+        loadBindings();
+
+        var paramsSymbol = SymbolUtils.createPointableSymbolBuilder(AuthParametersGenerator.STRUCT_NAME).build();


Can we use AuthParametersGenerator.STRUCT_SYMBOL where paramsSymbol is used?

Yes this predates that symbol. Will update.

isaiahvita · 2023-09-27T14:29:04Z

codegen/smithy-go-codegen/build.gradle.kts

@@ -21,6 +21,7 @@ extra["moduleName"] = "software.amazon.smithy.go.codegen"

 dependencies {
    api("software.amazon.smithy:smithy-codegen-core:$smithyVersion")
+    api("software.amazon.smithy:smithy-aws-traits:$smithyVersion")


isaiahvita · 2023-09-27T14:38:50Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/CodegenVisitor.java

@@ -247,6 +247,11 @@ void execute() {
                protocolGenerator.generateEndpointResolution(context);
            });

+            writers.useFileWriter("auth.go", settings.getModuleName(), writer -> {
+                ProtocolGenerator.GenerationContext context = contextBuilder.writer(writer).build();
+                protocolGenerator.generateAuth(context);


should auth scheme generation be part of protocol generation? i remember the reasoning we used to put endpoint generation in there. we made the argument that an endpoint does have the transport protocol baked into it, so it is tied to a protocol. and the upside was that putting endpoint generation in protocol generation guarantees only one will be chosen.
i think this same sort of reasoning could be applied to auth scheme.
but im second-guessing whether we should have put endpoint generation in there at all. i think if we put auth scheme in protocol generation as well, we should prob get a better understanding of what belongs in here and what doesnt

syall · 2023-09-27T17:51:28Z

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/GoUniverseTypes.java

@@ -21,32 +21,33 @@
 * Collection of Symbol constants for golang universe types.
 * See <a href="https://go.dev/ref/spec#Predeclared_identifiers">predeclared identifiers</a>.
 */
+@SuppressWarnings("checkstyle:ConstantName")


nit, but is this because the constant names aren't all uppercased?

generate auth params and bindings

e7a4a61

lucix-aws requested review from a team as code owners September 26, 2023 15:21

lucix-aws mentioned this pull request Sep 26, 2023

add auth param customizations and regenerate aws/aws-sdk-go-v2#2291

Merged

syall suggested changes Sep 26, 2023

View reviewed changes

refactor

e46f637

isaiahvita reviewed Sep 27, 2023

View reviewed changes

syall approved these changes Sep 27, 2023

View reviewed changes

isaiahvita approved these changes Sep 27, 2023

View reviewed changes

lucix-aws merged commit 0ecb0d3 into feat-sraauth Sep 27, 2023
1 check passed

lucix-aws deleted the feat-sraauth-params branch September 27, 2023 23:36

lucix-aws added a commit that referenced this pull request Oct 2, 2023

generate auth params and bindings (#453)

45ee136

lucix-aws added a commit that referenced this pull request Oct 16, 2023

generate auth params and bindings (#453)

5264be9

lucix-aws added a commit that referenced this pull request Oct 16, 2023

generate auth params and bindings (#453)

f034697

lucix-aws added a commit that referenced this pull request Oct 30, 2023

generate auth params and bindings (#453)

7dbaa31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

generate auth params and bindings #453

generate auth params and bindings #453

lucix-aws commented Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

isaiahvita Sep 27, 2023

syall Sep 26, 2023

lucix-aws Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

syall Sep 26, 2023

lucix-aws Sep 26, 2023

isaiahvita Sep 27, 2023

isaiahvita Sep 27, 2023

syall Sep 27, 2023

	if (context.getService().hasTrait(SigV4Trait.class)) {
	if (context.getService().hasTrait(ShapeId.from("aws.auth#sigv4"))) {

		public static Symbol tString = universe("string");
		public static final Symbol STRING = universe("string");


		import software.amazon.smithy.codegen.core.Symbol;

		public record AuthParametersResolver(Symbol resolver) { }

generate auth params and bindings #453

generate auth params and bindings #453

Conversation

lucix-aws commented Sep 26, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment