From 3698a91ac81a31f763c55487f200458d5b5eaf0f Mon Sep 17 00:00:00 2001 From: Nathanael Law Date: Wed, 12 May 2021 04:32:44 -0600 Subject: [PATCH] fix(apigatewayv2): authorizer is not removed when HttpNoneAuthorizer is used (#14424) CloudFormation will not remove an existing Authorizer if AuthorizationType and AuthorizerId are simply removed. The AuthorizationType must be explicitly set to NONE for CloudFormation to remove the existing Authorizer. As such, I updated the HttpRoute constructor to include the AuthorizationType even if it is NONE; otherwise it is impossible to remove an authorizer in CDK. Some thought had obviously gone into this previously because of the following line: https://github.com/aws/aws-cdk/blob/2f5eeb08f8790c73db7305cc7f85116e2730267d/packages/%40aws-cdk/aws-apigatewayv2/lib/http/route.ts#L159 I did not manage to track down the reasoning for this in commit comments, so I would be interested to hear why this was done, since I may have overlooked a desired use case. I'm wondering if it was assumed that the default CloudFormation value for AuthorizationType is NONE, so to have a more compact template it was omitted. However, the behavior when AuthorizationType is not present, is to not change the existing Authorizer. Basically in the CloudFormation template, ```yaml APIGETintegrationgoogleapiregister1D8736BD: Type: AWS::ApiGatewayV2::Route Properties: ApiId: Ref: API62EA1CEE RouteKey: GET /integration/google-api/register Target: ... ``` does not have the same effect as ```yaml APIGETintegrationgoogleapiregister1D8736BD: Type: AWS::ApiGatewayV2::Route Properties: ApiId: Ref: API62EA1CEE RouteKey: GET /integration/google-api/register AuthorizationType: NONE Target: ... ``` Only the later will remove an existing Authorizer. If you think this is a bug in CloudFormation and not its intended behavior, please let me know. I am assuming that they would not change the behavior anyway since that could have unintended consequence for anyone who redeploys a template without the AuthorizationType set. BREAKING CHANGE: setting the authorizer of an API route to HttpNoneAuthorizer will now remove any existing authorizer on the route ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../test/http/integ.alb.expected.json | 1 + .../test/http/integ.http-proxy.expected.json | 2 ++ .../test/http/integ.lambda-proxy.expected.json | 1 + .../test/http/integ.nlb.expected.json | 1 + .../test/http/integ.service-discovery.expected.json | 1 + packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts | 4 +--- packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts | 1 + packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts | 1 + .../test/apigateway/integ.call-http-api.expected.json | 1 + 9 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.alb.expected.json b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.alb.expected.json index ae65d49847d54..b9f5d96ff4656 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.alb.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.alb.expected.json @@ -633,6 +633,7 @@ "Ref": "HttpProxyPrivateApiA55E154D" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.http-proxy.expected.json b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.http-proxy.expected.json index 378e7b2395f03..0e53d0a223e42 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.http-proxy.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.http-proxy.expected.json @@ -117,6 +117,7 @@ "Ref": "LambdaProxyApi67594471" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", @@ -185,6 +186,7 @@ "Ref": "HttpProxyApiD0217C67" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.lambda-proxy.expected.json b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.lambda-proxy.expected.json index 58e37b0f64e0a..7963d3534e099 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.lambda-proxy.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.lambda-proxy.expected.json @@ -117,6 +117,7 @@ "Ref": "LambdaProxyApi67594471" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.nlb.expected.json b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.nlb.expected.json index aed54a5a8395c..0a3241cdc8139 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.nlb.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.nlb.expected.json @@ -598,6 +598,7 @@ "Ref": "HttpProxyPrivateApiA55E154D" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.service-discovery.expected.json b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.service-discovery.expected.json index 1aaf644336b8c..00e587f8ac85f 100644 --- a/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.service-discovery.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2-integrations/test/http/integ.service-discovery.expected.json @@ -602,6 +602,7 @@ "Ref": "HttpProxyPrivateApiA55E154D" }, "RouteKey": "$default", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "", diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts index 2252630930c27..5178281d08953 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts @@ -156,8 +156,6 @@ export class HttpRoute extends Resource implements IHttpRoute { ])); } - const authorizationType = authBindResult?.authorizationType === HttpAuthorizerType.NONE ? undefined : authBindResult?.authorizationType; - if (authorizationScopes?.length === 0) { authorizationScopes = undefined; } @@ -167,7 +165,7 @@ export class HttpRoute extends Resource implements IHttpRoute { routeKey: props.routeKey.key, target: `integrations/${integration.integrationId}`, authorizerId: authBindResult?.authorizerId, - authorizationType, + authorizationType: authBindResult?.authorizationType ?? HttpAuthorizerType.NONE, // must be explicitly NONE (not undefined) for stack updates to work correctly authorizationScopes, }; diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts index 25b0a5bca3189..12d2c68aa0ecb 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/api.test.ts @@ -429,6 +429,7 @@ describe('HttpApi', () => { expect(stack).toHaveResource('AWS::ApiGatewayV2::Route', { RouteKey: 'GET /chickens', + AuthorizationType: 'NONE', AuthorizerId: ABSENT, }); }); diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts index 044d278e086e4..8de7d2ae7f1d6 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/route.test.ts @@ -30,6 +30,7 @@ describe('HttpRoute', () => { ], ], }, + AuthorizationType: 'NONE', }); expect(stack).toHaveResource('AWS::ApiGatewayV2::Integration', { diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.call-http-api.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.call-http-api.expected.json index 56d4889af3d55..c6a6abaa89273 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.call-http-api.expected.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/apigateway/integ.call-http-api.expected.json @@ -77,6 +77,7 @@ "Ref": "MyHttpApi8AEAAC21" }, "RouteKey": "ANY /", + "AuthorizationType": "NONE", "Target": { "Fn::Join": [ "",