KAFKA-6841: Add support for Prefixed ACLs by piyushvijay · Pull Request #5079 · apache/kafka

piyushvijay · 2018-05-25T09:43:11Z

Details are in KIP-290:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

big-andy-coates

Hey @piyushvijay,

Thanks for the PR! I know this is not complete yet, but thought I'd chime in with some comments.

The feature freeze approaching fast, (it's next Tuesday). I know you've got other things on your plate to work on besides this. The timezone difference is also not helping us with the round-trips on reviews. All of which leads me to thinking I can step in and help.

How about this: when I come in on Tuesday, after the bank holiday, (UK morning), I'll review the PR again in what ever state it is. Then, rather than play ping-pong with you, I'll just get stuck in and make any remaining tweaks that are needed to get this across the finish line. How does that sound?

So, could you make sure you push any changes before Tuesday morning please @piyushvijay?

Thanks,

Andy

big-andy-coates · 2018-05-25T15:17:25Z

clients/src/main/java/org/apache/kafka/common/requests/CreateAclsRequest.java

Looks like this could just fall through.

big-andy-coates · 2018-05-25T15:19:10Z

clients/src/main/java/org/apache/kafka/common/requests/CreateAclsResponse.java

Add a comment explaining 'v1 same as v0, request has additional resource_name_type'

big-andy-coates · 2018-05-25T15:21:06Z

clients/src/main/java/org/apache/kafka/common/requests/DeleteAclsRequest.java

fall through?

big-andy-coates · 2018-05-25T15:22:24Z

clients/src/main/java/org/apache/kafka/common/requests/DescribeAclsRequest.java

fall through

piyushvijay · 2018-05-28T08:19:28Z

clients/src/main/java/org/apache/kafka/common/requests/DescribeAclsResponse.java

it has DESCRIBE_ACLS_RESOURCE_V1 instead of DESCRIBE_ACLS_RESOURCE_V0

big-andy-coates · 2018-05-25T15:58:35Z

clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java

'valueInZK' is poor name here - it's the name of the wildcard-suffixed pattern.
'intput' is actually a concrete resource name.

valueInZK won't have the '*' suffix, (as discussed on the thread). - update the java doc accordingly too.

Personally, I see a lot of scope for this being called in code where it shouldn't, (as it currently is). A more defensive API might be for the signature to be:

public static boolean matchWildcardSuffixed(Resource wildcard, Resource concrete) public static boolean matchWildcardSuffixed(ResourceFilter wildcard, Resource concrete)

And then to have the method throw if the wrong types of wildcard and concrete are passed in, i.e. wildcard should always have type WILDCARD_SUFFIXED and concrete should always to LITERAL

big-andy-coates · 2018-05-25T16:01:18Z

clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java

anti-pattern:

if (...) { return x; } else if (...) { return y; } else { return z; }

Can be simplified to:

if (...) { return x; } if (...) { return y; } return z;

big-andy-coates · 2018-05-25T16:23:12Z

core/src/main/scala/kafka/security/auth/Resource.scala

need to change the split(Separator, 2) to split(Separator, 3) me thinks.

big-andy-coates · 2018-05-25T16:40:12Z

core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala

Need to only call the wildcard-suffix matching logic if the valueInZk is wildcard-suffix

big-andy-coates · 2018-05-25T16:47:33Z

core/src/main/scala/kafka/utils/ZkUtils.scala

As per the mail thread, we should use 'Prefixed' rather than 'WildcardSuffixed' everywhere.

I think we decided to use 'WildcardSuffixed' ;)

The last suggestion on the thread was from @cmccabe and was for 'prefixed'. I'm also happy with this as its short! Wildcard Suffixed is a bit of a mouthful and is less intuitive - a good test for this is to try and explain how "wildcard suffixed ACLs" interact with literal and wildcard acls. It ends up being really confusing, as we've got 'wildcard-suffixed' and 'wildcard' ACLs - much easy to discuss and digest if its 'literal', 'prefixed' and 'wildcard' ACLs. :D

Yes, I think "prefixed" makes sense. Everyone who talks about this talks about it as "prefix" support. it's even in the name of the KIP and the JIRA.

