Skip to content

feat(helm): deploy the RustFS COSI driver via cosiDriver.enabled - #246

Merged
GatewayJ merged 2 commits into
rustfs:mainfrom
BenjaminFuentesEviden:feat/cosi-driver-helm-chart
Sep 10, 2026
Merged

feat(helm): deploy the RustFS COSI driver via cosiDriver.enabled#246
GatewayJ merged 2 commits into
rustfs:mainfrom
BenjaminFuentesEviden:feat/cosi-driver-helm-chart

Conversation

@BenjaminFuentesEviden

Copy link
Copy Markdown
Contributor

Type of Change

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

Related Issues

Follow-up to #219 (merged) — the COSI driver binary itself. That PR intentionally left Helm
chart wiring out of scope ("can be added in a follow-up or kept in external GitOps").

Summary of Changes

Makes the RustFS COSI driver (rustfs.objectstorage.k8s.io, from #219) turnkey-deployable
instead of requiring hand-written manifests, and documents a clear getting-started path.

Helm chart (deploy/rustfs-operator/)

  • cosiDriver.enabled (default false) in values.yaml, following the same conventions as
    the existing console.* block (image/resources/securityContext/RBAC sub-values).
  • cosi-driver-deployment.yaml — a Deployment running the rustfs-cosi-driver binary
    alongside the official upstream objectstorage-sidecar container (pinned to v0.2.2,
    matching the CRD API version below), sharing a Unix socket via an emptyDir volume. Verified
    against the real Rook/ceph-cosi reference implementation for image tag, socket path, and
    container wiring.
  • cosi-driver-serviceaccount.yaml / cosi-driver-clusterrole.yaml /
    cosi-driver-clusterrolebinding.yaml — RBAC covering both the sidecar
    (objectstorage.k8s.io CRDs, Leases) and the driver (Secrets/ConfigMaps for the CAS
    ownership records), modeled on the upstream ceph-cosi reference RBAC.
  • Reuses the chart's existing openshift.enabled toggle so the driver Deployment behaves the
    same as the operator/console Deployments on OpenShift (omits securityContext, sets
    hostUsers: false, letting the SCC assign the pod's UID instead of a hardcoded one that
    would fail SCC admission).

Docs & examples

  • examples/cosi-bucket-provisioning.yaml — a full worked
    BucketClass/BucketAccessClass/BucketClaim/BucketAccess example plus a workload Pod
    consuming the resulting Secret. Field names and the CRD's spec-nesting shape were verified
    directly against the upstream Go types at the exact CRD version (v1alpha1) matching the
    pinned sidecar release, not from memory.
  • New "13.2 Deploying the COSI driver" section in both English and Chinese user guides, with
    separate Vanilla Kubernetes and OpenShift getting-started subsections (verified by
    Dockerfile/manifest inspection of the upstream controller+sidecar images — both are
    non-privileged, non-root-via-Dockerfile-only containers with no SCC-incompatible
    requirements), plus a table documenting every key the driver writes into the generated
    credentials Secret.
  • CHANGELOG.md entry under [Unreleased].

Out of scope

  • The upstream COSI CRDs/controller themselves are a cluster-wide prerequisite installed via
    kubectl apply -k github.com/kubernetes-sigs/container-object-storage-interface?ref=v0.2.2,
    not something this chart manages (matches how Rook/ceph-cosi expect it to be installed too).

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed helm lint and helm template across default/enabled/OpenShift/custom-RBAC
    value combinations
  • Added/updated necessary tests (Helm chart has no automated test harness in this repo)
  • Documentation updated
  • CHANGELOG.md updated under [Unreleased]
  • CI/CD passed (relying on CI)

Impact

  • Breaking change (CRD/API compatibility)
  • Requires doc/config/deployment update
  • Other impact: new optional Helm-managed Deployment + RBAC, disabled by default
    (cosiDriver.enabled: false); no impact on existing deployments that don't opt in.

Verification

helm lint deploy/rustfs-operator
helm template test deploy/rustfs-operator --set cosiDriver.enabled=true
helm template test deploy/rustfs-operator --set cosiDriver.enabled=true --set openshift.enabled=true

Wires the RustFS COSI driver (rustfs.objectstorage.k8s.io), added in
rustfs#219, into the Helm chart so it is turnkey-deployable instead of
requiring hand-written manifests. Adds a Deployment running the driver
alongside the upstream objectstorage-sidecar container over a shared
Unix socket, a dedicated ServiceAccount/ClusterRole covering both
containers, and a worked BucketClass/BucketAccessClass/BucketClaim/
BucketAccess example.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@BenjaminFuentesEviden

Copy link
Copy Markdown
Contributor Author

Better with a clear documentation @GatewayJ :D

@GatewayJ

Copy link
Copy Markdown
Member

Thanks for adding the Helm wiring and documentation for the RustFS COSI driver. I reviewed commit 5bd2db4372e50c07c35a63f8749a619f2cb187ed.

The Helm chart renders successfully for the default, enabled, OpenShift, external RBAC, and custom ServiceAccount configurations. The driver binary path, Unix socket path, sidecar image, and basic COSI v1alpha1 resource shapes also look correct.

However, I do not think this is ready to merge yet:

  1. The documented credential Secret format is incorrect.

    The example and both user guides state that the Secret created for credentialsSecretName contains flat keys such as AWS_ACCESS_KEY_ID, accessKeyID, endpoint, and BUCKETS, and that workloads can consume them directly through envFrom.

    With objectstorage-sidecar:v0.2.2, the sidecar reads only accessKeyID, accessSecretKey, endpoint, and region from the driver response and creates a Secret containing a single BucketInfo JSON entry. Extra aliases and BUCKETS are discarded.

    As written, the example workload receives only a BucketInfo environment variable and will not receive the AWS credential variables it expects. Please update the example and documentation to mount and parse BucketInfo, or provide an actual adapter that produces the documented flat Secret format.

  2. The ClusterRole is over-privileged and incomplete.

    The role grants cluster-wide list access to Secrets, although neither the driver nor the pinned sidecar requires Secret listing. A compromise of either container would therefore allow enumeration and retrieval of all Secrets in the cluster.

    At the same time, the role does not grant patch on Events. The pinned sidecar uses the client-go event recorder, which PATCHes aggregated/repeated Events, so those updates will fail with 403 Forbidden.

    Please split the Secret and Event rules, remove unnecessary Secret list, add Event patch, and tighten the remaining COSI, ConfigMap, and Lease permissions to the operations and namespaces actually used.

  3. The BucketAccess example should explicitly set the protocol.

    Please add:

    protocol: S3
    

Although the CRD describes this field as optional, sidecar v0.2.2 copies BucketAccess.spec.protocol directly into BucketInfo and does not implement the documented fallback. Omitting it produces an empty protocol value.
There is also no automated Helm or COSI integration coverage in this PR. The current CI workflow installs Helm but does not run helm lint or helm template. Please add CI assertions for the disabled, enabled, OpenShift, custom ServiceAccount, and custom RBAC branches, along with coverage of the actual generated BucketInfo Secret format.
Verdict: Request changes. The chart syntax is valid, but the documented credential-consumption path and RBAC contract need to be corrected before merge.

Addresses review feedback on rustfs#246 (commit 5bd2db4), verified against
objectstorage-sidecar v0.2.2 and client-go source:

- Document and demonstrate the real credential Secret shape: sidecar
  v0.2.2 writes a single `BucketInfo` JSON key, not the flat
  AWS_ACCESS_KEY_ID-style keys previously documented. Replace the
  envFrom example with a volume-mounted, JSON-parsing Pod.
- Add `protocol: S3` to the BucketAccess example: the sidecar copies
  this field verbatim into BucketInfo with no fallback despite the
  CRD's optional-field doc comment, so omitting it yields an empty
  protocol.
- Tighten the COSI driver ClusterRole: drop unused `list` on Secrets
  (neither the driver nor the pinned sidecar ever lists Secrets) and
  add `patch` on Events (client-go's event recorder PATCHes
  repeated/aggregated events, which 403'd without it).
- Add a Helm lint/template CI step covering the disabled, enabled,
  OpenShift, custom-ServiceAccount, and custom-RBAC branches; ci.yml
  previously installed Helm but never invoked it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@BenjaminFuentesEviden

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — all four points checked out against the pinned objectstorage-sidecar:v0.2.2 and client-go sources, and are fixed in 0487210:

  1. Credential Secret format: confirmed via sidecar/pkg/bucketaccess/bucketaccess_controller.go — the sidecar writes a single BucketInfo JSON key, discarding everything else. Replaced the envFrom example with a volume-mounted Pod that reads/parses that JSON, and corrected the key table in both user guides to show the actual BucketInfo shape.
  2. RBAC: split Secrets/Events into separate rules, dropped list on Secrets (verified neither the driver nor the pinned sidecar ever lists Secrets — no informer, only Get/Create/Update/Delete), and added patch on Events (confirmed client-go's tools/record/event.go calls sink.Patch for repeated/aggregated events).
  3. protocol: S3: added to the example, with a note that v0.2.2 copies BucketAccess.spec.protocol verbatim into BucketInfo with no fallback, despite the CRD doc-comment describing one.
  4. CI coverage: added a Helm lint/template step covering disabled, enabled, OpenShift, custom-ServiceAccount, and custom-RBAC branches (ci.yml previously installed Helm but never invoked it).

One caveat worth flagging: full end-to-end coverage of the actual generated BucketInfo Secret (i.e., a live cluster running the real driver + sidecar + a RustFS backend) is out of scope for this Helm-chart PR — the new CI step verifies the chart renders correctly, not runtime COSI behavior. Happy to discuss a follow-up e2e test if that's wanted.

🤖 Generated with Claude Code

@GatewayJ
GatewayJ added this pull request to the merge queue Sep 10, 2026
Merged via the queue into rustfs:main with commit 4a0a504 Sep 10, 2026
3 checks passed
@BenjaminFuentesEviden

Copy link
Copy Markdown
Contributor Author

Amazing !

Thank you @GatewayJ

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.

2 participants