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

How to safely deploy stacks using AlbToFargate into the same VPC #918

Open
phsiao opened this issue Mar 10, 2023 · 5 comments
Open

How to safely deploy stacks using AlbToFargate into the same VPC #918

phsiao opened this issue Mar 10, 2023 · 5 comments
Labels
Backlog We don't have the bandwidth to support this task right now, but will consider it in the future.

Comments

@phsiao
Copy link

phsiao commented Mar 10, 2023

This is not necessarily a bug but it is not clear to me what the best practices are.

Because AlbToFargate (and other constructs) attaches service endpoint (e.g., ecr, ddb) into the VPC it uses, if you have multiple stacks all using AlbToFargate and share the same vpc only the first stack would succeed because of the endpoints can't be attached again.

What one would have done is to perform the service endpoint attachment in where the VPC is defined, and when importing the VPC into a stack one also places the interfaceTag marker into the VPC construct so the service endpoints are not attached again. This gets around the problem but it does not seem to be a general solution.

Reproduction Steps

Error Log

Environment

  • CDK CLI Version :
  • CDK Framework Version:
  • AWS Solutions Constructs Version :
  • OS :
  • Language :

Other


This is 🐛 Bug Report

@phsiao phsiao added bug Something isn't working needs-triage The issue or PR still needs to be triaged labels Mar 10, 2023
@biffgaut
Copy link
Contributor

Thanks for the note! Have you seen behavior where two identical endpoints are added to the same VPC? When we run the code below only 1 endpoint is created:

export class Issue918Stack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const firstConstruct = new LambdaToSqs(this, 'first', {
      deployVpc: true,
      lambdaFunctionProps: {
         code: lambda.Code.fromAsset(`lambda`),
         runtime: lambda.Runtime.NODEJS_14_X,
         handler: '.handler',
      },
    });

    const secondConstruct = new LambdaToSqs(this, 'second', {
      existingVpc: firstConstruct.vpc,
      lambdaFunctionProps: {
         code: lambda.Code.fromAsset(`lambda`),
         runtime: lambda.Runtime.NODEJS_14_X,
         handler: '.handler',
      },
    });
  }
}

If you've got an example that creates two endpoints of the same type, please share it.

@biffgaut
Copy link
Contributor

Ah - is your point that if I obtain a pre-existing VPC from another stack using something like fromVpcAttributes, then a subsequent construct could add a second endpoint? That could be - it could be the CDK that is preventing the redundancy in my example.

@phsiao
Copy link
Author

phsiao commented Mar 10, 2023

Right. If the two constructs shares the same VPC construct, the invocation of the first construct would place some markers in the VPC construct so the invocation of the second construct would avoid attaching the endpoints again.

But if the second construct does a fromVpcAttributes to import the same VPC, the second construct can't detect the endpoints are already attached and would attempt to attach again and fail.

@biffgaut
Copy link
Contributor

I'm not sure that an answer to this exists in the CDK world. fromVpcAttributes only populates the existing resources defined in VpcAttributes - and the interface has no entry to include endpoints. It would appear that CDK cannot learn about the endpoints of existing VPCs getting pulled into the stack.

.fromLookup might have different behavior, I have never experimented with it.

@biffgaut
Copy link
Contributor

To the best of our knowledge, this cannot be accomplished in the CDK at this time. We'll put it on our backlog on the possibility that this changes in the future.

@biffgaut biffgaut added Backlog We don't have the bandwidth to support this task right now, but will consider it in the future. and removed bug Something isn't working needs-triage The issue or PR still needs to be triaged labels Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog We don't have the bandwidth to support this task right now, but will consider it in the future.
Projects
None yet
Development

No branches or pull requests

2 participants