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

(ec2.Vpc): Vpc.fromLookup should throw if subnet group name tag is explicitly given and does not exist #13962

Closed
1oglop1 opened this issue Apr 2, 2021 · 4 comments · Fixed by #18714
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@1oglop1
Copy link

1oglop1 commented Apr 2, 2021

I suspect this may be a bug or I'm missing something.

I have an existing VPC with 6 subnets with a tag subnet_tier

AZ1 AZ2 TAG: subnet_tier
public_subnet_1 public_subnet_2 public
private_subnet_1 private_subnet_2 private
data_subnet_1 data_subnet_2 data

From my understanding of Vpc.from_lookup I can supply subnet_group_name_tag will assign the subnets to the correct group inside cdk.context.json
So I can then create a SubnetSelection expected by multiple resources (eg. Function)
So when I add following code in my stack I get an error.

vpc = Vpc.from_lookup(
                self,
                "VPC",
                vpc_id="vpc-123456",
                subnet_group_name_tag="subnet_tier",
            )
subnets = vpc.select_subnets(subnet_group_name='data').subnets

PythonFunction(
    self,
    "Function",
    ...
    vpc=vpc,
    vpc_subnets=subnets
)

Actual result

cdk.context.json is created with SubnetGroups

"subnetGroups": [
      {
        "name": "Private",
        "type": "Private",
       },
             {
        "name": "public",
        "type": "Public",
        }
]

Error message raised:

jsii.errors.JSIIError: You cannot reference a Subnet's availability zone if it was not supplied. Add the availabilityZone when importing using Subnet.fromSubnetAttributes()

Expected result

"subnetGroups": [
      {
        "name": "private",
        "type": "Private",
       },
      {
        "name": "data",
        "type": "Private",
       },
            {
        "name": "public",
        "type": "Public",
        }
]

No errors.

Environment

  • CDK CLI Version: 1.96.0 (build 39f3df8)
  • Module Version: ec2
  • Node.js Version: N/A python CDK 1.96.0
  • OS: macOS
  • Language (Version): Python (3.8.6)
@1oglop1 1oglop1 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 2, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Apr 2, 2021
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 6, 2021

subnets = vpc.select_subnets(subnet_group_name='data').subnets

PythonFunction(
    self,
    "Function",
    ...
    vpc=vpc,
    vpc_subnets=SubnetSelection(
        subnets=subnets
    )
)

This code does duplicate work. I wonder if not doing duplicate work will fix it:

PythonFunction(
    self,
    "Function",
    ...
    vpc=vpc,
    vpc_subnets=SubnetSelection(
        subnet_group_name='data'
    )
)

@rix0rrr rix0rrr added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 6, 2021
@1oglop1
Copy link
Author

1oglop1 commented Apr 8, 2021

@rix0rrr Sorry I made a typo in vpc_subnets=... I tried many variations to make it work.
On top of that I found out that the tags were not properly assigned to all subnets.
data and private had tag subnet-tier while public had subnet_tier.
However I still think there is an undesired behaviour:

I supplied the wrong tag the context was created with Private and Public subnets. And I got error that data subnet is not there instead I'd expect error that tag subnet-tier does not exist

Also this is very confusing:

ec2.SubnetSelection vs ec2.SelectedSubnets

I also updated to 1.97.0 and can confirm it now works as expected!
You can close the issue unless you find it relevant to investigate my report of undesired behaviour

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 9, 2021
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 12, 2021

SelectedSubnets is what you get from doing a SubnetSelection.

@rix0rrr rix0rrr added the p1 label Apr 12, 2021
@rix0rrr rix0rrr changed the title (ec2.Vpc): Vpc.fromLookup fails select subgroup from private subnets (ec2.Vpc): Vpc.fromLookup should throw if subnet group name tag is explicitly given and does not exist Apr 12, 2021
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md labels Apr 12, 2021
@ryparker ryparker removed the needs-triage This issue or PR still needs to be triaged. label Jun 2, 2021
@rix0rrr rix0rrr assigned njlynch and unassigned rix0rrr Jun 3, 2021
@mergify mergify bot closed this as completed in #18714 Jan 31, 2022
mergify bot pushed a commit that referenced this issue Jan 31, 2022
…licitly given and does not exist (#18714)

Currently if `subnetGroupNameTag` is provided in `Vpc.fromLookup()` and
a tag with that key does not exist, the error that is returned is very
generic and just indicates that the VPC could not be found. This makes
it very hard to troubleshoot what the real issue is (invalid
subnetGroupNameTag).

Now if the user provides a `subnetGroupNameTag` and a tag with that Key
does not exist an error is thrown indicating that an invalid
`subnetGroupNameTag` was provided

fixes #13962


----

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

⚠️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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…licitly given and does not exist (aws#18714)

Currently if `subnetGroupNameTag` is provided in `Vpc.fromLookup()` and
a tag with that key does not exist, the error that is returned is very
generic and just indicates that the VPC could not be found. This makes
it very hard to troubleshoot what the real issue is (invalid
subnetGroupNameTag).

Now if the user provides a `subnetGroupNameTag` and a tag with that Key
does not exist an error is thrown indicating that an invalid
`subnetGroupNameTag` was provided

fixes aws#13962


----

*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 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. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants