Skip to content
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

(aws-cdk-lib): Aspects doesn't pick up resources that have been symlinked #18778

Closed
bestickley opened this issue Feb 2, 2022 · 4 comments
Closed

(aws-cdk-lib): Aspects doesn't pick up resources that have been symlinked #18778

bestickley opened this issue Feb 2, 2022 · 4 comments
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@bestickley
Copy link

What is the problem?

Aspects doesn't pick up resources that have been symlinked

Reproduction Steps

Follow steps in README here: https://github.com/bestickley/cdk-aspects-error

What did you expect to happen?

Aspects should throw error picking up bucket

What actually happened?

Aspects doesn't throw error picking up bucket.

CDK CLI Version

2.10.0

Framework Version

2.10.0

Node.js Version

16.13.2

OS

macOS

Language

Typescript

Language Version

3.9.7

Other information

No response
@bestickley bestickley added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 2, 2022
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Feb 2, 2022
@ryparker ryparker added the p1 label Feb 2, 2022
@apriceaws
Copy link

apriceaws commented Feb 11, 2022 •
edited

Hi, I recently ran into this problem and conducted a very similar test:

class AspectTest implements cdk.IAspect {
    public visit(node: any): void {
        if(node.hasOwnProperty("cfnResourceType")){
            console.log(`${node instanceof CfnResource} ${node.cfnResourceType} ${node.node.path}`)
        }
    }
}

I've discovered that the Aspect does actually visit all resources, but the 'instanceof' always evals to false if the construct was created in another library in my lerna monorepo.

@apriceaws
Copy link

I was able to successfully run my test with expected results (node instanceof CfnResource evals to true) by pushing my shared libs from my mono repo into proper NPM packages (instead of relying on symlinks created by lerna bootstrap).

@corymhall corymhall mentioned this issue Mar 17, 2022
Closed
rix0rrr added a commit that referenced this issue Mar 21, 2022
@rix0rrr
fix(core): Aspects from symlinked modules are not applied
4011448 
When construct libraries are purposely symlinked (as opposed of
collectively `npm install`ed), depending on how this is done they may
end up with multiple copies of `aws-cdk-lib`. If that happens, Aspects
from a different `aws-cdk-lib` copy than the one providing `App` will
not be applied.

The reason is the use of `Symbol('...')` instead of `Symbol.for('...')`
to keep the list of aspects on the construct object.

- The first version creates a unique symbol per library, while adding
  a naming hint.
- The second version deduplicates: all symbols with the same naming
  hint will receive the same symbol.

The second version is necessary to make sure that different copies
of the `aws-cdk-lib` library store their aspects under the same key.

Fixes #18921, #18778, #19390.
@rix0rrr rix0rrr mentioned this issue Mar 21, 2022
Merged
@madeline-k madeline-k added effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2022
@madeline-k madeline-k removed their assignment Mar 22, 2022
mergify bot pushed a commit that referenced this issue Mar 22, 2022
@rix0rrr
fix(core): Aspects from symlinked modules are not applied (#19491)
eaeaed7 
When construct libraries are purposely symlinked (as opposed of
collectively `npm install`ed), depending on how this is done they may
end up with multiple copies of `aws-cdk-lib`. If that happens, Aspects
from a different `aws-cdk-lib` copy than the one providing `App` will
not be applied.

The reason is the use of `Symbol('...')` instead of `Symbol.for('...')`
to keep the list of aspects on the construct object.

- The first version creates a unique symbol per library, while adding
  a naming hint.
- The second version deduplicates: all symbols with the same naming
  hint will receive the same symbol.

The second version is necessary to make sure that different copies
of the `aws-cdk-lib` library store their aspects under the same key.

Fixes #18921, #18778, #19390, #18914
@bestickley
Copy link
Author

This works now for me!

@github-actions
Copy link

github-actions bot commented May 2, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ryparker @bestickley @apriceaws @madeline-k