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

Best practices for cross-stack CDK to CFN and vice versa #603

Closed
mipearson opened this issue Aug 19, 2018 · 17 comments
Closed

Best practices for cross-stack CDK to CFN and vice versa #603

mipearson opened this issue Aug 19, 2018 · 17 comments
Assignees
Labels
docs/guide Related to the developer guide effort/medium Medium work item – several days of effort

Comments

@mipearson
Copy link

I wanted to see whether something was at all possible - whether a VPC created using a CDK construct could be used with a LoadBalancer created via cloudformation and vice versa.

This is important for me as:

  • the constructs currently aren't fine grained enough for us to use instead of cloudformation primitives in some cases (eg, no accessLoggingPolicy on the load balancer)
  • we may want to use our existing cloudformation-based VPCs with CDK constructs

The example is here: https://gist.github.com/mipearson/aeaf303b0770c25f8b5f6e360594cfbf

Is this what is recommended for solving this sort of problem?

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 20, 2018

Hi @mipearson,

There are a lot of dimensions to your question, and it kind of depends on what you want to do exactly. I just wrote a whole response assuming you meant to share resources between CDK apps and plain CloudFormation templates, but upon rereading your comment and code I now realize you might mean sharing resources between higher-level CDK constructs and lower-level CDK constructs (i.e., use a "higher-level" VPC construct with some lower-level direct CloudFormation resources).

I will post both of my responses below.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 20, 2018

Sharing between CDK apps and CloudFormation templates

You got it. If there is information that needs to be shared between stacks, the mechanisms we have are Outputs & Parameters, and Exports & Fn::ImportValues.

We could also use SSM Parameter Store values, which work much like Exports but without the "foreign key constraints" that Exports bring.

CDK to CloudFormation

If you define a VPC inside a CDK app and want to use it from a CFN template, it actually functions much the same as how you would share the template between plain CFN templates. You would output/export in the one template and parameter/import in the other.

The exporting works by calling vpc.export() inside your CDK app. What that does is create Exports for all attributes of your VPC that another CDK stack would need to import it again... but those outputs and exports are available to any CFN stack as well! Deploy a template and pick your favorite method of getting the VPC information into your template.

If you're unhappy about the default names of the Exports (understandable since they are designed to be consumed transparently), you're free to add some new Output()s to your CDK stack, which translate directly into CloudFormation Outputs and can be made into Exports as well.

CloudFormation to CDK

