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

feat(efs): replicating file systems #29347

Merged
merged 61 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
c426b12
feat: configure replication
badmintoncryer Mar 2, 2024
6dcf485
fix: replication config correctly
badmintoncryer Mar 3, 2024
b1361d8
fix: debug
badmintoncryer Mar 3, 2024
f31fc43
test: add integ test
badmintoncryer Mar 3, 2024
2737c3f
test: add unit test
badmintoncryer Mar 3, 2024
9956a3b
fix: property name
badmintoncryer Mar 3, 2024
9a101c7
chore: add comments
badmintoncryer Mar 3, 2024
75dc9da
feat: add region validation
badmintoncryer Mar 3, 2024
c575735
chore: format
badmintoncryer Mar 3, 2024
8537be0
fix: add comments
badmintoncryer Mar 3, 2024
9cb4146
feat: specify region when filesystem is passed
badmintoncryer Mar 4, 2024
2b664b2
fix: remove unused import
badmintoncryer Mar 4, 2024
2525202
fix: readme
badmintoncryer Mar 4, 2024
e73da37
Update efs-file-system.ts
badmintoncryer Mar 5, 2024
41dcee4
feat: remove `enable` property
badmintoncryer Mar 5, 2024
f77fb2c
feat: replicationConfiguration is to be array
badmintoncryer Mar 5, 2024
6be4342
test: remove integ test files
badmintoncryer Mar 5, 2024
0fd10b0
test: add integ test files
badmintoncryer Mar 5, 2024
1d19f2f
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 6, 2024
6914efd
Revert "test: add integ test files"
badmintoncryer Mar 7, 2024
ee9b069
Revert "test: remove integ test files"
badmintoncryer Mar 7, 2024
67e4a90
Revert "feat: replicationConfiguration is to be array"
badmintoncryer Mar 7, 2024
8cff88a
docs: update README.md
badmintoncryer Mar 7, 2024
3bf0713
fix: readme
badmintoncryer Mar 7, 2024
e666eeb
fix: integ test
badmintoncryer Mar 7, 2024
0db02bb
Update packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts
badmintoncryer Mar 8, 2024
f9f7879
Update packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts
badmintoncryer Mar 8, 2024
831b22f
Update packages/aws-cdk-lib/aws-efs/README.md
badmintoncryer Mar 8, 2024
ad40a5c
Update packages/aws-cdk-lib/aws-efs/README.md
badmintoncryer Mar 8, 2024
14d5256
feat: validate whether to specify region or destinationFileSystem
badmintoncryer Mar 8, 2024
9cdb6eb
docs: udpate readme
badmintoncryer Mar 8, 2024
7e24d32
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 8, 2024
9b6bba7
fix: unit test
badmintoncryer Mar 8, 2024
dba033b
Update README.md
badmintoncryer Mar 9, 2024
aeeaae8
docs: update readme
badmintoncryer Mar 9, 2024
495d41a
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 9, 2024
1b2e48b
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 12, 2024
ef15ab1
Update README.md
badmintoncryer Mar 16, 2024
a3f177f
Update efs-file-system.ts
badmintoncryer Mar 16, 2024
1d9092c
Update efs-file-system.test.ts
badmintoncryer Mar 16, 2024
1700075
Update efs-file-system.test.ts
badmintoncryer Mar 16, 2024
9360944
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 27, 2024
ffb634f
refactor: create ReplicationConfiguration class
badmintoncryer Mar 31, 2024
786643b
test: update integ test
badmintoncryer Mar 31, 2024
37cea2b
refactor
badmintoncryer Mar 31, 2024
6f1c6dd
fix: rename
badmintoncryer Mar 31, 2024
163b45a
fix: build
badmintoncryer Mar 31, 2024
1ac2a5e
chore: update comments
badmintoncryer Mar 31, 2024
f202048
Merge branch 'main' into 21455-efsReplication
badmintoncryer Mar 31, 2024
e41e3f3
chore: update comments
badmintoncryer Apr 1, 2024
869b1a0
Merge branch 'main' into 21455-efsReplication
badmintoncryer Apr 1, 2024
85c33eb
docs: udpate readme
badmintoncryer Apr 2, 2024
46f81da
Merge branch 'main' into 21455-efsReplication
badmintoncryer Apr 2, 2024
6aa6666
Merge branch 'main' into 21455-efsReplication
badmintoncryer Apr 14, 2024
edf0c08
chore: refactor
badmintoncryer Apr 22, 2024
59e8cd4
Merge branch 'main' into 21455-efsReplication
badmintoncryer Apr 22, 2024
7c04a23
fix: add export
badmintoncryer Apr 22, 2024
c26a21f
fix: optional
badmintoncryer Apr 22, 2024
b806c5e
Update packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts
badmintoncryer Apr 24, 2024
7e3380f
chore: update comments
badmintoncryer Apr 24, 2024
ecfd8c0
Merge branch 'main' into 21455-efsReplication
mergify[bot] Apr 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import * as cdk from 'aws-cdk-lib';
import * as efs from 'aws-cdk-lib/aws-efs';
import * as kms from 'aws-cdk-lib/aws-kms';
import * as integ from '@aws-cdk/integ-tests-alpha';

