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.

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

[@aws-cdk/aws-ec2] VpnConnection does not support unresolved tokens #11633

Closed
dreamstride opened this issue Nov 23, 2020 · 5 comments
Closed
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@dreamstride
Copy link

I am attempting to create a target group using IP addresses returned from a created construct in the same stack. Unfortunately when attempting to create the target group I'm getting the following error:

The IP address '#{Token[Token.200]}' is not a valid Ipv4 address...

This makes sense because this tokenized string definitely is not an Ipv4 address. However, it seems to me that this validation should happen after the token is resolved.

Reproduction Steps

const broker = new CfnBroker(this 'CfnBroker', {
    ....
});

const targets = broker.attrIpAddresses.map(ip => new IpTarget(ip, port, az))
const targetGroup = new NetworkTargetGroup(this, 'NetworkTargetGroup', {
    ....
    targets: targets
});

What did you expect to happen?

I expected that my construct would be created and the tokens would be resolved/validated after it was created.

What actually happened?

The following error returned and the stack creation failed.
The IP address '#{Token[Token.200]}' is not a valid Ipv4 address...

Environment

  • **CDK CLI Version : 1.74.0
  • **Framework Version: 1.74.0
  • **Node.js Version: v12.18.2
  • **OS : OSX
  • **Language (Version): Typescript (3.7.2)

This is 🐛 Bug Report

@dreamstride dreamstride added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 23, 2020
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Nov 23, 2020
@njlynch
Copy link
Contributor

njlynch commented Nov 24, 2020

The only culprit for this error message appears to be here:

if (!net.isIPv4(props.ip)) {
throw new Error(`The \`ip\` ${props.ip} is not a valid IPv4 address.`);
}

From a quick trace of the code, I'm not sure this is ELB-specific. Are you creating an ec2.VpnConnection directly, or creating an ec2.Vpc with vpnConnections?

Either way, the fix is the same -- just need to add a check for Token.isUnresolved as part of the IPV4 check above.

@njlynch njlynch added @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud and removed @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 labels Nov 24, 2020
@njlynch njlynch assigned rix0rrr and unassigned njlynch Nov 24, 2020
@njlynch njlynch changed the title [@aws-cdk/aws-elasticloadbalancingv2] IpTarget does not support resolved tokens [@aws-cdk/aws-ec2] VpnConnection does not support unresolved tokens Nov 24, 2020
@njlynch njlynch added the effort/small Small work item – less than a day of effort label Nov 24, 2020
@rix0rrr rix0rrr added p2 good first issue Related to contributions. See CONTRIBUTING.md labels Nov 30, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 30, 2020
@jwoehrle
Copy link
Contributor

jwoehrle commented Feb 6, 2021

can we get a status of this?

Either way, the fix is the same -- just need to add a check for Token.isUnresolved as part of the IPV4 check above.

That sounds rather simple... Happy to submit a PR if necessary

@njlynch
Copy link
Contributor

njlynch commented Feb 8, 2021

@jwoehrle - A PR would be greatly appreciated! I agree this should be relatively simple; change the above check to only validate the IP if the string isn't an unresolved Token. Then add a test to validate the new behavior, by using a reference of some kind as the IP input to the VpnConnection.

mergify bot pushed a commit that referenced this issue Feb 9, 2021
Add support to use Token for `VpnConnectionProps.ip` and skip `net.isIPv4(...)` validation in that case.

Fixes issue #11633 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TLadd pushed a commit to TLadd/aws-cdk that referenced this issue Feb 9, 2021
Add support to use Token for `VpnConnectionProps.ip` and skip `net.isIPv4(...)` validation in that case.

Fixes issue aws#11633 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@jwoehrle
Copy link
Contributor

I submitted the PR and it was released as part of 1.89.0.

@deadlymustard can you update to the latest version and try again?

NovakGu pushed a commit to NovakGu/aws-cdk that referenced this issue Feb 18, 2021
Add support to use Token for `VpnConnectionProps.ip` and skip `net.isIPv4(...)` validation in that case.

Fixes issue aws#11633 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@github-actions
Copy link

github-actions bot commented Jun 3, 2022

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 3, 2022
@github-actions github-actions bot closed this as completed Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

No branches or pull requests

5 participants