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

RFC300: Programmatic Toolkit #654

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Nov 12, 2024

This is a request for comments about Programmatic Toolkit. See #300 for
additional details.

Rendered Version

APIs are signed off by @rix0rrr.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license

@mrgrain mrgrain force-pushed the mrgrain/0300-programmatic-toolkit branch from 9f9403f to 74205a4 Compare November 20, 2024 10:45

#### Messages and Requests

The toolkit emits *messages* and *requests* to structure interactions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example of a sequence of messages back and forth would be nice.

text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
});

// Synth and store a cached CloudExecutable, this won't run synth again
const cxCache = await cdk.synth(cx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is cxCache the same type as cx? Or implements the same interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes see "Actions - synth". I don't want the example to be polluted by types. Would it be more clear to use let here?

text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
text/0300-programmatic-toolkit.md Show resolved Hide resolved
text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
A detailed definition of what would constitute a breaking change is included in the readme above.

To support integrations, we will need to publish a list of all messages.
We will use static code analysis to automatically compile and publish this list without adding additional maintenance burden on to the team.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts to how that would work, or TBD?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we could use the TS compiler to detect all calls to certain functions/methods and extract it that way.
No POC though.

text/0300-programmatic-toolkit.md Outdated Show resolved Hide resolved
@mrgrain mrgrain force-pushed the mrgrain/0300-programmatic-toolkit branch 3 times, most recently from bed1c2d to 137b1af Compare November 24, 2024 12:41
@mrgrain mrgrain force-pushed the mrgrain/0300-programmatic-toolkit branch from 137b1af to 3f7072f Compare November 24, 2024 13:35
@guysqr
Copy link

guysqr commented Nov 25, 2024

This is going to be a really useful capability that will enable many interesting and valuable use cases, including being able to bake CDK into an Electron app and compose, deploy and destroy CDK apps entirely in code. This will put the benefits of the CDK into the hands of a whole new audience - those who can't or won't install command line tools.

I think it would be good to remove any interdependency between this and the consuming app around SDK, so consider if rather than passing an SdkProvider we could just pass a SDKv3 credentials provider.

Also, one of the challenges I saw when trying to use the CDK programatically in the past is how it sends CFN outputs to STDOUT and STDERR - would be ideal if we wanted to consume the CFN stack creation events via CDK to have a way to tap into that feed.

* **API Bar Raiser**: @rix0rrr

The [Programmatic Toolkit] allows integrators to build lifecycle management solutions for CDK applications.
It provides them with full control over all output and allows to interweave actions with custom steps.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and allows to interweave actions with custom steps.

Would be nice to include a quick example of this right off the bat.

#### Messages and Requests

The toolkit emits *messages* and *requests* to structure interactions.
A *request* is a *message* that requires the receiver to respond, otherwise the toolkit will continue with a default response.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a hard time imagining what the CLI would require a response for, and what default response would be. Can you show an example?

interface IoMessage<T> {
time: string;
level: 'error' | 'warn' | 'info' | 'debug';
action: 'synth' | 'list' | 'deploy' ... ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these meant to represent the top level CLI command the user invoked? Or can they be sub actions like wait-for-stack-stabilization?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meant to be the invoked command, i.e. "what is the toolkit currently doing".

code would be used for sub actions.

#### IoHost

Message receivers need to implement the `IIoHost` interface.
The `IoHost` is responsible for handling all logging and interactions:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming here is throwing me off. IoHost seems to suggest it responds to I/O events like logging or network calls. But actually, it can respond to any event that we decide can be responded to...is that right? If so, is EventHost a more appropriate name?

This implementation will do nothing for messages and requests that don't have a registered listener, i.e. the default response will be used as required.

```ts
const io = new IoEmitter();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implements IoHost and dispatches to event listeners?

Comment on lines +508 to +513
#### Readme

The `AwsSdkProvider` defines how the toolkit interacts with AWS services.
A default provider is available.
You can provide a custom class implementing the `IAwsSdkProvider` interface.
A standard `SdkProviderForCdk` is available that includes all features of the AWS CDK CLI [TODO: this might end up in a separate package].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow. Is this part of the previous section? Also it doesn't sound like an alternate solution, but rather a possible future extension?


#### Event stream

> Streams are non blocking by design.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> Streams are non blocking by design.
> Disregarded. Streams are non blocking by design.

It also implies refactoring of existing interfaces to match the new API design.
Both of these things are prone to human errors.

Mitigation for this is test coverage and separating out PRs that are purely moving code, from actual changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to coordinate rolling this out only after we address the coverage gaps we detected.


No.

### What alternative solutions did you consider?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the option of not providing an event API at all, and instead just focus on the objects the CLI returns, and make sure they contain all the information necessary for customers to take actions?

That is:

// no hooks available on the `cdk` instance
const deployment = cdk.deploy(...);

// take the output and do something
const stacks = deployment.listStacks();

....

I'm not really advocating for this approach, but I want to understand exactly what use-cases this solution doesn't address.

Comment on lines +735 to +737
| FR10 | Events |
| | In extension to logging focused events, the CDK should emit events for API operations including progress and state transitions (e.g. for deployment: synth complete, assets built, assets published, Hotswap/CFN deployment started ...). If available, these events should include additional data like the arn of the stack or a progress percentage. An integrator must be able to execute blocking code on these events. This will allow them to e.g. schedule pre-build scripts before synth starts or to update a service registry after deployment completed. |
| FR11 | Event hooks |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you map these requirements to specific use-cases? that is, specific actions we know customers want to take inside events.

This will help us understand if the use case can be addressed in some other way.

const deployment = await cdk.deploy(cxCache, { stacks: ['MyTestStack'] });

// Returns a list of ARNs of the deployment
return deployment.listResources().filter(r => r.arn);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this method also allow access to resource attributes like AWS::Lambda::Url's FunctionUrl? I could see this being very helpful for automated testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't think that's easily possible as CFN stack APIs don't include this information. You'd need to use a Stack output or make the request to the service API manually. But at least that should be much easier.

Copy link

@misterjoshua misterjoshua Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stack outputs would work fine, but how would accessing those values work under this proposal?

For example, deployment.listOutputs() to get the outputs? Or perhaps deployment.describe() to wrap calls to DescribeStacks for the descriptions of all the deployed stacks?

const deployment = await cdk.deploy(cxCache, { stacks: ['MyTestStack'] });

// Returns a list of ARNs of the deployment
return deployment.listResources().filter(r => r.arn);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there the possibility of filtering resources here by node path? This might make it easier to locate specific resources for a system under test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great suggestion! Might not be in the first version, but should be easy to add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants