lasdf1234 commented on code in PR #12249: URL: https://github.com/apache/gravitino/pull/12249#discussion_r3679591629
########## design-docs/gravitino-entity-secrets.md: ########## @@ -0,0 +1,962 @@ +<!-- + 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. Additional provider implementations are out of +scope for this design (the SPI remains open for them). + +--- + +## 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**; reserved key + **`gravitino.secret.keys`** (all secret keys) is comma-separated and server-managed. + Whether to `deleteSecret` on entity drop is decided from the **URN shape** (write-through embeds + `entityType`/`entityId`/`propertyKey` — §5.5.2 C), not a second reserved list. + +4. **Backend registry**: register named secrets-provider instances in **server configuration + files**. Catalogs reference instances by **`provider_name` in the secret URN**. Clients + **discover** registered names via read-only `GET /api/secrets/providers` (§5.9.6) — + registration is still conf-only (no CRUD REST). + +5. **Backward compatible reads**: existing all-string entity properties continue to + work as plaintext with no migration required. + +6. **Omit secrets on GET/list and audit**: GET/list **omit** any key listed in + persisted **`gravitino.secret.keys`** (same strip behavior as today's `PropertiesMetadata.hidden`). + +7. **In-memory provider**: ship a process-local `InMemorySecretsProvider` for UT / IT / local + quick-start (not for production). + +8. **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. **Sensitive-key allowlists as the resolution gate**: keys listed in + **`gravitino.secret.keys`** resolve via the secrets provider (value is a URN string); other + keys stay plaintext. REST **`secretReferences` / `secretBindings`** declare which keys are + 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; the SPI stays pluggable via `className`. + +## 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** + reserved **`gravitino.secret.keys`**; 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`**; persist listed keys in **`gravitino.secret.keys`**; omit those 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; see reserved keys below | + +**Reserved persistence keys** (server-managed; clients must not set them in REST `properties`): + +| Key | Meaning | +| ----------------------- | ------------------------------------------------------------------------------------------------- | +| `gravitino.secret.keys` | Comma-separated keys whose values are secret **URN strings** (external ref **and** write-through) | + +| Rule | Behavior | +| ------- | -------------------------------------------------------------------- | +| Present | At least one secret key; omit the key entirely when the set is empty | +| Example | `gravitino.secret.keys=jdbc-password,s3-secret-access-key` | + +After create/alter: + +```text +gravitino.secret.keys = all keys from secretReferences ∪ secretBindings (and prior secrets kept) +``` Review Comment: So this configuration is used to track the keys that should be deleted when the catalog drops, right? ########## design-docs/gravitino-entity-secrets.md: ########## @@ -0,0 +1,962 @@ +<!-- + 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. Additional provider implementations are out of +scope for this design (the SPI remains open for them). + +--- + +## 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**; reserved key + **`gravitino.secret.keys`** (all secret keys) is comma-separated and server-managed. + Whether to `deleteSecret` on entity drop is decided from the **URN shape** (write-through embeds + `entityType`/`entityId`/`propertyKey` — §5.5.2 C), not a second reserved list. + +4. **Backend registry**: register named secrets-provider instances in **server configuration + files**. Catalogs reference instances by **`provider_name` in the secret URN**. Clients + **discover** registered names via read-only `GET /api/secrets/providers` (§5.9.6) — + registration is still conf-only (no CRUD REST). + +5. **Backward compatible reads**: existing all-string entity properties continue to + work as plaintext with no migration required. + +6. **Omit secrets on GET/list and audit**: GET/list **omit** any key listed in + persisted **`gravitino.secret.keys`** (same strip behavior as today's `PropertiesMetadata.hidden`). + +7. **In-memory provider**: ship a process-local `InMemorySecretsProvider` for UT / IT / local + quick-start (not for production). + +8. **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. **Sensitive-key allowlists as the resolution gate**: keys listed in + **`gravitino.secret.keys`** resolve via the secrets provider (value is a URN string); other + keys stay plaintext. REST **`secretReferences` / `secretBindings`** declare which keys are + 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; the SPI stays pluggable via `className`. + +## 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** + reserved **`gravitino.secret.keys`**; 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`**; persist listed keys in **`gravitino.secret.keys`**; omit those 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; see reserved keys below | + +**Reserved persistence keys** (server-managed; clients must not set them in REST `properties`): + +| Key | Meaning | +| ----------------------- | ------------------------------------------------------------------------------------------------- | +| `gravitino.secret.keys` | Comma-separated keys whose values are secret **URN strings** (external ref **and** write-through) | + +| Rule | Behavior | +| ------- | -------------------------------------------------------------------- | +| Present | At least one secret key; omit the key entirely when the set is empty | +| Example | `gravitino.secret.keys=jdbc-password,s3-secret-access-key` | + +After create/alter: + +```text +gravitino.secret.keys = all keys from secretReferences ∪ secretBindings (and prior secrets kept) +``` Review Comment: Yes. gravitino.secret.keys track the keys. When the catalog is deleted, if sensitive information was created by gravitino, it will be removed from the corresponding ksm client. -- 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]
