Repository navigation
add test ldap test command and button - #13954
DaanHoogland wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13954 +/- ##
============================================
+ Coverage 20.76% 20.77% +0.01%
- Complexity 21903 21920 +17
============================================
Files 6496 6497 +1
Lines 591612 591651 +39
Branches 72585 72586 +1
============================================
+ Hits 122851 122931 +80
+ Misses 455424 455374 -50
- Partials 13337 13346 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 25.88% |
| Branch coverage | 20.20% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
c8a8518 to
e031775
Compare
|
DaanHoogland
moved this from Backlog
to conflict/waiting for author
in CloudStack Testing
There was a problem hiding this comment.
🟡 Changes recommended
The new/modified LDAP bind exception handling can throw an NPE on null exception messages, and the UI action is duplicated across list/data views in a way that risks divergence.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a “test LDAP connection” capability end-to-end: a new backend API command that performs an LDAP bind without persisting configuration, plus corresponding UI actions and label text so admins can test connectivity from the LDAP configuration screen.
Changes:
- UI: adds a new “Test LDAP Connection” action/button for LDAP configuration and registers the required icon.
- Backend: introduces
testLdapConfigurationAPI command and refactors LDAP bind logic into a shared helper used by both add-config and test-connection paths. - Tests: adds unit tests to ensure test-connection binds (with default port behavior) and never persists configuration.
File summaries
| File | Description |
|---|---|
| ui/src/core/lazy_lib/icons_use.js | Registers ExperimentOutlined so the new UI action can render its icon. |
| ui/src/config/section/config.js | Adds UI actions for invoking testLdapConfiguration from LDAP settings. |
| ui/public/locales/en.json | Adds the English label for the new LDAP test action. |
| plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/LdapManagerImplTest.java | Adds unit tests validating bind + non-persistence behavior for the new test flow. |
| plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java | Refactors bind logic and implements testConnection for the new API command. |
| plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManager.java | Extends the LDAP manager interface with the new testConnection entry point. |
| plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapTestConfigurationCmd.java | Introduces the testLdapConfiguration API command returning a SuccessResponse. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@blueorangutan package |
|
@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19158 |
kiranchavala
left a comment
kiranchavala
left a comment
There was a problem hiding this comment.
There is no message reported back on the UI if the ldap test passed or failed
Screenrecording
Screen.Recording.2026-09-09.at.12.46.49.PM.mov
Success
[root@ref-trl-12337-k-Mol8-kiran-chavala-mgmt1 ~]# cat /var/log/cloudstack/management/management-server.log |grep -i "logid:afb7bf46"
2026-09-09 06:51:11,849 DEBUG [c.c.a.ApiServlet] (qtp1151704483-18:[ctx-7ade5468]) (logid:afb7bf46) ===START=== 10.0.3.251 -- POST
2026-09-09 06:51:11,849 DEBUG [c.c.a.ApiServlet] (qtp1151704483-18:[ctx-7ade5468]) (logid:afb7bf46) Two factor authentication is already verified for the user 2, so skipping
2026-09-09 06:51:11,855 DEBUG [c.c.a.ApiServer] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) CIDRs from which account 'Account [{"accountName":"admin","id":2,"uuid":"bd890cfb-ac12-11f1-9106-1e00050003e3"}]' is allowed to perform API calls: 0.0.0.0/0,::/0
2026-09-09 06:51:11,857 INFO [o.a.c.a.DynamicRoleBasedAPIAccessChecker] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) Account [Account [{"accountName":"admin","id":2,"uuid":"bd890cfb-ac12-11f1-9106-1e00050003e3"}]] is Root Admin and there aren't any API key pair permissions involved, thus, all APIs are allowed.
2026-09-09 06:51:11,857 DEBUG [o.a.c.a.StaticRoleBasedAPIAccessChecker] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) RoleService is enabled. We will use it instead of StaticRoleBasedAPIAccessChecker.
2026-09-09 06:51:11,857 DEBUG [o.a.c.r.ApiRateLimitServiceImpl] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) API rate limiting is disabled. We will not use ApiRateLimitService.
2026-09-09 06:51:11,863 DEBUG [o.a.c.l.LdapContextFactory] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) initializing ldap with provider url: ldap://10.0.33.32:389
2026-09-09 06:51:11,864 DEBUG [c.c.a.ApiServlet] (qtp1151704483-18:[ctx-7ade5468, ctx-f20b7d3a]) (logid:afb7bf46) ===END=== 10.0.3.251 -- POST
Failure
[root@ref-trl-12337-k-Mol8-kiran-chavala-mgmt1 ~]# cat /var/log/cloudstack/management/management-server.log |grep -i "logid:58b737d9"
2026-09-09 07:01:48,384 DEBUG [c.c.a.ApiServlet] (qtp1151704483-16:[ctx-01c990aa]) (logid:58b737d9) ===START=== 10.0.3.251 -- POST
2026-09-09 07:01:48,384 DEBUG [c.c.a.ApiServlet] (qtp1151704483-16:[ctx-01c990aa]) (logid:58b737d9) Two factor authentication is already verified for the user 2, so skipping
2026-09-09 07:01:48,389 DEBUG [c.c.a.ApiServer] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) CIDRs from which account 'Account [{"accountName":"admin","id":2,"uuid":"bd890cfb-ac12-11f1-9106-1e00050003e3"}]' is allowed to perform API calls: 0.0.0.0/0,::/0
2026-09-09 07:01:48,394 INFO [o.a.c.a.DynamicRoleBasedAPIAccessChecker] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) Account [Account [{"accountName":"admin","id":2,"uuid":"bd890cfb-ac12-11f1-9106-1e00050003e3"}]] is Root Admin and there aren't any API key pair permissions involved, thus, all APIs are allowed.
2026-09-09 07:01:48,394 DEBUG [o.a.c.a.StaticRoleBasedAPIAccessChecker] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) RoleService is enabled. We will use it instead of StaticRoleBasedAPIAccessChecker.
2026-09-09 07:01:48,394 DEBUG [o.a.c.r.ApiRateLimitServiceImpl] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) API rate limiting is disabled. We will not use ApiRateLimitService.
2026-09-09 07:01:48,398 DEBUG [o.a.c.l.LdapContextFactory] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) initializing ldap with provider url: ldap://10.0.33.32:389
2026-09-09 07:01:48,399 DEBUG [o.a.c.l.LdapManagerImpl] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) NamingException while doing an LDAP bind javax.naming.CommunicationException: 10.0.33.32:389 [Root exception is java.net.ConnectException: Connection refused]
2026-09-09 07:01:48,400 DEBUG [c.c.a.ApiServlet] (qtp1151704483-16:[ctx-01c990aa, ctx-5392760e]) (logid:58b737d9) ===END=== 10.0.3.251 -- POST
that seems kind of central to the issue. will have to work on that... |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
Hi @DaanHoogland can you please address the conflicts? |
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19893 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-17488)
|
Description
This PR...
Fixes: #6934
a new test ldap connection command and button.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?