Fix tenant label for custom tenant when both Global and Private tenan… #1277
Conversation
…ts are disabled Signed-off-by: Ryan Liang <[email protected]>
There was a problem hiding this comment.
Thank you @RyanL1997! The change looks good to me, but I'd like to see a test to verify the change.
Signed-off-by: Ryan Liang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1277 +/- ##
=======================================
Coverage 71.78% 71.78%
=======================================
Files 88 88
Lines 2027 2027
Branches 269 274 +5
=======================================
Hits 1455 1455
Misses 509 509
Partials 63 63 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| Object.keys(availableTenantsClone).length > 1 && | ||
| availableTenantsClone.hasOwnProperty(globalTenantName) | ||
| ) { | ||
| delete availableTenantsClone[globalTenantName]; |
There was a problem hiding this comment.
This will work, but it's a bit confusing why global_tenant is in this data structure in the first place. I went over to the security plugin to find where its coming from and it looks like its being added here: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java#L1135-L1140
I'm finding that block hard to follow and not sure what the reason for it is.
This PR looks good to me overall. I will approve when the test descriptions are updated.
There was a problem hiding this comment.
I think this global_tenant is basically functioning as the 'Global' tenant. Just for the case that user doesn't have any custom tenants with multi-tenancy enabled but disable both 'Global' and 'Private' tenants.
Referencing to the information from @cliu123, I also left a comment in the original issue: #1248 (comment)
There was a problem hiding this comment.
@cwperks Yes, global_tenant comes from there.
@RyanL1997 You're right. global_tenant is global tenant.
Signed-off-by: Ryan Liang <[email protected]>
|
LGTM！ |
#1277) * Fix tenant label for custom tenant when both Global and Private tenants are disabled Signed-off-by: Ryan Liang <[email protected]> (cherry picked from commit 1cffd90)
#1277) (#1280) * Fix tenant label for custom tenant when both Global and Private tenants are disabled Signed-off-by: Ryan Liang <[email protected]> (cherry picked from commit 1cffd90) Co-authored-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang [email protected]
Description
Fix tenant label for custom tenant when both Global and Private tenants are disabled
Category
Bug fix
Issues Resolved
Check List
New functionality has been documentedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.