aws-cdk
aws-cdk copied to clipboard
fix(aws-apigateway): CloudWatch logging should be disabled by default (under feature flag)
Currently when you create a RestApi cloudwatch logging is enabled by
default. This will create an IAM role and a AWS::ApiGateway::Account
resource, which is what is used to allow API Gateway to write API logs
to CloudWatch logs. There can only be a single API Gateway account per
AWS environment (account/region), but CloudFormation will not throw an
error if you try to create additional accounts. Instead it will update
the existing account with the new configuration.
This can cause issues if customers create more than 1 RestApi. The following scenario is an example.
- Create a single
RestApi
A newAWS::ApiGateway::Account
and IAM role is created. - Create a second
RestApi
AnotherAWS::ApiGateway::Account
/IAM role is created which overwrites the first one. The first RestApi now uses the account/role created by thisRestApi
. - Delete the second
RestApi
TheAWS::ApiGateway::Account
/IAM role is deleted along with the secondRestApi
. The firstRestApi
no longer has access to write to CloudWatch logs.
Because of this behavior, the correct thing to do is to disable
CloudWatch logs by default so that the user has to create the global
resource separately. This new behavior is behind a feature flag
@aws-cdk/aws-apigateway:disableCloudWatchLogs
.
In addition, the default retention policy for both the API Gateway
account and IAM role has been set to RETAIN
so that existing
implementations that do not use the feature flag can avoid the above
scenario. The resources will be unmanaged, but existing RestApis will
not break.
I've updated all the existing integration tests to use the old behavior by explicitly setting
cloudWatchLogs: true
. I then added a new integration test for the new behavior.
closes #10878
All Submissions:
- [x] Have you followed the guidelines in our Contributing guide?
Adding new Unconventional Dependencies:
- [ ] This PR adds new unconventional dependencies following the process described here
New Features
- [x] Have you added the new feature to an integration test?
- [x] Did you use
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?
- [x] Did you use
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Might this be a good opportunity to update the integration tests to the new style?
Might this be a good opportunity to update the integration tests to the new style?
I should have been the one to suggest this 🤣
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).
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
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 746b9e67d028df4ca55e1b7c27f4a1303b7a4e59
- Result: SUCCEEDED
- Build Logs (available for 30 days)
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).