mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-07 20:46:11 +00:00
Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 0486ca9877 | |||
| 54716aa61c |
@@ -1,33 +0,0 @@
|
||||
# Adversarial Review Shape
|
||||
|
||||
Use when root `AGENTS.md` triggers adversarial validation or for a substantial PR
|
||||
review. Paths below are repository-relative. The root finding standard and
|
||||
completion rule apply; selecting a lens does not require finding a defect.
|
||||
|
||||
Risk and review shape:
|
||||
|
||||
- **Exempt:** documentation, comments, formatting, or typos with no runtime,
|
||||
build, test, or agent-execution effect.
|
||||
- **Mechanical:** renames, moves, test/tooling-only changes, and agent-rule
|
||||
changes. Run correctness and simplicity lenses.
|
||||
- **Standard:** localized behavior changes. Run one integrated final-diff pass
|
||||
covering correctness, simplicity, and test coverage; add only domain lenses
|
||||
matched by the diff.
|
||||
- **High risk / substantial PR review:** high risk includes locking,
|
||||
erasure/quorum/heal, replication, multipart, RPC, lifecycle/tiering,
|
||||
persistence/fsync, IAM/KMS/auth, cryptography, on-disk/on-wire formats, and
|
||||
S3-visible semantics. Cover all applicable lenses using exactly two
|
||||
independent reviewers when delegation is explicitly authorized. Split the
|
||||
lenses between them. Otherwise perform two fresh sequential passes.
|
||||
- **Outbound client defaults:** what `TargetClient`, `PutObjectOptions`, or
|
||||
the remote SDK configuration sends to every replication or migration target
|
||||
is high risk for every target class even when the change fixes one. Follow
|
||||
the SOP in `docs/postmortems/2026-09-03-replication-checksum-default-regression.md`:
|
||||
run the outbound target matrix, document each new env knob in the same PR,
|
||||
and list verified and unverified target classes in the PR Impact section.
|
||||
|
||||
Available domain lenses are security, concurrency/durability, compatibility,
|
||||
and performance. Select `.agents/skills/adversarial-validation/SKILL.md` for an
|
||||
explicit adversarial request, a high-risk change, or a substantial PR review;
|
||||
then read only its matching role references. A routine standard pass does not
|
||||
load the playbook unless the reviewer needs a RustFS-specific probe.
|
||||
@@ -1,65 +0,0 @@
|
||||
# Implementation Rules
|
||||
|
||||
Applies when changing code or running artifact-heavy work. Paths below
|
||||
are repository-relative. Read only the relevant sections during read-only review.
|
||||
|
||||
## Worktree and Disk Hygiene
|
||||
|
||||
- Start implementation from the latest `origin/main` and confirm the requested
|
||||
change is not already present.
|
||||
- An existing clean, isolated task worktree is sufficient. Create another
|
||||
worktree only when the current checkout is shared, dirty with unrelated work,
|
||||
or belongs to another task.
|
||||
- Never commit from a shared checkout.
|
||||
- Use a task-specific branch named `<type>/<topic>`, such as `fix/...`,
|
||||
`feat/...`, `test/...`, or `docs/...`, unless the user specifies a name.
|
||||
- Do not include agent, tool, contributor, account, or organization names in
|
||||
branch names.
|
||||
- Push to the user-requested remote or the repository's configured push remote.
|
||||
Do not hard-code or infer a remote from an account name.
|
||||
- Check free space before artifact-heavy builds, tests, coverage, or downloads.
|
||||
Re-check before a broad gate when space is tight.
|
||||
- Remove only task-owned temporary/build artifacts. Never delete another task's
|
||||
worktree or uncommitted data.
|
||||
- At handoff, mention disk or cleanup details only when they affected execution
|
||||
or artifacts/worktrees remain intentionally.
|
||||
|
||||
## Change Style
|
||||
|
||||
- Preserve existing control flow unless changing it is required for correctness.
|
||||
- Prefer a direct local edit over new files, wrappers, managers, or speculative
|
||||
abstractions.
|
||||
- Add a helper only when it removes current duplication, names a real domain
|
||||
boundary, or isolates a non-trivial invariant.
|
||||
- Remove an in-scope path superseded by the change. If compatibility requires it,
|
||||
adapt at the boundary to one canonical core and use the repository's
|
||||
`RUSTFS_COMPAT_TODO` policy.
|
||||
- Comments explain non-obvious invariants or reasons. Do not narrate code or
|
||||
record change history.
|
||||
- Mention unrelated problems when useful; do not fix them in a narrow task.
|
||||
|
||||
## Reuse and Boundary Rules
|
||||
|
||||
- Before adding helpers, constants, fixtures, or wrappers, search the touched
|
||||
crate, the domain-owning crate, `crates/utils`, `crates/common`, and relevant
|
||||
direct dependencies.
|
||||
- Reuse requires matching semantics: normalization, error types, deadlines,
|
||||
durability, and compatibility must fit the call site. A narrowly named local
|
||||
helper is better than forced reuse with different semantics.
|
||||
- Validate untrusted input at its trust boundary, then trust the validated type.
|
||||
Values crossing disk, RPC, persistence, or version boundaries remain
|
||||
untrusted at every consumer.
|
||||
- Re-check boundary values immediately before destructive actions such as
|
||||
delete, overwrite, or quorum decisions.
|
||||
- Every new branch needs a concrete triggering input/state. For decoded or peer
|
||||
data, corruption and mixed-version input are valid triggers.
|
||||
- Required values must return a typed error when absent or corrupt; do not use a
|
||||
default that converts corruption into a plausible result.
|
||||
- Attach error context once where it is actionable. Do not erase typed errors
|
||||
below aggregation or quorum layers.
|
||||
|
||||
## Naming
|
||||
|
||||
Use Rust API naming: `SCREAMING_SNAKE_CASE` constants/statics, `snake_case`
|
||||
functions/variables, and `PascalCase` types. Do not rename unrelated existing
|
||||
violations.
|
||||
@@ -1,51 +0,0 @@
|
||||
# Git and Pull Request Rules
|
||||
|
||||
Applies to commits, pushes, PRs, and issue/discussion actions. Paths below are
|
||||
repository-relative. User authorization and root `AGENTS.md` still govern scope.
|
||||
|
||||
## Final PR Preflight
|
||||
|
||||
Before creating or updating a PR, reuse completed review and verification:
|
||||
|
||||
- Verify the actual base (normally `origin/main`) and the complete task diff,
|
||||
including file names and whitespace. Exclude secrets, logs, generated
|
||||
artifacts, and unrelated edits. Retain an existing PR's base unless requested.
|
||||
- Confirm the final diff passed the root verification tier; fix task-owned
|
||||
failures and run missing scoped checks. Report unresolved required checks or
|
||||
authority without expanding the task. Do not start another general review.
|
||||
- Keep the English Conventional Commit title at most 72 characters. Use the
|
||||
template headings, actual checks, material risks, and rollback notes.
|
||||
- Immediately before writing to GitHub, confirm the head and task diff are
|
||||
unchanged. Rerun only checks invalidated by edits or relevant state changes.
|
||||
|
||||
## Pull Request Lifecycle
|
||||
|
||||
- Creating or updating a PR includes one immediate snapshot of checks,
|
||||
mergeability, reviews, and unresolved threads.
|
||||
- Unless the user explicitly requests monitoring, a release workflow requires
|
||||
it, or an automation already owns it, hand off after the PR is open with the
|
||||
current state and next event to watch. Do not delay ordinary handoff with
|
||||
fixed quiet-period sleeps.
|
||||
- For requested monitoring, use event-driven or bounded waits. Report only state
|
||||
changes, actionable failures, or a meaningful prolonged delay.
|
||||
- Investigate failures/comments before changing code. Fix task-attributable
|
||||
issues, rerun affected verification, push, reply or resolve the thread, then
|
||||
resume the requested monitor.
|
||||
- Never merge without required reviewer approval or explicit authority.
|
||||
- After an observed merge, verify the commit reached the base, then clean the
|
||||
task worktree/branch when safe. Preserve unmerged work for closed PRs unless
|
||||
deletion was explicitly authorized.
|
||||
|
||||
## Git and PR Baseline
|
||||
|
||||
- Follow Conventional Commits; keep the subject at most 72 characters.
|
||||
- Source comments, commits, PR titles, and PR bodies are in English.
|
||||
- Keep every heading from `.github/pull_request_template.md`; use `N/A` where
|
||||
needed and include commands actually run.
|
||||
- Use `--body-file` for multiline `gh pr create`/`gh pr edit` content.
|
||||
- PR/issue/discussion content must not contain the literal sequence `\n` or
|
||||
hard-wrapped prose paragraphs.
|
||||
- Do not include local absolute paths or tool-specific labels/prefixes in GitHub
|
||||
content.
|
||||
- Resolve review threads after the underlying issue is fixed. If declining a
|
||||
suggestion, reply with a short evidence-based reason.
|
||||
@@ -1,11 +1,11 @@
|
||||
---
|
||||
name: adversarial-validation
|
||||
description: Review RustFS diffs or designs for explicit adversarial requests, high-risk changes under the repository review policy, or substantial PR reviews. Skip ordinary questions, diagnosis, planning, status, routine low-risk implementation, and prose with no execution effect.
|
||||
description: Review a final RustFS diff adversarially when the user requests adversarial review, the root AGENTS.md classifies the change as high risk, or a substantial PR is being reviewed. Do not use for ordinary questions, diagnosis, planning, status, documentation-only work, or routine low-risk implementation.
|
||||
---
|
||||
|
||||
# RustFS Adversarial Validation
|
||||
|
||||
Use the [repository risk tiers and review shape](../../references/adversarial-validation.md). This skill
|
||||
Use the risk tier and review shape defined in the root `AGENTS.md`. This skill
|
||||
routes a review to RustFS-specific probes without loading unrelated domains.
|
||||
|
||||
## Select Lenses
|
||||
@@ -31,18 +31,15 @@ adversarial review.
|
||||
|
||||
## Review Protocol
|
||||
|
||||
1. Freeze the exact final diff/head (or the design under review) and list the
|
||||
selected lenses.
|
||||
2. Run the review shape required by the repository risk tier.
|
||||
1. Freeze the exact final diff/head and list the selected lenses.
|
||||
2. Run the review shape required by root `AGENTS.md`.
|
||||
3. For each selected lens, either report a concrete finding or a null verdict
|
||||
naming the attacks performed.
|
||||
4. Apply root `AGENTS.md`'s finding standard. Test each candidate against callers,
|
||||
existing coverage, and invariants before accepting it; an adversarial role
|
||||
does not have to produce a defect.
|
||||
5. Fix or rebut supported findings with code-path, test, or invariant evidence.
|
||||
4. A finding needs `file:line`, a triggering input/state/interleaving, the wrong
|
||||
outcome, and a focused fix or missing regression check.
|
||||
5. Fix or rebut every finding with code-path, test, or invariant evidence.
|
||||
6. After a non-trivial edit, rerun only lenses affected by that edit against the
|
||||
new exact diff.
|
||||
|
||||
Do not turn a null verdict into a long checklist. Record concise evidence that
|
||||
the relevant failure classes were attacked, then stop under the root completion
|
||||
rule. Keep the required per-lens verdicts for high-risk PRs.
|
||||
the relevant failure classes were attacked.
|
||||
|
||||
@@ -3,10 +3,9 @@
|
||||
- For every behavior claim, name the focused test/check that fails if the
|
||||
changed hunk is reverted. If none is practical, require the reason and
|
||||
residual risk.
|
||||
- Confirm tests exercise the real production path and distinguish the intended
|
||||
behavior from the named regression. A success, `is_err()`, or no-panic check
|
||||
can be sufficient when that is the actual contract; require exact values,
|
||||
bytes, state, or error variants when those distinctions matter to the change.
|
||||
- Confirm tests exercise the real production path and assert returned values,
|
||||
exact bytes, stored state, or the specific error variant—not only success,
|
||||
`is_err()`, or no panic.
|
||||
- For new flags/modes, verify each branch and ask which test fails if the branch
|
||||
is inverted.
|
||||
- For new error propagation, inject the failure and assert the caller observes
|
||||
|
||||
@@ -1,13 +1,12 @@
|
||||
---
|
||||
name: arch-checks
|
||||
description: Diagnose failures from check_layer_dependencies.sh, check_architecture_migration_rules.sh, check_unsafe_code_allowances.sh, check_logging_guardrails.sh, check_doc_paths.sh, or check_no_planning_docs.sh. Use when one of these guards fails, not for every architecture question or documentation edit.
|
||||
description: Resolve failures from the repository's architecture guard scripts — check_layer_dependencies.sh, check_architecture_migration_rules.sh, check_unsafe_code_allowances.sh, check_logging_guardrails.sh, check_doc_paths.sh. Use when make pre-commit / pre-pr or CI fails on one of these checks.
|
||||
---
|
||||
|
||||
# Architecture Guard Checks
|
||||
|
||||
Read only the section for the failing guard. Use `.config/make/` and the current
|
||||
workflow to verify its wiring; not every guard is part of every gate. Fix the
|
||||
cause and rerun the failed guard; never weaken a check to get green.
|
||||
All five run in `make pre-commit` / `make pre-pr` and in CI. Fix the cause;
|
||||
never weaken a check to get green.
|
||||
|
||||
## `check_layer_dependencies.sh` — layer DAG in `rustfs/src`
|
||||
|
||||
@@ -55,8 +54,7 @@ Instruction docs (`AGENTS.md`, `CLAUDE.md`, `ARCHITECTURE.md`) and every
|
||||
Markdown file under `docs/` (architecture, operations, testing, index) must not
|
||||
reference repo file paths that no longer exist. If your refactor moved code,
|
||||
update the docs that point at it — the error message lists `doc -> stale-path`
|
||||
pairs. In durable docs, cite paths plus symbol names rather than line numbers
|
||||
(see `docs/architecture/README.md`). Review findings still need `file:line`.
|
||||
pairs. Cite paths plus symbol names, never line numbers (see `docs/README.md`).
|
||||
|
||||
## `check_no_planning_docs.sh`
|
||||
|
||||
|
||||
@@ -8,11 +8,19 @@ description: Review a commit, PR, or merged patch when the user requests ordinar
|
||||
Use this skill for an ordinary requested review. If the root policy or user calls
|
||||
for adversarial validation, use `adversarial-validation` instead of running both.
|
||||
|
||||
## Quick Start
|
||||
|
||||
1. Read the scope: commit, PR, patch, or file list.
|
||||
2. Map each changed area by risk and user impact.
|
||||
3. Inspect each risky change in context.
|
||||
4. Report findings first, ordered by severity.
|
||||
5. Close with residual risks and verification recommendations.
|
||||
|
||||
## Core Workflow
|
||||
|
||||
### 1) Scope and assumptions
|
||||
- Derive the change source, target branch, and relevant runtime/version from the
|
||||
supplied diff and metadata. Ask only when missing context could change the verdict.
|
||||
- Confirm change source (diff, commit, PR, files), target branch, language/runtime, and version.
|
||||
- If context is missing, state assumptions before deeper analysis.
|
||||
- Focus only on requested scope; avoid reviewing unrelated files.
|
||||
|
||||
### 2) Risk map
|
||||
@@ -32,20 +40,43 @@ for adversarial validation, use `adversarial-validation` instead of running both
|
||||
- unchecked assumptions and null/empty/error-path handling
|
||||
- stale tests, fixtures, and configs
|
||||
- hidden coupling to shared helpers/constants/features
|
||||
- Apply root `AGENTS.md`'s finding standard: try to disprove a candidate before
|
||||
reporting it. Mention an unresolved question only when it could materially
|
||||
change the verdict; do not fill the report with speculative possibilities.
|
||||
- If a point is uncertain, mark it as an open question instead of guessing.
|
||||
|
||||
#### Rust-specific checks
|
||||
#### Rust-specific checks (apply to all Rust changes)
|
||||
|
||||
For changed Rust behavior, use the matching sections of [rust-code-quality](../rust-code-quality/SKILL.md). Reuse checks already performed by the selected review workflow. Comment-only or formatting-only Rust diffs do not require the full Rust checklist. Carry its P0–P3 ratings over unchanged and use this skill's output format.
|
||||
Run the full checklist in [rust-code-quality](../rust-code-quality/SKILL.md) — the canonical Rust review checklist for the unwrap/casting/cloning/locking/recursion/error-type/serde/test rules and the reuse-and-necessity checks (duplicated helpers, defensive branches without a nameable trigger, redundant error wrapping). Do not restate those rules here; carry its P0–P3 ratings over unchanged and use this skill's output format.
|
||||
|
||||
### 4) Findings-first output
|
||||
- Order supported findings by P0–P3 severity; preserve the Rust ratings above.
|
||||
Include `path:line`, the failure and impact, a focused fix, and its validation.
|
||||
- If no supported issues remain, state `No findings` with the reviewed scope and
|
||||
any material verification limitation. Do not append optional improvements to
|
||||
make a clean review look productive.
|
||||
- Order findings by severity:
|
||||
- P0: critical failure, security breach, or data loss risk
|
||||
- P1: high-impact regression
|
||||
- P2: medium risk correctness gap
|
||||
- P3: low risk/quality debt
|
||||
- For each finding include:
|
||||
- Severity
|
||||
- `path:line` reference
|
||||
- concise issue statement
|
||||
- impact and likely failure mode
|
||||
- specific fix or mitigation
|
||||
- validation step to confirm
|
||||
- If no issues exist, explicitly state `No findings` and why.
|
||||
|
||||
Close after the required review. Recommend additional verification only for an
|
||||
identified unresolved risk or required gate; reuse evidence for unchanged code.
|
||||
### 5) Close
|
||||
- Report assumptions and unknowns.
|
||||
- Suggest targeted checks (tests, canary checks, logs/metrics, migration validation).
|
||||
|
||||
## Output Template
|
||||
|
||||
1. Findings
|
||||
2. No findings (if applicable)
|
||||
3. Assumptions / Unknowns
|
||||
4. Recommended verification steps
|
||||
|
||||
## Finding Template
|
||||
|
||||
- `[P1] Missing timeout for downstream call`
|
||||
- Location: `path/to/file.rs:123`
|
||||
- Issue: ...
|
||||
- Impact: ...
|
||||
- Fix suggestion: ...
|
||||
- Validation: ...
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: issue-triage
|
||||
description: Assess whether a GitHub issue is fixed, needs implementation, or can be closed by checking related work and current code. Use for issue completion/triage requests. Status questions are read-only; comment, close, or change labels only when the conversation authorizes that action.
|
||||
description: Triage a GitHub issue — determine if it is already fixed, needs implementation, or should be closed. Searches related commits and PRs, verifies implementation status, and posts a triage comment or closes the issue. Use when the user provides an issue URL and asks whether it can be closed or needs work.
|
||||
---
|
||||
|
||||
# Issue Triage
|
||||
@@ -20,8 +20,6 @@ Read the issue body to understand what was requested. Extract:
|
||||
- Any linked PRs or commits mentioned in the body or comments.
|
||||
- Any checklist items or sub-issues.
|
||||
|
||||
Resolve the issue repository and implementation repository separately (for example, `rustfs/backlog` tracks work in `rustfs/rustfs`). Pass the implementation repository explicitly to PR queries; the current checkout may belong to another repository.
|
||||
|
||||
### 2. Search for related work
|
||||
|
||||
Search git history for commits referencing the issue:
|
||||
@@ -31,29 +29,26 @@ git log --oneline --all --grep="<N>" | head -30
|
||||
|
||||
Search for related PRs:
|
||||
```bash
|
||||
gh pr list --repo <implementation-repo> --search "<issue-url>" --state all --json number,title,state,mergedAt
|
||||
gh pr list --search "fixes #<N> OR closes #<N> OR #<N>" --state all --json number,title,state,mergedAt
|
||||
```
|
||||
|
||||
Also search qualified issue references and subject keywords; for same-repository
|
||||
issues, include `#<N>`. Follow explicit links even without a text match. A search
|
||||
page with no match does not prove the work is absent.
|
||||
|
||||
If the issue mentions specific PRs, check their status:
|
||||
```bash
|
||||
gh pr view <PR_N> --repo <implementation-repo> --json state,mergedAt,title,mergeCommit,baseRefName
|
||||
gh pr view <PR_N> --json state,mergedAt,title
|
||||
```
|
||||
|
||||
### 3. Verify implementation
|
||||
|
||||
Fetch the implementation repository's current base branch. For each merged candidate, verify its merge commit is present and inspect the current code for the claimed behavior; a commit message match alone is not proof:
|
||||
For each linked or related PR that is merged, verify the fix is actually present on the current main branch:
|
||||
```bash
|
||||
git fetch <implementation-remote> <base-branch>
|
||||
git merge-base --is-ancestor <merge-commit> <implementation-remote>/<base-branch>
|
||||
git log --oneline main | grep -i "<keyword>"
|
||||
# or
|
||||
git log --oneline main --grep="<PR_N>"
|
||||
```
|
||||
|
||||
If the issue describes a specific defect, inspect the fetched base's code rather than assuming the current checkout contains it:
|
||||
If the issue describes a specific defect, check the relevant code to confirm the fix is in place:
|
||||
```bash
|
||||
git show <implementation-remote>/<base-branch>:crates/<relevant>/src/<file>.rs
|
||||
grep -n "<pattern>" crates/<relevant>/src/<file>.rs
|
||||
```
|
||||
|
||||
For issues with checklists, verify each item individually. If sub-items are tracked as separate issues, check those too:
|
||||
@@ -63,15 +58,13 @@ gh issue view <SUB_N> --repo <owner/repo> --json state
|
||||
|
||||
### 4. Determine verdict
|
||||
|
||||
- **All items fixed and merged**: Recommend closing; name the verified PRs and behavior.
|
||||
- **Some items fixed, some remaining**: Keep open; report each remaining item.
|
||||
- **Not yet implemented**: Keep open; report what remains.
|
||||
- **Superseded or no longer relevant**: Recommend closing with evidence.
|
||||
- **All items fixed and merged**: Close with a summary comment listing what was fixed and which PRs.
|
||||
- **Some items fixed, some remaining**: Comment with status of each item. Do not close.
|
||||
- **Not yet implemented**: Comment with a summary of what remains. Do not close.
|
||||
- **Superseded or no longer relevant**: Close with explanation.
|
||||
|
||||
### 5. Take action
|
||||
|
||||
For a status-only request, return the assessment without GitHub writes. If commenting, closing, or label edits are authorized, perform only those actions; do not ask again for authority already given. Prepare the final assessment before asking for any missing authority. Write `rustfs/backlog` issue content in Chinese.
|
||||
|
||||
Close with comment:
|
||||
```bash
|
||||
gh issue close <N> --repo <owner/repo> --comment "<body>"
|
||||
@@ -82,9 +75,9 @@ Comment without closing:
|
||||
gh issue comment <N> --repo <owner/repo> --body-file /tmp/triage.md
|
||||
```
|
||||
|
||||
Update labels only when label changes are authorized, using existing repository labels; never add tool-specific labels:
|
||||
Update issue labels if needed:
|
||||
```bash
|
||||
gh issue edit <N> --repo <owner/repo> --add-label "<existing-label>"
|
||||
gh issue edit <N> --repo <owner/repo> --add-label "completed" --remove-label "needs-triage"
|
||||
```
|
||||
|
||||
Always use `--body-file` for multiline content, never inline `--body`.
|
||||
@@ -92,16 +85,27 @@ Always use `--body-file` for multiline content, never inline `--body`.
|
||||
### 6. Handle multi-issue batches
|
||||
|
||||
When the user asks to check multiple issues (e.g., "check all issues by user X" or "scan backlog for closable issues"):
|
||||
1. List the full requested scope with pagination (for example `gh api --paginate 'repos/<repo>/issues?state=open&per_page=100'`, excluding entries with `pull_request`). Add an author filter only when the user requested one; the default page/limit is not evidence that all issues were checked.
|
||||
1. List the issues: `gh issue list --repo <repo> --author <user> --state open --json number,title,updatedAt`
|
||||
2. For each issue, run steps 1-5 above.
|
||||
3. Report a summary table of all triaged issues with verdicts.
|
||||
|
||||
## Report
|
||||
## Output format
|
||||
|
||||
Identify the issue and current state, verified implementation/PR evidence,
|
||||
remaining items, verdict, and action actually taken. Use a table for batches;
|
||||
a single issue does not require a heading for each field. Follow step 4's
|
||||
verdicts without repeating the assessment in another template.
|
||||
### Issue Triage: #<N> — <title>
|
||||
|
||||
**State**: OPEN / CLOSED
|
||||
**Linked PRs**: <list with merge status>
|
||||
|
||||
#### Assessment
|
||||
<what was requested vs what is implemented>
|
||||
|
||||
#### Verdict
|
||||
- Close — all items resolved by <PR list>
|
||||
- Keep open — <remaining items>
|
||||
- Not started — <what needs to be done>
|
||||
|
||||
#### Action taken
|
||||
- Closed with comment / Commented / No action
|
||||
|
||||
## Notes
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: plugin-contract-guard
|
||||
description: Guard changes to target-plugin manifests, extension schemas, admin catalog/instance contracts, secret redaction, and external-plugin install policy. Use when a diff changes those contracts in crates/targets, crates/extension-schema, or admin plugin/extension handlers; path membership alone, comments, and unrelated runtime internals do not trigger it.
|
||||
description: Invariants and change procedure for the target-plugin / extension system — plugin manifests, admin plugin/extension catalog and instance APIs, secret redaction, external-plugin install policy. Use when editing crates/targets (manifest, plugin, control_plane, catalog, runtime), crates/extension-schema, or rustfs/src/admin plugin_contract.rs / plugins_*.rs / extensions.rs / target_descriptor.rs.
|
||||
---
|
||||
|
||||
# Plugin & Extension Contract Guard
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
---
|
||||
name: pr-creation-checker
|
||||
description: Perform the final RustFS PR preflight and draft compliant English title/body metadata immediately before creating or updating a PR. Do not use during implementation or as a second general code review.
|
||||
---
|
||||
|
||||
# PR Creation Checker
|
||||
|
||||
Use this skill only at the PR boundary. Reuse completed diff review and
|
||||
verification evidence; do not reread the repository or rerun equivalent checks.
|
||||
|
||||
## Preflight
|
||||
|
||||
1. Confirm the branch is based on current `origin/main` and contains only the
|
||||
intended task diff.
|
||||
2. Inspect `git diff --stat`, `git diff --check`, and changed file names for
|
||||
secrets, logs, generated artifacts, or unrelated edits.
|
||||
3. Confirm the checks selected by root `AGENTS.md` passed on the final diff.
|
||||
Do not replace focused behavioral tests with a generic gate or rerun checks
|
||||
already covered by an unchanged umbrella run.
|
||||
4. Read `.github/pull_request_template.md`. Consult `Makefile`, `.config/make/`,
|
||||
or CI only when the required command/current gate is uncertain.
|
||||
5. Return `BLOCKED` for an unclean scope, missing required evidence, failed
|
||||
required checks, or non-compliant metadata.
|
||||
|
||||
## Metadata
|
||||
|
||||
- Title: English Conventional Commit, at most 72 characters, with no tool
|
||||
prefix.
|
||||
- Body: English, exact template headings, `N/A` where needed, concise rationale,
|
||||
actual verification commands, and material risks/rollback notes.
|
||||
- Use repository-relative paths; never include local absolute paths.
|
||||
- Keep prose paragraphs on one logical line and never include the literal
|
||||
sequence `\n`.
|
||||
- Use a temporary body file with `gh pr create --body-file` or
|
||||
`gh pr edit --body-file`; never pass multiline Markdown inline.
|
||||
|
||||
## Output
|
||||
|
||||
- Status: `READY` or `BLOCKED`.
|
||||
- Title.
|
||||
- Complete PR body.
|
||||
- Verification commands and results.
|
||||
- Risks or `N/A`.
|
||||
|
||||
Immediately before the GitHub write, repeat only the five preflight checks above
|
||||
against the final head.
|
||||
@@ -0,0 +1,4 @@
|
||||
interface:
|
||||
display_name: "PR Creation Checker"
|
||||
short_description: "Draft RustFS-ready PRs with checks, template, and blockers."
|
||||
default_prompt: "Use $pr-creation-checker for final PR preflight and compliant English title/body metadata."
|
||||
@@ -1,93 +1,147 @@
|
||||
---
|
||||
name: pr-review
|
||||
description: Review a GitHub PR from a URL or number using its actual base/head and risk-appropriate code review. Use when the user asks for a PR review, not a status lookup or PR wording edit. Publish a review only when authorized; delegation and monitoring follow the requested scope and root AGENTS.md.
|
||||
description: Review a GitHub PR end-to-end from a URL or number — fetch metadata, inspect the diff, run multi-role adversarial review, check CI status, and post the review comment. Use when the user provides a PR link and asks to review it.
|
||||
---
|
||||
|
||||
# PR Review
|
||||
|
||||
Use this skill for PR context and review delivery. An ordinary review request is read-only unless the conversation also authorizes posting or fixes. Reuse that authorization without asking again; prepare the review before requesting any missing publication approval.
|
||||
Use this skill when the user provides a GitHub PR URL or number and asks to review it. This covers the full review lifecycle: data gathering, code review, CI verification, and posting the result.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- Follow root `AGENTS.md`; classify risk with the [review policy](../../references/adversarial-validation.md) and consult relevant [change-style and boundary rules](../../references/implementation.md).
|
||||
- Select `code-change-verification` for ordinary review or `adversarial-validation` for explicitly adversarial, substantial, or high-risk review; do not run both on the same diff.
|
||||
- Read `AGENTS.md` for the repository's adversarial validation policy and change-style rules.
|
||||
- The `adversarial-validation` skill handles the review role playbooks; this skill orchestrates the workflow around it.
|
||||
|
||||
## Workflow
|
||||
|
||||
### 1. Gather PR context
|
||||
|
||||
```bash
|
||||
gh pr view <N> --repo <owner/repo> --json title,author,state,body,additions,deletions,changedFiles,commits,baseRefName,headRefName,baseRefOid,headRefOid
|
||||
gh pr diff <N> --repo <owner/repo> --name-only
|
||||
gh pr view <N> --json title,author,state,body,additions,deletions,changedFiles,commits,baseRefName,headRefName
|
||||
gh pr diff <N> --name-only
|
||||
```
|
||||
|
||||
Read the PR body and linked issues to understand the change's purpose. If the PR references an issue, fetch that too:
|
||||
```bash
|
||||
gh issue view <ISSUE> --repo <issue-owner/repo> --json title,body,state
|
||||
gh issue view <ISSUE> --json title,body,state
|
||||
```
|
||||
|
||||
### 2. Fetch the diff and classify the change
|
||||
|
||||
```bash
|
||||
git fetch <repo-remote> <baseRefName> refs/pull/<N>/head
|
||||
git diff <baseRefOid>...<headRefOid> --stat
|
||||
git fetch origin pull/<N>/head:pr-<N>
|
||||
git diff main...pr-<N> --stat
|
||||
```
|
||||
|
||||
Resolve `<repo-remote>` to the PR repository; do not assume the current checkout's `origin` or `main` matches. Record the exact base/head used. If either moved during fetching, refresh the snapshot before reviewing. Classify using the repository review policy; instruction changes that affect agent execution are mechanical, not exempt.
|
||||
Classify the change by risk tier (per AGENTS.md):
|
||||
- **Exempt**: docs/comments/instruction-only, formatting, typos.
|
||||
- **Mechanical**: renames, file moves, test-only or tooling changes.
|
||||
- **Standard** (default): any behavior change.
|
||||
- **High risk**: locking, erasure coding, quorum/heal, replication, multipart, RPC, lifecycle/tiering, metadata formats, persistence/fsync, IAM/KMS/auth, on-disk/on-wire formats, S3 API-visible behavior.
|
||||
|
||||
### 3. Review the changed behavior
|
||||
### 3. Cluster changed files and delegate review
|
||||
|
||||
Group files by functional area to trace callers and invariants. Use the root risk tier's review shape and only matching lenses. File count does not authorize delegation. When delegation is explicitly authorized, high-risk/substantial reviews use exactly two independent reviewers with the applicable lenses split between them; otherwise use two fresh sequential passes. Reviewers do not spawn further agents.
|
||||
Group the changed files into logical clusters (by crate or functional area). For each cluster, spawn a subagent with a focused review prompt that includes:
|
||||
- The cluster's changed files and their diffs.
|
||||
- The applicable adversarial role probes (from the `adversarial-validation` skill).
|
||||
- The repository's AGENTS.md rules relevant to that domain.
|
||||
|
||||
Findings need a concrete failure scenario with `file:line`; a null verdict briefly names the relevant probes. Reuse existing evidence and choose local checks from the final diff under the root verification policy.
|
||||
For standard-tier changes: correctness adversary + simplicity adversary + test-coverage skeptic, plus every role whose domain the diff touches.
|
||||
For high-risk changes: run all seven roles.
|
||||
|
||||
Each subagent must produce findings (concrete failure scenario with file:line) or a null report ("attacked X, Y, Z — no break found").
|
||||
|
||||
### 4. Check CI status
|
||||
|
||||
```bash
|
||||
gh pr checks <N> --repo <owner/repo>
|
||||
gh pr checks <N>
|
||||
```
|
||||
|
||||
Investigate a failed check when it bears on a finding or the user requested CI diagnosis/merge readiness:
|
||||
If any checks fail, investigate:
|
||||
```bash
|
||||
gh run view --repo <owner/repo> --log-failed --job=<JOB_ID>
|
||||
gh run view --log-failed --job=<JOB_ID>
|
||||
```
|
||||
|
||||
Use current evidence to distinguish pre-existing, flaky, and PR-caused failures. Do not classify them by guesswork or turn a code-only review into unrelated CI repair.
|
||||
Determine whether failures are pre-existing (on main), flaky, or caused by the PR.
|
||||
|
||||
### 5. Synthesize findings
|
||||
|
||||
Report the PR, reviewed base/head, and risk tier, then summarize the assessment.
|
||||
Use the selected review's P0–P3 ratings and root finding standard: supported
|
||||
findings with `file:line`, failure scenario, and fix, or `No findings`.
|
||||
State the observed check status, including pending or unavailable checks, and
|
||||
the verdict (`APPROVE`, `REQUEST_CHANGES`, or `COMMENT`). Do not infer a pass
|
||||
from missing checks or add style nits to populate a clean review.
|
||||
Combine all subagent findings into a structured review:
|
||||
- **Summary**: one-paragraph overview of the change and overall assessment.
|
||||
- **Findings**: each finding with severity (critical/major/minor/nit), file:line, concrete failure scenario, and suggested fix.
|
||||
- **CI status**: pass/fail with notes on any failures.
|
||||
- **Verdict**: APPROVE, REQUEST_CHANGES, or COMMENT.
|
||||
|
||||
### 6. Post the review
|
||||
|
||||
Only when posting is authorized, write the review body to a temp file and post via CLI. Refresh the PR head first; if it changed, review the delta and update the verdict before posting:
|
||||
Write the review body to a temp file and post via CLI:
|
||||
```bash
|
||||
# Request changes
|
||||
gh pr review <N> --repo <owner/repo> --request-changes --body-file /tmp/pr_review.md
|
||||
gh pr review <N> --request-changes --body-file /tmp/pr_review.md
|
||||
|
||||
# Approve
|
||||
gh pr review <N> --repo <owner/repo> --approve --body-file /tmp/pr_review.md
|
||||
gh pr review <N> --approve --body-file /tmp/pr_review.md
|
||||
|
||||
# Comment only (no verdict)
|
||||
gh pr review <N> --repo <owner/repo> --comment --body-file /tmp/pr_review.md
|
||||
gh pr review <N> --comment --body-file /tmp/pr_review.md
|
||||
```
|
||||
|
||||
For authorized inline comments, use [the submission example](references/posting.md).
|
||||
For inline comments on specific lines, use the GitHub API:
|
||||
```bash
|
||||
cat > /tmp/pr_review.json <<'EOF'
|
||||
{
|
||||
"body": "review body",
|
||||
"event": "REQUEST_CHANGES",
|
||||
"comments": [
|
||||
{
|
||||
"path": "crates/foo/src/bar.rs",
|
||||
"line": 42,
|
||||
"body": "finding description"
|
||||
}
|
||||
]
|
||||
}
|
||||
EOF
|
||||
gh api --method POST /repos/{owner}/{repo}/pulls/<N>/reviews --input /tmp/pr_review.json
|
||||
```
|
||||
|
||||
Always use `--body-file` or `--input`, never inline multiline `--body`.
|
||||
|
||||
### 7. Handle follow-up
|
||||
|
||||
Follow the [PR lifecycle](../../references/pull-requests.md) and any explicit monitoring request. For follow-up, fetch the new head and compare the recorded reviewed SHA with the new SHA; revisit affected callers and findings. Never use an unfetched `origin/pull/<N>/head` ref as evidence. Update the posted review or resolve addressed threads only within existing authorization.
|
||||
If the review requests changes:
|
||||
- Monitor for new commits: `gh pr view <N> --json commits`
|
||||
- Re-review changed files only: `git diff pr-<N>..origin/pull/<N>/head`
|
||||
- Update the review when findings are addressed.
|
||||
|
||||
If CI was failing due to pre-existing main breakage:
|
||||
- Comment on the PR noting the failure is pre-existing.
|
||||
- Suggest updating the branch: `gh pr update-branch <N>`
|
||||
|
||||
## Output format
|
||||
|
||||
### PR Review: #<N> — <title>
|
||||
|
||||
**Author**: <author>
|
||||
**Risk tier**: exempt | mechanical | standard | high-risk
|
||||
**Changed files**: <count> across <cluster count> clusters
|
||||
|
||||
#### Summary
|
||||
<one-paragraph overview>
|
||||
|
||||
#### Findings
|
||||
| Severity | Location | Finding |
|
||||
|----------|----------|---------|
|
||||
| critical | file:line | concrete failure scenario |
|
||||
|
||||
#### CI Status
|
||||
- All checks pass / Failing: <details>
|
||||
|
||||
#### Verdict
|
||||
APPROVE / REQUEST_CHANGES / COMMENT
|
||||
|
||||
## Notes
|
||||
|
||||
- The user may ask for review in Chinese; respond in the same language but keep the review body in English per AGENTS.md rules.
|
||||
- When the user asks for "多角色对抗 review", run the full adversarial validation protocol — this skill's step 3 covers that.
|
||||
- If the PR is from a fork, check `maintainerCanModify` before attempting to push fixes.
|
||||
- For very large PRs, batch the review by functional area while keeping the same bounded review shape.
|
||||
- For very large PRs (>50 files), cluster aggressively and delegate in parallel to keep review time reasonable.
|
||||
|
||||
@@ -1,22 +0,0 @@
|
||||
# Inline PR Review Submission
|
||||
|
||||
Read only when an inline review is authorized. Recheck the PR head before posting and bind the review to the reviewed commit.
|
||||
|
||||
For inline comments on specific lines, use the GitHub API:
|
||||
```bash
|
||||
cat > /tmp/pr_review.json <<'EOF'
|
||||
{
|
||||
"commit_id": "<reviewed-head-sha>",
|
||||
"body": "review body",
|
||||
"event": "REQUEST_CHANGES",
|
||||
"comments": [
|
||||
{
|
||||
"path": "crates/foo/src/bar.rs",
|
||||
"line": 42,
|
||||
"body": "finding description"
|
||||
}
|
||||
]
|
||||
}
|
||||
EOF
|
||||
gh api --method POST /repos/{owner}/{repo}/pulls/<N>/reviews --input /tmp/pr_review.json
|
||||
```
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: rust-code-quality
|
||||
description: Run a focused Rust quality review when the user requests one or a selected review workflow needs Rust-specific checks for changed behavior. Do not auto-load for every implementation edit, comment-only or formatting-only Rust diff, or repeat an already completed review.
|
||||
description: Run a focused Rust quality review when the user requests one, when reviewing a Rust PR/commit, or when another selected review workflow delegates Rust-specific checks. Do not auto-load for every implementation edit.
|
||||
---
|
||||
|
||||
# Rust Code Quality Gate
|
||||
@@ -8,18 +8,12 @@ description: Run a focused Rust quality review when the user requests one or a s
|
||||
Use this skill for a dedicated Rust review to cover rules that `cargo clippy`
|
||||
does not catch.
|
||||
|
||||
Search matches and checklist items are candidates, not findings. Apply the root
|
||||
finding standard; distinguish a demonstrated bug, an explicit rule violation,
|
||||
and an optional preference. P2/P3 suggestions do not need to be invented or
|
||||
included in an otherwise clean correctness review.
|
||||
|
||||
## Quick Start
|
||||
|
||||
1. Identify changed `.rs` files.
|
||||
2. Run the matching candidate searches on changed files.
|
||||
3. Apply the manual checklist sections whose behavior the diff touches.
|
||||
4. Report or rebut every finding with evidence; P0/P1 findings block approval.
|
||||
Fix them when implementation is authorized; a read-only review reports them.
|
||||
2. Run automated checks on changed files.
|
||||
3. Run manual review checklist on the diff.
|
||||
4. Resolve or rebut every finding with evidence; P0/P1 findings cannot be deferred.
|
||||
|
||||
## Automated Checks
|
||||
|
||||
@@ -41,7 +35,7 @@ rg -n 'Result<.*String>' <changed-files>
|
||||
rg -n 'Box<dyn.*Error' <changed-files>
|
||||
|
||||
# 5. println/eprintln in production
|
||||
rg -n 'println!|eprintln!' <changed-files>
|
||||
rg -n 'println!\|eprintln!' <changed-files>
|
||||
|
||||
# 6. Ordering::Relaxed usage (verify each is intentional)
|
||||
rg -n 'Ordering::Relaxed' <changed-files>
|
||||
@@ -86,7 +80,7 @@ For the Rust diff under review, verify:
|
||||
- [ ] Test volume and line count are never treated as production-code growth
|
||||
|
||||
### Serde
|
||||
- [ ] Structs from untrusted input reject unknown fields where the compatibility contract permits; otherwise validate security-critical fields explicitly and test the supported input shape
|
||||
- [ ] Structs from untrusted input have `#[serde(deny_unknown_fields)]`
|
||||
- [ ] `#[serde(default)]` not used on security-critical fields without validation
|
||||
|
||||
### Code Hygiene
|
||||
@@ -110,7 +104,20 @@ For the Rust diff under review, verify:
|
||||
|
||||
## Output Template
|
||||
|
||||
Use the calling review's output format. For a standalone review, report supported
|
||||
findings with severity, location, impact, fix, and validation, or `No findings`.
|
||||
Include only material unverified checks. Candidate counts are not a quality
|
||||
metric and do not need a separate scan report.
|
||||
```
|
||||
## Rust Code Quality Report
|
||||
|
||||
### Automated Scan
|
||||
- unwrap/expect candidates inspected: N
|
||||
- numeric-cast candidates inspected: N
|
||||
- error-type candidates inspected: N
|
||||
- output-macro candidates inspected: N
|
||||
|
||||
### Findings
|
||||
- [P1] `path:line` — description
|
||||
- Fix: ...
|
||||
- Validation: ...
|
||||
|
||||
### Verdict
|
||||
PASS / BLOCKED (list blocking findings)
|
||||
```
|
||||
|
||||
@@ -4,14 +4,9 @@ description: "Run the end-to-end RustFS console gate, version bump, preview vali
|
||||
---
|
||||
# RustFS Release Publish (preview-validated pipeline)
|
||||
|
||||
This skill orchestrates a full release. It wraps `rustfs-release-version-bump` (invoked here with the authorized commit/push/PR scope) with a mandatory preview-tag validation loop before the final tag is published.
|
||||
This skill orchestrates a full release. It wraps `rustfs-release-version-bump` (which only edits version files and opens the PR) with a mandatory preview-tag validation loop before the final tag is published.
|
||||
|
||||
The binary reports its build tag (`build::TAG` via shadow_rs; `SHORT_VERSION` in
|
||||
`rustfs/src/config/cli.rs`), and `build.yml` derives asset names and preview
|
||||
classification from that tag. Cargo.toml supplies only the no-tag fallback.
|
||||
Preview and final tags must therefore share the validated source commit;
|
||||
their tag-dependent version and asset names differ. The channel and cleanup
|
||||
constraints are defined once under Preview tag naming and Hard rules below.
|
||||
Core design: **version files never carry a `-preview.N` suffix**. The preview suffix exists only in tag names. A preview tag creates a visible GitHub Release marked Prerelease and uploads versioned assets, but it never becomes GitHub Latest and never updates `*-latest`, `latest.json`, R2, Docker, or Helm channels. That Release is temporary: `build.yml` deletes it automatically once the final tag's Release is published, so the Releases page ends up carrying deliverables only while the `-preview.N` tags stay behind as the traceability record. This works because the binary self-reports the git tag it was built from (`build::TAG` via shadow_rs, see `rustfs/src/config/cli.rs` `SHORT_VERSION`), and `build.yml` derives artifact names and preview classification from the tag name — Cargo.toml's version is only a no-tag fallback. Therefore the preview tag and the final tag can (and MUST) point at the exact same commit: what you validated is byte-for-byte the source that ships.
|
||||
|
||||
Pipeline shape:
|
||||
|
||||
@@ -34,9 +29,9 @@ On validation failure: fix lands on main via normal PR (version files are alread
|
||||
- Final target version, for example `1.0.0-beta.10`.
|
||||
- Preview iteration `N` (default: next unused preview tag for that target; check with `git tag -l '<target>-preview.*'` after `git fetch --tags`).
|
||||
|
||||
If the target version is missing or ambiguous, collect the current release/tag baseline and ask before version edits or publication. Continue independent read-only preflight while the answer is pending (see the semver gate below).
|
||||
If the target version is missing or ambiguous, stop and ask before doing anything (see the semver gate below).
|
||||
|
||||
## Semver gate — resolve the target before version edits or publication
|
||||
## Semver gate — confirm the target version before touching anything
|
||||
|
||||
Versions follow [SemVer 2.0.0](https://semver.org/). Precedence reminder:
|
||||
|
||||
@@ -48,7 +43,7 @@ Numeric prerelease identifiers compare numerically (`beta.9 < beta.10`), not lex
|
||||
|
||||
Rules:
|
||||
|
||||
- A request like "发个版" / "release the next version" without an exact version string is ALWAYS ambiguous. Derive the current latest tag (`git tag --sort=-v:refname | head`), then ask the user to choose with concrete candidates, e.g. from `1.0.0-beta.10`: next prerelease `1.0.0-beta.11`, promote to `1.0.0-rc.1`, promote to stable `1.0.0`. Never guess between these — they have very different meanings (channel promotion vs. iteration) and different CI classification consequences.
|
||||
- A request like "发个版" / "release the next version" without an exact version string is ALWAYS ambiguous. Derive the current latest tag (`git tag --sort=-v:refname | head`), then ask the user to choose via AskUserQuestion with concrete candidates, e.g. from `1.0.0-beta.10`: next prerelease `1.0.0-beta.11`, promote to `1.0.0-rc.1`, promote to stable `1.0.0`. Never guess between these — they have very different meanings (channel promotion vs. iteration) and different CI classification consequences.
|
||||
- After a stable `X.Y.Z` exists, the next version must state which component bumps: patch `X.Y.(Z+1)` for fixes only, minor `X.(Y+1).0` for backward-compatible features, major `(X+1).0.0` for breaking changes. If the user names a bump type but not a number, compute it from the latest stable tag and echo the exact resulting version back for confirmation.
|
||||
- Echo the final confirmed version string verbatim in your first status report; every later phase must use exactly that string. If at any point the user's wording and the confirmed version diverge, stop and re-confirm.
|
||||
|
||||
@@ -82,7 +77,58 @@ Rules:
|
||||
|
||||
### Console release gate
|
||||
|
||||
Read and complete [the Console gate](references/console-gate.md) before Phase 1. Verify the latest published Console asset and exact commit; if Console main is ahead, complete its release and asset verification first. A successful build alone does not satisfy this gate.
|
||||
Complete this gate before changing any RustFS version file or creating any RustFS tag. RustFS `build.yml` downloads the asset returned by `repos/rustfs/console/releases/latest`, so a successful Console build alone is insufficient.
|
||||
|
||||
1. Read the latest published Console tag and compare it with Console `main`:
|
||||
|
||||
```bash
|
||||
CONSOLE_REPO="rustfs/console"
|
||||
CONSOLE_LATEST=$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)
|
||||
gh api "repos/${CONSOLE_REPO}/compare/${CONSOLE_LATEST}...main" \
|
||||
--jq '{status, ahead_by, behind_by, commits: [.commits[] | {sha, message: .commit.message}]}'
|
||||
```
|
||||
|
||||
- `ahead_by == 0`: no merged Console change is waiting for release. Still verify the current latest asset using step 4, then continue to Phase 1.
|
||||
- `ahead_by > 0` and `behind_by == 0`: publish Console before continuing. Report the merged commits and select the next unused `vX.Y.Z` tag. Default to the next patch version when the changes are fixes or backward-compatible UI work; stop for confirmation if a minor/major bump is plausible.
|
||||
- Any diverged history or `behind_by > 0`: stop and resolve the Console release baseline explicitly. Do not guess a range or publish RustFS.
|
||||
|
||||
2. Clone/fetch `rustfs/console` into a scratch directory and record its exact `main` commit. Before creating a tag, check for a `v*` tag or Release workflow already associated with that hash. If one is in progress, wait for it instead of creating another version:
|
||||
|
||||
```bash
|
||||
CONSOLE_SCRATCH=$(mktemp -d)
|
||||
gh repo clone "$CONSOLE_REPO" "$CONSOLE_SCRATCH/console"
|
||||
git -C "$CONSOLE_SCRATCH/console" fetch origin main --tags
|
||||
CONSOLE_HASH=$(git -C "$CONSOLE_SCRATCH/console" rev-parse origin/main)
|
||||
git -C "$CONSOLE_SCRATCH/console" tag --points-at "$CONSOLE_HASH" 'v*'
|
||||
gh run list -R "$CONSOLE_REPO" --workflow release.yml --commit "$CONSOLE_HASH" --limit 5
|
||||
```
|
||||
|
||||
If no release exists or is running for `CONSOLE_HASH`, create the selected annotated tag at that exact hash and push it:
|
||||
|
||||
```bash
|
||||
git -C "$CONSOLE_SCRATCH/console" tag -a "<console-tag>" -m "Release <console-tag>" "$CONSOLE_HASH"
|
||||
git -C "$CONSOLE_SCRATCH/console" push origin "<console-tag>"
|
||||
```
|
||||
|
||||
Console tags include the `v` prefix. Pushing the tag triggers `.github/workflows/release.yml` (`🚀 Release`). Remove `CONSOLE_SCRATCH` after the gate completes.
|
||||
|
||||
3. Find the exact tag run and wait for completion:
|
||||
|
||||
```bash
|
||||
gh run list -R "$CONSOLE_REPO" --workflow release.yml --branch "<console-tag>" --limit 1
|
||||
gh run watch -R "$CONSOLE_REPO" "<console-run-id>" --exit-status
|
||||
```
|
||||
|
||||
4. Block until the published Release is non-draft, the latest endpoint returns the expected tag, and `rustfs-console-<console-tag>.zip` is uploaded, non-empty, and carries a `sha256:` digest:
|
||||
|
||||
```bash
|
||||
gh release view -R "$CONSOLE_REPO" "<console-tag>" --json isDraft,isPrerelease,assets,url
|
||||
test "$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)" = "<console-tag>"
|
||||
test "$(gh api "repos/${CONSOLE_REPO}/releases/tags/<console-tag>" \
|
||||
--jq '[.assets[] | select(.name == "rustfs-console-<console-tag>.zip" and .state == "uploaded" and .size > 0 and (.digest | startswith("sha256:")))] | length')" -eq 1
|
||||
```
|
||||
|
||||
Treat a missing/mismatched asset, digest, latest tag, or failed/cancelled workflow as BLOCKED. Do not start Phase 1 until the Console gate passes. Record `CONSOLE_TAG`, `CONSOLE_HASH`, Console run URL, and Release URL for the final report.
|
||||
|
||||
## Phase 1 — Version bump to the final target (once)
|
||||
|
||||
@@ -118,9 +164,54 @@ On a restart (N+1), refresh `PREVIEW_HASH=$(git rev-parse origin/main)` first
|
||||
- Record `PREVIOUS_DELIVERABLE`, selected from published Releases by `publishedAt` after excluding the current tag and every `-preview.N` tag. Verify `gh release view "<preview-tag>" --json body --jq .body` contains `## What's Changed` and, when `PREVIOUS_DELIVERABLE` exists, `**Full Changelog**: https://github.com/rustfs/rustfs/compare/<PREVIOUS_DELIVERABLE>...<preview-tag>`. For a repository with no previous deliverable, verify a Full Changelog link exists and record the GitHub baseline fallback.
|
||||
- Confirm preview-triggered Docker and Helm jobs are skipped. Preview validation covers the built RustFS binaries, embedded console, and rc compatibility; Docker image construction and Helm publication are deferred to the final tag because the Dockerfiles consume GitHub Release assets.
|
||||
|
||||
## Phases 4–5 — Local artifact, Console, and rc acceptance
|
||||
## Phase 4 — Run the artifact locally, verify the console
|
||||
|
||||
Read and complete [preview acceptance](references/preview-acceptance.md): verify the downloaded binary's tag/SHA and readiness, exercise Console CRUD with byte-identical download, and pass the full latest-rc command matrix. Any failure blocks final publication. Retain the results for the confirmation gate below.
|
||||
Work inside the session scratchpad directory; never leave stray data dirs.
|
||||
|
||||
```bash
|
||||
gh release download "<preview-tag>" -p "rustfs-macos-aarch64-v<preview-tag>.zip" -D "$SCRATCH"
|
||||
cd "$SCRATCH" && unzip -o rustfs-*.zip
|
||||
./rustfs --version # must report the PREVIEW TAG (build::TAG), not the Cargo.toml version, plus expected short SHA
|
||||
mkdir -p data
|
||||
RUSTFS_ACCESS_KEY=rustfsadmin RUSTFS_SECRET_KEY=rustfsadmin ./rustfs ./data
|
||||
```
|
||||
|
||||
Defaults: S3 endpoint `:9000`, embedded console `:9001`.
|
||||
|
||||
Checks (all must pass):
|
||||
|
||||
- `./rustfs --version` reports the preview tag name and the short SHA of `PREVIEW_HASH`. Reporting `<target>` without the `-preview.N` suffix means the build did not embed the tag — treat as FAIL and investigate before proceeding.
|
||||
- `curl -fsS http://localhost:9000/health/ready` returns ready.
|
||||
- Startup log shows the embedded console being served (this was the regression that `fix(release): require embedded console assets` guards).
|
||||
- Open `http://localhost:9001` in the browser: login with `rustfsadmin`/`rustfsadmin`; dashboard renders without JS console errors; create a bucket, upload a file, download it back (byte-identical), delete the object and bucket. Keep the server running for Phase 5.
|
||||
|
||||
## Phase 5 — Validate with the latest rc client
|
||||
|
||||
`rc` is the RustFS CLI client from <https://github.com/rustfs/cli>.
|
||||
|
||||
- Ensure the latest release is installed: compare `rc --version` against `gh api repos/rustfs/cli/releases/latest --jq .tag_name`; update via `brew upgrade rustfs/tap/rc` (or download the release binary).
|
||||
- Point it at the preview server and run the command matrix, recording PASS/FAIL per command:
|
||||
|
||||
```bash
|
||||
rc alias set preview http://localhost:9000 rustfsadmin rustfsadmin
|
||||
rc ls preview/
|
||||
rc mb preview/rel-check
|
||||
rc cp <local-file> preview/rel-check/
|
||||
rc stat preview/rel-check/<file>
|
||||
rc cat preview/rel-check/<file> # matches source
|
||||
rc cp preview/rel-check/<file> ./out && cmp <local-file> ./out
|
||||
rc cp -r <local-dir>/ preview/rel-check/dir/
|
||||
rc find preview/rel-check --name "*"
|
||||
rc share download preview/rel-check/<file> --expire 1h # presigned URL fetchable via curl
|
||||
rc rm preview/rel-check/<file> && rc rm -r --force preview/rel-check/dir
|
||||
rc rb preview/rel-check
|
||||
rc admin user list preview/
|
||||
rc admin user add preview/ relcheckuser relchecksecret12
|
||||
rc admin user remove preview/ relcheckuser
|
||||
rc alias remove preview
|
||||
```
|
||||
|
||||
- Any FAIL blocks the release. Afterwards stop the server and delete the scratch data directory.
|
||||
|
||||
### Manual confirmation gate
|
||||
|
||||
|
||||
@@ -1,58 +0,0 @@
|
||||
# Console Release Gate
|
||||
|
||||
Read during Phase 0, before changing RustFS version files or tags. Follow the parent skill's release scope and authorization rules.
|
||||
|
||||
### Console release gate
|
||||
|
||||
Complete this gate before changing any RustFS version file or creating any RustFS tag. RustFS `build.yml` downloads the asset returned by `repos/rustfs/console/releases/latest`, so a successful Console build alone is insufficient.
|
||||
|
||||
1. Read the latest published Console tag and compare it with Console `main`:
|
||||
|
||||
```bash
|
||||
CONSOLE_REPO="rustfs/console"
|
||||
CONSOLE_LATEST=$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)
|
||||
gh api "repos/${CONSOLE_REPO}/compare/${CONSOLE_LATEST}...main" \
|
||||
--jq '{status, ahead_by, behind_by, commits: [.commits[] | {sha, message: .commit.message}]}'
|
||||
```
|
||||
|
||||
- `ahead_by == 0`: no merged Console change is waiting for release. Still verify the current latest asset using step 4, then continue to Phase 1.
|
||||
- `ahead_by > 0` and `behind_by == 0`: publish Console before continuing. Report the merged commits and select the next unused `vX.Y.Z` tag. Default to the next patch version when the changes are fixes or backward-compatible UI work; stop for confirmation if a minor/major bump is plausible.
|
||||
- Any diverged history or `behind_by > 0`: stop and resolve the Console release baseline explicitly. Do not guess a range or publish RustFS.
|
||||
|
||||
2. Clone/fetch `rustfs/console` into a scratch directory and record its exact `main` commit. Before creating a tag, check for a `v*` tag or Release workflow already associated with that hash. If one is in progress, wait for it instead of creating another version:
|
||||
|
||||
```bash
|
||||
CONSOLE_SCRATCH=$(mktemp -d)
|
||||
gh repo clone "$CONSOLE_REPO" "$CONSOLE_SCRATCH/console"
|
||||
git -C "$CONSOLE_SCRATCH/console" fetch origin main --tags
|
||||
CONSOLE_HASH=$(git -C "$CONSOLE_SCRATCH/console" rev-parse origin/main)
|
||||
git -C "$CONSOLE_SCRATCH/console" tag --points-at "$CONSOLE_HASH" 'v*'
|
||||
gh run list -R "$CONSOLE_REPO" --workflow release.yml --commit "$CONSOLE_HASH" --limit 5
|
||||
```
|
||||
|
||||
If no release exists or is running for `CONSOLE_HASH`, create the selected annotated tag at that exact hash and push it:
|
||||
|
||||
```bash
|
||||
git -C "$CONSOLE_SCRATCH/console" tag -a "<console-tag>" -m "Release <console-tag>" "$CONSOLE_HASH"
|
||||
git -C "$CONSOLE_SCRATCH/console" push origin "<console-tag>"
|
||||
```
|
||||
|
||||
Console tags include the `v` prefix. Pushing the tag triggers `.github/workflows/release.yml` (`🚀 Release`). Remove `CONSOLE_SCRATCH` after the gate completes.
|
||||
|
||||
3. Find the exact tag run and wait for completion:
|
||||
|
||||
```bash
|
||||
gh run list -R "$CONSOLE_REPO" --workflow release.yml --branch "<console-tag>" --limit 1
|
||||
gh run watch -R "$CONSOLE_REPO" "<console-run-id>" --exit-status
|
||||
```
|
||||
|
||||
4. Block until the published Release is non-draft, the latest endpoint returns the expected tag, and `rustfs-console-<console-tag>.zip` is uploaded, non-empty, and carries a `sha256:` digest:
|
||||
|
||||
```bash
|
||||
gh release view -R "$CONSOLE_REPO" "<console-tag>" --json isDraft,isPrerelease,assets,url
|
||||
test "$(gh api "repos/${CONSOLE_REPO}/releases/latest" --jq .tag_name)" = "<console-tag>"
|
||||
test "$(gh api "repos/${CONSOLE_REPO}/releases/tags/<console-tag>" \
|
||||
--jq '[.assets[] | select(.name == "rustfs-console-<console-tag>.zip" and .state == "uploaded" and .size > 0 and (.digest | startswith("sha256:")))] | length')" -eq 1
|
||||
```
|
||||
|
||||
Treat a missing/mismatched asset, digest, latest tag, or failed/cancelled workflow as BLOCKED. Do not start Phase 1 until the Console gate passes. Record `CONSOLE_TAG`, `CONSOLE_HASH`, Console run URL, and Release URL for the final report.
|
||||
@@ -1,52 +0,0 @@
|
||||
# Preview Artifact Acceptance
|
||||
|
||||
Read after Phase 3 succeeds. Complete every check below before the parent skill's manual confirmation gate. These checks cover the downloaded artifact, embedded Console, and latest rc client.
|
||||
|
||||
## Phase 4 — Run the artifact locally, verify the console
|
||||
|
||||
Work inside the session scratchpad directory; never leave stray data dirs.
|
||||
|
||||
```bash
|
||||
gh release download "<preview-tag>" -p "rustfs-macos-aarch64-v<preview-tag>.zip" -D "$SCRATCH"
|
||||
cd "$SCRATCH" && unzip -o rustfs-*.zip
|
||||
./rustfs --version # must report the PREVIEW TAG (build::TAG), not the Cargo.toml version, plus expected short SHA
|
||||
mkdir -p data
|
||||
RUSTFS_ACCESS_KEY=rustfsadmin RUSTFS_SECRET_KEY=rustfsadmin ./rustfs ./data
|
||||
```
|
||||
|
||||
Defaults: S3 endpoint `:9000`, embedded console `:9001`.
|
||||
|
||||
Checks (all must pass):
|
||||
|
||||
- `./rustfs --version` reports the preview tag name and the short SHA of `PREVIEW_HASH`. Reporting `<target>` without the `-preview.N` suffix means the build did not embed the tag — treat as FAIL and investigate before proceeding.
|
||||
- `curl -fsS http://localhost:9000/health/ready` returns ready.
|
||||
- Startup log shows the embedded console being served (this was the regression that `fix(release): require embedded console assets` guards).
|
||||
- Open `http://localhost:9001` in the browser: login with `rustfsadmin`/`rustfsadmin`; dashboard renders without JS console errors; create a bucket, upload a file, download it back (byte-identical), delete the object and bucket. Keep the server running for Phase 5.
|
||||
|
||||
## Phase 5 — Validate with the latest rc client
|
||||
|
||||
`rc` is the RustFS CLI client from <https://github.com/rustfs/cli>.
|
||||
|
||||
- Ensure the latest release is installed: compare `rc --version` against `gh api repos/rustfs/cli/releases/latest --jq .tag_name`; update via `brew upgrade rustfs/tap/rc` (or download the release binary).
|
||||
- Point it at the preview server and run the command matrix, recording PASS/FAIL per command:
|
||||
|
||||
```bash
|
||||
rc alias set preview http://localhost:9000 rustfsadmin rustfsadmin
|
||||
rc ls preview/
|
||||
rc mb preview/rel-check
|
||||
rc cp <local-file> preview/rel-check/
|
||||
rc stat preview/rel-check/<file>
|
||||
rc cat preview/rel-check/<file> # matches source
|
||||
rc cp preview/rel-check/<file> ./out && cmp <local-file> ./out
|
||||
rc cp -r <local-dir>/ preview/rel-check/dir/
|
||||
rc find preview/rel-check --name "*"
|
||||
rc share download preview/rel-check/<file> --expire 1h # presigned URL fetchable via curl
|
||||
rc rm preview/rel-check/<file> && rc rm -r --force preview/rel-check/dir
|
||||
rc rb preview/rel-check
|
||||
rc admin user list preview/
|
||||
rc admin user add preview/ relcheckuser relchecksecret12
|
||||
rc admin user remove preview/ relcheckuser
|
||||
rc alias remove preview
|
||||
```
|
||||
|
||||
- Any FAIL blocks the release. Afterwards stop the server and delete the scratch data directory.
|
||||
@@ -4,16 +4,17 @@ description: "Prepare the version-file and release-asset bump for an exact RustF
|
||||
---
|
||||
# RustFS Release Version Bump
|
||||
|
||||
Use this skill to prepare and verify release version files. Commit, push, and PR steps apply only when included in the user's delivery scope; publishing release tags belongs to `rustfs-release-publish`.
|
||||
Use this skill to publish a RustFS release (alpha, beta, or stable) with a minimal, auditable diff and a complete ship flow (`edit -> verify -> commit -> push -> PR`).
|
||||
|
||||
Validated baseline: release pattern used in PR `#2957`.
|
||||
|
||||
## Required inputs
|
||||
|
||||
- Exact target version, for example `1.0.0-beta.4`.
|
||||
- Delivery scope: local (`edit/verify`), git (`commit/push`), or GitHub
|
||||
(`commit/push/PR`). Derive it from the conversation; when unspecified, prepare
|
||||
and verify locally without blocking on a delivery question.
|
||||
- Delivery scope:
|
||||
- Local only (`edit/verify`).
|
||||
- Local + git (`commit/push`).
|
||||
- Full GitHub flow (`commit/push/PR`).
|
||||
|
||||
If target version is missing or ambiguous, stop and ask before editing.
|
||||
|
||||
@@ -22,7 +23,7 @@ Reject any target version containing `-preview`: preview identifiers are tag-onl
|
||||
## Read before editing
|
||||
|
||||
- `AGENTS.md` (root and nearest path-specific files).
|
||||
- `.github/pull_request_template.md` only when preparing a PR.
|
||||
- `.github/pull_request_template.md`.
|
||||
- Current branch status and diff against `origin/main`.
|
||||
|
||||
## Default release file scope
|
||||
@@ -49,7 +50,8 @@ Only drop a file when the current repository release process clearly no longer r
|
||||
## Step-by-step workflow
|
||||
|
||||
1. Confirm intent and isolate scope
|
||||
- Use the exact target and delivery scope already supplied; ask only for a missing or ambiguous target or a material release-policy choice.
|
||||
- Confirm target version string exactly.
|
||||
- Confirm whether user requested local-only or full GitHub flow.
|
||||
- Inspect current branch and ensure only release-related files are touched for this task.
|
||||
|
||||
2. Update workspace versions
|
||||
@@ -80,18 +82,18 @@ Only drop a file when the current repository release process clearly no longer r
|
||||
4. Verify before shipping
|
||||
- Run:
|
||||
- `make pre-commit`
|
||||
- If `make pre-commit` fails, fix task-attributable failures and rerun affected checks. Report unresolved required checks as `BLOCKED`; do not silently widen scope to fix unrelated issues.
|
||||
- If `make pre-commit` fails, return `BLOCKED` with root cause and do not silently widen scope to fix unrelated issues unless user asks.
|
||||
|
||||
5. Commit strategy (only when committing is authorized)
|
||||
5. Commit strategy
|
||||
- Preferred split when both parts changed:
|
||||
- `chore(release): prepare <version>` for `Cargo.toml` and `Cargo.lock`.
|
||||
- `chore(release): align release assets for <version>` for docs and packaging files.
|
||||
- If user asks for one commit, use one commit.
|
||||
- Stage only intended release files; do not include unrelated working tree changes.
|
||||
|
||||
6. Push and PR (only for the authorized delivery scope)
|
||||
6. Push and PR
|
||||
- Push branch:
|
||||
- Use the user-requested or configured push remote: `git push -u <push-remote> <branch>` (first push), or `git push` when tracking is already configured.
|
||||
- `git push -u origin <branch>` (first push), or `git push` (tracking already exists).
|
||||
- Create PR with template headings unchanged:
|
||||
- `gh pr create --base main --head <branch> --title ... --body-file ...`
|
||||
- PR title/body must be English.
|
||||
|
||||
@@ -12,9 +12,8 @@ matched security surface, the concise security reference under
|
||||
## Workflow
|
||||
|
||||
1. Freeze the exact diff/head and identify the changed trust boundaries.
|
||||
2. Inspect the headings in [advisory-patterns.md](references/advisory-patterns.md),
|
||||
then read the matching sections. Read the full map only for a broad security
|
||||
audit. Useful headings are
|
||||
2. Read [advisory-patterns.md](references/advisory-patterns.md), then apply only
|
||||
the matching sections. Useful headings are
|
||||
auth/admin, IAM/STS/OIDC, policy/plugins, S3/copy/multipart, protocols, paths,
|
||||
secrets/logging/RPC, browser/CORS/proxy, SSE, Object Lock, and serde.
|
||||
3. Trace unauthenticated, low-privilege, wrong-action/owner/bucket, malformed,
|
||||
|
||||
@@ -108,9 +108,9 @@ Update this file only when an advisory adds or changes a reusable lesson, affect
|
||||
|
||||
### Serde deserialization and input validation
|
||||
|
||||
- Reject unknown fields in untrusted S3 API XML/JSON, lifecycle, policy, and replication input where compatibility permits. Check the current type and supported payload fixtures; do not infer the repository's current coverage from an older audit. Where extra fields are part of the compatibility contract, validate security-critical values explicitly.
|
||||
- No `#[serde(deny_unknown_fields)]` found across the entire codebase. Lesson: all structs deserialized from untrusted input (S3 API XML/JSON, lifecycle rules, bucket policies, replication configs) should have `#[serde(deny_unknown_fields)]` to reject malformed or adversarial payloads.
|
||||
- `#[serde(default)]` on security-critical fields silently accepts missing values as zero/empty. Lesson: when a field has security implications (retention days, permissions, limits), validate the deserialized value explicitly rather than relying on defaults.
|
||||
- Integer fields deserialized from user input and cast with `as` (e.g., `i32 as u32`) can wrap negative values to large positives. Lesson: validate ranges before casting; use `try_into()` with a typed error, or clamp only when the domain explicitly requires saturation.
|
||||
- Integer fields deserialized from user input and cast with `as` (e.g., `i32 as u32`) can wrap negative values to large positives. Lesson: validate ranges before casting; use `try_into()` or clamp.
|
||||
- XML config typos (e.g., `"NoncurentDays"` instead of `"NoncurrentDays"`) are silently accepted when `deny_unknown_fields` is absent. Lesson: strict deserialization prevents silent misconfiguration that could cause data loss or unexpected retention behavior.
|
||||
|
||||
## Useful Search Seeds
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: test-coverage-improver
|
||||
description: Analyze a supplied coverage report or perform an explicitly requested RustFS coverage assessment, rank uncovered risks, and propose focused tests. Do not trigger for ordinary implementation verification, a single regression test, documentation wording, or release preparation without a coverage request.
|
||||
description: Run project coverage checks, rank high-risk gaps, and propose high-impact tests to improve regression confidence for changed and critical code paths before release.
|
||||
---
|
||||
|
||||
# Test Coverage Improver
|
||||
@@ -9,49 +9,58 @@ Use this skill when you need a prioritized, risk-aware plan to improve tests fro
|
||||
|
||||
## Usage assumptions
|
||||
- Focus scope is either changed lines/files, a module, or the whole repository.
|
||||
- Reuse a supplied coverage artifact when its revision, scope, and format match.
|
||||
- Coverage artifact must be generated or provided in a supported format.
|
||||
- If required context is missing, call out assumptions explicitly before proposing work.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. Define scope and baseline
|
||||
- Derive the revision and scope from the request, diff, or supplied report.
|
||||
- Default to the affected files/module; whole-workspace coverage requires that
|
||||
scope in the request. Ask only if a wrong scope would change the result.
|
||||
- Confirm target language, framework, and branch.
|
||||
- Confirm whether the scope is changed files only or full-repo.
|
||||
|
||||
2. Obtain coverage evidence
|
||||
- First inspect a matching existing artifact; do not regenerate it merely
|
||||
because this skill was selected.
|
||||
- If measurement is needed, read the Coverage section of
|
||||
[the testing guide](../../../docs/testing/README.md#coverage), check disk
|
||||
space/tool availability, and select package/test-scoped `cargo llvm-cov`
|
||||
using the repository's nextest configuration. `make coverage` measures the
|
||||
whole workspace (excluding E2E) and is only for that requested scope.
|
||||
- Collect only metrics the report supports. Missing branch/changed-line
|
||||
coverage is unknown, not zero.
|
||||
- If measurement cannot run, continue with code-based test proposals and
|
||||
mark measured coverage unverified; do not invent a coverage percentage.
|
||||
2. Produce coverage snapshot
|
||||
- Rust: `cargo llvm-cov` (or `cargo tarpaulin`) with existing repo config.
|
||||
- JavaScript/TypeScript: `npm test -- --coverage` and read `coverage/coverage-final.json`.
|
||||
- Python: `pytest --cov=<pkg> --cov-report=json` and read `coverage.json`.
|
||||
- Collect total, per-file, and changed-line coverage.
|
||||
|
||||
3. Rank highest-risk gaps
|
||||
- Prioritize changed code, branch coverage gaps, and low-confidence boundaries.
|
||||
- Apply the risk rubric in [coverage-prioritization.md](references/coverage-prioritization.md).
|
||||
- Report up to 5–8 evidenced gaps; do not pad a small scope.
|
||||
- Keep shortlist to 5–8 gaps.
|
||||
- For each gap, capture: file, lines, uncovered branches, and estimated risk score.
|
||||
|
||||
4. Propose high-impact tests
|
||||
- For each gap, name the behavior and regression, distinguishing assertions,
|
||||
relevant normal/edge/failure cases, necessary setup, and estimated effort.
|
||||
- Include only scenarios and setup that apply; reuse shared fixture details.
|
||||
- For each shortlisted gap, output:
|
||||
- Intent and expected behavior.
|
||||
- Normal, edge, and failure scenarios.
|
||||
- Assertions and side effects to verify.
|
||||
- Setup needs (fixtures, mocks, integration dependencies).
|
||||
- Estimated effort (`S/M/L`).
|
||||
|
||||
5. Close with validation plan
|
||||
- State which gaps remain after proposals.
|
||||
- Give a scoped verification command and behavior-based acceptance criterion;
|
||||
use a coverage threshold only when the task or repository requires one.
|
||||
- Provide concrete verification command and acceptance threshold.
|
||||
- List assumptions or blockers (environment, fixtures, flaky dependencies).
|
||||
|
||||
## Report
|
||||
## Output template
|
||||
|
||||
Summarize the supported metrics, then combine each ranked gap with its proposed
|
||||
test and validation. Include source lines only when supplied or inspected;
|
||||
mark missing metrics or locations as unknown. Do not duplicate gaps and tests
|
||||
in separate templates or fill empty categories for an otherwise small report.
|
||||
### Coverage Snapshot
|
||||
- total / branch coverage
|
||||
- changed-file coverage
|
||||
- top missing regions by size
|
||||
|
||||
### Top Gaps (ranked)
|
||||
- `path:line-range` | risk score | why critical
|
||||
|
||||
### Test Proposals
|
||||
- `path:line-range`
|
||||
- Test name
|
||||
- scenarios
|
||||
- assertions
|
||||
- effort
|
||||
|
||||
### Validation Plan
|
||||
- command
|
||||
- pass criteria
|
||||
- remaining risk
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
interface:
|
||||
display_name: "Test Coverage Improver"
|
||||
short_description: "Find top uncovered risk areas and propose high-impact tests."
|
||||
default_prompt: "Use $test-coverage-improver to analyze coverage for the requested scope, reuse matching reports, and propose tests for evidenced risks."
|
||||
default_prompt: "Run coverage checks, identify largest gaps, and recommend highest-impact test cases to improve risk coverage."
|
||||
|
||||
@@ -5,11 +5,8 @@ description: Debug ILM tiering / lifecycle transition issues — NoSuchVersion o
|
||||
|
||||
# Tier / ILM Debugging
|
||||
|
||||
Playbook: [docs/operations/tier-ilm-debugging.md](../../../docs/operations/tier-ilm-debugging.md).
|
||||
Read the section matching the symptom: metadata/`xl.meta`, runtime versionId,
|
||||
manual jobs, or retained-record recovery. Read the local-first expiry invariant
|
||||
before changing cleanup ordering. Before a reconcile/disposition action, read
|
||||
its entire procedure and retain its exact-evidence and confirmation gates.
|
||||
Full playbook: [docs/operations/tier-ilm-debugging.md](../../../docs/operations/tier-ilm-debugging.md)
|
||||
— read it before changing tier code.
|
||||
|
||||
Quick moves:
|
||||
|
||||
|
||||
@@ -1,5 +0,0 @@
|
||||
# Bound individual tool outputs retained in context; retrieve relevant ranges
|
||||
# from task-owned log files when more evidence is needed.
|
||||
# https://learn.chatgpt.com/docs/config-file/config-reference
|
||||
|
||||
tool_output_token_limit = 4000
|
||||
@@ -82,14 +82,6 @@ jobs:
|
||||
cache_key: e2e-odm-config-rollback
|
||||
test: rc5_rollback_requires_restoring_odm_configuration
|
||||
artifact: odm-config-rollback
|
||||
- name: Multipart layouts survive the rc.5 upgrade
|
||||
cache_key: e2e-multipart-layout-upgrade
|
||||
test: direct_upgrade_from_rc5_preserves_multipart_layouts
|
||||
artifact: multipart-layout-upgrade
|
||||
- name: rc.5 multipart replication baseline
|
||||
cache_key: e2e-multipart-layout-baseline
|
||||
test: rc5_baseline_replicates_multipart_layouts
|
||||
artifact: multipart-layout-baseline
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
env:
|
||||
|
||||
@@ -7,11 +7,8 @@ This file contains repository-wide rules. Use the nearest subdirectory
|
||||
|
||||
1. System/developer instructions.
|
||||
2. The current user request.
|
||||
3. Applicable `AGENTS.md` files, with the nearest file winning conflicts.
|
||||
4. Selected skills and reference documents.
|
||||
|
||||
Nested instructions add to ancestor rules; they do not discard non-conflicting
|
||||
rules. A skill cannot expand the user's requested scope or grant authorization.
|
||||
3. The nearest `AGENTS.md`.
|
||||
4. This file.
|
||||
|
||||
## Operating Model
|
||||
|
||||
@@ -24,29 +21,63 @@ rules. A skill cannot expand the user's requested scope or grant authorization.
|
||||
- Do not load every skill or inspect unrelated modules preemptively. Select a
|
||||
skill only when its description directly matches the request or changed
|
||||
surface.
|
||||
- Resolve repository workflow skills under `.agents/skills/` when a global
|
||||
skill has the same name, unless the user explicitly selects another path.
|
||||
- Avoid repeated reads and equivalent verification commands once enough
|
||||
evidence exists.
|
||||
- Search for relevant symbols/headings before reading long files; return only
|
||||
matching ranges. If output is truncated, narrow the query instead of repeating
|
||||
a full read. Keep reusable raw logs in task artifacts and report the evidence.
|
||||
- Reuse authorization already given in the conversation. Resolve routine choices
|
||||
within that scope and continue independent work while a material question is
|
||||
pending. Before requesting missing approval, prepare the concrete result that
|
||||
is already authorized; retain explicit merge and release gates.
|
||||
|
||||
## Task-Specific Guidance
|
||||
## Worktree and Disk Hygiene
|
||||
|
||||
Read only the reference needed for the current task, once per unchanged context:
|
||||
- Start implementation from the latest `origin/main` and confirm the requested
|
||||
change is not already present.
|
||||
- An existing clean, isolated task worktree is sufficient. Create another
|
||||
worktree only when the current checkout is shared, dirty with unrelated work,
|
||||
or belongs to another task.
|
||||
- Never commit from a shared checkout.
|
||||
- Use a task-specific branch named `<type>/<topic>`, such as `fix/...`,
|
||||
`feat/...`, `test/...`, or `docs/...`, unless the user specifies a name.
|
||||
- Do not include agent, tool, contributor, account, or organization names in
|
||||
branch names.
|
||||
- Push to the user-requested remote or the repository's configured push remote.
|
||||
Do not hard-code or infer a remote from an account name.
|
||||
- Check free space before artifact-heavy builds, tests, coverage, or downloads.
|
||||
Re-check before a broad gate when space is tight.
|
||||
- Remove only task-owned temporary/build artifacts. Never delete another task's
|
||||
worktree or uncommitted data.
|
||||
- At handoff, mention disk or cleanup details only when they affected execution
|
||||
or artifacts/worktrees remain intentionally.
|
||||
|
||||
- Before code changes or artifact-heavy work, read [implementation rules](.agents/references/implementation.md).
|
||||
For a read-only code review, use its change-style and boundary sections as needed.
|
||||
- Before commits, pushes, PR creation/updates, or posting to PRs/issues/discussions,
|
||||
read [Git and PR rules](.agents/references/pull-requests.md).
|
||||
Reuse existing authorization; a reference does not authorize posting, merging, or publishing.
|
||||
- Preserve unrelated work. Never commit from a shared checkout or delete another task's artifacts.
|
||||
- Source comments, commits, PR titles, and PR bodies are in English.
|
||||
## Change Style
|
||||
|
||||
- Preserve existing control flow unless changing it is required for correctness.
|
||||
- Prefer a direct local edit over new files, wrappers, managers, or speculative
|
||||
abstractions.
|
||||
- Add a helper only when it removes current duplication, names a real domain
|
||||
boundary, or isolates a non-trivial invariant.
|
||||
- Remove an in-scope path superseded by the change. If compatibility requires it,
|
||||
adapt at the boundary to one canonical core and use the repository's
|
||||
`RUSTFS_COMPAT_TODO` policy.
|
||||
- Comments explain non-obvious invariants or reasons. Do not narrate code or
|
||||
record change history.
|
||||
- Mention unrelated problems when useful; do not fix them in a narrow task.
|
||||
|
||||
## Reuse and Boundary Rules
|
||||
|
||||
- Before adding helpers, constants, fixtures, or wrappers, search the touched
|
||||
crate, the domain-owning crate, `crates/utils`, `crates/common`, and relevant
|
||||
direct dependencies.
|
||||
- Reuse requires matching semantics: normalization, error types, deadlines,
|
||||
durability, and compatibility must fit the call site. A narrowly named local
|
||||
helper is better than forced reuse with different semantics.
|
||||
- Validate untrusted input at its trust boundary, then trust the validated type.
|
||||
Values crossing disk, RPC, persistence, or version boundaries remain
|
||||
untrusted at every consumer.
|
||||
- Re-check boundary values immediately before destructive actions such as
|
||||
delete, overwrite, or quorum decisions.
|
||||
- Every new branch needs a concrete triggering input/state. For decoded or peer
|
||||
data, corruption and mixed-version input are valid triggers.
|
||||
- Required values must return a typed error when absent or corrupt; do not use a
|
||||
default that converts corruption into a plausible result.
|
||||
- Attach error context once where it is actionable. Do not erase typed errors
|
||||
below aggregation or quorum layers.
|
||||
|
||||
## Sources of Truth
|
||||
|
||||
@@ -116,25 +147,73 @@ requested adversarial/design reviews, and agent-instruction changes that alter
|
||||
execution. Ordinary questions, diagnoses, status reports, non-adversarial code
|
||||
reviews, and low-risk planning do not trigger it.
|
||||
|
||||
For applicable work and substantial PR reviews, read the [risk tiers and review shape](.agents/references/adversarial-validation.md).
|
||||
Load only the matching domain probes; ordinary reviews do not become adversarial
|
||||
merely because this reference exists.
|
||||
Risk and review shape:
|
||||
|
||||
A review has no finding quota; `No findings` is a complete outcome. A request to
|
||||
find problems is not evidence that a defect exists. Before reporting a candidate,
|
||||
check callers, invariants, and existing tests for evidence that disproves it.
|
||||
Findings need `file:line` and a concrete failure or violation of an explicit
|
||||
requirement. Missing required tests/checks are verification gaps, not proof of a
|
||||
runtime bug; name the unprotected behavior or unmet gate. Keep optional style or
|
||||
refactoring preferences out of defect findings unless that review was requested.
|
||||
- **Exempt:** documentation, comments, formatting, or typos with no runtime,
|
||||
build, test, or agent-execution effect.
|
||||
- **Mechanical:** renames, moves, test/tooling-only changes, and agent-rule
|
||||
changes. Run correctness and simplicity lenses.
|
||||
- **Standard:** localized behavior changes. Run one integrated final-diff pass
|
||||
covering correctness, simplicity, and test coverage; add only domain lenses
|
||||
matched by the diff.
|
||||
- **High risk / substantial PR review:** high risk includes locking,
|
||||
erasure/quorum/heal, replication, multipart, RPC, lifecycle/tiering,
|
||||
persistence/fsync, IAM/KMS/auth, cryptography, on-disk/on-wire formats, and
|
||||
S3-visible semantics. Cover all applicable lenses using exactly two
|
||||
independent reviewers when delegation is explicitly authorized. Split the
|
||||
lenses between them. Otherwise perform two fresh sequential passes.
|
||||
- **Outbound client defaults:** what `TargetClient`, `PutObjectOptions`, or
|
||||
the remote SDK configuration sends to every replication or migration target
|
||||
is high risk for every target class even when the change fixes one. Follow
|
||||
the SOP in `docs/postmortems/2026-09-03-replication-checksum-default-regression.md`:
|
||||
run the outbound target matrix, document each new env knob in the same PR,
|
||||
and list verified and unverified target classes in the PR Impact section.
|
||||
|
||||
Fix or rebut supported findings within the authorized scope. Once the required
|
||||
passes are complete, stop. Reopen only for changed code, new evidence, an
|
||||
unresolved finding, or an explicit re-review request; an unchanged diff does not
|
||||
need another pass at every conversation turn or workflow handoff.
|
||||
Available domain lenses are security, concurrency/durability, compatibility,
|
||||
and performance. Select `.agents/skills/adversarial-validation/SKILL.md` for an
|
||||
explicit adversarial request, a high-risk change, or a substantial PR review;
|
||||
then read only its matching role references. A routine standard pass does not
|
||||
load the playbook unless the reviewer needs a RustFS-specific probe.
|
||||
|
||||
A finding must name a concrete input/state/interleaving and wrong outcome, or a
|
||||
specific missing regression check, with `file:line`. Resolve it by fixing the
|
||||
diff or rebutting it with code-path/test/invariant evidence. After a non-trivial
|
||||
fix, rerun only affected lenses.
|
||||
|
||||
For high-risk PRs, record one concise verdict per covered lens in the PR body.
|
||||
|
||||
## Pull Request Lifecycle
|
||||
|
||||
- Creating or updating a PR includes one immediate snapshot of checks,
|
||||
mergeability, reviews, and unresolved threads.
|
||||
- Unless the user explicitly requests monitoring, a release workflow requires
|
||||
it, or an automation already owns it, hand off after the PR is open with the
|
||||
current state and next event to watch. Do not delay ordinary handoff with
|
||||
fixed quiet-period sleeps.
|
||||
- For requested monitoring, use event-driven or bounded waits. Report only state
|
||||
changes, actionable failures, or a meaningful prolonged delay.
|
||||
- Investigate failures/comments before changing code. Fix task-attributable
|
||||
issues, rerun affected verification, push, reply or resolve the thread, then
|
||||
resume the requested monitor.
|
||||
- Never merge without required reviewer approval or explicit authority.
|
||||
- After an observed merge, verify the commit reached the base, then clean the
|
||||
task worktree/branch when safe. Preserve unmerged work for closed PRs unless
|
||||
deletion was explicitly authorized.
|
||||
|
||||
## Git and PR Baseline
|
||||
|
||||
- Follow Conventional Commits; keep the subject at most 72 characters.
|
||||
- Source comments, commits, PR titles, and PR bodies are in English.
|
||||
- Keep every heading from `.github/pull_request_template.md`; use `N/A` where
|
||||
needed and include commands actually run.
|
||||
- Use `--body-file` for multiline `gh pr create`/`gh pr edit` content.
|
||||
- PR/issue/discussion content must not contain the literal sequence `\n` or
|
||||
hard-wrapped prose paragraphs.
|
||||
- Do not include local absolute paths or tool-specific labels/prefixes in GitHub
|
||||
content.
|
||||
- Resolve review threads after the underlying issue is fixed. If declining a
|
||||
suggestion, reply with a short evidence-based reason.
|
||||
|
||||
## Security Baseline
|
||||
|
||||
- Never commit secrets, credentials, or key material.
|
||||
@@ -171,6 +250,12 @@ Use `.agents/skills/rustfs-logging-governance/SKILL.md` for logging changes.
|
||||
- `DataUsageCacheInfo` and `DataUsageEntry` keep their hand-written map
|
||||
serialization and new fields remain `#[serde(default)]` for older readers.
|
||||
|
||||
## Naming
|
||||
|
||||
Use Rust API naming: `SCREAMING_SNAKE_CASE` constants/statics, `snake_case`
|
||||
functions/variables, and `PascalCase` types. Do not rename unrelated existing
|
||||
violations.
|
||||
|
||||
## Scoped Guidance
|
||||
|
||||
Before editing, locate the nearest instructions with:
|
||||
@@ -180,4 +265,4 @@ git ls-files '*AGENTS.md'
|
||||
```
|
||||
|
||||
The nearest file wins for domain invariants. Keep generic workflow and
|
||||
validation policy in this root file and its task-specific references.
|
||||
validation policy in this root file.
|
||||
|
||||
@@ -13,7 +13,7 @@ what Claude Code needs on top: commands and pointers.
|
||||
cargo build --release --bin rustfs # production binary
|
||||
cargo check -p <crate> # fast type-check one crate
|
||||
cargo test -p <crate> # test one crate
|
||||
cargo fmt --all --check # for Rust changes; see AGENTS.md verification tiers
|
||||
cargo fmt --all # format (required before PR)
|
||||
make pre-commit # fast gate: fmt + arch checks + quick-check (NO clippy/tests)
|
||||
make pre-pr # optional full gate for broad cross-module changes
|
||||
make build-docker BUILD_OS=ubuntu22.04
|
||||
@@ -42,5 +42,5 @@ make build-docker BUILD_OS=ubuntu22.04
|
||||
|
||||
Repo-wide domain invariants (dual internal metadata keys, defensive UUID
|
||||
reads, unversioned tier buckets) live in [AGENTS.md](AGENTS.md) under
|
||||
"Cross-Cutting Storage Invariants" — read them before touching metadata or
|
||||
"Cross-Cutting Domain Invariants" — read them before touching metadata or
|
||||
tiering code.
|
||||
|
||||
+2
-5
@@ -33,11 +33,8 @@ Applies to all paths under `crates/`.
|
||||
|
||||
## Type Casting
|
||||
|
||||
- Never use `as` for numeric conversions that may truncate or overflow. Use
|
||||
`try_into()` with typed error handling; clamp or saturate only when the domain
|
||||
explicitly requires it.
|
||||
- Before converting floating-point input to an integer, validate finiteness,
|
||||
sign, and the destination range. A lower-bound clamp alone is insufficient.
|
||||
- Never use `as` for numeric conversions that may truncate or overflow. Use `try_into()` with explicit error handling, or clamp with `value.max(0) as usize` when the domain is bounded.
|
||||
- `f64 as usize` saturates but is fragile; clamp to `[0, usize::MAX as f64]` first.
|
||||
- Treat every `as` cast in a PR review as a potential bug; require justification.
|
||||
|
||||
## Testing
|
||||
|
||||
@@ -20,7 +20,7 @@ use crate::heal::{
|
||||
task::{HealOptions, HealPriority, HealRequest, HealTask, HealTaskStatus, HealType, demote_to_debug_when},
|
||||
};
|
||||
use crate::{Error, Result};
|
||||
use metrics::{counter, gauge};
|
||||
use metrics::{counter, gauge, histogram};
|
||||
use rustfs_concurrency::WorkloadAdmissionSnapshotProvider;
|
||||
use rustfs_concurrency::workload::{ForegroundPressure, foreground_pressure};
|
||||
#[cfg(test)]
|
||||
@@ -34,7 +34,7 @@ use std::sync::LazyLock;
|
||||
use std::{
|
||||
collections::{BinaryHeap, HashMap, HashSet},
|
||||
sync::{Arc, Mutex as StdMutex, MutexGuard as StdMutexGuard},
|
||||
time::{Duration, SystemTime},
|
||||
time::{Duration, Instant, SystemTime},
|
||||
};
|
||||
use tokio::{
|
||||
sync::{Mutex, Notify, RwLock},
|
||||
@@ -181,6 +181,13 @@ fn lock_displaced_terminals(
|
||||
}
|
||||
}
|
||||
|
||||
fn lock_admission_telemetry(registry: &StdMutex<HealAdmissionTelemetry>) -> StdMutexGuard<'_, HealAdmissionTelemetry> {
|
||||
match registry.lock() {
|
||||
Ok(guard) => guard,
|
||||
Err(poisoned) => poisoned.into_inner(),
|
||||
}
|
||||
}
|
||||
|
||||
fn record_displaced_terminal(
|
||||
registry: &StdMutex<HashMap<String, Arc<CompletedHealStatus>>>,
|
||||
request: &HealRequest,
|
||||
@@ -384,6 +391,61 @@ impl HealSourceCounts {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
|
||||
#[serde(rename_all = "camelCase", deny_unknown_fields)]
|
||||
pub struct HealAdmissionTelemetry {
|
||||
pub accepted: u64,
|
||||
pub merged: u64,
|
||||
pub full: u64,
|
||||
pub dropped: u64,
|
||||
pub duplicate: u64,
|
||||
pub overlap_rejected: u64,
|
||||
pub displaced: u64,
|
||||
pub force_start: u64,
|
||||
pub max_start_duration_micros: u64,
|
||||
pub max_lock_phase_micros: u64,
|
||||
}
|
||||
|
||||
impl HealAdmissionTelemetry {
|
||||
fn record(&mut self, observation: HealAdmissionObservation) {
|
||||
match observation.result {
|
||||
HealAdmissionResult::Accepted => self.accepted = self.accepted.saturating_add(1),
|
||||
HealAdmissionResult::Merged => self.merged = self.merged.saturating_add(1),
|
||||
HealAdmissionResult::Full => self.full = self.full.saturating_add(1),
|
||||
HealAdmissionResult::Dropped(_) => self.dropped = self.dropped.saturating_add(1),
|
||||
}
|
||||
if observation.context == "duplicate" {
|
||||
self.duplicate = self.duplicate.saturating_add(1);
|
||||
}
|
||||
if observation.context == "overlap_rejected" {
|
||||
self.overlap_rejected = self.overlap_rejected.saturating_add(1);
|
||||
}
|
||||
if observation.displaced {
|
||||
self.displaced = self.displaced.saturating_add(1);
|
||||
}
|
||||
if observation.force_start {
|
||||
self.force_start = self.force_start.saturating_add(1);
|
||||
}
|
||||
self.max_start_duration_micros = self
|
||||
.max_start_duration_micros
|
||||
.max(duration_micros_saturated(observation.start_duration));
|
||||
self.max_lock_phase_micros = self
|
||||
.max_lock_phase_micros
|
||||
.max(duration_micros_saturated(observation.lock_phase));
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
struct HealAdmissionObservation {
|
||||
source: HealRequestSource,
|
||||
result: HealAdmissionResult,
|
||||
context: &'static str,
|
||||
force_start: bool,
|
||||
displaced: bool,
|
||||
start_duration: Duration,
|
||||
lock_phase: Duration,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
|
||||
#[serde(rename_all = "camelCase", deny_unknown_fields)]
|
||||
pub struct HealOperationsSnapshot {
|
||||
@@ -396,12 +458,18 @@ pub struct HealOperationsSnapshot {
|
||||
pub queued_by_source: HealSourceCounts,
|
||||
pub active_by_source: HealSourceCounts,
|
||||
pub retrying_by_source: HealSourceCounts,
|
||||
#[serde(default)]
|
||||
pub admission: HealAdmissionTelemetry,
|
||||
}
|
||||
|
||||
fn usize_to_u64_saturated(value: usize) -> u64 {
|
||||
u64::try_from(value).unwrap_or(u64::MAX)
|
||||
}
|
||||
|
||||
fn duration_micros_saturated(duration: Duration) -> u64 {
|
||||
u64::try_from(duration.as_micros()).unwrap_or(u64::MAX)
|
||||
}
|
||||
|
||||
fn heal_type_matches_path(heal_type: &HealType, heal_path: &str) -> bool {
|
||||
let heal_path = heal_path.trim_matches('/');
|
||||
if heal_path.is_empty() || heal_path == LEGACY_ROOT_HEAL_PATH {
|
||||
@@ -764,6 +832,9 @@ pub struct HealManager {
|
||||
notify: Arc<Notify>,
|
||||
/// Optional runtime workload snapshot provider used to protect foreground data-plane work.
|
||||
workload_provider: Option<WorkloadSnapshotProviderRef>,
|
||||
/// Bounded, low-cardinality admission telemetry exposed through the
|
||||
/// existing operations snapshot for cluster E2E assertions.
|
||||
admission_telemetry: Arc<StdMutex<HealAdmissionTelemetry>>,
|
||||
}
|
||||
|
||||
/// Where a task-id lookup resolved. The variants carry the resolved state
|
||||
@@ -919,6 +990,33 @@ impl HealManager {
|
||||
.increment(1);
|
||||
}
|
||||
|
||||
fn record_admission_observation(&self, observation: HealAdmissionObservation) {
|
||||
let result = observation.result.result_label().to_string();
|
||||
let reason = observation.result.reason_label().to_string();
|
||||
let source = observation.source.as_str().to_string();
|
||||
let context = observation.context.to_string();
|
||||
let force_start = observation.force_start.to_string();
|
||||
histogram!(
|
||||
"rustfs_heal_admission_start_duration_seconds",
|
||||
"source" => source.clone(),
|
||||
"result" => result.clone(),
|
||||
"reason" => reason.clone(),
|
||||
"context" => context.clone(),
|
||||
"force_start" => force_start.clone()
|
||||
)
|
||||
.record(observation.start_duration.as_secs_f64());
|
||||
histogram!(
|
||||
"rustfs_heal_admission_lock_phase_seconds",
|
||||
"source" => source,
|
||||
"result" => result,
|
||||
"reason" => reason,
|
||||
"context" => context,
|
||||
"force_start" => force_start
|
||||
)
|
||||
.record(observation.lock_phase.as_secs_f64());
|
||||
lock_admission_telemetry(&self.admission_telemetry).record(observation);
|
||||
}
|
||||
|
||||
fn remove_mrf_repair_notice_targets_for_task(&self, task_id: &str) {
|
||||
let targets = lock_mrf_repair_notice_targets(&self.mrf_repair_notice_targets).remove(task_id);
|
||||
if let Some(targets) = targets {
|
||||
@@ -1265,6 +1363,7 @@ impl HealManager {
|
||||
statistics: Arc::new(RwLock::new(HealStatistics::new())),
|
||||
notify: Arc::new(Notify::new()),
|
||||
workload_provider,
|
||||
admission_telemetry: Arc::new(StdMutex::new(HealAdmissionTelemetry::default())),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1455,6 +1554,9 @@ impl HealManager {
|
||||
preserve_alias: bool,
|
||||
mrf_notice_target: Option<MrfRepairNoticeTarget>,
|
||||
) -> Result<HealAdmissionReceipt> {
|
||||
let admission_start = Instant::now();
|
||||
let source = request.source;
|
||||
let force_start = request.force_start;
|
||||
// HS-06 forceStart semantics (admin only): MinIO stops the old task
|
||||
// first and then starts the new one. Cancel any active admin task
|
||||
// overlapping this request's path before entering admission, so the
|
||||
@@ -1505,6 +1607,7 @@ impl HealManager {
|
||||
// Match the scheduler's active -> queue order and keep retry ownership
|
||||
// in the same atomic view. Otherwise queue -> active and
|
||||
// active -> retrying transitions can slip between duplicate checks.
|
||||
let lock_phase_start = Instant::now();
|
||||
let active_heals = self.active_heals.lock().await;
|
||||
#[cfg(test)]
|
||||
pause_duplicate_admission_after_active_lock(&request.id).await;
|
||||
@@ -1539,7 +1642,17 @@ impl HealManager {
|
||||
drop(retrying_heals);
|
||||
drop(queue);
|
||||
drop(active_heals);
|
||||
let lock_phase = lock_phase_start.elapsed();
|
||||
Self::record_admission_metric(request.source, admission, "duplicate");
|
||||
self.record_admission_observation(HealAdmissionObservation {
|
||||
source,
|
||||
result: admission,
|
||||
context: "duplicate",
|
||||
force_start,
|
||||
displaced: false,
|
||||
start_duration: admission_start.elapsed(),
|
||||
lock_phase,
|
||||
});
|
||||
|
||||
match admission {
|
||||
HealAdmissionResult::Merged => {
|
||||
@@ -1618,7 +1731,17 @@ impl HealManager {
|
||||
drop(retrying_heals);
|
||||
drop(queue);
|
||||
drop(active_heals);
|
||||
let lock_phase = lock_phase_start.elapsed();
|
||||
Self::record_admission_metric(request.source, HealAdmissionResult::Dropped(reason), "overlap_rejected");
|
||||
self.record_admission_observation(HealAdmissionObservation {
|
||||
source,
|
||||
result: HealAdmissionResult::Dropped(reason),
|
||||
context: "overlap_rejected",
|
||||
force_start,
|
||||
displaced: false,
|
||||
start_duration: admission_start.elapsed(),
|
||||
lock_phase,
|
||||
});
|
||||
warn!(
|
||||
target: "rustfs::heal::manager",
|
||||
event = EVENT_HEAL_QUEUE_ADMISSION,
|
||||
@@ -1663,6 +1786,8 @@ impl HealManager {
|
||||
drop(retrying_heals);
|
||||
drop(queue);
|
||||
drop(active_heals);
|
||||
let lock_phase = lock_phase_start.elapsed();
|
||||
let displaced = displaced_terminal.is_some();
|
||||
|
||||
if let (Some(displaced_task_id), Some(displaced_terminal)) = (displaced_task_id, displaced_terminal) {
|
||||
// The queue has already removed the displaced request, so the
|
||||
@@ -1676,6 +1801,16 @@ impl HealManager {
|
||||
self.notify.notify_one();
|
||||
}
|
||||
|
||||
self.record_admission_observation(HealAdmissionObservation {
|
||||
source,
|
||||
result: admission,
|
||||
context: "submit",
|
||||
force_start,
|
||||
displaced,
|
||||
start_duration: admission_start.elapsed(),
|
||||
lock_phase,
|
||||
});
|
||||
|
||||
Ok(HealAdmissionReceipt {
|
||||
result: admission,
|
||||
task_id,
|
||||
@@ -2111,17 +2246,25 @@ impl HealManager {
|
||||
}
|
||||
publish_active_heal_count(&active_heals);
|
||||
publish_heal_queue_length(&queue);
|
||||
let queue_length = usize_to_u64_saturated(queue.len());
|
||||
let active_tasks = usize_to_u64_saturated(active_heals.len());
|
||||
let retrying_tasks = usize_to_u64_saturated(retrying_heals.len());
|
||||
drop(retrying_heals);
|
||||
drop(queue);
|
||||
drop(active_heals);
|
||||
let admission = *lock_admission_telemetry(&self.admission_telemetry);
|
||||
|
||||
HealOperationsSnapshot {
|
||||
queue_length: usize_to_u64_saturated(queue.len()),
|
||||
active_tasks: usize_to_u64_saturated(active_heals.len()),
|
||||
retrying_tasks: usize_to_u64_saturated(retrying_heals.len()),
|
||||
queue_length,
|
||||
active_tasks,
|
||||
retrying_tasks,
|
||||
queued_by_priority,
|
||||
active_by_priority,
|
||||
retrying_by_priority,
|
||||
queued_by_source,
|
||||
active_by_source,
|
||||
retrying_by_source,
|
||||
admission,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2792,6 +2792,88 @@ async fn admin_force_start_cancels_overlapping_active_task_first() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn admission_snapshot_tracks_start_duplicate_force_start_and_displacement() {
|
||||
let storage: Arc<dyn HealStorageAPI> = Arc::new(MockStorage);
|
||||
let manager = Arc::new(HealManager::new(
|
||||
storage,
|
||||
Some(HealConfig {
|
||||
queue_size: 1,
|
||||
..Default::default()
|
||||
}),
|
||||
));
|
||||
|
||||
let mut paused = admin_prefix_request("bucket-a", "logs/");
|
||||
paused.priority = HealPriority::Low;
|
||||
let hook = Arc::new(DuplicateAdmissionTestHook {
|
||||
request_id: paused.id.clone(),
|
||||
active_lock_reached: Notify::new(),
|
||||
active_lock_release: Notify::new(),
|
||||
});
|
||||
*DUPLICATE_ADMISSION_TEST_HOOK.lock().await = Some(hook.clone());
|
||||
|
||||
let submit_manager = Arc::clone(&manager);
|
||||
let mut paused_submission = tokio::spawn(async move { submit_manager.submit_heal_request(paused).await });
|
||||
tokio::time::timeout(Duration::from_secs(1), hook.active_lock_reached.notified())
|
||||
.await
|
||||
.expect("admission should reach the test-only lock phase hook");
|
||||
assert!(
|
||||
tokio::time::timeout(Duration::from_millis(10), &mut paused_submission)
|
||||
.await
|
||||
.is_err(),
|
||||
"admission must wait while the lock-phase hook is held"
|
||||
);
|
||||
hook.active_lock_release.notify_one();
|
||||
assert_eq!(
|
||||
paused_submission
|
||||
.await
|
||||
.expect("paused admission task should join")
|
||||
.expect("paused admission should succeed"),
|
||||
HealAdmissionResult::Accepted
|
||||
);
|
||||
*DUPLICATE_ADMISSION_TEST_HOOK.lock().await = None;
|
||||
|
||||
let duplicate = admin_prefix_request("bucket-a", "logs/");
|
||||
let duplicate_receipt = manager
|
||||
.submit_heal_request_with_receipt(duplicate)
|
||||
.await
|
||||
.expect("duplicate admission should return a canonical receipt");
|
||||
assert_eq!(duplicate_receipt.result, HealAdmissionResult::Merged);
|
||||
|
||||
let mut high = admin_prefix_request("bucket-b", "logs/");
|
||||
high.priority = HealPriority::High;
|
||||
assert_eq!(
|
||||
manager
|
||||
.submit_heal_request(high)
|
||||
.await
|
||||
.expect("higher priority admin request should displace queued low-priority work"),
|
||||
HealAdmissionResult::Accepted
|
||||
);
|
||||
|
||||
let mut forced = admin_prefix_request("bucket-c", "logs/");
|
||||
forced.force_start = true;
|
||||
assert_eq!(
|
||||
manager
|
||||
.submit_heal_request(forced)
|
||||
.await
|
||||
.expect("forceStart should keep explicit admission semantics"),
|
||||
HealAdmissionResult::Accepted
|
||||
);
|
||||
|
||||
let admission = manager.operations_snapshot().await.admission;
|
||||
assert_eq!(admission.accepted, 3);
|
||||
assert_eq!(admission.merged, 1);
|
||||
assert_eq!(admission.full, 0);
|
||||
assert_eq!(admission.dropped, 0);
|
||||
assert_eq!(admission.duplicate, 1);
|
||||
assert_eq!(admission.displaced, 1);
|
||||
assert_eq!(admission.force_start, 1);
|
||||
assert!(
|
||||
admission.max_lock_phase_micros > 0,
|
||||
"snapshot should expose a measurable queue/admission lock phase for p95-style external aggregation"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_operations_snapshot_counts_active_by_source_and_priority() {
|
||||
let storage: Arc<dyn HealStorageAPI> = Arc::new(MockStorage);
|
||||
|
||||
@@ -34,7 +34,7 @@ use storage_api::owner::{
|
||||
};
|
||||
|
||||
pub use erasure_healer::ErasureSetHealer;
|
||||
pub use manager::{HealManager, HealOperationsSnapshot, HealPriorityCounts, HealSourceCounts};
|
||||
pub use manager::{HealAdmissionTelemetry, HealManager, HealOperationsSnapshot, HealPriorityCounts, HealSourceCounts};
|
||||
pub use resume::{CheckpointManager, ResumeCheckpoint, ResumeManager, ResumeState, ResumeUtils};
|
||||
pub use task::{HealOptions, HealPriority, HealRequest, HealTask, HealType};
|
||||
|
||||
|
||||
@@ -516,6 +516,7 @@ async fn submit_mrf_heal_request(manager: &HealManager, intent: &MrfIntent) -> c
|
||||
|
||||
struct MrfRuntime {
|
||||
queue: MrfQueue,
|
||||
retained_replay_intents: Vec<MrfIntent>,
|
||||
config: MrfConsumerConfig,
|
||||
new_since_flush: usize,
|
||||
/// True while the in-memory pending set has changed since the last
|
||||
@@ -535,7 +536,7 @@ impl MrfRuntime {
|
||||
fn snapshot(&self) -> (Vec<u8>, Vec<u8>) {
|
||||
let mut authoritative = Vec::new();
|
||||
let mut legacy = Vec::new();
|
||||
for intent in self.queue.intents() {
|
||||
for intent in self.retained_replay_intents.iter().chain(self.queue.intents()) {
|
||||
let scoped_identity =
|
||||
!matches!(intent.kind, rustfs_common::mrf_channel::MrfKind::MetadataCorruption) && intent.scope.is_some();
|
||||
if !encode_intent(intent, &mut authoritative) {
|
||||
@@ -673,10 +674,11 @@ pub async fn replay_journal_once(manager: &Arc<HealManager>) -> usize {
|
||||
struct ReplayOutcome {
|
||||
replayed: usize,
|
||||
journal_on_disk: bool,
|
||||
retained_replay_intents: Vec<MrfIntent>,
|
||||
}
|
||||
|
||||
fn replay_must_retain_journal(rearm_incomplete: bool, pending_depth: usize) -> bool {
|
||||
rearm_incomplete || pending_depth > 0
|
||||
fn replay_must_retain_journal(rearm_incomplete: bool, pending_depth: usize, retained_replay_depth: usize) -> bool {
|
||||
rearm_incomplete || pending_depth > 0 || retained_replay_depth > 0
|
||||
}
|
||||
|
||||
/// Shared replay core: read + decode + re-arm, then drain what fits. The
|
||||
@@ -698,6 +700,7 @@ async fn replay_into(
|
||||
return ReplayOutcome {
|
||||
replayed: 0,
|
||||
journal_on_disk: false,
|
||||
retained_replay_intents: Vec::new(),
|
||||
};
|
||||
}
|
||||
},
|
||||
@@ -737,10 +740,13 @@ async fn replay_into(
|
||||
|
||||
// Drain the replayed intents immediately; whatever the manager refuses
|
||||
// stays armed in `queue` for the consumer's retry loop.
|
||||
let mut retained_replay_intents = Vec::new();
|
||||
if backoff_until.is_none() {
|
||||
while let Some(mut intent) = queue.pop_front() {
|
||||
match submit_mrf_heal_request(manager, &intent).await {
|
||||
Ok(HealAdmissionResult::Accepted) | Ok(HealAdmissionResult::Merged) => {}
|
||||
Ok(HealAdmissionResult::Accepted) | Ok(HealAdmissionResult::Merged) => {
|
||||
retained_replay_intents.push(intent);
|
||||
}
|
||||
Ok(HealAdmissionResult::Full) | Ok(HealAdmissionResult::Dropped(HealAdmissionDropReason::QueueFull)) => {
|
||||
intent.attempts = intent.attempts.saturating_add(1);
|
||||
if intent.attempts < MRF_MAX_ATTEMPTS {
|
||||
@@ -769,7 +775,7 @@ async fn replay_into(
|
||||
}
|
||||
}
|
||||
}
|
||||
let journal_on_disk = if replay_must_retain_journal(rearm_incomplete, queue.depth()) {
|
||||
let journal_on_disk = if replay_must_retain_journal(rearm_incomplete, queue.depth(), retained_replay_intents.len()) {
|
||||
true
|
||||
} else {
|
||||
!delete_journals().await
|
||||
@@ -777,6 +783,7 @@ async fn replay_into(
|
||||
ReplayOutcome {
|
||||
replayed,
|
||||
journal_on_disk,
|
||||
retained_replay_intents,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -786,6 +793,7 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
|
||||
let config = MrfConsumerConfig::default();
|
||||
let mut runtime = MrfRuntime {
|
||||
queue: MrfQueue::new(config.queue_capacity, config.journal_max_bytes),
|
||||
retained_replay_intents: Vec::new(),
|
||||
config: config.clone(),
|
||||
new_since_flush: 0,
|
||||
dirty: false,
|
||||
@@ -797,6 +805,7 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
|
||||
// on disk whenever any replayed intent still needs a successor snapshot.
|
||||
let replay = replay_into(&manager, &mut runtime.queue, &mut runtime.backoff_until).await;
|
||||
runtime.journal_on_disk = replay.journal_on_disk;
|
||||
runtime.retained_replay_intents = replay.retained_replay_intents;
|
||||
// Anything still pending (e.g. the manager was full and backoff armed)
|
||||
// must be re-persisted by the next flush before replay can delete the
|
||||
// startup anchor.
|
||||
@@ -814,7 +823,7 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
|
||||
// provably current AND idle (a dirty or pending state
|
||||
// gets one last persist attempt, matching the shutdown
|
||||
// retry the unconditional flush used to provide).
|
||||
if runtime.dirty || runtime.queue.depth() > 0 {
|
||||
if runtime.dirty || runtime.queue.depth() > 0 || !runtime.retained_replay_intents.is_empty() {
|
||||
runtime.flush().await;
|
||||
}
|
||||
tracing::info!(
|
||||
@@ -843,6 +852,7 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
|
||||
match tick_action(
|
||||
runtime.dirty,
|
||||
runtime.queue.depth(),
|
||||
runtime.retained_replay_intents.len(),
|
||||
runtime.journal_on_disk,
|
||||
) {
|
||||
TickAction::Flush => {
|
||||
@@ -857,8 +867,8 @@ async fn run_mrf_consumer(manager: Arc<HealManager>, mut receiver: mpsc::Receive
|
||||
runtime.dispatch(manager.as_ref()).await;
|
||||
}
|
||||
TickAction::DeleteJournal => {
|
||||
// All replayed intents have either been accepted,
|
||||
// merged, or replaced by a pending successor snapshot.
|
||||
// Only remove a stale journal after every replayed
|
||||
// intent has a durable successor proof.
|
||||
if delete_journals().await {
|
||||
runtime.journal_on_disk = false;
|
||||
gauge!("rustfs_heal_mrf_journal_bytes").set(0.0);
|
||||
@@ -887,11 +897,13 @@ enum TickAction {
|
||||
Idle,
|
||||
}
|
||||
|
||||
fn tick_action(dirty: bool, depth: usize, journal_on_disk: bool) -> TickAction {
|
||||
fn tick_action(dirty: bool, depth: usize, retained_replay_depth: usize, journal_on_disk: bool) -> TickAction {
|
||||
if dirty {
|
||||
TickAction::Flush
|
||||
} else if depth > 0 {
|
||||
TickAction::Retry
|
||||
} else if retained_replay_depth > 0 {
|
||||
TickAction::Idle
|
||||
} else if journal_on_disk {
|
||||
TickAction::DeleteJournal
|
||||
} else {
|
||||
@@ -924,34 +936,73 @@ mod tests {
|
||||
|
||||
// Dirty dominates: a changed pending set flushes even when idle
|
||||
// otherwise.
|
||||
assert!(matches!(tick_action(true, 0, false), Flush));
|
||||
assert!(matches!(tick_action(true, 3, true), Flush));
|
||||
assert!(matches!(tick_action(true, 0, 0, false), Flush));
|
||||
assert!(matches!(tick_action(true, 3, 0, true), Flush));
|
||||
|
||||
// Clean backlog: no rewrite, but keep draining so an expired
|
||||
// admission backoff retries on time.
|
||||
assert!(matches!(tick_action(false, 1, false), Retry));
|
||||
assert!(matches!(tick_action(false, 2, true), Retry));
|
||||
assert!(matches!(tick_action(false, 1, 0, false), Retry));
|
||||
assert!(matches!(tick_action(false, 2, 0, true), Retry));
|
||||
|
||||
// Replayed records accepted by the manager are still restart anchors
|
||||
// until a durable successor proof can tombstone them.
|
||||
assert!(matches!(tick_action(false, 0, 1, true), Idle));
|
||||
|
||||
// Quiescent with a stale journal file on disk: remove it.
|
||||
assert!(matches!(tick_action(false, 0, true), DeleteJournal));
|
||||
assert!(matches!(tick_action(false, 0, 0, true), DeleteJournal));
|
||||
|
||||
// Fully quiescent: nothing to do.
|
||||
assert!(matches!(tick_action(false, 0, false), Idle));
|
||||
assert!(matches!(tick_action(false, 0, 0, false), Idle));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replay_cleanup_retains_journal_for_unarmed_or_refused_records() {
|
||||
assert!(
|
||||
replay_must_retain_journal(true, 0),
|
||||
replay_must_retain_journal(true, 0, 0),
|
||||
"a rejected replay record still needs its disk anchor"
|
||||
);
|
||||
assert!(
|
||||
replay_must_retain_journal(false, 1),
|
||||
replay_must_retain_journal(false, 1, 0),
|
||||
"a Full admission retry must keep the startup journal until the next snapshot"
|
||||
);
|
||||
assert!(
|
||||
!replay_must_retain_journal(false, 0),
|
||||
"only a fully consumed replay snapshot may be deleted"
|
||||
replay_must_retain_journal(false, 0, 1),
|
||||
"an accepted replay record still needs a durable successor before cleanup"
|
||||
);
|
||||
assert!(
|
||||
!replay_must_retain_journal(false, 0, 0),
|
||||
"only a fully consumed replay snapshot with no retained anchors may be deleted"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retained_replay_anchor_remains_in_successor_snapshot() {
|
||||
let retained = intent("accepted-replay", "object", 0);
|
||||
let mut runtime = MrfRuntime {
|
||||
queue: MrfQueue::new(8, 8192),
|
||||
retained_replay_intents: vec![retained.clone()],
|
||||
config: MrfConsumerConfig::default(),
|
||||
new_since_flush: 0,
|
||||
dirty: false,
|
||||
journal_on_disk: true,
|
||||
backoff_until: None,
|
||||
};
|
||||
assert_eq!(
|
||||
runtime.queue.try_push_typed(intent("new-pending", "object", 0)),
|
||||
MrfQueuePushResult::Enqueued
|
||||
);
|
||||
|
||||
let (authoritative, legacy) = runtime.snapshot();
|
||||
let (decoded, truncated) = decode_journal(&authoritative);
|
||||
let (legacy_decoded, legacy_truncated) = decode_journal(&legacy);
|
||||
|
||||
assert_eq!(truncated, 0);
|
||||
assert_eq!(legacy_truncated, 0);
|
||||
assert_eq!(decoded.len(), 2);
|
||||
assert_eq!(legacy_decoded.len(), 2);
|
||||
assert!(
|
||||
decoded.iter().any(|intent| intent.bucket == retained.bucket),
|
||||
"accepted replay anchor must remain crash-replayable"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -19,7 +19,8 @@ pub mod heal;
|
||||
|
||||
pub use error::{Error, Result};
|
||||
pub use heal::{
|
||||
HealManager, HealOperationsSnapshot, HealOptions, HealPriority, HealPriorityCounts, HealRequest, HealSourceCounts, HealType,
|
||||
HealAdmissionTelemetry, HealManager, HealOperationsSnapshot, HealOptions, HealPriority, HealPriorityCounts, HealRequest,
|
||||
HealSourceCounts, HealType,
|
||||
channel::HealChannelProcessor,
|
||||
progress::{HealProgress, aggregate_heal_progress},
|
||||
resume::{ReplacementRecoveryRecord, ReplacementRecoveryState, ResumeUtils},
|
||||
|
||||
@@ -51,17 +51,14 @@ prior key versions would go green.
|
||||
|
||||
The lane creates real keys under unique names (`behavior-kv2-*`,
|
||||
`behavior-transit-*`) and does not remove them, so a dev Vault accumulates them
|
||||
across runs. On a dev server, remove only exact keys confirmed to belong to the
|
||||
current task. A shared prefix does not prove ownership; preserve another run's
|
||||
keys and leave ambiguous keys for the operator.
|
||||
across runs. Clear them out periodically — against a dev server only:
|
||||
|
||||
```bash
|
||||
vault write transit/keys/<task-owned-transit-key>/config deletion_allowed=true
|
||||
vault delete transit/keys/<task-owned-transit-key>
|
||||
vault list -format=json transit/keys | jq -r '.[] | select(startswith("behavior-transit-"))' | while read -r k; do vault write "transit/keys/$k/config" deletion_allowed=true >/dev/null && vault delete "transit/keys/$k"; done
|
||||
```
|
||||
|
||||
```bash
|
||||
vault kv metadata delete secret/rustfs/kms/keys/<task-owned-kv2-key>
|
||||
vault list -format=json secret/metadata/rustfs/kms/keys | jq -r '.[] | select(startswith("behavior-kv2-"))' | xargs -I{} vault kv metadata delete secret/rustfs/kms/keys/{}
|
||||
```
|
||||
|
||||
## Local Key Export for SSE-S3 Migration Tests
|
||||
|
||||
@@ -15,14 +15,14 @@ Keep changes narrow and source-driven.
|
||||
|
||||
## Source of Truth
|
||||
|
||||
Start with the file that owns the changed behavior; follow callers and shared
|
||||
types only as needed:
|
||||
Before changing behavior, read these files first:
|
||||
|
||||
- Scan/sampling: `src/scan.rs`.
|
||||
- Refresh and per-disk cache state: `src/capacity_manager.rs`.
|
||||
- Scope propagation: `src/capacity_scope.rs`; shared data shapes: `src/types.rs`.
|
||||
- Admin integration: `../../rustfs/src/capacity/service.rs`.
|
||||
- Configuration/defaults: `../config/src/constants/capacity.rs`.
|
||||
- `src/scan.rs`
|
||||
- `src/capacity_manager.rs`
|
||||
- `src/capacity_scope.rs`
|
||||
- `src/types.rs`
|
||||
- `../../rustfs/src/capacity/service.rs`
|
||||
- `../config/src/constants/capacity.rs`
|
||||
|
||||
Do not treat shell scripts or old docs as the authoritative behavior definition when the Rust code says otherwise.
|
||||
|
||||
|
||||
@@ -60,6 +60,6 @@ Required headings and strings in these files are asserted by `scripts/check_arch
|
||||
| [minio-rustfs-router-compatibility.md](minio-rustfs-router-compatibility.md) | a client or `mc` call that works against MinIO fails against RustFS and you need to know whether the endpoint is missing, stubbed, or deliberately different |
|
||||
| [minio-file-format-compat.md](minio-file-format-compat.md) | deciding whether a MinIO drive set, bucket-metadata blob, or SSE object can be read or imported by a given RustFS build, or before touching a listed version anchor |
|
||||
|
||||
Operations runbooks live in [../operations/](../operations/) and testing references in [../testing/README.md](../testing/README.md).
|
||||
Operations runbooks live in [../operations/](../README.md#operations) and testing references in [../testing/README.md](../testing/README.md).
|
||||
|
||||
For per-node HTTP failure ratios and cached storage probe provenance, see [S3 write failure diagnostics](../operations/s3-write-failure-diagnostics.md).
|
||||
|
||||
@@ -39,7 +39,7 @@ The wire prefix is `/rustfs/admin/v3`. `GET /kms/status` and `GET /kms/service-s
|
||||
| `POST /kms/restore/dry-run` | `kms:Restore` | sensitive | no | Preflight; writes nothing |
|
||||
| `POST /kms/restore` | `kms:Restore` | high | no | Requires `confirm_backup_id` and `confirm_conflict_policy` |
|
||||
| `POST /kms/restore/abort` | `kms:Restore` | high | no | Requires `confirm_target_key_dir` |
|
||||
| `POST /kms/create-key`, `POST /kms/key/create` | `kms:Configure` | high | no | Legacy `mc` aliases of `POST /kms/keys`; the key name comes from the `key-id` query parameter (`mc`'s form) or the `name` tag, and a request carrying both with different values is refused with `400` |
|
||||
| `POST /kms/create-key`, `POST /kms/key/create` | `kms:Configure` | high | no | Legacy `mc` aliases of `POST /kms/keys` |
|
||||
| `GET /kms/describe-key`, `GET /kms/key/status` | `kms:DescribeKey` | sensitive | yes | Legacy aliases of `GET /kms/keys/{key_id}` |
|
||||
| `GET /kms/list-keys` | `kms:ListKeys` | sensitive | no | Legacy alias of `GET /kms/keys`; same listing contract |
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
| Outbound target matrix | Replication of every object shape (empty, plain, retention, legal hold, multipart) against every remote-target failure mode the fake target models; an explicit expectation table pins known-red cells to an open issue | `cargo nextest run -p e2e_test -E 'test(/^replication_target_matrix_test::/)'` (build `target/debug/rustfs` first) | With `e2e-repl-nightly`; required locally for any change to outbound client defaults (SOP: [`docs/postmortems/2026-09-03-replication-checksum-default-regression.md`](../postmortems/2026-09-03-replication-checksum-default-regression.md)) |
|
||||
# RustFS Testing
|
||||
|
||||
**Use this when:** you need to pick a test layer for a change, name a test so a gate keeps selecting it, understand why `#[serial]` does nothing under nextest, or handle a flaky test.
|
||||
@@ -12,7 +13,6 @@ Pick the lowest layer that can prove the change; add a higher-layer test only wh
|
||||
| Unit & crate integration | Per-crate logic and in-process integration tests | `cargo nextest run --all --exclude e2e_test` (or `-p <crate>`); `make test` wraps it | Every PR, required (`Test and Lint`, `ci` profile) |
|
||||
| ecstore black-box | Erasure-coded read/write/recovery validation; profiles `quick` / `full` / `destructive` / `fuzz` | `scripts/run_ecstore_validation_suite.sh --profile quick` | Local and release validation only; not wired into any workflow. Contract: [ecstore-validation-suite-design.md](ecstore-validation-suite-design.md) |
|
||||
| e2e (`e2e_test` crate) | A real `rustfs` binary per test, driven over the S3, admin, and protocol APIs | `cargo nextest run --profile e2e-smoke -p e2e_test` | PR: `e2e-smoke` (report-only); merge queue / main push: `e2e-full`; nightly: `e2e-repl-nightly`, `e2e-nightly`, `e2e-protocols`, `e2e-distributed`. Guide: [`crates/e2e_test/README.md`](../../crates/e2e_test/README.md); 4-node 4-disk map: [distributed-e2e.md](distributed-e2e.md) |
|
||||
| Outbound target matrix | Replication of every object shape (empty, plain, retention, legal hold, multipart) against every remote-target failure mode the fake target models; an explicit expectation table pins known-red cells to an open issue | `cargo nextest run -p e2e_test -E 'test(/^replication_target_matrix_test::/)'` (build `target/debug/rustfs` first) | With `e2e-repl-nightly`; required locally for any change to outbound client defaults (SOP: [`docs/postmortems/2026-09-03-replication-checksum-default-regression.md`](../postmortems/2026-09-03-replication-checksum-default-regression.md)) |
|
||||
| s3s-e2e conformance | External S3 conformance tool against a live server | `./scripts/e2e-run.sh ./target/debug/rustfs <data-dir>` | PR, report-only (second half of the `End-to-End Tests` job) |
|
||||
| S3 compatibility | `ceph/s3-tests` (boto3; allow-list `scripts/s3-tests/implemented_tests.txt`) and MinIO `mint` | `scripts/s3-tests/run.sh`; mint via `.github/workflows/mint.yml` | s3-tests: PR report-only plus a weekly full sweep; mint: weekly, report-only |
|
||||
| Chaos / fault-injection | Single-node disk fault injection (`crates/e2e_test/src/chaos.rs`, `crates/e2e_test/src/fault_proxy.rs`) plus the 4-node kill/fresh-drive/blackhole cases in `crates/e2e_test/src/distributed/chaos_test.rs` | Part of the e2e crate (`e2e-reliability` and `e2e-distributed`) | Reliability cases with `e2e-full`; 4-node chaos on storage-sensitive PRs and nightly via `e2e-distributed` |
|
||||
|
||||
@@ -339,6 +339,19 @@ fn add_source_counts(total: &mut rustfs_heal::HealSourceCounts, next: rustfs_hea
|
||||
total.mrf = total.mrf.saturating_add(next.mrf);
|
||||
}
|
||||
|
||||
fn add_admission_telemetry(total: &mut rustfs_heal::HealAdmissionTelemetry, next: rustfs_heal::HealAdmissionTelemetry) {
|
||||
total.accepted = total.accepted.saturating_add(next.accepted);
|
||||
total.merged = total.merged.saturating_add(next.merged);
|
||||
total.full = total.full.saturating_add(next.full);
|
||||
total.dropped = total.dropped.saturating_add(next.dropped);
|
||||
total.duplicate = total.duplicate.saturating_add(next.duplicate);
|
||||
total.overlap_rejected = total.overlap_rejected.saturating_add(next.overlap_rejected);
|
||||
total.displaced = total.displaced.saturating_add(next.displaced);
|
||||
total.force_start = total.force_start.saturating_add(next.force_start);
|
||||
total.max_start_duration_micros = total.max_start_duration_micros.max(next.max_start_duration_micros);
|
||||
total.max_lock_phase_micros = total.max_lock_phase_micros.max(next.max_lock_phase_micros);
|
||||
}
|
||||
|
||||
fn add_operations(total: &mut rustfs_heal::HealOperationsSnapshot, next: rustfs_heal::HealOperationsSnapshot) {
|
||||
total.queue_length = total.queue_length.saturating_add(next.queue_length);
|
||||
total.active_tasks = total.active_tasks.saturating_add(next.active_tasks);
|
||||
@@ -349,6 +362,7 @@ fn add_operations(total: &mut rustfs_heal::HealOperationsSnapshot, next: rustfs_
|
||||
add_source_counts(&mut total.queued_by_source, next.queued_by_source);
|
||||
add_source_counts(&mut total.active_by_source, next.active_by_source);
|
||||
add_source_counts(&mut total.retrying_by_source, next.retrying_by_source);
|
||||
add_admission_telemetry(&mut total.admission, next.admission);
|
||||
}
|
||||
|
||||
fn aggregate_cluster_heal_status(snapshots: Vec<NodeHealStatusSnapshot>) -> ClusterHealStatusSnapshot {
|
||||
@@ -2307,6 +2321,10 @@ mod tests {
|
||||
assert!(json["healOperations"]["queuedBySource"]["admin"].is_u64());
|
||||
assert!(json["healOperations"]["queuedByPriority"]["low"].is_u64());
|
||||
assert!(json["healOperations"]["queuedByPriority"]["high"].is_u64());
|
||||
assert!(json["healOperations"]["admission"]["accepted"].is_u64());
|
||||
assert!(json["healOperations"]["admission"]["duplicate"].is_u64());
|
||||
assert!(json["healOperations"]["admission"]["forceStart"].is_u64());
|
||||
assert!(json["healOperations"]["admission"]["maxLockPhaseMicros"].is_u64());
|
||||
assert_eq!(json["state"], "active");
|
||||
assert_eq!(json["clusterStatusComplete"], true);
|
||||
assert!(json["progress"].is_null());
|
||||
@@ -2486,6 +2504,18 @@ mod tests {
|
||||
queued_by_source: sources(value),
|
||||
active_by_source: sources(value),
|
||||
retrying_by_source: sources(value),
|
||||
admission: rustfs_heal::HealAdmissionTelemetry {
|
||||
accepted: value,
|
||||
merged: value,
|
||||
full: value,
|
||||
dropped: value,
|
||||
duplicate: value,
|
||||
overlap_rejected: value,
|
||||
displaced: value,
|
||||
force_start: value,
|
||||
max_start_duration_micros: value,
|
||||
max_lock_phase_micros: value,
|
||||
},
|
||||
};
|
||||
let progress = |value| NodeHealProgress {
|
||||
objects_scanned: value,
|
||||
|
||||
@@ -18,7 +18,6 @@ use super::kms_audit::{KmsAdminAudit, KmsAdminOperation};
|
||||
use crate::admin::auth::{validate_admin_request, validate_admin_request_with_kms_key};
|
||||
use crate::admin::router::{AdminOperation, Operation, S3Router};
|
||||
use crate::admin::runtime_sources::{current_kms_runtime_service_manager, current_or_init_kms_runtime_service_manager};
|
||||
use crate::admin::storage_api::s3;
|
||||
use crate::admin::utils::extract_query_params;
|
||||
use crate::auth::{check_key_valid, get_session_token};
|
||||
use crate::kms_deletion_gate::current_key_impact;
|
||||
@@ -198,25 +197,6 @@ fn extract_key_id(uri: &hyper::Uri) -> Option<String> {
|
||||
.find_map(|name| query_params.get(name).filter(|value| !value.is_empty()).cloned())
|
||||
}
|
||||
|
||||
/// Name of the key a legacy create request asks for.
|
||||
///
|
||||
/// `mc admin kms key create <name>` sends the name as the `key-id` query
|
||||
/// parameter with no body, while RustFS clients send it as the `name` tag.
|
||||
/// Both are honored. A request carrying both has to agree with itself:
|
||||
/// picking one silently would create a key under a name the caller never
|
||||
/// sees in its own request.
|
||||
fn legacy_create_key_name(uri: &hyper::Uri, tags: &HashMap<String, String>) -> S3Result<Option<String>> {
|
||||
let query_name = extract_key_id(uri);
|
||||
let tag_name = tags.get("name").cloned();
|
||||
match (query_name, tag_name) {
|
||||
(Some(query), Some(tag)) if query != tag => Err(s3::error(
|
||||
s3::S3ErrorCode::InvalidRequest,
|
||||
format!("key name in the query ({query}) and in tags.name ({tag}) differ"),
|
||||
)),
|
||||
(query, tag) => Ok(query.or(tag)),
|
||||
}
|
||||
}
|
||||
|
||||
/// The `key_id` of a KMS admin request body, read without committing to the
|
||||
/// strict schema of the endpoint: the authorization gate needs the target key
|
||||
/// before the body is parsed for execution, and a body that fails the strict
|
||||
@@ -352,8 +332,9 @@ impl Operation for CreateKeyHandler {
|
||||
return Err(s3_error!(InternalError, "kms service is not initialized"));
|
||||
};
|
||||
|
||||
// Extract key name from tags if provided
|
||||
let tags = request.tags.unwrap_or_default();
|
||||
let key_name = legacy_create_key_name(&req.uri, &tags)?;
|
||||
let key_name = tags.get("name").cloned();
|
||||
|
||||
let kms_request = CreateKeyRequest {
|
||||
key_name,
|
||||
@@ -498,8 +479,8 @@ mod tests {
|
||||
DescribeKmsKeyResponse, GenerateDataKeyApiRequest, GenerateDataKeyApiResponse, ListKeysApiResponse, ListKmsKeysResponse,
|
||||
delete_key_error_status, delete_request_from_query, extract_key_id, extract_query_params, key_impact_if_requested,
|
||||
key_list_filters, kms_create_key_actions, kms_delete_key_actions, kms_describe_key_actions,
|
||||
kms_generate_data_key_actions, kms_list_keys_actions, legacy_create_key_name, parse_list_limit, scoped_key_id,
|
||||
stable_json_value, wants_key_impact,
|
||||
kms_generate_data_key_actions, kms_list_keys_actions, parse_list_limit, scoped_key_id, stable_json_value,
|
||||
wants_key_impact,
|
||||
};
|
||||
use http::Uri;
|
||||
use hyper::StatusCode;
|
||||
@@ -520,46 +501,6 @@ mod tests {
|
||||
assert!(!actions.contains(&action), "expected action list not to contain {action:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_create_key_name_honors_the_minio_key_id_query() {
|
||||
let uri: Uri = "/rustfs/admin/v3/kms/key/create?key-id=minio-key"
|
||||
.parse()
|
||||
.expect("uri should parse");
|
||||
|
||||
let name = legacy_create_key_name(&uri, &HashMap::new()).expect("a query-only name is valid");
|
||||
assert_eq!(name.as_deref(), Some("minio-key"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_create_key_name_falls_back_to_the_name_tag() {
|
||||
let uri: Uri = "/rustfs/admin/v3/kms/key/create".parse().expect("uri should parse");
|
||||
let tags = HashMap::from([("name".to_string(), "tagged-key".to_string())]);
|
||||
|
||||
let name = legacy_create_key_name(&uri, &tags).expect("a tag-only name is valid");
|
||||
assert_eq!(name.as_deref(), Some("tagged-key"));
|
||||
assert_eq!(legacy_create_key_name(&uri, &HashMap::new()).expect("no name is valid"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_create_key_name_accepts_agreeing_sources_and_refuses_conflicting_ones() {
|
||||
let uri: Uri = "/rustfs/admin/v3/kms/key/create?key-id=minio-key"
|
||||
.parse()
|
||||
.expect("uri should parse");
|
||||
|
||||
let agreeing = HashMap::from([("name".to_string(), "minio-key".to_string())]);
|
||||
let name = legacy_create_key_name(&uri, &agreeing).expect("agreeing sources are valid");
|
||||
assert_eq!(name.as_deref(), Some("minio-key"));
|
||||
|
||||
let conflicting = HashMap::from([("name".to_string(), "other-key".to_string())]);
|
||||
let refused = legacy_create_key_name(&uri, &conflicting).expect_err("conflicting names must be refused");
|
||||
assert_eq!(*refused.code(), super::s3::S3ErrorCode::InvalidRequest);
|
||||
assert!(
|
||||
refused
|
||||
.message()
|
||||
.is_some_and(|message| message.contains("minio-key") && message.contains("other-key"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_extract_key_id_supports_minio_aliases() {
|
||||
for (uri, expected) in [
|
||||
|
||||
+2
-52
@@ -21,23 +21,6 @@ use s3s::{S3Error, S3ErrorCode};
|
||||
const MAX_VERSIONS_EXCEEDED_CODE: &str = "MaxVersionsExceeded";
|
||||
const MAX_VERSIONS_EXCEEDED_MESSAGE: &str = "You've exceeded the limit on the number of versions you can create on this object";
|
||||
|
||||
/// S3 error code for a request that names a KMS key the KMS does not hold.
|
||||
pub const KMS_KEY_NOT_FOUND_ERROR_CODE: &str = "KMS.NotFoundException";
|
||||
|
||||
/// HTTP status of the error codes s3s cannot derive on its own.
|
||||
///
|
||||
/// s3s answers `None` for every `Custom` code, which the response layer turns
|
||||
/// into a 500; a code that means "your request named something that does not
|
||||
/// exist" has to say so itself.
|
||||
fn custom_error_status(code: &S3ErrorCode) -> Option<StatusCode> {
|
||||
match code {
|
||||
S3ErrorCode::Custom(custom) if &**custom == KMS_KEY_NOT_FOUND_ERROR_CODE || &**custom == MAX_VERSIONS_EXCEEDED_CODE => {
|
||||
Some(StatusCode::BAD_REQUEST)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Marks a request body that exceeded a presigned upload size capability.
|
||||
///
|
||||
/// This marker must survive the body-reader and storage layers so the client
|
||||
@@ -385,10 +368,9 @@ fn error_chain_s3s_body_stream_error(err: &(dyn std::error::Error + 'static)) ->
|
||||
|
||||
impl From<ApiError> for S3Error {
|
||||
fn from(err: ApiError) -> Self {
|
||||
let status = custom_error_status(&err.code);
|
||||
let mut s3e = S3Error::with_message(err.code, err.message);
|
||||
if let Some(status) = status {
|
||||
s3e.set_status_code(status);
|
||||
if matches!(s3e.code(), S3ErrorCode::Custom(code) if &**code == MAX_VERSIONS_EXCEEDED_CODE) {
|
||||
s3e.set_status_code(StatusCode::BAD_REQUEST);
|
||||
}
|
||||
if let Some(source) = err.source {
|
||||
s3e.set_source(source);
|
||||
@@ -460,19 +442,6 @@ impl From<StorageError> for ApiError {
|
||||
source: Some(Box::new(err)),
|
||||
};
|
||||
}
|
||||
|
||||
// A request header or bucket default naming a key the KMS does not
|
||||
// hold is the caller's mistake to correct, and S3 reports it as
|
||||
// 400 `KMS.NotFoundException`. Left to the fallthrough it became a
|
||||
// 500 whose generic message hid which key was missing.
|
||||
if let Some(rustfs_kms::KmsError::KeyNotFound { key_id }) = inner.downcast_ref::<rustfs_kms::KmsError>() {
|
||||
let message = format!("KMS key not found: {key_id}");
|
||||
return ApiError {
|
||||
code: S3ErrorCode::Custom(KMS_KEY_NOT_FOUND_ERROR_CODE.into()),
|
||||
message,
|
||||
source: Some(Box::new(err)),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
let code = match &err {
|
||||
@@ -1011,25 +980,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_kms_key_not_found_maps_to_bad_request_kms_not_found_exception() {
|
||||
let api_error = ApiError::from(StorageError::other(rustfs_kms::KmsError::key_not_found("no-such-key")));
|
||||
|
||||
assert_eq!(api_error.code, S3ErrorCode::Custom(KMS_KEY_NOT_FOUND_ERROR_CODE.into()));
|
||||
assert_eq!(api_error.message, "KMS key not found: no-such-key");
|
||||
|
||||
// s3s knows no status for a custom code; the conversion has to supply it.
|
||||
let s3_error = S3Error::from(api_error);
|
||||
assert_eq!(s3_error.status_code(), Some(StatusCode::BAD_REQUEST));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_generated_error_codes_keep_their_own_status() {
|
||||
let s3_error = S3Error::from(ApiError::from(StorageError::other(rustfs_kms::KmsError::backend_error("down"))));
|
||||
|
||||
assert_eq!(s3_error.status_code(), Some(StatusCode::SERVICE_UNAVAILABLE));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_unknown_authoritative_quota_usage_maps_to_retryable_error() {
|
||||
let api_error = ApiError::from(QuotaError::UsageUnavailable {
|
||||
|
||||
@@ -726,6 +726,7 @@ mod tests {
|
||||
assert_eq!(decoded.info().bitrot_start_cycle, 9);
|
||||
assert_eq!(decoded.operations.queue_length, 2);
|
||||
assert_eq!(decoded.operations.queued_by_source.mrf, 0);
|
||||
assert_eq!(decoded.operations.admission, rustfs_heal::HealAdmissionTelemetry::default());
|
||||
let progress = decoded.progress.expect("legacy progress should decode");
|
||||
assert_eq!(progress.objects_scanned, 7);
|
||||
assert!(!progress.baseline_known);
|
||||
|
||||
@@ -4336,12 +4336,9 @@ mod tests {
|
||||
fn kms_operation_errors_preserve_retryability_classification() {
|
||||
let unavailable = kms_operation_error(rustfs_kms::KmsError::backend_error("connection refused"));
|
||||
let corrupt = kms_operation_error(rustfs_kms::KmsError::cryptographic_error("decrypt", "authentication failed"));
|
||||
let missing = kms_operation_error(rustfs_kms::KmsError::key_not_found("no-such-key"));
|
||||
|
||||
assert_eq!(unavailable.code, S3ErrorCode::ServiceUnavailable);
|
||||
assert_eq!(corrupt.code, S3ErrorCode::InternalError);
|
||||
assert_eq!(missing.code, S3ErrorCode::Custom(crate::error::KMS_KEY_NOT_FOUND_ERROR_CODE.into()));
|
||||
assert_eq!(super::kms_data_plane_error_class(&missing), "key_not_found");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -5563,37 +5560,6 @@ mod tests {
|
||||
reset_sse_dek_provider();
|
||||
}
|
||||
|
||||
/// A write whose resolved key — from the request header or a bucket
|
||||
/// default rule — is unknown to the KMS must come back as the client
|
||||
/// error S3 uses for it, all the way from the backend lookup. Answering
|
||||
/// 500 here made a bucket default pointing at a deleted or mistyped key
|
||||
/// look like a server outage (rustfs/backlog#2330, KMS-312).
|
||||
#[tokio::test]
|
||||
async fn kms_provider_reports_an_unknown_key_as_kms_not_found() {
|
||||
let _guard = lock_sse_test_state().await;
|
||||
reset_sse_dek_provider();
|
||||
|
||||
let manager = configure_test_global_local_kms().await;
|
||||
let provider = KmsSseDekProvider::new_with_service_manager(manager)
|
||||
.await
|
||||
.expect("kms provider should initialize from the configured test manager");
|
||||
|
||||
let context = super::build_object_encryption_context("bucket", "object", None);
|
||||
let error = provider
|
||||
.generate_sse_dek(&context, "no-such-key")
|
||||
.await
|
||||
.expect_err("the Local backend must refuse a key it does not hold");
|
||||
assert_eq!(
|
||||
error.code,
|
||||
S3ErrorCode::Custom(crate::error::KMS_KEY_NOT_FOUND_ERROR_CODE.into()),
|
||||
"got {error:?}"
|
||||
);
|
||||
assert!(error.message.contains("no-such-key"), "the missing key must be named: {error:?}");
|
||||
assert_eq!(super::kms_data_plane_error_class(&error), "key_not_found");
|
||||
|
||||
reset_sse_dek_provider();
|
||||
}
|
||||
|
||||
/// Objects without a rewrappable envelope — plaintext, SSE-C, or a
|
||||
/// MinIO-sealed opaque data key — are reported NotApplicable without any
|
||||
/// provider call.
|
||||
|
||||
Reference in New Issue
Block a user