Skip to content

feat(cosi): add RustFS COSI v1alpha1 driver with idempotent grants - #217

Open
BenjaminFuentesEviden wants to merge 4 commits into
rustfs:mainfrom
BenjaminFuentesEviden:feat/cosi-v1alpha1-driver
Open

feat(cosi): add RustFS COSI v1alpha1 driver with idempotent grants#217
BenjaminFuentesEviden wants to merge 4 commits into
rustfs:mainfrom
BenjaminFuentesEviden:feat/cosi-v1alpha1-driver

Conversation

@BenjaminFuentesEviden

@BenjaminFuentesEviden BenjaminFuentesEviden commented Aug 4, 2026

Copy link
Copy Markdown

Type of Change

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

Related Issues

N/A

Summary of Changes

Adds a RustFS COSI v1alpha1 driver (rustfs.objectstorage.k8s.io) and hardens DriverGrantBucketAccess so shared preferredAccessKey cannot rotate secrets out from under another BucketAccess (Ceph-like isolation).

What this PR adds

  1. New binary rustfs-cosi-driver (crates/cosi-driver)

    • Implements COSI Identity + Provisioner gRPC (official objectstorage-sidecar UNIX socket pattern).
    • Built and shipped in the same operator image (Dockerfile builds -p operator -p rustfs-cosi-driver).
    • Driver name: rustfs.objectstorage.k8s.io.

  2. BucketClass / BucketAccessClass parameters (Rook-style admin secret + endpoint)

    • Required: endpoint, objectStoreUserSecretName, objectStoreUserSecretNamespace
    • Optional: region, policy, TLS CA ConfigMap refs
    • Optional RustFS extensions: bucketName / buckets (multi-bucket, * for broad policy), preferredAccessKey / accessKey

  3. Admin client support

    • get_user_info on RustfsAdminClient (parse attached policy names from /rustfs/admin/v3/user-info)
    • Used for ownership checks on grant retries / reuse

  4. Docs

    • User-guide note: preferredAccessKey must be unique per BucketAccess; default (omit it) is safest

Design choices (important for reviewers)

Choice Why
Rust in-repo driver (not Go) Matches operator language/tooling; reuses existing RustfsAdminClient
COSI API v1alpha1 (release-0.2 style) Aligns with current kubernetes-sigs COSI / Rook Ceph driver
No CephCOSIDriver-style CR in MVP Helm/GitOps toggles deployment externally; keep operator CRD surface small
Tenant linkage via BucketClass parameters (admin Secret + endpoint) Same pattern as Rook; no Tenant reconcile coupling
Default account id = COSI grant name (ba-<UID>) Ceph-compatible unique account per BucketAccess
Optional preferredAccessKey Allows stable human-readable S3 access keys when explicitly requested
Deterministic secrets sha256("rustfs-cosi-v1:{account_id}") Idempotent retries without reading secret keys from RustFS (user-info does not return them)
Never call add_user when user already exists Avoids rotating secretKey via PUT and breaking live workloads (SignatureDoesNotMatch)
Owner marker policy cosi-grant-{grant_name} Distinguishes same-claim retry vs bad reuse of preferredAccessKey
Reuse of existing preferredAccessKey by another claim → AlreadyExists Fail closed instead of silently sharing/rotating credentials
Multi-bucket via buckets / bucketName Supports workloads that need several buckets or * in IAM policy

Out of scope / follow-ups

  • Helm chart templates for deploying the COSI driver Deployment/RBAC (can be added in a follow-up or kept in external GitOps).
  • Returning existing secrets from RustFS admin API (not available today); deterministic derivation is the chosen workaround.
  • Cluster ops remediation for already-rotated keys (recreate BucketAccess / unique preferred keys).

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)
  • CHANGELOG.md updated under [Unreleased] (if user-visible change)
  • CI/CD passed (if applicable)

Notes on checklist:

  • cargo fmt --check, cargo clippy -p rustfs-cosi-driver --all-targets -- -D warnings, and unit tests for cosi-driver + get_user_info parsing were run locally.
  • Full make pre-commit (including console lint/fmt) was not fully green in this contributor environment; please rely on CI.

Impact

  • Breaking change (CRD/API compatibility)
  • Requires doc/config/deployment update
  • Other impact: new container binary rustfs-cosi-driver in the operator image; deploy with official COSI sidecar

Verification

# Unit tests
cargo test -p rustfs-cosi-driver
cargo test -p operator --lib -- parse_tests

# Lint (driver)
cargo clippy -p rustfs-cosi-driver --all-targets -- -D warnings

# Full gate (maintainers / CI)
make pre-commit

Manual expectation for grant semantics:

  1. Two BucketAccess with the same preferredAccessKey → second grant returns gRPC AlreadyExists.
  2. Retry of the same BucketAccess → OK with identical secret (no rotation).
  3. Omit preferredAccessKey → unique account per COSI grant name (ba-<UID>).

