sudheesh-87 opened a new issue, #19184:
URL: https://github.com/apache/pinot/issues/19184

   ## Problem
   
   Realtime tables with Kafka SSL (mutual TLS) require keystore/truststore 
passwords in table `streamConfigs`. These values are stored in ZooKeeper and 
must be updated on every certificate/password rotation (e.g. Strimzi cert sync 
to Vault).
   
   Apache Kafka supports externalizing sensitive config via [Config 
Providers](https://kafka.apache.org/documentation/#config_providers) (KIP-421), 
including `FileConfigProvider`:
   
   ```properties
   config.providers=file
   
config.providers.file.class=org.apache.kafka.common.config.provider.FileConfigProvider
   
ssl.keystore.password=${file:/path/to/keystore-password.txt:keystore.password}
   
ssl.truststore.password=${file:/path/to/truststore-password.txt:truststore.password}
   ```
   
   **This does not work in Pinot today.** Operators must either:
   
   1. Store plaintext passwords in table config (bad for security/rotation), or
   2. Use `${ENV_VAR}` substitution (still requires pod env sync; values can 
leak via API — see #13983, fixed in #14002 for GET masking only), or
   3. Build out-of-band automation (e.g. controller REST hook) to PATCH table 
config on every secret rotation.
   
   Option 3 is fragile and couples platform secret management to Pinot table 
metadata.
   
   ## Root cause
   
   Pinot filters Kafka client properties before constructing `KafkaConsumer` / 
`AdminClient`.
   
   In `KafkaPartitionLevelConnectionHandler` (kafka 2.0 / 3.0 / 4.0 modules), 
`filterKafkaProperties()` only retains keys present in 
`ConsumerConfig.configNames()` or `AdminClientConfig.configNames()`.
   
   `config.providers` and `config.providers.file.class` are **not** included in 
those allowlists, so they are **stripped** before the properties reach the 
Kafka client. File-based `${file:...}` references in `ssl.*.password` are never 
resolved.
   
   Relevant code path (all kafka connector versions):
   
   ```
   createConsumer() → filterKafkaProperties(props, CONSUMER_CONFIG_NAMES)
   AdminClient creation → filterKafkaProperties(props, 
ADMIN_CLIENT_CONFIG_NAMES)
   ```
   
   ## Use case
   
   Kubernetes deployments commonly mount rotated secrets via Vault Agent / CSI 
/ K8s secrets to paths like:
   
   ```
   /vault/secrets/kafka-keypass.txt      # keystore.password=...
   /vault/secrets/kafka-trustpass.txt    # truststore.password=...
   /vault/secrets/kafka-keystore.p12
   /vault/secrets/kafka-truststore.p12
   ```
   
   **Desired table config** (passwords never stored in ZK):
   
   ```json
   {
     "streamConfigs": {
       "streamType": "kafka",
       "security.protocol": "SSL",
       "ssl.keystore.type": "PKCS12",
       "ssl.keystore.location": "/vault/secrets/kafka-keystore.p12",
       "ssl.truststore.type": "PKCS12",
       "ssl.truststore.location": "/vault/secrets/kafka-truststore.p12",
       "config.providers": "file",
       "config.providers.file.class": 
"org.apache.kafka.common.config.provider.FileConfigProvider",
       "stream.kafka.consumer.prop.config.providers": "file",
       "stream.kafka.consumer.prop.config.providers.file.class": 
"org.apache.kafka.common.config.provider.FileConfigProvider",
       "stream.kafka.consumer.prop.ssl.keystore.password": 
"${file:/vault/secrets/kafka-keypass.txt:keystore.password}",
       "stream.kafka.consumer.prop.ssl.truststore.password": 
"${file:/vault/secrets/kafka-trustpass.txt:truststore.password}",
       "ssl.keystore.password": 
"${file:/vault/secrets/kafka-keypass.txt:keystore.password}",
       "ssl.truststore.password": 
"${file:/vault/secrets/kafka-trustpass.txt:truststore.password}"
     }
   }
   ```
   
   After secret files on disk are updated (e.g. Vault Agent re-render), Kafka 
consumers should pick up new passwords on consumer recreation (`forceCommit` / 
segment rollover) **without** PATCHing table config.
   
   ## Additional context: controller AdminClient
   
   Controller uses a **non-prefixed** `ssl.*` block in `streamConfigs` for 
partition metadata / AdminClient (see `KafkaPartitionLevelConnectionHandler` 
admin client init). Server consumers use `stream.kafka.consumer.prop.ssl.*` 
(prefix stripped by StreamConfig).
   
   Both code paths call `filterKafkaProperties()`, so both need 
`config.providers` passthrough.
   
   ## Related upstream work (does not solve this)
   
   | Item | Gap |
   |------|-----|
   | [PR #12249](https://github.com/apache/pinot/pull/12249) Dynamic Kafka SSL 
| Passwords still in table config; rotation = update ZK + `forceCommit` |
   | [#12107](https://github.com/apache/pinot/issues/12107) / [PR 
#12277](https://github.com/apache/pinot/pull/12277) TLS rotation | Pinot 
**internal** TLS (HTTP/JDBC), not Kafka stream consumer config |
   | [#13983](https://github.com/apache/pinot/issues/13983) / [PR 
#14002](https://github.com/apache/pinot/pull/14002) | Env var masking in GET 
API; still not file-based externalization |
   | [PR #16965](https://github.com/apache/pinot/pull/16965) | May tighten 
property filtering further — worth coordinating |
   
   PR #12277 review comment noted Kafka client work was separate and suggested 
filing a generic issue — **no issue was filed**.
   
   ## Proposed solution
   
   1. **Pass through config provider keys** in `filterKafkaProperties()`:
      - `config.providers`
      - Any key matching `config.providers.<name>.class` (and other provider 
sub-keys per KIP-421)
   
   2. **Apply to both** consumer and admin client property filtering.
   
   3. **Document** supported pattern in Kafka ingestion docs (file paths must 
be absolute and readable by server/controller pods).
   
   4. **Optional enhancement:** hot-reload keystore/truststore **files** on 
existing consumers (similar to internal TLS work in #12107) — separate from 
config provider passthrough but valuable for rotation without `forceCommit`.
   
   ## Acceptance criteria
   
   - [ ] Table config with `config.providers` + `${file:...}` passwords creates 
a working Kafka consumer (integration test with temp password files)
   - [ ] Controller AdminClient connects with non-prefixed `ssl.*` + file 
provider config
   - [ ] Password values are **not** required in ZK table config (only file 
paths / provider config)
   - [ ] Documented in [Kafka ingestion 
docs](https://docs.pinot.apache.org/build-with-pinot/ingestion/stream-ingestion/import-from-apache-kafka)
   
   ## Environment
   
   - Pinot with `pinot-kafka-3.0` or `pinot-kafka-4.0` connector
   - Kafka brokers with SSL (Strimzi / mutual TLS)
   - Secrets mounted as files (Vault Agent, K8s secrets, etc.)
   
   ## Workaround today
   
   Platform-maintained hook script on controller pod: Vault Agent 
`agent-inject-command` → read `/vault/secrets/*` → PATCH 4 password fields per 
table via REST → `forceCommit`. Requires static password placeholders in table 
config and operational coupling to secret rotation events.


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

Reply via email to