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

App.synthesize methods are marked as deprecated with no replacement #2016

Closed
thesurlydev opened this issue Mar 14, 2019 · 5 comments · Fixed by #2636
Closed

App.synthesize methods are marked as deprecated with no replacement #2016

thesurlydev opened this issue Mar 14, 2019 · 5 comments · Fixed by #2636

Comments

@thesurlydev
Copy link
Contributor

In latest version (0.25.3) the App.synthesize methods are marked as deprecated but don't offer a replacement or pointer as to why. Please provide guidance for the "new" sythesize.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 15, 2019

From your question I can tell you are running/synthesizing CDK apps not in the way we intended them to.

The intended way is to let the cdk toolkit drive your application by launching it as a subprocess, not to run CDK apps in-memory. In other words, you are using an unsupported part of the API, and those synthesizeStack methods were never supposed to have been there or public in the first place (they were there for the benefit of our unit tests). Please don't do that, and if you do, be aware that we can break you at any time.

Now, the slightly better news is that you can probably keep on doing what you are doing today by calling app.run() and inspecting the object that is returned to you from it. It will be an InMemoryStore and contain the templates of the stacks that got synthesized. But again, these APIs are not intended to be called by users directly.

@mbulman
Copy link

mbulman commented Mar 15, 2019

A bit tangential, but I have a need for adding business-specific validation to Stacks. A contrived example would be something like "in dev always use on-demand billing for dynamodb". Right now (0.24.1) I've extended Stack and overridden toCloudFormation to validate against the synthesized stack object. I worry that this falls into the same category of "not intended to be called by users directly" and will break in a future upgrade.

@digitalsanctum what is your use case for relying on synthesize directly?

@rix0rrr depending on his answer, what's your guidance on the best place to hook in? You mention app.run but then say don't do that ;) Perhaps this is a feature request?

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 15, 2019

I would consider it a feature request at the moment.

As a practical matter, you should be able to achieve the same effect by having a class implement IAspect, like this:

class DynamoValidator implements IAspect {
  public visit(construct: IConstruct) {
    if (Resource.isResource(construct) && construct.resourceType == 'AWS::DynamoDB::Table') {
      if (!whatIWant(construct)) { 
        throw new Error('oops');
     }
    }
  }
}

app.node.apply(new DynamoValidator());

There might be some details wrong with the code above (around type checking), but the pattern should hold.

@thesurlydev
Copy link
Contributor Author

The short version is I'd like to use CDK (without the CLI bits) in concert with the SDK to easily do things like integration tests. Yes, I could just use the SDK but I think there's a huge opportunity to be able to just use the CDK and the benefits that come with it like automatic IAM role creation, reusable constructs, etc. Being able to use the CDK synth and deploy without worrying about installing the CLI would be ideal.

For the purposes of this issue, it would be nice to have your explanation noted in the code comment along with the deprecation for other folks that are using the public synth methods.

@thesurlydev
Copy link
Contributor Author

It seems the feature I describe above has already been proposed and voted for by several people here: https://github.com/awslabs/aws-cdk/issues/601

eladb pushed a commit that referenced this issue Mar 18, 2019
…tion")

The method `toCloudFormation` is not supposed to be directly called by
users. Mark it as `@internal` and rename to `_toCloudFormation`.

Fixes #2044
Related #2016
eladb pushed a commit that referenced this issue Mar 19, 2019
…tion") (#2047)

The method `toCloudFormation` is not supposed to be directly called by
users. Mark it as `@internal` and rename to `_toCloudFormation`.

Fixes #2044
Related #2016

BREAKING CHANGE: “toCloudFormation” is now internal and should not be called directly. Instead use “app.synthesizeStack”
rix0rrr pushed a commit to alex-berger/aws-cdk that referenced this issue Mar 19, 2019
…tion") (aws#2047)

The method `toCloudFormation` is not supposed to be directly called by
users. Mark it as `@internal` and rename to `_toCloudFormation`.

Fixes aws#2044
Related aws#2016

BREAKING CHANGE: “toCloudFormation” is now internal and should not be called directly. Instead use “app.synthesizeStack”
eladb pushed a commit that referenced this issue May 29, 2019
Formalize the concept of a cloud assembly to allow decoupling "synth" and "deploy". the main
motivation is to allow "deploy" to run in a controlled environment without needing to execute the app for security purposes.

`cdk synth` now produces a self-contained assembly in the output directory, which we call a "cloud assembly". this directory includes synthesized templates (similar to current behavior) but also a "manifest.json" file and the asset staging directories.

`cdk deploy -a assembly-dir` will now skip synthesis and will directly deploy the assembly.

To that end, we modified the behavior of asset staging such that all synth output always goes to the output directory, which is by default `cdk.out` (can be set with `--output`). if there's a single template, it is printed to stdout, otherwise the name of output directory is printed.

This PR also includes multiple clean ups and deprecations of stale APIs and modules such as:

- `cdk.AppProps` includes various new options.
- An error is thrown if references between stacks that are not in the same app (mostly in test cases).
- Added the token creation stack trace for errors emitted by tokens
- Added `ConstructNode.root` which returns the tree root.
 

**TESTING**: verified that all integration tests passed and added a few tests to verify zip and docker assets as well as cloud assemblies. See: https://github.com/awslabs/cdk-ops/pull/364

Closes #1893 
Closes #2093 
Closes #1954 
Closes #2310 
Related #2073 
Closes #1245
Closes #341
Closes #956
Closes #233

BREAKING CHANGE: *IMPORTANT*: apps created with the CDK version 0.33.0 and above cannot be used with an older CLI version.
- `--interactive` has been removed
- `--numbered` has been removed
- `--staging` is now a boolean flag that indicates whether assets should be copied to the `--output` directory or directly referenced (`--no-staging` is useful for e.g. local debugging with SAM CLI)
- Assets (e.g. Lambda code assets) are now referenced relative to the output directory.
- `SynthUtils.templateForStackName` has been removed (use `SynthUtils.synthesize(stack).template`).
- `cxapi.SynthesizedStack` renamed to `cxapi.CloudFormationStackArtifact` with multiple API changes.
- `cdk.App.run()` now returns a `cxapi.CloudAssembly` instead of `cdk.ISynthesisSession`.
- `cdk.App.synthesizeStack` and `synthesizeStacks` has been removed. The `cxapi.CloudAssembly` object returned from `app.run()` can be used as an alternative to explore a synthesized assembly programmatically (resolves #2016).
- `cdk.CfnElement.creationTimeStamp` may now return `undefined` if there is no stack trace captured.
- `cdk.ISynthesizable.synthesize` now accepts a `cxapi.CloudAssemblyBuilder` instead of `ISynthesisSession`.
- `cdk`: The concepts of a synthesis session and session stores have been removed (`cdk.FileSystemStore`, cdk.InMemoryStore`, `cdk.SynthesisSession`, `cdk.ISynthesisSession` and `cdk.SynthesisOptions`).
- No more support for legacy manifests (`cdk.ManifestOptions` member `legacyManifest` has been removed)
- All support for build/bundle manifest have been removed (`cxapi.BuildManifest`, `cxapi.BuildStep`, etc).
- Multiple deprecations and breaking changes in the `cxapi` module (`cxapi.AppRuntime` has been renamed to `cxapi.RuntimeInfo`, `cxapi.SynthesizeResponse`, `cxapi.SynthesizedStack` has been removed)
- The `@aws-cdk/applet-js` program is no longer available. Use [decdk](https://github.com/awslabs/aws-cdk/tree/master/packages/decdk) as an alternative.
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 a pull request may close this issue.

3 participants