Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws-apigateway): race condition between Stage and CfnAccount on SpecRestApi #18925

Closed
arnulfojr opened this issue Feb 10, 2022 · 1 comment · Fixed by #22671
Closed

(aws-apigateway): race condition between Stage and CfnAccount on SpecRestApi #18925

arnulfojr opened this issue Feb 10, 2022 · 1 comment · Fixed by #22671
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. p1

Comments

@arnulfojr
Copy link
Contributor

What is the problem?

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 Stages (whether defined by the user or created automatically) and the CloudWatch CfnAccount, if it exists.

This PR solved it for the RestApi #18011 but not entirely for the SpecRestApi.

Currently when we instantiate a SpecRestApi the class will first create the Deployment and the Stage

this._configureDeployment(props);
if (props.domainName) {
this.addDomainName('CustomDomain', props.domainName);
}
const cloudWatchRole = props.cloudWatchRole ?? true;
if (cloudWatchRole) {
this._configureCloudWatchRole(resource);
}
}

then attach the Stage to the API

// stage name is part of the endpoint, so that makes sense.
const stageName = (props.deployOptions && props.deployOptions.stageName) || 'prod';
this.deploymentStage = new Stage(this, `DeploymentStage.${stageName}`, {
deployment: this._latestDeployment,
...props.deployOptions,

The problem is that when calling the _attachStage the cloudwatchAccount does not exist yet, so it does not create the dependency between each other (as in RestApi)

this.restApi = props.deployment.api;
if (RestApiBase._isRestApiBase(this.restApi)) {
this.restApi._attachStage(this);
}
}

If my understanding is correct, we would just need to swap the calls, first create the cloudwatchAccount then create the deployment and stage.

Reproduction Steps

Create a SpecRestApi and deploy the stack all at once.

What did you expect to happen?

Deployment to complete.

What actually happened?

When creating a SpecRestApi the race condition still happens

CDK CLI Version

1.142.0 (build 5dd8e50)

Framework Version

1.142.0

Node.js Version

17.3.0

OS

macOS

Language

Typescript

Language Version

No response

Other information

No response

@arnulfojr arnulfojr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 10, 2022
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Feb 10, 2022
@arnulfojr arnulfojr changed the title (aws-apigateway): race condition between Stage and CfnAccount (aws-apigateway): race condition between Stage and CfnAccount on SpecRestApi Feb 10, 2022
@ryparker ryparker added the p1 label Feb 21, 2022
@otaviomacedo otaviomacedo removed their assignment Mar 4, 2022
@otaviomacedo otaviomacedo removed the needs-triage This issue or PR still needs to be triaged. label Mar 4, 2022
@mergify mergify bot closed this as completed in #22671 Oct 28, 2022
mergify bot pushed a commit that referenced this issue Oct 28, 2022
…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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants