Skip to content

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

Merged
Jason918 merged 2 commits into
apache:masterfrom
TakaHiR07:improve_revoke_permission_async
Sep 4, 2022
Merged

[improve] clean the empty topicAuthenticationMap in zk when revoke permission#16815
Jason918 merged 2 commits into
apache:masterfrom
TakaHiR07:improve_revoke_permission_async

Conversation

@TakaHiR07

@TakaHiR07 TakaHiR07 commented Jul 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@codelipenghui codelipenghui left a comment

Copy link
Copy Markdown
Contributor

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

TakaHiR07 commented Aug 8, 2022

Copy link
Copy Markdown
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

Jason918 commented Aug 27, 2022

Copy link
Copy Markdown
Contributor

@TakaHiR07 Please rebase the master and resolve the conflict.

@github-actions

github-actions Bot commented Aug 27, 2022

Copy link
Copy Markdown

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

1 similar comment
@github-actions

github-actions Bot commented Aug 27, 2022

Copy link
Copy Markdown

@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

TakaHiR07 commented Aug 30, 2022

Copy link
Copy Markdown
Contributor Author

@TakaHiR07 Please rebase the master and resolve the conflict.

@Jason918 done.

@TakaHiR07

TakaHiR07 commented Sep 2, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

Jason918
Jason918 approved these changes Sep 2, 2022
View reviewed changes

@Jason918 Jason918 left a comment

Copy link
Copy Markdown
Contributor

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

nodece commented Sep 5, 2022

Copy link
Copy Markdown
Member

Link to #17393.

@mattisonchao

mattisonchao commented Sep 13, 2022

Copy link
Copy Markdown
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

@Jason918 Jason918 Jason918 approved these changes

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