Repository navigation
Add integrity metadata to TypeScript bundles - #72046
Conversation
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Thanks a lot for the improvement!
A separate coordinator/spec PR will add reading, digest verification, corruption rejection, caching, and the formal Task SDK format documentation.
It's necessary to implement in this PR as we already wired the e2e test up.
|
Regarding the coordinator/spec scope in this review: agreed. I’ve combined the coordinator reader, digest verification, corruption rejection, caching, format documentation, and tests into this PR locally so the existing E2E path can consume the new bundles. I’ll update the PR description when I publish the revised branch. Drafted-by: Codex; reviewed by @shivaam before posting |
988063f to
77458fe
Compare
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
I will mark the PR as draft for now, please feel free to mark it as ready to review once resolve the CI failure and the coments, thanks.
77458fe to
eeb46e5
Compare
Embed checksummed metadata and executable byte ranges so the coordinator can validate packed bundles without executing user code. Select the first verified bundle containing the requested Dag, with useful diagnostics for rejected candidates. Keep parsing and integrity checks in a private reader, document the single-version JSON contract, and cover malformed bundles, cache invalidation, routing, and corruption rejection.
eeb46e5 to
888ac79
Compare
Make private names and purpose comments explain declared sections, computed digests, file identity, and cache bounds. Inline two shallow validators so the read path is easier to follow, without changing validation behavior or error messages.
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Thanks for the follow-up.
I just found our that there're more necessary changes to make the TypeScript bundle spec make sense for the e2e feature. Since I will actively work on TypeScript SDK refactoring recently, I will follow-up them myself.
| """Return the first verified configured bundle that declares *dag_id*.""" | ||
| rejected: list[tuple[pathlib.Path, str]] = [] | ||
| for root in bundles_root: | ||
| candidate = root / BUNDLE_FILENAME |
There was a problem hiding this comment.
Just found out that we hard coded the artifact file name.
| @@ -0,0 +1,24 @@ | |||
| //# airflowBundle={"code":{"start":"0000000000000203","end":"0000000000000592","sha256":"f814358e0d4aa5d38c10c365515179049171bb9d4bba5b80c06ea549d6f16337"},"metadata":{"start":"000000000000013e","end":"0000000000000202","sha256":"a51dfd6f0c9e8ea867900e55c0387b556d3cb0e98321b62d4625f522ed465041"}} | |||
There was a problem hiding this comment.
I just realized the .mjs here isn't the min js.
From my perspative, we should compile into .min.js so that user ourself only need to deploy one artifact instead of multiple artifacts in the DagBundle.
The shape I imagined of .mjs is
- the current metadata layout as comments
- have a multiple line comments that comment out the visible entry point file source
- then the actual compiled min JS content (should be human unreadable)
Otherwise the code in the airflowBundle means nothing.
This PR adds integrity checks to TypeScript Dag bundles and verifies them before the Node coordinator starts task execution.
airflow-ts-packproduces a directly executablebundle.mjscontaining:airflowBundlecomment with byte ranges and SHA-256 digests.airflowMetadatacomment describing the SDK, Dags, and task IDs.Both comments contain compact JSON, with no base64 encoding. Metadata and executable bytes have independent digests.
airflow_bundle_metadata_versionversions the bundle contract; the header has no separate version field.The Python reader handles bounded reads, framing validation, digest verification, and a bounded file-identity cache. The coordinator selects the first usable configured bundle declaring the requested Dag. It skips corrupt or incompatible candidates, includes wrong-Dag candidates in rejection diagnostics, and retains traceback logging. The format is documented in
task-sdk/docs/ts-bundle-spec.rst.The digests detect corruption, not authenticity: someone who can replace the bundle can also replace its digests. The documented verify-to-execution race and filesystem cache assumptions remain deployment limitations.
Author-written source embedding is deliberately deferred. The
sourcemetadata field is a logical filename, not retrievable source content. This follows the narrower integrity-focused direction discussed in the issue clarification, rather than implementing every AFBNDL01 feature. The unreleased metadata-first prototype is not supported.Validation includes cross-language golden fixtures, malformed framing, metadata/code corruption, cache invalidation, direct Node execution, and ordered multi-bundle selection. A real PostgreSQL/LocalExecutor environment with scheduler and standalone Dag processor exercised the SDK example, overlapping bundles, corruption rejection before any Node launch, fallback, exact restoration, and recovery. Host UI-port forwarding was unavailable in Colima; backend health and execution were verified through the internal API.
After the final rebase, all 166 coordinator tests and 237 TypeScript tests passed, along with TypeScript build/lint/typecheck, focused formatting, Ruff, Task SDK mypy, and spelling checks. The full daemon run used the identical contribution code before the final unrelated base refresh. Independent review checked both the original and rebased diff.
Related: #69905
Was generative AI tooling used to co-author this PR?
Generated-by: Codex following the guidelines
Drafted-by: Codex (no human review before posting)