Hi again, Colin. I also just realized a couple of other incompatibilities
with the way kafka-configs works today that prevents --bootstrap-server
from being a drop-in replacement. This may or may not be a hard
requirement, but we should explicitly decide on these one way or the other.
One issue is that it is legal to list the SCRAM credentials for a single
user with kafka-configs (e.g. bin/kafka-configs.sh --zookeeper
localhost:2181 --describe --entity-type users --entity-name alice). The
current ListScramUsersRequest API does not support specifying an (optional)
user name, so it always returns all users' SCRAM credentials. We could
filter the lst on the client side, of course, but that seems inefficient.
The second issue is that the content of the data returned by the new API
does not match the content that kafka-configs currently returns. Here is
what the tool currently returns:
# add a user with a pair of SCRAM credentials
$ bin/kafka-configs.sh --zookeeper localhost:2181 --alter --add-config
'SCRAM-SHA-256=[iterations=8192,password=alice-secret],
SCRAM-SHA-512=[password=alice-secret]' --entity-type users --entity-name
alice
# list that user's SCRAM credentials
$ bin/kafka-configs.sh --zookeeper localhost:2181 --describe --entity-type
users --entity-name alice
Configs for user-principal 'alice' are
SCRAM-SHA-512=salt=MXNpcXViZmwxcmN4ZzhjeDkwam51c2I3Yw==,stored_key=V3IYnwwC5qjrzoRMyLrzgBstrJVDimQkFfAnJbVrG5mIEaJ/W6j5iV6xITF6HWvtsYrhGDIxsNSZbvVxAIck1w==,server_key=/BpX9JtxqzqyQS6NQcvyksN8Hs8XV65f2G7jx9PMy8Z9s22qCQrqCAtpvLPReYIMMDYRZ9/5x4aSlU/1rfLvVA==,iterations=4096,SCRAM-SHA-256=salt=N3g1eTB2aWUyeDlkYXZ5NDljM3h2aTd4dA==,stored_key=zzliLFJtNeQGY07lBAzzxM6jz0dEm5OkpaJUTfRrD+Y=,server_key=punFVKLCKcZymuRCqh6f6Gjp+VU8ZE3qd8iTboMqHbA=,iterations=8192
The API as currently defined only returns the number of iterations. I
would like to confirm that this particular lack of drop-in compatibility is
acceptable.
Even if the difference in content is acceptable, I think the inability to
list a single user is probably something we should fix, and the previous
issue I raised about kafka-configs being able to specify alterations and
deletions simultaneously also still stands as something we need to decide
about -- perhaps drop-in compatibility is not a requirement given the
content difference, in which case we could make it an error to specify both
alterations and deletions when using --bootstrap-server.
I think there are some issues with the response JSO as well. Here is what
I think the response JSON should be for listing users (assuming
ThrottleTimeMs belongs there, which it may not):
{
"apiKey": 50,
"type": "response",
"name": "ListScramUsersResponse",
"validVersions": "0",
"flexibleVersions": "0+",
"fields": [
{ "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
"about": "The duration in milliseconds for which the request was
throttled due to a quota violation, or zero if the request did not
violate any quota." },
{ "name": "Error", "type": "int16", "versions": "0+",
"about": "The message-level error code." },
{ "name": "ErrorMessage", "type": "string", "versions": "0+",
"nullableVersions": "0+",
"about": "The message-level error message." },
{ "name": "Users", "type": "[]ScramUser", "versions": "0+",
"about": "The SCRAM users.", "fields": [
{ "name": "Name", "type": "string", "versions": "0+",
"about": "The user name." },
{ "name": "MechanismInfos", "type": "[]ScramUserMechanismInfo",
"versions": "0+",
"about": "The mechanism credentials", "fields": [
{ "name": "Mechanism", "type": "int8", "versions": "0+",
"about": "The SCRAM mechanism." },
{ "name": "Iterations", "type": "int32", "versions": "0+",
"about": "The number of iterations used in the SCRAM mechanism." }]}
]}
]
}
Also, on the delete request that you added, assuming we keep it, there
is no way to specify which credential to delete -- it currently
deletes everything. The kafka-configs tool requires the specification
of which credential to delete. For example:
$ bin/kafka-configs.sh --zookeeper localhost:2181 --alter
--delete-config SCRAM-SHA-512 --entity-type users --entity-name alice
$ bin/kafka-configs.sh --zookeeper localhost:2181 --describe
--entity-type users --entity-name alice
Configs for user-principal 'alice' are
SCRAM-SHA-256=salt=N3g1eTB2aWUyeDlkYXZ5NDljM3h2aTd4dA==,stored_key=zzliLFJtNeQGY07lBAzzxM6jz0dEm5OkpaJUTfRrD+Y=,server_key=punFVKLCKcZymuRCqh6f6Gjp+VU8ZE3qd8iTboMqHbA=,iterations=8192
Ron
On Mon, Jul 13, 2020 at 4:21 PM Ron Dagostino <rndg...@gmail.com> wrote:
> 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
>>
>
