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.61k stars 3.91k forks source link

[ecs] EFS Volume configuration is not abstracted #10683

Open justin8 opened 4 years ago

justin8 commented 4 years ago

There's kind of a few related points on this bug, but I figured one issue was the way to go instead of multiple. Happy to split it out though if that's better for the team.

The theme here is that EFS volume configuration is very raw and not abstracted at all in the L2 constructs for task definitions, and some things are just outright incorrect.

  1. Specifying transitEncryption requires the string ENABLED or DISABLED - this should be true/false
  2. If you enable an access point it tells you that you must enable transitEncryption - if this is a requirement of using an access point then it should be enabled automatically in this case
  3. transitEncryption is disabled by default - I could be wrong on this, but I do not see any downside to enabling this by default, and it is a more secure option which would be more in-line with the CDK's mission to implement best practices by default
  4. If an access point is specified, the root directory value will be relative to the directory set for the access point. If specified, transit encryption must be enabled in the EFSVolumeConfiguration. -> If you try to deploy with both an access point and a root directory specified you get an error Invalid request provided: Create TaskDefinition: When using an EFS access point, the root directory must either be set to "/" or be omitted.. So at the very least the documentation is wrong, but we should catch this on the CDK side and error during creation if these are mutually exclusive options. I'd also note that the docs for cloudformation state pretty much the exact error that I get back; which is much clearer.
  5. Enabling iam auth in the authorizationConfig block also advises that transitEncryption is required - Again, this should make it turn on automatically
  6. When specifying an EFS filesystem and access point, you must pass in the IDs themselves, instead of the filesystem/access point objects themselves (e.g. filesystemId: fs.filesystemId instead of filesystem: fs as you'd expect)

Reproduction Steps

What did you expect to happen?

What actually happened?

Environment

Other


This is :bug: Bug Report

kastork commented 3 years ago

Also, I notice that (at least for Fargate), the produced template doesn't add the capability for EC2, which is required to mount a volume.

Generated..

"requiresCompatibilities": [
        "FARGATE"
    ],

Needed...

"requiresCompatibilities": [
        "FARGATE",
        "EC2"
    ],
justin8 commented 3 years ago

I was able to mount it in fargate without setting that?

kastork commented 3 years ago

I guess I'm wrong about that then. Was going by example in https://aws.amazon.com/blogs/containers/developers-guide-to-using-amazon-efs-with-amazon-ecs-and-aws-fargate-part-3/ which does add that compatibility.

I can't get the mount to work with or without it.

related: https://github.com/aws-samples/aws-cdk-examples/issues/359

justin8 commented 3 years ago

This is the code I'm using for a working fargate+EFS service (Using an access point, but it should work fine without that too). So long as both the ECS cluster and EFS filesystem are in the same VPC, you should just need to open the port and force fargate version 1.4, so something like below (this is copy+pasted from a working setup, but kind of smooshed together from 3 separate constructs that talk to each other):

    const NFSPort = ec2.Port.tcp(2049);
    const filesystemSecurityGroup = new ec2.SecurityGroup(
      this,
      "efs-security",
      {
        vpc,
        allowAllOutbound: true,
      }
    );

    const serviceSecurityGroup = new ec2.SecurityGroup(this, "serviceSG", {
      vpc,
      allowAllOutbound: true,
    });

    filesystemSecurityGroup.addIngressRule(serviceSecurityGroup, NFSPort);
    serviceSecurityGroup.addEgressRule(filesystemSecurityGroup, NFSPort);

    const filesystem = new efs.FileSystem(this, "efs", {
      vpc,
      encrypted: true,
      lifecyclePolicy: efs.LifecyclePolicy.AFTER_60_DAYS,
      securityGroup: filesystemSecurityGroup,
      enableAutomaticBackups: true,
    });

    const taskDefinition = new ecs.FargateTaskDefinition(this, "task-def", {
      cpu: CORE_COUNT * 1024,
      memoryLimitMiB: MEMORY_GB * 1024,
      volumes: [
        { name: "logs" },
        { name: "sockets" },
        {
          name: "www",
          efsVolumeConfiguration: {
            fileSystemId: filesystem.fileSystemId,
            transitEncryption: "ENABLED",
          },
        },
      ],
    });

    const service = new ecs.FargateService(this, "service", {
        ...
      securityGroups: [serviceSecurityGroup],
      platformVersion: ecs.FargatePlatformVersion.VERSION1_4,
    }
kastork commented 3 years ago

@justin8 Thanks for this example! Could you clarify what is in your props? there are several references to it...

justin8 commented 3 years ago

Ah sorry. I updated it. I had combined resources from 3 different constructs to give a more concise example and the props naming was not consistent. It should be good now.

kastork commented 3 years ago

@justin8 Thanks, I've got it working.

revmischa commented 3 years ago

Not clear to me how to mount an EFS access point as a volume in ECS

slotnick commented 3 years ago

I wholeheartedly agree with all the points in the original (excellent) post. The experience using EFS with ECS has been uncharacteristically difficult, please give it a little love.