Skip to content

aws-efs: FileSystem deployment fails if vpcSubnets are referenced from existing subnets #33876

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.

Sign up for GitHub

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

Jump to bottom
Closed
vumdao opened this issue Mar 22, 2025 · 3 comments · Fixed by #34041
Closed

aws-efs: FileSystem deployment fails if vpcSubnets are referenced from existing subnets #33876

vumdao opened this issue Mar 22, 2025 · 3 comments · Fixed by #34041
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@vumdao
Copy link

vumdao commented Mar 22, 2025 •
edited
Loading

Describe the feature

Code

    const efs = new FileSystem(this, 'FileSystem', {
      vpc,
      encrypted: true,
      lifecyclePolicy: LifecyclePolicy.AFTER_7_DAYS,
      vpcSubnets: { subnets: [ Subnet.fromSubnetAttributes(this, subnetId, {subnetId: subnetId, availabilityZone: az}) ] },
      removalPolicy: RemovalPolicy.DESTROY,
    });

Error:

Error: Resolution error: ID components may not include unresolved tokens: EfsMountTarget-${Token[TOKEN.1221]}.
Object creation stack:
  at Execute again with CDK_DEBUG=true to capture stack traces
    at makeUniqueId (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/private/uniqueid.js:1:660)
    at orgStack.allocateLogicalId (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/stack.js:1:19234)
    at orgStack.getLogicalId (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/stack.js:1:8662)
    at CfnMountTarget.synthesizeLogicalId (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/cfn-element.js:2:687)
    at Object.produce (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/cfn-element.js:1:915)
    at LazyString.resolve (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/lazy.js:1:4176)
    at RememberingTokenResolver.resolveToken (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/resolvable.js:1:1401)
    at RememberingTokenResolver.resolveToken (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:4116)
    at resolve (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:2747)
    at Object.resolve (/Users/dev/workspace/github/org/core/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:1115)```

Related: https://github.com/aws/aws-cdk/pull/26155 where I see it uses `EfsMountTarget-${subnet.node.id}` as the stack ID of mount target

### Use Case

Create FileSystem with subnets from references

### CDK version used

2.185.0

### Environment details (OS name and version, etc.)

macOS Sequoia 15.3.2
@vumdao vumdao added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2025
@github-actions github-actions bot added the @aws-cdk/aws-efs Related to Amazon Elastic File System label Mar 22, 2025
@pahud
Copy link
Contributor

pahud commented Mar 24, 2025 •
edited
Loading

Current/Expected Behavior

Image

Affected Source Code

The issue occurs in packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts, specifically in the FileSystem constructor where mount targets are created. Here's the problematic code section:

aws-cdk/packages/aws-cdk-lib/aws-efs/lib/efs-file-system.ts

Line 867 in 2dc5d70

`EfsMountTarget-${subnet.node.id}`,

The problem occurs when:

  1. The useMountTargetOrderInsensitiveLogicalID feature flag is enabled (default in CDK v2)
  2. A subnet is imported using Subnet.fromSubnetAttributes
  3. The code then tries to use subnet.node.id in the mount target ID, but it contains unresolved tokens

Possible Fix

// We now have to create the mount target for each of the mentioned subnet

// we explicitly use FeatureFlags to maintain backwards compatibility
const useMountTargetOrderInsensitiveLogicalID = FeatureFlags.of(this).isEnabled(cxapi.EFS_MOUNTTARGET_ORDERINSENSITIVE_LOGICAL_ID);
this.mountTargetsAvailable = [];
if (useMountTargetOrderInsensitiveLogicalID) {
  let mountTargetCount = 0; // Fallback counter for cases with tokens
  subnets.subnets.forEach((subnet) => {
    // Check if subnet.node.id contains tokens or if we're dealing with an imported subnet
    const containsTokens = Token.isUnresolved(subnet.node.id);
    const mountTargetId = containsTokens 
      ? `EfsMountTarget-${++mountTargetCount}` // Fallback to counter-based ID if tokens present
      : `EfsMountTarget-${subnet.node.id}`;    // Use subnet.node.id when safe
      
    const mountTarget = new CfnMountTarget(this,
      mountTargetId,
      {
        fileSystemId: this.fileSystemId,
        securityGroups: Array.of(securityGroup.securityGroupId),
        subnetId: subnet.subnetId,
      });
    this._mountTargetsAvailable.add(mountTarget);
  });
} else {
  let mountTargetCount = 0;
  subnets.subnetIds.forEach((subnetId: string) => {
    const mountTarget = new CfnMountTarget(this,
      'EfsMountTarget' + (++mountTargetCount),
      {
        fileSystemId: this.fileSystemId,
        securityGroups: Array.of(securityGroup.securityGroupId),
        subnetId,
      });
    this._mountTargetsAvailable.add(mountTarget);
  });
}

The fix adds logic to:

  1. Check if subnet.node.id contains unresolved tokens using Token.isUnresolved()
  2. If it contains tokens, fallback to a counter-based ID approach (similar to the non-feature flag case)
  3. Only use the subnet's node.id in the ID when it's safe to do so (doesn't contain tokens)

Making this a p1 and we welcome PRs.

@pahud pahud added bug This issue is a bug. p1 effort/medium Medium work item – several days of effort and removed feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 24, 2025
@github-actions github-actions bot mentioned this issue Apr 1, 2025
Open
@phuhung273 phuhung273 mentioned this issue Apr 4, 2025
Merged
1 task
@mergify mergify bot closed this as completed in #34041 Apr 28, 2025
@mergify mergify bot closed this as completed in 20df8fb Apr 28, 2025
@github-actions GitHub Actions
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2025
shikha372 pushed a commit that referenced this issue May 7, 2025
@phuhung273 @shikha372
fix(efs): support imported subnet (#34041)
5bf33e5 
### Issue # (if applicable)

Closes #33876

### Description of changes
LogicalId generation cover unresolved token case.

### Description of how you validated changes
Unit + Integ

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
@aws-cdk/aws-efs Related to Amazon Elastic File System bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(efs): support imported subnet phuhung273/aws-cdk
2 participants
@pahud @vumdao