Add support to set base in EmbeddedLdapAutoConfiguration

[eddumelendez]

Currently, full path should be present when queries are performed.
This commit introduces support to set base in EmbeddedLdapAutoConfiguration.

By default, first entry in spring.ldap.embedded.base-dn will be
resolved.

See gh-11693

[snicoll]

Thanks for the PR @eddumelendez - I am a bit nervous about that change considering this comment and the lack of documentation.

I need to understand whether Spring Data Integration tests are legit or not. Regardless of that, I have a feeling (though I don't know Spring LDAP at all) that there should be a way to back-off from this (not register any base as this is the case now.

Thoughts?

[eddumelendez]

AFAICS spring-ldap just support one base. So, the documentation should be pretty similar to handling multiple datasources (I can add some documentation if we agreed to proceed with this change). In this case, building LdapContextSource for every dn and its LdapTemplate. But, thinking about that, it would look cumbersome.

Just another idea, would be create a MultiLdapTemplate which can be query all the ldapTemplate with the right base set in LdapContextSource. I think, this can be implemented in spring-ldap project and reused by spring-boot.

On the other hand, I also considering this such a breaking change not only for current integration test also for current projects using embedded ldap support.

@snicoll WDYT?

[snicoll]

To be completely honest? Not much I am afraid and I didn't get the analogy with our DataSource support.

It's not so much about the "one base" bit. It is about "the first on the list" and there is no way to disable that behaviour. If we want to move forward, we have to take care of the Spring Data integration tests situation: either we believe these are wrong (and then we should do something about that) or they are legit and then we shouldn't break them. Applying this PR will last time I checked.

[eddumelendez]

It is about "the first on the list" and there is no way to disable that behaviour.

Agree with this and I understand your worries.

Another option can be adding a customizer.

[snicoll]

Or a boolean flag that sets the base, possibly true by default so that the behaviour we want is enabled. Or with no default value so that embedded and non embedded mode have different defaults.

That said, I have no idea, I need to better understand the feature and the impact.

@eddumelendez thoughts?

[snicoll]

@eddumelendez gentle ping. As you know Spring LDAP way better than I do, perhaps you have some feedback on my (potentially dumb) proposal.

[eddumelendez]

@snicoll sorry I miss the first notification and thanks for the gentle reminder about this topic.

Set base in LdapContextSource will work fine when there is just one base to work with if we are using just LdapTemplate but will raise an issue if the project is using Object Directory Mapping with the base attribute set in @Entry, that's the reason why integration tests were reported broken here. e.g spring.ldap.embedded.base-dn:dc=spring,dc=org

If we set more than one

spring.ldap.embedded.base-dn[0]:dc=spring,dc=org
spring.ldap.embedded.base-dn[1]:dc=pivotal,dc=io

we will not be able to resolve a default base to set in LdapContextSource because spring-ldap supports just one base per LdapContextSource. But, in 270dc2c and eab0b84 multi baseDn was introduced for EmbeddedLdapAutoConfiguration. Thinking more about it, even a flag enabling set the base would be weird. Otherwise, we can evaluate if there is just one baseDn and set the value, if there are more we will not set any base, we should also document this behaviour and take into account what I said before

will raise issue if the project is using Object Directory Mapping with the base attribute set in @Entry

IMHO, the current state is good and we shouldn't modify it. This PR doesn't make sense to me now.

[snicoll]

Thanks for the feedback Eddú