-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Add EFS transit encryption and authorization config options for ECS task definition #13136
Conversation
Also have code ready for authorization config but depends on #11965 to use an access point Id in acc tests. |
@jukie Thanks for this. |
Sure, I’ll update it now |
New Acctests output:
|
Verified acceptance tests: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSEcsTaskDefinition_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSEcsTaskDefinition_ -timeout 120m
=== RUN TestAccAWSEcsTaskDefinition_basic
=== PAUSE TestAccAWSEcsTaskDefinition_basic
=== RUN TestAccAWSEcsTaskDefinition_withScratchVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withScratchVolume
=== RUN TestAccAWSEcsTaskDefinition_withDockerVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withDockerVolume
=== RUN TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
=== PAUSE TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
=== RUN TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== PAUSE TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== RUN TestAccAWSEcsTaskDefinition_withEFSVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withEFSVolume
=== RUN TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== RUN TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== RUN TestAccAWSEcsTaskDefinition_withEcsService
=== PAUSE TestAccAWSEcsTaskDefinition_withEcsService
=== RUN TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== PAUSE TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== RUN TestAccAWSEcsTaskDefinition_withNetworkMode
=== PAUSE TestAccAWSEcsTaskDefinition_withNetworkMode
=== RUN TestAccAWSEcsTaskDefinition_withIPCMode
=== PAUSE TestAccAWSEcsTaskDefinition_withIPCMode
=== RUN TestAccAWSEcsTaskDefinition_withPidMode
=== PAUSE TestAccAWSEcsTaskDefinition_withPidMode
=== RUN TestAccAWSEcsTaskDefinition_constraint
=== PAUSE TestAccAWSEcsTaskDefinition_constraint
=== RUN TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== PAUSE TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== RUN TestAccAWSEcsTaskDefinition_arrays
=== PAUSE TestAccAWSEcsTaskDefinition_arrays
=== RUN TestAccAWSEcsTaskDefinition_Fargate
=== PAUSE TestAccAWSEcsTaskDefinition_Fargate
=== RUN TestAccAWSEcsTaskDefinition_ExecutionRole
=== PAUSE TestAccAWSEcsTaskDefinition_ExecutionRole
=== RUN TestAccAWSEcsTaskDefinition_Inactive
=== PAUSE TestAccAWSEcsTaskDefinition_Inactive
=== RUN TestAccAWSEcsTaskDefinition_Tags
=== PAUSE TestAccAWSEcsTaskDefinition_Tags
=== RUN TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== PAUSE TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== RUN TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== PAUSE TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== CONT TestAccAWSEcsTaskDefinition_basic
=== CONT TestAccAWSEcsTaskDefinition_withPidMode
=== CONT TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== CONT TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== CONT TestAccAWSEcsTaskDefinition_Tags
=== CONT TestAccAWSEcsTaskDefinition_withIPCMode
=== CONT TestAccAWSEcsTaskDefinition_Inactive
=== CONT TestAccAWSEcsTaskDefinition_withNetworkMode
=== CONT TestAccAWSEcsTaskDefinition_ExecutionRole
=== CONT TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== CONT TestAccAWSEcsTaskDefinition_Fargate
=== CONT TestAccAWSEcsTaskDefinition_withEcsService
=== CONT TestAccAWSEcsTaskDefinition_arrays
=== CONT TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== CONT TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== CONT TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== CONT TestAccAWSEcsTaskDefinition_constraint
=== CONT TestAccAWSEcsTaskDefinition_withEFSVolume
=== CONT TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== CONT TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
--- PASS: TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume (27.77s)
=== CONT TestAccAWSEcsTaskDefinition_withDockerVolume
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig (27.97s)
=== CONT TestAccAWSEcsTaskDefinition_withScratchVolume
--- PASS: TestAccAWSEcsTaskDefinition_inferenceAccelerator (27.99s)
--- PASS: TestAccAWSEcsTaskDefinition_constraint (28.31s)
--- PASS: TestAccAWSEcsTaskDefinition_arrays (28.69s)
--- PASS: TestAccAWSEcsTaskDefinition_withIPCMode (29.49s)
--- PASS: TestAccAWSEcsTaskDefinition_withNetworkMode (29.50s)
--- PASS: TestAccAWSEcsTaskDefinition_withPidMode (29.84s)
--- PASS: TestAccAWSEcsTaskDefinition_ExecutionRole (29.89s)
--- PASS: TestAccAWSEcsTaskDefinition_withTaskRoleArn (29.94s)
--- PASS: TestAccAWSEcsTaskDefinition_ProxyConfiguration (38.56s)
--- PASS: TestAccAWSEcsTaskDefinition_Fargate (38.90s)
--- PASS: TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume (39.97s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolume (40.02s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal (40.15s)
--- PASS: TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource (44.26s)
--- PASS: TestAccAWSEcsTaskDefinition_Inactive (44.54s)
--- PASS: TestAccAWSEcsTaskDefinition_basic (44.55s)
--- PASS: TestAccAWSEcsTaskDefinition_withScratchVolume (23.66s)
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolume (24.33s)
--- PASS: TestAccAWSEcsTaskDefinition_Tags (68.27s)
--- PASS: TestAccAWSEcsTaskDefinition_withEcsService (118.16s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 118.264s |
Now that #11965 is merged I'll update this to include the changes for efs access points. |
Didn't mean to change the reviewer if someone's able to change it back to @terraform-providers/aws-provider |
This looks good. Any idea on how to push this further? I will update with my passing test shortly. EDIT: Tests look good too. ➜ terraform-provider-aws git:(12809-efs-vol-conf) make testacc TESTARGS='-run=TestAccAWSEcsTaskDefinition_' TEST=./aws
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSEcsTaskDefinition_ -timeout 120m
=== RUN TestAccAWSEcsTaskDefinition_basic
=== PAUSE TestAccAWSEcsTaskDefinition_basic
=== RUN TestAccAWSEcsTaskDefinition_withScratchVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withScratchVolume
=== RUN TestAccAWSEcsTaskDefinition_withDockerVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withDockerVolume
=== RUN TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
=== PAUSE TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
=== RUN TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== PAUSE TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== RUN TestAccAWSEcsTaskDefinition_withEFSVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withEFSVolume
=== RUN TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== RUN TestAccAWSEcsTaskDefinition_withEFSAccessPoint
=== PAUSE TestAccAWSEcsTaskDefinition_withEFSAccessPoint
=== RUN TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== PAUSE TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== RUN TestAccAWSEcsTaskDefinition_withEcsService
=== PAUSE TestAccAWSEcsTaskDefinition_withEcsService
=== RUN TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== PAUSE TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== RUN TestAccAWSEcsTaskDefinition_withNetworkMode
=== PAUSE TestAccAWSEcsTaskDefinition_withNetworkMode
=== RUN TestAccAWSEcsTaskDefinition_withIPCMode
=== PAUSE TestAccAWSEcsTaskDefinition_withIPCMode
=== RUN TestAccAWSEcsTaskDefinition_withPidMode
=== PAUSE TestAccAWSEcsTaskDefinition_withPidMode
=== RUN TestAccAWSEcsTaskDefinition_constraint
=== PAUSE TestAccAWSEcsTaskDefinition_constraint
=== RUN TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== PAUSE TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== RUN TestAccAWSEcsTaskDefinition_arrays
=== PAUSE TestAccAWSEcsTaskDefinition_arrays
=== RUN TestAccAWSEcsTaskDefinition_Fargate
=== PAUSE TestAccAWSEcsTaskDefinition_Fargate
=== RUN TestAccAWSEcsTaskDefinition_ExecutionRole
=== PAUSE TestAccAWSEcsTaskDefinition_ExecutionRole
=== RUN TestAccAWSEcsTaskDefinition_Inactive
=== PAUSE TestAccAWSEcsTaskDefinition_Inactive
=== RUN TestAccAWSEcsTaskDefinition_Tags
=== PAUSE TestAccAWSEcsTaskDefinition_Tags
=== RUN TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== PAUSE TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== RUN TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== PAUSE TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== CONT TestAccAWSEcsTaskDefinition_basic
=== CONT TestAccAWSEcsTaskDefinition_inferenceAccelerator
=== CONT TestAccAWSEcsTaskDefinition_Inactive
=== CONT TestAccAWSEcsTaskDefinition_withEcsService
=== CONT TestAccAWSEcsTaskDefinition_arrays
=== CONT TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource
=== CONT TestAccAWSEcsTaskDefinition_constraint
=== CONT TestAccAWSEcsTaskDefinition_withPidMode
=== CONT TestAccAWSEcsTaskDefinition_withIPCMode
=== CONT TestAccAWSEcsTaskDefinition_withNetworkMode
=== CONT TestAccAWSEcsTaskDefinition_withTaskRoleArn
=== CONT TestAccAWSEcsTaskDefinition_withEFSVolume
=== CONT TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume
=== CONT TestAccAWSEcsTaskDefinition_withEFSAccessPoint
=== CONT TestAccAWSEcsTaskDefinition_Tags
=== CONT TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume
=== CONT TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig
=== CONT TestAccAWSEcsTaskDefinition_ProxyConfiguration
=== CONT TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal
=== CONT TestAccAWSEcsTaskDefinition_Fargate
--- PASS: TestAccAWSEcsTaskDefinition_inferenceAccelerator (40.44s)
=== CONT TestAccAWSEcsTaskDefinition_withDockerVolume
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig (40.50s)
=== CONT TestAccAWSEcsTaskDefinition_withScratchVolume
--- PASS: TestAccAWSEcsTaskDefinition_arrays (40.52s)
=== CONT TestAccAWSEcsTaskDefinition_ExecutionRole
--- PASS: TestAccAWSEcsTaskDefinition_constraint (41.54s)
--- PASS: TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume (42.43s)
--- PASS: TestAccAWSEcsTaskDefinition_withIPCMode (43.81s)
--- PASS: TestAccAWSEcsTaskDefinition_withTaskRoleArn (44.38s)
--- PASS: TestAccAWSEcsTaskDefinition_withNetworkMode (44.54s)
--- PASS: TestAccAWSEcsTaskDefinition_withPidMode (44.94s)
--- PASS: TestAccAWSEcsTaskDefinition_ProxyConfiguration (55.94s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolume (56.45s)
--- PASS: TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume (56.78s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal (58.86s)
--- PASS: TestAccAWSEcsTaskDefinition_Fargate (58.91s)
--- PASS: TestAccAWSEcsTaskDefinition_Inactive (67.74s)
--- PASS: TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource (69.27s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSAccessPoint (70.78s)
--- PASS: TestAccAWSEcsTaskDefinition_basic (71.92s)
--- PASS: TestAccAWSEcsTaskDefinition_withScratchVolume (41.32s)
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolume (42.19s)
--- PASS: TestAccAWSEcsTaskDefinition_ExecutionRole (47.46s)
--- PASS: TestAccAWSEcsTaskDefinition_Tags (103.71s)
--- PASS: TestAccAWSEcsTaskDefinition_withEcsService (133.23s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 135.491s
➜ terraform-provider-aws git:(12809-efs-vol-conf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small changes. ill add the provider as reviewer
added @bflad as i cant re-add the provider |
Does anyone here know who could or how to push this further? Seems a pretty requested feature. |
+1 to push this further! This is exactly what I currently need. My options are: wait for this change to be included or find a workaround (maybe create my task definition using CloudFormation). But this would cause some serious changes on my code base. |
Is there something we can do to help releasing it with the next version of the AWS provider? |
I've compiled the PR here and tested attaching an EFS volume to a container that does not run as root and everything went as expected. Access PointVolume created on the Task DefinitionTerraform coderesource "aws_ecs_task_definition" "default" {
family = var.name
container_definitions = var.container_definitions
task_role_arn = var.task_role_arn
execution_role_arn = var.execution_role_arn
network_mode = var.task_network_mode
dynamic "volume" {
for_each = var.efs_volumes
content {
name = volume.value["name"]
efs_volume_configuration {
file_system_id = volume.value["file_system_id"]
root_directory = volume.value["root_directory"]
transit_encryption = volume.value["transit_encryption"] != "ENABLED" ? "" : "ENABLED"
dynamic "authorization_config" {
for_each = volume.value["access_point_id"] != "" ? [1] : []
content {
access_point_id = volume.value["access_point_id"]
iam = volume.value["iam"] != "ENABLED" ? "" : "ENABLED"
}
}
}
}
}
} |
Same for me! |
Hello @bflad, this very useful feature, Any idea when you are going to complete the review. Thanks in advance. |
+1 me too also on hold exactly at this place. For now, as a workaround I would manually adding the this EFS AP block after creating the task from the TF. Then, passing the incremented task version to the service as a variable. I know, it sounds weird and not a good practice, however still easier than moving to CF for this purpose as feature gonna release soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @jukie and everyone involved. Looks great. 🚀
Output from acceptance testing:
--- PASS: TestAccAWSEcsTaskDefinition_arrays (20.05s)
--- PASS: TestAccAWSEcsTaskDefinition_basic (33.04s)
--- PASS: TestAccAWSEcsTaskDefinition_changeVolumesForcesNewResource (32.93s)
--- PASS: TestAccAWSEcsTaskDefinition_constraint (20.00s)
--- PASS: TestAccAWSEcsTaskDefinition_ExecutionRole (20.53s)
--- PASS: TestAccAWSEcsTaskDefinition_Fargate (27.83s)
--- PASS: TestAccAWSEcsTaskDefinition_Inactive (35.23s)
--- PASS: TestAccAWSEcsTaskDefinition_inferenceAccelerator (20.16s)
--- PASS: TestAccAWSEcsTaskDefinition_ProxyConfiguration (32.37s)
--- PASS: TestAccAWSEcsTaskDefinition_Tags (53.63s)
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolume (19.53s)
--- PASS: TestAccAWSEcsTaskDefinition_withDockerVolumeMinimalConfig (19.77s)
--- PASS: TestAccAWSEcsTaskDefinition_withEcsService (55.57s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSAccessPoint (36.35s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolume (32.50s)
--- PASS: TestAccAWSEcsTaskDefinition_withEFSVolumeMinimal (32.62s)
--- PASS: TestAccAWSEcsTaskDefinition_withIPCMode (20.97s)
--- PASS: TestAccAWSEcsTaskDefinition_withNetworkMode (20.95s)
--- PASS: TestAccAWSEcsTaskDefinition_withPidMode (20.92s)
--- PASS: TestAccAWSEcsTaskDefinition_withScratchVolume (19.86s)
--- PASS: TestAccAWSEcsTaskDefinition_withTaskRoleArn (19.80s)
--- PASS: TestAccAWSEcsTaskDefinition_withTaskScopedDockerVolume (20.78s)
--- PASS: TestAccAWSEcsTaskDefinition_withTransitEncryptionEFSVolume (32.56s)
This has been released in version 2.68.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Closes #12809
Release note for CHANGELOG:
Output from acceptance testing: