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

Add auth action for package management service #8893

Merged
merged 17 commits into from
Jan 21, 2021
Merged

Add auth action for package management service #8893

merged 17 commits into from
Jan 21, 2021

Conversation

zymap
Copy link
Member

@zymap zymap commented Dec 10, 2020 •
edited
Loading

Master Issue: #8676

Motivation

Add auth action 'package' for package management operations.
Only the role who is granted the 'package' permission can do
the package operations.

Modifications

  • Add 'package' auth action and check the permissions when access the REST API
  • Add integration test for this

Verify this change

  • Add integration test for this

@zymap zymap marked this pull request as draft December 10, 2020 08:47
@zymap zymap force-pushed the package-management-auth branch from 59ed5cd to 310c877 Compare December 10, 2020 08:48
@zymap zymap mentioned this pull request Dec 10, 2020
Closed
8 tasks
zymap added 3 commits December 14, 2020 19:52
@zymap
Add auth action for package management service
0150d1c 
---

Master Issue: apache#8676

*Motivation*

Add auth action 'package' for package management operations.
Only the role who is granted the 'package' permission can do
the package operations.

*Modifications*

- Add 'package' auth action and check the permissions when access the REST API
- Add integration test for this

*Verify this change*

- Add integraion test for this
@zymap
Add integration tests for this
e662769
@zymap
Fix the tests
45cc622
@zymap zymap force-pushed the package-management-auth branch from 310c877 to 45cc622 Compare December 15, 2020 02:24
@zymap zymap changed the title (WIP)Add auth action for package management service Add auth action for package management service Dec 15, 2020
@zymap zymap marked this pull request as ready for review December 15, 2020 02:31
@zymap
Copy link
Member Author

zymap commented Dec 16, 2020

/pulsarbot run-failure-checks

3 similar comments
@zymap
Copy link
Member Author

zymap commented Dec 16, 2020

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Dec 16, 2020

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Dec 16, 2020

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Dec 21, 2020

@codelipenghui Please take a look when you have time. Thanks.

@zymap zymap force-pushed the package-management-auth branch from 517fd34 to aedc6ae Compare December 25, 2020 09:41
@zymap
Copy link
Member Author

zymap commented Jan 4, 2021

@sijie I use the allowNamespaceOperationAsync for checking the permission. I found we only do the tenant admin check in it. Should we extend a more detailed check of the operations?

@zymap
Copy link
Member Author

zymap commented Jan 4, 2021

/pulsarbot run-failure-checks

@sijie
Copy link
Member

sijie commented Jan 4, 2021

I found we only do the tenant admin check in it. Should we extend a more detailed check of the operations?

We need to add finer granular permissions for package management.

@zymap
Copy link
Member Author

zymap commented Jan 5, 2021

@sijie Ok. I open a new issue #9122 to track that and reconsider the auth implement in the package management service. I will improve the permissions in the package management with another PR.
This PR enables the package manager has the ability to enable auth and access with the proper permissions. So could we merge this PR first?

@sijie
Copy link
Member

sijie commented Jan 5, 2021

@zymap why do you need a separate issue? We are introducing auth action for the first time. We should get the implementation correctly. Let's not merge this until we have a correct implementation.

@zymap
Copy link
Member Author

zymap commented Jan 13, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 18, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 18, 2021

@sijie I improve the auth implement. Please take a look when you have time. Thanks.

@zymap
Copy link
Member Author

zymap commented Jan 18, 2021

/pulsarbot run-failure-checks

5 similar comments
@zymap
Copy link
Member Author

zymap commented Jan 19, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 19, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 19, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 19, 2021

/pulsarbot run-failure-checks

@zymap
Copy link
Member Author

zymap commented Jan 20, 2021

/pulsarbot run-failure-checks

@sijie sijie added this to the 2.8.0 milestone Jan 21, 2021
sijie
sijie approved these changes Jan 21, 2021
View reviewed changes
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@zymap overall looks strong to me! +1

@zymap
Copy link
Member Author

zymap commented Jan 21, 2021

Thanks!

@codelipenghui codelipenghui merged commit 7c5d6b1 into apache:master Jan 21, 2021
@zymap zymap mentioned this pull request Jan 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@sijie sijie sijie approved these changes

Assignees

@zymap zymap

Labels
None yet
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@zymap @sijie @codelipenghui