pgsql: oauth: Add unit tests for multiplexer handling

Started by Jacob Champion5 months ago10 messages
#1Jacob Champion
jchampion@postgresql.org

oauth: Add unit tests for multiplexer handling

To better record the internal behaviors of oauth-curl.c, add a unit test
suite for the socket and timer handling code. This is all based on TAP
and driven by our existing Test::More infrastructure.

This commit is a replay of 1443b6c0e, which was reverted due to
buildfarm failures. Compared with that, this version protects the build
targets in the Makefile with a with_libcurl conditional, and it tweaks
the code style in 001_oauth.pl.

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: /messages/by-id/CAOYmi+nDZxJHaWj9_jRSyf8uMToCADAmOfJEggsKW-kY7aUwHA@mail.gmail.com
Discussion: /messages/by-id/CAOYmi+m=xY0P_uAzAP_884uF-GhQ3wrineGwc9AEnb6fYxVqVQ@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/4e1e417330d42cb19c7d439cd50eea20f25c7518

Modified Files
--------------
src/interfaces/libpq-oauth/Makefile | 36 +-
src/interfaces/libpq-oauth/meson.build | 35 ++
src/interfaces/libpq-oauth/t/001_oauth.pl | 24 ++
src/interfaces/libpq-oauth/test-oauth-curl.c | 527 +++++++++++++++++++++++++++
4 files changed, 618 insertions(+), 4 deletions(-)

