feat(core): make StackSynthesizer easier to subclass
#22308
Merged
+771
−408
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`StackSynthesizer`s used to make heavy use of private APIs, which make it impossible for people outside the CDK core framework to implement their own Stack Synthesizers and modify the way assets are referenced. This question comes up more and more, so this PR opens up the facilities for doing that. The changes are as follows: - Implement `bind()` in `StackSynthesizer`, and add a getter called `boundStack` which will return the bound stack. - Change `synthesizeStackTemplate -> synthesizeTemplate`, `emitStackArtifact -> emitArtifact`, both of which will implicitly operate on the bound stack. - Add `addBootstrapVersionRule` so that any subclass can choose to use this or not. - Expose `AssetManifestBuilder`, which stack synthesizers can use to build an asset manifest. Refactor the class to do less at the same time. - Expose `cloudFormationLocationFrom...Asset` functions to translate a published asset location into a CloudFormation location. - Document the contract of each method better. The end result is that it will be easier to mix and match the different behaviors of the existing synthesizers together into new custom behavior. Two new unit test files have been added to show how it is possible to write ONE intruction to the asset manifest (to upload assets to a given location) while returning a completely different instruction to the template (referencing the asset bucket using a CloudFormation parameter).
|
|
aws-cdk-automation
previously requested changes
Sep 30, 2022
There was a problem hiding this comment.
The Pull Request Linter fails with the following errors:
❌ Features must contain a change to a README file.
❌ Features must contain a change to an integration test file and the resulting snapshot.
PRs must pass status checks before we can provide a meaningful review.
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
aws-cdk-automation
previously requested changes
Sep 30, 2022
There was a problem hiding this comment.
The Pull Request Linter fails with the following errors:
❌ Features must contain a change to a README file.
❌ Features must contain a change to an integration test file and the resulting snapshot.
PRs must pass status checks before we can provide a meaningful review.
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
aws-cdk-automation
previously requested changes
Sep 30, 2022
There was a problem hiding this comment.
The Pull Request Linter fails with the following errors:
❌ Features must contain a change to a README file.
❌ Features must contain a change to an integration test file and the resulting snapshot.
PRs must pass status checks before we can provide a meaningful review.
rix0rrr
added
pr-linter/exempt-integ-test
aws-cdk-automation
dismissed
their stale review
Pull Request updated. Dissmissing previous PRLinter Review.
|
@Mergifyio update |
✅ Branch has been successfully updated |
Naumel
approved these changes
Oct 3, 2022
| import { AssetManifestArtifact } from '@aws-cdk/cx-api'; | ||
| import { DockerImageAsset } from '../lib'; | ||
|
|
||
| // const DEMO_IMAGE_ASSET_HASH = '0a3355be12051c9984bf2b0b2bba4e6ea535968e5b6e7396449701732fe5ed14'; |
There was a problem hiding this comment.
Left over or should have more context to be considered part of docs?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
arewa
pushed a commit
to arewa/aws-cdk
that referenced
this pull request
Oct 8, 2022
`StackSynthesizer`s used to make heavy use of private APIs, which make it impossible for people outside the CDK core framework to implement their own Stack Synthesizers and modify the way assets are referenced. This question comes up more and more, so this PR opens up the facilities for doing that. The changes are as follows: - Implement `bind()` in `StackSynthesizer`, and add a getter called `boundStack` which will return the bound stack. - Change `synthesizeStackTemplate -> synthesizeTemplate`, `emitStackArtifact -> emitArtifact`, both of which will implicitly operate on the bound stack. - Add `addBootstrapVersionRule` so that any subclass can choose to use this or not. - Expose `AssetManifestBuilder`, which stack synthesizers can use to build an asset manifest. Refactor the class to do less at the same time. - Expose `cloudFormationLocationFrom...Asset` functions to translate a published asset location into a CloudFormation location. - Document the contract of each method better. The end result is that it will be easier to mix and match the different behaviors of the existing synthesizers together into new custom behavior. Two new unit test files have been added to show how it is possible to write ONE intruction to the asset manifest (to upload assets to a given location) while returning a completely different instruction to the template (referencing the asset bucket using a CloudFormation parameter). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk
pushed a commit
to homakk/aws-cdk
that referenced
this pull request
Dec 1, 2022
`StackSynthesizer`s used to make heavy use of private APIs, which make it impossible for people outside the CDK core framework to implement their own Stack Synthesizers and modify the way assets are referenced. This question comes up more and more, so this PR opens up the facilities for doing that. The changes are as follows: - Implement `bind()` in `StackSynthesizer`, and add a getter called `boundStack` which will return the bound stack. - Change `synthesizeStackTemplate -> synthesizeTemplate`, `emitStackArtifact -> emitArtifact`, both of which will implicitly operate on the bound stack. - Add `addBootstrapVersionRule` so that any subclass can choose to use this or not. - Expose `AssetManifestBuilder`, which stack synthesizers can use to build an asset manifest. Refactor the class to do less at the same time. - Expose `cloudFormationLocationFrom...Asset` functions to translate a published asset location into a CloudFormation location. - Document the contract of each method better. The end result is that it will be easier to mix and match the different behaviors of the existing synthesizers together into new custom behavior. Two new unit test files have been added to show how it is possible to write ONE intruction to the asset manifest (to upload assets to a given location) while returning a completely different instruction to the template (referencing the asset bucket using a CloudFormation parameter). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
StackSynthesizers used to make heavy use of private APIs, which make it impossible for people outside the CDK core framework to implement their own Stack Synthesizers and modify the way assets are referenced. This question comes up more and more, so this PR opens up the facilities for doing that.The changes are as follows:
bind()inStackSynthesizer, and add a getter calledboundStackwhich will return the bound stack.synthesizeStackTemplate -> synthesizeTemplate,emitStackArtifact -> emitArtifact, both of which will implicitly operate on the bound stack.addBootstrapVersionRuleso that any subclass can choose to use this or not.AssetManifestBuilder, which stack synthesizers can use to build an asset manifest. Refactor the class to do less at the same time.cloudFormationLocationFrom...Assetfunctions to translate a published asset location into a CloudFormation location.The end result is that it will be easier to mix and match the different behaviors of the existing synthesizers together into new custom behavior.
Two new unit test files have been added to show how it is possible to write ONE intruction to the asset manifest (to upload assets to a given location) while returning a completely different instruction to the template (referencing the asset bucket using a CloudFormation parameter).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license