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-ec2): Allow selecting subnets by Id or CIDR mask #15228

Closed
1 task done
ABevier opened this issue Jun 21, 2021 · 3 comments · Fixed by #15373
Closed
1 task done

(aws-ec2): Allow selecting subnets by Id or CIDR mask #15228

ABevier opened this issue Jun 21, 2021 · 3 comments · Fixed by #15373
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@ABevier
Copy link
Contributor

ABevier commented Jun 21, 2021

I would like the ability to create a SubnetSelection based on the exact subnet ids or based or the CIDR range of subnets in a VPC. While SubnetSelection does allow specifying a list of explicit subnets to use, these use cases are probably common enough support first class without requiring a "custom" filter.

Use Case

I would like this feature because some of the VPC's I'm using have multiple public and private subnets with different CIDR masks and the current subnet selection capability forces me to write my own filters on the list of ISubnets in the vpc and then pass them explicitly to the SubnetSelection.

Proposed Solution

Ideally I would like to do something like this:

    const vpc = Vpc.fromLookup(this, "Vpc", {
      vpcId: props.vpcId,
    });

    const subnetsById: SubnetSelection = {
      subnetFilters: [SubnetFilter.byId('subnet-abc', 'subnet-efg', 'subnet-hij')]
    }

or something like this:

    const vpc = Vpc.fromLookup(this, "Vpc", {
      vpcId: props.vpcId,
    });

    const subnetsByCidr: SubnetSelection = {
      subnetFilters: [SubnetFilter.withCidrMask('26')]
    }
  • 👋 I may be able to implement this feature request

This is a 🚀 Feature Request

@ABevier ABevier added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 21, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Jun 21, 2021
@njlynch
Copy link
Contributor

njlynch commented Jun 24, 2021

Thanks for the feature request -- and to those who've already 👍 it.

Just so I understand the use case, this is functionality equivalent to using SubnetSelection.subnets, with the main difference being that you're selecting by subnet IDs, rather than ISubnets. Is that correct?

The quick (untested) version (as the intermediate workaround) looks like this:

class SubnetIdSubnetFilter extends SubnetFilter {
  constructor(private readonly subnetIds: string[]) {  }
  public selectSubnets(subnets: ISubnet[]): ISubnet[] {
    return subnets.filter(s => this.subnetIds.includes(s.subnetId));
  }
}
const subnetsById: SubnetSelection = {
  subnetFilters: [new SubnetIdSubnetFilter('subnet-abc', 'subnet-efg', 'subnet-hij')]
}

@njlynch njlynch added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 24, 2021
@njlynch njlynch removed their assignment Jun 24, 2021
@ABevier
Copy link
Contributor Author

ABevier commented Jun 24, 2021

Yup, your understanding is correct. It would be nice to make the more common filtering use cases part of the library itself instead of us building our own custom filters.

This is what we've typically been doing:

const subnetIds = ['subnet-abc, 'subnet-efg, 'subnet-hij'];
const subnets = subnetIds.map(subnetId => Subnet.fromSubnetId(this, subnetId, subnetId));
const subnetSelection = { subnets }

@mergify mergify bot closed this as completed in #15373 Jul 30, 2021
mergify bot pushed a commit that referenced this issue Jul 30, 2021
This PR adds a couple of basic SubnetFilters into the CDK directly so they don't have to be needlessly reimplemented.  The `SubnetIdSubnetFilter` filters subnets by ID and the `CidrMaskSubnetFilter` filters subnets based on the CIDR netmask.

closes #15228
----

*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 Aug 3, 2021
This PR adds a couple of basic SubnetFilters into the CDK directly so they don't have to be needlessly reimplemented.  The `SubnetIdSubnetFilter` filters subnets by ID and the `CidrMaskSubnetFilter` filters subnets based on the CIDR netmask.

closes aws#15228
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
This PR adds a couple of basic SubnetFilters into the CDK directly so they don't have to be needlessly reimplemented.  The `SubnetIdSubnetFilter` filters subnets by ID and the `CidrMaskSubnetFilter` filters subnets based on the CIDR netmask.

closes aws#15228
----

*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 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants