KAFKA-8190; Don't update keystore modification time during validation#6539
Conversation
omkreddy
left a comment
omkreddy
left a comment
There was a problem hiding this comment.
@rajinisivaram Thanks for the PR. Good find. LGTM.
looks like we don't have a test in DynamicBrokerReconfigurationTest to test this scenario.
|
@omkreddy Thanks for the review. Updated DynamicBrokerReconfigurationTest to test this scenario. Can you take a look? Thanks. |
|
Thanks for the update. LGTM. |
|
There was a test failure in one of the builds. Looks like this was because file modification time hadn't changed on the file. Updated test to change file modification time explicitly (I could recreate the failure by running in a loop and it passes reliably for me). Also added info-level log entry for SSLContext update. For all config changes, we log at info-level in @omkreddy Can you review once more please? Sorry about that. |
|
Thanks for the update. LGTM. |
|
@omkreddy Thanks for the reviews! Merging to trunk, 2.2 and 2.1. Build failure is unrelated (ConsumerBounceTest). |
…#6539) Ensure that modification time is checked against the file used to create the SSLContext that is in-use so that SSLContext is updated whenever file is modified and a config update request is received. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
…#6539) Ensure that modification time is checked against the file used to create the SSLContext that is in-use so that SSLContext is updated whenever file is modified and a config update request is received. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
* apache/trunk: MINOR: Add security considerations for remote JMX in Kafka docs (apache#6544) MINOR: Remove redundant access specifiers from metrics interfaces (apache#6527) MINOR: Correct KStream documentation (apache#6552) KAFKA-8013; Avoid underflow when reading a Struct from a partially correct buffer (apache#6340) KAFKA-8058: Fix ConnectClusterStateImpl.connectors() method (apache#6384) MINOR: Move common consumer tests out of abstract consumer class (apache#6548) KAFKA-8168; Add a generated ApiMessageType class KAFKA-7893; Refactor ConsumerBounceTest to reuse functionality from BaseConsumerTest (apache#6238) MINOR: Tighten up metadata upgrade test (apache#6531) KAFKA-8190; Don't update keystore modification time during validation (apache#6539) MINOR: Fixed a few warning in core and connects (apache#6545) KAFKA-7904; Add AtMinIsr partition metric and TopicCommand option (KIP-427) MINOR: fix throttling and status in ConnectionStressWorker KAFKA-8090: Use automatic RPC generation in ControlledShutdown KAFKA-6399: Remove Streams max.poll.interval override (apache#6509) KAFKA-8126: Flaky Test org.apache.kafka.connect.runtime.WorkerTest.testAddRemoveTask (apache#6475) HOTFIX: Update unit test for KIP-443 KAFKA-7190: KIP-443; Remove streams overrides on repartition topics (apache#6511) KAFKA-8183: Add retries to WorkerUtils#verifyTopics (apache#6532) KAFKA-8181: Removed Avro topic from TOC on kafka (apache#6529)
…apache#6539) Ensure that modification time is checked against the file used to create the SSLContext that is in-use so that SSLContext is updated whenever file is modified and a config update request is received. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>
2 participants
We currently store keystore file modification time when loading keystores in a
SecurityStoreinstance. When dynamically updating keystores without filename change, we compare the time at the last load against the current file modification time. But we load keystore for validation of dynamic configs and as a result, we dont recreate SSLContext when performing actual reconfiguration after the validation. We always create a newSecurityStoreinstance for reconfiguration of the store, so we only need to store file modification time when we construct the instance.Committer Checklist (excluded from commit message)