kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rsiva...@apache.org
Subject [kafka] branch trunk updated: MINOR: Tidy up pattern type comparisons, remove unused producer-id (#5593)
Date Tue, 04 Sep 2018 20:24:13 GMT
This is an automated email from the ASF dual-hosted git repository.

rsivaram pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 4a46a35  MINOR: Tidy up pattern type comparisons, remove unused producer-id (#5593)
4a46a35 is described below

commit 4a46a353482116ed5502737f7809bc6c0d37bf36
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
AuthorDate: Tue Sep 4 21:24:06 2018 +0100

    MINOR: Tidy up pattern type comparisons, remove unused producer-id (#5593)
    
    Reviewers: Ismael Juma <ismael@juma.me.uk>, Manikumar Reddy <manikumar.reddy@gmail.com>
---
 .../common/resource/ResourcePatternFilter.java     |  2 +-
 core/src/main/scala/kafka/admin/AclCommand.scala   |  2 +-
 .../main/scala/kafka/security/SecurityUtils.scala  |  1 +
 .../main/scala/kafka/security/auth/Resource.scala  | 11 ++++----
 core/src/main/scala/kafka/server/KafkaApis.scala   |  3 +-
 core/src/main/scala/kafka/zk/ZkData.scala          |  4 +--
 .../scala/unit/kafka/admin/AclCommandTest.scala    | 32 +++++++++++++++++++++-
 7 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/clients/src/main/java/org/apache/kafka/common/resource/ResourcePatternFilter.java
b/clients/src/main/java/org/apache/kafka/common/resource/ResourcePatternFilter.java
index 83f5c88..6f511c9 100644
--- a/clients/src/main/java/org/apache/kafka/common/resource/ResourcePatternFilter.java
+++ b/clients/src/main/java/org/apache/kafka/common/resource/ResourcePatternFilter.java
@@ -139,7 +139,7 @@ public class ResourcePatternFilter {
         if (name == null)
             return "Resource name is NULL.";
         if (patternType == PatternType.MATCH)
-            return "Resource pattern type is ANY.";
+            return "Resource pattern type is MATCH.";
         if (patternType == PatternType.UNKNOWN)
             return "Resource pattern type is UNKNOWN.";
         return null;
diff --git a/core/src/main/scala/kafka/admin/AclCommand.scala b/core/src/main/scala/kafka/admin/AclCommand.scala
index e86e1a3..31e6c53 100644
--- a/core/src/main/scala/kafka/admin/AclCommand.scala
+++ b/core/src/main/scala/kafka/admin/AclCommand.scala
@@ -88,7 +88,7 @@ object AclCommand extends Logging {
 
   private def addAcl(opts: AclCommandOptions) {
     val patternType: PatternType = opts.options.valueOf(opts.resourcePatternType)
-    if (patternType == PatternType.MATCH || patternType == PatternType.ANY)
+    if (!patternType.isSpecific)
       CommandLineUtils.printUsageAndDie(opts.parser, s"A '--resource-pattern-type' value
of '$patternType' is not valid when adding acls.")
 
     withAuthorizer(opts) { authorizer =>
diff --git a/core/src/main/scala/kafka/security/SecurityUtils.scala b/core/src/main/scala/kafka/security/SecurityUtils.scala
index 74bd404..5d42871 100644
--- a/core/src/main/scala/kafka/security/SecurityUtils.scala
+++ b/core/src/main/scala/kafka/security/SecurityUtils.scala
@@ -50,4 +50,5 @@ object SecurityUtils {
     new AclBinding(resourcePattern, entry)
   }
 
+  def isClusterResource(name: String): Boolean = name.equals(Resource.ClusterResourceName)
 }
diff --git a/core/src/main/scala/kafka/security/auth/Resource.scala b/core/src/main/scala/kafka/security/auth/Resource.scala
index c475596..b062ce2 100644
--- a/core/src/main/scala/kafka/security/auth/Resource.scala
+++ b/core/src/main/scala/kafka/security/auth/Resource.scala
@@ -23,9 +23,11 @@ object Resource {
   val Separator = ":"
   val ClusterResourceName = "kafka-cluster"
   val ClusterResource = Resource(Cluster, Resource.ClusterResourceName, PatternType.LITERAL)
-  val ProducerIdResourceName = "producer-id"
   val WildCardResource = "*"
 
+  @deprecated("This resource name is not used by Kafka and will be removed in a future release",
since = "2.1")
+  val ProducerIdResourceName = "producer-id" // This is not used since we don't have a producer
id resource
+
   def fromString(str: String): Resource = {
     ResourceType.values.find(resourceType => str.startsWith(resourceType.name + Separator))
match {
       case None => throw new KafkaException("Invalid resource string: '" + str + "'")
@@ -53,11 +55,8 @@ object Resource {
  */
 case class Resource(resourceType: ResourceType, name: String, patternType: PatternType) {
 
-  if (patternType == PatternType.MATCH || patternType == PatternType.ANY)
-    throw new IllegalArgumentException("patternType must not be " + patternType)
-
-  if (patternType == PatternType.UNKNOWN)
-    throw new IllegalArgumentException("patternType must not be UNKNOWN")
+  if (!patternType.isSpecific)
+    throw new IllegalArgumentException(s"patternType must not be $patternType")
 
   /**
     * Create an instance of this class with the provided parameters.
diff --git a/core/src/main/scala/kafka/server/KafkaApis.scala b/core/src/main/scala/kafka/server/KafkaApis.scala
index 096ff38..3a81b89 100644
--- a/core/src/main/scala/kafka/server/KafkaApis.scala
+++ b/core/src/main/scala/kafka/server/KafkaApis.scala
@@ -1950,8 +1950,7 @@ class KafkaApis(val requestChannel: RequestChannel,
           SecurityUtils.convertToResourceAndAcl(aclCreation.acl.toFilter) match {
             case Left(apiError) => new AclCreationResponse(apiError)
             case Right((resource, acl)) => try {
-                if (resource.resourceType.equals(Cluster) &&
-                    !resource.name.equals(Resource.ClusterResourceName))
+                if (resource.resourceType.equals(Cluster) && !SecurityUtils.isClusterResource(resource.name))
                   throw new InvalidRequestException("The only valid name for the CLUSTER
resource is " +
                       Resource.ClusterResourceName)
                 if (resource.name.isEmpty)
diff --git a/core/src/main/scala/kafka/zk/ZkData.scala b/core/src/main/scala/kafka/zk/ZkData.scala
index 760bd67..ed687cb 100644
--- a/core/src/main/scala/kafka/zk/ZkData.scala
+++ b/core/src/main/scala/kafka/zk/ZkData.scala
@@ -497,9 +497,7 @@ sealed trait ZkAclStore {
 
 object ZkAclStore {
   private val storesByType: Map[PatternType, ZkAclStore] = PatternType.values
-    .filter(patternType => patternType != PatternType.MATCH)
-    .filter(patternType => patternType != PatternType.ANY)
-    .filter(patternType => patternType != PatternType.UNKNOWN)
+    .filter(_.isSpecific)
     .map(patternType => (patternType, create(patternType)))
     .toMap
 
diff --git a/core/src/test/scala/unit/kafka/admin/AclCommandTest.scala b/core/src/test/scala/unit/kafka/admin/AclCommandTest.scala
index 26ae073..05f6189 100644
--- a/core/src/test/scala/unit/kafka/admin/AclCommandTest.scala
+++ b/core/src/test/scala/unit/kafka/admin/AclCommandTest.scala
@@ -21,8 +21,9 @@ import java.util.Properties
 import kafka.admin.AclCommand.AclCommandOptions
 import kafka.security.auth._
 import kafka.server.KafkaConfig
-import kafka.utils.{Logging, TestUtils}
+import kafka.utils.{Exit, Logging, TestUtils}
 import kafka.zk.ZooKeeperTestHarness
+import org.apache.kafka.common.resource.PatternType
 import org.apache.kafka.common.resource.PatternType.{LITERAL, PREFIXED}
 import org.apache.kafka.common.security.auth.KafkaPrincipal
 import org.junit.{Before, Test}
@@ -158,6 +159,35 @@ class AclCommandTest extends ZooKeeperTestHarness with Logging {
     AclCommand.withAuthorizer(new AclCommandOptions(args))(null)
   }
 
+  @Test
+  def testPatternTypes() {
+    Exit.setExitProcedure { (status, _) =>
+      if (status == 1)
+        throw new RuntimeException("Exiting command")
+      else
+        throw new AssertionError(s"Unexpected exit with status $status")
+    }
+    def verifyPatternType(cmd: Array[String], isValid: Boolean): Unit = {
+      if (isValid)
+        AclCommand.main(cmd)
+      else
+        intercept[RuntimeException](AclCommand.main(cmd))
+    }
+    try {
+      PatternType.values.foreach { patternType =>
+        val addCmd = zkArgs ++ Array("--allow-principal", principal.toString, "--producer",
"--topic", "Test",
+          "--add", "--resource-pattern-type", patternType.toString)
+        verifyPatternType(addCmd, isValid = patternType.isSpecific)
+        val listCmd = zkArgs ++ Array("--topic", "Test", "--list", "--resource-pattern-type",
patternType.toString)
+        verifyPatternType(listCmd, isValid = patternType != PatternType.UNKNOWN)
+        val removeCmd = zkArgs ++ Array("--topic", "Test", "--force", "--remove", "--resource-pattern-type",
patternType.toString)
+        verifyPatternType(removeCmd, isValid = patternType != PatternType.UNKNOWN)
+      }
+    } finally {
+      Exit.resetExitProcedure()
+    }
+  }
+
   private def testRemove(resources: Set[Resource], resourceCmd: Array[String], brokerProps:
Properties) {
     for (resource <- resources) {
       AclCommand.main(zkArgs ++ resourceCmd :+ "--remove" :+ "--force")


Mime
View raw message