feat(release): application release command family - #336
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 |
bd8ed3d to
7d71b5c
Compare
The release contract logic moves here from fsl_libs' .github/scripts (bash), per the doctrine that fslabscli owns CI logic; the fsl_libs workflows become thin invocations of a pinned fslabscli (fslabs/fsl_libs#4230). This also makes fslabscli the single owner of the library-vs-application release boundary: `release classify` shares the publish- tag-prefix constant with the publish path, so a library release this tool creates is skipped by the app pipeline by construction. This PR is the pipeline CORE - everything a release cannot ship without; the operational periphery (resolve, record, healthcheck, cleanup-drafts) stacks on top in a follow-up PR. - classify: library skip vs validated <app>-<version> app release, keyed on the TAG (this tool leaves library release names null); apps are discovered from [package.metadata.fslabs.release] across every git-tracked manifest, INCLUDING excluded workspaces (fdk_apps holds the only shipped app). - verify-production: revision resolved from the tag itself (never target_commitish, which is a branch name), ancestry of main, workspace version binding, green check runs. - probe-store: the conditional-write go/no-go gate against a deployed store. Object lock plus versioning stops deletion but NOT overwrites; only conditional writes make published objects immutable. - publish: completeness against configured targets (missing OR extra artifacts fail; every linux artifact needs its .asc), signature re-verification against the bytes (osslsigncode per MSI, gpg against the PUBLISHED key for linux; DMG is stapler-validated on the mac host), digesting, monotonicity derived from COMMITTED MANIFESTS (never index.json), artifact writes read back and digest-compared, the manifest written LAST as the atomic commit point, index compare-and-swap. A refused overwrite propagates as-is: the version is spent. - promote: the only writer of channels.json. Production-manifest gate ("previews never"), per-target artifact presence, selected-artifact digest re-verification, and a pure apply_move whose backward gate re-runs INSIDE the CAS retry loop; acknowledged backward moves (the rollback procedure) record backward:true plus the reason in a capped provenance log. - bundle-linux: cargo-deb plus a bundle-nothing AppImage (pinned appimagetool by sha256) with the glibc floor asserted from the binary's versioned symbols; no versioned glibc symbols at all is an error, not a pass. - sign-linux: temp GNUPGHOME, detach-sign, self-verify, fingerprint out. types.rs is the single source of truth for the manifest / index / channels.json shapes; store.rs holds the storage semantics on the existing opendal dependency; http.rs and keyring.rs are the one HTTP client and one gpg keyring every command shares. New deps: semver, thiserror. All commands run on Linux runners, so the existing linux-musl release asset remains the only distributed binary. Proven end to end with the built binary against a throwaway MinIO (publish 4 artifacts + manifest-last; re-publish refused; backward move refused then acknowledged with provenance) and against the real fsl_libs tree (classify discovers spatial_engine from metadata in the excluded workspace). Integration defects found by those live runs, invisible to the unit tests: toml 0.9 document-vs-value parsing, clap's propagated auto --version flag colliding with --version arguments, and opendal expressing S3 If-None-Match:* as if_not_exists. Known wart: main.rs requires a git root before dispatch; a follow-up. The legacy publish.binary installer/channel model in check_workspace/binary.rs is superseded by this family; deprecation is follow-up after the fsl_libs cutover. cargo test: 311 passed. clippy: zero diagnostics.
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>
7d71b5c to
8072722
Compare
JulianRuiseco
left a comment
There was a problem hiding this comment.
Inline findings on the current head.
| ); | ||
|
|
||
| let passed = cases.iter().all(|c| c.passed); | ||
| Ok(ProbeStoreResult { |
There was a problem hiding this comment.
[P1] Make a failed probe exit nonzero
passed can be false while this returns Ok, and the CLI maps every Ok result to exit code 0. A store that ignores stale If-Match therefore prints FAIL but succeeds in automation. Return an error after assembling the report, or otherwise make a failed verdict exit nonzero.
| .cas_update( | ||
| &format!("{}/channels.json", options.app), | ||
| Some(initial), | ||
| |doc: Channels| { |
There was a problem hiding this comment.
[P2] Refuse incompatible live channel documents
The CAS reads the current object directly into Channels without checking schema_version or app. Serde ignores unknown fields, so an older CLI can accept a future schema and reserialize it with new fields removed. Validate identity and schema on every fresh read, or preserve unknown fields through the update.
Objective
fslabscli is the de facto location for CI logic. This PR gives it the
application release pipeline: the logic behind fsl_libs' release workflows
(fslabs/fsl_libs#4230 rewrites them as thin, pinned invocations of these
commands), which previously lived as bash under that repo's
.github/scripts/.It also makes fslabscli the single owner of the library-vs-application
release boundary:
release classifyshares thepublish-tag-prefixconstant with the publish path, so a library release created by
fslabscli publishis skipped by the app pipeline by construction.Solution
This PR is the pipeline CORE - everything a release cannot ship without.
The operational periphery (
resolve,record,healthcheck,cleanup-drafts) stacks on top in a follow-up PR so each unit staysreviewable.
A
releasesubcommand family (cargo fslabscli release <cmd>):classify<app>-<version>app release; app discovery from[package.metadata.fslabs.release]across every git-tracked manifest, including excluded workspacesverify-productiontarget_commitish), ancestry of main, workspace version binding, green check runsprobe-storepublishpromotebundle-linuxsign-linuxThe manifest / index / channels.json serde definitions in
release/types.rsare the single source of truth for the published objectshapes; storage semantics live in
release/store.rson the existingopendal dependency (put-if-absent via
if_not_exists, ETag If-Match CASwith the transform re-run on every retry). New deps:
semver,thiserror. All commands run on Linux runners, so the existinglinux-musl release asset remains the only distributed binary.
This family supersedes the legacy
publish.binaryinstaller/channel modelin
check_workspace/binary.rs(nightly/alpha blob channels); deprecatingand removing that is follow-up work after the fsl_libs cutover.
Size
One squashed commit, ~3.4k lines: seven subcommands, their 45 unit tests
(the ported specification of the bash this replaces), and the shared
contract types / storage / HTTP / keyring modules. The doc comments carry
the published contract (why the manifest is written last, why
monotonicity ignores index.json, why a backward move re-validates inside
the CAS loop) and are load-bearing. Review path: types.rs + store.rs
first (the contract), then classify/publish/promote (the decisions), then
bundle/sign/probe (tool wrappers). The stacked periphery PR is ~1.3k.
Testing
cargo test: 324 passed (58 in the release module: classificationgrammar and tag-boundary scenarios, conditional-write semantics, the
backward-move gate and provenance cap, manifest completeness both
directions, glibc symbol parsing, api-url normalisation).
sign-linux -> publish 1.0.0 and 1.1.0 (six objects each, manifest last;
re-publishing 1.0.0 refused with the version-spent error) -> promote
latest -> unacknowledged backward move refused, acknowledged one records
backward: true+ reason -> resolve--downloaddigest- andGPG-verifies against the published key -> healthcheck reports the
surface healthy, and names both digests when an object is tampered.
release classifydiscoversspatial_engine from its metadata in the excluded fdk_apps workspace,
routes production/preview correctly, and skips
publish-*releasesincluding null-name ones.
invisible to unit tests): toml 0.9 document-vs-value parsing, clap's
propagated auto
--versionflag colliding with--versionarguments,and opendal expressing S3 If-None-Match:* as
if_not_exists.Known wart:
main.rsrequires a git root before dispatch, sorelease resolvepanics outside a checkout; relaxing that touches sharedcode and is left for a follow-up.
Release
After merge, tag a release so fslabs/fsl_libs#4230 can set its pin
(version + binary sha256) in
.github/actions/install-fslabscli.