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): Fails to create subnet with fromSubnetAttributes without routeTableId #19786

Open
nomike opened this issue Apr 6, 2022 · 14 comments · May be fixed by #31998
Open

(ec2): Fails to create subnet with fromSubnetAttributes without routeTableId #19786

nomike opened this issue Apr 6, 2022 · 14 comments · May be fixed by #31998
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@nomike
Copy link

nomike commented Apr 6, 2022

Describe the bug

When I create a Subnet using ec2.Subnet.from_subnet_attributes and don't specify a routeTableId, I'm getting the following warning:

[Warning at /euc1-dv-rest-alb-alb/subnet-0] No routeTableId was provided to the subnet at 'euc1-dv-rest-alb-alb/subnet-0'. Attempting to read its .routeTable.routeTableId will return null/undefined. (More info: https://github.com/aws/aws-cdk/pull/3171)

While this warning definitely serves it's purpose, I'm missing the option to turn it off or acknowledge it when for example I anyway don't have the intention to read the routeTableId of the subnet.

It is just spamming the console when running synth but also prevents me from using --strict.

Expected Behavior

This warning shouldn't be there. at all.

Current Behavior

cdk synth produces said warning, regardless of whether you're actually trying to read the routeTableId parameter or not.

Reproduction Steps

Just do this in one of your stacks.

ec2.Subnet.from_subnet_attributes(self, id="subnet", subnet_id='subnet-0123456789abcdef0')

Possible Solution

Either the routeTableId Parameter should be mandatory for ec2.Subnet.from_subnet_attributes or if it stays optional (which is probably the better solution) it should not automatically trigger a warning.
Instead synth should fail if you omit the routeTableId parameter and try to read it later. Maybe an error message with similar content could be created if this happens.

Additional Information/Context

The line of code where this happens is https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-ec2/lib/vpc.ts#L2103.

CDK CLI Version

2.19.0 (build e0d3e62)

Framework Version

No response

Node.js Version

v14.19.0

OS

Ubuntu 20.04.3 LTS, Linux VIE-NOMPOS-MOB 5.10.60.1-microsoft-standard-WSL2 #1 SMP Wed Aug 25 23:20:18 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Language

Typescript, Python, .NET, Java, Go

Language Version

not relevant

Other information

No response

@nomike nomike added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 6, 2022
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Apr 6, 2022
@NGL321 NGL321 changed the title (module name): (short issue description) (ec2): Fails to create subnet with fromSubnetAttributes without routeTableId Apr 7, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Apr 7, 2022
@NGL321 NGL321 added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 7, 2022
@NGL321 NGL321 added feature-request A feature should be added or improved. feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. and removed bug This issue is a bug. labels Apr 7, 2022
@NGL321
Copy link
Contributor

NGL321 commented Apr 7, 2022

Hey @nomike,

Thanks for reporting this. I have converted to a feature-request for now since it is intended behavior, but I wonder if this brings up a more large-scale question about disabling warning conditionally (or only enabling them on conditions). This would likely be a larger-scale endeavor though.

@rix0rrr I will pass it off to you since I am uncertain about what this would entail.

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 7, 2022

Only adding the warning when routeTableId is read sounds good. Happy to accept a PR that changes this behavior.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 and removed p1 labels Apr 7, 2022
@rix0rrr rix0rrr removed their assignment Apr 7, 2022
@jordan-brough
Copy link
Contributor

jordan-brough commented Jun 14, 2022

This happens with ec2.Subnet.fromSubnetId as well, which doesn't even have an option to specify a routeTableId.

My current terrible workaround to avoid warning spam:

const subnet = cdk.aws_ec2.Subnet.fromSubnetId(this, 'Subnet', 'subnet-XXX');
// hack to avoid warning spam
subnet.node._metadata = subnet.node._metadata.filter((item) =>
  item.type !== 'aws:cdk:warning' || !/No routeTableId was provided to the subnet/.test(item.data)
);

It would be great to have the warning (or an exception) only when routeTableId is read.

@nomike
Copy link
Author

nomike commented Jun 15, 2022 via email

@DesAWSume
Copy link

This happens with ec2.Subnet.fromSubnetId as well, which doesn't even have an option to specify a routeTableId.

My current terrible workaround to avoid warning spam:

const subnet = cdk.aws_ec2.Subnet.fromSubnetId(this, 'Subnet', 'subnet-XXX');
// hack to avoid warning spam
subnet.node._metadata = subnet.node._metadata.filter((item) =>
  item.type !== 'aws:cdk:warning' || !/No routeTableId was provided to the subnet/.test(item.data)
);

It would be great to have the warning (or an exception) only when routeTableId is read.

Do you have a Python way to hack this?

Doesn't seem like I can modify the metadata

@peterwoodworth
Copy link
Contributor

I don't see the point in this waning at all

Lots of customers frequently get tripped up by their imported resources having undefined values, usually due to expecting CDK to look the values up itself.

This warning has been here for a long time, and we have other warnings similar to this - but usually only when the value would be read. Personally I'd be perfectly ok with removing this warning altogether, because we're actively working on documentation which will reduce confusion surrounding imported resources. Once that new documentation is live we could remove this warning. In the meantime, we'd definitely still accept a PR which only throws this warning when the routeTableId is read

@DesAWSume Check out our Python examples of escape hatches

@petejewels
Copy link

Using dotnet , having same issue:

TaskSubnets = new SubnetSelection
                    {
                        Subnets = new[] {
                           Subnet.FromSubnetId(this, "subnet1", "subnet-64564"),
                            Subnet.FromSubnetId(this, "subnet2", "subnet-987987")
                        },
                        
                    },

If we are given warning we should have ability to add a route table id to theFromSubnetId these subnets use....

@entest-hai
Copy link

entest-hai commented Jul 8, 2023

Having the same issue when specify subnets from theirs ids

aws_ec2.Subnet.fromSubnetId

@TakaakiFuruse
Copy link
Contributor

TakaakiFuruse commented Aug 3, 2023

I just got the warning by simply doing this.

ec2.Subnet.from_subnet_id(
    scope=scope,
    id="my_subnet1",
    subnet_id="subnet-xxxxx",
)

Getting and ignoring any kind of warning is not good for our health, so I am very happy to create a pull request.

How should I fix it?

  1. How about suggesting developers to use fromSubnetAttributes not fromSubnetId through new documentation or new warning(or info?), since fromSubnetId is just a wrapper of fromSubnetAttributes ?

  2. Rather than sending warning, why not send info instead? We can create the resource but just get null/undefined, if no routeTableId was not supplied, so info sounds better for me.

Any ideas?

@whereisaaron
Copy link

we'd definitely still accept a PR which only throws this warning when the routeTableId is read

Both Subnet.FromSubnetAttributes and Subnet.FromSubnetId generate the same warning unless you supply an unnecessary route table id. I agree referencing the route table id should be the warning.

@dontirun
Copy link
Contributor

Better "workaround" with the recent addition of addWarningsV2

    const subnet = ec2.Subnet.fromSubnetId(
      this,
      'subnet',
      'subnet-01234567890abcdef'),
    );
    cdk.Annotations.of(subnet).acknowledgeWarning('@aws-cdk/aws-ec2:noSubnetRouteTableId');

@guidospadavecchia
Copy link

Better "workaround" with the recent addition of addWarningsV2

    const subnet = ec2.Subnet.fromSubnetId(
      this,
      'subnet',
      'subnet-01234567890abcdef'),
    );
    cdk.Annotations.of(subnet).acknowledgeWarning('@aws-cdk/aws-ec2:noSubnetRouteTableId');

Any chance to translate this for .NET?

Copy link

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@github-actions github-actions bot added p1 and removed p2 labels Mar 31, 2024
@WhiteAutumn
Copy link

This issue was created from missing an option to acknowledge the warning. With the addition of addWarningsV2 @dontirun mentioned, this issue can probably be closed.

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 effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet