Skip to content

Support LDAPGroupProvider #4001

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

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

Support LDAPGroupProvider #4001

wants to merge 10 commits into from

Conversation

wheatxiong
Copy link
Contributor

@wheatxiong wheatxiong commented Dec 18, 2022

Why are the changes needed?

#3897 had exposed GroupProvider, we implement LDAPGroupProvider to special group mapping mechanisms through LDAP

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@github-actions github-actions bot added the kind:documentation Documentation is a feature! label Dec 19, 2022
@pan3793
Copy link
Member

pan3793 commented Dec 19, 2022

Hi @wheatxiong, the Apache GitHub repo requires the first contributors to run CI after approving, you can send a small PR to unlock it, e.g. #3965

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2022

Codecov Report

Merging #4001 (f7e8843) into master (7631e7d) will increase coverage by 52.95%.
The diff coverage is 38.88%.

❗ Current head f7e8843 differs from pull request most recent head 34ec82f. Consider uploading reports for the commit 34ec82f to get more accurate results

@@              Coverage Diff              @@
##             master    #4001       +/-   ##
=============================================
+ Coverage      0.00%   52.95%   +52.95%     
- Complexity        0       13       +13     
=============================================
  Files           564      548       -16     
  Lines         31311    29931     -1380     
  Branches       4098     4004       -94     
=============================================
+ Hits              0    15850    +15850     
+ Misses        31311    12618    -18693     
- Partials          0     1463     +1463     
Impacted Files Coverage Δ
.../org/apache/kyuubi/session/LDAPGroupProvider.scala 0.00% <0.00%> (ø)
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 97.34% <97.22%> (+97.34%) ⬆️

... and 554 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@wheatxiong
Copy link
Contributor Author

Hi @wheatxiong, the Apache GitHub repo requires the first contributors to run CI after approving, you can send a small PR to unlock it, e.g. #3965

OK, i get it

@wheatxiong
Copy link
Contributor Author

@pan3793 Please help to review this pr to prevent ugly code, thanks

@pan3793
Copy link
Member

pan3793 commented Jan 12, 2023

@wheatxiong sorry for the late reply, I'm working on #4152 which mainly referring the Hive codebase. AFAIK, the Hive ecosystem w/ LDAP is mature, let's keep the same behavior(configuration name and logic) w/ Hive LDAP unless the new design has significant advantages.

@wheatxiong
Copy link
Contributor Author

@wheatxiong sorry for the late reply, I'm working on #4152 which mainly referring the Hive codebase. AFAIK, the Hive ecosystem w/ LDAP is mature, let's keep the same behavior(configuration name and logic) w/ Hive LDAP unless the new design has significant advantages.

thanks, i will refer to the implementation of HIVE

@github-actions github-actions bot removed the kind:documentation Documentation is a feature! label Feb 8, 2023
@nqvuong1998
Copy link

Hi @wheatxiong @pan3793 , any update for LDAP group provider feature?

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.

5 participants