piyushvijay · 2018-05-28T08:19:28Z

clients/src/main/java/org/apache/kafka/common/requests/DescribeAclsResponse.java

it has DESCRIBE_ACLS_RESOURCE_V1 instead of DESCRIBE_ACLS_RESOURCE_V0

piyushvijay · 2018-05-29T07:41:06Z

Hi @big-andy-coates,

Looks in a pretty good shape now to merge today and then iterate. Thanks for the offer I think it's manageable now. Please let me know if you have any concerns (expect some failing tests which I'll fix in the morning). I still need to add more tests and documentation (which can be done after Tuesday as per the release doc). Thanks for the comments and the offer.

Two things:

Default can and should be Literal to handle v0 functionality.
I think match algorithm should be different for delete vs get. We don't want delete to be too disruptive. Example: getAcls(Topic, "topicName", Literal) should return acls on Resource(Topic, "topic*", WildcardSuffixed) but deleteAcls(Topic, "topicName", Literal) should not delete those acls.
I would go one step further and think that deleteAcls() in AdminClient should probably take AclBinding as input instead of AclBindingFilter just like addAcls take in AclBinding and not AclBindingFilter. Thoughts?

This is a WIP and require major refactoring for storing Acls at dual location on ZK. I'm forward-porting it from 1.0.x and code has changed significantly since then. I'll post a new version tomorrow but this is for very early preview. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

This is a WIP. This change includs improvements to support storing Acls at dual location on ZK. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

This is a WIP. This change includes improvements to the matching method, critical bug fixes and addresses comments from the previous review. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

This change includes fixes for hung tests and improvements to the matching method. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

Fix a bug in process notification due to change in Resource.toString Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

Fix unit tests. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

piyushvijay · 2018-05-29T11:02:15Z

@big-andy-coates rebased and fixed all unit tests, except one which is related to matching in deleteAcls as pointed above.

big-andy-coates

Hi @piyushvijay,

Thanks for the new version!

I see the KIP affecting three main areas:

How the authorizer stores and evaluates ACLs
How users create/read/update/delete ACLs via the kafka-acls.sh script
How users create/read/update/delete ACLs via the broker API / AdminClient.

The PR at the moment focuses almost exclusively on only the first point, and I think we need to be thinking about the implications on the other two as well, as these are key to the overall design making sense.

We need to see the changes to the CLI. I think we discussed a new command line switch to control the name type, which needs adding. We then need to think about how the other commands should work, e.g. the add/remove commands need to work for literal and prefixed resource names, but what about list? Personally, I would think it most useful if list just took a concrete resource name, (i.e. no resourceNameType), and then listed all the ACLs that affect it.

How do you see the DESCRIBE_ACLS and DELETE_ACLS broker calls working? Your current implementation is passing a RESOURCE_NAME_TYPE. However, like the CLI, I don't think DESCRIBE_ACLS should take a resource name type as I think it should work on concrete resources. We could maybe accept an optional resource name type, which allows ACLs within a specific ZK node to be listed, but should also accept a null resource name type, which should be taken to mean 'return all acls affecting this concrete resource'. The delete side could do something similar to allow users to delete only ACLS on literal, or only on prefixed, resources, or pass some 'Any' marked to delete all.

I'm concerned about the lack of tests for the new functionality, and lack of any documentation changes. I realise that you're probably thinking that this can be done later, but I think actually going through the motions of documenting both the CLI and API/adminClient changes will bring to light issues and ambiguities. The documentation can also drive the functional testing.

I've also been thinking about the changes to the Authorizer itself. The code at the moment is a bit of a mess, (that's the existing code, not yours!), and we're having to add a ResourceNameType all over the place, including the Resource call passed to the Authorizer methods. I think it would help if the resource name type was optional, i.e. could be null/Any, rather than defaulting to literal. Resources with a null, (or maybe ANY), name type should be treated as a 'concrete' resource, and hence be affected by both ACLs on Literal and Prefixed resources.

