Add ldap module - #9987
Conversation
Provide LLdapContainer supporting `lldap/lldap` image. Fixes #9960
| * </ul> | ||
| */ | ||
| @Slf4j | ||
| public class LLdapContainer extends GenericContainer<LLdapContainer> { |
There was a problem hiding this comment.
what do you think of adding an abstract LdapContainer class as adapter (similar to org.testcontainers.containers.JdbcDatabaseContainer).
By doing so, we could (later) add support for other containers like 'bitnami/openldap' or 'docker-openldap' at a later time
There was a problem hiding this comment.
I think it is different. I don't see much shared between ldap as you can see in the implementation. It only took exposed ports and specific wait strategy.
We still can add other implementations later with the proper setup.
There was a problem hiding this comment.
Hello,
I believe introducing an abstract class for the LDAP container could be beneficial once a second LDAP container is added to Testcontainers. For Spring Boot, this would allow the creation of a single ContainerConnectionDetailsFactory, similar to the JdbcContainerConnectionDetailsFactory.
@eddumelendez: Do you think it would be valuable to add another LDAP container for the bitnami/openldap image in the near future? If so, please let me know if you would welcome an OSS contribution on this.
There was a problem hiding this comment.
I believe introducing an abstract class for the LDAP container could be beneficial once a second LDAP container is added to Testcontainers. For Spring Boot, this would allow the creation of a single ContainerConnectionDetailsFactory, similar to the JdbcContainerConnectionDetailsFactory.
Yes, I understand that. I think this in general could be something to revisit in a major version.
Do you think it would be valuable to add another LDAP container for the bitnami/openldap image in the near future?
bitnami has really great images with easy configuration for some scenarios. I wish there is a bitnami module with support for their images. But, the suggestion was declined, see bitnami/containers#74973. And for us, it is getting a bit complex to handle many modules.
There was a problem hiding this comment.
Hi @eddumelendez ,
Yes, I understand that. I think this in general could be something to revisit in a major version.
I understand your point of delaying this abstraction.
Still, I think that such introduction "now" would have made the in the integration in "spring-boot" testcontainers easier and "future" proven.
By "future" proven, I mean there are a few ldap Information any ldap container should share:
- "admin-user/password" (similar to a database user) so that the app can interact with the ldap server
- base-dn, the majority - if not all - ldap server will provide base directory setup and not a blank one
- another feature would be "ldif" importer
Provide LLdapContainer supporting
lldap/lldapimage.Fixes #9960