fix(efs): FileSystem deployment fails if vpcSubnets is changed#26155
fix(efs): FileSystem deployment fails if vpcSubnets is changed#26155mergify[bot] merged 2 commits intoaws:mainfrom
Conversation
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
cc46389 to
42abb22
Compare
d65c0ca to
4e66d80
Compare
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
2 similar comments
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
e6274d8 to
e54b414
Compare
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
| }); | ||
| }); | ||
|
|
||
| test('mountTargetOrderInsensitiveLogicalId flag is true', () => { |
There was a problem hiding this comment.
Can you update the test to use custom subnets to check that the subnet ID is used as a suffix?
For example:
new FileSystem(customVpc, 'EfsFileSystem', {
vpc: customVpc,
vpcSubnets: { subnets: [ec2.Subnet.fromSubnetId(stack, 'SubnetExample', 'subnet-1')] },
});
There was a problem hiding this comment.
@lpizzinidev hello, when checking subnetID from created VPC, SubnetID attr tokenised and hard to correlate with template output in this test.
I think with this PR, we're actually not passing SubnetID attribute to MountTarget's logical IDs, we're passing ${subnet.node.id} which i believe sort of concatenation like ${subnetgroupname}Subnet${index}. So i'm a bit unsure passing subnet selection to FileSystem Contruct. Did you want me to do this because of some future changes that might have happened in VPC definitions in this repository cause test failure?
Isn't it a good method that checking generated logical ids of mount target from resulting different cfn templates regarding to this feature flag?
thank you for your review.
There was a problem hiding this comment.
Thanks for the clarification.
I probably interpreted the test case in the wrong way.
lpizzinidev
left a comment
There was a problem hiding this comment.
Thanks for opening a new PR! 👍
Looks good overall, just a small change on the test.
fix(aws-efs): change logical id for EfsMountTarget fix(efs): recreate integ.ecs-job-definition fix(efs): recreate integ.ecs-job-definition fix(efs): recreate integ.ecs-job-definition fix(aws-efs): change logical id for EfsMountTarget Revert "chore(release): 2.87.0" This reverts commit 2879e47.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Changing Logical IDs for EfsMountTarget
While using the forEach, index will be added as a suffix to logical id of EfsMountTarget
this is causing an error when the subnets array in the props has changed.
Closes #25099
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license