Use cdk-cli-lib to synthesize CDK application #170

corymhall · 2024-10-07T19:35:36Z

This PR introduces a breaking change to the API changes the entrypoint of the application from the Stack construct to the App component.

Previous API

class MyStack extends pulumicdk.Stack {
  constructor(id: string, props: pulumicdk.StackProps) {
      super(id, props);
          const bucket = new s3.Bucket(this, 'Bucket');
          this.synth();
  }
}

New API

new pulumicdk.App('app', (scope: pulumicdk.App) => {
    const stack = new pulumicdk.Stack(scope, 'stack');
    const bucket = new s3.Bucket(this, 'Bucket');
});

This will allow for a couple of benefits

No longer need to subclass pulumicdk.Stack and call the protected synth method.
Will enable multiple stacks
Allows for native use of CDK Context and Lookups

For a more detailed breakdown on why we are making this change, i've included and ADR based on the internal doc.

closes #153

examples/lookups/index.ts

corymhall · 2024-10-07T19:39:33Z

src/stack.ts

-    /** @internal */
-    registerOutput(outputId: string, output: any) {
-        this.stack.outputs[outputId] = pulumi.output(output);
+    async produce(context: Record<string, any>): Promise<string> {


This is what will get executed multiple times.

This sounds like piece-de-resistane in this PR. So it's calling app.synth() and returning dir.

So couple of low-level questions here perhaps.

if this method fails, are we OK? do we expect it to fail or we don't expect it to regularly fail?

if app.synth() creates temp dirs with assets, what cleans it up? is it something to worry about?

the state of this is modified this.assemblyDir = dir and this.stacks[child.artifactId] = child - do we need to worry about something concurrently accessing the partial state before the framework is done calling us repeatedly or it all works out as needed?

if this method fails, are we OK? do we expect it to fail or we don't expect it to regularly fail?

This is called when we call cli.synth() as part of initialize and we do catch errors there.

if app.synth() creates temp dirs with assets, what cleans it up? is it something to worry about?

Maybe? It will create it as a system tmp dir which gets cleaned up whenever the computer restarts.

the state of this is modified this.assemblyDir = dir and this.stacks[child.artifactId] = child - do we need to worry about something concurrently accessing the partial state before the framework is done calling us repeatedly or it all works out as needed?

We shouldn't need to worry about it. This is called in blocking code as part of initialize.

System temp dir should be fine, yes. All good then.

corymhall · 2024-10-07T19:40:07Z

src/stack.ts

     */
-    outputs: { [outputId: string]: pulumi.Output<any> } = {};
+    public static isPulumiStack(x: any): x is Stack {


instanceOf doesn't work for constructs...

corymhall · 2024-10-23T17:40:51Z

examples/ecscluster/index.ts

This test didn't really test anything. It appears to test lookups, but it doesn't actually.

corymhall · 2024-10-29T15:17:36Z

.github/workflows/run-acceptance-tests.yml

@@ -44,8 +44,8 @@ jobs:
  test:
    name: acceptance-test
    concurrency:
-      group: acceptance-test-${{ matrix.index }} # TODO: concurrent tests across PRs can cause problems
-      cancel-in-progress: false
+      group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.index }} # TODO: concurrent tests across PRs can cause problems


I think we've fixed the concurrency issues

corymhall · 2024-10-29T15:18:09Z

adr/cdk-cli-lib.md

Copied most of the doc here so it can live in the repo.

README.md

adr/cdk-cli-lib.md

examples/s3-object-lambda/index.ts

src/stack.ts

t0yv0 · 2024-10-31T17:33:42Z

Quick question on the API.

new pulumicdk.App('app', (scope: pulumicdk.App) => {
    const stack = new pulumicdk.Stack(scope, 'stack');
    const bucket = new s3.Bucket(this, 'Bucket');
});

What is this refer to? Is this some JavaScript magic binding this in the callback? Can we have something explicit instead?

Will users always create exactly one stack as the first thing in the callback? Or it is possible to have more than one or not have one? Ah "Will enable multiple stacks" - got this. This is by design.

.github/workflows/run-acceptance-tests.yml

t0yv0 · 2024-10-31T17:35:58Z

README.md

@@ -1,12 +1,22 @@
 # Pulumi CDK Adapter (preview)

-The Pulumi CDK Adapter is a library that enables [Pulumi](https://github.com/pulumi/pulumi) programs to use [AWS CDK](https://github.com/aws/aws-cdk) constructs.
+The Pulumi CDK Adapter is a library that enables


Is this change necessary? Looks like Emacs fil-paragraph got applied. I think we prefer to have the previous form.

I don't know, I kind of prefer the 80 line length limit. It makes looking/editing so much easier. It's also the default in most markdown linters, (e.g. markdownlint

AH nice, one of those things that'd just nice to have a standard. We can adopt it in the project. I'll setup column width 80 in Emacs.

t0yv0 · 2024-10-31T17:37:00Z

README.md

@@ -81,99 +94,111 @@ Try the workshop at https://apprunnerworkshop.com
 Read the docs at https://docs.aws.amazon.com/apprunner
 ```

-## Getting Started
-
-### Bootstrapping


We now auto-bootstrap? Perhaps still worth a mention that it does this.

Added some docs around bootstrapping and what resources we create for you.

src/converters/app-converter.ts

corymhall · 2024-10-31T17:46:53Z

@t0yv0

What is this refer to? Is this some JavaScript magic binding this in the callback? Can we have something explicit instead?

This is actually a mistake on my part so good catch! This should say stack instead.

t0yv0

Looking good at a glance. Sorry for a rather shallow review, I need to get better at this codebase.

t0yv0 · 2024-10-31T19:43:06Z

flostadler

LGTM!

pulumi-bot · 2024-11-27T16:06:29Z

This PR has been shipped in release v1.0.0.

corymhall commented Oct 7, 2024

View reviewed changes

mjeffryes assigned corymhall Oct 11, 2024

corymhall commented Oct 23, 2024

View reviewed changes

corymhall force-pushed the corymhall/cdk-cli-v3 branch from 189a0e0 to f84d8be Compare October 23, 2024 18:24

This was referenced Oct 23, 2024

Use custom pulumi app synthesizer #172

Merged

Add another example and map additional resources #178

Merged

Synthesizer resources should inherit the aws providers #181

Closed

corymhall force-pushed the corymhall/cdk-cli-v3 branch 2 times, most recently from 38076ae to 7289835 Compare October 28, 2024 19:23

corymhall commented Oct 29, 2024

View reviewed changes

corymhall marked this pull request as ready for review October 29, 2024 18:09

corymhall requested review from t0yv0 and flostadler October 29, 2024 18:09

corymhall added 10 commits October 29, 2024 14:12

Use cdk-cli-lib to synthesize CDK application

f2248ec

Fixing rebase conflicts

2395e3d

fixing some stuff

1b78a9e

fixing build issues

24cd053

fixing deps

2f78143

fixing lookup

9eec898

Add adr

5b08629

cleaning up the api and updating README

b613347

fixing dep

176d9b2

rebasing upstream examples

42df90a

corymhall force-pushed the corymhall/cdk-cli-v3 branch from bfa47d5 to 42df90a Compare October 29, 2024 18:16

revert workflow changes

fd02d81

flostadler reviewed Oct 31, 2024

View reviewed changes

t0yv0 reviewed Oct 31, 2024

View reviewed changes

.github/workflows/run-acceptance-tests.yml Outdated Show resolved Hide resolved

t0yv0 reviewed Oct 31, 2024

View reviewed changes

src/converters/app-converter.ts Show resolved Hide resolved

t0yv0 self-requested a review October 31, 2024 17:52

t0yv0 approved these changes Oct 31, 2024

View reviewed changes

updating based on review comments

37dcabb

corymhall requested a review from flostadler October 31, 2024 19:31

flostadler approved these changes Oct 31, 2024

View reviewed changes

corymhall merged commit 8be5049 into main Nov 1, 2024
8 checks passed

corymhall deleted the corymhall/cdk-cli-v3 branch November 1, 2024 11:37

pulumi-bot mentioned this pull request Nov 27, 2024

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use cdk-cli-lib to synthesize CDK application #170

Use cdk-cli-lib to synthesize CDK application #170

corymhall commented Oct 7, 2024 •

edited

Loading

corymhall Oct 7, 2024

t0yv0 Oct 31, 2024

corymhall Oct 31, 2024

t0yv0 Oct 31, 2024

corymhall Oct 7, 2024

corymhall Oct 23, 2024

corymhall Oct 29, 2024

corymhall Oct 29, 2024

t0yv0 commented Oct 31, 2024 •

edited

Loading

t0yv0 Oct 31, 2024

corymhall Oct 31, 2024

t0yv0 Oct 31, 2024

t0yv0 Oct 31, 2024

corymhall Oct 31, 2024

corymhall commented Oct 31, 2024

t0yv0 left a comment

t0yv0 commented Oct 31, 2024

flostadler left a comment

pulumi-bot commented Nov 27, 2024

Use cdk-cli-lib to synthesize CDK application #170

Use cdk-cli-lib to synthesize CDK application #170

Conversation

corymhall commented Oct 7, 2024 • edited Loading

Previous API

New API

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t0yv0 commented Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corymhall commented Oct 31, 2024

t0yv0 left a comment

Choose a reason for hiding this comment

t0yv0 commented Oct 31, 2024

flostadler left a comment

Choose a reason for hiding this comment

pulumi-bot commented Nov 27, 2024

corymhall commented Oct 7, 2024 •

edited

Loading

t0yv0 commented Oct 31, 2024 •

edited

Loading