feat(deploy-check): replace the duplicated render script with one tested package - #94
Merged
Merged
Conversation
…ted package Rendering a deployment and scoring it against SC-11 was implemented twice in bash: once in actions/deploy-preview/run.sh and once as platform/render-local.sh, roughly 400 lines copied into each of seven service repositories. The copies disagreed. The CI version read deployment.yml with yq and consulted the artifact contract; the per-repo version re-derived the same answers by grepping raw text. The per-repo version also could not run at all -- it passed a directory where --output names a file, and invoked artifact leak-scan and artifact validate-raw-manifests, neither of which the toolkit publishes. tools/deploy-check now holds that logic once. The scorecard is a pure function over parsed documents, the toolkit invocations live behind one wrapper, and the preview comment's workload, route and endpoint counts come from the same parsed documents as the scorecard rather than from a separate pass of yq and jq. deploy-preview/run.sh drops from 579 lines to 172 and keeps only what is specific to running inside Actions: pulling the context package, emitting the gate summary, and posting the sticky comment. It runs the checker from the checked-out workflow SHA, not from the registry, so a publish failure cannot break the deployment gate and CI always executes the code belonging to the pinned tag. Two behaviours are deliberately different from the bash they replace: - Rollback retention is judged per workload. The predecessor counted acknowledgements across all workloads and took the highest minimumDays, so one compliant workload vouched for the rest. Verified a no-op against all seven current deployment.yml files before tightening. - Gatus endpoints are counted from the rendered fragment's entries rather than by counting matching files, which previously reported 0 or 1 regardless. Failures now carry a reason, so a red check names the workload, host or file at fault instead of leaving the reader to re-derive it. The bash-specific tests are removed and their intent is preserved in the new suite: the --output file form, the five fragments each rendering to their own path, emit-contract's required flags, refusal to invoke an unpublished subcommand, and an empty manifest tree reporting not_applicable rather than pass. Tests that cover deploy-artifact, which is still bash, are retargeted to it rather than deleted.
This was referenced Aug 20, 2026
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
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.
Rendering a deployment and scoring it against SC-11 was implemented twice in bash: once in
actions/deploy-preview/run.sh, and once asplatform/render-local.sh— roughly 400 lines copied into each of seven service repositories.The copies disagreed about the same repository. The CI version read
deployment.ymlwithyqand consulted the artifact contract; the per-repo version re-derived the same answers by grepping raw text. The per-repo version also could not run at all: it passed a directory where--outputnames a file, and invokedartifact leak-scanandartifact validate-raw-manifests, neither of which the toolkit publishes.Why a separate package rather than a subcommand of the toolkit
The obvious home would be
deploy-config-schema, since it already owns rendering. It is the wrong home.validateClusterContextcompares the toolkit's ownpackage.jsonversion against the context'sspec.schemaVersionwith strict string equality:So any toolkit release, including a patch, breaks every consumer until the OCI context is republished at the matching version and all seven repositories bump their pins. Putting developer tooling there would couple every tooling fix to a fleet-wide context republish.
tools/deploy-checklives here instead, versioned with this repository's existing release tags — the same tags the service repos already pin for the workflows.Why here rather than a new repository
The CI half of the duplication already lives here. Keeping the shared logic in the same repository as the action that consumes it means the two cannot drift again, and it adds no repository to maintain. The action runs the checker from the checked-out workflow SHA, not from the registry, so a publish failure cannot break the deployment gate and CI always executes the code belonging to the pinned tag. The published package exists purely so developers can run the same checks locally.
npxalso sidesteps a practical problem: only three of the seven service repos have apackage.json, so a devDependency would not work uniformly. Node is already required, since the toolkit is an npm package.What changed
tools/deploy-check—computeScorecardis a pure function over parsed documents; every toolkit invocation sits behind one wrapper; the preview comment's workload, route and endpoint counts come from the same parsed documents as the scorecard instead of a separate pass ofyqandjq.actions/deploy-preview/run.sh— 579 lines down to 172, keeping only what is specific to Actions: pulling the context package, emitting the gate summary, posting the sticky comment.ci.yml— aDeploy Check Testsjob, gatingPipeline Complete.release.yml— publishes the package at the release tag.Two deliberate behaviour changes
Rollback retention is judged per workload. The predecessor counted acknowledgements across all workloads and took the highest
minimumDays, so a single compliant workload vouched for the rest. Confirmed a no-op against all seven currentdeployment.ymlfiles before tightening.Gatus endpoints are counted from the rendered fragment's entries, not by counting matching files, which previously reported 0 or 1 regardless of content.
Failures now carry a reason.
knowledgereportsno health.path on knowledge-ingest-workerwhere the bash reported a barefail.Verification
Run against all seven service repos with the current public context: six pass, and
knowledgefails on the one real gap already known — its ingest worker declares no health block. Five fragments render, the apply bundle builds, and the contract is emitted with a render hash in every case.40 unit tests. Each was checked against a reintroduced defect rather than assumed sound: restoring the directory
--outputfails four of them, restoring the vacuouspassfails two, and flipping rollback retention back to any-of fails one.The bash-specific tests are removed and their intent preserved in the new suite — the
--outputfile form, five fragments each rendering to their own path,emit-contract's required flags, refusal to invoke an unpublished subcommand, and an empty manifest tree reportingnot_applicable. Tests coveringdeploy-artifact, which is still bash, are retargeted to it rather than deleted; the Python suite goes from 183 to 142 tests with no loss of coverage over code that still exists.Follow-up
A companion PR per service repo replaces the 400-line
platform/render-local.shwith a thin wrapper around this package.deploy-artifactstill carries its own bash render loop and is a candidate for the same treatment; it is left alone here to keep this diff reviewable.