Additional Notes

This work was validated against a demo cluster using RustFS + COSI BucketClaim/BucketAccess, where shared preferredAccessKey + non-idempotent add_user previously caused SignatureDoesNotMatch on PutObject. The grant path above is specifically designed to prevent that class of failure while remaining Ceph-COSI-compatible by default.

Driver name for BucketClass / BucketAccessClass:

driverName: rustfs.objectstorage.k8s.io

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.

BenjaminFuentesEviden and others added 4 commits August 3, 2026 17:48
@BenjaminFuentesEviden @cursoragent
feat(cosi): add RustFS COSI v1alpha1 driver
5934b60 
Extract shared rustfs-admin client, ship a tonic COSI driver with Helm
toggle, and document BucketClass parameters for Tenant-backed S3.

Co-authored-by: Cursor <cursoragent@cursor.com>
@BenjaminFuentesEviden @cursoragent
fix(cosi): make grant idempotent, reject key reuse
ef72ff2 
Align DriverGrantBucketAccess with Ceph-style isolation: deterministic
secrets, never rotate existing users, and return AlreadyExists when
preferredAccessKey is claimed by another BucketAccess.

Co-authored-by: Cursor <cursoragent@cursor.com>
@BenjaminFuentesEviden
chore: update .gitignore to include .kubeconfig
3de8b37
@BenjaminFuentesEviden
chore: correct .gitignore entry for kubeconfig
2bbfe28
@BenjaminFuentesEviden

BenjaminFuentesEviden commented Aug 4, 2026

Copy link
Copy Markdown
Author

Hi @GatewayJ, I know you are quite busy delivering the stable V1, but what do you think about my COSI PR here?
It is important for us to keep close to Standards (even if it is in an early stage)

@GatewayJ

GatewayJ commented Aug 4, 2026

Copy link
Copy Markdown
Member

Thanks for contributing COSI support to RustFS. The overall direction is valuable, but this PR is not ready to merge yet. Please address the following items.

CLA

