feat(release): operational periphery - resolve, record, healthcheck, cleanup-drafts - #337
Open
JulianRuiseco wants to merge 1 commit into
Open
feat(release): operational periphery - resolve, record, healthcheck, cleanup-drafts#337JulianRuiseco wants to merge 1 commit into
JulianRuiseco wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
fslabs-bot
Bot
added
the
size/XXL
…cleanup-drafts Stacks on the pipeline core. Nothing here gates a release shipping; these are the commands operators and clients use around one: - resolve: the client download-resolution contract in executable form - pointer lookup with no fallback, schema_version gates that refuse rather than guess, streamed download with incremental sha256, detached signature verified against the published key whose fingerprint must match the manifest's. Uses the shared keyring; proven end to end against a throwaway MinIO (digest and GPG verification both green, and the no-pointer case fails with the exact contract wording). - record: best-effort fsl_sling recorder by reference (token mint, find-or-create release, per-artifact by-reference registration, draft flag cleared). Normalises the /api prefix the pre-fslabscli implementation missed; never touches sling's channel routes, whose alpha/beta/prod set membership cannot express a per-target pointer. - healthcheck: credential-less verification of the published surface; deserializing into the contract types IS the schema validation; digest-verifies every pointed artifact; builds the full report before failing so one run names every problem. - cleanup-drafts: dry-run by default over the legacy asset-free draft backlog; deletes only with --delete after review. Also restores the ops-only shared helpers the core trimmed: http::request/get_to_file/head_present and keyring::fingerprints. cargo test: 324 passed. clippy: zero diagnostics.
JulianRuiseco
pushed a commit
that referenced
this pull request
Aug 27, 2026
Review of the release command family surfaced four defects that each let a failure through silently, plus the supporting should-fixes. All sit in the #336/#337 stack and none change the published object shapes. bundle-linux could never have succeeded. cargo-deb's default output is `target_dir_base.join("debian")` with no target triple (config.rs `default_deb_output_dir`; the triple is appended only by `target_dependent_path`, which the deb output does not go through), so `--target` does not move the artifact and reading a tripled path was an unconditional ENOENT. `-o` is now passed explicitly at the per-triple directory, which is cleared first and from which exactly one .deb must emerge: the previous take-the-first-by-sort-order would ship a stale build. The production check-run gate was satisfied by the job it runs inside. `conclusion` is null until `status` reaches `completed`, so a queued or running check read as green, and this command's own check run is attached to the tagged commit, making the count-based no-checks refusal unreachable. The gate now takes named `--required-checks` and demands completed-and-success for each. BREAKING: `--required-checks` is mandatory unless `--skip-check-runs` is passed. fsl_libs supplies it via `vars.RELEASE_REQUIRED_CHECKS`, defaulting to `test`. Failure is considered only for the required names. A commit on main routinely carries unrelated bad check runs: measured on fsl_libs main, 20 check runs including a failed docker_build and several cancelled by concurrency. Refusing on any bad run anywhere would block every release. The same commit carries two runs named `test`, one failed and one successful, because the postsubmit and the nightly both attach to main's SHA, so the best outcome per name wins. resolve built the manifest URL from `channels.manifest_base`. channels.json is written by the promotion credential, whose stated invariant is that it cannot write artifacts; sourcing the manifest location from inside it hands over exactly that power, and a `kind: authenticode` entry then skips client signature verification on every platform. Resolution now uses the client's own production base, promote re-asserts the field on every write rather than only at creation, and the docs state it is informational. probe-store could not fail on the property it gates. Its only If-Match coverage asserted that a successful CAS update succeeded, which a store ignoring the precondition also does. A stale-ETag write is now attempted and must be refused. Note this may legitimately FAIL against the deployed MinIO, which is the point: publication must not be enabled until it passes. Also: publish verifies detached signatures, which are immutable once written but carry no manifest digest and so were never read back; artifacts stay compared against the digest the MANIFEST records rather than one recomputed from the bytes just sent, which would be self-consistent by construction. key_from_url takes the last bucket segment, since a base URL containing the bucket name yielded a doubled prefix that surfaced only after the artifacts were immutable. assert_monotonic parses the candidate version before the comparison loop, which an empty published set skipped entirely, permanently bricking future publishes. healthcheck distinguishes a definite 404 from a transport failure via a typed status error rather than matching on message text, and treats only 404 as absent: 403 against an anonymously readable bucket is a broken read policy, and calling it healthy let the workflow close its own tracking issue during an outage. license_macos is omitted when absent rather than serialized as null, which jq -r renders as the string "null". sign-linux feeds the passphrase over --passphrase-fd 0 instead of the argv, never echoes gpg's arguments into an error, and writes stdin concurrently with draining stdout so a full pipe buffer cannot deadlock the signing job. 15 new tests, 339 passing. The check-run gate is extracted to a pure function so the still-running, satisfied-by-itself and unrelated-failure cases are covered directly. Signed-off-by: Loïs Postula <admin@conceptions.postu.la>
JulianRuiseco
force-pushed
the
jrr/release-commands-ops
branch
from
August 27, 2026 17:08
3314202 to
ce684e2
Compare
JulianRuiseco
commented
Aug 27, 2026
JulianRuiseco
left a comment
JulianRuiseco
left a comment
Contributor
Author
There was a problem hiding this comment.
Inline findings on the current head.
|
|
||
| let candidates: Vec<DraftRecord> = releases | ||
| .into_iter() | ||
| .filter(|r| r.draft && r.assets.is_empty()) |
Contributor
Author
There was a problem hiding this comment.
[P2] Restrict deletion to legacy publish drafts
This selects every old asset-free draft in the repository. Running with --delete can remove an unrelated manual or application draft even though the command is scoped to the legacy publish backlog. Require the expected publish-* tag shape before including a candidate.
| .with_context(|| format!("cannot import the published signing key from {key_url}"))?; | ||
| let keyring = TempKeyring::import(&key) | ||
| .with_context(|| format!("cannot import the published signing key from {key_url}"))?; | ||
| if !keyring.fingerprints()?.iter().any(|f| f == key_fingerprint) { |
Contributor
Author
There was a problem hiding this comment.
[P1] Bind verification to the manifest fingerprint
The expected fingerprint only needs to appear somewhere in the imported bundle, while gpg may verify the signature with another imported key. A two-key probe confirms that a signature from key B succeeds when the manifest names key A. Parse the VALIDSIG status, compare the actual primary signer fingerprint, and apply the same binding during publication.
| "checksum": artifact.sha256, | ||
| "content_type": "application/octet-stream", | ||
| }))?; | ||
| let outcome = request( |
Contributor
Author
There was a problem hiding this comment.
[P2] Make release asset recording idempotent
A rerun reuses the existing release and POSTs every artifact again. The Sling asset repository always inserts, and the table has no uniqueness constraint for release and filename, so partial retries duplicate every previously successful asset. Upsert these records or reconcile existing assets before POSTing.
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
1 participant
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.
Objective
Stacks on #336 (the release pipeline core). The operational periphery:
nothing here gates a release shipping; these are the commands operators
and clients use around one.
Solution
release resolverelease recordrelease healthcheckrelease cleanup-draftsAlso restores the ops-only shared helpers the core trimmed
(http::request/get_to_file/head_present, keyring::fingerprints).
Testing
cargo test: 324 passed (the core's 311 plus 13 here).resolve
--download, digest and GPG verification both green; theunpromoted-channel case fails with the exact contract wording.
artifact (names both digests).
Merge after #336; fsl_libs#4230's healthcheck workflow and the publish
job's record step invoke these commands, so the release that sets the
fsl_libs pin should include both PRs.