Hi Ying,
I think the question is can we add a module in the core which can take
up the dynamic config and does a block certain APIs. This module will be
called in each of the APIs like the authorizer does today to check if the API
is supported for the client.
Instead of throwing AuthorizationException like the authorizer does today it
can throw UnsupportedException.
Benefits are, we are keeping the authorizer interface as is and adding the
flexibility based on dynamic configs without the need for categorizing broker
APIs and it will be easy to extend to do additional options, like turning off
certain features which might be in interest to the service providers.
One drawback, It will introduce another call to check instead of centralizing
everything around Authorizer.
Thanks,
Harsha
On Mon, Feb 25, 2019, at 2:43 PM, Ying Zheng wrote:
> If you guys don't like the extension of authorizer interface, I will just
> propose a single broker dynamic configuration: client.min.api.version, to
> keep things simple.
>
> What do you think?
>
> On Mon, Feb 25, 2019 at 2:23 PM Ying Zheng <yi...@uber.com> wrote:
>
> > @Viktor Somogyi-Vass, @Harsha, It seems the biggest concern is the
> > backward-compatibility to the existing authorizers. We can put the new
> > method into a new trait / interface:
> > trait AuthorizerEx extends Authorizer {
> > def authorize(session: Session, operation: Operation, resource: Resource,
> > apiVersion: Short): Boolean
> > }
> >
> > When loading an authorizer class, broker will check if the class
> > implemented AuthorizerEx interface. If not, broker will wrapper the
> > Authorizer object with an Adapter class, in which authorizer(...
> > apiVersion) call is translated to the old authorizer() call. So that, both
> > old and new Authorizer is supported and can be treated as AuthorizerEx in
> > the new broker code.
> >
> > As for the broker dynamic configuration approach, I'm not sure how to
> > correctly categorize the 40+ broker APIs into a few categories.
> > For example, describe is used by producer, consumer, and admin. Should it
> > be controlled by producer.min.api.version or consumer.min.api.version?
> > Should producer.min.api.version apply to transaction operations?
> >
> >
> > On Mon, Feb 25, 2019 at 10:33 AM Harsha <ka...@harsha.io> wrote:
> >
> >> I think the motivation of the KIP is to configure which API we want to
> >> allow for a broker.
> >> This is challenging for a hosted service where you have customers with
> >> different versions of clients.
> >> It's not just about down conversion but for example transactions, there
> >> is a case where we do not want to allow users to start using transactions
> >> and there is no way to disable to this right now and as specified in the
> >> KIP, having a lock on which client versions we support.
> >> Authorizer's original purpose is to allow policies to be enforced for
> >> each of the Kafka APIs, specifically in the context of security.
> >> Extending this to a general purpose gatekeeper might not be suitable and
> >> as mentioned in the thread every implementation of authorizer needs to
> >> re-implement to provide the same set of functionality.
> >> I think it's better to add an implementation which will use a broker's
> >> dynamic config as mentioned in approach 1.
> >>
> >> Thanks,
> >> Harsha
> >>
> >> On Sat, Feb 23, 2019, at 6:21 AM, Ismael Juma wrote:
> >> > Thanks for the KIP. Have we considered the existing topic config that
> >> makes
> >> > it possible to disallow down conversions? That's the biggest downside in
> >> > allowing older clients.
> >> >
> >> > Ismael
> >> >
> >> > On Fri, Feb 22, 2019, 2:11 PM Ying Zheng <yi...@uber.com.invalid>
> >> wrote:
> >> >
> >> > >
> >> > >
> >> >
> >>
> >
>
