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.

Sign up for GitHub

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

Jump to bottom

[improve] clean the empty topicAuthenticationMap in zk when revoke permission #16815

Merged
merged 2 commits into from
Sep 4, 2022
Merged

[improve] clean the empty topicAuthenticationMap in zk when revoke permission #16815

merged 2 commits into from
Sep 4, 2022

Conversation

TakaHiR07
Copy link
Contributor

@TakaHiR07 TakaHiR07 commented Jul 27, 2022 •
edited
Loading

Motivation

Steps to reproduce:

  1. grant permission for role1 on topic "persistent://public/default/test"
  2. revoke permission for role1 on topic "persistent://public/default/test"
  3. zk remain empty record, as shown in the picture

企业微信截图_2725005a-a13e-4f07-b27a-112986e1b856

Modifications

If the topic has no roles after revoke permission, remove topicKey from TopicAuthenticationMap

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as PersistentTopicsTest.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-not-needed
    (Please explain why)

@TakaHiR07 TakaHiR07 changed the title improve revoke permission [improve] clean the empty topicAuthenticationMap in zk when revoke permission Jul 27, 2022
@codelipenghui codelipenghui requested a review from nodece August 8, 2022 10:18
@codelipenghui codelipenghui added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/security labels Aug 8, 2022
@codelipenghui codelipenghui added this to the 2.12.0 milestone Aug 8, 2022
codelipenghui
codelipenghui reviewed Aug 8, 2022
View reviewed changes
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

The change looks good, could you please help add a test for the new change?

@TakaHiR07
Copy link
Contributor Author

The change looks good, could you please help add a test for the new change?

Thanks for your review. I have added a test. PTAL

@Jason918
Copy link
Contributor

@TakaHiR07 Please rebase the master and resolve the conflict.

@github-actions
Copy link

@TakaHiR07 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

1 similar comment
@github-actions
Copy link

@TakaHiR07 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

@nodece nodece mentioned this pull request Aug 27, 2022
Closed
@TakaHiR07 TakaHiR07 force-pushed the improve_revoke_permission_async branch from 3b68294 to 1b31908 Compare August 30, 2022 07:28
@TakaHiR07
Copy link
Contributor Author

@TakaHiR07 Please rebase the master and resolve the conflict.

@Jason918 done.

@TakaHiR07
Copy link
Contributor Author

/pulsarbot run-failure-checks

Jason918
Jason918 approved these changes Sep 2, 2022
View reviewed changes
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@Jason918 Jason918 merged commit d139d88 into apache:master Sep 4, 2022
@nodece
Copy link
Member

nodece commented Sep 5, 2022

Link to #17393.

@mattisonchao
Copy link
Member

Hi @TakaHiR07
It looks like we got many conflicts when cherry-picking it to branch-2.9.
Would you mind pushing a PR to branch-2.9? (To avoid cherry-picking involving bugs)

@TakaHiR07 TakaHiR07 mentioned this pull request Sep 13, 2022
Closed
congbobo184 pushed a commit that referenced this pull request Nov 10, 2022
@TakaHiR07
[improve] clean the empty topicAuthenticationMap in zk when revoke pe…
0c6f899 
…rmission (#16815)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
(cherry picked from commit d139d88)
@congbobo184 congbobo184 added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Nov 10, 2022
congbobo184 pushed a commit that referenced this pull request Dec 1, 2022
@TakaHiR07
[improve] clean the empty topicAuthenticationMap in zk when revoke pe…
63096e6 
…rmission (#16815)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
(cherry picked from commit d139d88)
liangyepianzhou pushed a commit that referenced this pull request Dec 5, 2022
@TakaHiR07 @liangyepianzhou
[improve] clean the empty topicAuthenticationMap in zk when revoke pe…
7b711ca 
…rmission (#16815)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
(cherry picked from commit d139d88)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Dec 6, 2022
@TakaHiR07 @nicoloboschi
[improve] clean the empty topicAuthenticationMap in zk when revoke pe…
9aac483 
…rmission (apache#16815)

Co-authored-by: fanjianye <fanjianye@bigo.sg>
(cherry picked from commit d139d88)
(cherry picked from commit 7b711ca)
@TakaHiR07 TakaHiR07 mentioned this pull request Dec 8, 2023
Merged
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nodece nodece nodece approved these changes

@Jason918 Jason918 Jason918 approved these changes

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@TakaHiR07 TakaHiR07

Labels
area/security cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-label-missing release/2.8.5 release/2.9.4 release/2.10.3 release/2.11.5 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
9 participants
@TakaHiR07 @Jason918 @nodece @mattisonchao @codelipenghui @lhotari @Technoboy- @congbobo184 @liangyepianzhou