const app = new cdk.App();
const stack = new cdk.Stack(app, 'efsReplication');

const vpc = new ec2.Vpc(stack, 'Vpc', {
natGateways: 0,
});

const kmsKey = new kms.Key(stack, 'Key', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
});

new efs.FileSystem(stack, 'oneZoneReplicationFileSystem', {
vpc,
removalPolicy: cdk.RemovalPolicy.DESTROY,
replicationConfiguration: [{
kmsKey,
region: 'us-east-1',
availabilityZone: 'us-east-1a',
}],
});

const destination = new efs.FileSystem(stack, 'destinationFileSystem', {
vpc,
removalPolicy: cdk.RemovalPolicy.DESTROY,
replicationOverwriteProtection: efs.ReplicationOverwriteProtection.DISABLED,
});

new efs.FileSystem(stack, 'existFileSystemReplication', {
vpc,
removalPolicy: cdk.RemovalPolicy.DESTROY,
replicationConfiguration: [{
destinationFileSystem: destination,
}],
});

new integ.IntegTest(app, 'efsReplicationIntegTest', {
testCases: [stack],
});
app.synth();
36 changes: 36 additions & 0 deletions packages/aws-cdk-lib/aws-efs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,42 @@ This is to prevent deployment failures due to cross-AZ configurations.

⚠️ When `oneZone` is enabled, `vpcSubnets` cannot be specified.

### [Replicating file systems](https://docs.aws.amazon.com/efs/latest/ug/efs-replication.html)
badmintoncryer marked this conversation as resolved.
Show resolved Hide resolved

You can create a replica of your EFS file system in the AWS Region of your preference.

```ts
import * as kms from 'aws-cdk-lib/aws-kms';

declare const vpc: ec2.Vpc;
declare const kmsKey: kms.Key;

// auto generate a replication destination file system
new efs.FileSystem(this, 'ReplicationSourceFileSystem1', {
vpc,
replicationConfiguration: [{
kmsKey,
region: 'us-east-1',
availabilityZone: 'us-east-1a', // Specifing the AZ means creating a One Zone file system as the replication destination
}]
});

// specify the replication destination file system
const destinationFileSystem = new efs.FileSystem(this, 'DestinationFileSystem', {
vpc,
// set as the read-only file system for use as a replication destination
replicationOverwriteProtection: efs.ReplicationOverwriteProtection.DISABLED,
});

new efs.FileSystem(this, 'ReplicationSourceFileSystem2', {
vpc,
replicationConfiguration: [{
destinationFileSystem,
// cannot configure other properties when destinationFileSystem is specified
}]
});
```

### IAM to control file system data access

You can use both IAM identity policies and resource policies to control client access to Amazon EFS resources in a way that is scalable and optimized for cloud environments. Using IAM, you can permit clients to perform specific actions on a file system, including read-only, write, and root access.
Expand Down
82 changes: 81 additions & 1 deletion packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { CfnFileSystem, CfnMountTarget } from './efs.generated';
import * as ec2 from '../../aws-ec2';
import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import { ArnFormat, FeatureFlags, Lazy, RemovalPolicy, Resource, Size, Stack, Tags } from '../../core';
import { ArnFormat, FeatureFlags, Lazy, RemovalPolicy, Resource, Size, Stack, Tags, Token } from '../../core';
import * as cxapi from '../../cx-api';

/**
Expand Down Expand Up @@ -324,6 +324,13 @@ export interface FileSystemProps {
* @default ReplicationOverwriteProtection.ENABLED
*/
readonly replicationOverwriteProtection?: ReplicationOverwriteProtection;

/**
* Replication configuration for the file system.
*
* @default - no replication
*/
readonly replicationConfiguration?: ReplicationConfiguration[];
}

/**
Expand All @@ -350,6 +357,44 @@ export interface FileSystemAttributes {
readonly fileSystemArn?: string;
}

/**
* Replication configuration for the file system.
*/
export interface ReplicationConfiguration {
Copy link
Contributor

@gracelu0 gracelu0 Mar 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are multiple ways to configure the destination file system in ReplicationConfiguration, I'd suggest making this an enum-like class with static functions to enforce stronger typing and coupling between required properties. For example:

export abstract class ReplicationConfiguration {
    
    public static destinationFileSystem(fileSystem: IFileSystem): ReplicationConfiguration {
        // configure properties and return 
    }

