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