feat(lib)!: add asset staging core with no bundling - #433
Conversation
jsteinich
left a comment
There was a problem hiding this comment.
AssetStaging itself looks good — hashing eager, stage(targetPath) taking the path from the caller, and the cache scoped per construct-tree root rather than module-global. Two notes on how TerraformAsset consumes it.
e148ca9 to
f5673a2
Compare
### Related issue Follow-up from #380. ### Description #380 claimed asset staging lands as a sibling of `stacks/`, breaking once uploaded to a hosted runner. Traced it: `TerraformAsset` already stages inside `stacks/<stack-id>/`, and has since before #371. The bug only ever existed in the old, never-merged spike's `AssetStaging`, which #433 didn't carry forward. No code fix needed. Adds a regression test pinning this contract so it can't regress silently later (e.g. in #339). ### Checklist - [x] I have updated the PR title to match [CDKTN's style guide](https://github.com/open-constructs/cdk-terrain/blob/main/CONTRIBUTING.md#pull-requests-1) - [x] I have run the linter on my code locally - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the [documentation](https://github.com/open-constructs/cdk-terrain-docs/tree/main/content) if applicable - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works if applicable - [x] New and existing unit tests pass locally with my changes
jsteinich
left a comment
There was a problem hiding this comment.
Both points addressed — and the custom-packaging test pins exactly the right thing.
Two resulting behaviour changes need the breaking marker, same as #371 got:
-
IAssetPackaging.omitsDirectoryEntriesis a new required member on an interface that has been published since #371 (v0.25.0-pre.21onward). jsii behavioural interfaces require implementers to supply every member, so an external implementation stops compiling. -
Custom
assetHashvalues are now validated for everyTerraformAsset. They were returned verbatim before; nowSAFE_ASSET_HASHrejects anything outside[A-Za-z0-9_.-], soassetHash: "feature/my-branch"throws where it used to work. Right change — it just needs to reach the changelog.
No version cost to marking it: the manifest is at 0.24.0 with bump-minor-pre-major, and #371 already landed as feat(lib)!:, so this cycle targets 0.25.0 either way. Release-please applies the highest bump once rather than compounding per commit.
Second slice of the asset pipeline (foundation types landed in open-constructs#371). Gives AssetHashType.OUTPUT, exclude, and extraHash real behavior in TerraformAsset, backed by a new AssetStaging construct. No Docker/bundling. BREAKING CHANGE: IAssetPackaging gains a required member omitsDirectoryEntries; external implementations must add it. Custom TerraformAsset assetHash values are now validated against /^[A-Za-z0-9_.-]+$/ and throw if they contain other characters (e.g. "feature/my-branch").
f5673a2 to
65be865
Compare
Related issue
Spin-off from #339
Description
Second slice of the asset pipeline (foundation types landed in #371). Gives
AssetHashType.OUTPUT,exclude, andextraHashreal behavior inTerraformAsset, backed by a newAssetStagingconstruct — no Docker/bundling code, which stays scoped to #339.AssetStaging— resolvesSOURCE/CUSTOM/OUTPUThashing (OUTPUTno longer throws: without a bundler, an asset's "output" is its source verbatim) withexclude/ignoreStrategy/extraHash, and astage()method that packs content honoring the same exclusions used for hashing. Hashing happens eagerly in the constructor; the filesystem side effect is deferred tostage(), called from the owning construct'sonSynthesize.TerraformAssetgainsexclude/extraHashconfig options. Routes throughAssetStagingonly when either is set, so existing simple usage is unchanged — no new construct child, no behavior change. Also fixes a latent gap: packing now receives the same ignore strategy used for hashing, so excluded files are actually absent from the staged/zipped output, not just the hash.AssetStagingnever changes packaging based onexclude/extraHash— a directory asset stays a directory unless the caller explicitly asks forARCHIVE. This is a deliberate departure from the original feat(lib): Initial Asset Pipeline implementation #339 spike, which silently zipped directories once any advanced option was set.Appcontent-keyed cache for theSOURCE/OUTPUThash walk (same asset referenced from multiple resources/stacks hashes once), andASSET_HASH_SALT_CONTEXT_KEYfor app-wide cache-busting alongside the existing per-assetextraHash. Both ported from feat(lib): Initial Asset Pipeline implementation #339, minus its Docker-bundling coupling.Held back for #339:
IAssetBundler, Docker bundling, and theILocalBundling/bind-mount/volume-copy execution.Not in scope here: the publisher layer (
IAssetPublisher/AssetReference) and fixingTerraformAsset's staging directory to live inside the stack directory (#380's concern #1) — both pre-existing, separate concerns.Checklist
BREAKING CHANGE: IAssetPackaging gains a required member omitsDirectoryEntries; external implementations must add it. Custom TerraformAsset assetHash values are now validated against
/^[A-Za-z0-9_.-]+$/and throw if they contain other characters (e.g. "feature/my-branch").