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’t store metadata in ZooKeeper, ensure 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
On 8/16/19, 6:49 AM, "Rajini Sivaram" <rajinisiva...@gmail.com> 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 <rajinisiva...@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 <cmcc...@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 distinguish follower
>> fetch
>> > and consumer fetch from the operation being authorized. So 16-bit
>> request
>> > type should be sufficient for audit logging. Also replaced 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 <cmcc...@apache.org>
>> wrote:
>> >
>> > > Hi Rajini,
>> > >
>> > > I think it would be good to rename KafkaRequestContext to 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're 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 well 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 = true, true.
>> > > OPTIONAL_AUTHORIZE = true, false. FILTER = true, false.
>> LIST_AUTHORIZED =
>> > > false, false. Would there be anything lost versus having 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 <
>> rajinisiva...@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 as 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 ports 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 <dja...@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 <
>> > > rajinisiva...@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`. It 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 <
>> dja...@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 Durai <
>> > > bo...@apache.org>
>> > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > Hi Rajini
>> > > > > > > > >
>> > > > > > > > > 3.2 - This makes sense. Thanks for clarifying.
>> > > > > > > > >
>> > > > > > > > > Rest looks fine. Once the implementations are done, it
>> will be
>> > > more
>> > > > > > > clear
>> > > > > > > > > on the different values RequestType and Mode.
>> > > > > > > > >
>> > > > > > > > > Thanks
>> > > > > > > > >
>> > > > > > > > > Bosco
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > On 8/9/19, 5:08 AM, "Rajini Sivaram" <
>> rajinisiva...@gmail.com
>> > > >
>> > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > Hi Don,
>> > > > > > > > >
>> > > > > > > > > Thanks for the suggestions. A few responses below:
>> > > > > > > > >
>> > > > > > > > > 3.1 Can rename and improve docs if we keep 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 if 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 for 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 Bosco Durai <
>> > > > > > bo...@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 better 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 request should be
>> denied
>> > > for
>> > > > > > the
>> > > > > > > > > user/group
>> > > > > > > > > > or condition. So if you are thinking of 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 less stress on
>> the
>> > > > > > > Authorizer.
>> > > > > > > > > Ranger
>> > > > > > > > > > already supports the "count" concept and also has
>> > > batching
>> > > > > > > > > capability to
>> > > > > > > > > > aggregate similar requests to reduce the number of
>> audit
>> > > logs
>> > > > > > to
>> > > > > > > > > write. We
>> > > > > > > > > > should be able to pass this through.
>> > > > > > > > > > #5 - Assuming if the object instance is going out
of
>> > > scope, we
>> > > > > > > > > should be
>> > > > > > > > > > fine. Not a super important ask. Ranger is already
>> > > catching
>> > > > > > KILL
>> > > > > > > > > signal for
>> > > > > > > > > > clean up.
>> > > > > > > > > >
>> > > > > > > > > > Thanks again. These are good enhancements.
>> Appreciate
>> > > your
>> > > > > > > efforts
>> > > > > > > > > here.
>> > > > > > > > > >
>> > > > > > > > > > Bosco
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > On 8/8/19, 2:03 AM, "Rajini Sivaram" <
>> > > rajinisiva...@gmail.com
>> > > > > > >
>> > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > Hi Don,
>> > > > > > > > > >
>> > > > > > > > > > Thanks for reviewing the KIP.
>> > > > > > > > > >
>> > > > > > > > > > 1. I had this originally as a single 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 records 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 optimize 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 the 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 multiple instances
>> of
>> > > the same
>> > > > > > > > > authorizable
>> > > > > > > > > > resource. For example a produce request may
>> contain
>> > > records
>> > > > > > > for
>> > > > > > > > > 10
>> > > > > > > > > > partitions of the same topic. At the moment, we
>> > > invoke
>> > > > > > > > authorize
>> > > > > > > > > > method 10
>> > > > > > > > > > times. The proposal is to invoke it once with
>> > > count=10. 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
<
>> > > > > > > > bo...@apache.org
>> > > > > > > > > >
>> > > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > > Rajini
>> > > > > > > > > > >
>> > > > > > > > > > > Thanks for putting this together. It is
>> looking
>> > > good. I
>> > > > > > > have
>> > > > > > > > > few
>> > > > > > > > > > > questions...
>> > > > > > > > > > >
>> > > > > > > > > > > 1. List<AuthorizationResult> authorize(...,
>> > > List<Action>
>> > > > > > > > > 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 multiple 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<Void> 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
>> > > > > > > > > > >
>> > > > > > > > > > > On 8/7/19, 3:59 PM, "Rajini Sivaram" <
>> > > > > > > > rajinisiva...@gmail.com
>> > > > > > > > > >
>> > > > > > > > > > wrote:
>> > > > > > > > > > >
>> > > > > > > > > > > Hi Ron/Harsha/Satish,
>> > > > > > > > > > >
>> > > > > > > > > > > Thanks for reviewing the KIP!
>> > > > > > > > > > >
>> > > > > > > > > > > We should perhaps have a wider discussion
>> > > outside
>> > > > > > this
>> > > > > > > > KIP
>> > > > > > > > > for
>> > > > > > > > > > > refactoring
>> > > > > > > > > > > clients so that others who are not
>> following
>> > > this KIP
>> > > > > > > > also
>> > > > > > > > > > notice the
>> > > > > > > > > > > discussion. Satish, would you like to
>> start a
>> > > > > > > discussion
>> > > > > > > > > thread
>> > > > > > > > > > on dev?
>> > > > > > > > > > >
>> > > > > > > > > > > Regards,
>> > > > > > > > > > >
>> > > > > > > > > > > Rajini
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > > On Wed, Aug 7, 2019 at 6:21 PM Satish
>> Duggana <
>> > > > > > > > > > > satish.dugg...@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 can 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 be used in any
>> of
>> > > the
>> > > > > > other
>> > > > > > > > > modules
>> > > > > > > > > > in Kafka
>> > > > > > > > > > > > like client, Kafka-server-common and
>> server
>> > > etc.
>> > > > > > > > > > > >
>> > > > > > > > > > > > kafka-client-common:
>> > > > > > > > > > > > common classes which can be 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:28 PM Harsha
>> > > Chintalapani
>> > > > > > <
>> > > > > > > > > > ka...@harsha.io>
>> > > > > > > > > > > > wrote:
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Thanks for the KIP Rajini.
>> > > > > > > > > > > > > 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 <
>> > > > > > > > > > > rajinisiva...@gmail.com
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > wrote:
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > > Hi all,
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > I have created a KIP to 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 for 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 have come across
>> other
>> > > > > > > > > limitations that
>> > > > > > > > > > you
>> > > > > > > > > > > would
>> > > > > > > > > > > > like
>> > > > > > > > > > > > > > to see addressed in the 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
>> > > > > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > >
>> > >
>> >
>>
>
