Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-13852: Kafka Acl documentation bug for wildcard '*' #12090

Merged
merged 2 commits into from
Apr 24, 2022

Conversation

Hongten
Copy link
Contributor

@Hongten Hongten commented Apr 23, 2022

The bug for wildcard '*' in Kafka Acl documentation(docs/security.html).
In this fix, add single quotes for the wildcard '*' in the script.

Committer Checklist (excluded from commit message)

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

@Hongten Hongten changed the title [KAFKA-13852]Kafka Acl documentation bug for wildcard '*' KAFKA-13852]: Kafka Acl documentation bug for wildcard '*' Apr 23, 2022
@Hongten Hongten changed the title KAFKA-13852]: Kafka Acl documentation bug for wildcard '*' KAFKA-13852: Kafka Acl documentation bug for wildcard '*' Apr 23, 2022
Copy link
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hongten , thanks for the PR. So you mean the * without single quote will be replaced into all files under the current folder by bash, right? Interesting issue. Could you also update the option description in core/src/main/scala/kafka/admin/AclCommand.scala? i.e.:

A value of * indicates ACL should apply to all topics. -> A value of '*' indicates ACL should apply to all topics. (add single quote around *)

Thanks.

@Hongten
Copy link
Contributor Author

Hongten commented Apr 24, 2022

@Hongten , thanks for the PR. So you mean the * without single quote will be replaced into all files under the current folder by bash, right? Interesting issue. Could you also update the option description in core/src/main/scala/kafka/admin/AclCommand.scala? i.e.:

A value of * indicates ACL should apply to all topics. -> A value of '*' indicates ACL should apply to all topics. (add single quote around *)

Thanks.

hi @showuon , the * without single quotes will be replaced into One file(File names are sorted alphabetically) under the current folder by bash.
I will update the description. I try to set ACL for other resources(e.g transactional-id, group, delegation-token) with wildcard and encountered the same issue.
One more thing is User:* in the bach will encounter no matches found: User:*. We need to use User:'*'.
So I update all descriptions together.

> ~/bin/kafka-acls.sh --authorizer-properties zookeeper.connect=localhost:2181 --add --allow-principal User:Peter --allow-host 198.51.200.1 --operation Describe --delegation-token *

Adding ACLs for resource `ResourcePattern(resourceType=DELEGATION_TOKEN, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=DESCRIBE, permissionType=ALLOW)

Current ACLs for resource `ResourcePattern(resourceType=DELEGATION_TOKEN, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=DESCRIBE, permissionType=ALLOW)
> ~/bin/kafka-acls.sh --authorizer-properties zookeeper.connect=localhost:2181 --add --allow-principal User:Peter --allow-host 198.51.200.1 --operation Read --group *

Adding ACLs for resource `ResourcePattern(resourceType=GROUP, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=READ, permissionType=ALLOW)

Current ACLs for resource `ResourcePattern(resourceType=GROUP, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=READ, permissionType=ALLOW)
> ~/bin/kafka-acls.sh --authorizer-properties zookeeper.connect=localhost:2181 --add --allow-principal User:Peter --allow-host 198.51.200.1 --operation Write --transactional-id *

Adding ACLs for resource `ResourcePattern(resourceType=TRANSACTIONAL_ID, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=WRITE, permissionType=ALLOW)

Current ACLs for resource `ResourcePattern(resourceType=TRANSACTIONAL_ID, name=foo.txt, patternType=LITERAL)`:
    (principal=User:Peter, host=198.51.200.1, operation=WRITE, permissionType=ALLOW)

> ~bin/kafka-acls.sh --authorizer-properties zookeeper.connect=localhost:2181 --add --allow-principal User:'*' --allow-host 198.51.200.1 --operation Read --group my_group

Adding ACLs for resource `ResourcePattern(resourceType=GROUP, name=my_group, patternType=LITERAL)`:
    (principal=User:*, host=198.51.200.1, operation=READ, permissionType=ALLOW)

Current ACLs for resource `ResourcePattern(resourceType=GROUP, name=my_group, patternType=LITERAL)`:
    (principal=User:*, host=198.51.200.1, operation=READ, permissionType=ALLOW)

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the PR!

@showuon
Copy link
Contributor

showuon commented Apr 24, 2022

Failed tests are unrelated:

    Build / JDK 8 and Scala 2.12 / integration.kafka.server.FetchRequestBetweenDifferentIbpTest.testControllerNewToOldIBP()
    Build / JDK 8 and Scala 2.12 / kafka.server.KRaftClusterTest.testIncrementalAlterConfigs()
    Build / JDK 8 and Scala 2.12 / kafka.server.KRaftClusterTest.testSetLog4jConfigurations()
    Build / JDK 8 and Scala 2.12 / kafka.server.KRaftClusterTest.testLegacyAlterConfigs()
    Build / JDK 8 and Scala 2.12 / kafka.server.ProduceRequestTest.testSimpleProduceRequest()
    Build / JDK 8 and Scala 2.12 / org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldRejectNonExistentStoreName
    Build / JDK 11 and Scala 2.13 / kafka.admin.LeaderElectionCommandTest.[1] Type=Raft, Name=testElectionResultOutput, Security=PLAINTEXT
    Build / JDK 11 and Scala 2.13 / kafka.server.KRaftClusterTest.testIncrementalAlterConfigs()
    Build / JDK 11 and Scala 2.13 / kafka.server.KRaftClusterTest.testCreateClusterAndCreateListDeleteTopic()
    Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldRejectWronglyTypedStore

@showuon showuon merged commit ff3d42a into apache:trunk Apr 24, 2022
@showuon
Copy link
Contributor

showuon commented Apr 24, 2022

@Hongten , thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants