Skip to content

feat(storage): add Storage Provider contract (ADR-0001 + design spec)#26

Draft
alex-mabrouk wants to merge 5 commits into
mainfrom
feat/storage-provider-abstraction
Draft

feat(storage): add Storage Provider contract (ADR-0001 + design spec)#26
alex-mabrouk wants to merge 5 commits into
mainfrom
feat/storage-provider-abstraction

Conversation

@alex-mabrouk

@alex-mabrouk alex-mabrouk commented Jul 7, 2026

Copy link
Copy Markdown

Description

Make OKDP storage-agnostic by introducing a Storage Provider abstraction (Adapter pattern):
data services (Trino, Hive Metastore, Spark History, JupyterHub, Polaris) depend only on a stable,
backend-neutral contract (defaultStorage: provider / buckets / grants), and each backend
(SeaweedFS, RustFS, MinIO, Ceph RGW, AWS S3) implements it as an adapter that never references a
consumer context.

Today the SeaweedFS package is tightly coupled to every consumer (it reads .Context.trino,
.Context.hiveMetastore, .Context.polaris, …), so a second backend would be a full rewrite. This
PR replaces that inverted dependency with a clean contract.

**Delivered in three commits **

  1. docs: the contract — ADR-0001 (why) + design spec (how).
  2. refactor(seaweedfs): turn the SeaweedFS package into a compliant adapter consuming only
    defaultStorage.{provider,buckets,grants} (behaviour-preserving) + mirror the neutral contract
    into the catalog contexts.
  3. feat(rustfs): add RustFS as the second provider (official chart + mc-based provisioning Job).

Related Issue

Fixes #25

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / chore
  • Breaking change

How to Test

Contract review (this commit): read docs/adr/0001-storage-provider-abstraction.md and
docs/design/storage-provider-contract.md.

End-to-end (once commits 2 & 3 land), validated locally on a Kind cluster:

  1. Refactored SeaweedFS adapter deploys with no consumer-context reference; identities/buckets are
    built from grants/buckets; Polaris STS AssumeRole + real S3 write succeed.
  2. Swap SeaweedFS → RustFS by changing only defaultStorage.provider.endpoints (+ name) and the
    deployed infra Release: RustFS auto-provisions the 7 grants (users/policies) and 5 buckets, and
    Polaris STS AssumeRole + write round-trip succeed — with zero change to any consumer.

Checklist

  • I have read CONTRIBUTING.md
  • I have tested my changes
  • Documentation updated if needed
  • If breaking change: migration path described above
  • I hereby declare this contribution to be licensed under the Apache License Version 2.0.
  • I hereby agree to grant TOSIT a copyright license to use my contributions.

Introduce the Storage Provider abstraction that makes OKDP storage-agnostic:
consumers depend on a stable, backend-neutral contract (defaultStorage:
provider/buckets/grants) and each backend (SeaweedFS, RustFS, MinIO, Ceph RGW,
AWS S3) implements it as an adapter, without ever referencing consumer contexts.
@alex-mabrouk
alex-mabrouk requested review from idirze and mlahouar July 7, 2026 16:07
@alex-mabrouk
alex-mabrouk marked this pull request as draft July 7, 2026 16:07
@alex-mabrouk alex-mabrouk added enhancement New feature or request documentation Improvements or additions to documentation labels Jul 7, 2026
Alex added 4 commits July 7, 2026 19:08
Refactor the SeaweedFS package into a compliant adapter of the Storage Provider
contract (docs/design/storage-provider-contract.md): it now consumes ONLY
defaultStorage.{provider,buckets,grants} and no longer reaches into consumer
contexts (.Context.trino / .hiveMetastore / .jupyterHub / .sparkHistory / .polaris).
@idirze

idirze commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@alex-mabrouk ,

Looking at the implementation, issue #25 and Design model, I think two concerns get conflated: what the current abstraction already standardizes, and what it doesn't standardize yet.

The current provider model is consumer-facing only: as a platform contract, OKDP doesn't provision storage and doesn't mandate a particular object storage backend yet. It assumes Storage, IdP, and Database already exist and wires services to them (consumption). SeaweedFS provisioning runs only as a self-contained sandbox/demo convenience, it's not meant to be the good abstraction for storage.

So the gap your ADR closes is good: standardize the provisioning side of storage into a backend-neutral contract, the same way consumption already is.

A couple of specifics on the field shape:

  • I would treat the provisioner as a separate design concern that probably deserves its own model, rather than merging provisioning fields into the consumption part.
  • pathStyle is S3-addressing-specific (path-style vs virtual-host), it doesn't apply to non-S3 backends, so it can't be a universal field, it's better modeled as a provider capability. roleArn is very specific also.
  • The provisioning bits currently live under defaultStorage.settings and aren't standardized, that's maybe the part to lift out into a dedicated provisioning abstraction.

@alex-mabrouk

alex-mabrouk commented Jul 10, 2026

Copy link
Copy Markdown
Author

@idirze

Following up on the discussion in #25 , here's how i propose to reflect it. Since the PR is still draft and the contract isn't merged anywhere, this is the cheapest moment to reshape it; i'd rather settle the structure now than after adapters ship against it.

defaultStorage:
  # CONSUMPTION — what consumers need to connect (thin, per your sketch)
  provider:
    type: s3
    name: RustFS
    region: dummy
    tls: true
    endpoints: { apiUrl: ..., stsUrl: ..., consoleUrl: ... }
    s3:                      # protocol-scoped capabilities, not universal fields
      pathStyle: true
      sts: { roleArn: ... }

  # REQUIREMENTS — what the platform needs to exist. Self-hosted providers
  # realize it; external providers (AWS) treat it as satisfied out-of-band.
  requirements:
    buckets: [ ... ]
    grants:  [ ... ]

Notes:

  • I picked requirements over provisioning deliberately: on BYO/AWS nothing is provisioned, yet the declaration still has value (spec for out-of-band setup + validation target). Happy to go with provisioning if you prefer the term.
  • defaultStorage.settings is already removed in this PR (commit dd32a8a) . backend-specific bits live in each provider package's schema.
  • Zero behavior change: both adapters just re-path $storage.grants$storage.requirements.grants, and consumers see pathStyle move under provider.s3.

If this matches what you had in mind, I'll push the restructure (ADR + design spec + both adapters) on this branch. WDYT?

@Pierrotws

Copy link
Copy Markdown
Contributor

A migrer sur l’autre repo (prerequisites/core)

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

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce a storage provider abstraction to make OKDP storage backend-agnostic

3 participants