Skip to content

Conversation

@drewmullen
Copy link
Contributor

@drewmullen drewmullen commented Dec 19, 2025

Closes: #745

Added support for ldap secret engine. both dynamic and static roles are supported. After running the tests locally against a working LDAP i have disabled them because its causing CI to fail. LMK how you want to proceed regarding testing

For testing:

  • included unit tests
  • integration tests. I enabled them then manually setup connectivity to a local openldap, then i ran the integration tests and they worked great. Below are the dynamic roles created from the unit tests
ldapsearch -x -H ldap://localhost:389 -b "ou=users,dc=example,dc=com" -D "cn=admin,dc=example,dc=com" -w admin "(cn=dynamic-role_*)" cn
...
# dynamic-role_luRsOyQMH3, users, example.com
dn: cn=dynamic-role_luRsOyQMH3,ou=users,dc=example,dc=com
cn: dynamic-role_luRsOyQMH3

# dynamic-role_YtrbBxSbsd, users, example.com
dn: cn=dynamic-role_YtrbBxSbsd,ou=users,dc=example,dc=com
cn: dynamic-role_YtrbBxSbsd
$ ./mvnw test -pl spring-cloud-vault-config-ldap -Dtest=LdapSecretIntegrationTests
...
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.springframework.cloud.vault.config.ldap.LdapSecretIntegrationTests
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.355 s -- in org.springframework.cloud.vault.config.ldap.LdapSecretIntegrationTests
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.033 s
[INFO] Finished at: 2025-12-19T16:28:21-05:00
[INFO] ------------------------------------------------------------------------

@drewmullen
Copy link
Contributor Author

drewmullen commented Jan 5, 2026

Morning! Happy Monday and new year. Hoping once you're back from break we can start a discussion on this PR @ryanjbaxter 🙏 . This was a very simple implementation. TBH a lot of copy code. If you want to take these kinds of features another direction please let me know

@ryanjbaxter
Copy link
Contributor

@mp911de would be more knowledgable here

@mp911de
Copy link
Member

mp911de commented Jan 5, 2026

@drewmullen looks like a decent proposal. Would you mind adding integration tests based on unboundid? IIRC, unboundid has an in-memory config option allowing to accept connections via the LDAP port.

@drewmullen drewmullen force-pushed the ldap-secret-engine branch 2 times, most recently from fdcd4a7 to 8bc9a02 Compare January 5, 2026 19:12
@drewmullen
Copy link
Contributor Author

@mp911de Thank you for pointing out UnboundId, that was pretty straight-forward to implement. Please take a look and see what you think!

While I have your attention, I have a couple other small PRs in the project as well, mostly house cleaning, if you have a min can you check those out as well?

@mp911de mp911de self-assigned this Jan 6, 2026
@mp911de mp911de added this to the 5.1.0-M1 milestone Jan 6, 2026
@mp911de
Copy link
Member

mp911de commented Jan 6, 2026

Thanks a lot. I'll take the change from here once 5.1 development starts. main currently hosts the 5.0 development line and we don't want to introduce new API/new modules in a bugfix release.

@drewmullen
Copy link
Contributor Author

@mp911de tyvm for the review and for setting expectation.

Do you have a rough ETA? I'm asking because I also realized that we may want to support Vault LDAP Libraries. I havent seen that feature used too much but for completness sake it should probably be included. Depending on your timeline I can try to include in this PR or as a follow on

@mp911de
Copy link
Member

mp911de commented Jan 6, 2026 via email

@drewmullen
Copy link
Contributor Author

I actually have to explore it myself 😅 lol

once i have a better understanding ill report back here as to whether or not it belongs in this library. appreciate the heads up

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.

Add support for Hashicorp LDAP secrets engine

3 participants