Hi Chris, About the worker id: makes sense. I changed the wording, but kept it listed as it is a change compared to existing MM2 code. Updated the KIP accordingly.
About the REST server configurations: again, I agree, it should be generalized. But I'm not sure about those exceptions you listed, as all of them make sense in MM2 as well. For example, security related rest extensions could work with MM2 as well. Admin listeners are also useful, as they would allow the same admin operations for MM2 nodes. Am I mistaken here? Updated the KIP without mentioning exceptions for now. I think yes, the lazy config resolution should be unconditional. Even if we don't consider the distributed mode of MM2, the eager resolution does not allow using sensitive configs in the mm2 properties, as they will be written by value into the config topic. I didn't really consider this as breaking change, but I might be wrong. Enable flag property name: also makes sense, updated the KIP. Thanks a lot! Daniel Chris Egerton <chr...@aiven.io.invalid> ezt írta (időpont: 2022. szept. 20., K, 20:29): > Hi Daniel, > > Looking pretty good! Regarding the worker ID generation--apologies, I was > unclear with my question. I was wondering if we had to adopt any special > logic at all for MM2, or if we could use the same logic that vanilla Kafka > Connect does in distributed mode, where the worker ID is its advertised URL > (e.g., "connect:8083" or "localhost:25565"). Unless I'm mistaken, this > should allow MM2 nodes to be identified unambiguously. Do you think it > makes sense to follow this strategy? > > Now that the details on the new REST interface have been fleshed out, I'm > also wondering if we want to add support for the " > rest.advertised.host.name", > "rest.advertised.port", and "rest.advertised.listener" properties, which > are vital for being able to run Kafka Connect in distributed mode from > within containers. In fact, I'm wondering if we can generalize some of the > specification in the KIP around which REST properties are accepted by > stating that all REST-related properties that are available with vanilla > Kafka Connect will be supported for dedicated MM2 nodes when > "mm.enable.rest" is set to "true", except for ones related to the > public-facing REST API like "rest.extension.classes", "admin.listeners", > and "admin.listeners.https.*". > > One other thought--is the lazy evaluation of config provider references > going to take place unconditionally, or only when the internal REST API is > enabled on a worker? > > Finally, do you think we could change "mm.enable.rest" to > "mm.enable.internal.rest"? That way, if we want to introduce a > public-facing REST API later on, we can use "mm.enable.rest" as the name > for that property. > > Cheers, > > Chris > > On Fri, Sep 16, 2022 at 9:28 AM Dániel Urbán <urb.dani...@gmail.com> > wrote: > > > Hi Chris, > > > > I went through the KIP and updated it based on our discussion. I think > your > > suggestions simplified (and shortened) the KIP significantly. > > > > Thanks, > > Daniel > > > > Dániel Urbán <dur...@cloudera.com.invalid> ezt írta (időpont: 2022. > szept. > > 16., P, 15:15): > > > > > Hi Chris, > > > > > > 1. For the REST-server-per-flow setup, it made sense to introduce some > > > simplified configuration. With a single REST server, it doesn't make > > sense > > > anymore, I'm removing it from the KIP. > > > 2. I think that changing the worker ID generation still makes sense, > > > otherwise there is no way to differentiate between the MM2 instances. > > > > > > Thanks, > > > Daniel > > > > > > On Wed, Aug 31, 2022 at 8:39 PM Chris Egerton <chr...@aiven.io.invalid > > > > > wrote: > > > > > > > Hi Daniel, > > > > > > > > I've taken a look at the KIP in detail. Here are my complete thoughts > > > > (minus the aforementioned sections that may be affected by changes to > > an > > > > internal-only REST API): > > > > > > > > 1. Why introduce new mm.host.name and mm.rest.protocol properties > > > instead > > > > of using the properties that are already used by Kafka Connect: > > > listeners, > > > > rest.advertised.host.name, rest.advertised.host.port, and > > > > rest.advertised.listener? We used to have the rest.host.name and > > > rest.port > > > > properties in Connect but deprecated and eventually removed them in > > favor > > > > of the listeners property in KIP-208 [1]; I'm hoping we can keep > things > > > as > > > > similar as possible between MM2 and Connect in order to make it > easier > > > for > > > > users to work with both. I'm also hoping that we can allow users to > > > > configure the port that their MM2 nodes listen on instead of > hardcoding > > > MM2 > > > > to bind to port 0. > > > > > > > > 2. Do we still need to change the worker IDs that get used in the > > status > > > > topic? > > > > > > > > Everything else looks good, or should change once the KIP is updated > > with > > > > the internal-only REST API alternative. > > > > > > > > Cheers, > > > > > > > > Chris > > > > > > > > [1] - > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-208%3A+Add+SSL+support+to+Kafka+Connect+REST+interface > > > > > > > > On Mon, Aug 29, 2022 at 1:55 PM Chris Egerton <chr...@aiven.io> > wrote: > > > > > > > > > Hi Daniel, > > > > > > > > > > Yeah, I think that's the way to go. Adding multiple servers for > each > > > > > herder seems like it'd be too much of a pain for users to > configure, > > > and > > > > if > > > > > we keep the API strictly internal for now, we shouldn't be painting > > > > > ourselves into too much of a corner if/when we decide to expose a > > > > > public-facing REST API for dedicated MM2 clusters. > > > > > > > > > > I plan to take a look at the rest of the KIP and provide a complete > > > > review > > > > > sometime this week; I'll hold off on commenting on anything that > > seems > > > > like > > > > > it'll be affected by switching to an internal-only REST API until > > those > > > > > changes are published, but should be able to review everything > else. > > > > > > > > > > Cheers, > > > > > > > > > > Chris > > > > > > > > > > On Mon, Aug 29, 2022 at 6:57 AM Dániel Urbán < > urb.dani...@gmail.com> > > > > > wrote: > > > > > > > > > >> Hi Chris, > > > > >> > > > > >> I understand your point, sounds good to me. > > > > >> So in short, we should opt for an internal-only API, and > preferably > > a > > > > >> single server solution. Is that right? > > > > >> > > > > >> Thanks > > > > >> Daniel > > > > >> > > > > >> Chris Egerton <chr...@aiven.io.invalid> ezt írta (időpont: 2022. > > aug. > > > > >> 26., > > > > >> P, 17:36): > > > > >> > > > > >> > Hi Daniel, > > > > >> > > > > > >> > Glad to hear from you! > > > > >> > > > > > >> > With regards to the stripped-down REST API alternative, I don't > > see > > > > how > > > > >> > this would prevent us from introducing the fully-fledged Connect > > > REST > > > > >> API, > > > > >> > or even an augmented variant of it, at some point down the road. > > If > > > we > > > > >> go > > > > >> > with the internal-only API now, and want to expand later, can't > we > > > > gate > > > > >> the > > > > >> > expansion behind a feature flag configuration property that by > > > default > > > > >> > disables the new feature? > > > > >> > > > > > >> > I'm also not sure that we'd ever want to expose the raw Connect > > REST > > > > API > > > > >> > for dedicated MM2 clusters. If people want that capability, they > > can > > > > >> > already spin up a vanilla Connect cluster and run as many MM2 > > > > >> connectors as > > > > >> > they'd like on it, and as of KIP-458 [1], it's even possible to > > use > > > a > > > > >> > single Connect cluster to replicate between any two Kafka > clusters > > > > >> instead > > > > >> > of only targeting the Kafka cluster that the vanilla Connect > > cluster > > > > >> > operates on top of. I do agree that it'd be great to be able to > > > > >> dynamically > > > > >> > adjust things like topic filters without having to restart a > > > dedicated > > > > >> MM2 > > > > >> > node; I'm just not sure that the vanilla Connect REST API is the > > > > >> > appropriate way to do that, especially since the exact > mechanisms > > > that > > > > >> make > > > > >> > a single Connect cluster viable for replicating across any two > > Kafka > > > > >> > clusters could be abused and cause a dedicated MM2 cluster to > > start > > > > >> writing > > > > >> > to a completely different Kafka cluster that's not even defined > in > > > its > > > > >> > config file. > > > > >> > > > > > >> > Finally, as far as security goes--since this is essentially a > bug > > > fix, > > > > >> I'm > > > > >> > inclined to make it as easy as possible for users to adopt it. > > MTLS > > > > is a > > > > >> > great start for securing a REST API, but it's not sufficient on > > its > > > > own > > > > >> > since anyone who could issue an authenticated REST request > against > > > the > > > > >> MM2 > > > > >> > cluster would still be able to make any changes they want (with > > the > > > > >> > exception of accessing internal endpoints, which were secured > with > > > > >> > KIP-507). If we were to bring up the fully-fledged Connect REST > > API, > > > > >> > cluster administrators would also likely have to add some kind > of > > > > >> > authorization layer to prevent people from using the REST API to > > > mess > > > > >> with > > > > >> > the configurations of the connectors that MM2 brought up. One > way > > of > > > > >> doing > > > > >> > that is to add a REST extension to your Connect cluster, but > > > > >> implementing > > > > >> > and configuring one in order to be able to run a multi-node MM2 > > > > cluster > > > > >> > without hitting this bug seems like too much work to be worth > it. > > > > >> > > > > > >> > I think if we had a better picture of what a REST API for > > dedicated > > > > MM2 > > > > >> > clusters would/should look like, then it would be easier to go > > along > > > > >> with > > > > >> > this, and we could even just add the feature flag in this KIP > > right > > > > now > > > > >> to > > > > >> > address any security concerns. My instinct would be to address > > this > > > > in a > > > > >> > follow-up KIP in order to reduce scope creep, though, and keep > > this > > > > KIP > > > > >> > focused on addressing the bug with multi-node dedicated MM2 > > > clusters. > > > > >> What > > > > >> > do you think? > > > > >> > > > > > >> > Cheers, > > > > >> > > > > > >> > Chris > > > > >> > > > > > >> > [1] - > > > > >> > > > > > >> > > > > > >> > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-458%3A+Connector+Client+Config+Override+Policy > > > > >> > > > > > >> > On Thu, Aug 25, 2022 at 3:55 AM Dániel Urbán < > > urb.dani...@gmail.com > > > > > > > > >> > wrote: > > > > >> > > > > > >> > > Hi Chris, > > > > >> > > > > > > >> > > Thanks for bringing this up again :) > > > > >> > > > > > > >> > > 1. I think that is reasonable, though I find the current state > > of > > > > MM2 > > > > >> to > > > > >> > be > > > > >> > > confusing. The current issue with the distributed mode is not > > > > >> documented > > > > >> > > properly, but maybe the logging will help a bit. > > > > >> > > > > > > >> > > 2. Going for an internal-only Connect REST version would lock > > MM2 > > > > out > > > > >> of > > > > >> > a > > > > >> > > path where the REST API can be used to dynamically reconfigure > > > > >> > > replications. For now, I agree, it would be easy to corrupt > the > > > > state > > > > >> of > > > > >> > > MM2 if someone wanted to use the properties and the REST at > the > > > same > > > > >> > time, > > > > >> > > but in the future, we might have a chance to introduce a > > different > > > > >> config > > > > >> > > mechanism, where only the cluster connections have to be > > specified > > > > in > > > > >> the > > > > >> > > properties file, and everything else can be configured through > > > REST > > > > >> > > (enabling replications, changing topic filters, etc.). Because > > of > > > > >> this, > > > > >> > I'm > > > > >> > > leaning towards a full Connect REST API. To avoid issues with > > > > >> conflicts > > > > >> > > between the props file and the REST, we could document > security > > > best > > > > >> > > practices (e.g. turn on basic auth or mTLS on the Connect REST > > to > > > > >> avoid > > > > >> > > unwanted interactions). > > > > >> > > > > > > >> > > 3. That is a good point, and I agree, a big plus for > motivation. > > > > >> > > > > > > >> > > I have a working version of this in which all flows spin up a > > > > >> dedicated > > > > >> > > Connect REST, but I can give other solutions a try, too. > > > > >> > > > > > > >> > > Thanks, > > > > >> > > Daniel > > > > >> > > > > > > >> > > Chris Egerton <chr...@aiven.io.invalid> ezt írta (időpont: > > 2022. > > > > aug. > > > > >> > 24., > > > > >> > > Sze, 17:46): > > > > >> > > > > > > >> > > > Hi Daniel, > > > > >> > > > > > > > >> > > > I'd like to resurface this KIP in case you're still > interested > > > in > > > > >> > > pursuing > > > > >> > > > it. I know it's been a while since you published it, and it > > > hasn't > > > > >> > > received > > > > >> > > > much attention, but I'm hoping we can give it a try now and > > > > finally > > > > >> put > > > > >> > > > this long-standing bug to rest. To that end, I have some > > > thoughts > > > > >> about > > > > >> > > the > > > > >> > > > proposal. This isn't a complete review, but I wanted to give > > > > enough > > > > >> to > > > > >> > > get > > > > >> > > > the ball rolling: > > > > >> > > > > > > > >> > > > 1. Some environments with firewalls or strict security > > policies > > > > may > > > > >> not > > > > >> > > be > > > > >> > > > able to bring up a REST server for each MM2 node. If we > decide > > > > that > > > > >> > we'd > > > > >> > > > like to use the Connect REST API (or even just parts of it) > to > > > > >> address > > > > >> > > this > > > > >> > > > bug with MM2, it does make sense to eventually make the > > > > >> availability of > > > > >> > > the > > > > >> > > > REST API a hard requirement for running MM2, but it might > be a > > > bit > > > > >> too > > > > >> > > > abrupt to do that all in a single release. What do you think > > > about > > > > >> > making > > > > >> > > > the REST API optional for now, but noting that it will > become > > > > >> required > > > > >> > > in a > > > > >> > > > later release (probably 4.0.0 or, if that's not enough time, > > > > >> 5.0.0)? We > > > > >> > > > could choose not to bring the REST server for any node whose > > > > >> > > configuration > > > > >> > > > doesn't explicitly opt into one, and maybe log a warning > > message > > > > on > > > > >> > > startup > > > > >> > > > if none is configured. In effect, we'd be marking the > current > > > mode > > > > >> (no > > > > >> > > REST > > > > >> > > > server) as deprecated. > > > > >> > > > > > > > >> > > > 2. I'm not sure that we should count out the "Creating an > > > > >> internal-only > > > > >> > > > derivation of the Connect REST API" rejected alternative. > > Right > > > > now, > > > > >> > the > > > > >> > > > single source of truth for the configuration of a MM2 > cluster > > > > >> (assuming > > > > >> > > > it's being run in dedicated mode, and not as a connector in > a > > > > >> vanilla > > > > >> > > > Connect cluster) is the configuration file used for the > > process. > > > > By > > > > >> > > > bringing up the REST API, we'd expose endpoints to modify > > > > connector > > > > >> > > > configurations, which would not only add complexity to the > > > > operation > > > > >> > of a > > > > >> > > > MM2 cluster, but even qualify as an attack vector for > > malicious > > > > >> > entities. > > > > >> > > > Thanks to KIP-507 we have some amount of security around the > > > > >> > > internal-only > > > > >> > > > endpoints used by the Connect framework, but for any public > > > > >> endpoints, > > > > >> > > the > > > > >> > > > Connect REST API doesn't come with any security out of the > > box. > > > > >> > > > > > > > >> > > > 3. Small point, but with support for exactly-once source > > > > connectors > > > > >> > > coming > > > > >> > > > out in 3.3.0, it's also worth noting that that's another > > feature > > > > >> that > > > > >> > > won't > > > > >> > > > work properly with multi-node MM2 clusters without adding a > > REST > > > > >> server > > > > >> > > for > > > > >> > > > each node (or some substitute that accomplishes the same > > goal). > > > I > > > > >> don't > > > > >> > > > think this will affect the direction of the design > discussion > > > too > > > > >> much, > > > > >> > > but > > > > >> > > > it does help strengthen the motivation. > > > > >> > > > > > > > >> > > > Cheers, > > > > >> > > > > > > > >> > > > Chris > > > > >> > > > > > > > >> > > > On 2021/02/18 15:57:36 Dániel Urbán wrote: > > > > >> > > > > Hello everyone, > > > > >> > > > > > > > > >> > > > > * Sorry, I meant KIP-710. > > > > >> > > > > > > > > >> > > > > Right now the MirrorMaker cluster is somewhat unreliable, > > and > > > > not > > > > >> > > > > supporting running in a cluster properly. I'd say that > > fixing > > > > this > > > > >> > > would > > > > >> > > > be > > > > >> > > > > a nice addition. > > > > >> > > > > Does anyone have some input on this? > > > > >> > > > > > > > > >> > > > > Thanks in advance > > > > >> > > > > Daniel > > > > >> > > > > > > > > >> > > > > Dániel Urbán <ur...@gmail.com> ezt írta (időpont: 2021. > > jan. > > > > >> 26., K, > > > > >> > > > > 15:56): > > > > >> > > > > > > > > >> > > > > > Hello everyone, > > > > >> > > > > > > > > > >> > > > > > I would like to start a discussion on KIP-709, which > > > addresses > > > > >> some > > > > >> > > > > > missing features in MM2 dedicated mode. > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-710%3A+Full+support+for+distributed+mode+in+dedicated+MirrorMaker+2.0+clusters > > > > >> > > > > > > > > > >> > > > > > Currently, the dedicated mode of MM2 does not fully > > support > > > > >> running > > > > >> > > in > > > > >> > > > a > > > > >> > > > > > cluster. The core issue is that the Connect REST Server > is > > > not > > > > >> > > included > > > > >> > > > in > > > > >> > > > > > the dedicated mode, which makes follower->leader > > > communication > > > > >> > > > impossible. > > > > >> > > > > > In some cases, this results in the cluster not being > able > > to > > > > >> react > > > > >> > to > > > > >> > > > > > dynamic configuration changes (e.g. dynamic topic filter > > > > >> changes). > > > > >> > > > > > Another smaller detail is that MM2 dedicated mode > eagerly > > > > >> resolves > > > > >> > > > config > > > > >> > > > > > provider references in the Connector configurations, > which > > > is > > > > >> > > > undesirable > > > > >> > > > > > and a breaking change compared to vanilla Connect. This > > can > > > > >> cause > > > > >> > an > > > > >> > > > issue > > > > >> > > > > > for example when there is an environment variable > > reference, > > > > >> which > > > > >> > > > contains > > > > >> > > > > > some host-specific information, like a file path. The > > leader > > > > >> > resolves > > > > >> > > > the > > > > >> > > > > > reference eagerly, and the resolved value is propagated > to > > > > other > > > > >> > MM2 > > > > >> > > > nodes > > > > >> > > > > > instead of the reference being resolved locally, > > separately > > > on > > > > >> each > > > > >> > > > node. > > > > >> > > > > > > > > > >> > > > > > The KIP addresses these by adding the Connect REST > Server > > to > > > > the > > > > >> > MM2 > > > > >> > > > > > dedicated mode for each replication flow, and postponing > > the > > > > >> config > > > > >> > > > > > provider reference resolution. > > > > >> > > > > > > > > > >> > > > > > Please discuss, I know this is a major change, but also > an > > > > >> > important > > > > >> > > > > > feature for MM2 users. > > > > >> > > > > > > > > > >> > > > > > Daniel > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > >
