Skip to content

Commit

Permalink
Endpoint Prefix & Idempotency Token Auto-fill, and Authentication Bug…
Browse files Browse the repository at this point in the history
…fixes (#1518)

* Ensure TopDownIndex is used when iterating service operations
* Temporary Model Fix Transforms for Location Service
* Fixed Idempotency Token Autofill
* Fixed Endpoint Prefix Bug
* Add Changelog Annotations
  • Loading branch information
skmcgrail authored Dec 3, 2021
1 parent a29bcb0 commit 2c81f80
Show file tree
Hide file tree
Showing 143 changed files with 4,513 additions and 61 deletions.
17 changes: 17 additions & 0 deletions .changelog/b51f6004654f4c5eb4799d04a5045020.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"id": "b51f6004-654f-4c5e-b479-9d04a5045020",
"type": "bugfix",
"description": "Fixed an issue that prevent auto-filling of an API's idempotency parameters when not explictly provided by the caller.",
"modules": [
"service/accessanalyzer",
"service/amp",
"service/appmesh",
"service/braket",
"service/codeguruprofiler",
"service/grafana",
"service/nimble",
"service/proton",
"service/snowdevicemanagement",
"service/wisdom"
]
}
9 changes: 9 additions & 0 deletions .changelog/b55ca4d66d1d4102baf0cb7c49f3bde3.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"id": "b55ca4d6-6d1d-4102-baf0-cb7c49f3bde3",
"type": "bugfix",
"description": "Fixed a bug that prevented the resolution of the correct endpoint for some API operations.",
"modules": [
"service/evidently",
"service/location"
]
}
8 changes: 8 additions & 0 deletions .changelog/ce7b78be34014ab39c6d0f40cd08bd56.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "ce7b78be-3401-4ab3-9c6d-0f40cd08bd56",
"type": "bugfix",
"description": "Fixed an issue that caused some operations to not be signed using sigv4, resulting in authentication failures.",
"modules": [
"service/location"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import software.amazon.smithy.go.codegen.SymbolUtils;
import software.amazon.smithy.go.codegen.integration.GoIntegration;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.TopDownIndex;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
Expand Down Expand Up @@ -198,8 +199,7 @@ public void writeAdditionalFiles(
writeConvertToPresignMiddleware(writer, model, symbolProvider, serviceShape);
});

