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

Remove dependency from cdk toolkit on @aws-cdk/* #293

Closed
RomainMuller opened this issue Jul 10, 2018 · 1 comment · Fixed by #352
Closed

Remove dependency from cdk toolkit on @aws-cdk/* #293

RomainMuller opened this issue Jul 10, 2018 · 1 comment · Fixed by #352
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@RomainMuller
Copy link
Contributor

Currently, the cdk toolkit uses a CDK stack to deploy the "toolkit stack" (including an S3 bucket that will be used to stage further cloudformation templates for deployment). This introduces a dependency cycle (aws-cdk@aws-cdk/s3@aws-cdk/assertaws-cdk).

Breaking the cycle would be as easy as replacing the CDK stack with the actual synthesized CloudFormation template, which would make the CDK toolkit not dependent upon CDK to work (which was awkward anyway).

RomainMuller added a commit that referenced this issue Jul 10, 2018
The `create_missing_libraries.sh` script was used to bootstrap L2
packages for all the namespaces that didn't have one already.

Existing L2 packages were manually edited to:
- Invoke `cfn2ts` to generate the L1 class library for the service/scope
- Depend on `@aws-cdk/runtime` (which used to be blended in
  `@aws-cdk/resources`, and was extracted out)
- Export the L1 library as expected
- Generated resource and property classes are namespaced under 'cloudformation'

The `@aws-cdk/assert` behavior was changed to rely on
`@aws-cdk/cdk-cfnspec` (formerly `@aws-cdk/cloudformation-resource-spec`)
to determine the `UpdateType` of attributes, rendering the `registry`
mechanism that was introduced to `@aws-cdk/resources` a while
ago redundant (hence it got dropped).

This required temporarily disabling use of '--reject-cycles' on 'lerna bootstrap'
as it would blow up on the cycle created (see #293).
@RomainMuller
Copy link
Contributor Author

Don't forget to re-introduce the --reject-cycle option in the lerna bootstrap invocation of build.sh once the cycle is broken.

@eladb eladb self-assigned this Jul 17, 2018
eladb pushed a commit that referenced this issue Jul 17, 2018
Due to a cyclic dependency caused by the need for
CDK libraries to depend on the toolkit for testing
purposes, we decided not to use the CDK to define
the "toolkit stack". It's a very simple stack
that essentially contains a single bucket.

Therefore, we just embed the template in the
toolkit code, and finally we don't have cycles
anymore.

Tested this manually by bootstrapping the toolkit
stack in my dev account and also updating it.

The empty "WaitHandle" stack also used the CDK,
so this was also embedded inline.

--reject-cycles is enabled now

Fixes #293
eladb pushed a commit that referenced this issue Jul 17, 2018
Due to a cyclic dependency caused by the need for CDK libraries to depend on the toolkit for testing purposes, we decided not to use the CDK to define the "toolkit stack". It's a very simple stack that essentially contains a single bucket.

Therefore, we just embed the template in the toolkit code, and finally we don't have cycles anymore.

Tested this manually by:
* Bootstrapping the toolkit stack in my dev account
* Updating the toolkit stack
* Verifying that "deploy" can discover the bucket and upload a template

The empty "WaitHandle" stack also used the CDK, so this was also embedded inline.

--reject-cycles is enabled now

Fixes #293
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants