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

[KYUUBI #5687] Using session user as proxy user to execute statements at the server and group share level #5772

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

theoryxu
Copy link

@theoryxu theoryxu commented Nov 24, 2023

🔍 Description

Issue References 🔗

This pull request fixes #5687

Describe Your Solution 🔧

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklists

📝 Author Self Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

  • Pull request title is okay.
  • No license issues.
  • Milestone correctly set?
  • Test coverage is ok
  • Assignees are selected.
  • Minimum number of approvals
  • No changes are requested

Be nice. Be informative.

For caution, the parameter set false as the default

…ements at the server and group share level
@pan3793
Copy link
Member

pan3793 commented Nov 27, 2023

@theoryxu it would be nice if you could add a UT for this feature, to demonstrate the behavior differences with this feature enabled/disabled.

@theoryxu
Copy link
Author

theoryxu commented Nov 27, 2023

@theoryxu it would be nice if you could add a UT for this feature, to demonstrate the behavior differences with this feature enabled/disabled.

@pan3793 Thank you for your review.

I‘ll figure out how Kyuubi's UT framework works and add one for this.

By the way, Are there other flaws I should know about? I could fix it at once.

@pan3793
Copy link
Member

pan3793 commented Nov 27, 2023

@theoryxu I'm looking at the implementation in STS, which seems different, I suspect the behaviors are also different.

…ements at the server and group share level; Kerberos Compatible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] Using [session user] as [proxy user] to execute statements at the server share level
3 participants