Skip to content

feat(lib): add support for single-file bundler outputs - #464

Merged
jsteinich merged 1 commit into
open-constructs:mainfrom
eduardomourar:feat/asset-pipeline-followup
Oct 1, 2026
Merged

jsteinich merged 1 commit into
open-constructs:mainfrom
eduardomourar:feat/asset-pipeline-followup

Conversation

@eduardomourar

@eduardomourar eduardomourar commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

Spin-off from #339, per the core split in #380.

Description

Lets a bundler produce a single-file artifact (a tarball, a deterministic .zip) instead of only a directory tree, so a directory source can bundle into an AssetType.FILE.

  • Add BundleResult as the IAssetBundler.bundle() return type (replacing the bare directory string), carrying the artifact path and its shape via the new BundleOutputType enum
  • Add a decline protocol: a bundler returns BundleResult.declined() when it cannot run in the current environment, and ChainBundler falls through to the next bundler
  • Add IAssetBundler.outputFileName so a single-file artifact is staged under a declared name; thread it through TerraformAsset.fileName
  • Replace compile-time bundler/packaging validation with runtime validation in AssetStaging.validateOutputShape(), which checks the declared output type against the filesystem and against the packaging (file output needs AssetType.FILE, directory output needs directory-accepting packaging)
  • Skip archive framing when hashing a single-file output
  • Add error types for output-shape validation failures
  • Cover the new validation, decline protocol, and file-vs-directory hashing in the asset-staging and canonical-hash test suites

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

@eduardomourar
eduardomourar requested a review from a team as a code owner September 28, 2026 22:30

@jsteinich jsteinich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closes the two items the bundler harness flagged as capability gaps — archive-shaped artifacts and the decline protocol — and ChainBundler is careful in the places that matter: folding every leg's key so identity cannot depend on which leg runs, rejecting conflicting outputFileName values at construction, and throwing when all legs decline. validateOutputShape checks the declared type against both the filesystem and the packaging on both branches. CI is green.

One blocker and a few smaller things, below. Two more that do not anchor anywhere in particular:

Validation moved entirely to synth time. The constructor guard is gone, so an incompatible bundler/packaging pair now fails inside stage() — after the bundler has run, which for a Docker build is minutes rather than milliseconds. Some of that is unavoidable, since the output shape is not known until bundle() returns. But outputFileName is declared statically: a bundler that sets it is announcing FILE output, so checking at construction that it pairs with file-accepting packaging would catch the common misconfiguration immediately and leave validateOutputShape as the backstop for the dynamic case. That is the same code site as the traversal fix below.

BundleResult.declined() reports outputType: DIRECTORY as a placeholder. Gated by isDeclined, so harmless today, but anyone reading outputType without checking it first gets a wrong answer.

Comment thread packages/cdktn/src/terraform-asset.ts
Comment thread packages/cdktn/src/assets.ts
Comment thread packages/cdktn/src/assets.ts Outdated
Comment thread packages/cdktn/src/asset-staging.ts
@eduardomourar
eduardomourar force-pushed the feat/asset-pipeline-followup branch from 78c9be8 to abe2310 Compare September 30, 2026 00:46
@eduardomourar

Copy link
Copy Markdown
Contributor Author

Thanks for the review. All addressed:

  • outputFileName traversal — added a filename guard (rejects empty, ., .., absolute, /, ) via assetBundlerOutputFileNameInvalid, matching the SAFE_ASSET_HASH precedent. Tests cover traversal, bare .., leading /, and Windows paths.
  • Static FILE check — a bundler declaring outputFileName now must pair with AssetType.FILE at construction; validateOutputShape stays the dynamic backstop.
  • declined() outputType — now undefined instead of a placeholder DIRECTORY.
  • Raw errors — the three ChainBundler throws now use errors.ts factories.
  • Fallback equivalence — documented that legs must produce equivalent output, with OUTPUT hashing for divergent legs.
  • hashOutput cleanup — wrapped the eager build so a throw reclaims the scratch immediately. Folded it in here since it was small; happy to split if you'd rather.

@jsteinich jsteinich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous round all landed — SAFE_OUTPUT_FILE_NAME (verified against traversal, absolute, separator and ./.. inputs), the construction-time packaging check, the ChainBundler error factories, the equivalence caveat in the docs, and hashOutput using catch rather than finally so the success path still hands its scratch to stage().

Two more, one blocking.

Comment thread packages/cdktn/src/assets.ts
Comment thread packages/cdktn/src/assets.ts Outdated
@eduardomourar
eduardomourar force-pushed the feat/asset-pipeline-followup branch from abe2310 to c37671a Compare September 30, 2026 14:04
@jsteinich
jsteinich merged commit 3592db0 into open-constructs:main Oct 1, 2026
497 of 518 checks passed
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 this pull request may close these issues.

2 participants