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

fix(query): account_admin should support grant ownership to new role after drop an created role #14597

Merged
merged 9 commits into from
Feb 19, 2024
Merged

fix(query): account_admin should support grant ownership to new role after drop an created role #14597

merged 9 commits into from
Feb 19, 2024

Conversation

TCeason
Copy link
Collaborator

@TCeason TCeason commented Feb 5, 2024 •
edited by drmingdrmer
Loading

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

Summary

  1. if has ownership but owner role not exists, the owner role will be set to account_admin

  2. has_ownership should get ownership and role from meta

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

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Feb 5, 2024
@TCeason
Copy link
Collaborator Author

TCeason commented Feb 5, 2024

cc @everpcpc I already add label ci-benchmark-suites but not trigger the suite benchmark. Could you please help to this question?

@TCeason
Copy link
Collaborator Author

TCeason commented Feb 5, 2024

cc @everpcpc I already add label ci-benchmark-suites but not trigger the suite benchmark. Could you please help to this question?

Maybe need to add to here: https://github.com/datafuselabs/databend/blob/023972b9c2ec97b83a3c4e15d7254dcfc251046e/.github/workflows/cloud.yml#L115?

But that is strange.. Because this test not deploy in cloud.

@TCeason TCeason requested a review from flaneur2020 February 5, 2024 13:30
@TCeason TCeason force-pushed the fix_14572 branch 3 times, most recently from 26bffd6 to e76e478 Compare February 5, 2024 14:59
flaneur2020
flaneur2020 approved these changes Feb 6, 2024
View reviewed changes
Copy link
Member

@flaneur2020 flaneur2020 left a comment

Choose a reason for hiding this comment

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

LGTM

@TCeason TCeason requested a review from flaneur2020 February 6, 2024 05:00
@TCeason TCeason added the ci-benchmark-local Benchmark: run only local test label Feb 6, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 6, 2024

Docker Image for PR

  • tag: pr-14597-1cfb85b

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

@TCeason TCeason added ci-benchmark Benchmark: run all test and removed ci-benchmark-local Benchmark: run only local test ci-benchmark Benchmark: run all test labels Feb 6, 2024
@TCeason TCeason changed the title fix(query): after drop role account_admin should support grant ownership to new role fix(query): account_admin should support grant ownership to new role after drop an created role Feb 19, 2024
@TCeason
fix(query): after drop role account_admin should support grant owners…
3a0502f 
…hip to new role

1. if has ownership but owner role not exists, the owner role will be
   set to account_admin

2. has_ownership should get ownership and role from meta
@TCeason
modify api get_ownership
4241f56 
if has ownership but role not exists

this object's owner set to account_admin
@TCeason
fix err
e848a7b
@TCeason
refactor has_ownership
de8aa61
@TCeason
fix build err
68f0ca2
@TCeason
find_object_owner driectly return role_name
41e5dd1
@BohuTANG BohuTANG requested a review from flaneur2020 February 19, 2024 03:18
flaneur2020
flaneur2020 approved these changes Feb 19, 2024
View reviewed changes
Copy link
Member

@flaneur2020 flaneur2020 left a comment

Choose a reason for hiding this comment

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

LGTM

@BohuTANG BohuTANG merged commit 6f47727 into databendlabs:main Feb 19, 2024
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flaneur2020 flaneur2020 flaneur2020 approved these changes

Assignees
No one assigned
Labels
pr-bugfix this PR patches a bug in codebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: Account admin role need support grant ownership
4 participants
@TCeason @flaneur2020 @BohuTANG @everpcpc