Open mmieluch opened 1 week ago
Hi @mmieluch , thanks for reaching out.
This error is reproducible -
However after looking at the code, here is my analysis -
Tokens represent values that can only be resolved at a later time in the app lifecycle. As seen in the above code block, the token value is evaluated to null (bacause VPC id is not available till deployment) which is causing the error.
In the Readme, its mentioned that .fromSecurityGroupByName()
, result will be written to cdk.context.json and read from there.-
So in that scenario, I think one workaround would be to store this value in SSMParameter store , import in in other stack and then pass it on to fromSecurityGroupByName()
to get the desired group. You could refer to this article for more detailed example.
Let me know if this helps!
OK, thank you for the explanation! In that case, would it be possible to improve the error message to indicate which argument the method has a problem with? I spent quite some time trying to debug, file an issue, etc., and I'm sure I'm not the only one that attempted to tackle this. The error message makes sense in hindsight, but even now I think it's a bit cryptic. It wouldn't be a bugfix, but a quality of life improvement.
Thank you for taking your time to investigate this ticket.
fromLookupByName() actually calls fromLookupByAttributes()
I believe it's actually your VPC is an unresolved token
Token.isUnresolved(options.vpc?.vpcId)
In your original provided code:
const sg2 = SecurityGroup.fromLookupByName(
this,
"TestSecurityGroup",
"sg-1",
network.mainVpc,
);
It's not clear to me where is network.mainVpc
defined so I guess it's network.mainVpc
that is unresolvable.
Why this has to be resolvable? It's because when you just provide the name of the SG, CDK would not be able to lookup the SG ID only with that name. It requires a concrete VPC ID as well. If you scroll down a little bit
CDK would need that VPC ID for the context provider to lookup the real SG ID. What's happening here is that a real SDK call would be invoked. If this VPC ID can't be resolved then the lookup won't succeed with the context provider.
In many cases it would be unresolvable for example:
cdk synth
.Hope it clarifies.
Hi @pahud, massive thanks for the explanation! Like I said earlier, it absolutely makes sense in hindsight. I'm only proposing maybe improving upon the error message given in that situation, because it was not clear from the error I posted that it was in fact the VPC ID (which existed prior to me trying to reference the SG in another stack) that was causing the issue. Not sure if that's even possible or timeworthy. Maybe I was just too hung up on the security group name part of the equasion.
Whatever you decide to do with this ticket and my plea, you have my huge thanks for responding and explaining in such great detail!
@mmieluch No, you are absolutely right and I agree with you. CDK is an open source project and if you found anything that deserve the improvement, you don't need to ask for approval. Just submit a PR for that and it could be reviewed just in 1-2 weeks and hopefully get merged. You would make CDK the way you love.
I guess we could increase the verbosity here
If you haven't contributed to any CDK pull requests yet, we encourage you to check out this blog post and give it a try.
I'll absolutely give it a shot! Thank you for your guidance.
Describe the bug
This piece of code will fail:
with error stating that
The group name in the call is not a token, however, but a hard-coded string.
Regression Issue
Last Known Working CDK Version
No response
Expected Behavior
The snippet from description should correctly resolve the security group if it exists.
Current Behavior
cdk deploy
fails with error:All arguments to look up a security group must be concrete (no Tokens)
Reproduction Steps
Possible Solution
No response
Additional Information/Context
No response
CDK CLI Version
2.159.1
Framework Version
No response
Node.js Version
v20.17.0
OS
Linux arch 6.6.52-1-lts x86_64
Language
TypeScript
Language Version
5.4.5
Other information
No response