Re: [oauth] Add TLS support to OAuth tests

From: "Jonathan Gonzalez V(dot)" <jonathan(dot)abdiel(at)gmail(dot)com>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [oauth] Add TLS support to OAuth tests
Date: 2026-02-28 05:09:43
Message-ID: 2bd4fd90e49175f83d764b09b733905617edb88d.camel@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello!

> Yep, we need to support back to Python 3.6.8 (which I've tested your
> patch with locally).

That's indeed good to know, there's a way to know which is the oldest
version of Python used? What do you think about adding this information
on top of the `oauth_server.py` file? Just as a reminder because I
suspected that the lower version will be 3.9 not 3.6

> Comments:
>
> >     Signed-off-by: Jonathan Gonzalez V. <jonathan(dot)abdiel(at)gmail(dot)com>
>
> Note that we have no signoff convention here, at least that I'm aware
> of. It's not a problem to include it, but this will be replaced by an
> `Author:` line in the final commit. Submissions here fall under the
> Archive Policy:
>
>     https://www.postgresql.org/about/policies/archives/

Yes, I'm fully aware of this, it's just my default git configuration to
add the sign-off, I'll check to remove that, thanks for the point!

>
> > +my $certdir = dirname(__FILE__) . "/../../../ssl/ssl";
> > +$ENV{PGOAUTHCAFILE} = "$certdir/root+server_ca.crt";
>
> I think we should inject this directory from the build system,
> instead
> of walking across the tree.

I wasn't aware of this, will keep in mind for hte future

>
> > -  my $issuer = "http://127.0.0.1:$port";
> > +  my $issuer = "https://localhost:$port";
>
> Unfortunately this will cause strange bugs [1]: we aren't listening
> on
> IPv6, but Curl will attempt to contact a DNS hostname on both IPv4
> and
> IPv6 simultaneously, leading to intermittent failures. So we'll need
> to get the certificate's SANs working...

I wasn't aware of this, good learning! I'll keep it in mind for the
future, and probably we can work on IPv6 support later.

> >     server-ip-in-dnsname \
> > +   server-ip-localhost \
> >     server-single-alt-name \
>
> nitpick: Though the naming scheme is not really all that well
> defined,
> I think this probably belongs with the other -alt-name certs rather
> than the -ip- certs.

I had my questions if add it to the alt-name too, and now I'm thinking
in other test ideas due to this, like using multiple alternative names
for the host and issuer, but for sure, for the future not here!

> --
>
> To avoid making you play fetch-a-rock, I've attached these comments
> in
> code form as v1.1-0002 (but you're under no obligation to take them
> if
> you'd prefer to do it a different way :D). I also ran oauth_server.py
> through Black.

I did the same! just added the comments I mentioned, besides that, all
good on my side.

Thank you!

---
Jonathan Gonzalez V. <jonathan(dot)abdiel(at)gmail(dot)com>
EnterpriseDB

Attachment Content-Type Size
v1.2-0003-squash-add-a-few-comments-to-the-scripts.patch text/x-patch 1.2 KB
v1.2-0002-squash-Add-TLS-support-for-the-OAuth-tests.patch text/x-patch 16.4 KB
v1.2-0001-Add-TLS-support-for-the-OAuth-tests.patch text/x-patch 10.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2026-02-28 07:08:05 Re: Eliminating SPI / SQL from some RI triggers - take 3
Previous Message Amit Kapila 2026-02-28 05:03:28 Re: Fix slotsync worker busy loop causing repeated log messages