Skip to content

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

Merged
showuon merged 2 commits into
apache:trunkfrom
Hongten:bugfix/acl_doc_bugfix
Apr 24, 2022
Merged

KAFKA-13852: Kafka Acl documentation bug for wildcard '*'#12090
showuon merged 2 commits into
apache:trunkfrom
Hongten:bugfix/acl_doc_bugfix

Conversation

@Hongten

@Hongten Hongten commented Apr 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
dengziming
dengziming approved these changes Apr 23, 2022
View reviewed changes

@dengziming dengziming left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

showuon
showuon reviewed Apr 24, 2022
View reviewed changes

@showuon showuon left a comment

Copy link
Copy Markdown
Member

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

Hongten commented Apr 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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)

showuon
showuon approved these changes Apr 24, 2022
View reviewed changes

@showuon showuon left a comment

Copy link
Copy Markdown
Member

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

showuon commented Apr 24, 2022

Copy link
Copy Markdown
Member

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

showuon commented Apr 24, 2022

Copy link
Copy Markdown
Member

@Hongten , thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dengziming dengziming dengziming approved these changes

@showuon showuon showuon approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Hongten @showuon @dengziming