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

Use cdk-cli-lib to synthesize cdk app #153

Closed
corymhall opened this issue Aug 22, 2024 · 1 comment · Fixed by #170
Closed

Use cdk-cli-lib to synthesize cdk app #153

corymhall opened this issue Aug 22, 2024 · 1 comment · Fixed by #170
Assignees
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Milestone

Comments

@corymhall
Copy link
Contributor

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

CDK functionality is split between the framework and the cli. Currently pulumi-cdk directly integrates with the framework in order to synthesize and convert the resources. We should instead integrate with the cli and allow the cli to synth. This would allow us to utilize the complete feature set of CDK.

For example this would enable CDK lookups like Vpc.fromLookup(). When lookups are performed:

  1. The CLI calls framework synth
  2. The framework looks for the lookup value in cdk.context.json and if it is not there then it registers that a lookup needs to take place
  3. The CLI reads the CloudAssembly and then performs any necessary lookups, storing the information back in cdk.context.json
  4. The CLI calls framework synth again with the fully looked up values.

We should be able to use the new @aws-cdk/cli-lib-alpha library to register a CloudAssembly directory producer.

Affected area/feature

@corymhall corymhall added the kind/enhancement Improvements or new features label Aug 22, 2024
@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Aug 22, 2024
@flostadler flostadler removed the needs-triage Needs attention from the triage team label Aug 23, 2024
@corymhall corymhall self-assigned this Sep 30, 2024
@mjeffryes mjeffryes assigned flostadler and corymhall and unassigned corymhall and flostadler Oct 1, 2024
corymhall added a commit that referenced this issue Oct 1, 2024
Previously this library walked the construct tree in-memory and called the
internal `_toCloudFormation()` method each `Cfn*` construct has to get
the CloudFormation resource fragment. This is both inefficient (because
we call `synth` which internally does the same thing, and then we do it
again), and potentially inaccurate (`synth` internally does a lot more
than we do when we walk the construct tree).

This PR is a refactor (maybe closer to a rewrite) that switches to
taking the output of `app.synth()` and processing the resulting
CloudAssembly. The CloudAssembly has everything necessary to do the
conversion and contains the fully resolved CloudFormation template (we don't
have to worry about unresolved tokens!).

This also sets us up for future work like nested stacks, multiple
stacks, and #153.

Note to reviewers. This is a pretty big refactor so I would recommend
reviewing this instead as a restart rather than trying to figure out
what the old code is doing.

Closes #18
@mjeffryes mjeffryes added this to the 0.111 milestone Oct 2, 2024
corymhall added a commit that referenced this issue Oct 7, 2024
Previously this library walked the construct tree in-memory and called
the internal `_toCloudFormation()` method each `Cfn*` construct has to
get the CloudFormation resource fragment. This is both inefficient
(because we call `synth` which internally does the same thing, and then
we do it again), and potentially inaccurate (`synth` internally does a
lot more than we do when we walk the construct tree).

This PR is a refactor (maybe closer to a rewrite) that switches to
taking the output of `app.synth()` and processing the resulting
CloudAssembly. The CloudAssembly has everything necessary to do the
conversion and contains the fully resolved CloudFormation template (we
don't have to worry about unresolved tokens!).

This also sets us up for future work like nested stacks, multiple
stacks, and #153.

Note to reviewers. This is a pretty big refactor so I would recommend
reviewing this instead as a restart rather than trying to figure out
what the old code is doing.

Closes #18
@t0yv0 t0yv0 assigned t0yv0 and corymhall and unassigned corymhall and t0yv0 Oct 28, 2024
@mjeffryes mjeffryes modified the milestones: 0.111, 0.112 Oct 30, 2024
@pulumi-bot pulumi-bot added resolution/fixed This issue was fixed labels Nov 1, 2024
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #170 and shipped in release v1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants