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
fix(apigateway): race condition exists between stage and cfnaccount in specrestapi #22671
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
I think an exemption to the linter error should be granted for this package. The actual integration tests themselves don't need to be touched given the nature of the bug fix, but the relevant integration test snapshots have been updated to reflect the behavior of the API. |
That't reasonable. Providing the exemption. |
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
@Mergifyio update |
✅ Branch has been successfully updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the limited updates to the integ test output, it doesn't look like the tests were actually run. Please revert the output of the integ tests and then run yarn integ --update-on-failed
and commit the output.
Pull request has been modified.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
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:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license