apache / apache/parquet-java

Allow supplying a KmsClient instance/supplier instead of only a reflectively-instantiated class name

オープン
#3,683 コメント 5 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Java
スター
3.1k
フォーク
1.6k
平均マージ
3日 12時間
マージ済み PR(30日)
33

説明

### Describe the enhancement requested

The key-tools KMS integration (`org.apache.parquet.crypto.keytools`) instantiates the `KmsClient` purely by reflection from a class name: `KeyToolkit.getKmsClient(...)` reads `parquet.encryption.kms.client.class` and calls `newInstance()`, requiring a public no-arg constructor, with credentials expected to arrive later via `KmsClient.initialize(conf, kmsInstanceID, kmsInstanceURL, accessToken)`. The same reflective no-arg pattern applies to `CryptoFactory` / `DecryptionPropertiesFactory` / `EncryptionPropertiesFactory` via `parquet.crypto.factory.class`.

This works well when a KMS client is stateless and can bootstrap all of its credentials from the `Configuration` plus the access-token string. It does not work for clients that must be *constructed with* their dependencies and can't be reduced to a class name + string token, e.g.:

- clients holding a live, credential-bearing SDK handle (federated / workload-identity credentials that aren't representable as a token string);
- clients created and wired by a dependency-injection container;
- in-memory / fake KMS clients used in tests, which carry per-test state and have no meaningful no-arg form.

For these, users must build a static side-channel: register the real instance in a static map keyed by a UUID written into the `Configuration`, point `parquet.encryption.kms.client.class` at a thin reflective shim that looks the instance back up in `initialize()`, and override `parquet.encryption.kms.instance.id` per instance to avoid colliding on `KeyToolkit`'s per-`(kmsInstanceID, accessToken)` client cache. That's global mutable state with its own lifecycle/leak management and a one-`Configuration`-per-client invariant — boilerplate every such user reinvents.

**Proposal.** Add an opt-in, fully backward-compatible way to supply a pre-built `KmsClient` (or a `Supplier` / small factory) programmatically, which `KeyToolkit.getKmsClient(...)` prefers over class-name reflection when present. Reflection stays the default, so existing configs are untouched. Rough shape (names TBD):

```java
// today (still works):
conf.set("parquet.encryption.kms.client.class", "com.example.MyKmsClient");

// proposed addition:
KeyToolkit.setKmsClientFactory(conf, () -> myPreBuiltKmsClient); // or a KmsClientFactory
```

`initialize(...)` would still be invoked on the supplied instance, so credential/token plumbing is unchanged.

**Scope / non-goals.** No new dependencies and no vendor-specific code — this is only about *how* a `KmsClient` is provided, not *which* one. `PropertiesDrivenCryptoFactory`, the `KeyMaterial` format, caching, per-column keys, and key-rotation tooling are all unchanged; this only lifts the requirement that the client be reflectively no-arg constructible.

**Question for maintainers.** Would a change along these lines be welcome? And do you prefer (a) a `Supplier` set on the `Configuration` / read-write options, or (b) a settable `KmsClientFactory` on `KeyToolkit`? Happy to implement it and open a PR (with tests using a non-no-arg client) once there's agreement on direction.

### Component(s)

Core

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

調査の方向性

まず KeyToolkit.getKmsClient(...) と、既存の CryptoFactory、DecryptionPropertiesFactory、EncryptionPropertiesFactory のリフレクション経路を読みます。maintainer と推奨する API およびストレージメカニズムを確認してから、引数なしコンストラクターを持たない、提供された KmsClient のテストカバレッジを追加し、リフレクションによる構築と initialize(...) の動作を維持します。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
java
領域
backend, security
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
活発
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。