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

feat(query): task support ownership #15458

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented May 9, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Ownership cover Task Object

  1. Added Task to OwnershipObject, storing the task name (After confirmation that the task will not have a rename operation, storing the name is more appropriate)

  2. task. proto adds task_names. Added owner judgment in tasks/task_history_table.rs

  3. owenrship.proto added the OwnershipTaskObject

  4. user.proto added GrantTaskObject

  5. Add the global permission to CREATE TASK. drop/alter task requires the DROP/ALTER permission. Only the owner of a task can execute the show/desc/execute task command

  6. Delete showtasksinterpreter

  7. grant task ownership when a task is created and revoke task ownership when a task is dropped

  8. Add list_tasks_ownerships and only list __fd_object_owners//task-by-name/ prefix

  9. if user has global Super privilege, it can execute/drop/alter/create/show all tasks and tasks_history

Attention:

  1. The ddl operation of the task also retains the previous permission verification. That so long as has the Global level of super permissions can be to create a task/drop/execute/desc/show.

  2. Integration testing is somewhat difficult and has been tested manually in the dev environment. warehouse 'task_p'.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@TCeason TCeason requested a review from drmingdrmer as a code owner May 9, 2024 13:26
@TCeason TCeason marked this pull request as draft May 9, 2024 13:26
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label May 9, 2024
@TCeason TCeason force-pushed the task_owner branch 2 times, most recently from 7b8788b to 1cb5d08 Compare May 9, 2024 15:03
@TCeason TCeason force-pushed the task_owner branch 2 times, most recently from bb6e048 to 6bcba30 Compare May 10, 2024 08:54
@drmingdrmer
Copy link
Member

If this pull request is ready for review, please mark it as Ready for review.

If it is not yet ready, I would prefer to be removed from the reviewer list to avoid being overwhelmed by notifications from pushes. 🤔

@TCeason TCeason removed the request for review from drmingdrmer May 10, 2024 09:32
@TCeason TCeason added the ci-cloud Build docker image for cloud test label May 10, 2024
@TCeason TCeason force-pushed the task_owner branch 2 times, most recently from f3f341e to 5204627 Compare May 10, 2024 10:14
@datafuselabs datafuselabs deleted a comment from github-actions bot May 10, 2024
@TCeason TCeason added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels May 10, 2024
Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewed 13 of 38 files at r1, 13 of 22 files at r3, all commit messages.
Reviewable status: 26 of 40 files reviewed, 4 unresolved discussions (waiting on @TCeason and @ZhiHanZ)


src/meta/proto-conv/src/util.rs line 122 at r3 (raw file):

    (88, "2024-04-17: Add: SequenceMeta"),
    (89, "2024-04-19: Add: geometry_output_format settings"),
    (90, "2024-05-09: Add: GrantTaskObject"),

The name of the new message is OwnershipTaskObject?

@TCeason
Copy link
Collaborator Author

TCeason commented May 10, 2024

Reviewed 13 of 38 files at r1, 13 of 22 files at r3, all commit messages.
Reviewable status: 26 of 40 files reviewed, 4 unresolved discussions (waiting on @TCeason and @ZhiHanZ)


src/meta/proto-conv/src/util.rs line 122 at r3 (raw file):

    (88, "2024-04-17: Add: SequenceMeta"),
    (89, "2024-04-19: Add: geometry_output_format settings"),
    (90, "2024-05-09: Add: GrantTaskObject"),

The name of the new message is OwnershipTaskObject?

Two

owenrship.proto added the OwnershipTaskObject

user.proto added GrantTaskObject

Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 38 files at r1.
Reviewable status: 27 of 40 files reviewed, 3 unresolved discussions (waiting on @ZhiHanZ)

@TCeason TCeason added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels May 13, 2024
@TCeason
Copy link
Collaborator Author

TCeason commented May 13, 2024

This pr can be merged when this pr #15496 released.

@TCeason TCeason force-pushed the task_owner branch 4 times, most recently from 628a32a to 08e0fba Compare May 14, 2024 09:31
1. add function: list_task_ownerships, only list task prefix.

2. add visibility in task/task_history system table.

3. refactor showtasks, directly use select * from system.task;

4. fix ut err
2. add tasks and task_history to SYSTEM_TABLES_ALLOW_LIST

3. delete useless test
@TCeason TCeason added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels May 20, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot May 20, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot May 20, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot May 20, 2024
@TCeason TCeason added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels May 20, 2024
@datafuselabs datafuselabs deleted a comment from github-actions bot May 20, 2024
@TCeason TCeason removed the ci-cloud Build docker image for cloud test label May 20, 2024
@TCeason TCeason added ci-cloud Build docker image for cloud test and removed ci-cloud Build docker image for cloud test labels May 20, 2024
@TCeason TCeason added the ci-cloud Build docker image for cloud test label May 20, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-15458-297dc76

note: this image tag is only available for internal use,
please check the internal doc for more details.

@TCeason
Copy link
Collaborator Author

TCeason commented May 20, 2024

I generate some case and test this pr in cloud dev: warehouse 'task_p'.

So I think we can review this pr now. cc @flaneur2020 @ZhiHanZ

@TCeason TCeason marked this pull request as ready for review May 20, 2024 08:03
@TCeason TCeason marked this pull request as draft May 20, 2024 09:04
@TCeason TCeason marked this pull request as ready for review May 21, 2024 03:47
@TCeason TCeason requested a review from flaneur2020 May 21, 2024 03:52
@TCeason TCeason marked this pull request as draft May 23, 2024 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cloud Build docker image for cloud test pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add ownership to Task
3 participants