This change to Resource would also allow the getAcls call to be used to get ACLs for a specific path in ZK, e.g. getAcls(Resource(Group, "foo*", Literal)) or get all ACLS for a specific concrete resource, e.g. getAcls(Resource(Group, "foo*, Any)). The Authorizers remove methods would also handle this optional name type.

Oh, and we should definitely rename this feature 'prefixed' rather than 'wildcard-suffixed'. :D

In summary, I think there is still a chunk of work to do! So, again, I'll offer to pick this up. What I'm thinking is that you finished off what you think needs doing for the main authorize call work. (Going though my comments, adding tests, etc). Then I could pick up the other areas of work.

big-andy-coates · 2018-05-29T12:49:04Z

checkstyle/suppressions.xml


    <suppress checks="CyclomaticComplexity"
-              files="(ConsumerCoordinator|Fetcher|Sender|KafkaProducer|BufferPool|ConfigDef|RecordAccumulator|KerberosLogin|AbstractRequest|AbstractResponse|Selector|SslFactory|SslTransportLayer|SaslClientAuthenticator|SaslClientCallbackHandler).java"/>
+              files="(ConsumerCoordinator|Fetcher|Sender|KafkaProducer|BufferPool|ConfigDef|RecordAccumulator|KerberosLogin|AbstractRequest|AbstractResponse|Selector|SslFactory|SslTransportLayer|SaslClientAuthenticator|SaslClientCallbackHandler|SecurityUtils).java"/>


Don't add more exclusions! Fix the code!

big-andy-coates · 2018-05-29T12:50:57Z

clients/src/main/java/org/apache/kafka/common/protocol/types/Field.java

            super(name, Type.INT8, docString, false, null);
        }
+        public Int8(String name, String docString, byte byt) {
+            super(name, Type.INT8, docString, true, byt);


nit: rename byt to defaultValue

big-andy-coates · 2018-05-29T12:57:12Z

clients/src/main/java/org/apache/kafka/common/requests/Resource.java


-    public Resource(ResourceType type, String name) {
+    /**
+     * @param type resource type


Would be good to document which of these, if any, can be null and what that means. (Same for other versions of the constructor and other classes of the same name, ResourceFilter and the AclBindings* family).

Also, enforce this with null checks Objects.requireNotNull(param, "param")

big-andy-coates · 2018-05-29T12:58:46Z

clients/src/main/java/org/apache/kafka/common/requests/Resource.java

        Resource resource = (Resource) o;

-        return type == resource.type && name.equals(resource.name);
+        return type == resource.type && name.equals(resource.name) && resourceNameType.equals(resource.resourceNameType);


Use Objects.equals for non-primitives.

big-andy-coates · 2018-05-29T12:59:02Z

clients/src/main/java/org/apache/kafka/common/requests/Resource.java

    public int hashCode() {
        int result = type.hashCode();
        result = 31 * result + name.hashCode();
+        result = 31 * result + resourceNameType.hashCode();


Switch this to Objects.hashCode()

big-andy-coates · 2018-05-29T15:13:12Z

core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala

  }

+  @Test
+  def testAuthorizeTrueOnWildCardAcl(): Unit = {


This test needs splitting - a unit test should test one thing.

Splits:

shouldDenyAccessIfNoAclsPresent

shouldAllowAccessIfWildcardAclPresent

shouldDenyAccessWhenAclRemoved

shouldAllowAccessIfPrefixedAclMatches

Then add more positive & negative tests to cover new prefixed authorise functionality. e.g.

shouldAllowAccessIfPrefixedAclIsExactMatch.

shouldDenyAccessIfPrefixedAclDoesNotMatch.

shouldThrowFromAuthorizeIfNotConcreteResource

etc.

big-andy-coates · 2018-05-29T15:14:24Z

core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala

+  }
+
+  @Test
+  def testMatchPrincipal(): Unit = {


PR should not change principal handling...

big-andy-coates · 2018-05-29T15:14:46Z

core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala

+  }
+
+  @Test
+  def testGetAcls(): Unit = {


Split into many tests.

big-andy-coates · 2018-05-29T15:15:02Z

core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala

+  }
+
+  @Test
+  def testGetAclsPrincipal(): Unit = {


PR should not change principal handling...

big-andy-coates · 2018-05-29T15:15:26Z

core/src/test/scala/unit/kafka/utils/TestUtils.scala

 import java.util.concurrent.{Callable, Executors, TimeUnit}
-import javax.net.ssl.X509TrustManager

+import javax.net.ssl.X509TrustManager


white space only change.

junrao

@piyushvijay : Thanks for the patch. A few comments below. Also, during the rolling upgrade, it's possible that some brokers can get the changes through the prefix acl change path, but some others can't. So, at the minimum, we probably want to document in upgrade.html that the prefix acl feature won't work fully until the whole cluster is upgraded.

junrao · 2018-05-30T17:34:39Z

core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala

+    */
+  def matchPrincipal(valueInAcl: KafkaPrincipal, input: KafkaPrincipal): Boolean = {
+    (valueInAcl.getPrincipalType == input.getPrincipalType || valueInAcl.getPrincipalType == Acl.WildCardString) &&
+      (valueInAcl.getName.equals(input.getName) || valueInAcl.getName.equals(Acl.WildCardString))


Agree with @big-andy-coates, it seems that we agreed not to extend the prefix support for principals. The principal type is currently intended for things like groups. So, we probably don't want to overload it with wildcard. Finally, we have to make sure that * principal set in the current way still works.

junrao · 2018-05-30T17:54:13Z

clients/src/main/java/org/apache/kafka/common/requests/ResourceNameType.java

+    ANY((byte) 1),
+
+    /**
+     * A Kafka topic.


The comment is accurate. It should be sth like "A concrete type such as a specific Kafka topic".

junrao · 2018-05-30T17:54:25Z

clients/src/main/java/org/apache/kafka/common/requests/ResourceNameType.java

+    LITERAL((byte) 2),
+
+    /**
+     * A consumer group.


Inaccurate comment.

junrao · 2018-05-30T18:35:46Z

clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java

+                                || SecurityUtils.matchWildcardSuffixedString(stored.name() + WILDCARD_MARKER, input.name() + WILDCARD_MARKER);
+                    case LITERAL:
+                        return input.name() == null
+                                || SecurityUtils.matchWildcardSuffixedString(stored.name() + WILDCARD_MARKER, input.name());


Hmm, this is a bit weird. The stores.name() should have * as the suffix already, right?

Hey @junrao,

It was discussed on the dev channel that we shouldn't store or pass around the '' the end as this then requires places to validate the resource name ends in a '' on API calls and when loading from ZK.

Also, with the rebrand of this from 'wildcard-suffixed' to simply 'prefixed' then I think we can drop the '' completely. e.g. the user would add an ACL to any topic resource the has the prefix 'foo'. Look mum, no asterisks! This also helps separate this from the current 'wildcard' support i.e. ''.

Ok. Then the KIP wiki needs to be updated.

KIP updated.

junrao · 2018-05-30T18:37:19Z

clients/src/main/java/org/apache/kafka/common/utils/SecurityUtils.java

+                                || stored.name().equals(WILDCARD_MARKER);
+                    case WILDCARD_SUFFIXED:
+                        return input.name() == null
+                                || SecurityUtils.matchWildcardSuffixedString(stored.name(), input.name() + WILDCARD_MARKER);


If the filter is of WILDCARD_SUFFIXED, we'd expect its name to be suffixed with* already, right?

junrao · 2018-05-30T21:22:19Z

clients/src/main/java/org/apache/kafka/common/requests/CreateAclsRequest.java

                    OPERATION,
                    PERMISSION_TYPE))));

+    private static final Schema CREATE_ACLS_REQUEST_V1 = new Schema(


It would be useful to add a comment on what has changed in v1. Ditto in other changed requests.

junrao · 2018-05-30T21:37:34Z

clients/src/main/java/org/apache/kafka/common/requests/DeleteAclsResponse.java

+            ERROR_MESSAGE,
+            RESOURCE_TYPE,
+            RESOURCE_NAME,
+            RESOURCE_NAME_TYPE,


Since we are adding a new field here, toStruct(short version) and DeleteAclsResponse(Struct) need to be adjusted to take into account whether the new field is present or not.

junrao · 2018-05-30T21:42:23Z

clients/src/main/java/org/apache/kafka/common/requests/DescribeAclsResponse.java


+    private static final Schema DESCRIBE_ACLS_RESOURCE_V1 = new Schema(
+            RESOURCE_TYPE,
+            RESOURCE_NAME,


Similar to the above, since we are adding a new field here, toStruct(short version) and DescribeAclsResponse(Struct) need to be adjusted to take into account whether the new field is present or not.

junrao · 2018-05-30T22:21:52Z

core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala


  private def updateAclChangedFlag(resource: Resource) {
-    zkClient.createAclChangeNotification(resource.toString)
+    zkClient.createAclChangeNotification(AclStore.fromResource(resource), resource.toString)


Since resource.toString() doesn't encode resourceNameType, we need to obtain resourceNameType from the acl change path in AclChangedNotificationHandler.

junrao · 2018-05-30T23:25:58Z

core/src/main/scala/kafka/utils/ZkUtils.scala

  val KafkaAclPath = "/kafka-acl"
  val KafkaAclChangesPath = "/kafka-acl-changes"
+  val KafkaWildcardSuffixedAclPath = "/kafka-wildcard-acl"
+  val KafkaWildcardSuffixedAclChangesPath = "/kafka-wildcard-acl-changes"


We should just use the one defined in ZkData.

Given the existing KafkaAclPath and KafkaAclChangePath exist in ZkUtils already, I think it makes sense to add the new ones in here as well. (I know myself, as a user of Kafka, I've used this class to pick up path constants).

I'll make the ZkData uses reference ZkUtils, so the strings are only defined in one place.

How does that sound?

It's just that our plan is to phase out ZkUtils since it uses the old 101tec.ZkClient api.

Also ZkUtils is not a public API class even though lots of people use it. So, we kept it as it is for now to give people a chance to upgrade to the AdminClient, but we don't want to add anything new to it.

junrao · 2018-05-31T18:26:31Z

@big-andy-coates and @big-andy-coates A couple of other comments. (1) Currently, we have 2 almost identical Resource class, one in o.a.k.c.resource and another in o.a.k.c.requests. @ijuma mentioned that they were there because the code was added in parallel and we should just keep one. Since we are evolving Resource, perhaps we can just keep o.a.k.c.resource.Resource since it's public. We probably don't want to introduce ResourceNameType in 2 different packages. (2) The KIP proposes to propagate prefix acl changes through a separate path /kafka-wildcard-acl-changes in ZK. Another approach is to propagate the change on the existing path /kafka-acl-changes, but extend the value from a string to a JSON that specifies the resource name type. This has the slight benefit that in the future, if we want to add some other forms of wildcard, we don't need to introduce a separate change propagation path. To prevent old brokers from breaking during upgrade, we can fail the prefix acl requests before the inter.broker.protocol has been upgraded to 2.0.

big-andy-coates · 2018-06-01T13:59:15Z

Hey @junrao,

Thanks for the feedback!

(1) Currently, we have 2 almost identical Resource class, one in o.a.k.c.resource and another in o.a.k.c.requests. @ijuma mentioned that they were there because the code was added in parallel and we should just keep one. Since we are evolving Resource, perhaps we can just keep o.a.k.c.resource.Resource since it's public. We probably don't want to introduce ResourceNameType in 2 different packages.

I've a local change that removes the need for the two ResourceNameType classes. This means only o.a.k.c.resource.Resource if modified now. I can look to remove the other Resource class in a secondary PR.

Though... TBH, the ResourceNameType makes no conceptual sense where the other Resource class is used. It makes no real sense on either Resource class. This comes back to the issue that really AclBinding and the CRUD operations on the Authorizer shouldn't be using Resource, but something else, e.g. ResourceFilter or ResourceMatcher. A Resource should be a concrete resource: it should have no concept of its ResourceNameType. Changing the Authorizer breaks a lot of downstream clients, so should be avoided, but can we fix AclBinding under this KIP? We can fix the authoriser as part of KIP-50. AclBinding is marked as Evolving.

I can do this in a separate PR and we can discuss more there.

(2) The KIP proposes to propagate prefix acl changes through a separate path /kafka-wildcard-acl-changes in ZK. Another approach is to propagate the change on the existing path /kafka-acl-changes, but extend the value from a string to a JSON that specifies the resource name type. This has the slight benefit that in the future, if we want to add some other forms of wildcard, we don't need to introduce a separate change propagation path. To prevent old brokers from breaking during upgrade, we can fail the prefix acl requests before the inter.broker.protocol has been upgraded to 2.0.

Nice idea! I think even if we fail any prefix acl requests if the interbroker protocol is < 2.0, we should still write the JSON values to a different path, e.g. /kafka-acl-changes-v2, as otherwise we could end up with a mixture of string / Json values for a time, and this might have tricky edge cases with downgrades. Of do you think this won't be an issue?

Again, I propose we do this in a follow up PR.

Minor refactoring and fixes. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+Prefixed+ACLs

big-andy-coates · 2018-06-04T18:33:16Z

If @piyushvijay, it looks like you're starting to work on this PR again... but I've already moved the code forward, as agreed, in a separate PR: #5117. (I've only worked in a separate PR as I don't have rights to push to your PR).

Things are going to get real messy if you're making changes in your own PR too! We are very very close to the deadline, so I really think we need to be focusing on one PR at the moment!

Can I suggest that you review the changes in my PR and, assuming your happy and @jun is happy, we can look to merge it.

I have absolutely no issues with you cloning my PR branch, once everyone is happy, and creating your own PR so that you get the credit of the commit if you want. Just make that very clear on my PR thread, and call this out to @junrao and @ijuma, to ensure they don't merge my PR.

piyushvijay · 2018-06-05T01:16:00Z

Let's keep moving your PR and we can discuss the credit part later ;)

big-andy-coates reviewed May 25, 2018

View reviewed changes

piyushvijay commented May 28, 2018

View reviewed changes

piyushvijay force-pushed the wildcardAcls branch 3 times, most recently from a497ea2 to 1af6609 Compare May 29, 2018 07:29

piyushvijay added 6 commits May 29, 2018 02:31

[WIP] KAFKA-6841: Add support for wildcard suffixed ACLs

c7e8941

This is a WIP. This change includs improvements to support storing Acls at dual location on ZK. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

[WIP] KAFKA-6841: Add support for wildcard suffixed ACLs

2897dde

This change includes fixes for hung tests and improvements to the matching method. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

[WIP] KAFKA-6841: Add support for wildcard suffixed ACLs

251ab66

Fix a bug in process notification due to change in Resource.toString Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

[WIP] KAFKA-6841: Add support for wildcard suffixed ACLs

7cd4d87

Fix unit tests. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+wildcard+suffixed+ACLs

piyushvijay force-pushed the wildcardAcls branch from 1af6609 to 7cd4d87 Compare May 29, 2018 11:01

piyushvijay changed the title ~~[WIP] KAFKA-6841: Add support for wildcard suffixed ACLs~~ KAFKA-6841: Add support for wildcard suffixed ACLs May 29, 2018

big-andy-coates reviewed May 29, 2018

View reviewed changes

junrao reviewed May 31, 2018

View reviewed changes

big-andy-coates mentioned this pull request Jun 1, 2018

KIP-290 Prefixed ACLs #5117

Merged

7 tasks

piyushvijay changed the title ~~KAFKA-6841: Add support for wildcard suffixed ACLs~~ KAFKA-6841: Add support for Prefixed ACLs Jun 4, 2018

KAFKA-6841: Add support for Prefixed ACLs

25d2742

Minor refactoring and fixes. Details are in KIP-290: https://cwiki.apache.org/confluence/display/KAFKA/KIP-290%3A+Support+for+Prefixed+ACLs

piyushvijay closed this Jun 5, 2018

Conversation

piyushvijay commented May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

big-andy-coates left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

big-andy-coates May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

big-andy-coates May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyushvijay commented May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

piyushvijay commented May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

big-andy-coates left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

big-andy-coates May 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyushvijay commented May 25, 2018 •

edited

Loading

big-andy-coates left a comment •

edited

Loading

big-andy-coates May 25, 2018 •

edited

Loading

big-andy-coates May 29, 2018 •

edited

Loading

piyushvijay commented May 29, 2018 •

edited

Loading

piyushvijay commented May 29, 2018 •

edited

Loading

big-andy-coates left a comment •

edited

Loading

big-andy-coates May 29, 2018 •

edited

Loading