Hi everyone. Multiple changes came up during the discussion of the PR for
KIP-554 (https://github.com/apache/kafka/pull/9032). I will update the KIP
soon (likely tomorrow) but wanted to send notification to this email thread
in case anyone wishes to discuss the changes. They are as follows:
1. We removed -1 as a valid iterations value in the code. It was described
as meaning "use the server-side default" but it turns out that the client
side must know the number of iterations to use when salting the password,
and there is no way for it to know any such "server-side default" value.
It can't be defined in the client code because the server may be running a
different version of that code.
2. We added a restriction to not allow users who authenticated using
delegation tokens to create or update user SCRAM credentials. We don't
allow such authenticated users to create new tokens, and it would be odd if
they could create a new user or change the password of the user for the
token.
3. We relaxed the requirement that Describe requests be sent to the
controller -- this is no longer necessary. Alter stil must go to the
controller, though -- but not Describe.
4. We expanded the Describe response to include user-level
errorCode/errorMessage fields. The KIP only specified a top-level error
corresponding to the entire request, but we need the flexibility to fail a
particular user (as is already possible with Alter). For example, there
may be corrupted data that prevents a user from being described (and we
decided to fail a user if it is specified multiple times as mentioned
previously). If a describe-all request is made and there is only a
top-level error then nothing would be described because the entire request
will fail. There would also be no place to specify the user(s) that caused
the failure aside from the free-form error message.
5. We settled upon the following method signatures for
DescribeUserScramCredentialsResult:
/**
*
* @return a future for the results of all described users with map keys
(one per user) being consistent with the
* contents of the list returned by {@link #users()}. The future will
complete successfully only if all such user
* descriptions complete successfully.
*/
public KafkaFuture<Map<String, UserScramCredentialsDescription>> all()
/**
*
* @return a future indicating the distinct users that meet the request
criteria and that have at least one
* credential. The future will not complete successfully if the user is
not authorized to perform the describe
* operation; otherwise, it will complete successfully as long as the list
of users with credentials can be
* successfully determined within some hard-coded timeout period. Note that
the returned list will not include users
* that do not exist/have no credentials: a request to describe an explicit
list of users, none of which existed/had
* a credential, will result in a future that returns an empty list being
returned here. A returned list will
* include users that have a credential but that could not be described.
*/
public KafkaFuture<List<String>> users() {
/**
*
* @param userName the name of the user description being requested
* @return a future indicating the description results for the given user.
The future will complete exceptionally if
* the future returned by {@link #users()} completes exceptionally. Note
that if the given user does not exist in
* the list of described users then the returned future will complete
exceptionally with
* {@link org.apache.kafka.common.errors.ResourceNotFoundException}.
*/
public KafkaFuture<UserScramCredentialsDescription> description(String
userName)
6. The KIP was silent on the issue of what to do if the client asks to
Describe a user that has no credentials. Should we ignore the request for
that user and not include it in the results or treat it as an error? We
decided to treat it as an error in the response so that we have the
flexibility for the client to know that this is what happened, but we
decided to have the DescribeUserScramCredentialsResult skip over this and
act as though the user was not described for the purposes of its users()
and al() methods. Invoking description() for a user that either was not
originally requested or that had not credentials will result in a future
that completes exceptionally.
7. The KIP was silent on what to do if a single request is sent to describe
the same user multiple times -- treat it as though the user was specified
only once, or treat it as a failure? We decided to treat it as a failure
for that user.
8. We added 3 new Errors/Exceptions as follows:
a) Errors.RESOURCE_NOT_FOUND / ResourceNotFoundException: The server
returns this error code when a user that has no credentials is described.
The exception is thrown if the client calls get() on a future returned by a
call to describe(user) any user that was not described.
b) Errors.DUPLICATE_RESOURCE / DuplicateResourceException: If an attempt
is made to alter or describe a user twice in the same request.
c) Errors.UNACCEPTABLE_CREDENTIAL / UncceptableCredentialException: if an
attempt is made to create a credential for the empty username or with too
few or too many iterations (and we set a hard-coded limit of 16384 for
iterations)
9. We return the existing UNSUPPORTED_SASL_MECHANISM error if the broker
does not recognize the requested mechanism.
I will update the KIP tomorrow. Please do not hesitate to respond if you
feel that any of these changes need further discussion.
Thanks,
Ron
On Mon, Aug 3, 2020 at 8:23 PM Colin McCabe <cmcc...@apache.org> wrote:
>
> Thanks, Ron. Sounds good.
>
> best,
> Colin
>
> On Tue, Jul 28, 2020, at 15:26, Ron Dagostino wrote:
> > Hi everyone. I just wanted to notify that while implementing this I
> > discovered that we had declared the ScramMechanism enum to have the
values
> > HMAC_SHA_{256,512} instead of SCRAM_SHA_{256,512}. I believe Rajini had
> > indicated that this should be changed back on May 7th, and the change
makes
> > sense to me given that these are the formal SASL SCRAM mechanism names
> > (with dash replaced by underscore so they are valid Java identifiers).
I
> > have updated the KIP. Let me know if you have any questions/concerns,
> > otherwise we can assume this change is acceptable.
> >
> > Ron
> >
> > On Tue, Jul 21, 2020 at 1:57 PM Colin McCabe <cmcc...@apache.org> wrote:
> >
> > > Hi all,
> > >
> > > With binding +1s from Rajini Sivaram, David Arthur, and Boyang Chen,
and
> > > non-binding +1s from Tom Bentley, the vote passes.
> > >
> > > Thanks to everyone who commented and helped to improve the proposal,
> > > especially Ron Dagostino, David, and Boyang.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Thu, Jul 16, 2020, at 16:02, Ron Dagostino wrote:
> > > > Hi Colin. I updated the KIP with various renames. I've also
created a
> > > > draft PR at https://github.com/apache/kafka/pull/9032 that still
needs a
> > > > bunch of real implementation but nonetheless represents the renames
in
> > > code.
> > > >
> > > > The biggest changes are that there are now derived classes public
class
> > > > UserScramCredentialAdditionUpsertion and public class
> > > > UserScramCredentialDeletion. I don't know what the reaction to the
use
> > > of
> > > > the term "upsertion" will be, but that's the best thing I could
come up
> > > > with to reflect that these requests are "upserts" (update if there,
> > > > otherwise insert). It was referred to as an "Addition" before,
which I
> > > > felt was not technically correct. If you diff the most recent two
> > > versions
> > > > of the KIP it diffs pretty cleanly and makes the changes pretty
apparent.
> > > >
> > > > Ron
> > > >
> > > >
> > > > On Thu, Jul 16, 2020 at 11:38 AM Colin McCabe <cmcc...@apache.org>
> > > wrote:
> > > >
> > > > > On Thu, Jul 16, 2020, at 08:06, Ron Dagostino wrote:
> > > > > > Thanks, Colin. The standard "about" message for ThrottleTimeMs
seems
> > > > > > to be "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."
> > > > > > as opposed to "The time spent waiting for quota." Should we
adjust to
> > > > > > match the typical definition?
> > > > > >
> > > > >
> > > > > Hi Ron,
> > > > >
> > > > > Good point. Let's keep the "about" text consistent. I changed
it.
> > > > >
> > > > > >
> > > > > > I'm wondering if describing Scram credentials should require
READ
> > > > > privilege
> > > > > > rather than ALTER on the cluster? Altering SCRAM credentials
of
> > > course
> > > > > > requires ALTER privilege, and I can see the argument for
requiring
> > > ALTER
> > > > > > privilege to describe them as well, but it did catch my eye as
> > > something
> > > > > > worth questioning/confirming.
> > > > > >
> > > > >
> > > > > Also a good point. I spoke with Rajini about this offline, and
she
> > > > > pointed out that we can already see user names in ACLs if we have
> > > DESCRIBE
> > > > > on CLUSTER. So it should be fine to have describeScramUsers
require
> > > > > DESCRIBE on CLUSTER as well.
> > > > >
> > > > > >
> > > > > > I'm also now thinking that "UNKNOWN" shouldn't be listed in the
> > > > > > ScramMechanism enum. I thought maybe it was a catch-all so we
will
> > > > > always
> > > > > > be able to deserialize something regardless of what key actually
> > > appears,
> > > > > > but I just realized that SCRAM credentials and Client Quotas are
> > > mixed
> > > > > > together in the same JSON, so it will be up to the
corresponding API
> > > to
> > > > > > ignore what it doesn't recognize -- i.e. if both client quotas
and
> > > SCRAM
> > > > > > credentials are defined for a user, then invoking
> > > DescribeClientQuotas
> > > > > must
> > > > > > only describe the quota configs and invoking DescribeScramUsers
must
> > > only
> > > > > > describe the SCRAM configs.
> > > > > >
> > > > >
> > > > > The reason to have the UNKNOWN enum is so that we can add new
SCRAM
> > > > > mechanisms in the future. If we don't have it, then we're
basically
> > > saying
> > > > > we can never add new mechanisms.
> > > > >
> > > > > I agree that the decision to put SCRAM users under the same ZK
path as
> > > > > client quotas makes this more complex than we'd like it to be,
but all
> > > is
> > > > > not lost. For one thing, we could always just add a new ZK path
for
> > > SCRAM
> > > > > users in the future if we really need to. With a new path we
wouldn't
> > > have
> > > > > to worry about namespace collisions. For another thing, in the
> > > > > post-KIP-500 world this won't be an issue.
> > > > >
> > > > > In the short term, a simpler solution might work here. For
example,
> > > can
> > > > > we just assume that any key that starts with "SCRAM-" is not a
quota,
> > > but a
> > > > > scram user? (Or look at some other aspect of the key).
> > > > >
> > > > > >
> > > > > > Also, note that invoking kafka-configs.sh
> > > > > > --bootstrap-server ... --entity-type user --describe will
require the
> > > > > > invocation of two separate APIs -- one to describe quotas and
one to
> > > > > > describe SCRAM credentials; I don't think this is a problem,
but I
> > > did
> > > > > want
> > > > > > to call it out explicitly.
> > > > > >
> > > > >
> > > > > That should be OK.
> > > > >
> > > > > >
> > > > > > Finally, there is a lack of consistency regarding how we name
things.
> > > > > For
> > > > > > example, we are calling these APIs {Describe,Alter}ScramUsers
and we
> > > have
> > > > > > declared the classes {Describe,Alter}ScramUserOptions, which
matches
> > > up
> > > > > > fine. We also have public class DescribeScramUsersResult,
which also
> > > > > > matches. But then we have public class
AlterScramCredentialsResult,
> > > > > > interface ScramCredentialAlteration, public class
> > > > > ScramCredentialAddition,
> > > > > > and public class ScramCredentialDeletion, none of which match
up in
> > > terms
> > > > > > of consistency of naming. I wonder if we should always use
> > > > > > "UserScramCredential" everywhere since that is technically what
these
> > > > > API's
> > > > > > are about: describing/altering Users' SCRAM credentials. So
the APis
> > > > > would
> > > > > > be {Describe,Alter}UserScramCredentials, and everything else
that is
> > > > > > publiuc that now refers inconsistently to either ScramUsers or
> > > > > > ScramCredential would instead refer to UserScramCredentials
> > > (sometimes
> > > > > > singular rather than plural if warranted). For example: public
> > > class {
> > > > > > Describe,Alter}UserScramCredentialsResult, interface User
> > > > > > ScramCredentialAlteration, public class
UserScramCredentialAddition,
> > > and
> > > > > > public class UserScramCredentialDeletion
> > > > > >
> > > > >
> > > > > Yeah, there is a bit of a mismatch between "credentials" and
"users."
> > > > > Really, these APIs are about credentials, not users. So I agree
--
> > > let's
> > > > > rename it.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > > >
> > > > > > On Wed, Jul 15, 2020 at 5:53 PM Colin McCabe <cmcc...@apache.org
>
> > > wrote:
> > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > Thanks, everyone, for reviewing.
> > > > > > >
> > > > > > > Since we made a few changes to the RPCs in the last few days,
I'm
> > > > > going to
> > > > > > > extend the vote until Monday and close it out then if it looks
> > > good.
> > > > > > >
> > > > > > > best,
> > > > > > > Colin
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Jul 15, 2020, at 14:47, Colin McCabe wrote:
> > > > > > > > On Tue, Jul 14, 2020, at 16:23, Ron Dagostino wrote:
> > > > > > > > > Thanks, Colin.
> > > > > > > > >
> > > > > > > > > DescribeScramUsersResponse returns a list of ScramUser
> > > instances,
> > > > > which
> > > > > > > > > makes sense, but then each of the ScramUser instances
only has
> > > a
> > > > > single
> > > > > > > > > ScramUserMechanismInfo instance. I believe it needs a
List of
> > > > > those?
> > > > > > > >
> > > > > > > > Hi Ron,
> > > > > > > >
> > > > > > > > Sorry, that was a typo in the response JSON. Fixed.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Also, ScramUserMechanismInfo probably needs a better
"about"
> > > value
> > > > > (it
> > > > > > > > > currently says "The user name.")
> > > > > > > > >
> > > > > > > >
> > > > > > > > Also fixed :)
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Should both responses include ThrottleTimeMs fields?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Good call. I added this.
> > > > > > > >
> > > > > > > > best,
> > > > > > > > Colin
> > > > > > > >
> > > > > > > > >
> > > > > > > > > I haven't looked at the AlterScramUsers stuff yet; I'll
look at
> > > > > that in
> > > > > > > > > detail tomorrow.
> > > > > > > > >
> > > > > > > > > Ron
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Tue, Jul 14, 2020 at 4:11 PM Colin McCabe <
> > > cmcc...@apache.org>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > On Tue, Jul 14, 2020, at 07:57, Ron Dagostino wrote:
> > > > > > > > > > > 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.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Hi Ron,
> > > > > > > > > >
> > > > > > > > > > Yes, I think we should allow listing just a particular
scram
> > > > > user or
> > > > > > > > > > users. I will change this to "describe" and add a list
of
> > > user
> > > > > > > names which
> > > > > > > > > > can be supplied, or null to list all.
> > > > > > > > > >
> > > > > > > > > > (more responses below)
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > 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.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Yes, this is expected. The argument was that returning
the
> > > > > salted
> > > > > > > > > > password and hash was not secure, so we elected not to
return
> > > > > this
> > > > > > > > > > information.
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > 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 you brought up some very good points. I got
rid of
> > > the
> > > > > > > delete
> > > > > > > > > > operation and replaced it with an Alter that can remove
> > > > > individual
> > > > > > > > > > credentials as needed. We certainly need this, given
what
> > > the
> > > > > > > command line
> > > > > > > > > > tool needs to be able to do.
> > > > > > > > > >
> > > > > > > > > > Thanks for the comments. Take a look and see if the
latest
> > > > > changes
> > > > > > > fix
> > > > > > > > > > it...
> > > > > > > > > >
> > > > > > > > > > best,
> > > > > > > > > > Colin
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > 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
> > > > > > > > > > > >>
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