    public static oneZoneFileSystem(availabilityZone: string, region: string, kmsKey?: kms.IKey): ReplicationConfiguration {
        // configure properties and return 
    }

    public static regionalFileSystem(region: string, kmsKey?: kms.IKey): ReplicationConfiguration {
        // configure properties and return  
    }
    public abstract readonly destinationFileSystem: IFileSystem;
    public abstract readonly availabilityZoneName: string;
    public abstract readonly region: string;
    public abstract readonly kmsKey: kms.IKey;
}

This should also reduce the need for some of the validation/error-handling below which is ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I'll try to implement it.

/**
* The existing destination file system for the replication.
*
* You cannot configure `kmsKey`, `region` and `az` when `destinationFileSystem` is set.
badmintoncryer marked this conversation as resolved.
Show resolved Hide resolved
*
* @default - create a new file system for the replication destination
*/
readonly destinationFileSystem?: IFileSystem;

/**
* AWS KMS key used to protect the encrypted file system.
*
* @default - service-managed KMS key for Amazon EFS is used
*/
readonly kmsKey?: kms.IKey;

/**
* The AWS Region in which the destination file system is located.
*
* @default - the region of the stack
*/
readonly region?: string;

/**
* The availability zone name of the destination file system.
* One zone file system is used as the destination file system when this property is set.
*
* You have to specify the `region` property for the region that the specified availability zone belongs to.
*
* @default - create regional file system for the replication destination
*/
readonly availabilityZone?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the availability zone is only a valid field if the region is provided, these two should fields should be a single object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further down I'm also seeing a lot of checking for whether certain properties are compatible with one another. This tells me that the contract here is too loose. I get that you are working from a construct that uses a lot of optional props when they really shouldn't be but I'd like further changes to not follow that pattern.

Can we structure this in a way that enforces the dependent props in the contract and disallows the combinations you're checking for below?

}

enum ClientAction {
MOUNT = 'elasticfilesystem:ClientMount',
WRITE = 'elasticfilesystem:ClientWrite',
Expand Down Expand Up @@ -553,6 +598,30 @@ export class FileSystem extends FileSystemBase {
if (props.throughputMode === ThroughputMode.ELASTIC && props.performanceMode === PerformanceMode.MAX_IO) {
throw new Error('ThroughputMode ELASTIC is not supported for file systems with performanceMode MAX_IO');
}

if (props.replicationConfiguration) {
if (props.replicationOverwriteProtection === ReplicationOverwriteProtection.DISABLED) {
badmintoncryer marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('Cannot configure `replicationConfiguration` when `replicationOverwriteProtection` is set to `DISABLED`');
badmintoncryer marked this conversation as resolved.
Show resolved Hide resolved
}
if (props.replicationConfiguration.length !== 1) {
throw new Error('`replicationConfiguration` must contain exactly one destination');
}

props.replicationConfiguration.forEach((config) => {
const { destinationFileSystem, region, availabilityZone, kmsKey } = config;

if (destinationFileSystem && (region || availabilityZone || kmsKey)) {
throw new Error('Cannot configure `replicationConfiguration.region`, `replicationConfiguration.az` or `replicationConfiguration.kmsKey` when `replicationConfiguration.destinationFileSystem` is set');
}
if (region && !Token.isUnresolved(region) && !/^[a-z]{2}-((iso[a-z]{0,1}-)|(gov-)){0,1}[a-z]+-{0,1}[0-9]{0,1}$/.test(region)) {
throw new Error('`replicationConfiguration.region` is invalid.');
}
if (availabilityZone && !Token.isUnresolved(availabilityZone) && !region) {
throw new Error('`replicationConfiguration.availabilityZone` cannot be specified without `replicationConfiguration.region`');
}
});
}

// we explictly use 'undefined' to represent 'false' to maintain backwards compatibility since
// its considered an actual change in CloudFormations eyes, even though they have the same meaning.
const encrypted = props.encrypted ?? (FeatureFlags.of(this).isEnabled(
Expand All @@ -579,6 +648,16 @@ export class FileSystem extends FileSystemBase {
replicationOverwriteProtection: props.replicationOverwriteProtection,
} : undefined;

const replicationConfiguration = props.replicationConfiguration ? {
destinations: props.replicationConfiguration.map(
(config) => ({
fileSystemId: config.destinationFileSystem?.fileSystemId,
kmsKeyId: config.kmsKey?.keyArn,
region: config.destinationFileSystem ? config.destinationFileSystem.env.region : (config.region ?? Stack.of(this).region),
availabilityZoneName: config.availabilityZone,
})),
} : undefined;

this._resource = new CfnFileSystem(this, 'Resource', {
encrypted: encrypted,
kmsKeyId: props.kmsKey?.keyArn,
Expand Down Expand Up @@ -611,6 +690,7 @@ export class FileSystem extends FileSystemBase {
}),
fileSystemProtection,
availabilityZoneName: props.oneZone ? oneZoneAzName : undefined,
replicationConfiguration,
});
this._resource.applyRemovalPolicy(props.removalPolicy);

Expand Down
172 changes: 171 additions & 1 deletion packages/aws-cdk-lib/aws-efs/test/efs-file-system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import { App, RemovalPolicy, Size, Stack, Tags } from '../../core';
import * as cxapi from '../../cx-api';
import { FileSystem, LifecyclePolicy, PerformanceMode, ThroughputMode, OutOfInfrequentAccessPolicy, ReplicationOverwriteProtection } from '../lib';
import { FileSystem, LifecyclePolicy, PerformanceMode, ThroughputMode, OutOfInfrequentAccessPolicy, ReplicationOverwriteProtection, ReplicationConfiguration } from '../lib';

let stack = new Stack();
let vpc = new ec2.Vpc(stack, 'VPC');
Expand Down Expand Up @@ -958,3 +958,173 @@ test.each([
},
});
});

