aws-cdk: aws-ecs: wrong cfnspec for EFS volume configuration (casing issue)
CDK currently generates two resources with casing mistakes when EFSVolumeConfiguration is specified for task definitions.
EfsVolumeConfiguration should be
EFSVolumeConfiguration
FileSystemId should be
FilesystemId
The corresponding test is also wrong: https://github.com/aws/aws-cdk/blob/23986d70c5cd69ce212b5ffdc1bcf059f438f15b/packages/%40aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts#L1192
Reproduction Steps
The following (incomplete) snippet recreates the issue:
const efs = new FileSystem(this, 'etcdata', {
vpc: service.cluster.vpc,
});
taskDefinition.addVolume({
name: 'etcdata',
efsVolumeConfiguration: {
fileSystemId: efs.fileSystemId
},
});
This is based on the ApplicationLoadBalancedFargateService pattern.
The above generates this CFN JSON:
"Volumes": [
{
"Name": "etcdata",
"EfsVolumeConfiguration": {
"FileSystemId": {
"Ref": "etcdata80702D7D"
}
}
}
]
Which then yields CFN errors:
message: #/Volumes/0: extraneous key [EfsVolumeConfiguration] is not permitted
And if I fix the above to use the correct case manually, then later it uncovers the 2nd problem:
#/Volumes/0/EFSVolumeConfiguration: required key [FilesystemId] not found
Fixing both manually causes the template to be deployed successfully.
Environment
- CDK CLI Version : 1.107.0 (build 52c4434)
- Framework Version: ??
- Node.js Version: v16.2.0
- OS : OSX
- **Language (Version): typescript 4.3.2 -->
This is đ Bug Report
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 22
- Comments: 31 (5 by maintainers)
Commits related to this issue
- fix(ecs): get rid of EFS casing warnings About a year ago, ECS TaskDefinition handler changed the casing of some EFS-related properties: * `EfsVolumeConfiguration` -> `EFSVolumeConfiguration` * `Fil... — committed to aws/aws-cdk by rix0rrr 2 years ago
- fix(ecs): get rid of EFS casing warnings (#19681) About a year ago, ECS TaskDefinition handler changed the casing of some EFS-related properties: * `EfsVolumeConfiguration` -> `EFSVolumeConfiguratio... — committed to aws/aws-cdk by rix0rrr 2 years ago
- fix(ecs): get rid of EFS casing warnings (#19681) About a year ago, ECS TaskDefinition handler changed the casing of some EFS-related properties: * `EfsVolumeConfiguration` -> `EFSVolumeConfiguratio... — committed to StevePotter/aws-cdk by rix0rrr 2 years ago
Still not solved in v1.116.0 đ˘
Any update on this? Bug still there with CDK v2.8
TLDR: This validation message is a warning, if youâre stack is failing to deploy itâs because of another problem. If itâs unclear in the cloudformation console check out ECS console and look at your stopped tasks during the stack deploy to find the error messaging. We will fix this in 2.0!
So we know about the incorrect casing on these properties, but it doesnât cause the stack deploy to fail. When reproducing this I got this message in the CFN console as well, but it didnât fail my deploy. What is most likely happening (what happened to me and took me a bit to figure out) is that youâre missing the
fs.connections.allowDefaultPortFrom(service);which causes your tasks to fail the healthcheck withResourceInitializationError: failed to invoke EFS utils. It will keep redeploying these containers for like 45 minutes before timing out and the stack deploy fails. You can look in the ECS console during the deploy and find the service, then click thetaskstab, then click thestoppedfilter. If you have stopped tasks there, you can click on them to see the details and this is where youâll see the error.Here is a minimal stack that I was able to deploy.
Click Me!
Long story, but what happened is a contributor edited the CloudFormation spec manually to add these properties so they could use EFS with ECS. We ingest the new cloudformation spec once a week, but sometimes there are bugs that cause this to take a little longer and without knowing that this was automated, they made this change to unblock themselves and we mistakenly accepted the PR. This is where the properties with the bad casing were ingested. https://github.com/aws/aws-cdk/pull/8467
Tests showed that stacks deployed correctly with this initial PR.
Then, a couple weeks later when we ingested the new spec with these properties included, we saw that they were in fact incorrectly cased. So if we changed the casing, this would break users who already had stacks deployed using this casing. âBreakâ as in break backwards compatibility because they would have to update their stack code to use the correct property names. Instead we patched the CFN spec so we wouldnât break users.
Regardless we should fix this, and we have a v2.0 in progress that gives us the opportunity to do so. Iâm keeping this open for @SoManyHs and @madeline-k to track removing this patch for 2.0.
A hack approach to remove those warnings
I have also experienced this bug with the same symptoms and reproducibility. This is a significant issue for those needing to mount EFS volumes to a garbage container.
One month later this is still not fixed. Can we expect a solution?
Still present in 2.24.1
I can confirm that the bug is still there with CDV v2.12
Me tooâŚ
Chiming in that for me the error message did not cause any problems and EFS still mounted correctly to my fargate container.
Hereâs a gist https://gist.github.com/jeznag/623132d2a98eff5392acd702702f1ef7
The fix involves a change like this - https://github.com/aws/aws-cdk/commit/dd308b6610a670f0dd91df2476253cffb4cb04a3 - but behind a feature flag to avoid resource replacements on stacks where this already works.
@MrArnoldPalmer so OK, maybe there is another error but itâs got to be EFS-related, because if I replace
efsVolumeConfiguration()withhost()my stack deploys just fine.But I havenât been able to find that specific error yet.