singhpk234 commented on code in PR #17155:
URL: https://github.com/apache/iceberg/pull/17155#discussion_r3650422287
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3665,6 +3676,43 @@ components:
additionalProperties:
type: string
+ KeyManagementCredential:
+ type: object
+ description: |
+ Provider-specific credential config for accessing one or more KMS keys
required by an encrypted
+ table operation.
+
+ The key-management provider is advertised in catalog configuration,
such as `encryption.kms-type`
+ returned from `/v1/config`. The `config` map contains
provider-specific properties for the
+ selected key-management provider.
+
+ Catalogs that return `key-management-credentials` for an operation
must include credentials for all
+ KMS key IDs required by table encryption metadata. Clients should
select the credential config by
Review Comment:
in loadTable we don't know which snapshot the client is gonna end up
reading, so we might have provide access to all the keys ...
one good thing though is if some one has read only access to the table we
just vend
decrypt creds .... if one has both R/W we can do decrypt + encrypt ....
older keys ...
AuthZ is anyways not defined in IRC so this just a TMI :) just adding this
here for completion
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3674,6 +3722,10 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ key-management-credentials:
+ type: array
+ items:
+ $ref: '#/components/schemas/KeyManagementCredential'
Review Comment:
is LoadCreds not always required to return KMS creds ? if yes do we need new
`kms-vended-credentials` in access-delegation ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3665,6 +3676,43 @@ components:
additionalProperties:
type: string
+ KeyManagementCredential:
+ type: object
+ description: |
+ Provider-specific credential config for accessing one or more KMS keys
required by an encrypted
+ table operation.
+
+ The key-management provider is advertised in catalog configuration,
such as `encryption.kms-type`
+ returned from `/v1/config`. The `config` map contains
provider-specific properties for the
+ selected key-management provider.
+
+ Catalogs that return `key-management-credentials` for an operation
must include credentials for all
+ KMS key IDs required by table encryption metadata. Clients should
select the credential config by
+ matching KMS key identifiers referenced by table encryption metadata,
such as
+ `EncryptedKey.encrypted-by-id`, against `kms-key-ids`.
+
+ Credential configs should use provider-specific expiration mechanisms
where available and should
+ be scoped to the minimum required KMS operations and listed KMS key
IDs where the provider
+ supports it. Clients must not persist credentials beyond any
provider-specific expiration.
+ required:
+ - kms-key-ids
+ - config
+ properties:
+ kms-key-ids:
+ type: array
+ description: |
+ KMS key identifiers for which the credential config is relevant.
+
+ Clients should match these values against KMS key identifiers
referenced by table encryption
+ metadata, such as `EncryptedKey.encrypted-by-id`.
+ items:
+ type: string
+ config:
+ type: object
+ description: Provider-specific credential configuration for
accessing the listed KMS key IDs.
+ additionalProperties:
+ type: string
Review Comment:
is it possible to standarize this across provider rather than keeping a kv
map ?
##########
docs/docs/catalog-properties.md:
##########
@@ -55,6 +56,7 @@ The following properties configure the behavior of the REST
catalog client.
| `rest-page-size` | null | The page size to
use when listing namespaces, tables, or other paginated resources.
|
| `namespace-separator` | `%1F` | The separator
character used for namespace levels when communicating with the REST server.
|
| `scan-planning-mode` | `CLIENT` | Controls where
scan planning is performed. Supported values: `CLIENT` (client-side planning),
`SERVER` (server-side planning). Can be overridden per-table by the server in
LoadTableResponse. |
+| `encryption.kms-type` | null | KMS provider
type used for encrypted tables. Clients may configure this value, but REST
catalog servers may advertise it through `/v1/config`; server `overrides` take
precedence over client configuration. When using `key-management-credentials`,
clients should use the final catalog configuration value after applying server
defaults and overrides. The value is an open enum with initial values `aws`,
`gcp`, and `azure`; catalogs may return additional values when the catalog and
client agree on KMS client resolution and credential config. |
Review Comment:
is this absolutely required, can't we just expect this in the loadTable
response ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3711,6 +3763,13 @@ components:
Credentials for ADLS / GCS / S3 / ... are provided through the
`storage-credentials` field.
Clients must first check whether the respective credentials exist in
the `storage-credentials` field before checking the `config` for credentials.
+ ## Key Management Credentials
+
+ Credentials for KMS / key-management systems are provided through the
`key-management-credentials` field.
+ Clients must first check whether the respective credentials exist in
the `key-management-credentials`
+ field before checking the `config` for credentials.
Review Comment:
you mean storage credentials ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3665,6 +3676,43 @@ components:
additionalProperties:
type: string
+ KeyManagementCredential:
+ type: object
+ description: |
+ Provider-specific credential config for accessing one or more KMS keys
required by an encrypted
+ table operation.
+
+ The key-management provider is advertised in catalog configuration,
such as `encryption.kms-type`
+ returned from `/v1/config`. The `config` map contains
provider-specific properties for the
+ selected key-management provider.
+
+ Catalogs that return `key-management-credentials` for an operation
must include credentials for all
+ KMS key IDs required by table encryption metadata. Clients should
select the credential config by
+ matching KMS key identifiers referenced by table encryption metadata,
such as
+ `EncryptedKey.encrypted-by-id`, against `kms-key-ids`.
Review Comment:
is this java class ref ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3711,6 +3763,13 @@ components:
Credentials for ADLS / GCS / S3 / ... are provided through the
`storage-credentials` field.
Clients must first check whether the respective credentials exist in
the `storage-credentials` field before checking the `config` for credentials.
+ ## Key Management Credentials
+
+ Credentials for KMS / key-management systems are provided through the
`key-management-credentials` field.
+ Clients must first check whether the respective credentials exist in
the `key-management-credentials`
+ field before checking the `config` for credentials.
+ Clients must use `key-management-credentials` consistently with the
REST access-delegation credential sourcing rules.
Review Comment:
```suggestion
Clients must use `key-management-credentials` if present
consistently with the REST access-delegation credential sourcing rules.
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