#2Christoph Berg
myon@debian.org
In reply to: Jacob Champion (#1)
Re: pgsql: oauth: Add unit tests for multiplexer handling

Re: Jacob Champion

oauth: Add unit tests for multiplexer handling

This seems to require more linking on Debian bullseye:

20:19:29 gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -moutline-atomics -g -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC test-oauth-curl.o oauth-utils.o -L../../../src/port -L../../../src/common -L../../../src/common -lpgcommon_shlib -L../../../src/port -lpgport_shlib -L../../../src/interfaces/libpq -lpq -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -lcurl -lm -o oauth_tests
20:19:29 /usr/bin/ld: oauth-utils.o: undefined reference to symbol 'pthread_sigmask@@GLIBC_2.17'
20:19:29 /usr/bin/ld: /lib/aarch64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
20:19:29 collect2: error: ld returned 1 exit status

Full log:
https://jengus.postgresql.org/job/postgresql-19-binaries-snapshot/architecture=arm64,distribution=bullseye/156/console

Distro releases newer than bullseye are fine.

Christoph

#3Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Christoph Berg (#2)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On Tue, Aug 26, 2025 at 12:48 PM Christoph Berg <myon@debian.org> wrote:

This seems to require more linking on Debian bullseye:

Bleh. Thanks for the report; looks like none of the bullseye animals
in the farm are building with Curl yet.

(cc Stefan: guaibasaurus seems like it'd be a good candidate; would
you consider using --with-libcurl there?)

20:19:29 gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -moutline-atomics -g -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC test-oauth-curl.o oauth-utils.o -L../../../src/port -L../../../src/common -L../../../src/common -lpgcommon_shlib -L../../../src/port -lpgport_shlib -L../../../src/interfaces/libpq -lpq -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -lcurl -lm -o oauth_tests
20:19:29 /usr/bin/ld: oauth-utils.o: undefined reference to symbol 'pthread_sigmask@@GLIBC_2.17'
20:19:29 /usr/bin/ld: /lib/aarch64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
20:19:29 collect2: error: ld returned 1 exit status

Okay. So I need to pull in PTHREAD_CFLAGS/LIBS as well...

--Jacob

#4Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Jacob Champion (#3)
1 attachment(s)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On Tue, Aug 26, 2025 at 1:10 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:

Okay. So I need to pull in PTHREAD_CFLAGS/LIBS as well...

Christoph, are you able to verify the attached patch fixes the build?

--Jacob

Attachments:

fix-bullseye.diffapplication/octet-stream; name=fix-bullseye.diffDownload
diff --git a/src/interfaces/libpq-oauth/Makefile b/src/interfaces/libpq-oauth/Makefile
index c8c38947ace..51145f085a8 100644
--- a/src/interfaces/libpq-oauth/Makefile
+++ b/src/interfaces/libpq-oauth/Makefile
@@ -25,6 +25,7 @@ override shlib := lib$(NAME)$(DLSUFFIX)
 override stlib := libpq-oauth.a
 
 override CPPFLAGS := -I$(libpq_srcdir) -I$(top_builddir)/src/port $(CPPFLAGS) $(LIBCURL_CPPFLAGS)
+override CFLAGS += $(PTHREAD_CFLAGS)
 
 OBJS = \
 	$(WIN32RES)
@@ -47,7 +48,7 @@ $(stlib): override OBJS += $(OBJS_STATIC)
 $(stlib): $(OBJS_STATIC)
 
 SHLIB_LINK_INTERNAL = $(libpq_pgport_shlib)
-SHLIB_LINK = $(LIBCURL_LDFLAGS) $(LIBCURL_LDLIBS) $(filter -lintl -lm, $(LIBS))
+SHLIB_LINK = $(LIBCURL_LDFLAGS) $(LIBCURL_LDLIBS) $(filter -lintl -lm $(PTHREAD_LIBS), $(LIBS))
 SHLIB_PREREQS = submake-libpq
 SHLIB_EXPORTS = exports.txt
 
#5Christoph Berg
myon@debian.org
In reply to: Jacob Champion (#4)
Re: pgsql: oauth: Add unit tests for multiplexer handling

Re: Jacob Champion

On Tue, Aug 26, 2025 at 1:10 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:

Okay. So I need to pull in PTHREAD_CFLAGS/LIBS as well...

Christoph, are you able to verify the attached patch fixes the build?

The patch fixes the autoconf build here.

Thanks,
Christoph

#6Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Christoph Berg (#5)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On Tue, Aug 26, 2025 at 1:45 PM Christoph Berg <myon@debian.org> wrote:

The patch fixes the autoconf build here.

Thanks! The meson side needs an additional thread_dep as well; once
that passes CI I will push.

--Jacob

#7Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Jacob Champion (#6)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On Tue, Aug 26, 2025 at 2:18 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:

Thanks! The meson side needs an additional thread_dep as well; once
that passes CI I will push.

Committed and backpatched. Thanks again!

--Jacob

#8Christoph Berg
myon@debian.org
In reply to: Jacob Champion (#7)
Re: pgsql: oauth: Add unit tests for multiplexer handling

Re: Jacob Champion

Committed and backpatched. Thanks again!

The apt.pg.o build is already happy again, thanks!

https://jengus.postgresql.org/job/postgresql-19-binaries-snapshot/

Christoph

#9Stefan Kaltenbrunner
stefan@kaltenbrunner.cc
In reply to: Jacob Champion (#3)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On 26.08.25 22:10, Jacob Champion wrote:

On Tue, Aug 26, 2025 at 12:48 PM Christoph Berg <myon@debian.org> wrote:

This seems to require more linking on Debian bullseye:

Bleh. Thanks for the report; looks like none of the bullseye animals
in the farm are building with Curl yet.

(cc Stefan: guaibasaurus seems like it'd be a good candidate; would
you consider using --with-libcurl there?)

done - though gaibasauris is actually on bookworm...

Stefan

#10Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Stefan Kaltenbrunner (#9)
Re: pgsql: oauth: Add unit tests for multiplexer handling

On Fri, Aug 29, 2025 at 5:57 AM Stefan Kaltenbrunner
<stefan@kaltenbrunner.cc> wrote:

done -

Thank you!

though gaibasauris is actually on bookworm...

Oh, good to know. The system description currently says

Debian GNU/Linux 11 (bullseye) gcc 10.2.1 x86_64

--Jacob