KAFKA-9888: Copy connector configs before passing to REST extensions#8511
Merged
kkonstantine merged 4 commits intoapache:trunkfrom May 23, 2020
Merged
KAFKA-9888: Copy connector configs before passing to REST extensions#8511kkonstantine merged 4 commits intoapache:trunkfrom
kkonstantine merged 4 commits intoapache:trunkfrom
Conversation
Contributor
Author
|
@stan-confluent, @ncliang, @gharris1727 would any of you mind taking a look when you have a moment? |
ncliang
approved these changes
Apr 20, 2020
Contributor
Author
|
Thanks @ncliang! @kkonstantine, @rhauch would one of you mind taking a look at this when you get a chance? |
…/health/ConnectClusterStateImpl.java Co-authored-by: Konstantine Karantasis <konstantine@confluent.io>
Contributor
Author
|
Thanks @kkonstantine, I've addressed both of your comments. Ready for another round when you have time. |
…/health/ConnectClusterStateImpl.java
Contributor
|
ok to test |
Contributor
|
2/3 build green, yet a failure that seems unrelated but happened during a connect IT. Given that I'll rerun once more. |
Contributor
|
retest this please |
1 similar comment
Contributor
|
retest this please |
Contributor
|
jdk8: success |
kkonstantine
pushed a commit
that referenced
this pull request
May 23, 2020
…8511) The changes made in KIP-454 involved adding a `connectorConfig` method to the ConnectClusterState interface that REST extensions could use to query the worker for the configuration of a given connector. The implementation for this method returns the Java `Map` that's stored in the worker's view of the config topic (when running in distributed mode). No copying is performed, which causes mutations of that `Map` to persist across invocations of `connectorConfig` and, even worse, propagate to the worker when, e.g., starting a connector. In this commit the map is copied before it's returned to REST extensions. An existing unit test is modified to ensure that REST extensions receive a copy of the connector config, not the original. Reviewers: Nigel Liang <nigel@nigelliang.com>, Konstantine Karantasis <konstantine@confluent.io>
kkonstantine
pushed a commit
that referenced
this pull request
May 23, 2020
…8511) The changes made in KIP-454 involved adding a `connectorConfig` method to the ConnectClusterState interface that REST extensions could use to query the worker for the configuration of a given connector. The implementation for this method returns the Java `Map` that's stored in the worker's view of the config topic (when running in distributed mode). No copying is performed, which causes mutations of that `Map` to persist across invocations of `connectorConfig` and, even worse, propagate to the worker when, e.g., starting a connector. In this commit the map is copied before it's returned to REST extensions. An existing unit test is modified to ensure that REST extensions receive a copy of the connector config, not the original. Reviewers: Nigel Liang <nigel@nigelliang.com>, Konstantine Karantasis <konstantine@confluent.io>
kkonstantine
pushed a commit
that referenced
this pull request
May 23, 2020
…8511) The changes made in KIP-454 involved adding a `connectorConfig` method to the ConnectClusterState interface that REST extensions could use to query the worker for the configuration of a given connector. The implementation for this method returns the Java `Map` that's stored in the worker's view of the config topic (when running in distributed mode). No copying is performed, which causes mutations of that `Map` to persist across invocations of `connectorConfig` and, even worse, propagate to the worker when, e.g., starting a connector. In this commit the map is copied before it's returned to REST extensions. An existing unit test is modified to ensure that REST extensions receive a copy of the connector config, not the original. Reviewers: Nigel Liang <nigel@nigelliang.com>, Konstantine Karantasis <konstantine@confluent.io>
Kvicii
pushed a commit
to Kvicii/kafka
that referenced
this pull request
May 24, 2020
* 'trunk' of github.com:apache/kafka: KAFKA-9888: Copy connector configs before passing to REST extensions (apache#8511) KAFKA-9931: Implement KIP-605 to expand support for Connect worker internal topic configurations (apache#8654) KAFKA-6145: Add unit tests for assignments of only stateless tasks (apache#8713) MINOR: Fix join group request timeout lower bound (apache#8702) MINOR: Improve security documentation for Kafka Streams apache#8710 KAFKA-6145: KIP-441: Enforce Standby Task Stickiness (apache#8696) KAFKA-10003: Mark KStream.through() as deprecated and update Scala API (apache#8679)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira
The changes made in KIP-454 involved adding a
connectorConfigmethod to the ConnectClusterState interface that REST extensions could use to query the worker for the configuration of a given connector. The implementation for this method returns the JavaMapthat's stored in the worker's view of the config topic (when running in distributed mode). No copying is performed, which causes mutations of thatMapobject to persist across invocations ofconnectorConfigand, even worse, propagate to the worker when, e.g., starting a connector.The changes here just cause the framework to copy that map before sending it to REST extensions, and alter a comment in
KafkaConfigBackingStorethat addresses the mutability of the snapshots that it provides to warn against changes that may lead to bugs like this one.An existing unit test is modified to ensure that REST extensions receive a copy of the connector config, not the original.
Committer Checklist (excluded from commit message)