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

Simplify dependency management for applets #1245

Closed
eladb opened this issue Nov 24, 2018 · 4 comments · Fixed by #2636
Closed

Simplify dependency management for applets #1245

eladb opened this issue Nov 24, 2018 · 4 comments · Fixed by #2636
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container

Comments

@eladb
Copy link
Contributor

eladb commented Nov 24, 2018

When trying to use applets in 0.18.1 via a globally installed toolkit, you will get the following error:

Error: The child XXX of Program must be a Stack

This is because when the toolkit is globally installed, the copy of @aws-cdk/cdk (which defines Stack) is different than the one installed in the local node_modules. In JavaScript, instanceof is based on constructor object equality and they have to be the exact same copy in order to align.

Reported by @clareliguori

@eladb
Copy link
Contributor Author

eladb commented Nov 24, 2018

A workaround is to use the toolkit from a local install instead of a global install. As long as versions match, npm would de-dup the @aws-cdk/cdk module and Stack will be Stack.

For example, here is how to use the Fargate Service Applet:

$ mkdir fargate-service
$ npm i aws-cdk @aws-cdk/aws-ecs # notice that we install the toolkit locally here
$ cat > my-service.yaml <<HERE
applets:
  LoadBalancedFargateService:
    type: @aws-cdk/aws-ecs:LoadBalancedFargateServiceApplet
    properties:
      image: 'amazon/amazon-ecs-sample'
      cpu: "2048"
      memoryMiB: "1024"

HERE
$ npx cdk -a my-service.yaml synth # notice we use "npx" to invoke the cdk from the local install

@eladb eladb added bug This issue is a bug. @aws-cdk/aws-ecs Related to Amazon Elastic Container labels Nov 24, 2018
@eladb
Copy link
Contributor Author

eladb commented Nov 24, 2018

There are a few things here to take care of:

  • Stop using instanceof
  • Maybe when using applets, the dependency closure should be managed by the toolkit. Would be nice if I didn't even have to install @aws-cdk/aws-ecs (but then, we might need a way to specify semver requirements). If we don't do that, we should get the user to install @aws-cdk/applet-js and use the local one and not install it globally
  • The fact that the toolkit takes a dependency on the CDK framework is not a good idea in general

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 27, 2018

The fact that the toolkit takes a dependency on the CDK framework is not a good idea in general.

True.

But the alternative is having to make a package.json and npm installing in a directory which is supposed to only contain a YAML file, which seems suboptimal as well. I was talking @SoManyHs through how to set up an applet and I couldn't in good conscience justify all the steps.

  • Write your YAML (should be the one and only step)
  • Write a package.json
  • Install @aws-cdk/applets-js and @aws-cdk/aws-ecs.
  • Create a cdk.json with the following in it:
{
  "app": "node_modules/.bin/cdk-applet-js template.yml"
}
  • Finally you get to run cdk deploy

The single cdk.json makes it impossible to have multiple applet files in the same directory and access them easily (Yes, we can have multiple stacks in one file, but I think there's value in organizing into files).

We need to simplify this process.

rix0rrr added a commit that referenced this issue Nov 27, 2018
Make the Stack instance test no longer use instanceof, which doesn't
work properly in case of multiple installed copies of the library.

This does not resolve but helps with #1245.
@eladb
Copy link
Contributor Author

eladb commented Nov 28, 2018

I agree that we need to simplify but I think we should think of the end-to-end experience. If we really want only a .yaml file to be needed, then all dependencies should be implicitly managed somehow, which also means we need to be able to define version requirements in the applet, and we need to figure out how to cache those dependencies so that subsequent invocations won't need to install everything every time.

rix0rrr added a commit that referenced this issue Nov 30, 2018
Make the Stack instance test no longer use instanceof, which doesn't
work properly in case of multiple installed copies of the library.

This does not resolve but helps with #1245.
@rix0rrr rix0rrr removed the bug This issue is a bug. label Dec 3, 2018
@rix0rrr rix0rrr changed the title Applets are broken because App uses "instanceof Stack" Simplify dependency management for applets Dec 3, 2018
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
@aws-cdk/aws-ecs Related to Amazon Elastic Container
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants