You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by rs...@apache.org on 2018/09/04 20:24:13 UTC
[kafka] branch trunk updated: MINOR: Tidy up pattern type
comparisons, remove unused producer-id (#5593)
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 <ra...@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 <is...@juma.me.uk>, Manikumar Reddy <ma...@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")