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 <rajinisiva...@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 <cmcc...@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 <cmcc...@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 <cmcc...@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 > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >
