kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin McCabe" <cmcc...@apache.org>
Subject Re: [VOTE] KIP-430 - Return Authorized Operations in Describe Responses
Date Fri, 24 May 2019 22:47:57 GMT
Hi all,

We discovered an issue with how compatibility was handled in this KIP.  There should always
be a way for the client to know that the information that was requested was omitted by the
broker because it was too old.  I changed the "Compatibility, Deprecation, and Migration Plan"
to add some information about how this should be done.

Specifically, I added this section:

 > When the AdminClient is talking to a broker which does not support 
 > KIP-430, it will fill in either null or UnsupportedVersionException for 
 > the returned ACL operations fields in objects.  For example, 
 > ConsumerGroupDescription#authorizedOperations will be null if the broker 
 > did not supply this information.  
 > DescribeClusterResult#authorizedOperations will throw an 
 > UnsupportedVersionException if the broker did not supply this information.

Also see https://github.com/apache/kafka/pull/6812/files

best,
Colin

On Wed, Feb 27, 2019, at 05:29, Rajini Sivaram wrote:
> The vote has passed with 5 binding votes (Harsha, Gwen, Manikumar, Colin,
> me) and 2 non-binding votes (Satish, Mickael). I will update the KIP page.
> 
> Many thanks to everyone for the feedback and votes.
> 
> Regards,
> 
> Rajini
> 
> 
> On Tue, Feb 26, 2019 at 6:41 PM Rajini Sivaram <rajinisivaram@gmail.com>
> wrote:
> 
> > Thanks Colin, I have updated the KIP to mention that we don't return
> > UNKNOWN, ANY or ALL.
> >
> > Regards,
> >
> > Rajini
> >
> > On Tue, Feb 26, 2019 at 6:10 PM Colin McCabe <cmccabe@apache.org> wrote:
> >
> >> Thanks, Rajini!  Just to make it clear, can we spell out that we don't
> >> set the UNKNOWN, ANY, or ALL bits ever?
> >>
> >> +1 once that's resolved.
> >>
> >> cheers,
> >> Colin
> >>
> >>
> >> On Mon, Feb 25, 2019, at 04:11, Rajini Sivaram wrote:
> >> > Hi Colin,
> >> >
> >> > Yes, it makes sense to reduce response size by using bit fields. Updated
> >> > the KIP.
> >> >
> >> > I have also updated the KIP to say that clients will ignore any bits
> >> set by
> >> > the broker that are unknown to the client, so there will be no UNKNOWN
> >> > operations in the set returned by AdminClient. Brokers may however set
> >> bits
> >> > regardless of client version. Does that match your expectation?
> >> >
> >> > Thank you,
> >> >
> >> > Rajini
> >> >
> >> >
> >> > On Sat, Feb 23, 2019 at 1:03 AM Colin McCabe <cmccabe@apache.org>
> >> wrote:
> >> >
> >> > > Hi Rajini,
> >> > >
> >> > > Thanks for the explanations.
> >> > >
> >> > > On Fri, Feb 22, 2019, at 11:59, Rajini Sivaram wrote:
> >> > > > Hi Colin,
> >> > > >
> >> > > > Thanks for the review. Sorry I meant that an array of INT8's,
each
> >> of
> >> > > which
> >> > > > is an AclOperation code will be returned. I have clarified that
in
> >> the
> >> > > KIP.
> >> > >
> >> > > Do you think it's worth considering a bitfield here still?  An array
> >> will
> >> > > take up at least 4 bytes for the length, plus whatever length the
> >> elements
> >> > > are.  A 32-bit bitfield would pretty much always take up less space.
> >> And
> >> > > we can have a new version of the RPC with 64 bits or whatever if we
> >> outgrow
> >> > > 32 operations.  MetadataResponse for a big cluster could contain
> >> quite a
> >> > > lot of topics, tens or hundreds of thousands.  So the space savings
> >> could
> >> > > be considerable.
> >> > >
> >> > > >
> >> > > > All permitted operations will be returned from the set of supported
> >> > > > operations on each resource. This is regardless of whether the
> >> access was
> >> > > > implicitly or explicitly granted. Have clarified that in the
KIP.
> >> > >
> >> > > Thanks.
> >> > >
> >> > > >
> >> > > > Since the values returned are INT8 codes, clients can simply
ignore
> >> any
> >> > > > they don't recognize. Java clients convert these into
> >> > > AclOperation.UNKNOWN.
> >> > > > That way we don't need to update Metadata/describe request versions
> >> when
> >> > > > new operations are added to a resource. This is consistent with
> >> > > > DescribeAcls behaviour. Have added this to the compatibility
> >> section of
> >> > > the
> >> > > > KIP.
> >> > >
> >> > > Displaying "unknown" for new AclOperations made sense for
> >> DescribeAcls,
> >> > > since the ACL is explicitly referencing the new AclOperation.   For
> >> > > example, if you upgrade your Kafka cluster to a new version that
> >> supports
> >> > > DESCRIBE_CONFIGS, your old ACLs still don't reference
> >> DESCRIBE_CONFIGS.
> >> > >
> >> > > In contrast, in the case here, existing topics (or other resources)
> >> might
> >> > > pick up the new ACLOperation just by upgrading Kafka.  For example,
> >> if you
> >> > > had ALL permission on a topic and you upgrade to a new version with
> >> > > DESCRIBE_CONFIGS, you now have DESCRIBE_CONFIGS permission on that
> >> topic.
> >> > > This would result in a lot of "unknowns" being displayed here, which
> >> might
> >> > > not be ideal.
> >> > >
> >> > > Also, there is an argument from intent-- the intention here is to
let
> >> you
> >> > > know what you can do with a resource that already exists.  Knowing
> >> that you
> >> > > can do an unknown thing isn't very useful.  In contrast, for
> >> DescribeAcls,
> >> > > knowing that an ACL references an operation your software is too old
> >> to
> >> > > understand is useful (you may choose not to modify that ACL, since
you
> >> > > don't know what it does, for example.)  What do you think?
> >> > >
> >> > > cheers,
> >> > > Colin
> >> > >
> >> > >
> >> > > >
> >> > > > Thank you,
> >> > > >
> >> > > > Rajini
> >> > > >
> >> > > >
> >> > > >
> >> > > > On Fri, Feb 22, 2019 at 6:46 PM Colin McCabe <cmccabe@apache.org>
> >> wrote:
> >> > > >
> >> > > > > Hi Rajini,
> >> > > > >
> >> > > > > Thanks for the KIP!
> >> > > > >
> >> > > > > The KIP specifies that "Authorized operations will be returned
as
> >> [an]
> >> > > > > INT8 consistent with [the] AclOperation used in ACL requests
and
> >> > > > > responses."  But there may be more than one AclOperation
that is
> >> > > applied to
> >> > > > > a given resource.  For example, a principal may have both
READ and
> >> > > WRITE
> >> > > > > permission on a topic.
> >> > > > >
> >> > > > > One option for representing this would be a bitfield.  A
32-bit
> >> > > bitfield
> >> > > > > could have the appropriate bits set.  For example, if READ
and
> >> WRITE
> >> > > > > operations were permitted, bits 3 and 4 could be set.
> >> > > > >
> >> > > > > Another thing to think about here is that certain AclOperations
> >> imply
> >> > > > > certain others.  For example, having WRITE on a topic gives
you
> >> > > DESCRIBE on
> >> > > > > that topic as well automatically.  Does that mean that a
topic
> >> with
> >> > > WRITE
> >> > > > > on it should automatically get DESCRIBE set in the bitfield?
 I
> >> would
> >> > > argue
> >> > > > > that the answer is yes, for consistency's sake.
> >> > > > >
> >> > > > > We will inevitably add new AclOperations over time, and
we have to
> >> > > think
> >> > > > > about how to do this in a compatible way.  The simplest
approach
> >> would
> >> > > be
> >> > > > > to just leave out the new AclOperations when a describe
request
> >> comes
> >> > > in
> >> > > > > from an older version client.  This should be spelled out
in the
> >> > > > > compatibility section.
> >> > > > >
> >> > > > > best,
> >> > > > > Colin
> >> > > > >
> >> > > > >
> >> > > > > On Thu, Feb 21, 2019, at 02:28, Rajini Sivaram wrote:
> >> > > > > > I would like to start vote on KIP-430 to optionally
obtain
> >> authorized
> >> > > > > > operations when describing resources:
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > >
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-430+-+Return+Authorized+Operations+in+Describe+Responses
> >> > > > > >
> >> > > > > > Thank you,
> >> > > > > >
> >> > > > > > Rajini
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>

Mime
View raw message