Skip to content

Commit 4cb008b

Browse files
nwinansnwinans
authored
fix(apigateway): race condition exists between stage and cfnaccount in 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*
1 parent efd24d1 commit 4cb008b

20 files changed

+403
-384
lines changed

‎packages/@aws-cdk/aws-apigateway/lib/restapi.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -662,16 +662,16 @@ export class SpecRestApi extends RestApiBase {
662662
this.restApiRootResourceId = resource.attrRootResourceId;
663663
this.root = new RootResource(this, {}, this.restApiRootResourceId);
664664

665-
this._configureDeployment(props);
666-
if (props.domainName) {
667-
this.addDomainName('CustomDomain', props.domainName);
668-
}
669-
670665
const cloudWatchRoleDefault = FeatureFlags.of(this).isEnabled(APIGATEWAY_DISABLE_CLOUDWATCH_ROLE) ? false : true;
671666
const cloudWatchRole = props.cloudWatchRole ?? cloudWatchRoleDefault;
672667
if (cloudWatchRole) {
673668
this._configureCloudWatchRole(resource);
674669
}
670+
671+
this._configureDeployment(props);
672+
if (props.domainName) {
673+
this.addDomainName('CustomDomain', props.domainName);
674+
}
675675
}
676676
}
677677

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/cdk.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"version":"20.0.0"}
1+
{"version":"21.0.0"}

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/integ.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
{
2-
"version": "20.0.0",
2+
"version": "21.0.0",
33
"testCases": {
44
"restapi-fromdefinition/DefaultTest": {
55
"stacks": [
66
"integtest-restapi-fromdefinition-asset"
77
],
8-
"assertionStack": "restapi-fromdefinition/DefaultTest/DeployAssert"
8+
"assertionStack": "restapi-fromdefinition/DefaultTest/DeployAssert",
9+
"assertionStackName": "restapifromdefinitionDefaultTestDeployAssertDF3B0845"
910
}
1011
}
1112
}

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/integtest-restapi-fromdefinition-asset.assets.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "20.0.0",
2+
"version": "21.0.0",
33
"files": {
44
"68497ac876de4e963fc8f7b5f1b28844c18ecc95e3f7c6e9e0bf250e03c037fb": {
55
"source": {
@@ -14,15 +14,15 @@
1414
}
1515
}
1616
},
17-
"b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556": {
17+
"2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7": {
1818
"source": {
1919
"path": "integtest-restapi-fromdefinition-asset.template.json",
2020
"packaging": "file"
2121
},
2222
"destinations": {
2323
"current_account-current_region": {
2424
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
25-
"objectKey": "b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556.json",
25+
"objectKey": "2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7.json",
2626
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
2727
}
2828
}

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/integtest-restapi-fromdefinition-asset.template.json‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,6 @@
5757
]
5858
}
5959
},
60-
"myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96": {
61-
"Type": "AWS::ApiGateway::Deployment",
62-
"Properties": {
63-
"RestApiId": {
64-
"Ref": "myapi4C7BF186"
65-
},
66-
"Description": "Automatically created by the RestApi construct"
67-
},
68-
"DependsOn": [
69-
"myapibooksGETD6B2F597",
70-
"myapibooks51D54548"
71-
]
72-
},
73-
"myapiDeploymentStageprod298F01AF": {
74-
"Type": "AWS::ApiGateway::Stage",
75-
"Properties": {
76-
"RestApiId": {
77-
"Ref": "myapi4C7BF186"
78-
},
79-
"DeploymentId": {
80-
"Ref": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
81-
},
82-
"StageName": "prod"
83-
}
84-
},
8560
"myapiCloudWatchRole095452E5": {
8661
"Type": "AWS::IAM::Role",
8762
"Properties": {
@@ -130,6 +105,34 @@
130105
],
131106
"UpdateReplacePolicy": "Retain",
132107
"DeletionPolicy": "Retain"
108+
},
109+
"myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96": {
110+
"Type": "AWS::ApiGateway::Deployment",
111+
"Properties": {
112+
"RestApiId": {
113+
"Ref": "myapi4C7BF186"
114+
},
115+
"Description": "Automatically created by the RestApi construct"
116+
},
117+
"DependsOn": [
118+
"myapibooksGETD6B2F597",
119+
"myapibooks51D54548"
120+
]
121+
},
122+
"myapiDeploymentStageprod298F01AF": {
123+
"Type": "AWS::ApiGateway::Stage",
124+
"Properties": {
125+
"RestApiId": {
126+
"Ref": "myapi4C7BF186"
127+
},
128+
"DeploymentId": {
129+
"Ref": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
130+
},
131+
"StageName": "prod"
132+
},
133+
"DependsOn": [
134+
"myapiAccountEC421A0A"
135+
]
133136
}
134137
},
135138
"Outputs": {

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/manifest.json‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "20.0.0",
2+
"version": "21.0.0",
33
"artifacts": {
44
"Tree": {
55
"type": "cdk:tree",
@@ -23,7 +23,7 @@
2323
"validateOnSynth": false,
2424
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
2525
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
26-
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/b6f4053913c5258c89b2cb1e8d48d0a04346c7b0736f15235a74f8fd890de556.json",
26+
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/2fcb710210eb75f31b39dd1787cc4271df763ad6b541a188cb417095e1b83aa7.json",
2727
"requiresBootstrapStackVersion": 6,
2828
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
2929
"additionalDependencies": [
@@ -57,34 +57,34 @@
5757
"data": "myapibooksGETD6B2F597"
5858
}
5959
],
60-
"/integtest-restapi-fromdefinition-asset/my-api/Deployment/Resource": [
60+
"/integtest-restapi-fromdefinition-asset/my-api/CloudWatchRole/Resource": [
6161
{
6262
"type": "aws:cdk:logicalId",
63-
"data": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
63+
"data": "myapiCloudWatchRole095452E5"
6464
}
6565
],
66-
"/integtest-restapi-fromdefinition-asset/my-api/DeploymentStage.prod/Resource": [
66+
"/integtest-restapi-fromdefinition-asset/my-api/Account": [
6767
{
6868
"type": "aws:cdk:logicalId",
69-
"data": "myapiDeploymentStageprod298F01AF"
69+
"data": "myapiAccountEC421A0A"
7070
}
7171
],
72-
"/integtest-restapi-fromdefinition-asset/my-api/Endpoint": [
72+
"/integtest-restapi-fromdefinition-asset/my-api/Deployment/Resource": [
7373
{
7474
"type": "aws:cdk:logicalId",
75-
"data": "myapiEndpoint3628AFE3"
75+
"data": "myapiDeployment92F2CB49d7e5c9cfe50a1616e1cef4517d6b8f96"
7676
}
7777
],
78-
"/integtest-restapi-fromdefinition-asset/my-api/CloudWatchRole/Resource": [
78+
"/integtest-restapi-fromdefinition-asset/my-api/DeploymentStage.prod/Resource": [
7979
{
8080
"type": "aws:cdk:logicalId",
81-
"data": "myapiCloudWatchRole095452E5"
81+
"data": "myapiDeploymentStageprod298F01AF"
8282
}
8383
],
84-
"/integtest-restapi-fromdefinition-asset/my-api/Account": [
84+
"/integtest-restapi-fromdefinition-asset/my-api/Endpoint": [
8585
{
8686
"type": "aws:cdk:logicalId",
87-
"data": "myapiAccountEC421A0A"
87+
"data": "myapiEndpoint3628AFE3"
8888
}
8989
],
9090
"/integtest-restapi-fromdefinition-asset/PetsURL": [
@@ -110,15 +110,6 @@
110110
"type": "aws:cdk:logicalId",
111111
"data": "CheckBootstrapVersion"
112112
}
113-
],
114-
"myapiDeployment92F2CB49fe116fef7f552ff0fc433c9aa3930d2f": [
115-
{
116-
"type": "aws:cdk:logicalId",
117-
"data": "myapiDeployment92F2CB49fe116fef7f552ff0fc433c9aa3930d2f",
118-
"trace": [
119-
"!!DESTRUCTIVE_CHANGES: WILL_DESTROY"
120-
]
121-
}
122113
]
123114
},
124115
"displayName": "integtest-restapi-fromdefinition-asset"

‎packages/@aws-cdk/aws-apigateway/test/api-definition.asset.integ.snapshot/restapifromdefinitionDefaultTestDeployAssertDF3B0845.assets.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "20.0.0",
2+
"version": "21.0.0",
33
"files": {
44
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
55
"source": {

0 commit comments

Comments
 (0)