Thread: ldap/t/001_auth.pl fails with openldap 2.5

Hi,

The freebsd image I use for CI runs just failed because the package name for
openldap changed (it's now either openldap{24,25}-{client,server}, instead of
openldap-..}. I naively resolved that conflict by choosing the openldap25-*
packages. Which unfortunately turns out to break 001_auth.pl :(

https://api.cirrus-ci.com/v1/artifact/task/5061394509856768/tap/src/test/ldap/tmp_check/log/regress_log_001_auth

# Running: ldapsearch -h localhost -p 51649 -s base -b dc=example,dc=net -D cn=Manager,dc=example,dc=net -y
/tmp/cirrus-ci-build/src/test/ldap/tmp_check/ldappassword-n 'objectclass=*'
 
ldapsearch: unrecognized option -h
usage: ldapsearch [options] [filter [attributes...]]

Seems we need to replace -h & -p with a -H ldap://server:port/ style URI? I
think that's fine to do unconditionally, the -H schema is pretty old I think
(I seem to recall using it in the mid 2000s, when I learned to not like ldap
by experience).

The only reason I'm hesitating a bit is that f0e60ee4bc0, the commit adding
the ldap test suite, used an ldap:// uri for the server, but then 27cd521e6e7
(adding the ldapsearch) didn't use that for the ldapsearch? Thomas?

So, does anybody see a reason not to go for the trivial

diff --git i/src/test/ldap/t/001_auth.pl w/src/test/ldap/t/001_auth.pl
index f670bc5e0d5..a025a641b02 100644
--- i/src/test/ldap/t/001_auth.pl
+++ w/src/test/ldap/t/001_auth.pl
@@ -130,8 +130,8 @@ while (1)
     last
       if (
         system_log(
-            "ldapsearch", "-h", $ldap_server, "-p",
-            $ldap_port,   "-s", "base",       "-b",
+            "ldapsearch", "-H", "$ldap_url",  "-s",
+            "base",       "-b",
             $ldap_basedn, "-D", $ldap_rootdn, "-y",
             $ldap_pwfile, "-n", "'objectclass=*'") == 0);
     die "cannot connect to slapd" if ++$retries >= 300;


Although I'm mildly tempted to rewrap the parameters, it's kinda odd how the
trailing parameter on one line, has its value on the next line.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> So, does anybody see a reason not to go for the trivial
> [ patch ]

I'd be happy to rely on the buildfarm's opinion here.

> Although I'm mildly tempted to rewrap the parameters, it's kinda odd how the
> trailing parameter on one line, has its value on the next line.

I'm betting that perltidy did that.  If you want to fix it so it
stays fixed, maybe reordering the parameters could help.

            regards, tom lane

On Sun, Oct 10, 2021 at 12:39 PM Andres Freund <andres@anarazel.de> wrote:
> Seems we need to replace -h & -p with a -H ldap://server:port/ style URI? I
> think that's fine to do unconditionally, the -H schema is pretty old I think
> (I seem to recall using it in the mid 2000s, when I learned to not like ldap
> by experience).

+1

> The only reason I'm hesitating a bit is that f0e60ee4bc0, the commit adding
> the ldap test suite, used an ldap:// uri for the server, but then 27cd521e6e7
> (adding the ldapsearch) didn't use that for the ldapsearch? Thomas?

My mistake, I probably copied it from somewhere without reading the
deprecation notice in the man page.  I found the discussion[1], which
says "These options have been deprecated since 2000, so you've had
literally 20 years to migrate away from them".

[1] https://bugs.openldap.org/show_bug.cgi?id=8618

Hi,

On 2021-10-10 00:45:41 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Although I'm mildly tempted to rewrap the parameters, it's kinda odd how the
> > trailing parameter on one line, has its value on the next line.
> 
> I'm betting that perltidy did that.  If you want to fix it so it
> stays fixed, maybe reordering the parameters could help.

You were right on that front. Since perltidy insisted on reflowing due to the
reduction in number of parameters anyway, I did end up switching things around
so that the parameters look a bit more reasonable.

> > So, does anybody see a reason not to go for the trivial
> > [ patch ]
> 
> I'd be happy to rely on the buildfarm's opinion here.

Let's see what it says...

Greetings,

Andres Freund