sidb98 opened a new pull request, #23288:
URL: https://github.com/apache/kafka/pull/23288
## Summary
In KRaft, `KafkaRaftManager` builds its controller-quorum network client
with an
`SslChannelBuilder` that is never registered as a `Reconfigurable` on the
config that
processes dynamic updates. A renewed controller-listener certificate
therefore never
reaches the running Raft channel: it keeps the cert loaded at startup and,
on the next
reconnect (e.g. controller failover), can present a stale/expired cert,
breaking the
broker↔controller connection and stalling metadata propagation.
(KAFKA-20025.)
`SharedServer` creates three `KafkaConfig` objects (`sharedServerConfig`,
`brokerConfig`,
`controllerConfig`). `RaftManager` is constructed with `sharedServerConfig`,
whose
`dynamicConfig` is initialized but never receives cert-rotation updates
(those go to
`brokerConfig`/`controllerConfig`), so self-registering on RaftManager's own
config would
be a silent no-op.
## Changes
- `KafkaRaftManager`: expose the channel's `Reconfigurable`
(`reconfigurableChannelBuilder`)
and capture it when the channel builder is an SSL builder.
- `BrokerServer` / `ControllerServer`: register it on their own config
(`config.dynamicConfig.addReconfigurable`) alongside the other
reconfigurables, keeping
`RaftManager` on `sharedServerConfig` while routing the reconfigurable to
the config that
actually receives cert-rotation updates.
## Testing
**Unit** :
`RaftManagerTest.testRaftManagerExposesSslChannelBuilderAsReconfigurable`: a
RaftManager built with an SSL controller listener exposes its
`SslChannelBuilder` via
`reconfigurableChannelBuilder`.
**Integration** : `RaftManagerSslReconfigIntegrationTest`: brings up an SSL
KRaft cluster, rotates the broker's
`listener.name.controller.ssl.keystore.*` from cert A
to cert B via `incrementalAlterConfigs`, and asserts the certificate serial
loaded in the
running Raft channel's `SslFactory` (read by reflection) flips A → B.
Reverting the
registration makes it fail with "Raft channel SslFactory never loaded the
rotated keystore
(cert B)". Deterministic: it inspects the loaded keystore directly, not cert
expiry or
handshake timing. Cert A and B share a CA (controller trusts both; cluster
stays healthy)
and a subject DN.
## Alternate integration test considered but rejected
`Kraft cluster with short-lived cert A → rotate to B → force re-handshake →
assert no SSL
error`
This is flaky by construction: invalidating the old cert relies on expiry (A
and B share
a CA), and JSSE's PKIX validation reads the real system clock with no
injectable clock,
giving a bring-up race and an expiry race that generous margins only slow
down, never
remove. The implemented test asserts the fix's effect deterministically
instead.
--
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]
