lasdf1234 commented on code in PR #12249: URL: https://github.com/apache/gravitino/pull/12249#discussion_r3688829445
########## design-docs/gravitino-entity-secrets.md: ########## @@ -0,0 +1,946 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +# Design of Entity Secrets Management in Apache Gravitino + +## 1. Background + +Gravitino entities often require **connection secrets** in properties, for example: + +| Category | Where | Example keys | +| ----------------- | ----------------------------------------- | -------------------------------------------------- | +| JDBC | Catalog | `jdbc-password` | +| Static cloud keys | Catalog / schema / fileset (esp. Fileset) | `s3-secret-access-key`, `aws-secret-access-key`, … | +| Kerberos | Schema / fileset (Fileset catalog) | keytab / principal related properties | + +Today these values are commonly stored as **plaintext strings** in entity properties JSON +(`catalog_meta` / `schema_meta` / `fileset_version_info`). API responses may redact them, but +persistence is not a secrets manager. That creates: + +1. **Security risk** — DB / backup / support dumps may expose long-lived secrets. +2. **Governance gap** — enterprises already operate a central secret store and want Gravitino to + **reference** it, not fork a second password silo. + +Peer systems solve the same problem with an abstraction between **metadata** and **secret +material**: + +- **Apache Polaris** exposes a `UserSecretsManager` SPI and persists a typed `SecretReference` + object on the entity (not an ambiguous plaintext string). +- **Databricks** provides a **Secrets** service (scopes); Unity Catalog connections / foreign + catalogs reference secrets via `secret(scope, key)` instead of embedding passwords as bare + strings in connection options. + +Gravitino should define a pluggable **secrets-provider SPI**, persist durable **URN references** +instead of plaintext for marked keys, expose a clear **REST create/alter contract**, and ship an +**in-memory provider** for tests and local use. + +--- + +## 2. Goals + +1. **Secrets provider interface**: define a pluggable `GravitinoSecretProvider` for write / + read / delete of secret material behind durable **references**. + +2. **Follow existing `KmsClient` patterns when implementing `GravitinoSecretProvider`**: only a + minority of products can be connected the same way for both table-encryption KMS and entity + secrets. What can be reused is the Factory / registry style and, where applicable, the same + connection setup — not the `KmsClient` interface itself. + +3. **REST + persistence split**: HTTP **`properties`** stays **`map<string, string>`**; optional + **`secretReferences`** (key → locator object; server **builds** the URN) and/or + **`secretBindings`** (key → provider name for write-through) mark secrets on **create**; **alter** + adds `@type`s **`setSecretBinding`** / **`setSecretReference`** (§5.9.4) for **catalog, schema, and + fileset** (see §5.9). **Persistence** stays an all-string JSON map on each entity's properties + column. Secret property values are stored as **URN strings**. A property is treated as a secret + when its value matches the **URN recognition rule** (§5.1): starts with `urn:gravitino-secret` + and ends with that property's key. Whether to `deleteSecret` on entity drop or alter + `removeProperty` is decided from the **URN shape** (write-through embeds + `entityType`/`entityId`/`propertyKey` — §5.5.2 C). + +4. **Backward compatible reads**: existing all-string entity properties continue to + work as plaintext with no migration required. + +5. **Omit secrets on GET/list and audit**: GET/list **omit** any property whose value matches the + URN recognition rule (§5.1) (same strip behavior as today's `PropertiesMetadata.hidden`). + +6. **In-memory provider**: ship a process-local `InMemorySecretsProvider` for UT / IT / local + quick-start (not for production). + +7. **Server-side resolution only**: resolve references on the Gravitino server when loading + catalogs / schemas / filesets or connecting; call `readSecret` **on each use**. + +## 3. Non-Goals + +1. **Fixed sensitive-key allowlists as the resolution gate**: Polaris-style fixed property-name + allowlists are out of scope. Secrets are identified by **URN-shaped values** (§5.1), not by a + hardcoded or reserved list of property names. REST **`secretReferences` / `secretBindings`** + declare which keys become secrets on create. + +2. **Plaintext provider credentials in configuration**: if a future provider needs credentials, + long-lived credential **values** must not appear in `gravitino.properties`. The in-memory + provider needs none. Configuration stores only non-secret settings (and env var **names** when + a provider requires them). + +3. **Additional provider implementations**: this design ships only **`InMemorySecretsProvider`**. + Other backends are out of scope here. + +## 4. Industry Approaches (Polaris and Databricks) + +This section compares **Apache Polaris** and **Databricks Secrets**. + +### 4.1 Apache Polaris — typed `SecretReference`, not string sniffing + +On create, inline plaintext is **write-through** via `UserSecretsManager.writeSecret`; only the +`SecretReference` object is stored. Reads call `readSecret(reference)`. + +**Takeaway:** typed persistence + SPI; secret material lives in the secrets manager. URN shell follows +[RFC 8141](https://www.rfc-editor.org/rfc/rfc8141.html) `urn:<NID>:<NSS>` with NID `polaris-secret` +(`urn:polaris-secret:<type>:<type-specific-identifier>`); identifier **semantics** stay per-provider. + +### 4.2 Databricks — Secrets service + references from connections + +Databricks stores secret material in the **Secrets** platform service. Connections recommend +`secret('scope', 'key')` instead of password literals. Runtime resolves from the Secrets service; +displays redact as `[REDACTED]`. + +**Takeaway:** password material in a secrets service; catalog/connection config holds references. + +### 4.3 Cross-product summary + +| Topic | Apache Polaris | Databricks | +| --------------------------------------- | ---------------------------------------------------------------------------- | --------------------------------------------------------------------------- | +| Where secret material lives | `UserSecretsManager` backend | Databricks Secrets service | +| What is persisted on catalog/connection | Typed `SecretReference` object | `secret(scope,key)` | +| Secret binding model | **Fixed allowlist** (`clientSecret`, `bearerToken`, …); always write-through | **Same property** may be plaintext **or** `secret(scope,key)` | +| Official backend kinds | SPI — any implementation | **Databricks-backed** + Azure Key Vault (peer product; not Gravitino scope) | + +### 4.4 Why Gravitino follows Polaris (not Databricks) for the reference shape + +Both peers share one idea we keep: **secret material lives outside catalog metadata**; catalogs hold +**references**, and does not expose secret material on read. **How** that idea is expressed differs +— and Gravitino’s product shape matches **Polaris** more closely than **Databricks Secrets**. + +| Dimension | Databricks | Polaris | Gravitino choice | +| ------------------------------ | ------------------------------------------------------------- | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Where secrets are stored | First-party **Secrets** service (scopes) | Pluggable **`UserSecretsManager`** (BYO / impl) | Pluggable SPI + in-memory provider — not a new Gravitino “scopes” product | +| How catalogs reference secrets | Platform DSL `secret(scope, key)` in SQL / connection options | Typed **`SecretReference`** object on the entity | REST: create **`secretReferences` / `secretBindings`**; alter **`setSecretBinding` / `setSecretReference`**; persistence: **URN string**; recognize secrets by URN shape; no SQL/`secret()` runtime | +| Secret binding model | **Same property** may be plaintext **or** `secret(scope,key)` | **Fixed allowlist** only; always write-through | **`secretReferences` / `secretBindings`** on create; alter via **`setSecretBinding` / `setSecretReference`**; omit URN-shaped keys on GET | +| Multi-backend / multi-instance | Scoped under the Databricks Secrets service | SPI type + URN `type-specific-identifier` | Named entries in server conf; URN embeds **`provider_name`** only (`className` selects implementation at factory time) | +| Official backend kinds | Databricks-backed + Azure Key Vault (peer) | SPI — implementer’s choice | **In-memory** provider shipped; SPI remains pluggable | + +--- + +## 5. Proposal + +### 5.1 Value model (REST vs persistence) + +| Layer | Shape | Role | +| ----------------------------------- | --------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| **REST** `properties` | `map<string, string>` | Unchanged from today — create HTTP values are strings | +| **REST** `secretReferences` | `map<string, object>` | Optional on **create** — **property key → locator** (external ref; server builds URN — §5.9.2) | +| **REST** alter secret `@type`s | in `updates` | **`setSecretBinding`** / **`setSecretReference`** (§5.9.4) — same `{ "updates": [...] }` body; `setProperty` unchanged | +| **REST** `secretBindings` | `map<string, string>` | Optional on **create** — **property key → provider name** (write-through; plaintext in `properties`) | +| **Persistence** entity `properties` | JSON **string map** | `catalog_meta` / `schema_meta` / `fileset_version_info` — secret keys store URN strings (§5.1 recognition rule) | + +**Secret recognition rule** (server-side; no reserved metadata key): + +A property `(key, value)` is treated as a **secret property** when **both** hold: + +1. `value` **starts with** `urn:gravitino-secret` +2. `value` **ends with** `key` (the property key) + +Server-built URNs always place the property key as the **last segment**, so create/alter paths +satisfy this rule by construction. Plaintext values never match. + +**Server-side resolve path** (entity load / connect — URN shape, not a key list): + +| Condition | Runtime behavior | +| ---------------------------------- | ----------------------------------------------------------------- | +| Value matches the recognition rule | Value is a URN string → parse `provider_name` → `readSecret(urn)` | +| Value does **not** match | Use value as plaintext; **do not** call secrets provider | + +Drop / `removeProperty` `deleteSecret` uses URN shape (§5.5.2 C). + +#### 5.1.1 URN shape + +```text +urn:gravitino-secret:<provider_name>:<type-specific-identifier> +``` + +| Part | Unified? | Rule | +| ---------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `urn:gravitino-secret` | Yes | Fixed scheme + namespace | +| `<provider_name>` | Yes | Config key `gravitino.secret.provider.<name>.*`; **authoritative** for registry lookup (provider settings live here — **not** in the URN) | +| `<type-specific-identifier>` | No | Address of the secret **inside** the provider selected by `provider_name`. Colon-separated `[a-zA-Z0-9_-]+` segments. Layout is defined by the provider implementation (`className`). | + +Secret URNs follow [RFC 8141](https://www.rfc-editor.org/rfc/rfc8141.html) `urn:<NID>:<NSS>` — a +persistent, location-independent name (not a fetch URL). Gravitino uses informal NID +`gravitino-secret`. + +| RFC 8141 term | Role | Gravitino mapping | +| ----------------------------------- | ------------------------------------------------- | -------------------------------------------- | +| **Scheme** | Always `urn` | `urn` | +| **NID** (Namespace Identifier) | Which naming system; unique across all `urn:` IDs | `gravitino-secret` | +| **NSS** (Namespace Specific String) | Concrete ID within that namespace | `<provider_name>:<type-specific-identifier>` | + +We use `urn:gravitino-secret:` so a secret property value is unambiguously a **Gravitino secret +handle**. Resolve by **`provider_name`** only; the SPI parses `<type-specific-identifier>`. +For recognition (§5.1), the full URN must also **end with the property key**. + +##### Why catalog **and** schema / fileset + +Fileset catalogs may attach Kerberos / cloud credentials on **schema** and **fileset** properties +(not only on the catalog). The same secrets SPI and URN envelope therefore apply to all three +entity levels. Write-through identifiers must include an **entity type** plus a **stable entity id** +so that (1) secrets for different levels never collide, and (2) renames of metalake/catalog/schema/ +fileset **names** do not invalidate stored URNs. + +##### Write-through / `in-memory`: `type-specific-identifier` naming + +```text +<type-specific-identifier> ::= <entityType>:<entityId>:<propertyKey> +``` + +| Segment | Values | Notes | +| --------------- | --------------------------------- | ----------------------------------------------------------- | +| `<entityType>` | `catalog`, `schema`, or `fileset` | Discriminator for property-bag owner | +| `<entityId>` | Stable numeric id | From `SecretWriteContext`; survives rename | +| `<propertyKey>` | Entity property key | e.g. `jdbc-password`, `s3-secret-access-key` (last segment) | + +Examples: + +```text +urn:gravitino-secret:memory:catalog:10042:jdbc-password +urn:gravitino-secret:memory:schema:20007:authentication-type +urn:gravitino-secret:memory:fileset:30019:s3-secret-access-key +``` + +The property key from `SecretWriteContext` is the last segment (not an opaque ordinal). +Re-writing the same property key overwrites that URN / map entry (no ordinal allocation). + +Write-through URNs (including in-memory) embed `<entityType>:<entityId>:<propertyKey>` so drop +can decide whether to call `deleteSecret` (§5.5.2 C). External-ref identifier layouts are defined +by each provider implementation, but the built URN **must still end with the property key** so the +recognition rule applies. This design only specifies the in-memory write-through form. + +Legacy persistence (no `secretReferences` / `secretBindings` on create): + +```json +{ + "jdbc-url": "jdbc:postgresql://db.example.com:5432/inventory", + "jdbc-user": "app_reader", + "jdbc-password": "S3cret!Passw0rd" +} +``` + +Reference persistence (after external ref or write-through): + +```json +{ + "jdbc-url": "jdbc:postgresql://db.example.com:5432/inventory", + "jdbc-user": "app_reader", + "jdbc-password": "urn:gravitino-secret:memory:catalog:10042:jdbc-password" +} +``` + +(Write-through stores the URN under the secret key; ownership is visible in the URN shape.) + +### 5.2 Secrets-provider instance registry + +Register named instances in +**server configuration** (`gravitino.conf` / `gravitino.properties` and included files), not in a +database table. + +**Authentication model:** the in-memory provider needs no credentials. Configuration stores +`className` and any non-secret settings — never plaintext credential values. + +```properties +gravitino.secret.providers=memory + +# In-memory (default for tests / local) +gravitino.secret.provider.memory.className=org.apache.gravitino.secrets.memory.InMemorySecretsProvider +``` + +Same shape as `gravitino.eventListener.names` + `gravitino.eventListener.{name}.class`: the +list holds **instance names**; `className` selects the implementation; remaining keys are +instance settings. + +| Key pattern | Meaning | +| -------------------------------------------- | ----------------------------------------------------------------------------- | +| `gravitino.secret.providers` | Comma-separated **instance names** (cluster scope) | +| `gravitino.secret.provider.<name>.className` | Fully qualified `GravitinoSecretProvider` implementation class (**required**) | +| `gravitino.secret.provider.<name>.*` | Implementation-specific settings (none for in-memory beyond `className`) | + +**Startup sequence (v1):** + +1. Operator starts Gravitino. +2. Provider factory loads each instance's **`className`**, passes the remaining + `gravitino.secret.provider.<name>.*` keys, and constructs live `GravitinoSecretProvider` + instances. + +Example entry summary: + +| provider_name | className (short) | settings (excerpt) | +| ------------- | -------------------------- | ------------------------- | +| `memory` | `…InMemorySecretsProvider` | (none beyond `className`) | + +Operators register or change these entries by **editing configuration and restarting** the Gravitino +server (v1). See §8 for the full configuration reference. + +### 5.3 Architecture + +``` + gravitino.conf (cluster) + gravitino.secret.providers=memory + gravitino.secret.provider.memory.className=… + + Catalog load / create + │ + ▼ + for each property (key, value): + value starts with urn:gravitino-secret + AND value ends with key + → parse provider_name → SPI.readSecret(urn) + else → plaintext as stored + │ + ▼ + catalog_meta.properties: all-string JSON map (secret values = URN strings) + GET/list: omit keys whose values match the URN recognition rule +``` + +### 5.4 Scope of this design + +| In scope | Out of scope | +| ---------------------------------------------- | ------------------------------------------------- | +| SPI + URN recognition + resolve / omit-on-read | Additional provider implementations beyond memory | +| Load providers from server conf | | +| **In-memory** secrets provider (UT/IT / local) | | + +Missing / unloadable `className` ⇒ startup or resolve fails with a clear error. + +### 5.5 `GravitinoSecretProvider` + +#### 5.5.1 One provider instance per configured name + +Core loads each named conf entry into **one** live `GravitinoSecretProvider` (via `className`) +and passes the remaining instance properties. Catalog resolve does: + +```text +value matches URN recognition rule (§5.1) + → parse provider_name from URN + → lookup live instance by name + → instance.readSecret(urn) +``` + +Illustrative Java (names TBD): + +```java +/** + * Backend client for a single configured secrets provider. + * Not a cluster-wide facade — core routes by provider_name parsed from the URN. + * Instance name / className are bound at factory time; each impl must implement type(). + */ +public interface GravitinoSecretProvider { + + String type(); + + /** + * Write-through: store plaintext in this backend and return a durable reference URN. + * Read-only / external-ref-only implementations throw UnsupportedOperationException. + * Core may wrap a provider-returned type-specific identifier into the full URN using the + * factory-bound provider name — or the impl returns the full URN. + */ + String writeSecret(String plaintext, SecretWriteContext context); + + /** Fetch secret material. Caller must not log or return this to HTTP GET/list. */ + String readSecret(String urn); Review Comment: It's a good qeustion.I think your review was well-considered and excellent. I think SPI and persisted form as String(The database is also a storage urn, which is simpler and clearer.) and add a URN parse/build helper in core. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
