jerryshao commented on code in PR #12249:
URL: https://github.com/apache/gravitino/pull/12249#discussion_r3688644030


##########
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

Review Comment:
   How do you define a URN format for memory provider?



-- 
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]

Reply via email to