From dev-return-107111-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Tue Sep 3 12:50:38 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id EBEEF180637 for ; Tue, 3 Sep 2019 14:50:37 +0200 (CEST) Received: (qmail 80262 invoked by uid 500); 3 Sep 2019 14:42:41 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 80247 invoked by uid 99); 3 Sep 2019 14:42:41 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 03 Sep 2019 14:42:41 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id DED79C1CFA for ; Tue, 3 Sep 2019 12:50:35 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.801 X-Spam-Level: * X-Spam-Status: No, score=1.801 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-he-de.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id Nt34kwvzmMJf for ; Tue, 3 Sep 2019 12:50:30 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2607:f8b0:4864:20::d2f; helo=mail-io1-xd2f.google.com; envelope-from=rajinisivaram@gmail.com; receiver= Received: from mail-io1-xd2f.google.com (mail-io1-xd2f.google.com [IPv6:2607:f8b0:4864:20::d2f]) by mx1-he-de.apache.org (ASF Mail Server at mx1-he-de.apache.org) with ESMTPS id 27C5F7D51C for ; Tue, 3 Sep 2019 12:50:29 +0000 (UTC) Received: by mail-io1-xd2f.google.com with SMTP id h144so20192241iof.7 for ; Tue, 03 Sep 2019 05:50:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=lVjXejATY52nviPC9Rvho6+hjgp33RDtSuPdvvGHlF4=; b=ZZWw0bdE/w+ESx06W6LZnogeuWO35LmjDR9Ld1FE5LUeCkzwOXJzT51GPL3r3Qmbj1 Ay7UxSQJwgZTHVHqm0canlud8ierUuQ0VMXR1XmD3+NFdKQvVgGmc7iRgdObMzrzoFCF IyBFGWorJbohLeSvpVTjF6hcNpLDm7+fSYfKaEPhSbCjmO2473Mo0Wdi4KuweCl4zLut lYoyslphaz6Nb5C7SmFl8fkGetMfAVqKVYl9Y/T51vPDAoSOfkM+OwIK+a4RQDBk275w 9FEVDn84DMwDpE0jAgdaZ63Sy0T4gp2RF9zYzfonbeKO3Q6SXc3WkBLeKUCY5PNfhSlO lYow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=lVjXejATY52nviPC9Rvho6+hjgp33RDtSuPdvvGHlF4=; b=gFLZrQB5USw2I0RLgXciZhGynZ+Mr4ZmMI3XZkliBH46UJvyfrcyK3zVHeNhNK+cu1 2x3ARrN7ZLTNq8BAKWuQUgXT6Djve8WWNKAsJ/wrfCc0yXbBzHKkFADM46W0WiZpTb8P uGm9jhVYm/LNUjgDvKf+pybmlUBX3Ragm22KeTpY37llPBjiEOLbV6boCWvoSe2ihOyy 7eQZFFQ+3V7yOfpibxSgCCQ1JT3wH8y1e6Fx4ZK9EoVmfG84WBNr1q1wcklasunuNQoM CwYcI2Iol+EViPZZj+6hQrrKu/UKBrbdiS2iUkTWh7DVS3K+Wfvix+nfOPpijf0cgi8u nrVA== X-Gm-Message-State: APjAAAUzLeDlMV2rg723mDSppigxaYXjtX0V2xJSe896Bxtb4Vr/OPTf dTm+rJuLkCAYI0MZujim30JniVnWkaCDY1F2mvsdCvLUP64= X-Google-Smtp-Source: APXvYqx61nG1IUln0iwH0Wi3ypGanGTH1UFE6PSqhaUPUznI34PEnIVJ3gWsyQZAd8JBW32EcfL9YRVAHRoIvmZcGyY= X-Received: by 2002:a05:6602:157:: with SMTP id v23mr42641194iot.264.1567515027377; Tue, 03 Sep 2019 05:50:27 -0700 (PDT) MIME-Version: 1.0 References: <48CB3DB7-709A-4479-8EDB-A4D95B26728E@apache.org> <2FB0D77D-5E49-4232-B7DE-BD83780C9CFD@apache.org> In-Reply-To: From: Rajini Sivaram Date: Tue, 3 Sep 2019 13:50:15 +0100 Message-ID: Subject: Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface To: dev Content-Type: multipart/alternative; boundary="0000000000006313720591a588cc" --0000000000006313720591a588cc Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi all, Ismael brought up a point that it will be good to make the Authorizer interface asynchronous to avoid blocking request threads during remote operations. 1) Since we want to support different backends for authorization metadata, making createAcls() and deleteAcls() asynchronous makes sense since these always involve remote operations. When KIP-500 removes ZooKeeper, we would want to move ACLs to Kafka and async updates will avoid unnecessary blocking. 2) For authorize() method, we currently use cached ACLs in the built-in authorizers, so synchronous authorise operations work well now. But async authorize() would support this scenario as well as authorizers in large organisations where an LRU cache would enable a smaller cache even when the backend holds a large amount of ACLs for infrequently used resources or users who don't use the system frequently. For both cases, the built-in authorizer will continue to be synchronous, using CompletableFuture.completedFuture() to return the actual result. But async API will make custom authorizer implementations more flexible. I would like to know if there are any concerns with these changes before updating the KIP. *Proposed API:* public interface Authorizer extends Configurable, Closeable { Map> start(AuthorizerServerInfo serverI= nfo); List> authorize(AuthorizableRequestContext requestContext, List actions); List> createAcls(AuthorizableRequestContext requestContext, List aclBindings); List> deleteAcls(AuthorizableRequestContext requestContext, List aclBindingFilters); CompletionStage> acls(AclBindingFilter filter); } Thank you, Rajini On Sun, Aug 18, 2019 at 6:25 PM Don Bosco Durai wrote: > Hi Rajini > > Thanks for clarifying. I am good for now. > > Regards > > Bosco > > > =EF=BB=BFOn 8/16/19, 11:30 AM, "Rajini Sivaram" = wrote: > > Hi Don, > > That should be fine. I guess Ranger loads policies from the database > synchronously when the authorizer is configured, similar to > SimpleAclAuthorizer loading from ZooKeeper. Ranger can continue to lo= ad > synchronously from `configure()` or `start()` and return an empty map > from > `start()`. That would retain the existing behaviour.. When the same > database stores policies for all listeners and the policies are not > stored > in Kafka, there is no value in making the load asynchronous. > > Regards, > > Rajini > > > On Fri, Aug 16, 2019 at 6:43 PM Don Bosco Durai > wrote: > > > Hi Rajini > > > > Assuming this doesn't affect custom plugins, I don't have any > concerns > > regarding this. > > > > I do have one question regarding: > > > > "For authorizers that don=E2=80=99t store metadata in ZooKeeper, en= sure that > > authorizer metadata for each listener is available before starting > up the > > listener. This enables different authorization metadata stores for > > different listeners." > > > > Since Ranger uses its own database for storing policies, do you > anticipate > > any issues? > > > > Thanks > > > > Bosco > > > > > > =EF=BB=BFOn 8/16/19, 6:49 AM, "Rajini Sivaram" > wrote: > > > > Hi all, > > > > I made another change to the KIP. The KIP was originally > proposing to > > extend SimpleAclAuthorizer to also implement the new API (in > addition > > to > > the existing API). But since we use the new API when available, > this > > can > > break custom authorizers that extend this class and override > specific > > methods of the old API. To avoid breaking any existing custom > > implementations that extend this class, particularly because it > is in > > the > > public package kafka.security.auth, the KIP now proposes to > retain the > > old > > authorizer as-is. SimpleAclAuthorizer will continue to > implement the > > old > > API, but will be deprecated. A new authorizer implementation > > kafka.security.authorizer.AclAuthorizer will be added for the > new API > > (this > > will not be in the public package). > > > > Please let me know if you have any concerns. > > > > Regards, > > > > Rajini > > > > > > On Fri, Aug 16, 2019 at 8:48 AM Rajini Sivaram < > > rajinisivaram@gmail.com> > > wrote: > > > > > Thanks Colin. > > > > > > If there are no other concerns, I will start vote later today= . > Many > > thanks > > > to every one for the feedback. > > > > > > Regards, > > > > > > Rajini > > > > > > > > > On Thu, Aug 15, 2019 at 11:57 PM Colin McCabe < > cmccabe@apache.org> > > wrote: > > > > > >> Thanks, Rajini. It looks good to me. > > >> > > >> best, > > >> Colin > > >> > > >> > > >> On Thu, Aug 15, 2019, at 11:37, Rajini Sivaram wrote: > > >> > Hi Colin, > > >> > > > >> > Thanks for the review. I have updated the KIP to move the > > interfaces for > > >> > request context and server info to the authorizer package. > These > > are now > > >> > called AuthorizableRequestContext and AuthorizerServerInfo= . > > Endpoint is > > >> now > > >> > a class in org.apache.kafka.common to make it reusable > since we > > already > > >> > have multiple implementations of it. I have removed > requestName > > from the > > >> > request context interface since authorizers can distinguis= h > > follower > > >> fetch > > >> > and consumer fetch from the operation being authorized. So > 16-bit > > >> request > > >> > type should be sufficient for audit logging. Also replace= d > > AuditFlag > > >> with > > >> > two booleans as you suggested. > > >> > > > >> > Can you take another look and see if the KIP is ready for > voting? > > >> > > > >> > Thanks for all your help! > > >> > > > >> > Regards, > > >> > > > >> > Rajini > > >> > > > >> > On Wed, Aug 14, 2019 at 8:59 PM Colin McCabe < > cmccabe@apache.org> > > >> wrote: > > >> > > > >> > > Hi Rajini, > > >> > > > > >> > > I think it would be good to rename KafkaRequestContext t= o > > something > > >> like > > >> > > AuthorizableRequestContext, and put it in the > > >> > > org.apache.kafka.server.authorizer namespace. If we put > it in > > the > > >> > > org.apache.kafka.common namespace, then it's not really > clear > > that > > >> it's > > >> > > part of the Authorizer API. Since this class is purely = an > > interface, > > >> with > > >> > > no concrete implementation of anything, there's nothing > common > > to > > >> really > > >> > > reuse in any case. We definitely don't want someone to > > accidentally > > >> add or > > >> > > remove methods because they think this is just another > internal > > class > > >> used > > >> > > for requests. > > >> > > > > >> > > The BrokerInfo class is a nice improvement. It looks > like it > > will be > > >> > > useful for passing in information about the context we'r= e > > running > > >> in. It > > >> > > would be nice to call this class ServerInfo rather than > > BrokerInfo, > > >> since > > >> > > we will want to run the authorizer on controllers as wel= l > as on > > >> brokers, > > >> > > and the controller may run as a separate process post > KIP-500. > > I also > > >> > > think that this class should be in the > > >> org.apache.kafka.server.authorizer > > >> > > namespace. Again, it is an interface, not a concrete > > implementation, > > >> and > > >> > > it's an interface that is very specifically for the > authorizer. > > >> > > > > >> > > I agree that we probably don't have enough information > > preserved for > > >> > > requests currently to always know what entity made them. > So we > > can > > >> leave > > >> > > that out for now (except in the special case of Fetch). > > Perhaps we > > >> can add > > >> > > this later if it's needed. > > >> > > > > >> > > I understand the intention behind AuthorizationMode > (which is > > now > > >> called > > >> > > AuditFlag in the latest revision). But it still feels > > complex. What > > >> if we > > >> > > just had two booleans in Action: logSuccesses and > logFailures? > > That > > >> seems > > >> > > to cover all the cases here. MANDATORY_AUTHORIZE =3D tr= ue, > true. > > >> > > OPTIONAL_AUTHORIZE =3D true, false. FILTER =3D true, fa= lse. > > >> LIST_AUTHORIZED =3D > > >> > > false, false. Would there be anything lost versus havin= g > the > > enum? > > >> > > > > >> > > best, > > >> > > Colin > > >> > > > > >> > > > > >> > > On Wed, Aug 14, 2019, at 06:29, Mickael Maison wrote: > > >> > > > Hi Rajini, > > >> > > > > > >> > > > Thanks for the KIP! > > >> > > > I really like that authorize() will be able to take a > batch of > > >> > > > requests, this will speed up many implementations! > > >> > > > > > >> > > > On Tue, Aug 13, 2019 at 5:57 PM Rajini Sivaram < > > >> rajinisivaram@gmail.com> > > >> > > wrote: > > >> > > > > > > >> > > > > Thanks David! I have fixed the typo. > > >> > > > > > > >> > > > > Also made a couple of changes to make the context > > interfaces more > > >> > > generic. > > >> > > > > KafkaRequestContext now returns the 16-bit API key a= s > Colin > > >> suggested > > >> > > as > > >> > > > > well as the friendly name used in metrics which are > useful > > in > > >> audit > > >> > > logs. > > >> > > > > `Authorizer#start` is now provided a generic > `BrokerInfo` > > >> interface > > >> > > that > > >> > > > > gives cluster id, broker id and endpoint information= . > The > > generic > > >> > > interface > > >> > > > > can potentially be used in other broker plugins in > future > > and > > >> provides > > >> > > > > dynamically generated configs like broker id and por= ts > > which are > > >> > > currently > > >> > > > > not available to plugins unless these configs are > statically > > >> > > configured. > > >> > > > > Please let me know if there are any concerns. > > >> > > > > > > >> > > > > Regards, > > >> > > > > > > >> > > > > Rajini > > >> > > > > > > >> > > > > On Tue, Aug 13, 2019 at 4:30 PM David Jacot < > > djacot@confluent.io> > > >> > > wrote: > > >> > > > > > > >> > > > > > Hi Rajini, > > >> > > > > > > > >> > > > > > Thank you for the update! It looks good to me. > There is a > > typo > > >> in the > > >> > > > > > `AuditFlag` enum: `MANDATORY_AUTHOEIZE` -> > > >> `MANDATORY_AUTHORIZE`. > > >> > > > > > > > >> > > > > > Regards, > > >> > > > > > David > > >> > > > > > > > >> > > > > > On Mon, Aug 12, 2019 at 2:54 PM Rajini Sivaram < > > >> > > rajinisivaram@gmail.com> > > >> > > > > > wrote: > > >> > > > > > > > >> > > > > > > Hi David, > > >> > > > > > > > > >> > > > > > > Thanks for reviewing the KIP! Since questions > about > > >> `authorization > > >> > > mode` > > >> > > > > > > and `count` have come up multiple times, I have > renamed > > both. > > >> > > > > > > > > >> > > > > > > 1) Renamed `count` to `resourceReferenceCount`. > It is > > the > > >> number > > >> > > of times > > >> > > > > > > the resource being authorized is referenced > within the > > >> request. > > >> > > > > > > > > >> > > > > > > 2) Renamed `AuthorizationMode` to `AuditFlag`. I= t > is > > provided > > >> to > > >> > > improve > > >> > > > > > > audit logging in the authorizer. The enum values > have > > javadoc > > >> which > > >> > > > > > > indicate how the authorization result is used in > each > > of the > > >> modes > > >> > > to > > >> > > > > > > enable authorizers to log audit messages at the > right > > severity > > >> > > level. > > >> > > > > > > > > >> > > > > > > Regards, > > >> > > > > > > > > >> > > > > > > Rajini > > >> > > > > > > > > >> > > > > > > On Mon, Aug 12, 2019 at 5:54 PM David Jacot < > > >> djacot@confluent.io> > > >> > > wrote: > > >> > > > > > > > > >> > > > > > > > Hi Rajini, > > >> > > > > > > > > > >> > > > > > > > Thank you for the KIP. Overall, it looks good > to me. > > I have > > >> few > > >> > > > > > > > questions/suggestions: > > >> > > > > > > > > > >> > > > > > > > 1. It is hard to grasp what `Action#count` is > for. I > > guess I > > >> > > understand > > >> > > > > > > > where you want to go with it but it took me a > while to > > >> figure it > > >> > > out. > > >> > > > > > > > Perhaps, we could come up with a better name > than > > `count`? > > >> > > > > > > > > > >> > > > > > > > 2. I had a hard time trying to understand the > > >> `AuthorizationMode` > > >> > > > > > > concept, > > >> > > > > > > > especially wrt. the OPTIONAL one. My > understanding is > > that > > >> an > > >> > > ACL is > > >> > > > > > > either > > >> > > > > > > > defined or not. Could you elaborate a bit more > on > > that? > > >> > > > > > > > > > >> > > > > > > > Thanks, > > >> > > > > > > > David > > >> > > > > > > > > > >> > > > > > > > On Fri, Aug 9, 2019 at 10:26 PM Don Bosco Dura= i > < > > >> > > bosco@apache.org> > > >> > > > > > > wrote: > > >> > > > > > > > > > >> > > > > > > > > Hi Rajini > > >> > > > > > > > > > > >> > > > > > > > > 3.2 - This makes sense. Thanks for clarifyin= g. > > >> > > > > > > > > > > >> > > > > > > > > Rest looks fine. Once the implementations ar= e > done, > > it > > >> will be > > >> > > more > > >> > > > > > > clear > > >> > > > > > > > > on the different values RequestType and Mode= . > > >> > > > > > > > > > > >> > > > > > > > > Thanks > > >> > > > > > > > > > > >> > > > > > > > > Bosco > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > =EF=BB=BFOn 8/9/19, 5:08 AM, "Rajini Sivaram= " < > > >> rajinisivaram@gmail.com > > >> > > > > > >> > > > > > wrote: > > >> > > > > > > > > > > >> > > > > > > > > Hi Don, > > >> > > > > > > > > > > >> > > > > > > > > Thanks for the suggestions. A few > responses > > below: > > >> > > > > > > > > > > >> > > > > > > > > 3.1 Can rename and improve docs if we ke= ep > > this. Let's > > >> > > finish the > > >> > > > > > > > > discussion on Colin's suggestions > regarding this > > >> first. > > >> > > > > > > > > 3.2 No, I was thinking of some requests > that > > have an > > >> old > > >> > > way of > > >> > > > > > > > > authorizing > > >> > > > > > > > > and a new way where we have retained the > old > > way for > > >> > > backward > > >> > > > > > > > > compatibility. One example is > Cluster:Create > > >> permission to > > >> > > create > > >> > > > > > > > > topics. > > >> > > > > > > > > We have replaced this with fine-grained > topic > > create > > >> > > access using > > >> > > > > > > > > Topic:Create > > >> > > > > > > > > for topic patterns. But we still check i= f > user > > has > > >> > > Cluster:Create > > >> > > > > > > > > first. If > > >> > > > > > > > > Denied, the deny is ignored and we check > > >> Topic:Create. We > > >> > > dont > > >> > > > > > want > > >> > > > > > > > to > > >> > > > > > > > > log > > >> > > > > > > > > DENY for Cluster:Create at INFO level fo= r > this, > > since > > >> this > > >> > > is > > >> > > > > > not a > > >> > > > > > > > > mandatory ACL for creating topics. We > will get > > >> appropriate > > >> > > logs > > >> > > > > > > from > > >> > > > > > > > > the > > >> > > > > > > > > subsequent Topic:Create in this case. > > >> > > > > > > > > 3.3 They are not quite the same. FILTER > implies > > that > > >> user > > >> > > > > > actually > > >> > > > > > > > > requested access to and performed some > > operation on > > >> the > > >> > > filtered > > >> > > > > > > > > resources. > > >> > > > > > > > > LIST_AUTHORZED did not result in any > actual > > access. > > >> User > > >> > > simply > > >> > > > > > > > wanted > > >> > > > > > > > > to > > >> > > > > > > > > know what they are allowed to access. > > >> > > > > > > > > 3.4 Request types are Produce, JoinGroup= , > > OffsetCommit > > >> > > etc. So > > >> > > > > > that > > >> > > > > > > > is > > >> > > > > > > > > different from authorization mode, > operation > > etc. > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > On Thu, Aug 8, 2019 at 11:36 PM Don Bosc= o > Durai > > < > > >> > > > > > bosco@apache.org> > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > >> > > > > > > > > > Hi Rajini > > >> > > > > > > > > > > > >> > > > > > > > > > Thanks for clarifying. This is very > helpful... > > >> > > > > > > > > > > > >> > > > > > > > > > #1 - On the Ranger side, we should be > able to > > handle > > >> > > multiple > > >> > > > > > > > > requests at > > >> > > > > > > > > > the same time. I was just not sure how > much > > >> processing > > >> > > overhead > > >> > > > > > > > will > > >> > > > > > > > > be > > >> > > > > > > > > > there on the Broker side to split and > then > > >> consolidate > > >> > > the > > >> > > > > > > results. > > >> > > > > > > > > If it > > >> > > > > > > > > > is negligible, then this is the bette= r > way. > > It will > > >> > > make it > > >> > > > > > > future > > >> > > > > > > > > proof. > > >> > > > > > > > > > #2 - I agree, having a single "start" > call > > makes it > > >> > > cleaner. > > >> > > > > > The > > >> > > > > > > > > > Authorization implementation will only > have > > to do > > >> the > > >> > > > > > > > initialization > > >> > > > > > > > > only > > >> > > > > > > > > > once. > > >> > > > > > > > > > #3.1 - Thanks for the clarification. I > think > > it > > >> makes > > >> > > sense to > > >> > > > > > > have > > >> > > > > > > > > this. > > >> > > > > > > > > > The term "Mode" might not be explicit > enough. > > >> > > Essentially it > > >> > > > > > > seems > > >> > > > > > > > > you want > > >> > > > > > > > > > the Authorizer to know the > Intent/Purpose of > > the > > >> > > authorize call > > >> > > > > > > and > > >> > > > > > > > > let the > > >> > > > > > > > > > Authorizer decide what to log as Audit > event. > > >> Changing > > >> > > the > > >> > > > > > > > > class/field name > > >> > > > > > > > > > or giving more documentation will do. > > >> > > > > > > > > > #3.2 - Regarding the option "OPTIONAL"= . > Are > > you > > >> thinking > > >> > > from > > >> > > > > > > > > chaining > > >> > > > > > > > > > multiple Authorizer? If so, I am not > sure > > whether > > >> the > > >> > > Broker > > >> > > > > > > would > > >> > > > > > > > > have > > >> > > > > > > > > > enough information to make that > decision. I > > feel the > > >> > > Authorizer > > >> > > > > > > > will > > >> > > > > > > > > be the > > >> > > > > > > > > > one who would have that knowledge. E.g= . > in > > Ranger > > >> we have > > >> > > > > > > explicit > > >> > > > > > > > > deny, > > >> > > > > > > > > > which means no matter what, the reques= t > > should be > > >> denied > > >> > > for > > >> > > > > > the > > >> > > > > > > > > user/group > > >> > > > > > > > > > or condition. So if you are thinking o= f > > chaining > > >> > > Authorizers, > > >> > > > > > > then > > >> > > > > > > > > > responses should have the third state, > e.g. > > >> > > "DENIED_FINAL", in > > >> > > > > > > > which > > >> > > > > > > > > case > > >> > > > > > > > > > if there is an Authorization chain, it > will > > be stop > > >> and > > >> > > the > > >> > > > > > > request > > >> > > > > > > > > will be > > >> > > > > > > > > > denied and if it is just denied, then > you > > might fall > > >> > > back to > > >> > > > > > next > > >> > > > > > > > > > authorizer. If we don't have chaining = of > > >> Authorizing in > > >> > > mind, > > >> > > > > > > then > > >> > > > > > > > we > > >> > > > > > > > > > should reconsider OPTIONAL for now. Or > > clarify under > > >> > > which > > >> > > > > > > scenario > > >> > > > > > > > > > OPTIONAL will be called. > > >> > > > > > > > > > #3.3 Regarding, FILTER v/s > LIST_AUTHORIZED, > > isn't > > >> > > > > > > LIST_AUTHORIZED a > > >> > > > > > > > > > special case for "FILTER"? > > >> > > > > > > > > > #3.4 KafkaRequestContext. requestType(= ) > v/s > > Action. > > >> > > > > > > > > authorizationMode. I > > >> > > > > > > > > > am not sure about the overlap or > ambiguity. > > >> > > > > > > > > > #4 - Cool. This is good, it will be le= ss > > stress on > > >> the > > >> > > > > > > Authorizer. > > >> > > > > > > > > Ranger > > >> > > > > > > > > > already supports the "count" concept > and also > > has > > >> > > batching > > >> > > > > > > > > capability to > > >> > > > > > > > > > aggregate similar requests to reduce t= he > > number of > > >> audit > > >> > > logs > > >> > > > > > to > > >> > > > > > > > > write. We > > >> > > > > > > > > > should be able to pass this through. > > >> > > > > > > > > > #5 - Assuming if the object instance i= s > going > > out of > > >> > > scope, we > > >> > > > > > > > > should be > > >> > > > > > > > > > fine. Not a super important ask. Range= r > is > > already > > >> > > catching > > >> > > > > > KILL > > >> > > > > > > > > signal for > > >> > > > > > > > > > clean up. > > >> > > > > > > > > > > > >> > > > > > > > > > Thanks again. These are good > enhancements. > > >> Appreciate > > >> > > your > > >> > > > > > > efforts > > >> > > > > > > > > here. > > >> > > > > > > > > > > > >> > > > > > > > > > Bosco > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > =EF=BB=BFOn 8/8/19, 2:03 AM, "Rajini S= ivaram" < > > >> > > rajinisivaram@gmail.com > > >> > > > > > > > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > > >> > > > > > > > > > Hi Don, > > >> > > > > > > > > > > > >> > > > > > > > > > Thanks for reviewing the KIP. > > >> > > > > > > > > > > > >> > > > > > > > > > 1. I had this originally as a sing= le > > Action, but > > >> > > thought it > > >> > > > > > > may > > >> > > > > > > > > be > > >> > > > > > > > > > useful > > >> > > > > > > > > > to support batched authorize calls > as > > well and > > >> keep > > >> > > it > > >> > > > > > > > > consistent with > > >> > > > > > > > > > other methods. Single requests can > contain > > >> multiple > > >> > > topics. > > >> > > > > > > For > > >> > > > > > > > > > example a > > >> > > > > > > > > > produce request can contain record= s > for > > several > > >> > > partitions > > >> > > > > > of > > >> > > > > > > > > different > > >> > > > > > > > > > topics. Broker could potentially > > authorize these > > >> > > together. > > >> > > > > > > For > > >> > > > > > > > > > SimpleAclAuthorizer, batched > authorize > > methods > > >> don't > > >> > > > > > provide > > >> > > > > > > > any > > >> > > > > > > > > > optimisation since lookup is based > on > > resources > > >> > > followed by > > >> > > > > > > the > > >> > > > > > > > > > matching > > >> > > > > > > > > > logic. But some authorizers may > manage > > ACLs by > > >> user > > >> > > > > > principal > > >> > > > > > > > > rather > > >> > > > > > > > > > than > > >> > > > > > > > > > resource and may be able to optimi= ze > > batched > > >> > > requests. I am > > >> > > > > > > ok > > >> > > > > > > > > with > > >> > > > > > > > > > using > > >> > > > > > > > > > single Action if this is likely to > cause > > issues. > > >> > > > > > > > > > 2. If you have two listeners, one > for > > >> inter-broker > > >> > > traffic > > >> > > > > > > and > > >> > > > > > > > > another > > >> > > > > > > > > > for > > >> > > > > > > > > > external clients, start method is > invoked > > twice, > > >> > > once for > > >> > > > > > > each > > >> > > > > > > > > > listener. On > > >> > > > > > > > > > second thought, that may be > confusing and > > a > > >> single > > >> > > start() > > >> > > > > > > > > invocation > > >> > > > > > > > > > that > > >> > > > > > > > > > provides all listener information > and > > returns > > >> > > multiple > > >> > > > > > > futures > > >> > > > > > > > > would be > > >> > > > > > > > > > better. Will update the KIP. > > >> > > > > > > > > > 3. A typical example is a consumer > > subscribing > > >> to a > > >> > > regex > > >> > > > > > > > > pattern. We > > >> > > > > > > > > > request all topic metadata from th= e > > broker in > > >> order > > >> > > to > > >> > > > > > decide > > >> > > > > > > > > whether > > >> > > > > > > > > > the > > >> > > > > > > > > > pattern matches, expecting to > receive a > > list of > > >> > > authorised > > >> > > > > > > > > topics. The > > >> > > > > > > > > > user > > >> > > > > > > > > > is not asking to subscribe to an > > unauthorized > > >> topic. > > >> > > If > > >> > > > > > there > > >> > > > > > > > > are 10000 > > >> > > > > > > > > > topics in the cluster and the user > has > > access > > >> to 100 > > >> > > of > > >> > > > > > them, > > >> > > > > > > > at > > >> > > > > > > > > the > > >> > > > > > > > > > moment > > >> > > > > > > > > > we log 9900 DENIED log entries at > INFO > > level in > > >> > > > > > > > > SimpleAclAuthorizer. > > >> > > > > > > > > > The > > >> > > > > > > > > > proposal is to authorize this > request with > > >> > > > > > > > > AuthorizationMode.FILTER, so > > >> > > > > > > > > > that authorizers can log resources > that > > are > > >> filtered > > >> > > out at > > >> > > > > > > > > lower level > > >> > > > > > > > > > like DEBUG since this is not an > attempt to > > >> access > > >> > > > > > > unauthorized > > >> > > > > > > > > > resources. > > >> > > > > > > > > > Brokers already handle these > differently > > since > > >> no > > >> > > > > > > authorization > > >> > > > > > > > > error > > >> > > > > > > > > > is > > >> > > > > > > > > > returned to the client in these > cases. > > Providing > > >> > > > > > > authorization > > >> > > > > > > > > mode to > > >> > > > > > > > > > authorizers enables authorizer > > implementations > > >> to > > >> > > generate > > >> > > > > > > > > better audit > > >> > > > > > > > > > logs. > > >> > > > > > > > > > 4. Each request may contain multip= le > > instances > > >> of > > >> > > the same > > >> > > > > > > > > authorizable > > >> > > > > > > > > > resource. For example a produce > request > > may > > >> contain > > >> > > records > > >> > > > > > > for > > >> > > > > > > > > 10 > > >> > > > > > > > > > partitions of the same topic. At t= he > > moment, we > > >> > > invoke > > >> > > > > > > > authorize > > >> > > > > > > > > > method 10 > > >> > > > > > > > > > times. The proposal is to invoke i= t > once > > with > > >> > > count=3D10. The > > >> > > > > > > > > count is > > >> > > > > > > > > > provided to authorizer just for > audit > > logging > > >> > > purposes. > > >> > > > > > > > > > 5. Authorizer implements Closeable= , > so you > > >> could use > > >> > > > > > close() > > >> > > > > > > to > > >> > > > > > > > > flush > > >> > > > > > > > > > audits? > > >> > > > > > > > > > > > >> > > > > > > > > > On Thu, Aug 8, 2019 at 7:01 AM Don > Bosco > > Durai < > > >> > > > > > > > bosco@apache.org > > >> > > > > > > > > > > > >> > > > > > > > > > wrote: > > >> > > > > > > > > > > > >> > > > > > > > > > > Rajini > > >> > > > > > > > > > > > > >> > > > > > > > > > > Thanks for putting this together= . > It is > > >> looking > > >> > > good. I > > >> > > > > > > have > > >> > > > > > > > > few > > >> > > > > > > > > > > questions... > > >> > > > > > > > > > > > > >> > > > > > > > > > > 1. List > > authorize(..., > > >> > > List > > >> > > > > > > > > actions). > > >> > > > > > > > > > Do you > > >> > > > > > > > > > > see a scenario where the broker > will > > call > > >> > > authorize for > > >> > > > > > > > > multiple > > >> > > > > > > > > > topics at > > >> > > > > > > > > > > the same time? I can understand > that > > during > > >> > > > > > > creating/deleting > > >> > > > > > > > > ACLS, > > >> > > > > > > > > > > multiple permissions for multipl= e > > resources > > >> might > > >> > > be > > >> > > > > > done. > > >> > > > > > > > For > > >> > > > > > > > > > authorize > > >> > > > > > > > > > > call, would this be a case? And > does the > > >> Authorize > > >> > > > > > > > > implementation > > >> > > > > > > > > > will be > > >> > > > > > > > > > > able to do performance > optimization > > because of > > >> > > this? Or > > >> > > > > > > > should > > >> > > > > > > > > we > > >> > > > > > > > > > just keep > > >> > > > > > > > > > > it simple? I don't see it as an > issue > > from > > >> Apache > > >> > > Ranger > > >> > > > > > > > side, > > >> > > > > > > > > but > > >> > > > > > > > > > just > > >> > > > > > > > > > > checking to see whether we need > to be > > aware of > > >> > > something. > > >> > > > > > > > > > > 2. Should I assume that the > > SecurityProtocol > > >> passed > > >> > > > > > during > > >> > > > > > > > > start and > > >> > > > > > > > > > the > > >> > > > > > > > > > > one return by > > >> > > KafkaRequestContext.securityProtocol() will > > >> > > > > > > be > > >> > > > > > > > > the > > >> > > > > > > > > > same? > > >> > > > > > > > > > > CompletableFuture > start(String > > >> listenerName, > > >> > > > > > > > > SecurityProtocol > > >> > > > > > > > > > > securityProtocol); > > >> > > > > > > > > > > > KafkaRequestContext.securityProtocol() > > >> > > > > > > > > > > 3. What is the purpose of > > AuthorizationMode? > > >> How > > >> > > does the > > >> > > > > > > > > broker > > >> > > > > > > > > > decide > > >> > > > > > > > > > > what mode to use when the > authorize() > > method > > >> is > > >> > > called? > > >> > > > > > > > > > > 4. Can we clarify "count" in > Action a > > bit > > >> more? > > >> > > How is it > > >> > > > > > > > used? > > >> > > > > > > > > > > 5. Do you feel having "stop" > along with > > >> "start" be > > >> > > > > > helpful? > > >> > > > > > > > > E.g. In > > >> > > > > > > > > > Ranger > > >> > > > > > > > > > > we try to optimize the Audit > writing by > > >> caching > > >> > > the logs > > >> > > > > > > for > > >> > > > > > > > a > > >> > > > > > > > > fixed > > >> > > > > > > > > > > interval. But when the Broker > > terminates, we > > >> do a > > >> > > forced > > >> > > > > > > > flush. > > >> > > > > > > > > > Having an > > >> > > > > > > > > > > explicit "stop" might give us a > formal > > way to > > >> > > flush our > > >> > > > > > > > audits. > > >> > > > > > > > > > > > > >> > > > > > > > > > > Thanks > > >> > > > > > > > > > > > > >> > > > > > > > > > > Bosco > > >> > > > > > > > > > > > > >> > > > > > > > > > > =EF=BB=BFOn 8/7/19, 3:59 PM, "Ra= jini > Sivaram" < > > >> > > > > > > > rajinisivaram@gmail.com > > >> > > > > > > > > > > > >> > > > > > > > > > wrote: > > >> > > > > > > > > > > > > >> > > > > > > > > > > Hi Ron/Harsha/Satish, > > >> > > > > > > > > > > > > >> > > > > > > > > > > Thanks for reviewing the KIP= ! > > >> > > > > > > > > > > > > >> > > > > > > > > > > We should perhaps have a wid= er > > discussion > > >> > > outside > > >> > > > > > this > > >> > > > > > > > KIP > > >> > > > > > > > > for > > >> > > > > > > > > > > refactoring > > >> > > > > > > > > > > clients so that others who > are not > > >> following > > >> > > this KIP > > >> > > > > > > > also > > >> > > > > > > > > > notice the > > >> > > > > > > > > > > discussion. Satish, would yo= u > like > > to > > >> start a > > >> > > > > > > discussion > > >> > > > > > > > > thread > > >> > > > > > > > > > on dev? > > >> > > > > > > > > > > > > >> > > > > > > > > > > Regards, > > >> > > > > > > > > > > > > >> > > > > > > > > > > Rajini > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > On Wed, Aug 7, 2019 at 6:21 = PM > > Satish > > >> Duggana < > > >> > > > > > > > > > > satish.duggana@gmail.com> > > >> > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > >> > > > > > > > > > > > I felt the same need when > we want > > to > > >> add a > > >> > > > > > pluggable > > >> > > > > > > > API > > >> > > > > > > > > for > > >> > > > > > > > > > core > > >> > > > > > > > > > > > server functionality. This > does > > not > > >> need to > > >> > > be part > > >> > > > > > > of > > >> > > > > > > > > this > > >> > > > > > > > > > KIP, it > > >> > > > > > > > > > > > can be a separate KIP. I c= an > > contribute > > >> those > > >> > > > > > > > refactoring > > >> > > > > > > > > > changes if > > >> > > > > > > > > > > > others are OK with that. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > It is better to have a > structure > > like > > >> below. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > kafka-common: > > >> > > > > > > > > > > > common classes which can b= e > used > > in any > > >> of > > >> > > the > > >> > > > > > other > > >> > > > > > > > > modules > > >> > > > > > > > > > in Kafka > > >> > > > > > > > > > > > like client, > Kafka-server-common > > and > > >> server > > >> > > etc. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > kafka-client-common: > > >> > > > > > > > > > > > common classes which can b= e > used > > in the > > >> > > client > > >> > > > > > > module. > > >> > > > > > > > > This > > >> > > > > > > > > > can be > > >> > > > > > > > > > > > part of client module > itself. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > kafka-server-common: > > >> > > > > > > > > > > > classes required only for > > kafka-server. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > Thanks. > > >> > > > > > > > > > > > Satish. > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > On Wed, Aug 7, 2019 at 9:2= 8 > PM > > Harsha > > >> > > Chintalapani > > >> > > > > > < > > >> > > > > > > > > > kafka@harsha.io> > > >> > > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks for the KIP Rajin= i. > > >> > > > > > > > > > > > > Quick thought, it would > be good > > to > > >> have a > > >> > > common > > >> > > > > > > > module > > >> > > > > > > > > > outside of > > >> > > > > > > > > > > > clients > > >> > > > > > > > > > > > > that only applies to > server side > > >> > > interfaces & > > >> > > > > > > > changes. > > >> > > > > > > > > It > > >> > > > > > > > > > looks > > >> > > > > > > > > > > like we > > >> > > > > > > > > > > > are > > >> > > > > > > > > > > > > increasingly in favor of > using > > Java > > >> > > interface for > > >> > > > > > > > > pluggable > > >> > > > > > > > > > > modules on > > >> > > > > > > > > > > > the > > >> > > > > > > > > > > > > broker side. > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > Thanks, > > >> > > > > > > > > > > > > Harsha > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > On Tue, Aug 06, 2019 at > 2:31 PM, > > >> Rajini > > >> > > Sivaram < > > >> > > > > > > > > > > rajinisivaram@gmail.com > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Hi all, > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > I have created a KIP t= o > > replace the > > >> Scala > > >> > > > > > > > Authorizer > > >> > > > > > > > > API > > >> > > > > > > > > > with a > > >> > > > > > > > > > > new > > >> > > > > > > > > > > > Java > > >> > > > > > > > > > > > > > API: > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > - > > >> > > > > > > > > > > > > > > > >> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > >> > > > > > > > > > > > > > > > >> > > KIP-504+-+Add+new+Java+Authorizer+Interface > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > This is replacement fo= r > KIP-50 > > >> which was > > >> > > > > > accepted > > >> > > > > > > > > but never > > >> > > > > > > > > > > merged. > > >> > > > > > > > > > > > Apart > > >> > > > > > > > > > > > > > from moving to a Java > API > > consistent > > >> > > with other > > >> > > > > > > > > pluggable > > >> > > > > > > > > > > interfaces > > >> > > > > > > > > > > > in the > > >> > > > > > > > > > > > > > broker, KIP-504 also > attempts > > to > > >> address > > >> > > known > > >> > > > > > > > > limitations > > >> > > > > > > > > > in the > > >> > > > > > > > > > > > > > authorizer. If you hav= e > come > > across > > >> other > > >> > > > > > > > > limitations that > > >> > > > > > > > > > you > > >> > > > > > > > > > > would > > >> > > > > > > > > > > > like > > >> > > > > > > > > > > > > > to see addressed in th= e > new > > API, > > >> please > > >> > > raise > > >> > > > > > > these > > >> > > > > > > > > on the > > >> > > > > > > > > > > discussion > > >> > > > > > > > > > > > > > thread so that we can > > consider those > > >> > > too. All > > >> > > > > > > > > suggestions > > >> > > > > > > > > > and > > >> > > > > > > > > > > feedback > > >> > > > > > > > > > > > are > > >> > > > > > > > > > > > > > welcome. > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Thank you, > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > Rajini > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > > > > > > > > > > > > > > --0000000000006313720591a588cc--