aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.41k stars 3.8k forks source link

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

Open mallenLF opened 3 months ago

mallenLF commented 3 months ago

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

khushail commented 3 months ago

Thanks @mallenLF , i was able to repro this issue. This might be causing this as you pointed out as well -https://github.com/aws/aws-cdk/blob/6b41c8bf784cb2e8a77ee556fff5910277d3f458/packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts#L589. Please feel free to submit a PR.