Skip to content

Commit

Permalink
fix(apigateway): race condition between Stage and CfnAccount (#18011)
Browse files Browse the repository at this point in the history
When `cloudWatchRole` is enabled, a `CfnAccount` is created for it. Since there is no explicit dependency between the the stages and the account, CloudFormation may deploy them in the wrong order, causing the deployment to fail.

Add an explicit dependency between `Stage`s (whether defined by the user or created automatically) and the CloudWatch `CfnAccount`, if it exists.

Fixes #10722.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo committed Dec 15, 2021
1 parent 890c4c5 commit f11766e
Show file tree
Hide file tree
Showing 20 changed files with 110 additions and 26 deletions.
17 changes: 15 additions & 2 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Expand Up @@ -319,6 +319,8 @@ export abstract class RestApiBase extends Resource implements IRestApi {
private _latestDeployment?: Deployment;
private _domainName?: DomainName;

protected cloudWatchAccount?: CfnAccount;

constructor(scope: Construct, id: string, props: RestApiBaseProps = { }) {
super(scope, id);
this.restApiName = props.restApiName ?? id;
Expand Down Expand Up @@ -500,6 +502,17 @@ export abstract class RestApiBase extends Resource implements IRestApi {
ignore(deployment);
}

/**
* Associates a Stage with this REST API
*
* @internal
*/
public _attachStage(stage: Stage) {
if (this.cloudWatchAccount) {
stage.node.addDependency(this.cloudWatchAccount);
}
}

/**
* @internal
*/
Expand All @@ -509,11 +522,11 @@ export abstract class RestApiBase extends Resource implements IRestApi {
managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonAPIGatewayPushToCloudWatchLogs')],
});

const resource = new CfnAccount(this, 'Account', {
this.cloudWatchAccount = new CfnAccount(this, 'Account', {
cloudWatchRoleArn: role.roleArn,
});

resource.node.addDependency(apiResource);
this.cloudWatchAccount.node.addDependency(apiResource);
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-apigateway/lib/stage.ts
Expand Up @@ -3,7 +3,7 @@ import { Construct } from 'constructs';
import { AccessLogFormat, IAccessLogDestination } from './access-log';
import { CfnStage } from './apigateway.generated';
import { Deployment } from './deployment';
import { IRestApi } from './restapi';
import { IRestApi, RestApiBase } from './restapi';
import { parseMethodOptionsPath } from './util';

/**
Expand Down Expand Up @@ -256,6 +256,10 @@ export class Stage extends Resource implements IStage {

this.stageName = resource.ref;
this.restApi = props.deployment.api;

if (RestApiBase._isRestApiBase(this.restApi)) {
this.restApi._attachStage(this);
}
}

/**
Expand Down
Expand Up @@ -34,12 +34,12 @@
"myauthorizer23CB99DD": {
"Type": "AWS::ApiGateway::Authorizer",
"Properties": {
"Name": "CognitoUserPoolsAuthorizerIntegmyauthorizer10C804C1",
"RestApiId": {
"Ref": "myrestapi551C8392"
},
"Type": "COGNITO_USER_POOLS",
"IdentitySource": "method.request.header.Authorization",
"Name": "CognitoUserPoolsAuthorizerIntegmyauthorizer10C804C1",
"ProviderARNs": [
{
"Fn::GetAtt": [
Expand Down Expand Up @@ -123,7 +123,10 @@
"Ref": "myrestapiDeployment419B1464b903292b53d7532ca4296973bcb95b1a"
},
"StageName": "prod"
}
},
"DependsOn": [
"myrestapiAccountA49A05BE"
]
},
"myrestapiANY94B0497F": {
"Type": "AWS::ApiGateway::Method",
Expand Down
Expand Up @@ -198,7 +198,10 @@
"Ref": "MyRestApiDeploymentB555B582dcff966d69deeda8d47e3bf409ce29cb"
},
"StageName": "prod"
}
},
"DependsOn": [
"MyRestApiAccount2FB6DB7A"
]
},
"MyRestApiANY05143F93": {
"Type": "AWS::ApiGateway::Method",
Expand Down Expand Up @@ -239,6 +242,7 @@
"MyAuthorizer6575980E": {
"Type": "AWS::ApiGateway::Authorizer",
"Properties": {
"Name": "RequestAuthorizerIntegMyAuthorizer5D9D41C5",
"RestApiId": {
"Ref": "MyRestApi2D1F47A9"
},
Expand Down Expand Up @@ -266,8 +270,7 @@
]
]
},
"IdentitySource": "method.request.header.Authorization,method.request.querystring.allow",
"Name": "RequestAuthorizerIntegMyAuthorizer5D9D41C5"
"IdentitySource": "method.request.header.Authorization,method.request.querystring.allow"
}
}
},
Expand Down
Expand Up @@ -105,6 +105,7 @@
"MyAuthorizer6575980E": {
"Type": "AWS::ApiGateway::Authorizer",
"Properties": {
"Name": "TokenAuthorizerIAMRoleIntegMyAuthorizer1DFDE3B5",
"RestApiId": {
"Ref": "MyRestApi2D1F47A9"
},
Expand Down Expand Up @@ -138,8 +139,7 @@
]
]
},
"IdentitySource": "method.request.header.Authorization",
"Name": "TokenAuthorizerIAMRoleIntegMyAuthorizer1DFDE3B5"
"IdentitySource": "method.request.header.Authorization"
}
},
"MyAuthorizerauthorizerInvokePolicy0F88B8E1": {
Expand Down Expand Up @@ -241,7 +241,10 @@
"Ref": "MyRestApiDeploymentB555B582dcff966d69deeda8d47e3bf409ce29cb"
},
"StageName": "prod"
}
},
"DependsOn": [
"MyRestApiAccount2FB6DB7A"
]
},
"MyRestApiANY05143F93": {
"Type": "AWS::ApiGateway::Method",
Expand Down
Expand Up @@ -198,7 +198,10 @@
"Ref": "MyRestApiDeploymentB555B582dcff966d69deeda8d47e3bf409ce29cb"
},
"StageName": "prod"
}
},
"DependsOn": [
"MyRestApiAccount2FB6DB7A"
]
},
"MyRestApiANY05143F93": {
"Type": "AWS::ApiGateway::Method",
Expand Down Expand Up @@ -239,6 +242,7 @@
"MyAuthorizer6575980E": {
"Type": "AWS::ApiGateway::Authorizer",
"Properties": {
"Name": "TokenAuthorizerIntegMyAuthorizer793B1D5F",
"RestApiId": {
"Ref": "MyRestApi2D1F47A9"
},
Expand Down Expand Up @@ -266,8 +270,7 @@
]
]
},
"IdentitySource": "method.request.header.Authorization",
"Name": "TokenAuthorizerIntegMyAuthorizer793B1D5F"
"IdentitySource": "method.request.header.Authorization"
}
}
},
Expand Down
Expand Up @@ -77,7 +77,10 @@
"Ref": "corsapitestDeployment2BF1633A228079ea05e5799220dd4ca13512b92d"
},
"StageName": "prod"
}
},
"DependsOn": [
"corsapitestAccount7D1D6854"
]
},
"corsapitesttwitch0E3D1559": {
"Type": "AWS::ApiGateway::Resource",
Expand Down
Expand Up @@ -377,7 +377,10 @@
"Ref": "deployment3338197541aef5f15bf9a60b10e06fdbe72854f4"
},
"StageName": "prod"
}
},
"DependsOn": [
"lambdarestapiAccount856938D8"
]
}
}
}
Expand Up @@ -229,7 +229,10 @@
"Ref": "booksapiDeployment308B08F132cc25cf8168bd5e99b9e6d4915866b5"
},
"StageName": "prod"
}
},
"DependsOn": [
"booksapiAccountDBA98FB9"
]
},
"booksapiANYApiPermissionrestapibooksexamplebooksapi4538F335ANY73B3CDDC": {
"Type": "AWS::Lambda::Permission",
Expand Down
Expand Up @@ -73,7 +73,10 @@
"Ref": "myapiDeployment92F2CB4972a890db5063ec679071ba7eefc76f2a"
},
"StageName": "prod"
}
},
"DependsOn": [
"myapiAccountEC421A0A"
]
},
"myapiGETF990CE3C": {
"Type": "AWS::ApiGateway::Method",
Expand Down
Expand Up @@ -101,7 +101,10 @@
}
],
"StageName": "beta"
}
},
"DependsOn": [
"myapiAccountEC421A0A"
]
},
"myapiv113487378": {
"Type": "AWS::ApiGateway::Resource",
Expand Down
Expand Up @@ -144,7 +144,10 @@
"Ref": "BooksApiDeployment86CA39AF4ff82f86c127f53c9de94d266b1906be"
},
"StageName": "prod"
}
},
"DependsOn": [
"BooksApiAccount9C44AF8E"
]
},
"BooksApiANY0C4EABE3": {
"Type": "AWS::ApiGateway::Method",
Expand Down
Expand Up @@ -124,7 +124,10 @@
"Ref": "helloapiDeploymentFA89AEEC3622d8c965f356a33fd95586d24bf138"
},
"StageName": "prod"
}
},
"DependsOn": [
"helloapiAccountD8C38BCE"
]
},
"helloapihello4AA00177": {
"Type": "AWS::ApiGateway::Resource",
Expand Down Expand Up @@ -333,7 +336,10 @@
"Ref": "secondapiDeployment20F2C70088fa5a027620045bea3e5043c6d31f5a"
},
"StageName": "prod"
}
},
"DependsOn": [
"secondapiAccountDF729874"
]
},
"secondapihello7264EB69": {
"Type": "AWS::ApiGateway::Resource",
Expand Down
Expand Up @@ -700,7 +700,10 @@
"Ref": "MyApiDeploymentECB0D05E58dcfc85d01f2b81270e177f5347476d"
},
"StageName": "prod"
}
},
"DependsOn": [
"MyApiAccount13882D84"
]
},
"MyApiGETD0C7AA0C": {
"Type": "AWS::ApiGateway::Method",
Expand Down
Expand Up @@ -255,7 +255,10 @@
"Ref": "deployment33381975b5dafda9a97138f301ea25da405640e8"
},
"StageName": "prod"
}
},
"DependsOn": [
"StepFunctionsRestApiAccountBD0CCC0E"
]
}
},
"Outputs": {
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-apigateway/test/restapi.test.ts
Expand Up @@ -50,6 +50,7 @@ describe('restapi', () => {
DeploymentId: { Ref: 'myapiDeployment92F2CB4972a890db5063ec679071ba7eefc76f2a' },
StageName: 'prod',
},
DependsOn: ['myapiAccountEC421A0A'],
},
myapiCloudWatchRole095452E5: {
Type: 'AWS::IAM::Role',
Expand Down
16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/stage.test.ts
@@ -1,4 +1,5 @@
import '@aws-cdk/assert-internal/jest';
import { ResourcePart } from '@aws-cdk/assert-internal';
import * as logs from '@aws-cdk/aws-logs';
import * as cdk from '@aws-cdk/core';
import * as apigateway from '../lib';
Expand Down Expand Up @@ -69,6 +70,21 @@ describe('stage', () => {
});
});

test('stage depends on the CloudWatch role when it exists', () => {
// GIVEN
const stack = new cdk.Stack();
const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: true, deploy: false });
const deployment = new apigateway.Deployment(stack, 'my-deployment', { api });
api.root.addMethod('GET');

// WHEN
new apigateway.Stage(stack, 'my-stage', { deployment });

expect(stack).toHaveResourceLike('AWS::ApiGateway::Stage', {
DependsOn: ['testapiAccount9B907665'],
}, ResourcePart.CompleteDefinition);
});

test('common method settings can be set at the stage level', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
Expand Up @@ -80,7 +80,8 @@
"Ref": "apiDeployment149F1294891f10d69bae7c4d19bdee7af013a950"
},
"StageName": "prod"
}
},
"DependsOn": ["apiAccount57E28B43"]
},
"apiCloudWatchRoleAC81D93E": {
"Type": "AWS::IAM::Role",
Expand Down
Expand Up @@ -73,7 +73,8 @@
"Ref": "MyRestApiDeploymentB555B582d61dc696e12272a0706c826196fa8d62"
},
"StageName": "prod"
}
},
"DependsOn": ["MyRestApiAccount2FB6DB7A"]
},
"MyRestApiANYApiPermissionCallRestApiIntegMyRestApiB570839CANY0C27C1E3": {
"Type": "AWS::Lambda::Permission",
Expand Down
6 changes: 6 additions & 0 deletions packages/decdk/test/__snapshots__/synth.test.js.snap
Expand Up @@ -439,6 +439,9 @@ Object {
"Type": "AWS::ApiGateway::Deployment",
},
"MyApiDeploymentStageprodE1054AF0": Object {
"DependsOn": Array [
"MyApiAccount13882D84",
],
"Properties": Object {
"DeploymentId": Object {
"Ref": "MyApiDeploymentECB0D05E0597cc4592870e54c401cccc6090bd86",
Expand Down Expand Up @@ -1214,6 +1217,9 @@ Object {
"Type": "AWS::ApiGateway::Deployment",
},
"lambdaeventsHelloWorldFunctionAB27BB65ApiEventSourceA7A86A4FDeploymentStageprod6A86C016": Object {
"DependsOn": Array [
"lambdaeventsHelloWorldFunctionAB27BB65ApiEventSourceA7A86A4FAccountF7734F1E",
],
"Properties": Object {
"DeploymentId": Object {
"Ref": "lambdaeventsHelloWorldFunctionAB27BB65ApiEventSourceA7A86A4FDeploymentFF3F4A1Ac00dde791d2719be3e8ea69f9a61a5cd",
Expand Down

0 comments on commit f11766e

Please sign in to comment.