describe('replication configuration', () => {
test('default settings', () => {
// WHEN
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{}],
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EFS::FileSystem', {
ReplicationConfiguration: {
Destinations: [
{
Region: {
Ref: 'AWS::Region',
},
},
],
},
});
});

test('with destination file system', () => {
// WHEN
const destination = new FileSystem(stack, 'DestinationFileSystem', {
vpc,
replicationOverwriteProtection: ReplicationOverwriteProtection.DISABLED,
});
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
destinationFileSystem: destination,
}],
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EFS::FileSystem', {
ReplicationConfiguration: {
Destinations: [
{
FileSystemId: {
Ref: 'DestinationFileSystem12545967',
},
},
],
},
});
});

test('with full settings', () => {
// WHEN
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
kmsKey: new kms.Key(stack, 'customKey'),
region: 'us-east-1',
availabilityZone: 'us-east-1a',
}],
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EFS::FileSystem', {
ReplicationConfiguration: {
Destinations: [
{
Region: 'us-east-1',
AvailabilityZoneName: 'us-east-1a',
KmsKeyId: {
'Fn::GetAtt': [
'customKeyFEB2B57F',
'Arn',
],
},
},
],
},
});
});

test('throw error for read-only file system', () => {
// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
region: 'us-east-1',
}],
replicationOverwriteProtection: ReplicationOverwriteProtection.DISABLED,
});
}).toThrow('Cannot configure `replicationConfiguration` when `replicationOverwriteProtection` is set to `DISABLED`');
});

test.each([
{ region: 'us-east-1' },
{ availabilityZone: 'us-east-1a' },
])('throw error for specifing both destinationFileSystem and other parameters', (config) => {
// WHEN
const destination = new FileSystem(stack, 'DestinationFileSystem', {
vpc,
replicationOverwriteProtection: ReplicationOverwriteProtection.DISABLED,
});

// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
destinationFileSystem: destination,
...config,
}],
});
}).toThrow('Cannot configure `replicationConfiguration.region`, `replicationConfiguration.az` or `replicationConfiguration.kmsKey` when `replicationConfiguration.destinationFileSystem` is set');
});

test('throw error for specifing both destinationFileSystem and kmsKey', () => {
badmintoncryer marked this conversation as resolved.
Show resolved Hide resolved
// WHEN
const destination = new FileSystem(stack, 'DestinationFileSystem', {
vpc,
replicationOverwriteProtection: ReplicationOverwriteProtection.DISABLED,
});

// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
destinationFileSystem: destination,
kmsKey: new kms.Key(stack, 'customKey'),
}],
});
}).toThrow('Cannot configure `replicationConfiguration.region`, `replicationConfiguration.az` or `replicationConfiguration.kmsKey` when `replicationConfiguration.destinationFileSystem` is set');
});

test('throw error for invalid region', () => {
// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
region: 'invalid-region',
}],
});
}).toThrow('`replicationConfiguration.region` is invalid.');
});

test('throw error for specifying availabilityZone without region', () => {
// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration: [{
availabilityZone: 'us-east-1a',
}],
});
}).toThrow('`replicationConfiguration.availabilityZone` cannot be specified without `replicationConfiguration.region`');
});

test.each([
[[]], [[{ region: 'us-east-1' }, { region: 'ap-northeast-1' }]],
])('throw error for invalid length of replicationConfiguration', (replicationConfiguration) => {
// THEN
expect(() => {
new FileSystem(stack, 'EfsFileSystem', {
vpc,
replicationConfiguration,
});
}).toThrow('`replicationConfiguration` must contain exactly one destination');
});
});
Loading