From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: peter.eisentraut@2ndquadrant.com
Cc: LouPicciano@comcast.net
Cc: tgl@sss.pgh.pa.us
Cc: bruce@momjian.us
Cc: pgsql-hackers@postgresql.org
Subject: Re: pg_stat_ssl additions
Date: Tue, 29 Jan 2019 13:50:17 +0900
Message-ID: <20190129045017.GC3121@paquier.xyz> (raw)
In-Reply-To: <20190129.121829.117745808.horiguchi.kyotaro@lab.ntt.co.jp>
References: <3f8a7a73-cebe-016f-49e3-853733f8baf2@2ndquadrant.com>
<20190128.171400.111796002.horiguchi.kyotaro@lab.ntt.co.jp>
<24783370-5acd-e0f3-8eb7-7f42ff2a026d@2ndquadrant.com>
<20190129.121829.117745808.horiguchi.kyotaro@lab.ntt.co.jp>
On Tue, Jan 29, 2019 at 12:18:29PM +0900, Kyotaro HORIGUCHI wrote:
> At Mon, 28 Jan 2019 14:53:43 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in <24783370-5acd-e0f3-8eb7-7f42ff2a026d@2ndquadrant.com>
>> This is strange. The tests work for me, and also on the cfbot. The
>
> Agreed. It seemed so also to me.
The tests look sane to me for what it's worth.
> When initializing SSL context, it picks up the root certificate
> from my home directory, not in test installation and I had one
> there. It is not based on $HOME but pwent so it is unchangeable
> (and it is the right design for the purpose).
Oops. I agree that the tests ought to be as much isolated as
possible, without optionally fetching things depending on the user
environment.
> +# ssl-related properties may defautly set to the files in the users'
> +# environment. Explicitly provide them a value so that they don't
> +# point a valid file accidentially. Some other common properties are
> +# set here together.
> +# Attach this at the head of $common_connstr.
> +my $def_connstr = "user=ssltestuser dbname=trustdb sslcert=invalid sslkey=invalid sslrootcert=invalid sslcrl=invalid ";
> +
Maybe it is better to just put that in test_connect_ok and
test_connect_fails for only the SSL parameters? I don't see much
point in enforcing dbname and user.
> # The server should not accept non-SSL connections.
> test_connect_fails(
> @@ -185,7 +192,7 @@ test_connect_ok(
> # Check that connecting with verify-full fails, when the hostname doesn't
> # match the hostname in the server's certificate.
> $common_connstr =
> - "user=ssltestuser dbname=trustdb sslcert=invalid sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
> + $def_connstr."sslrootcert=ssl/root+server_ca.crt hostaddr=$SERVERHOSTADDR";
I think this is bad, inconsistent style. Adding the variable within
the quoted section is fine, as much as moving SERVERHOSTADDR out. But
mixing styles is not.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../20190129045017.GC3121@paquier.xyz/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, horiguchi.kyotaro@lab.ntt.co.jp, peter.eisentraut@2ndquadrant.com, LouPicciano@comcast.net, tgl@sss.pgh.pa.us, bruce@momjian.us
Subject: Re: pg_stat_ssl additions
In-Reply-To: <20190129045017.GC3121@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox