Hi Colin. I wanted to explicitly identify a side-effect that I think derives from having deletions separated out from the AlterScramUsersRequest and put in their own DeleteScramUsersRequest. The command line invocation of kafka-configs can specify alterations and deletions simultaneously: it is entirely legal for that tool to accept and process both --add-config and --delete-config (the current code removes any keys from the added configs that are also supplied in the deleted configs, it grabs the currently-defined keys, deletes the keys to be deleted, adds the ones to be added, and then sets the JSON for the user accordingly). If we split these two operations into separate APIs then an invocation of kafka-configs that specifies both operations can't complete atomically and could possibly have one of them succeed but the other fail. I am wondering if splitting the deletions out into a separate API is acceptable given this possibility, and if so, what the behavior should be. Maybe the kafka-configs command would have to prevent both from being specified simultaneously when --bootstrap-server is used. That would create an inconsistency with how the tool works with --zookeeper, meaning it is conceivable that switching over to --bootstrap-server would not necessarily be a drop-in replacement. Am I missing/misunderstanding something? Thoughts?
Also, separately, should the responses include a ThrottleTimeMs field? I believe so but would like to confirm. Ron On Mon, Jul 13, 2020 at 3:44 PM David Arthur <mum...@gmail.com> wrote: > Thanks for the clarification, Colin. +1 binding from me > > -David > > On Mon, Jul 13, 2020 at 3:40 PM Colin McCabe <cmcc...@apache.org> wrote: > > > Thanks, Boyang. Fixed. > > > > best, > > Colin > > > > On Mon, Jul 13, 2020, at 08:43, Boyang Chen wrote: > > > Thanks for the update Colin. One nit comment to fix the RPC type > > > for AlterScramUsersRequest as: > > > "apiKey": 51, > > > "type": "request", > > > "name": "AlterScramUsersRequest", > > > Other than that, +1 (binding) from me. > > > > > > > > > On Mon, Jul 13, 2020 at 8:38 AM Colin McCabe <cmcc...@apache.org> > wrote: > > > > > > > Hi David, > > > > > > > > The API is for clients. Brokers will still listen to ZooKeeper to > load > > > > the SCRAM information. > > > > > > > > best, > > > > Colin > > > > > > > > > > > > On Mon, Jul 13, 2020, at 08:30, David Arthur wrote: > > > > > Thanks for the KIP, Colin. The new RPCs look good to me, just one > > > > question: > > > > > since we don't return the password info through the RPC, how will > > brokers > > > > > load this info? (I'm presuming that they need it to configure > > > > > authentication) > > > > > > > > > > -David > > > > > > > > > > On Mon, Jul 13, 2020 at 10:57 AM Colin McCabe <cmcc...@apache.org> > > > > wrote: > > > > > > > > > > > On Fri, Jul 10, 2020, at 10:55, Boyang Chen wrote: > > > > > > > Hey Colin, thanks for the KIP. One question I have about > > > > AlterScramUsers > > > > > > > RPC is whether we could consolidate the deletion list and > > alteration > > > > > > list, > > > > > > > since in response we only have a single list of results. The > > further > > > > > > > benefit is to reduce unintentional duplicate entries for both > > > > deletion > > > > > > and > > > > > > > alteration, which makes the broker side handling logic easier. > > > > Another > > > > > > > alternative is to add DeleteScramUsers RPC to align what we > > currently > > > > > > have > > > > > > > with other user provided data such as delegation tokens > (create, > > > > change, > > > > > > > delete). > > > > > > > > > > > > > > > > > > > Hi Boyang, > > > > > > > > > > > > It can't really be consolidated without some awkwardness. It's > > > > probably > > > > > > better just to create a DeleteScramUsers function and RPC. I've > > > > changed > > > > > > the KIP. > > > > > > > > > > > > > > > > > > > > For my own education, the salt will be automatically generated > > by the > > > > > > admin > > > > > > > client when we send the SCRAM requests correct? > > > > > > > > > > > > > > > > > > > Yes, the client generates the salt before sending the request. > > > > > > > > > > > > best, > > > > > > Colin > > > > > > > > > > > > > Best, > > > > > > > Boyang > > > > > > > > > > > > > > On Fri, Jul 10, 2020 at 8:10 AM Rajini Sivaram < > > > > rajinisiva...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > +1 (binding) > > > > > > > > > > > > > > > > Thanks for the KIP, Colin! > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Jul 9, 2020 at 8:49 PM Colin McCabe < > > cmcc...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > I'd like to call a vote for KIP-554: Add a broker-side > SCRAM > > > > > > > > configuration > > > > > > > > > API. The KIP is here: > > > > https://cwiki.apache.org/confluence/x/ihERCQ > > > > > > > > > > > > > > > > > > The previous discussion thread is here: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://lists.apache.org/thread.html/r69bdc65bdf58f5576944a551ff249d759073ecbf5daa441cff680ab0%40%3Cdev.kafka.apache.org%3E > > > > > > > > > > > > > > > > > > best, > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > David Arthur > > > > > > > > > > > > > > > > > -- > David Arthur >
