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

(efs): enableAutomaticBackups property of FileSystem is always treated as if it is true #29881

Open
mallenLF opened this issue Apr 18, 2024 · 1 comment · May be fixed by #29914
Open

(efs): enableAutomaticBackups property of FileSystem is always treated as if it is true #29881

mallenLF opened this issue Apr 18, 2024 · 1 comment · May be fixed by #29914
Assignees
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@mallenLF
Copy link

Describe the bug

The enableAutomaticBackups property of props for efs.FileSystem is documented to be false by default, yet the apparent behavior is that it is always true, whether omitting the property or when setting it explicitly to false.

Expected Behavior

Invoking new efs.FileSystem without setting enableAutomaticBackups in the props or setting it to false should create an EFS file system without an enabled backup policy.

Current Behavior

When the enableAutomaticBackups property of props for efs.FileSystem is set to false or left undefined, the backupPolicy of the underlying CfnFileSystem construct is left undefined. This leads to the omitting the BackupPolicy property from the AWS::EFS::FileSystem resource in the generated CloudFormation template, as one might expect. However, experimentation seems to indicate that omitting BackupPolicy from the template results in an EFS file system that has an enabled backup policy. The end result is that an EFS volume created with efs.FileSystem always has an enabled backup policy.

Adding BackupPolicy: {Status: "DISABLED"} to the template does remove the backup policy, which can be achieved by setting the corresponding property in the CfnFileSystem construct.

Reproduction Steps

import * as ec2 from 'aws-cdk-lib/aws-ec2'
import * as efs from 'aws-cdk-lib/aws-efs'

const vpcId = 'vpc-changeme'
const vpc = ec2.Vpc.fromLookup(this, 'VPC', {
  vpcId
})
const filesystem = new efs.FileSystem(this, 'Filesystem', {
  vpc,
  enableAutomaticBackups: false
})

Possible Solution

Change efs-file-system.ts where it says:

backupPolicy: props.enableAutomaticBackups ? { status: 'ENABLED' } : undefined,

to:

backupPolicy: { status: props.enableAutomaticBackups ? 'ENABLED' : 'DISABLED' },

The above proposed changed is not, strictly speaking, a backwards compatible change, so more complex logic may be appropriate to omit the property when props.enableAutomaticBackups === undefined.

Additional Information/Context

In the reproduction sample, appending:

;(fileSystem.node.defaultChild as efs.CfnFileSystem).backupPolicy = { status: 'DISABLED' }

is a workaround.

CDK CLI Version

2.137.0 (build bb90b4c)

Framework Version

No response

Node.js Version

20.11.1

OS

Ubuntu 22.04

Language

TypeScript

Language Version

TypeScript (5.3.3)

Other information

No response

@mallenLF mallenLF added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 18, 2024
@github-actions github-actions bot added the @aws-cdk/aws-efs Related to Amazon Elastic File System label Apr 18, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Apr 18, 2024
@khushail khushail self-assigned this Apr 19, 2024
@khushail
Copy link
Contributor

khushail commented Apr 19, 2024

Thanks @mallenLF , i was able to repro this issue. This might be causing this as you pointed out as well -

backupPolicy: props.enableAutomaticBackups ? { status: 'ENABLED' } : undefined,
. Please feel free to submit a PR.

@khushail khushail added p2 effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 19, 2024
mallenLF added a commit to mallenLF/aws-cdk that referenced this issue Apr 20, 2024
…ated as if it is true (aws#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
2 participants