stevenwarejones opened a new issue, #3683:
URL: https://github.com/apache/parquet-java/issues/3683
### 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<KmsClient>` / 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<KmsClient>` 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
--
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]
