fix(efs): enableAutomaticBackups property of FileSystem is always treated as if it is true (#29881)#29914
Conversation
…ated as if it is true (#29881) Always set the backupPolicy property of CfnFileSystem instead of leaving it unset when enabledAutomaticBackups is false, because backups default to being enabled when not explicitly disabled in CloudFormation.
github-actions
bot
added
beginning-contributor
aws-cdk-automation
left a comment
aws-cdk-automation
left a comment
There was a problem hiding this comment.
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.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| throughputMode: props.throughputMode, | ||
| provisionedThroughputInMibps: props.provisionedThroughputPerSecond?.toMebibytes(), | ||
| backupPolicy: props.enableAutomaticBackups ? { status: 'ENABLED' } : undefined, | ||
| backupPolicy: { status: props.enableAutomaticBackups ? 'ENABLED' : 'DISABLED' }, |
There was a problem hiding this comment.
Will this cause a breaking change where backupPolicy was undefined before?
There was a problem hiding this comment.
Can we also have unit test and integ test for this change?
There was a problem hiding this comment.
Yes, it will cause a breaking change. The documentation says it's false by default, but it actually is always treated as true (see #29881).
There was a problem hiding this comment.
Yeah, Since this involves a breaking change, we need to handle via the feature flag. CDK Feature Flags are a mechanism that allows the CDK to evolve and change the behavior of certain classes and methods, without causing disruption to existing deployed infrastructure.
Below are the steps on adding a feature flag:
- Define the feature flag in features.ts
- Update the new feature flag in FEATURE_FLAGS.md
- Implement the feature flag. For reference here
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing ✅ A exemption request has been requested. Please wait for a maintainer's review. |
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing ✅ A exemption request has been requested. Please wait for a maintainer's review. |
3 participants
Always set the backupPolicy property of CfnFileSystem instead of leaving it unset when enabledAutomaticBackups is false, because backups default to being enabled when not explicitly disabled in CloudFormation.
Issue # (if applicable)
Closes #29881.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license