I could not find BenjaminFuentesEviden in the official [RustFS CLA signature records](https://github.com/rustfs/cla/tree/main/signatures/individual). Please sign the current [RustFS CLA v2](https://github.com/rustfs/cla/blob/main/README.md) before we continue with the merge process.

The Operator repository also does not currently run an automated CLA check, so the absence of a CLA status on this PR does not indicate that it has been signed.

Blocking correctness and security issues

  1. The generated S3 secret is predictable

    crates/cosi-driver/src/parameters.rs:165-169 derives the secret as:

    sha256("rustfs-cosi-v1:{account_id}")

    An access key is an identifier, not a secret. Anyone who knows the access key can calculate the corresponding secret key. Credentials should be randomly generated and durably persisted before creating the RustFS user. An unkeyed deterministic hash is not suitable for credential generation.

  2. Grant ownership is not crash-safe or fully idempotent

    In crates/cosi-driver/src/driver.rs:246-281, the RustFS user is created before the owner marker is attached. If the process exits or a policy operation fails after add_user, the next retry sees an existing user without the marker and returns AlreadyExists.

    This leaves a valid COSI request permanently stuck and also creates a race between concurrent grants. Ownership should be persisted through a Kubernetes CAS-backed PendingCreate checkpoint before external RustFS mutations, then promoted after provisioning succeeds.

  3. A shared policy name can change permissions for existing users

    driver.rs:229-267 uses the policy BucketAccessClass parameter as a global canned-policy name, while add_canned_policy replaces the existing document.

    Since a BucketAccessClass is normally shared by multiple BucketAccess resources, a later grant can overwrite the policy used by earlier users and silently move or broaden their bucket permissions. Generated policies must be unique per grant. Externally managed policies should only be referenced and validated, never replaced.

  4. Bucket ownership and deletion are unsafe

    driver.rs:149-158 treats every backend AlreadyExists result as a successful idempotent retry without verifying ownership or compatible parameters. driver.rs:184-194 later deletes all buckets expanded from the shared class parameters.

    This can cause different BucketClaims to share an existing bucket or allow one claim to delete a bucket it did not create. Dynamic provisioning should use the COSI-generated unique bucket name. Existing/static buckets need a separate adoption path and must never be deleted without ownership proof.

  5. The implementation is not fully compliant with COSI semantics

    The protobuf is wire-compatible with the latest stable [COSI v0.2.2](https://github.com/kubernetes-sigs/container-object-storage-interface/releases/tag/v0.2.2), but the behavior is not fully compliant:

    • Same bucket name with incompatible parameters must return AlreadyExists; the current code returns success for every collision.
    • UnknownAuthenticationType is accepted as KEY even though authentication type is required.
    • Grant retry and partial-failure semantics are not reliably idempotent.

    Please explicitly pin the supported sidecar/controller version to v0.2.2. The current upstream main branch uses v1alpha2 and is not wire-compatible with this v1alpha1 service.

Architecture concerns

The decision to run COSI as a separate driver process is reasonable, but the integration is not sufficiently unified with the existing Operator architecture:

  • rustfs-cosi-driver depends on the entire operator crate only to reuse RustfsAdminClient. The admin client should be extracted into a small shared crate used by both components.
  • Existing Tenant provisioning stores ownership using Tenant status and CAS checkpoints, while this driver uses IAM policy names as ownership markers. This creates two independent ownership models for the same RustFS users, policies, and buckets.
  • driver.rs currently combines gRPC transport, parameter validation, credential generation, policy construction, ownership decisions, lifecycle orchestration, and backend I/O. The gRPC layer should delegate to a testable provisioning/grant state machine.
  • The binary is added to the Operator image, but no Helm Deployment, sidecar version, socket configuration, RBAC, or lifecycle ownership is provided. Users cannot enable the feature through the existing Operator installation.

A more maintainable structure would be:

Operator provisioning ─┐
                       ├─ shared ownership/lifecycle service ─ rustfs-admin-client
COSI gRPC adapter ─────┘                  │
                                  Kubernetes CAS state

The Helm chart should then manage the COSI sidecar, driver, RBAC, socket, and supported protocol version together.

Required validation

Please add tests covering:

  • partial failure after user creation;
  • concurrent grants using the same preferred access key;
  • shared BucketAccessClass policy behavior;
  • existing bucket collisions and delete ownership;
  • retry/restart idempotency;
  • an end-to-end flow using the pinned COSI controller/sidecar version.

Given the credential vulnerability and ownership/lifecycle issues, this PR should not be merged in its current form. I recommend addressing the shared client and ownership model first, then adding the COSI transport and deployment integration on top of that foundation.

@BenjaminFuentesEviden

BenjaminFuentesEviden commented Aug 5, 2026 via email •
edited
Loading

Copy link
Copy Markdown
Author

@BenjaminFuentesEviden

BenjaminFuentesEviden commented Aug 5, 2026

Copy link
Copy Markdown
Author

Thanks @GatewayJ for the thorough review — I agree with the assessment and the recommended sequencing.

CLA: signed via rustfs/cla#6 (pending merge/record).

On the five blockers: I accept all of them.

  1. The deterministic sha256("rustfs-cosi-v1:{account_id}") secret was a temporary workaround for idempotent retries after we hit SignatureDoesNotMatch from secret rotation in production. It is not acceptable as a credential scheme — we will switch to randomly generated secrets persisted durably (Kubernetes Secret / CAS checkpoint) before add_user.
  2. Agree that creating the RustFS user before an ownership checkpoint is not crash-safe. We will move to a Kubernetes CAS-backed PendingCreate → provision → Ready flow, aligned with existing Tenant ownership patterns, instead of IAM policy-name markers.
  3. Agree that shared BucketAccessClass policy names must not be overwritten via add_canned_policy. Generated policies will be unique per grant; externally managed policies will be referenced/validated only.
  4. Agree on bucket ownership/deletion: dynamic provisioning will use the COSI-generated unique name; static/adoption needs an explicit path and must not delete without ownership proof.
  5. We will tighten COSI semantics (incompatible params → AlreadyExists, reject UnknownAuthenticationType as KEY) and explicitly pin the supported controller/sidecar to v0.2.2.

Plan going forward (split as you suggested):

  • PR A (foundation first): extract a small shared rustfs-admin client crate; introduce a shared ownership/lifecycle model with CAS checkpoints and durable credential persistence; unique grant policies; tests for partial failure / concurrency / retries.
  • PR B (COSI + Helm): thin gRPC adapter on top of that foundation; Helm Deployment + RBAC + socket + pinned sidecar/controller v0.2.2; e2e validation.

I will keep this PR (#217) as the discussion / tracking surface and open follow-up PRs against main (or update this branch once the foundation lands — happy to follow your preferred workflow).

Thanks again for the clear guidance.

@BenjaminFuentesEviden BenjaminFuentesEviden mentioned this pull request Aug 5, 2026
Open
16 tasks
@BenjaminFuentesEviden

BenjaminFuentesEviden commented Aug 5, 2026

Copy link
Copy Markdown
Author

PR A opened

Foundation work (shared rustfs-admin, durable random secrets, CAS PendingCreate→Ready ownership, unique/validate-only policies, split grant/bucket state machine) is in:

#219

Helm Deployment / sidecar pin v0.2.2 / full COSI semantic compliance remain for PR B. Keeping this PR open as tracking until B lands.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BenjaminFuentesEviden @GatewayJ