actions/checkout writes its token into .git/config as an http extraheader,
where it stays for the rest of the job. That matters more here than usual:
pull_request jobs run on self-hosted runners and execute the PR's own build.rs,
proc-macros and tests, any of which can read that file. Test and Lint holds
actions: write on top of that, so its token can cancel runs and delete the
Actions caches the whole pipeline now depends on — it was given
persist-credentials: false when that permission was added, and this extends the
same treatment to the other 45 checkouts.
Two are exempt because the token IS the credential the job needs.
helm-package's publish job pushes to rustfs/helm with it; clearing it would
break chart publishing. nix-flake-update is exempt pending verification: it
passes FLAKE_UPDATE_TOKEN to create-pull-request directly rather than reusing
.git/config, so it very likely does not need persistence, but that is unproven
and a broken weekly bot is not worth the guess. Both carry a
persist-credentials-exempt comment saying which.
scripts/security/check_persist_credentials.sh requires every checkout to either
clear its credentials or carry that comment, so the decision stays visible in
review rather than being an omission nobody notices.
Refs: rustfs/backlog#1598, rustfs/backlog#1602
sccache can only cache compilation units whose --emit includes link. That
covers workspace rlibs and nothing else: clippy is metadata-only, and the ~100
test binaries, the rustfs bin and every build script invoke the system linker.
So the headline claim that started this — s3select-query costing 17m13s inside
a 19m32s nextest build — does not by itself justify sccache, because cargo
prints "Compiling" when a crate starts and never when it finishes: that number
is the whole compilation tail, not one crate.
Adding --timings behind a workflow_dispatch input on Cache Warm answers it with
data instead. Read two shares off the report: workspace lib codegen against the
whole build, and s3select-query's own rlib. The plan in rustfs/backlog#1601
adopts sccache only above 50% and 25%; if linking dominates instead, the answer
is mold/lld plus split-debuginfo, which is precisely the region sccache cannot
reach.
Off by default: it doubles the ci-dev build, and this only needs running once
per question. Nothing about the normal warm path changes.
Refs: rustfs/backlog#1598, rustfs/backlog#1601
Two cleanups, neither changing what is built or published.
docker.yml loses its type=gha layer cache. The image build compiles nothing —
it downloads a release zip and runs apk/apt — so the cache could only save the
minute or two those take, against a real correctness problem: with
RELEASE=latest the binary URL is resolved by curl inside a RUN layer, and the
layer key does not include what it resolved to. A rebuild at the same RELEASE
value (a dispatch with version=latest, or a re-run of the same version) would
hit the old layer and ship the previous release's binary. mode=max also drew on
the same repo-wide 10GB Actions cache quota the Rust lanes are contending for.
Only RELEASE is passed as a build-arg now; it is the sole one the Dockerfiles
declare besides TARGETARCH. BUILDTIME, VERSION, BUILD_TYPE, REVISION and CHANNEL
were read by no stage, and BUILDTIME's $(date ...) was a literal string in the
YAML block rather than a substitution. The DOCKER_CHANNEL computation that fed
CHANNEL evaluated to "release" down every branch and is gone with it. BUILD_DATE
and VCS_REF stay unset even though the Dockerfiles declare them: supplying them
would change the published image labels.
The setup composite stops installing tools its callers do not use. protobuf-
compiler comes out of the apt list entirely — setup-protoc installs 34.1 into
the tool cache and prepends it to PATH, so the apt build was shadowed on every
run and never used; the same duplicate install is removed from the io_uring lane.
musl-tools, zip and unzip move behind install-build-packaging-tools, off for the
CI and coverage lanes and left on for build.yml, whose native musl leg needs
musl-gcc and whose packaging steps need zip. cargo-nextest and the
rustfmt/clippy components move behind install-test-tools, off only for build.yml,
which runs no tests and no lints; coverage and the nightly replication lane keep
them because both invoke nextest.
The action's github-token input is deleted along with its 20 call sites. Its
runs block never referenced it — setup-protoc uses github.token directly — so it
was 20 places handing a token to something that ignored it.
Refs: rustfs/backlog#1598, rustfs/backlog#1600, rustfs/backlog#1603
* docs(kms): record the cryptographic compliance position
RustFS links no FIPS-validated cryptographic module: the rustls provider is
the ordinary aws-lc-rs build, and every data-path AEAD is RustCrypto. The
crypto crate's default-on `fips` feature only selects PBKDF2+AES-GCM over
Argon2id, with the same RustCrypto implementations behind both branches, so
it cannot support a validation claim either.
Document that status, the terminology rules for external material, the real
semantics of the `fips` feature with a rename direction, the cost of the
three routes to a stronger position, and the sequencing rules for retiring an
algorithm.
Refs rustfs/backlog#1587 (part of rustfs/backlog#1562)
* docs(kms): document the mixed-version cluster constraints
Collect the cross-version constraints that landed with versioned rotation and
the check-and-set lifecycle work: which persisted formats decode both ways,
which guarantees only hold once every node is upgraded, how long nodes can
disagree on lifecycle state, and that reconfigure is persisted cluster-wide
but applied only on the node that handled it.
Adds the recommended rolling-upgrade sequence and the list of operations to
avoid while two builds are running.
Refs rustfs/backlog#1581 (part of rustfs/backlog#1562)
Two independent changes to the Test and Lint area.
The io_uring lane compiled 7 integration binaries to run none of their tests.
Job log 91309868055 shows the lib target reporting "18 passed; 3453 filtered
out" while every binary under crates/ecstore/tests/ reported "running 0 tests".
Adding --lib drops them from the build without changing the selected set.
The `uring_` filter itself must not be touched. libtest matches on substring, so
it also selects names containing `during_` — 6 of the 18 selected tests are such
incidental matches, and narrowing the filter to `io_uring` would silently drop
them. scripts/check_uring_lane_lib_only.sh asserts the precondition --lib
depends on: no test function whose name contains `uring_` may live under
crates/ecstore/tests/. A count floor would not do, because the dangerous case —
someone adding a matching test there — leaves the lib count unchanged and CI
green.
The resource sampler was measuring the wrong machine. The runners are ARC pods,
so /proc/loadavg, /proc/pressure/*, `free` and `df` are node-level and include
every other runner pod on the same Kubernetes node: one sample reported loadavg
6.67 with 832 threads node-wide while `ps` inside the pod showed about 10
processes. Judging a CARGO_BUILD_JOBS change on those numbers cannot work.
The sampler moves to scripts/ci/resource_sampler.sh and now records
/sys/fs/cgroup cpu.max, memory.max, memory.peak and the cpu/io/memory pressure
files, which are this pod's own. The node-level readings stay — co-tenancy is a
real cause of stalls, and a 9m57s plain `git checkout` was traced to it — but
are labelled NODE-LEVEL so nobody reads them as this job's load. Phase markers
are written on start, and clippy is now sampled too: it is the natural control
arm for a CARGO_BUILD_JOBS experiment, since --all-targets is check-only for
workspace members and never links the ~100 test binaries the limit throttles.
No numbers are changed in this commit. CARGO_BUILD_JOBS stays at 2 until there
is attributable data to change it on.
Refs: rustfs/backlog#1598, rustfs/backlog#1601
nix-flake-update opens a PR every Sunday that changes flake.lock and nothing
else. flake.lock is consumed only by Nix packaging — cargo never reads it — yet
it was in no paths filter, so each of those PRs ran the full Continuous
Integration pipeline and the merge then ran Build and Release too. Added to all
four lists that have to agree: ci.yml's push and pull_request paths-ignore,
build.yml's push paths-ignore, and ci-docs-only.yml's paths.
The cron stays. flake.lock still has consumers — anyone running `nix build` or
`nix develop` — so stopping the updates would remove the workflow's output, not
just its CI cost.
Those four lists drifting is a silent failure, so scripts/check_ci_paths_sync.sh
now asserts the pair that matters: ci.yml's pull_request paths-ignore must equal
ci-docs-only.yml's paths. An entry present only in the first means a PR touching
those files triggers neither workflow, nobody reports "Test and Lint" or "Quick
Checks", and the PR waits on a required check forever. It also asserts both
companion job names still exist, since renaming one produces the same hang. The
push list is not compared: no required check is reported for push events.
Also in this cleanup:
- nix-flake-update's GITHUB_TOKEN drops to contents: read. The branch push and
the pull request are both made by update-flake-lock with the
FLAKE_UPDATE_TOKEN PAT, so the write scopes were an unused repo-write
credential on an unattended weekly job.
- build.yml's build_docker dispatch input is documented as advisory. docker.yml
triggers on workflow_run and requires the triggering event to be a tag push,
so a manual dispatch never reaches it whatever this input says.
- The nine disabled workflow files get a banner saying so. Their
disabled_manually state lives in GitHub's UI and is invisible when reading the
file, which has already misled one audit into treating dead workflows as live.
Refs: rustfs/backlog#1598, rustfs/backlog#1603
A streaming GET holds snapshot leases on the object's data directories,
and DeleteObjects defers their physical cleanup until the leases are
released. A DeleteBucket issued inside that window passes the xl.meta
emptiness check but fails closed in the non-force delete_volume tree
removal on the leftover part files, returning BucketNotEmpty for a
logically empty bucket.
This is what intermittently failed the s3tests
test_encryption_sse_c_multipart_bad_download teardown in CI: the test
never reads its 30MiB GET body, so the server-side stream (and its
leases) stays alive until the connection drops, racing the teardown's
DeleteObjects + DeleteBucket sequence. With the body held open the
failure reproduces 5/5 locally; after this change it passes 20/20, and
the real s3-tests case passes 20 consecutive runs.
delete_volume now executes the registry-tracked pending deferred
deletions for the volume before removing the directory tree. Only data
dirs whose logical delete already committed are touched; unknown files
still fail closed with VolumeNotEmpty.
Three hardening changes with no effect on what any workflow produces.
Declare timeout-minutes on the 25 jobs that lacked it. GitHub's default is 360
minutes, and this repository has a history of runners stalling intermittently
(#5394) plus a measured 9m57s plain `git checkout` under node-level I/O
contention, so one wedged job could hold a runner for six hours out of a pool of
roughly 15-21. Budgets follow what the jobs actually do: 10 minutes for
echo-only and guard-script jobs, 30 for anything calling the GitHub API,
uploading release assets or pushing over the network.
scripts/security/check_job_timeouts.sh keeps it that way, checking only jobs
that declare runs-on so reusable-workflow callers are not flagged.
Pass workflow inputs and workflow_run fields through env instead of `${{ }}`
interpolation in run blocks. A git ref name may contain `$(...)` — any string
without a space is a legal tag — and interpolation pastes it into the script
where bash evaluates it. The worst instance was helm-package's final commit
message: it is built from the triggering tag name inside the job that holds the
cross-repository push token with rustfs/helm already checked out. Also converted
in build.yml, docker.yml and performance-ab.yml; the last is currently disabled,
but a disabled workflow can be re-enabled. Not touched: helm-package's
`contains(head_branch, '.')` tag test, since GitHub expressions have no regex
and this repository's tags carry no `v` prefix, so rewriting the condition would
change which builds publish a chart.
Give audit.yml a scheduled-failure alert and run it daily. A scheduled
cargo-deny failure usually means the dependency tree just matched a newly
published RustSec advisory — the most important signal this workflow produces,
and until now it was visible only to whoever happened to open the Actions tab.
coverage.yml and e2e-replication-nightly.yml already use this ci-8 mechanism.
The cron moves from weekly to daily so a new advisory against an unchanged tree
surfaces within a day instead of seven; the check list is untouched, since
splitting it into a light daily run and a weekly full run would create runs
where sources, bans and licenses go unverified.
Refs: rustfs/backlog#1598, rustfs/backlog#1602
#5532 consolidated ci.yml's nine cache sites into four keys with one writer
each, but the writers never run to completion: ci.yml cancels superseded runs
on main, and merges land far faster than its 70-minute pipeline. Measured over
15 consecutive main pushes — 12 cancelled, 2 failed, 0 succeeded. A cancelled
run never reaches rust-cache's post step (cache-on-failure does not cover
cancellation), so nothing was being saved and every PR paid a cold restore:
11.8-20.9 minutes of "Setup Rust environment" against 0.7-3.4 warm.
Move cache writing into its own workflow whose concurrency group does not
cancel in progress, and make every lane in ci.yml a pure reader. ci.yml keeps
cancelling superseded runs, which is correct — nobody needs test results for a
commit that is already three merges behind — while the caches still get
written.
Not simply disabling cancel-in-progress for main pushes in ci.yml: that would
run several full pipelines concurrently on a 15-21 runner self-hosted pool that
is already the bottleneck, which is the opposite of the goal. GitHub keeps at
most one running plus one pending run per concurrency group, so the new
workflow collapses a burst of merges into "current finishes, newest queued
follows" and occupies one runner at a time.
The warm builds are supersets of what the reading lanes compile, because a
reader restores only what the writer saved: --all-targets for the test binaries
nextest builds (including e2e_test, which test-and-lint excludes), plus the
e2e-test-hooks and rio-v2 feature combinations, whose different feature
resolution yields a different -Cmetadata. The ci-dev build carries the same
CARGO_BUILD_JOBS limit ci.yml puts on nextest, since it links the same ~100
test binaries and three concurrent links can wedge Cargo (#5394). ci-uring is
warmed on ubuntu-latest to match its reader: rust-cache's key covers runner.os
and arch but not the runner label or image.
Refs: rustfs/backlog#1598, rustfs/backlog#1600
The repository has 18 rust-cache families of 1.2-3.1GB each against GitHub's
fixed 10GB per-repo quota. Measured usage sat at 9.72GB, 9.96GB and 11.63GB on
three samples, so LRU eviction is continuous and the main lanes lose: ci-test
and ci-e2e were repeatedly absent from the surviving entries. That is what
makes "Setup Rust environment" bimodal — 0.7-3.4 minutes warm against
11.8-20.9 minutes cold.
Four changes, all cache-only. No job builds or tests anything different.
Collapse ci.yml's nine cache sites into four keys, each with exactly one
writer: ci-dev (test-and-lint writes; ILM, debug-binary, e2e-tests and e2e-full
read), ci-feat-rio (rio-v2 lint writes, its debug-binary reads), ci-feat-proto
(the swift leg writes for both protocol legs), and ci-uring, which stays alone.
e2e-full is easy to miss here: it already shared ci-e2e with e2e-tests and both
saved, so without an explicit 'false' it would have become a second unnamed
writer of ci-dev. rio/swift/sftp are deliberately NOT merged — measured at
2365/2307/1200MB they are not near-identical, and none is a superset of the
others.
Give each writer a superset warm-up on main. A writer's own steps are not
automatically a superset of its readers': clippy emits metadata only, the
nextest pass excludes e2e_test, and no lint lane enables e2e-test-hooks, whose
different feature resolution yields a different -Cmetadata. Without these
builds the readers would restore a cache missing precisely what they need.
Guarded to main, so the PR critical path is unaffected.
Stop writing tag-scoped caches in build.yml. A cache saved on refs/tags/X can
only be restored by a re-run of that same tag, so each release cycle wrote up
to 12 unreadable 1-2GB entries that evicted the hot lanes. Tag builds still
restore the main-scoped cache. The one real cost is that re-running a failed
leg of the same tag now falls back to main's cache.
Flip the composite action's cache-save-if default to 'false' and make the input
mandatory in practice. audit.yml was relying on the old "true" default: every
PR touching Cargo.toml or Cargo.lock saved a second, PR-scoped copy (~843MB
measured, job 91048468127) that pushed main-scoped lanes out of the quota. The
fail-safe direction is a cold cache, not a stolen quota slice.
scripts/security/check_cache_save_if.sh now asserts every call site states it,
wired into audit.yml next to the existing pin check.
While there, cargo-deny stops pulling the full setup composite. It compiles
nothing, so apt, protoc, flatc and nextest were pure overhead — but it does run
cargo metadata, and Cargo.toml pins datafusion and s3s as git dependencies that
must be materialised into ~/.cargo/git, so the cache itself stays.
Refs: rustfs/backlog#1598, rustfs/backlog#1600
* ci: stop running expensive jobs that cannot inform the result
Three independent fixes that all avoid burning self-hosted runners on work
whose outcome is already determined. None of them changes what is tested.
- ci-docs-only: add a "Quick Checks" companion job. It is a prerequisite for
gating ci.yml's expensive jobs behind quick-checks (rustfs/backlog#1599):
once "Quick Checks" is a required check, a docs-only PR would otherwise wait
on it forever. The steps are a byte-identical copy of ci.yml's quick-checks
rather than an `echo`, so that on a mixed PR the two same-named check runs
execute the same commands against the same merge ref and cannot disagree —
GitHub has no written contract for how it picks between same-named required
check runs, and the real job only takes 45-51s, leaving no timing margin to
rely on.
- ci: guard uring-integration with the same `closed` check every other job
already has. The pull_request trigger includes `closed` only so the
concurrency group cancels in-flight runs; this job had no guard and no
`needs`, so every closed or merged PR ran the full io_uring suite (4m17s,
7m19s and 7m31s on runs 30678272341, 30678117601 and 30662728539).
- ci: gate s3-lifecycle-behavior-tests on e2e-tests, matching
s3-implemented-tests. Both lanes only download the prebuilt debug binary, and
s3-implemented-tests already finishes later, so a green PR's wall clock is
unchanged; a red one stops holding a sm-standard-4 for up to 30 minutes.
Refs: rustfs/backlog#1598, rustfs/backlog#1599
* ci: gate the seven expensive jobs on Quick Checks
Every expensive job started in parallel with quick-checks, so a formatting or
architecture-guard failure still paid for the full pipeline. On run
30673292690 Quick Checks failed after 0.8 minutes and the run went on to burn
424.5 runner-minutes — 99.8% of it after the gate had already failed. The
self-hosted pool is 15-21 ARC runners and one full PR run needs about seven
sm-standard-4 concurrently, so those minutes come straight out of other PRs'
queue time (six runs measured 72-488 minutes queued).
quick-checks itself is compile-free and takes 45-51s, so a passing PR pays
about a minute of extra critical path.
REQUIRES the branch ruleset to list "Quick Checks" as a required check BEFORE
this merges. Adding `needs` gives these jobs a `skipped` conclusion for the
first time, and GitHub treats a skipped required check as satisfied — with
required_approving_review_count=0, a failing quick-checks would otherwise let
a broken PR merge. Ordering is tracked in rustfs/backlog#1599.
Refs: rustfs/backlog#1598, rustfs/backlog#1599
* ci: stop a PR run once Test and Lint has failed (#5530)
On run 30674613104 the e2e, ILM and sftp lanes had all failed while Test and
Lint and the rio-v2 variant kept running past 70 minutes. The run's verdict was
settled; the remaining lanes were spending sm-standard-4 time on a result
nobody could act on, and with one full PR run needing about seven of those
runners, that time comes out of other PRs' queue time.
Two mechanisms, both scoped to pull_request so main pushes, the merge queue and
the weekly schedule keep the full failure signal:
- test-and-lint-protocols: fail-fast on PRs, so one failing protocol leg stops
its sibling. This is the only part that also covers fork PRs, since it needs
no token.
- test-and-lint: on failure, cancel the run through the REST API.
Only test-and-lint may cancel. The lanes that are not required checks
(protocols, ILM, e2e, s3-tests) must never hold that power: a flake in one of
them would turn the required "Test and Lint" into `cancelled`, which blocks the
merge. A maintainer can merge today with sftp red, and that has to stay true.
The cancel step uses curl, not `gh`: every existing `gh` call in this repo runs
on ubuntu-latest, and the sm-standard-* images are custom and trimmed, so `gh`
is not known to exist there. Fork PRs are excluded by an explicit condition
rather than left to fail, since their GITHUB_TOKEN is forced read-only and
job-level permissions cannot raise it.
Job-level permissions must list contents: read alongside actions: write —
job-level permissions replace the workflow block instead of merging with it,
and dropping contents would break this job's checkout and the repo-token the
setup action passes to setup-protoc. Because that token can now cancel runs and
delete Actions caches, the checkout also sets persist-credentials: false so a
PR's own build.rs or proc-macro cannot read it back out of .git/config.
Refs: rustfs/backlog#1598, rustfs/backlog#1599
* ci: correct the companion-workflow comments
Addresses review feedback on #5528, which merged before these fixes were
pushed.
The ci-docs-only header claimed the ruleset already requires "Quick Checks".
It does not — that ruleset change is a separate step, and this file's whole
purpose is to land first so that change does not strand docs-only PRs. Say
what is true today.
Also move the byte-identical requirement onto ci.yml's quick-checks job, which
is the more likely edit site, instead of pointing at a comment that was not
there.
Three independent fixes that all avoid burning self-hosted runners on work
whose outcome is already determined. None of them changes what is tested.
- ci-docs-only: add a "Quick Checks" companion job. It is a prerequisite for
gating ci.yml's expensive jobs behind quick-checks (rustfs/backlog#1599):
once "Quick Checks" is a required check, a docs-only PR would otherwise wait
on it forever. The steps are a byte-identical copy of ci.yml's quick-checks
rather than an `echo`, so that on a mixed PR the two same-named check runs
execute the same commands against the same merge ref and cannot disagree —
GitHub has no written contract for how it picks between same-named required
check runs, and the real job only takes 45-51s, leaving no timing margin to
rely on.
- ci: guard uring-integration with the same `closed` check every other job
already has. The pull_request trigger includes `closed` only so the
concurrency group cancels in-flight runs; this job had no guard and no
`needs`, so every closed or merged PR ran the full io_uring suite (4m17s,
7m19s and 7m31s on runs 30678272341, 30678117601 and 30662728539).
- ci: gate s3-lifecycle-behavior-tests on e2e-tests, matching
s3-implemented-tests. Both lanes only download the prebuilt debug binary, and
s3-implemented-tests already finishes later, so a green PR's wall clock is
unchanged; a red one stops holding a sm-standard-4 for up to 30 minutes.
Refs: rustfs/backlog#1598, rustfs/backlog#1599
* feat(kms): add link_durably primitive and restore-marker startup guard
The local backend gains the two pieces the bundle restore path builds on:
a no-clobber hard-link publish primitive whose AlreadyExists case is
idempotent only for byte-identical content, and a fail-closed startup
guard that refuses to open a key directory holding a restore cutover
marker. The durable commit protocol and the key-id containment check
become pub(crate) so the restore module reuses them instead of copies.
* feat(kms): record a master-key verifier and pre-seal decrypt probe in export
Fill the manifest's master_key_verifier slot with an opaque one-way
value (scheme-prefixed, bound to the backup id and the KDF salt) so a
restore can detect a wrong operator-supplied master key before touching
any target state, and probe-decrypt every artifact as stored under the
backup KEK before the manifest may seal — digest equality alone only
proves the ciphertext landed intact. The payload decryption tail is
factored out and shared with the restore side so producer and consumer
cannot drift on the framing.
Also drops the stale KmsClient test import orphaned by the backend
refactor (#5501); the test suite did not compile without this.
* feat(kms): restore local backend key material from sealed backup bundles
The consumer side of the Local bundle export, as a four-phase protocol:
- Dry-run: full in-memory bundle decode (digest and AEAD verification of
every artifact), KDF-drift detection against the compiled-in
derivation, deployment and injected-generation checks (strictly lower
is rejected, equal stays allowed for repeated drills), master-key
verifier check, and target conflict enumeration - with zero writes.
- Staging: artifacts are committed durably into the .restore-staging/
subdirectory (invisible to the backend's key scan and orphan-temp
matcher) and every record is decryption-probed with the derived
master key both in memory before staging and again from the staged
bytes.
- Commit marker + cutover: the durably published .restore-commit.json
marker is the single commit point; cutover publishes staged files via
link_durably (salt first, keys after), then durably removes the
marker and drops staging.
- Crash re-entry: before the marker the target top level is untouched
and a re-run starts over; with the marker published, backend startup
fails closed and a re-run with the same bundle rolls forward while
abort_local_restore rolls back. Every interruption converges to the
complete old or complete new state.
Restore never goes through LocalKmsClient::new (which would mint a
fresh salt); the only write mode is the explicit
restore-into-empty-target policy, where an orphan salt or a foreign
marker already counts as non-empty. The bundle source stays strictly
read-only.
Refs rustfs/backlog#1572
Bring the dormant Resource::Kms variant to life: arn:aws:kms:::key/<key_id>
patterns (empty-account form, wildcards allowed in the id, alias/<name>
reserved as parse-only) now parse, validate, serialize back, and are matched
by KMS statements against the requested key id carried in Args::object.
Statements without KMS resources keep the legacy match-every-key behaviour,
as do call sites that pass no key resource, so nothing changes until the
admin/SSE authorization paths start passing key ids.
Statement validation rejects KMS resources on non-KMS statements, and bucket
policy validation rejects KMS actions and resources outright while stored
policies keep deserializing; evaluation skips pure-KMS bucket policy
statements with a warning. A kms:Decrypt action is added for the upcoming
SSE-KMS read path.
Refs rustfs/backlog#1582 (part of rustfs/backlog#1562)
* fix(kms): drop the KmsClient trait import removed by #5501
The local backup export tests (#5499) merged after #5501 folded the
KmsClient trait into KmsBackend, leaving a dead trait import that breaks
'cargo test -p rustfs-kms' compilation on main. create_key is an inherent
method on LocalKmsClient since #5501, so the import is unnecessary.
* fix(kms): CAS Transit metadata writes and bound the metadata cache
Transit KV metadata writes were whole-record overwrites with no
precondition, so two nodes mutating the same key could silently clobber
each other's lifecycle state, and the process-local metadata cache had
neither a TTL nor a capacity bound, so a key disabled or scheduled for
deletion on one node stayed usable on every other node until restart.
- Replace write_metadata_to_kv with a versioned read
(read_metadata_from_kv_versioned) plus a check-and-set write
(cas_write_metadata_to_kv); mutate_key_metadata re-reads the
authoritative record and re-runs the state gate on every attempt, and a
lost CAS race retries with a fresh snapshot (bounded budget) instead of
replaying the stale one.
- Migrate every read-modify-write caller: enable, disable, schedule and
cancel deletion, rotate version bump, the expired-key tombstone, and
both create paths (create-only CAS that read-confirms the winner on a
lost race).
- Bound the metadata cache with moka (300s TTL, 1024 entries) and drop a
key's entry when a transit data call reports it gone server-side.
- Fail closed when the synthesized-metadata fallback cannot be read or
persisted: the fabricated Enabled record is only served after a durable
create-only CAS write, closing the gate weakening documented as a KNOWN
RISK; the persistence fallback for pre-metadata keys is kept.
Refs rustfs/backlog#1581 (part of rustfs/backlog#1562)
* fix(kms): drop stale KmsClient trait import in local_export tests
The KmsClient trait was folded into KmsBackend (#5501), but the backup
export tests merged afterwards (#5499) still imported it, breaking the
crate's test build; create_key is an inherent LocalKmsClient method, so
the import is simply unused.
* fix(kms): make Vault KV2 lifecycle writes check-and-set
Every KV2 lifecycle write used to be a blind whole-record overwrite, so
two nodes racing on the same key could lose updates: a disable racing a
rotation wrote the pre-rotation record back (rolling back the version
and material of a committed rotation), concurrent same-name creates let
the later material win (orphaning DEKs wrapped under the earlier one),
and a cancellation racing the deletion sweep could be overwritten by
the tombstone (or resurrect an already tombstoned key).
All lifecycle mutations now go through a bounded check-and-set
read-modify-write loop: each attempt re-reads the record pinned to its
KV2 secret version, re-runs the state gate against the fresh snapshot,
and writes back check-and-set against exactly that version; after
LIFECYCLE_CAS_ATTEMPTS lost races the typed conflict error surfaces.
The loop composes with the operation policy's single-attempt rule for
non-idempotent writes: each write is still sent at most once, only the
whole read-gate-write cycle repeats. create_key becomes a create-only
write (cas=0) so exactly one of two concurrent creates commits and the
loser reports KeyAlreadyExists. The blind store_key_data primitive is
now test-only.
Reads and rotation additionally fail closed when the version history is
inconsistent: resolving material through a version record above the
current pointer is refused (that state only arises when a lost update
rolled back a committed rotation), and rotation refuses to extend a
history whose records reach more than one step past the current pointer
(one step ahead is the footprint of an interrupted rotation and still
recovers through the adopt path).
Refs rustfs/backlog#1581
Add a Grafana dashboard for the four KMS backend operation metrics
emitted at the policy choke point, Prometheus alert rules with
conservative default thresholds (pending staging baseline calibration),
and an operations runbook documenting the metric contract and the
response procedure for each alert. Register the new alert rules file in
the observability READMEs.
Refs rustfs/backlog#1584 (part of rustfs/backlog#1562)
The KmsClient trait was folded into KmsBackend (#5501), but the backup
export tests merged afterwards (#5499) still imported it, breaking the
crate's test build; create_key is an inherent LocalKmsClient method, so
the import is simply unused.
* feat(kms): export local backend key material as sealed backup bundles
Adds the producer side of the Local backup series on top of the #5483
contract: a directory-wide export fence gives the snapshot a single
consistent generation, every artifact is AEAD-wrapped under a
caller-supplied backup KEK that is separate from the business trust
hierarchy, and the sealed manifest with completeness marker is written
last so an interrupted export can never be mistaken for a restorable
bundle. Restore and the admin API land in follow-up changes.
* fix(kms): verify manifest digest against raw bytes, not re-serialized fields
The decode path recomputed the digest by re-serializing the parsed
manifest, which silently assumes every field's stored spelling survives
a parse-and-reprint round trip. Timestamps do not guarantee that: the
time zone annotation jiff emits depends on the host (IANA name, POSIX
TZ string, Etc/Unknown), and the legacy-compat parser rewrites
bracket-less spellings to +00:00[UTC]. On CI this made freshly written
bundles fail digest verification while passing locally.
Digest verification now operates on the raw stored bytes, normalized
only through the JSON value layer with the digest slot emptied in
place; parsed typed fields are never re-serialized on the decode path.
Sealing uses the same value-layer canonical form, and the export
additionally pins created_at to UTC so bundles are host-independent. A
regression test seals a manifest whose created_at spelling cannot
round-trip and proves decoding still verifies. One behavior sharpens:
inserting an explicit null reserved slot after sealing is now rejected
as a digest mismatch instead of being tolerated.
* fix(kms): make manifest digest canonicalization independent of map ordering
The canonical digest form serialized serde_json values directly, which
inherits the key order of serde_json's map type: sorted by default, but
insertion-ordered when any crate in the unified build graph enables the
preserve_order feature. The workspace-wide CI build unified that
feature while a per-crate local build did not, so the frozen fixture
digest matched in one environment and not the other — and a bundle
sealed by one build flavor would fail verification in the other.
Canonicalization now rebuilds every JSON object with bytewise-sorted
keys (array order preserved) before hashing, so the digest bytes are
identical regardless of feature unification. Reproduced by enabling
preserve_order in dev-dependencies (fixture test red), then verified
green with the fix under both map flavors.
Add opt-in hotpath feature surfaces to every workspace crate and wire the root rustfs feature passthrough for function, allocation, and CPU profiling.
Add a focused set of function-level measurements for scanner, heal, lock, target replay, IAM, KMS, Keystone, trusted proxy, and capacity paths without adding request-scoped primitive wrappers.
Co-authored-by: heihutu <heihutu@gmail.com>
* feat(kms): record operation metrics in the retry policy engine
Instrument policy::execute — the single choke point every outbound Vault
call and credential exchange already flows through — so no call site
needs its own instrumentation:
- rustfs_kms_backend_operations_total (counter): operation, op_class,
outcome (success / fatal / budget_exhausted / deadline_exceeded /
cancelled)
- rustfs_kms_backend_attempt_failures_total (counter): operation,
error_class (retryable_conn / retryable_status / fatal /
attempt_timeout)
- rustfs_kms_backend_operation_duration_seconds (histogram): wall-clock
duration including retries and backoff
- rustfs_kms_backend_operation_attempts (histogram): attempts used
Metric labels carry only static enum values (operation names, classes,
outcomes) — never key identifiers, key material, ciphertext, or tokens.
Emission goes through the process-global metrics facade recorder, the
same pattern the rest of the workspace uses, so no new wiring is needed
in rustfs/src.
Tests drive a paused-clock runtime under a thread-local debugging
recorder, so counts, attempts, and even the recorded (virtual-clock)
durations are asserted deterministically with zero real sleeps.
Refs rustfs/backlog#1569 (part of rustfs/backlog#1562)
* test(kms): add Vault fault-injection matrix
Offline cases inject transport faults locally and are fully
deterministic: a refused connection is retried up to the configured
budget, and a stalled connection is cut off by the per-attempt timeout
instead of hanging. Ignored cases run against a real dev Vault
(RUSTFS_KMS_VAULT_ADDR) and pin the fail-closed auth behavior: an
invalid token and a missing key each resolve in exactly one attempt.
Every case asserts through the policy metrics recorded by a
thread-local debugging recorder, which doubles as the request-count
assertion even against a real server. Throttling and recoverable 5xx
responses cannot be forced on a stock dev Vault; those paths stay
pinned by the scripted-Vault wiring tests and the engine tests.
Refs rustfs/backlog#1569 (part of rustfs/backlog#1562)
* feat(kms): add key lifecycle operations to the backend contract
Add enable_key/disable_key/rotate_key to KmsBackend with conservative
defaults returning the typed UnsupportedCapability error, mirroring
remove_expired_key. KmsManager gains matching pass-through methods and
drops cached key metadata after every successful state mutation so the
next describe observes backend truth. The local backend overrides
enable/disable, delegating to its state-machine-gated client methods;
rotation stays rejected, matching its advertised capabilities. New
dedicated policy actions kms:EnableKey and kms:DisableKey complete the
KMS action taxonomy alongside the existing kms:RotateKey.
* feat(admin): add KMS key enable/disable/rotate endpoints
POST /v3/kms/keys/enable, /v3/kms/keys/disable and /v3/kms/keys/rotate,
following the existing /v3/kms/keys handler conventions: key_id body
with keyId query fallback, {success, message, key_id, key_metadata}
responses, and 503 JSON while the KMS service is absent. Error mapping
keeps InvalidOperation/ValidationError at 400 like the sibling handlers
and surfaces UnsupportedCapability as 501 so a backend capability gap is
never mistaken for a missing key. Existing /v3/kms/keys handlers are
untouched apart from a visibility change on a private query helper.
* feat(kms): gate scheduled key deletion on bucket encryption references
Implement the DeletionReferenceChecker seam left by the deletion worker:
before any material is destroyed, every bucket's SSE configuration is
checked for a default KMS key reference and a hit blocks the removal.
The gate fails closed - an unpublished object store, a failed bucket
listing or an unreadable per-bucket encryption config all report a
blocking reference - because destroying key material is irreversible
while a blocked removal is simply retried on the next sweep. Registered
during init_kms_system before the service can start, so every worker
spawn observes it. Storage access goes through a new kms section of the
root storage facade.
* test(kms): add a scripted loopback Vault for policy wiring tests
A minimal HTTP/1.1 responder that serves canned Vault responses in order
and records the method/path sequence, so wiring tests can assert exactly
how many requests a code path performed (retries, read-confirm) without
a live Vault server.
* feat(kms): route Vault operations through the retry policy engine
Wire every outbound vaultrs call in the KV2 and Transit backends through
policy::execute, completing the wiring half of the operation policy work
(the engine landed separately):
- Reads (KV2 read/read_metadata/read_version/list, transit read/list/
encrypt/decrypt, health checks) run as ReadIdempotent: bounded retries
with exponential backoff and jitter on 429, recoverable 5xx, and
connection-level failures; 400/401/403/404 stay fatal.
- Writes (KV2 set/CAS set/delete_metadata, transit create/update/rotate/
delete, metadata writes) run as MutatingNonIdempotent: exactly one
attempt under the per-attempt timeout, never replayed. CAS conflicts
in the rotation protocol pass through unchanged as the concurrency
signal they are.
- Each attempt takes a fresh credential snapshot, so a retry after a
credential rotation uses the new token.
- Read-confirm recovery for lost create responses: when a create finds
an existing key that is exactly what it would have produced (same
algorithm, enabled, usable material, and for request-level creates the
same usage/description/tags), it reports the stored key as the create
result instead of KeyAlreadyExists. Any divergence keeps failing.
- Deletes treat already-deleted records as completed deletes (KV2
version records; transit metadata already did), so re-running an
interrupted deletion converges.
- A failed existence pre-check inside create now fails the create
instead of falling through to a blind overwrite (fail closed).
- The policy module sheds its allow(dead_code) now that it is wired.
Wiring tests run against a scripted loopback Vault and assert request
counts and endpoints for the retry, single-attempt, CAS-conflict, and
read-confirm paths.
Refs rustfs/backlog#1569 (part of rustfs/backlog#1562)
* feat(kms): enforce shared key state machine across backends
Unify the key state x operation matrix behind a single gate in
backends/mod.rs and wire it into the Local, Vault KV2 and Vault Transit
backends: Disabled keys reject encryption, data key generation and
rotation while still allowing decryption and lifecycle recovery;
PendingDeletion keys reject everything except decryption and
cancellation (including repeated deletion scheduling); cancellation now
requires an actual pending deletion everywhere. This closes the missing
gates on KV2 encrypt/generate and Local generate_data_key, and stops
enable_key from silently reverting a pending deletion.
Decryption is deliberately left ungated in Disabled/PendingDeletion — an
explicit, documented and tested deviation from AWS KMS, since gating it
would break reads of existing objects the moment a key is disabled.
Add shared contract tests driving the full matrix offline for Local (and
via ignored tests against a live Vault for KV2/Transit), a stateless
contract for Static, an SSE-shaped regression proving existing envelopes
stay decryptable after disable, and a pin on the known-risk Enabled
default of Transit's synthesized metadata fallback.
Refs rustfs/backlog#1571 (part of rustfs/backlog#1562)
* feat(kms): persist deletion deadlines and run a restartable deletion worker (#5491)
* feat(kms): add AppRole configuration surface for Vault auth
Extend VaultAuthMethod::AppRole with secret_id_file (re-read on every
login so external rotation is picked up), a configurable auth mount
(default "approle"), and an optional fail-closed safety window. All new
fields are serde(default) so previously persisted configurations keep
deserializing, and the strict admin-configure deserializer accepts them
as optional.
Environment selection: setting RUSTFS_KMS_VAULT_APPROLE_ROLE_ID switches
both Vault backends to AppRole; the secret_id comes from
RUSTFS_KMS_VAULT_APPROLE_SECRET_ID_FILE (path stored, file wins) or
RUSTFS_KMS_VAULT_APPROLE_SECRET_ID, following the static secret-key file
precedent. validate() rejects AppRole configs without a role_id, without
any secret_id source, or with an empty mount.
Also append the CredentialsUnavailable error variant used by the
fail-closed credential gate.
* feat(kms): implement AppRole login with background renewal and fail-closed expiry
Implement the AppRoleLogin token source (vaultrs approle login +
renew-self) and wire lease-bound credentials through the provider:
- Each successful login/renewal installs a new client generation in the
ArcSwap; in-flight requests finish on the generation they captured.
- A background renewal task refreshes at half the lease TTL: renewable
tokens are renewed in place, everything else (or a failed renewal)
falls back to a fresh login. Auth exchanges run under the typed retry
policy (OpClass::Auth) and failed cycles retry on a fixed cadence, so
the provider recovers once Vault does.
- Fail-closed: current() refuses to hand out a token inside the
configured safety window of its expiry (default: one attempt timeout),
returning CredentialsUnavailable instead of sending a request whose
token may lapse mid-flight.
- Refreshes are single-flight: concurrent triggers for the same
generation coalesce into one login.
- The renewal task's owner handle lives on the KMS service version:
stop() shuts it down explicitly and reconfigure recycles it via
cancel-on-drop when the old version is discarded.
- The secret_id file is re-read on every login attempt; missing or empty
files fail the attempt without contacting Vault. Crate-owned copies of
tokens and secret_ids are zeroized on drop, and Debug output of every
credential-carrying type stays redacted (leak regression tests).
The renewal machinery is covered by paused-clock tests driving a
scripted token source: renew-at-half-TTL timing, login fallback,
fail-closed window entry and recovery, prompt task recycling, and
coalesced concurrent refreshes.
* feat(kms): add Vault Agent token file authentication
Add the TokenFile source: the token is read from an agent-managed sink
file (RUSTFS_KMS_VAULT_TOKEN_FILE or the TokenFile auth config) and
re-read once per poll interval (default 30s) through the existing
renewal loop, so a token rotated by the agent installs a new client
generation within one poll of the atomic replace. Each successful read
extends the token's observed validity to twice the poll interval; a
file that disappears or turns empty keeps failing the refresh until the
fail-closed window trips, and heals the provider as soon as it is
restored.
Reads are strict and never contact Vault on failure: the file must be
non-empty after trimming, and on Unix group/other permission bits are a
hard error (mirroring the SFTP host-key rule). Rotation detection uses
a content digest; the token itself is never stored on the source and
the crate-owned copy is zeroized.
Configuring the token file together with AppRole or an explicit static
token is rejected as a configuration error. All new config fields are
serde(default) and the strict admin-configure deserializer accepts the
new variant.
Covered by paused-clock tests (atomic replacement installs a new
generation next cycle, deletion fails closed and recovers, prompt task
recycling) plus negatives for missing/empty/over-permissive files and a
Debug leak regression.
* docs(kms): add Vault authentication and credential lifecycle runbook
Cover choosing between static token, AppRole, and Vault Agent token
file auth; AppRole role setup with SecretID delivery and rotation;
Agent sink deployment with the permission requirements; and the
fail-closed window semantics with a troubleshooting table keyed on the
renewal task's log lines.
* feat(kms): retain historical master key versions for Vault KV2 rotation
Rotation previously had to be rejected outright because replacing the
stored material would orphan every DEK wrapped by earlier versions.
Vault KV2 now keeps each version's material in an immutable, create-only
record at {prefix}/{key_id}/versions/{N} and treats the top-level record
as the current-version pointer plus fast-path material copy:
- decrypt resolves the envelope's master_key_version to its version
record; a missing version fails closed with KeyVersionNotFound and
never falls back to the current material
- envelopes without a version (pre-versioning writers) resolve to the
baseline_version frozen at the key's first rotation, so never-rotated
keys behave exactly as before
- generate_data_key stamps the wrapping version from the same key record
snapshot that supplied the material
- rotate_key commits in check-and-set order: freeze baseline, persist
the next version's material, then switch the current pointer; any
failure leaves the current pointer untouched, and concurrent rotations
serialize on the CAS writes with monotonically unique versions
- key listings drop the versions/ directory entries and physical key
deletion purges version records before the key record
Refs rustfs/backlog#1565
* test(kms): pin rotation contracts across backends and document retention
Completes the backlog#1565 series with the cross-backend regression net
and operator documentation:
- Vault Transit: ignored integration test proving version-prefixed
historical ciphertext still decrypts after rotation, with no
RustFS-side version bookkeeping in the envelope
- Local: offline mixed-format test interleaving pre-versioning and
versioned envelopes through a rejected rotation; the existing
rotation-rejection pinning test already covers material immutability
- docs: kms-backend-security.md gains the KV2 versioned retention
model, version record retention/destruction preconditions, and the
upgrade-before-first-rotation cluster constraint
Refs rustfs/backlog#1565
Contract-only module for KMS backup/restore (no handler or backend
wiring): versioned manifest schema with completeness marker and sealed
digest, the (backend, at-rest protection) responsibility matrix, typed
fail-closed errors, and the zero-write restore dry-run report. Fields
whose shape depends on in-flight contracts are reserved and reject data
in format version 1.
Rotation previously had to be rejected outright because replacing the
stored material would orphan every DEK wrapped by earlier versions.
Vault KV2 now keeps each version's material in an immutable, create-only
record at {prefix}/{key_id}/versions/{N} and treats the top-level record
as the current-version pointer plus fast-path material copy:
- decrypt resolves the envelope's master_key_version to its version
record; a missing version fails closed with KeyVersionNotFound and
never falls back to the current material
- envelopes without a version (pre-versioning writers) resolve to the
baseline_version frozen at the key's first rotation, so never-rotated
keys behave exactly as before
- generate_data_key stamps the wrapping version from the same key record
snapshot that supplied the material
- rotate_key commits in check-and-set order: freeze baseline, persist
the next version's material, then switch the current pointer; any
failure leaves the current pointer untouched, and concurrent rotations
serialize on the CAS writes with monotonically unique versions
- key listings drop the versions/ directory entries and physical key
deletion purges version records before the key record
Refs rustfs/backlog#1565
* fix(kms): repair vault test call sites missed by the timeout refactor
Three offline tests still constructed VaultKmsClient with the pre-#5472
single-argument signature, leaving cargo test -p rustfs-kms unable to
compile. Pass the same 30s attempt timeout the neighbouring tests use.
* refactor(kms): route Vault clients through a rotatable credential provider
Both Vault backends previously built a VaultClient in their constructor
and held it for the lifetime of the backend, which leaves no seam for
re-authentication: rotating credentials would require tearing down the
whole backend.
Introduce backends/vault_credentials with a TokenSource trait (only
StaticToken for now; AppRole login and agent token files land in
follow-ups) and a VaultCredentialProvider that owns the authenticated
client behind an ArcSwap. Request paths take a per-call snapshot via
current(), so a future rotation swaps in a new client generation without
interrupting calls already in flight. Tokens held by this crate are
zeroized on drop, and Debug output of every credential-carrying type is
redacted (covered by a leak regression test).
Behavior is unchanged: static token, namespace, and per-attempt timeout
feed the same VaultClientSettings as before, and AppRole configurations
are still rejected at construction with the same message.
PR #5472 added a required attempt_timeout parameter to
VaultKmsClient::new, while PR #5474 (developed in parallel) added
three tests calling it with the old single-argument signature,
breaking compilation of cargo test -p rustfs-kms. Pass the same
30-second timeout the neighboring tests use.
PR #5472 added an attempt_timeout parameter to VaultKmsClient::new while
PR #5474 was developed in parallel and added three tests using the old
single-argument signature. Both merged cleanly at the text level, leaving
main unable to compile the rustfs-kms test target. Align the three call
sites with the current constructor signature.
* fix(kms): restore vault backend test compilation after timeout parameter
PR #5472 added an attempt_timeout parameter to VaultKmsClient::new while
PR #5474 landed tests still using the one-argument form, leaving
'cargo test -p rustfs-kms' unable to compile on main. Pass the same
30-second timeout the surrounding integration tests already use.
* feat(kms): add master key version to data key envelope contract
DataKeyEnvelope gains an optional master_key_version field recording
which KEK version wrapped the DEK, so rotation-aware backends can load
the matching historical material on decrypt. The field is skipped when
None, keeping envelopes from non-rotating backends byte-identical to
the historical seven-field JSON shape, and legacy envelopes without the
field deserialize to None. The envelope discriminator marker is
untouched, so mixed-format routing is unchanged in both directions.
Adds the KeyVersionNotFound typed error for version-addressed material
lookups that must fail closed instead of falling back to the current
version.
Refs rustfs/backlog#1565
Zhengchao An