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-lib: exporting a class that implements IVpc but only looks up a vpc by Id #27615

Open
2 tasks
quaverBit opened this issue Oct 19, 2023 · 5 comments
Open
2 tasks
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud feature-request A feature should be added or improved. p3

Comments

@quaverBit
Copy link

quaverBit commented Oct 19, 2023

Describe the feature

Having a class that is able to lookup a Vpc by it's Id and implementing IVpc so it can be extended with own methods and properties.

Use Case

Doing class MyVpc extends Vpc will try to deploy a new Vpc to aws and that is not intended. I want to be able to extend a class that looks up Vpc by it's id in something like:

class MyVpc extends VpcLookup {
  constructor(scope: Construct, id: string, props?: VpcLookupOptions) {
    super(scope, id, props);
  }

  get someSubnetGroup() {
    this.selectSubnets({
      ...
    })
  }
}

Proposed Solution

Maybe it's possible to move Vpc.fromLookup code/logic to inside of a seperated class constructor?

Other Information

Alternative solution would be export LookedUpVpc?

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2

Environment details (OS name and version, etc.)

linux

@quaverBit quaverBit added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 19, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Oct 19, 2023
@pahud
Copy link
Contributor

pahud commented Oct 20, 2023

Given we already have LookedUpVpc, it's unclear to me why we need a new class in aws-ec2 ? Can you elaborate more on it?

@pahud pahud added p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 20, 2023
@quaverBit
Copy link
Author

quaverBit commented Oct 20, 2023

Thank you for your prompt reply!

We don't need a new class, but consider the following:
First issue is LookedUpVpc is not exported.
so right now it's impossible to do:

import { LookedUpVpc } from "aws-cdk-lib/aws-ec2"

Second issue (or more like improvement) is the arguments in constructor of LookedUpVpc, it would be rather nice to have it the same as Vpc.fromLookup.

Maybe extending the use case would clarity of my intended use,
consider the following class extention:

class MyVpc extends LookedUpVpc {
  constructor(scope: Construct, id: string, props: VpcLookupOptions) { // VpcLookup as in arguments of Vpc.fromLookup
    super(scope, id, props);                                           // this is cunrrently wrong, but it's the desired functionality
  }

  get someSubnetGroup() {
    return this.selectSubnets({
      ...
    })
  }
}

then In my stack I can do something like:

class MyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);

    const exampleVpc = new MyVpc(this, "my-vpc", { vpcId: "exampleVpc" });

    new NodejsFunction(this, "my-function", {
      vpc: exampleVpc,
      vpcSubnets: exampleVpc.someSubnetGroup,
    })
  }
}

I can exand further the utility in creating sharable constructs with intended default properties like:

class MyNodejsFunction extends NodejsFunction {
  constructor(scope: Construct, id: string, props: NodejsFunctionProps ){
    const vpcSubnets = props.vpc instanceof MyVpc ? props.vpc.someSubnetGroup : undefined
    super(scope, id, { vpcSubnets, ...props });
  }
}

As there is no class exported with a looked up vpc, the best approach so far would be something of sorts:

class MyVpc extends Construct {
  private vpc: IVpc;

  constructor(scope: Construct, id: string, vpcId: string) {
    super(scope, id);
    this.vpc = Vpc.fromLookup(this, "vpc-lookup", { vpcId });
  }
}

But as MyVpc is not an implementation of IVpc no longer can be used directly in aws-cdk constructs.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 21, 2023
@gshpychka
Copy link
Contributor

Why not just have your class implement IVpc?

@ashishdhingra
Copy link
Contributor

@quaverBit Please confirm if this feature request is still valid and if you are unblocked.

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 6, 2024
Copy link

github-actions bot commented Jun 9, 2024

This issue has not received a response in a while. 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 the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 9, 2024
@pahud pahud added p3 and removed p2 closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jun 11, 2024
@moelasmar moelasmar added @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud and removed aws-cdk-lib Related to the aws-cdk-lib package labels Aug 28, 2024
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 feature-request A feature should be added or improved. p3
Projects
None yet
Development

No branches or pull requests

5 participants