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-18803: The acls would appear at the wrong level of the metadata shell "tree" #18916

Merged
merged 3 commits into from
Feb 16, 2025

Conversation

Yunyung
Copy link
Contributor

@Yunyung Yunyung commented Feb 15, 2025

see #18845 (comment)

Committer Checklist (excluded from commit message)

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

@github-actions github-actions bot added triage PRs from the community kraft small Small PRs labels Feb 15, 2025
Copy link
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

@Yunyung the fix LGTM. Can you do two things:

  1. Manually test this patch by creating some ACLs and showing the output from metadata shell
  2. Add a unit test for the MetadataImageNode to ensure we see the "acls" node correctly

@github-actions github-actions bot removed the triage PRs from the community label Feb 16, 2025
@Yunyung
Copy link
Contributor Author

Yunyung commented Feb 16, 2025

  1. Manually test this patch by creating some ACLs and showing the output from metadata shell

Before:

$ ./bin/kafka-metadata-shell.sh --snapshot /tmp/kraft-combined-logs/__cluster_metadata-0/00000000000000000000.log
Loading...
[2025-02-16 13:58:14,107] ERROR Ignoring control record with type KRAFT_VERSION at offset 1 (org.apache.kafka.metadata.util.SnapshotFileReader)
[2025-02-16 13:58:14,110] ERROR Ignoring control record with type KRAFT_VOTERS at offset 2 (org.apache.kafka.metadata.util.SnapshotFileReader)
Starting...
[ Kafka Metadata Shell ]
>> ls image/acls/
5Mu_djJ5QZaw79Fc3URLfw
>>

After:

$ ./bin/kafka-metadata-shell.sh --snapshot /tmp/kraft-combined-logs/__cluster_metadata-0/00000000000000000000.log
Loading...
[2025-02-16 21:59:06,250] ERROR Ignoring control record with type KRAFT_VERSION at offset 1 (org.apache.kafka.metadata.util.SnapshotFileReader)
[2025-02-16 21:59:06,251] ERROR Ignoring control record with type KRAFT_VOTERS at offset 2 (org.apache.kafka.metadata.util.SnapshotFileReader)
Starting...
[ Kafka Metadata Shell ]
>> ls image/acls/
byId
>> ls image/acls/byId/
cnTIY8C5R5-PIm0EKywItg
>> 

  1. Add a unit test for the MetadataImageNode to ensure we see the "acls" node correctly

Done.

Thanks for the review, PTAL.

@chia7712 chia7712 merged commit d0e516a into apache:trunk Feb 16, 2025
9 checks passed
chia7712 pushed a commit that referenced this pull request Feb 16, 2025
chia7712 pushed a commit that referenced this pull request Feb 16, 2025
chia7712 pushed a commit that referenced this pull request Feb 16, 2025
@chia7712
Copy link
Member

cherry-pick to 4.0, 3.9 and 3.8

manoj-mathivanan pushed a commit to manoj-mathivanan/kafka that referenced this pull request Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants