Skip to content

Commit

Permalink
fix(apigateway): race condition exists between stage and cfnaccount i…
Browse files Browse the repository at this point in the history
…n specrestapi (#22671)

This PR is based off of #18011, which fixed a race condition between RestApi stages and CloudWatch roles. The mentioned PR fixed the issue for RestApi, but not SpecRestApi, which this PR aims to fix. 

The fix was largely implemented in RestApiBase. Since SpecRestApi inherits the same RestApiBase as RestApi, all we need to do is swap the order of resource creation so that the CloudWatch role is created before the RestApi stage, and can be attached correctly. 

This PR also updates the integration tests to reflect the new dependency RestApi stage has on RestApi account. 

Fixes #18925

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
nwinans authored Oct 28, 2022
1 parent efd24d1 commit 4cb008b
Show file tree
Hide file tree
Showing 20 changed files with 403 additions and 384 deletions.
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -662,16 +662,16 @@ export class SpecRestApi extends RestApiBase {
this.restApiRootResourceId = resource.attrRootResourceId;
this.root = new RootResource(this, {}, this.restApiRootResourceId);

this._configureDeployment(props);
if (props.domainName) {
this.addDomainName('CustomDomain', props.domainName);
}

const cloudWatchRoleDefault = FeatureFlags.of(this).isEnabled(APIGATEWAY_DISABLE_CLOUDWATCH_ROLE) ? false : true;
const cloudWatchRole = props.cloudWatchRole ?? cloudWatchRoleDefault;
if (cloudWatchRole) {
this._configureCloudWatchRole(resource);
}

this._configureDeployment(props);
if (props.domainName) {
this.addDomainName('CustomDomain', props.domainName);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"20.0.0"}
{"version":"21.0.0"}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
{
"version": "20.0.0",
"version": "21.0.0",
"testCases": {
"restapi-fromdefinition/DefaultTest": {
"stacks": [
"integtest-restapi-fromdefinition-asset"
],
"assertionStack": "restapi-fromdefinition/DefaultTest/DeployAssert"
"assertionStack": "restapi-fromdefinition/DefaultTest/DeployAssert",
"assertionStackName": "restapifromdefinitionDefaultTestDeployAssertDF3B0845"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"files": {
"68497ac876de4e963fc8f7b5f1b28844c18ecc95e3f7c6e9e0bf250e03c037fb": {
"source": {
Expand All @@ -14,15 +14,15 @@
}
}
},
"b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556": {
"2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7": {
"source": {
"path": "integtest-restapi-fromdefinition-asset.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556.json",
"objectKey": "2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,31 +57,6 @@
]
}
},
"myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
"Ref": "myapi4C7BF186"
},
"Description": "Automatically created by the RestApi construct"
},
"DependsOn": [
"myapibooksGETD6B2F597",
"myapibooks51D54548"
]
},
"myapiDeploymentStageprod298F01AF": {
"Type": "AWS::ApiGateway::Stage",
"Properties": {
"RestApiId": {
"Ref": "myapi4C7BF186"
},
"DeploymentId": {
"Ref": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
},
"StageName": "prod"
}
},
"myapiCloudWatchRole095452E5": {
"Type": "AWS::IAM::Role",
"Properties": {
Expand Down Expand Up @@ -130,6 +105,34 @@
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
},
"myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
"Ref": "myapi4C7BF186"
},
"Description": "Automatically created by the RestApi construct"
},
"DependsOn": [
"myapibooksGETD6B2F597",
"myapibooks51D54548"
]
},
"myapiDeploymentStageprod298F01AF": {
"Type": "AWS::ApiGateway::Stage",
"Properties": {
"RestApiId": {
"Ref": "myapi4C7BF186"
},
"DeploymentId": {
"Ref": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
},
"StageName": "prod"
},
"DependsOn": [
"myapiAccountEC421A0A"
]
}
},
"Outputs": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand All @@ -23,7 +23,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -57,34 +57,34 @@
"data": "myapibooksGETD6B2F597"
}
],
"/integtest-restapi-fromdefinition-asset/my-api/Deployment/Resource": [
"/integtest-restapi-fromdefinition-asset/my-api/CloudWatchRole/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
"data": "myapiCloudWatchRole095452E5"
}
],
"/integtest-restapi-fromdefinition-asset/my-api/DeploymentStage.prod/Resource": [
"/integtest-restapi-fromdefinition-asset/my-api/Account": [
{
"type": "aws:cdk:logicalId",
"data": "myapiDeploymentStageprod298F01AF"
"data": "myapiAccountEC421A0A"
}
],
"/integtest-restapi-fromdefinition-asset/my-api/Endpoint": [
"/integtest-restapi-fromdefinition-asset/my-api/Deployment/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "myapiEndpoint3628AFE3"
"data": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
}
],
"/integtest-restapi-fromdefinition-asset/my-api/CloudWatchRole/Resource": [
"/integtest-restapi-fromdefinition-asset/my-api/DeploymentStage.prod/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "myapiCloudWatchRole095452E5"
"data": "myapiDeploymentStageprod298F01AF"
}
],
"/integtest-restapi-fromdefinition-asset/my-api/Account": [
"/integtest-restapi-fromdefinition-asset/my-api/Endpoint": [
{
"type": "aws:cdk:logicalId",
"data": "myapiAccountEC421A0A"
"data": "myapiEndpoint3628AFE3"
}
],
"/integtest-restapi-fromdefinition-asset/PetsURL": [
Expand All @@ -110,15 +110,6 @@
"type": "aws:cdk:logicalId",
"data": "CheckBootstrapVersion"
}
],
"myapiDeployment92F2CB49fe116fef7f552ff0fc433c9aa3930d2f": [
{
"type": "aws:cdk:logicalId",
"data": "myapiDeployment92F2CB49fe116fef7f552ff0fc433c9aa3930d2f",
"trace": [
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
]
}
]
},
"displayName": "integtest-restapi-fromdefinition-asset"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "20.0.0",
"version": "21.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
Expand Down
Loading

0 comments on commit 4cb008b

Please sign in to comment.