So you already have an existing VPC (deployed through CloudFormation or otherwise) that you want to consume in a CDK application. As you figured out, what you want to do is get a VpcNetworkRef instance from VpcNetworkRef.import(), which expects a number of properties (https://awslabs.github.io/aws-cdk/refs/_aws-cdk_aws-ec2.html#@aws-cdk/aws-ec2.VpcNetworkRefProps):

vpcId, availabilityZones, publicSubnetIds, privateSubnetIds

Again, use your favorite way of getting those values in there. You now have 3 options:

  • CloudFormation Parameters--add a new Parameter() to your Stack and use that as the value (but you're now responsible of specifying the parameter when deploying your synthesized template, which you can no longer do through the CDK toolkit).
  • CloudFormation Imports--use a new FnImportValue() expression with the name of the existing export for your VPC.
  • Synthesis-time parameters: not ideal in all cases, but you can choose to pass in the concrete values when RUNNING the CDK app (either as context values, or as a parameter to your constructs that are hardcoded into main.ts with different values for every account/region, for example) so the CloudFormation template comes out with the identifiers already filled in.

Of all these, Exports and Imports will give you the most transparent experience.

And from your example, I love how you abstract away the importing of the VPC inside a VpcCFNDemoStack class. For consumers, it is totally awesome to be able to write:

const vpc = OurStandardVPC.obtain(this);

new ThingThatNeedsAVPC(..., { vpc });

Or similar, and not have to worry where the VPC is coming from. It might be constructed on the spot, it might be loaded from another environment.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 20, 2018

Sharing between higher-level and lower-level Constructs

If this is what you're trying to do, it depends on how you want to deploy: in a single stack or across multiple stacks.

Multiple stacks

If it's across multiple stacks, the solution will be basically the same as what I described in my previous post, except the CloudFormation template will not be handwritten but generated by CDK. The mechanism used will be the same.

To make matters simpler, in the consuming stack you could forego the VpcNetworkRef.import() and just use the properties of VpcNetworkRefProps directly; you probably don't need the logic built into the VpcNetwork class anymore anyway.

Single stack

This would be even easier, because you can simply access the properties of VpcNetwork directly, such as vpc.vpcId.

@mipearson
Copy link
Author

So ... I kind of mean both! :)

As in if we start migrating from our existing solution to aws-cdk we're going to need to both go CFN to CDK (ie, to refer to a VPC defined elsewhere) and CDK to CDK w/ primitives.

Thanks for the feedback, good to know I'm on the right track. If you'd like to use what I've got in the gist as an example be my guest, let me know if you need me to sign a CLA or anything like that.

@mipearson
Copy link
Author

If you're unhappy about the default names of the Exports (understandable since they are designed to be consumed transparently), you're free to add some new Output()s to your CDK stack, which translate directly into CloudFormation Outputs and can be made into Exports as well.

Is there an example of this somewhere?

ie, naming my own stack outputs and then passing the variable to another CDK stack, similar to what's done with VpcNetworkRefProps.

Or, to phrase it another way, what's special about the .export() method that means I can assign its return value to this.vpcRef and then that reference is usable elsewhere in CDK, and CDK is smart enough to know what Fn::ImportValue bits to generate in the other stack?

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 20, 2018

If you take a look at the definition of VpcNetwork.export() (https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-ec2/lib/vpc-ref.ts#L62), you'll see that what it does is:

  • Create a new Output() object, which translates to a CloudFormation Output in the synthesized template.
  • Then, on that Output object, it calls makeImportValue(). What that does is mark the Output as an Export, and return the corresponding { Fn::ImportValue } primitive for that Output.

The result is that an Export will be created, and the returned value is the Import that will eventually take on the Export's value at deployment time.

The thing is, since this Output is created as a child of another construct, its LogicalID will be a long generated string with a unique identifier at the end. If you were to create the Output as a direct child of Stack, no such name mangling will occur and the name of the Output construct would also be the name of the Export. So you would mirror the implementation of VpcNetworkRef.export() but create the Outputs as children of Stack instead of as children of VpcNetworkRef.

By the way, looking at this code I'm noticing that the list of availabilityZones doesn't get turned into an Output. At the moment, if you need those, you will have to transport those values yourself. I will create an issue for this.

@pchaganti
Copy link

👍

@cookejames
Copy link
Contributor

cookejames commented Aug 21, 2018

I wish this was made a bit plainer in the documentation as it looks like I have basically duplicated the import / export functionality in my stack with the following. The one suggestion I would make is that the documentation explicitly mentions using isolated subnets for things like RDS clusters however when it comes to creating a cluster the RDS class uses an option usePublicSubnets rather which takes the private subnets but gives no option for the isolated subnets.

  getVpcImportRef(parent, name, { useIsolatedSubnetsAsPrivate = false } = {}) {
    const privateSubnets = useIsolatedSubnetsAsPrivate ? this.outputs.subnets.isolated : this.outputs.subnets.private;
    return VpcNetworkRef.import(parent, name, {
      vpcId: this.outputs.vpc.makeImportValue(),
      availabilityZones: this.vpc.availabilityZones,
      publicSubnetIds: Object.keys(this.outputs.subnets.public).map(id =>
        this.outputs.subnets.public[id].makeImportValue()
      ),
      privateSubnetIds: Object.keys(privateSubnets).map(id => privateSubnets[id].makeImportValue()),
    });
  }

The good news is that most of the time that I've tried to do something with the CDK and bumped up against a limitation I have found that there is already support to work around it.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 21, 2018

I apologize for all rough edges you're running into. We're very grateful for your investment though--it's specifically because we need people putting the library through its paces to figure out where our sharp design and documentation edges are.

To your point, the subnet selection is being addressed here: #610

@cookejames
Copy link
Contributor

Don't apologise the library is clearly marked at pre-production so rough edges are to be expected. Already with the CDK I have been able to shave over 500 lines off our existing CF template.

I only wish I could devote more time at work to adding some more features as this sort of library building is quite enjoyable.

@cookejames
Copy link
Contributor

Also great news about the subnet selection. When I come across something like this or the lack of tagging I have always it seems found that you're a step ahead and have something planned for it already.

@Doug-AWS
Copy link
Contributor

See #1525

@Doug-AWS Doug-AWS added the docs/guide Related to the developer guide label Jan 23, 2019
@Doug-AWS
Copy link
Contributor

I'm not sure how much of this is still true after the refactoring of export/import.

@Doug-AWS Doug-AWS added p1 pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. needs-sample effort/medium Medium work item – several days of effort labels Jan 29, 2019
@mipearson
Copy link
Author

Yeah - for cross-stack going between CDK and non-CDK I'm mostly entering values by hand right now anyway.

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 26, 2019

This documentation section will change completely when #1546 lands.

@Doug-AWS
Copy link
Contributor

Okay, I'll wait on #1546

@Doug-AWS Doug-AWS self-assigned this Feb 26, 2019
@Doug-AWS Doug-AWS added parked and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. needs-sample labels Feb 26, 2019
@Doug-AWS Doug-AWS removed the parked label Feb 27, 2019
@Doug-AWS Doug-AWS added post-GA and removed p1 labels Apr 16, 2019
@Doug-AWS
Copy link
Contributor

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs/guide Related to the developer guide effort/medium Medium work item – several days of effort
Projects
None yet
Development

No branches or pull requests

5 participants