for (ShapeId operationId : serviceShape.getAllOperations()) {
OperationShape operationShape = model.expectShape(operationId, OperationShape.class);
for (OperationShape operationShape : TopDownIndex.of(model).getContainedOperations(serviceShape)) {
if (!validOperations.contains(operationShape.getId())) {
continue;
}
Expand Down Expand Up @@ -229,11 +229,11 @@ private void writePresignOperationFunction(
writer.writeDocs(
String.format(
"Presign%s is used to generate a presigned HTTP Request which contains presigned URL, signed headers "
+ "and HTTP method used.", operationSymbol.getName())
+ "and HTTP method used.", operationSymbol.getName())
);
writer.openBlock(
"func (c *$T) Presign$T(ctx context.Context, params $P, optFns ...func($P)) "
+ "($P, error) {", "}", presignClientSymbol, operationSymbol,
+ "($P, error) {", "}", presignClientSymbol, operationSymbol,
operationInputSymbol, presignOptionsSymbol, v4PresignedHTTPRequestSymbol,
() -> {
writer.write("if params == nil { params = &$T{} }", operationInputSymbol).insertTrailingNewline();
Expand Down Expand Up @@ -338,17 +338,17 @@ private void writeConvertToPresignMiddleware(
smithyStack,
() -> {
Symbol smithyAfter = SymbolUtils.createValueSymbolBuilder("After",
SmithyGoDependency.SMITHY_MIDDLEWARE)
SmithyGoDependency.SMITHY_MIDDLEWARE)
.build();

// Middleware to remove
Symbol requestInvocationID = SymbolUtils.createPointableSymbolBuilder(
"ClientRequestID",
AwsGoDependency.AWS_MIDDLEWARE)
"ClientRequestID",
AwsGoDependency.AWS_MIDDLEWARE)
.build();

Symbol presignMiddleware = SymbolUtils.createValueSymbolBuilder("NewPresignHTTPRequestMiddleware",
AwsGoDependency.AWS_SIGNER_V4)
AwsGoDependency.AWS_SIGNER_V4)
.build();

// Middleware to add
Expand Down Expand Up @@ -385,25 +385,25 @@ private void writeConvertToPresignMiddleware(
writer.write("// add multi-region access point presigner");

// ==== multi-region access point support
Symbol PresignConstructor = SymbolUtils.createValueSymbolBuilder(
"NewPresignHTTPRequestMiddleware", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();
Symbol PresignConstructor = SymbolUtils.createValueSymbolBuilder(
"NewPresignHTTPRequestMiddleware", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();

Symbol PresignOptions = SymbolUtils.createValueSymbolBuilder(
"PresignHTTPRequestMiddlewareOptions", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();
Symbol PresignOptions = SymbolUtils.createValueSymbolBuilder(
"PresignHTTPRequestMiddlewareOptions", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();

Symbol RegisterPresigningMiddleware = SymbolUtils.createValueSymbolBuilder(
"RegisterPreSigningMiddleware", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();
Symbol RegisterPresigningMiddleware = SymbolUtils.createValueSymbolBuilder(
"RegisterPreSigningMiddleware", AwsCustomGoDependency.S3_CUSTOMIZATION
).build();

writer.openBlock("signermv := $T($T{", "})",
PresignConstructor,PresignOptions, () -> {
writer.write("CredentialsProvider : options.Credentials,");
writer.write("V4Presigner : c.Presigner,");
writer.write("V4aPresigner : c.presignerV4a,");
writer.write("LogSigning : options.ClientLogMode.IsSigning(),");
});
PresignConstructor, PresignOptions, () -> {
writer.write("CredentialsProvider : options.Credentials,");
writer.write("V4Presigner : c.Presigner,");
writer.write("V4aPresigner : c.presignerV4a,");
writer.write("LogSigning : options.ClientLogMode.IsSigning(),");
});

writer.write("err = $T(stack, signermv)", RegisterPresigningMiddleware);
writer.write("if err != nil { return err }");
Expand All @@ -420,7 +420,7 @@ private void writeConvertToPresignMiddleware(
"AddExpiresOnPresignedURL",
AwsCustomGoDependency.S3_CUSTOMIZATION).build();
writer.writeDocs("add middleware to set expiration for s3 presigned url, "
+ " if expiration is set to 0, this middleware sets a default expiration of 900 seconds");
+ " if expiration is set to 0, this middleware sets a default expiration of 900 seconds");
writer.write("err = stack.Build.Add(&$T{ Expires: c.Expires, }, middleware.After)",
expiresAsHeaderMiddleware);
writer.write("if err != nil { return err }");
Expand Down Expand Up @@ -506,7 +506,7 @@ private void writePresignClientHelpers(
// Helper function for NopClient
writer.openBlock("func $L(o *Options) {", "}", NOP_HTTP_CLIENT_OPTION_FUNC_NAME, () -> {
Symbol nopClientSymbol = SymbolUtils.createPointableSymbolBuilder("NopClient",
SmithyGoDependency.SMITHY_HTTP_TRANSPORT)
SmithyGoDependency.SMITHY_HTTP_TRANSPORT)
.build();

writer.write("o.HTTPClient = $T{}", nopClientSymbol);
Expand Down Expand Up @@ -604,8 +604,8 @@ public void writePresignOptionType(
writer.write("");
writer.writeDocs(
String.format("Expires sets the expiration duration for the generated presign url. This should "
+ "be the duration in seconds the presigned URL should be considered valid for. If "
+ "not set or set to zero, presign url would default to expire after 900 seconds."
+ "be the duration in seconds the presigned URL should be considered valid for. If "
+ "not set or set to zero, presign url would default to expire after 900 seconds."
)
);
writer.write("Expires time.Duration");
Expand All @@ -632,15 +632,15 @@ public void writePresignOptionType(
writer.openBlock("func $L(optFns ...func(*Options)) func($P) {", "}",
PRESIGN_OPTIONS_FROM_CLIENT_OPTIONS, presignOptionsSymbol, () -> {
writer.write("return $L(optFns).options", presignOptionsFromClientOptionsInternal.getName());
});
});

writer.insertTrailingNewline();

writer.write("type $L []func(*Options)", presignOptionsFromClientOptionsInternal.getName());
writer.openBlock("func (w $L) options (o $P) {", "}",
presignOptionsFromClientOptionsInternal.getName(), presignOptionsSymbol, () -> {
writer.write("o.ClientOptions = append(o.ClientOptions, w...)");
}).insertTrailingNewline();
}).insertTrailingNewline();


// s3 specific helpers
Expand All @@ -653,15 +653,15 @@ public void writePresignOptionType(
writer.openBlock("func $L(dur time.Duration) func($P) {", "}",
PRESIGN_OPTIONS_FROM_EXPIRES, presignOptionsSymbol, () -> {
writer.write("return $L(dur).options", presignOptionsFromExpiresInternal.getName());
});
});

writer.insertTrailingNewline();

writer.write("type $L time.Duration", presignOptionsFromExpiresInternal.getName());
writer.openBlock("func (w $L) options (o $P) {", "}",
presignOptionsFromExpiresInternal.getName(), presignOptionsSymbol, () -> {
writer.write("o.Expires = time.Duration(w)");
}).insertTrailingNewline();
}).insertTrailingNewline();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import software.amazon.smithy.go.codegen.integration.MiddlewareRegistrar;
import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.TopDownIndex;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
Expand Down Expand Up @@ -494,8 +495,7 @@ public void processFinalizedModel(GoSettings settings, Model model) {
.build());


for (ShapeId operationId : service.getAllOperations()) {
OperationShape operation = model.expectShape(operationId, OperationShape.class);
for (OperationShape operation : TopDownIndex.of(model).getContainedOperations(service)) {
String helperFuncName = generateAddDiscoverEndpointMiddlewareName(service, operation);

Collection<Symbol> middlewareArgs = ListUtils.of(
Expand Down Expand Up @@ -556,8 +556,7 @@ public void writeAdditionalFiles(
});

// generate code specific to the operation
for (ShapeId id : service.getOperations()) {
OperationShape operation = model.expectShape(id, OperationShape.class);
for (OperationShape operation : TopDownIndex.of(model).getContainedOperations(service)) {
goDelegator.useShapeWriter(operation, writer -> {
generateAddDiscoverEndpointMiddleware(model, symbolProvider, writer, service, operation);
generateFetchDiscoveredEndpointFunction(model, symbolProvider, writer, service, operation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import software.amazon.smithy.go.codegen.integration.ProtocolUtils;
import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.TopDownIndex;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
Expand Down Expand Up @@ -87,13 +88,12 @@ private void writeAccountIdSetter(
writer.writeDocs("setDefaultAccountID sets the AccountID to the given value if the current value is nil");
writer.openBlock("func setDefaultAccountID(input interface{}, accountID string) interface{} {", "}", () -> {
writer.openBlock("switch i := input.(type) {", "}", () -> {
for (ShapeId operationId : service.getAllOperations()) {
OperationShape operation = model.expectShape(operationId, OperationShape.class);
for (OperationShape operation : TopDownIndex.of(model).getContainedOperations(service)) {
StructureShape input = ProtocolUtils.expectInput(model, operation);

List<MemberShape> accountId = input.getAllMembers().values().stream()
.filter(m -> m.getMemberName().toLowerCase().equals("accountid"))
.collect(Collectors.toList());
.toList();

if (accountId.isEmpty()) {
continue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.aws.go.codegen.customization;

import java.util.List;
import java.util.logging.Logger;
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.ShapeId;
import software.amazon.smithy.model.traits.AuthTrait;
import software.amazon.smithy.utils.ListUtils;

public class LocationModelFixes implements GoIntegration {
private static final Logger LOGGER = Logger.getLogger(LocationModelFixes.class.getName());

private static final List<ShapeId> SHAPE_ID_EMPTY_AUTH_TRAIT_REMOVAL = ListUtils.of(
ShapeId.from("com.amazonaws.location#BatchEvaluateGeofences"),
ShapeId.from("com.amazonaws.location#DescribeGeofenceCollection"),
ShapeId.from("com.amazonaws.location#DescribeMap"),
ShapeId.from("com.amazonaws.location#DescribePlaceIndex"),
ShapeId.from("com.amazonaws.location#DescribeRouteCalculator"),
ShapeId.from("com.amazonaws.location#DescribeTracker")
);

@Override
public Model preprocessModel(
Model model,
GoSettings settings
) {
if (SHAPE_ID_EMPTY_AUTH_TRAIT_REMOVAL.size() == 0) {
return model;
}

var builder = model.toBuilder();

for (ShapeId shapeId : SHAPE_ID_EMPTY_AUTH_TRAIT_REMOVAL) {
var optionalShape = model.getShape(shapeId);

if (optionalShape.isEmpty()) {
continue;
}

var shape = optionalShape.get().asOperationShape().get();

var optionalAuthTrait = shape.getTrait(AuthTrait.class);

if (optionalAuthTrait.isEmpty()) {
LOGGER.warning(() -> String.format("%s no longer has an AuthTrait", shapeId));
continue;
}

var authTrait = optionalAuthTrait.get();

if (authTrait.getValueSet().size() != 0) {
LOGGER.warning(() -> String.format("%s has a non-empty AuthTrait list and will not be removed",
shapeId));
continue;
}

builder.addShape(shape.toBuilder()
.removeTrait(AuthTrait.ID)
.build());
}

return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import software.amazon.smithy.go.codegen.integration.ProtocolUtils;
import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.TopDownIndex;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
Expand Down Expand Up @@ -57,11 +58,9 @@ public void writeAdditionalFiles(
return;
}

service.getAllOperations().stream()
.filter(shapeId -> shapeId.getName(service).equalsIgnoreCase("Predict"))
TopDownIndex.of(model).getContainedOperations(service).stream()
.filter(shape -> shape.getId().getName(service).equalsIgnoreCase("Predict"))
.findAny()
.map(model::expectShape)
.flatMap(Shape::asOperationShape)
.ifPresent(operation -> {
goDelegator.useShapeWriter(operation, writer -> writeEndpointAccessor(
writer, model, symbolProvider, operation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import software.amazon.smithy.go.codegen.integration.MiddlewareRegistrar;
import software.amazon.smithy.go.codegen.integration.RuntimeClientPlugin;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.TopDownIndex;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
Expand Down Expand Up @@ -97,8 +98,7 @@ private void writeHostedZoneIDInputSanitizer(

writer.openBlock("func sanitizeHostedZoneIDInput(input interface{}) error {", "}", () -> {
writer.openBlock("switch i:= input.(type) {", "}", () -> {
service.getAllOperations().forEach((operationId)-> {
OperationShape operation = model.expectShape(operationId, OperationShape.class);
TopDownIndex.of(model).getContainedOperations(service).forEach((operation)-> {
StructureShape input = model.expectShape(operation.getInput().get(), StructureShape.class);
List<MemberShape> hostedZoneIDMembers = input.getAllMembers().values().stream()
.filter(m -> m.getTarget().getName(service).equalsIgnoreCase("ResourceId")
Expand Down
Loading

0 comments on commit 2c81f80

Please sign in to comment.