You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "tinaselenge (via GitHub)" <gi...@apache.org> on 2023/05/02 19:21:04 UTC
[GitHub] [kafka] tinaselenge opened a new pull request, #13660: KAFKA-14662: Update the ACL list in the doc
tinaselenge opened a new pull request, #13660:
URL: https://github.com/apache/kafka/pull/13660
*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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tinaselenge commented on a diff in pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "tinaselenge (via GitHub)" <gi...@apache.org>.
tinaselenge commented on code in PR #13660:
URL: https://github.com/apache/kafka/pull/13660#discussion_r1183526354
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_PARTITION (56)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
Thank you @showuon. Looking at the other ApiKeys, they are listed several times for each operation primitive and resource it checks, e.g FETCH. I followed the same format for DESCRIBE_QUORUM and UPDATE_FEATURES as they both had clusterAction set to true here (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java#L101). However now looking at the code again, I don't think ClusterAction operation is checked as part of the authorization check for them but used later for throttling the requests. So I think I should remove them, what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a diff in pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on code in PR #13660:
URL: https://github.com/apache/kafka/pull/13660#discussion_r1183193194
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_PARTITION (56)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
Duplicated entry.
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
Duplicated entry. And it should be `Describe` `Cluster`, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tinaselenge commented on a diff in pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "tinaselenge (via GitHub)" <gi...@apache.org>.
tinaselenge commented on code in PR #13660:
URL: https://github.com/apache/kafka/pull/13660#discussion_r1183527261
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
Re duplication, I commented below. And yes, it should have been Describe. Thanks for spotting that :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a diff in pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on code in PR #13660:
URL: https://github.com/apache/kafka/pull/13660#discussion_r1185661328
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_PARTITION (56)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
I think we should remove them since the permission is not required for this API key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13660:
URL: https://github.com/apache/kafka/pull/13660#issuecomment-1536155265
Retriggering the CI build since it failed with infra's issue.
https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13660/3/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13660:
URL: https://github.com/apache/kafka/pull/13660#issuecomment-1537831054
Failed tests are unrelated:
```
Build / JDK 8 and Scala 2.12 / integration.kafka.server.FetchFromFollowerIntegrationTest.testRackAwareRangeAssignor()
Build / JDK 11 and Scala 2.13 / org.apache.kafka.tools.MetadataQuorumCommandTest.[1] Type=Raft-Combined, Name=testDescribeQuorumReplicationSuccessful, MetadataVersion=3.5-IV2, Security=PLAINTEXT
Build / JDK 11 and Scala 2.13 / org.apache.kafka.tools.MetadataQuorumCommandTest.[2] Type=Raft-Isolated, Name=testDescribeQuorumReplicationSuccessful, MetadataVersion=3.5-IV2, Security=PLAINTEXT
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon merged pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon merged PR #13660:
URL: https://github.com/apache/kafka/pull/13660
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tinaselenge commented on a diff in pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "tinaselenge (via GitHub)" <gi...@apache.org>.
tinaselenge commented on code in PR #13660:
URL: https://github.com/apache/kafka/pull/13660#discussion_r1183526354
##########
docs/security.html:
##########
@@ -2089,6 +2089,144 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
<td>Topic</td>
<td></td>
</tr>
+ <tr>
+ <td>DESCRIBE_CLIENT_QUOTAS (48)</td>
+ <td>DescribeConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_CLIENT_QUOTAS (49)</td>
+ <td>AlterConfigs</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_USER_SCRAM_CREDENTIALS (50)</td>
+ <td>Describe</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_USER_SCRAM_CREDENTIALS (51)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>VOTE (52)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>BEGIN_QUORUM_EPOCH (53)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>END_QUORUM_EPOCH (54)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>DESCRIBE_QUORUM (55)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>ALTER_PARTITION (56)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>ClusterAction</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
+ <tr>
+ <td>UPDATE_FEATURES (57)</td>
+ <td>Alter</td>
+ <td>Cluster</td>
+ <td></td>
+ </tr>
Review Comment:
Thank you @showuon. Looking at the other ApiKeys, they are listed several times for each operation primitive and resource it checks, e.g FETCH. I followed the same format for DESCRIBE_QUORUM and UPDATE_FEATURES as they both had clusterAction set to true here (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java#L101). However now looking at them again, I'm not sure where ClusterAction operation is checked for them. I only see Describe or Alter operations being checked on Cluster resource.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #13660: KAFKA-14662: Update the ACL list in the doc
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13660:
URL: https://github.com/apache/kafka/pull/13660#issuecomment-1541563266
backport to v3.5 branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org