mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 16:28:15 +00:00
feat: harden site replication control plane (#3842)
This commit is contained in:
cxymds
@@ -0,0 +1,100 @@
|
||||
## Related Issues
|
||||
|
||||
N/A
|
||||
|
||||
## Summary of Changes
|
||||
|
||||
This PR hardens the site replication control plane in small, reviewable commits. The work starts with guardrails identified during the MinIO comparison and multi-review pass: credential redaction, stricter diagnostic authorization, remove cleanup, and clearer status diagnostics.
|
||||
|
||||
### Implementation Log
|
||||
|
||||
1. `docs: plan site replication hardening`
|
||||
- Purpose: establish the execution plan and PR draft before code changes.
|
||||
- Reason: the site replication follow-up touches security, compatibility, operations, and tests, so reviewers need a durable record of each step and why it exists.
|
||||
2. `fix: redact site replication target secrets`
|
||||
- Purpose: prevent bucket replication target credentials from leaking through admin listing, bucket metadata export, and debug formatting.
|
||||
- Reason: site replication stores the `site-replicator-0` secret in bucket targets today; keeping the internal storage format intact while redacting external surfaces reduces immediate blast radius without changing replication target loading.
|
||||
3. `fix: require operation access for replication diagnostics`
|
||||
- Purpose: require operation-level admin authorization for site replication diagnostic POST endpoints and clamp netperf duration.
|
||||
- Reason: `devnull` and `netperf` read request bodies and exercise diagnostic work, so granting them through read-only site replication info permission was broader than intended.
|
||||
4. `fix: clean site replication targets on remove`
|
||||
- Purpose: remove bucket targets and `site-repl-*` replication rules that point at a removed deployment before completing pending remove.
|
||||
- Reason: removing a site only from site replication state leaves bucket-level replication configuration behind, so object replication can continue attempting to reach a removed peer.
|
||||
5. `feat: expose site replication status diagnostics`
|
||||
- Purpose: add machine-readable peer fetch errors and pending operation progress to site replication status.
|
||||
- Reason: operators previously saw only `Unknown` sync state or logs when peer metainfo fetches failed or remove/rotation was pending, which made automation and troubleshooting unnecessarily opaque.
|
||||
6. `docs: expand site replication hardening scope`
|
||||
- Purpose: keep the single-PR plan aligned with the expanded request to include MinIO compatibility, add preflight, bootstrap, lifecycle compatibility, retry, and repair work.
|
||||
- Reason: the remaining work is larger than the first hardening pass, so reviewers need to see the intended sequence before more behavior changes land.
|
||||
7. `fix: align site replication peer paths with minio`
|
||||
- Purpose: send peer site-replication calls over MinIO-compatible admin paths and accept the MinIO-style peer join route.
|
||||
- Reason: RustFS already accepts `/minio/admin` as an alias, so using MinIO wire paths improves mixed-cluster compatibility without removing RustFS route support.
|
||||
8. `fix: validate site replication add topology`
|
||||
- Purpose: preflight add requests with remote metainfo and IDP settings before creating service accounts, joining peers, or persisting state.
|
||||
- Reason: MinIO rejects unsafe add topologies up front; RustFS now rejects duplicate deployment IDs, missing local deployment, IDP mismatch, multiple non-empty initial sites, and peers already configured with a different site-replication set.
|
||||
9. `feat: bootstrap site replication metadata on add`
|
||||
- Purpose: replay the local site replication snapshot to joined peers during add, before object backfill starts.
|
||||
- Reason: existing hooks only replicate changes after site replication is enabled; existing policies, users, groups, bucket metadata, and buckets must be bootstrapped so a newly joined site does not start with an incomplete control-plane snapshot.
|
||||
10. `fix: align lifecycle replication with minio semantics`
|
||||
- Purpose: replicate bucket lifecycle metadata only when site replication has `replicate-ilm-expiry` enabled.
|
||||
- Reason: MinIO keeps lifecycle expiry replication opt-in; RustFS should not replicate lifecycle metadata by default during live bucket metadata hooks or add-time bootstrap.
|
||||
11. `feat: add site replication retry and repair MVP`
|
||||
- Purpose: persist failed peer replication attempts as retry metadata, expose retry counts in status, and add an operation-level repair endpoint that replays the current local snapshot.
|
||||
- Reason: transient peer failures were previously only visible through logs and required manual reconstruction; a durable, secret-safe queue plus repair replay gives operators a concrete recovery path without persisting sensitive request payloads.
|
||||
12. `fix: satisfy site replication pre-pr checks`
|
||||
- Purpose: remove a redundant MinIO admin-path branch and construct retry test fixtures directly.
|
||||
- Reason: the full pre-PR gate enforces clippy warnings as errors; this keeps the final branch green without changing site replication behavior.
|
||||
|
||||
## Verification
|
||||
|
||||
Baseline started from `origin/main` at `758677da`, then was rebased onto the latest `origin/main` at `1d6a8259`.
|
||||
|
||||
- Passed: `cargo fmt --all --check`
|
||||
- Passed: `cargo test -p rustfs-ecstore bucket_target --lib`
|
||||
- Passed: `cargo test -p rustfs-madmin site_replication --lib`
|
||||
- Passed: `cargo test -p rustfs route_policy --lib`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed: `cargo test -p rustfs route_policy --lib`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed: `cargo fmt --all`
|
||||
- Passed: `cargo fmt --all --check`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed: `cargo fmt --all --check`
|
||||
- Passed: `cargo test -p rustfs-madmin site_replication --lib`
|
||||
- Passed: `cargo test -p rustfs route_policy --lib`
|
||||
- Passed: `cargo test -p rustfs route_registration_test --lib`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed: `cargo fmt --all --check`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed: `cargo fmt --all --check`
|
||||
- Passed: `cargo test -p rustfs site_replication --lib`
|
||||
- Passed before the final main rebase: `make pre-pr`
|
||||
- Not completed after rebasing onto `1d6a8259`: `make pre-pr` was interrupted during clippy at request time; formatting, unsafe-code, architecture, and logging guardrails had already passed in that run.
|
||||
|
||||
## Impact
|
||||
|
||||
The planned implementation is intended to reduce credential exposure, require stronger permission for diagnostic write-like endpoints, clean removed site replication targets, and make replication status more actionable. Compatibility-sensitive behavior is called out per commit in the implementation log.
|
||||
|
||||
The credential redaction step changes admin/export visibility of bucket target secrets. Stored target configuration is not migrated or reformatted by this PR step.
|
||||
|
||||
The diagnostic authorization step intentionally changes required permission for `POST /site-replication/devnull` and `POST /site-replication/netperf` from `SiteReplicationInfoAction` to `SiteReplicationOperationAction`.
|
||||
|
||||
The remove cleanup step preserves user-managed replication rules and non-replication targets, and only prunes targets/rules associated with removed site replication deployment IDs.
|
||||
|
||||
The status diagnostics step adds optional `PeerErrors` and `PendingOperation` fields to the status response. Existing clients can ignore them, while automation can use them to distinguish peer reachability/auth failures from content mismatch.
|
||||
|
||||
## Additional Notes
|
||||
|
||||
This draft PR includes bootstrap, durable retry, and repair work in one reviewable branch. It should remain draft until CI or a full local pre-PR run confirms the latest default branch baseline.
|
||||
|
||||
The expanded single-PR scope also includes MinIO wire-contract bootstrap validation, durable site-replication retry, full add-time IAM/bootstrap sync, lifecycle compatibility, and site-level repair.
|
||||
|
||||
The peer path compatibility step maps outbound peer requests to `/minio/admin/v3/site-replication/...`. Peer join uses the encrypted MinIO payload contract, while internal peer metadata/IAM/remove/edit requests remain plain JSON to match MinIO handlers.
|
||||
|
||||
The add preflight step performs remote reads before state mutation, so add fails earlier when a peer cannot report metainfo or IDP settings.
|
||||
|
||||
The bootstrap step replays IAM policies, built-in users with stored secret material, group membership/status, policy mappings, bucket creation, and bucket metadata through the existing peer replication handlers. It does not synthesize ordinary service-account secrets when the snapshot does not contain them; the site-replicator service account remains distributed through the join flow.
|
||||
|
||||
Lifecycle metadata is now skipped by default in site replication hooks and bootstrap snapshots unless the site replication peer state enables `replicate-ilm-expiry`.
|
||||
|
||||
The retry MVP intentionally stores peer/path/error/count metadata only. Repair regenerates payloads from the current local snapshot, avoiding persistent storage of user secrets, service-account secrets, or bucket target credentials in the retry queue.
|
||||
@@ -0,0 +1,328 @@
|
||||
# Site Replication Hardening Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Harden RustFS site replication against credential exposure, unsafe diagnostic authorization, incomplete remove cleanup, and weak operational visibility while preserving MinIO-compatible API behavior.
|
||||
|
||||
**Architecture:** Start with low-risk guardrails that reduce blast radius, then add deterministic cleanup and status surfaces that support later durable replay work. Keep changes local to existing admin, madmin, and bucket replication modules before introducing larger state-machine refactors.
|
||||
|
||||
**Tech Stack:** Rust, Axum admin handlers, serde madmin contracts, ecstore bucket replication metadata, RustFS route policy tests, focused cargo tests.
|
||||
|
||||
---
|
||||
|
||||
## File Map
|
||||
|
||||
- `docs/architecture/site-replication-hardening-pr.md`: PR body draft and implementation log.
|
||||
- `crates/ecstore/src/bucket/target/bucket_target.rs`: credential redaction helpers and safe `Debug`.
|
||||
- `rustfs/src/admin/handlers/replication.rs`: remote target response/log redaction.
|
||||
- `rustfs/src/admin/handlers/bucket_meta.rs`: bucket metadata export redaction.
|
||||
- `rustfs/src/admin/route_policy.rs`: site replication diagnostic route action mapping.
|
||||
- `rustfs/src/admin/handlers/site_replication.rs`: diagnostic authorization, remove cleanup, status detail, and focused tests.
|
||||
- `crates/madmin/src/site_replication.rs`: response DTO fields for peer errors and pending operation visibility when needed.
|
||||
|
||||
## Task 1: Documentation Baseline
|
||||
|
||||
**Files:**
|
||||
- Create: `docs/superpowers/plans/2026-06-25-site-replication-hardening.md`
|
||||
- Create: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Write the implementation plan and PR draft**
|
||||
|
||||
Record the phased plan, implementation log, and PR template sections.
|
||||
|
||||
- [ ] **Step 2: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-06-25-site-replication-hardening.md docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "docs: plan site replication hardening"
|
||||
```
|
||||
|
||||
## Task 2: Redact Site Replication Credentials
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/ecstore/src/bucket/target/bucket_target.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/replication.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/bucket_meta.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Add safe credential formatting**
|
||||
|
||||
Implement custom `Debug` for credential-bearing bucket target structures and expose redacted response/export helpers where existing APIs serialize targets.
|
||||
|
||||
- [ ] **Step 2: Replace unsafe logs and admin responses**
|
||||
|
||||
Stop returning or logging `secret_key` in remote target listing and bucket metadata export unless an existing privileged import path requires the raw stored form.
|
||||
|
||||
- [ ] **Step 3: Add focused tests**
|
||||
|
||||
Add assertions that serialized admin/export output and `Debug` output do not contain the configured secret value.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
```bash
|
||||
cargo test -p rustfs replication --lib
|
||||
cargo test -p rustfs bucket_meta --lib
|
||||
cargo test -p rustfs site_replication --lib
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add crates/ecstore/src/bucket/target/bucket_target.rs rustfs/src/admin/handlers/replication.rs rustfs/src/admin/handlers/bucket_meta.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: redact site replication target secrets"
|
||||
```
|
||||
|
||||
## Task 3: Tighten Diagnostic Route Authorization
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/admin/route_policy.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Move devnull and netperf off read-only info authorization**
|
||||
|
||||
Map diagnostic POST endpoints to `SiteReplicationOperationAction` or a stricter existing action and update handlers to validate the same action.
|
||||
|
||||
- [ ] **Step 2: Add request bounds**
|
||||
|
||||
Ensure diagnostic body or duration inputs cannot be used as unbounded work by a low-privilege caller.
|
||||
|
||||
- [ ] **Step 3: Add route policy tests**
|
||||
|
||||
Assert `devnull` and `netperf` require operation-level permission.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
```bash
|
||||
cargo test -p rustfs route_policy --lib
|
||||
cargo test -p rustfs site_replication --lib
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/admin/route_policy.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: require operation access for replication diagnostics"
|
||||
```
|
||||
|
||||
## Task 4: Clean Replication Targets on Site Remove
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Add removed-peer target cleanup**
|
||||
|
||||
When a site is removed, traverse buckets and remove `site-repl-{deployment_id}` rules and matching bucket targets before final state is saved.
|
||||
|
||||
- [ ] **Step 2: Preserve runtime cache consistency**
|
||||
|
||||
After changing `bucket-targets.json`, call existing target refresh paths so replication clients stop using removed peers.
|
||||
|
||||
- [ ] **Step 3: Add focused tests**
|
||||
|
||||
Create a bucket target/rule fixture and assert remove cleanup prunes only the removed deployment.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
```bash
|
||||
cargo test -p rustfs site_replication --lib
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: clean site replication targets on remove"
|
||||
```
|
||||
|
||||
## Task 5: Improve Status Diagnostics
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/madmin/src/site_replication.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Expose peer fetch errors**
|
||||
|
||||
Add machine-readable peer error fields to status responses without failing the entire status call.
|
||||
|
||||
- [ ] **Step 2: Expose pending operations**
|
||||
|
||||
Return pending remove or credential rotation state with acked and pending peers.
|
||||
|
||||
- [ ] **Step 3: Add focused tests**
|
||||
|
||||
Assert status includes peer fetch error details and pending operation summaries.
|
||||
|
||||
- [ ] **Step 4: Run focused tests**
|
||||
|
||||
```bash
|
||||
cargo test -p rustfs site_replication --lib
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add crates/madmin/src/site_replication.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "feat: expose site replication status diagnostics"
|
||||
```
|
||||
|
||||
## Task 6: Compatibility and Release Verification
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Run focused verification**
|
||||
|
||||
```bash
|
||||
cargo fmt --all --check
|
||||
cargo test -p rustfs route_policy --lib
|
||||
cargo test -p rustfs route_registration_test --lib
|
||||
cargo test -p rustfs site_replication --lib
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run broader gate if feasible**
|
||||
|
||||
```bash
|
||||
make pre-commit
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Record verification and residual risk in PR draft**
|
||||
|
||||
Update the PR draft with commands run, failures, skips, and remaining follow-up items.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "docs: finalize site replication hardening notes"
|
||||
```
|
||||
|
||||
## Task 7: Extend Scope for Full Single-PR Completion
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/superpowers/plans/2026-06-25-site-replication-hardening.md`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Record the expanded single-PR scope**
|
||||
|
||||
Add follow-up tasks for MinIO wire compatibility, add preflight validation, bootstrap sync, lifecycle compatibility, durable retry, and repair.
|
||||
|
||||
- [ ] **Step 2: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-06-25-site-replication-hardening.md docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "docs: expand site replication hardening scope"
|
||||
```
|
||||
|
||||
## Task 8: MinIO-Compatible Peer Transport Contract
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [ ] **Step 1: Add peer admin path mapping**
|
||||
|
||||
Route outbound peer requests through MinIO-compatible `/minio/admin/v3/site-replication/...` paths when talking to a MinIO-compatible peer, while preserving RustFS compatibility through the existing alias handling.
|
||||
|
||||
- [ ] **Step 2: Add focused tests**
|
||||
|
||||
Assert peer path mapping preserves the request path used for signing and URL construction.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: align site replication peer paths with minio"
|
||||
```
|
||||
|
||||
## Task 9: Add-Time Topology Preflight
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [x] **Step 1: Validate self, deployment identity, IDP, and initial data shape**
|
||||
|
||||
Before persisting add state, fetch remote metainfo and IDP settings, reject duplicate deployment IDs, missing local site, existing site-replication topology gaps, IDP mismatch, and multiple non-empty initial sites.
|
||||
|
||||
- [x] **Step 2: Add focused tests**
|
||||
|
||||
Cover pure topology validation for duplicate deployments, missing self, IDP mismatch, and more than one non-empty site.
|
||||
|
||||
- [x] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: validate site replication add topology"
|
||||
```
|
||||
|
||||
## Task 10: Full Bootstrap Sync
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [x] **Step 1: Add snapshot bootstrap after add/join**
|
||||
|
||||
Use `build_sr_info` as the canonical local snapshot and sync IAM and bucket metadata to peers after add, before object resync.
|
||||
|
||||
- [x] **Step 2: Add focused tests**
|
||||
|
||||
Cover bootstrap task planning order and item counts without requiring live peers.
|
||||
|
||||
- [x] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "feat: bootstrap site replication metadata on add"
|
||||
```
|
||||
|
||||
## Task 11: Lifecycle Compatibility
|
||||
|
||||
**Files:**
|
||||
- Modify: `rustfs/src/app/bucket_usecase.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [x] **Step 1: Stop default full lifecycle replication**
|
||||
|
||||
Only replicate lifecycle expiry metadata when `replicate_ilm_expiry` is enabled for site replication peers.
|
||||
|
||||
- [x] **Step 2: Add focused tests**
|
||||
|
||||
Assert lifecycle metadata is skipped by default and included only for expiry-enabled peers.
|
||||
|
||||
- [x] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add rustfs/src/app/bucket_usecase.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "fix: align lifecycle replication with minio semantics"
|
||||
```
|
||||
|
||||
## Task 12: Durable Retry and Repair MVP
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/madmin/src/site_replication.rs`
|
||||
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
|
||||
- Modify: `docs/architecture/site-replication-hardening-pr.md`
|
||||
|
||||
- [x] **Step 1: Add minimal persistent retry queue**
|
||||
|
||||
Record failed bucket/IAM/state replication events with peer, path, payload, retry count, and last error; expose pending/failed counts in status.
|
||||
|
||||
- [x] **Step 2: Add repair once operation**
|
||||
|
||||
Provide an operation-level admin path that compares local snapshot to peer metainfo and replays missing metadata.
|
||||
|
||||
- [x] **Step 3: Add focused tests**
|
||||
|
||||
Cover queue serialization, retry status summaries, and repair task generation.
|
||||
|
||||
- [x] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add crates/madmin/src/site_replication.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
|
||||
git commit -m "feat: add site replication retry and repair MVP"
|
||||
```
|
||||
Reference in New Issue
Block a user