From a8da412b36ba39ec1100c2ffe56dcadec37abb9e Mon Sep 17 00:00:00 2001 From: Chase Coalwell Date: Mon, 11 Jan 2021 14:28:07 -0800 Subject: [PATCH 1/3] Add validation for payloadFormatVersion with HTTP APIs --- .../apigateway/openapi/AddIntegrations.java | 15 ++ .../openapi/AddIntegrationsTest.java | 26 ++++ .../aws/apigateway/openapi/cors-model.json | 3 +- .../openapi/cors-model.openapi.json | 4 + .../cors-with-additional-headers.openapi.json | 4 + .../invalid-integration-for-http-api.json | 145 ++++++++++++++++++ 6 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/invalid-integration-for-http-api.json diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java index 37f680091d0..e4ce143e350 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java @@ -106,6 +106,7 @@ private static ObjectNode getIntegrationAsObject( if (integration instanceof MockIntegrationTrait) { integrationNode = integration.toNode().expectObjectNode().withMember("type", Node.from("mock")); } else if (integration instanceof IntegrationTrait) { + validateTraitConfiguration((IntegrationTrait) integration, context, shape); integrationNode = ((IntegrationTrait) integration).toExpandedNode(context.getService(), shape); } else { throw new OpenApiException("Unexpected integration trait: " + integration); @@ -121,6 +122,20 @@ private static ObjectNode getIntegrationAsObject( return integrationNode; } + private static void validateTraitConfiguration(IntegrationTrait trait, + Context context, + OperationShape operation) { + // For HTTP APIs, API Gateway requires that the payloadFormatVersion is set on integrations. + // https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-swagger-extensions-integration.html + // If the payloadFormatVersion has not been set on an integration and the apiGatewayType has been set to "HTTP", + // the conversion fails. + if (!trait.getPayloadFormatVersion().isPresent() && context.getConfig().getExtensions(ApiGatewayConfig.class) + .getApiGatewayType().equals(ApiGatewayConfig.ApiType.HTTP)) { + throw new OpenApiException("When using the HTTP apiGatewayType, a payloadFormatVersion must be set on the" + + " integration applied to the operation: " + operation.getId()); + } + } + private ObjectNode updateIntegrationWithCors( Context context, OperationObject operationObject, diff --git a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java index 0d576064bfb..13730e5cff0 100644 --- a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java +++ b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java @@ -15,11 +15,16 @@ package software.amazon.smithy.aws.apigateway.openapi; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; + +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.openapi.OpenApiConfig; +import software.amazon.smithy.openapi.OpenApiException; import software.amazon.smithy.openapi.fromsmithy.OpenApiConverter; import software.amazon.smithy.openapi.model.OpenApi; import software.amazon.smithy.utils.IoUtils; @@ -56,4 +61,25 @@ public void addsIntegrationsWithoutCredentials() { Node.assertEquals(result, expectedNode); } + + @Test + public void throwsOnInvalidIntegrationTraitForHttpApi() { + Model model = Model.assembler(getClass().getClassLoader()) + .discoverModels(getClass().getClassLoader()) + .addImport(getClass().getResource("invalid-integration-for-http-api.json")) + .assemble() + .unwrap(); + OpenApiConfig config = new OpenApiConfig(); + config.setService(ShapeId.from("smithy.example#Service")); + ApiGatewayConfig apiGatewayConfig = new ApiGatewayConfig(); + apiGatewayConfig.setApiGatewayType(ApiGatewayConfig.ApiType.HTTP); + config.putExtensions(apiGatewayConfig); + try { + OpenApiConverter.create().config(config).convertToNode(model); + Assertions.fail("Expected to throw"); + } catch (OpenApiException e) { + assertThat(e.getMessage(), containsString("a payloadFormatVersion must be set on the integration" + + " applied to the operation:")); + } + } } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.json index 445ffffdbf6..2c3624b30d6 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.json @@ -28,7 +28,8 @@ "type": "aws_proxy", "credentials": "arn:aws:iam::123456789012:role/{serviceName}{operationName}LambdaRole", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:{serviceName}{operationName}/invocations", - "httpMethod": "POST" + "httpMethod": "POST", + "payloadFormatVersion": "1.0" } } }, diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json index 6b03c019cb6..e46825b0edc 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json @@ -36,6 +36,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceListPayloads/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceListPayloadsLambdaRole", "responses": { "default": { @@ -137,6 +138,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceDeletePayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceDeletePayloadLambdaRole", "responses": { "default": { @@ -194,6 +196,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceGetPayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceGetPayloadLambdaRole", "responses": { "default": { @@ -336,6 +339,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServicePutPayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServicePutPayloadLambdaRole", "responses": { "default": { diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json index 73af2a4cde2..c8bf2eb6807 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json @@ -36,6 +36,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceListPayloads/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceListPayloadsLambdaRole", "responses": { "default": { @@ -137,6 +138,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceDeletePayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceDeletePayloadLambdaRole", "responses": { "default": { @@ -194,6 +196,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServiceGetPayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServiceGetPayloadLambdaRole", "responses": { "default": { @@ -336,6 +339,7 @@ "type": "aws_proxy", "uri": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:123456789012:function:MyServicePutPayload/invocations", "httpMethod": "POST", + "payloadFormatVersion": "1.0", "credentials": "arn:aws:iam::123456789012:role/MyServicePutPayloadLambdaRole", "responses": { "default": { diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/invalid-integration-for-http-api.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/invalid-integration-for-http-api.json new file mode 100644 index 00000000000..152e48746d4 --- /dev/null +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/invalid-integration-for-http-api.json @@ -0,0 +1,145 @@ +{ + "smithy": "1.0", + "shapes": { + "smithy.example#Service": { + "type": "service", + "version": "2018-03-17", + "operations": [ + { + "target": "smithy.example#Operation1" + } + ], + "traits": { + "aws.protocols#restJson1": {}, + "aws.api#service": { + "sdkId": "Some Value" + }, + "aws.apigateway#integration": { + "type": "aws", + "uri": "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:012345678901:function:HelloWorld/invocations", + "httpMethod": "POST", + "credentials": "arn:aws:iam::012345678901:role/apigateway-invoke-lambda-exec-role", + "requestTemplates": { + "application/json": "#set ($root=$input.path('$')) { \"stage\": \"$root.name\", \"user-id\": \"$root.key\" }", + "application/xml": "#set ($root=$input.path('$')) $root.name " + }, + "requestParameters": { + "integration.request.path.stage": "method.request.querystring.version", + "integration.request.querystring.provider": "method.request.querystring.vendor" + }, + "cacheNamespace": "cache namespace", + "cacheKeyParameters": [], + "passThroughBehavior": "never", + "responses": { + "2\\d{2}": { + "statusCode": "200", + "responseParameters": { + "method.response.header.requestId": "integration.response.header.cid" + }, + "responseTemplates": { + "application/json": "#set ($root=$input.path('$')) { \"stage\": \"$root.name\", \"user-id\": \"$root.key\" }", + "application/xml": "#set ($root=$input.path('$')) $root.name " + } + }, + "302": { + "statusCode": "302", + "responseParameters": { + "method.response.header.Location": "integration.response.body.redirect.url" + } + }, + "default": { + "statusCode": "400", + "responseParameters": { + "method.response.header.test-method-response-header": "'static value'" + } + } + } + } + } + }, + "smithy.example#Operation1": { + "type": "operation", + "input": { + "target": "smithy.example#OperationInput" + }, + "output": { + "target": "smithy.example#OperationOutput" + }, + "errors": [ + { + "target": "smithy.example#Default" + }, + { + "target": "smithy.example#Redirect" + } + ], + "traits": { + "smithy.api#http": { + "method": "POST", + "uri": "/1" + } + } + }, + "smithy.example#Default": { + "type": "structure", + "members": { + "testMethodResponseHeader": { + "target": "smithy.api#String", + "traits": { + "smithy.api#httpHeader": "test-method-response-header" + } + } + }, + "traits": { + "smithy.api#error": "client", + "smithy.api#httpError": 400 + } + }, + "smithy.example#Redirect": { + "type": "structure", + "members": { + "location": { + "target": "smithy.api#String", + "traits": { + "smithy.api#httpHeader": "Location" + } + } + }, + "traits": { + "smithy.api#error": "client", + "smithy.api#httpError": 302, + "smithy.api#suppress": ["HttpResponseCodeSemantics"] + } + }, + "smithy.example#OperationInput": { + "type": "structure", + "members": { + "vendor": { + "target": "smithy.api#Integer", + "traits": { + "smithy.api#httpQuery": "vendor", + "smithy.api#required": {} + } + }, + "version": { + "target": "smithy.api#Integer", + "traits": { + "smithy.api#httpQuery": "version", + "smithy.api#required": {} + } + } + } + }, + "smithy.example#OperationOutput": { + "type": "structure", + "members": { + "requestId": { + "target": "smithy.api#String", + "traits": { + "smithy.api#httpHeader": "requestId" + } + } + } + } + } +} From 1084971afef1c66f2cedaf6d5ebeb2fff92fdb4f Mon Sep 17 00:00:00 2001 From: Chase Coalwell Date: Mon, 11 Jan 2021 15:24:13 -0800 Subject: [PATCH 2/3] Simplify test assertion --- .../apigateway/openapi/AddIntegrations.java | 5 +-- .../openapi/AddIntegrationsTest.java | 31 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java index e4ce143e350..2e24275c3d5 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java @@ -129,8 +129,9 @@ private static void validateTraitConfiguration(IntegrationTrait trait, // https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-swagger-extensions-integration.html // If the payloadFormatVersion has not been set on an integration and the apiGatewayType has been set to "HTTP", // the conversion fails. - if (!trait.getPayloadFormatVersion().isPresent() && context.getConfig().getExtensions(ApiGatewayConfig.class) - .getApiGatewayType().equals(ApiGatewayConfig.ApiType.HTTP)) { + ApiGatewayConfig.ApiType apiType = context.getConfig().getExtensions(ApiGatewayConfig.class) + .getApiGatewayType(); + if (!trait.getPayloadFormatVersion().isPresent() && apiType.equals(ApiGatewayConfig.ApiType.HTTP)) { throw new OpenApiException("When using the HTTP apiGatewayType, a payloadFormatVersion must be set on the" + " integration applied to the operation: " + operation.getId()); } diff --git a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java index 13730e5cff0..6ec60402b86 100644 --- a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java +++ b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java @@ -17,8 +17,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertThrows; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.Node; @@ -64,22 +64,21 @@ public void addsIntegrationsWithoutCredentials() { @Test public void throwsOnInvalidIntegrationTraitForHttpApi() { - Model model = Model.assembler(getClass().getClassLoader()) - .discoverModels(getClass().getClassLoader()) - .addImport(getClass().getResource("invalid-integration-for-http-api.json")) - .assemble() - .unwrap(); - OpenApiConfig config = new OpenApiConfig(); - config.setService(ShapeId.from("smithy.example#Service")); - ApiGatewayConfig apiGatewayConfig = new ApiGatewayConfig(); - apiGatewayConfig.setApiGatewayType(ApiGatewayConfig.ApiType.HTTP); - config.putExtensions(apiGatewayConfig); - try { + OpenApiException thrown = assertThrows(OpenApiException.class, () -> { + Model model = Model.assembler(getClass().getClassLoader()) + .discoverModels(getClass().getClassLoader()) + .addImport(getClass().getResource("invalid-integration-for-http-api.json")) + .assemble() + .unwrap(); + OpenApiConfig config = new OpenApiConfig(); + config.setService(ShapeId.from("smithy.example#Service")); + ApiGatewayConfig apiGatewayConfig = new ApiGatewayConfig(); + apiGatewayConfig.setApiGatewayType(ApiGatewayConfig.ApiType.HTTP); + config.putExtensions(apiGatewayConfig); OpenApiConverter.create().config(config).convertToNode(model); - Assertions.fail("Expected to throw"); - } catch (OpenApiException e) { - assertThat(e.getMessage(), containsString("a payloadFormatVersion must be set on the integration" + }); + + assertThat(thrown.getMessage(), containsString("a payloadFormatVersion must be set on the integration" + " applied to the operation:")); - } } } From 9c1f2843642ef6157bcf3c2914933bd183d460e1 Mon Sep 17 00:00:00 2001 From: Chase Coalwell Date: Tue, 12 Jan 2021 11:32:23 -0800 Subject: [PATCH 3/3] Improve error message --- .../amazon/smithy/aws/apigateway/openapi/AddIntegrations.java | 4 ++-- .../smithy/aws/apigateway/openapi/AddIntegrationsTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java index 2e24275c3d5..77a63fbc64b 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java @@ -132,8 +132,8 @@ private static void validateTraitConfiguration(IntegrationTrait trait, ApiGatewayConfig.ApiType apiType = context.getConfig().getExtensions(ApiGatewayConfig.class) .getApiGatewayType(); if (!trait.getPayloadFormatVersion().isPresent() && apiType.equals(ApiGatewayConfig.ApiType.HTTP)) { - throw new OpenApiException("When using the HTTP apiGatewayType, a payloadFormatVersion must be set on the" - + " integration applied to the operation: " + operation.getId()); + throw new OpenApiException("When the 'apiGatewayType' OpenAPI conversion setting is 'HTTP', a " + + "'payloadFormatVersion' must be set on the aws.apigateway#integration trait."); } } diff --git a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java index 6ec60402b86..1d3d71e3195 100644 --- a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java +++ b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrationsTest.java @@ -78,7 +78,7 @@ public void throwsOnInvalidIntegrationTraitForHttpApi() { OpenApiConverter.create().config(config).convertToNode(model); }); - assertThat(thrown.getMessage(), containsString("a payloadFormatVersion must be set on the integration" - + " applied to the operation:")); + assertThat(thrown.getMessage(), containsString("When the 'apiGatewayType' OpenAPI conversion setting is" + + " 'HTTP', a 'payloadFormatVersion' must be set on the aws.apigateway#integration trait.")); } }