Skip to content

fix(provisioning): protect existing RustFS users - #212

Merged
GatewayJ merged 3 commits into
rustfs:mainfrom
GatewayJ:fix/issue-1098-protect-existing-users
Aug 2, 2026
Merged

fix(provisioning): protect existing RustFS users#212
GatewayJ merged 3 commits into
rustfs:mainfrom
GatewayJ:fix/issue-1098-protect-existing-users

Conversation

@GatewayJ

@GatewayJ GatewayJ commented Aug 1, 2026

Copy link
Copy Markdown
Member

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other: N/A

Related Issues

Closes rustfs/backlog#1098

Summary of Changes

  • Fail closed when a RustFS user already exists without a trusted Operator ownership checkpoint.
  • Persist a Tenant UID, user, and access-key ownership intent before external user creation.
  • Re-read the Tenant, validate UID/generation and the prior user snapshot, then use the latest resourceVersion for the status CAS.
  • Batch every ownership intent prepared in one reconcile into a single status CAS, eliminating per-user full-status Kubernetes round trips.
  • Verify the returned user state and complete ownership tuple before any RustFS write; old-CRD pruning is detected and blocked.
  • Keep ownership on user-specific provisioning status instead of exposing it on policy and bucket status schemas.
  • Support complete matching legacy Ready and Retained checkpoints while preserving crash recovery and credential rotation.
  • Report ownership conflicts and permanent checkpoint failures as actionable top-level Blocked states.
  • Add a live API-server contract test for stale resourceVersion conflicts and CRD unknown-field pruning.

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit (fmt-check + clippy + test + console-lint + console-fmt-check)
  • Added/updated necessary tests
  • Documentation updated (if needed) — deployment ordering is documented below
  • CHANGELOG.md updated under [Unreleased] (if user-visible change)
  • CI/CD passed (if applicable)

Impact

  • Breaking change (CRD/API compatibility)
  • Requires doc/config/deployment update
  • Other impact: Tenant user status gains additive ownership checkpoint fields; unmanaged existing users are no longer modified.

Verification

make pre-commit

Additional Notes

Apply the updated Tenant CRD before rolling out the new Operator. Helm does not upgrade CRDs already installed from a chart's crds/ directory. If the old schema prunes ownership, the Operator now fails closed before any RustFS credential or policy write.

RustFS currently has no conditional create-user API or external ownership metadata. The Kubernetes checkpoint and stale-snapshot guard protect the normal serialized controller path, but they cannot provide exactly-once delivery against independent RustFS actors. A fully atomic boundary requires a future RustFS create-only operation with a persistent idempotency or ownership token.


Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

@GatewayJ
GatewayJ force-pushed the fix/issue-1098-protect-existing-users branch from 23e50b9 to 3deb26e Compare August 2, 2026 09:23
@GatewayJ
GatewayJ marked this pull request as ready for review August 2, 2026 11:51
@GatewayJ
GatewayJ added this pull request to the merge queue Aug 2, 2026
Merged via the queue into rustfs:main with commit 8a51951 Aug 2, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c77c9a23f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1351 to +1355
matches!(
previous.state.as_str(),
state if state == ProvisioningItemState::Ready.as_str()
|| state == ProvisioningItemState::Retained.as_str()
) && previous.last_applied_access_key_hash.as_deref() == Some(current_access_key_hash.as_str())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allow proven failed legacy users to migrate

When upgrading a Tenant whose operator-managed user was previously Ready but whose latest reconciliation left it Failed—for example because a referenced policy became unavailable or set_user_policy failed—the legacy status still preserves the matching observedSecretName and lastAppliedAccessKeyHash. Restricting migration to Ready or Retained rejects that proof, so the existing user enters UserOwnershipConflict and remains blocked even after the original configuration problem is fixed; accept matching failed legacy statuses that retain complete applied-identity metadata.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant