Move some client-specific routines from SSLServer to PostgresNode

test_connect_ok() and test_connect_fails() have always been part of the
SSL tests, and check if a connection to the backend should work or not,
and there are sanity checks done on specific error patterns dropped by
libpq if the connection fails.

This was fundamentally wrong on two aspects.  First, SSLServer.pm works
mostly on setting up and changing the SSL configuration of a
PostgresNode, and has really nothing to do with the client.  Second,
the situation became worse in light of b34ca595, where the SSL tests
would finish by using a psql command that may not come from the same
installation as the node set up.

This commit moves those client routines into PostgresNode, making easier
the refactoring of SSLServer to become more SSL-implementation aware.
This can also be reused by the ldap, kerberos and authentication test
suites for connection checks, and a follow-up patch should extend those
interfaces to match with backend log patterns.

Author: Michael Paquier
Reviewed-by: Andrew Dunstan, Daniel Gustafsson, Álvaro Herrera
Discussion: https://postgr.es/m/YGLKNBf9zyh6+WSt@paquier.xyz

src/test/perl/PostgresNode.pm

@@ -1511,6 +1511,11 @@ the B<timed_out> parameter is also given.
 If B<timeout> is set and this parameter is given, the scalar it references
 is set to true if the psql call times out.
 
+=item connstr => B<value>
+
+If set, use this as the connection string for the connection to the
+backend.
+
 =item replication => B<value>
 
 If set, add B<replication=value> to the conninfo string.
@@ -1550,14 +1555,20 @@ sub psql
 	my $replication       = $params{replication};
 	my $timeout           = undef;
 	my $timeout_exception = 'psql timed out';
-	my @psql_params       = (
-		'psql',
-		'-XAtq',
-		'-d',
-		$self->connstr($dbname)
-		  . (defined $replication ? " replication=$replication" : ""),
-		'-f',
-		'-');
+
+	# Build the connection string.
+	my $psql_connstr;
+	if (defined $params{connstr})
+	{
+		$psql_connstr = $params{connstr};
+	}
+	else
+	{
+		$psql_connstr = $self->connstr($dbname);
+	}
+	$psql_connstr .= defined $replication ? " replication=$replication" : "";
+
+	my @psql_params = ('psql', '-XAtq', '-d', $psql_connstr, '-f', '-');
 
 	# If the caller wants an array and hasn't passed stdout/stderr
 	# references, allocate temporary ones to capture them so we
@@ -1849,6 +1860,51 @@ sub interactive_psql
 
 =pod
 
+=item $node->connect_ok($connstr, $test_name)
+
+Attempt a connection with a custom connection string.  This is expected
+to succeed.
+
+=cut
+
+sub connect_ok
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	my ($self, $connstr, $test_name) = @_;
+	my ($ret,  $stdout,  $stderr)    = $self->psql(
+		'postgres',
+		"SELECT \$\$connected with $connstr\$\$",
+		connstr       => "$connstr",
+		on_error_stop => 0);
+
+	ok($ret == 0, $test_name);
+}
+
+=pod
+
+=item $node->connect_fails($connstr, $expected_stderr, $test_name)
+
+Attempt a connection with a custom connection string.  This is expected
+to fail with a message that matches the regular expression
+$expected_stderr.
+
+=cut
+
+sub connect_fails
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+	my ($self, $connstr, $expected_stderr, $test_name) = @_;
+	my ($ret, $stdout, $stderr) = $self->psql(
+		'postgres',
+		"SELECT \$\$connected with $connstr\$\$",
+		connstr => "$connstr");
+
+	ok($ret != 0, $test_name);
+	like($stderr, $expected_stderr, "$test_name: matches");
+}
+
+=pod
+
 =item $node->poll_query_until($dbname, $query [, $expected ])
 
 Run B<$query> repeatedly, until it returns the B<$expected> result

src/test/ssl/t/002_scram.pl

@@ -53,39 +53,34 @@ $common_connstr =
   "dbname=trustdb sslmode=require sslcert=invalid sslrootcert=invalid hostaddr=$SERVERHOSTADDR";
 
 # Default settings
-test_connect_ok($common_connstr, "user=ssltestuser",
+$node->connect_ok(
+	"$common_connstr user=ssltestuser",
 	"Basic SCRAM authentication with SSL");
 
 # Test channel_binding
-test_connect_fails(
-	$common_connstr,
-	"user=ssltestuser channel_binding=invalid_value",
+$node->connect_fails(
+	"$common_connstr user=ssltestuser channel_binding=invalid_value",
 	qr/invalid channel_binding value: "invalid_value"/,
 	"SCRAM with SSL and channel_binding=invalid_value");
-test_connect_ok(
-	$common_connstr,
-	"user=ssltestuser channel_binding=disable",
+$node->connect_ok("$common_connstr user=ssltestuser channel_binding=disable",
 	"SCRAM with SSL and channel_binding=disable");
 if ($supports_tls_server_end_point)
 {
-	test_connect_ok(
-		$common_connstr,
-		"user=ssltestuser channel_binding=require",
+	$node->connect_ok(
+		"$common_connstr user=ssltestuser channel_binding=require",
 		"SCRAM with SSL and channel_binding=require");
 }
 else
 {
-	test_connect_fails(
-		$common_connstr,
-		"user=ssltestuser channel_binding=require",
+	$node->connect_fails(
+		"$common_connstr user=ssltestuser channel_binding=require",
 		qr/channel binding is required, but server did not offer an authentication method that supports channel binding/,
 		"SCRAM with SSL and channel_binding=require");
 }
 
 # Now test when the user has an MD5-encrypted password; should fail
-test_connect_fails(
-	$common_connstr,
-	"user=md5testuser channel_binding=require",
+$node->connect_fails(
+	"$common_connstr user=md5testuser channel_binding=require",
 	qr/channel binding required but not supported by server's authentication request/,
 	"MD5 with SSL and channel_binding=require");
 
@@ -96,9 +91,8 @@ test_connect_fails(
 my $client_tmp_key = "ssl/client_scram_tmp.key";
 copy("ssl/client.key", $client_tmp_key);
 chmod 0600, $client_tmp_key;
-test_connect_fails(
-	"sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR",
-	"dbname=certdb user=ssltestuser channel_binding=require",
+$node->connect_fails(
+	"sslcert=ssl/client.crt sslkey=$client_tmp_key sslrootcert=invalid hostaddr=$SERVERHOSTADDR dbname=certdb user=ssltestuser channel_binding=require",
 	qr/channel binding required, but server authenticated client without channel binding/,
 	"Cert authentication and channel_binding=require");
 

src/test/ssl/t/SSLServer.pm

@@ -37,46 +37,8 @@ use Exporter 'import';
 our @EXPORT = qw(
   configure_test_server_for_ssl
   switch_server_cert
-  test_connect_fails
-  test_connect_ok
 );
 
-# Define a couple of helper functions to test connecting to the server.
-
-# The first argument is a base connection string to use for connection.
-# The second argument is a complementary connection string.
-sub test_connect_ok
-{
-	local $Test::Builder::Level = $Test::Builder::Level + 1;
-
-	my ($common_connstr, $connstr, $test_name) = @_;
-
-	my $cmd = [
-		'psql', '-X', '-A', '-t', '-c',
-		"SELECT \$\$connected with $connstr\$\$",
-		'-d', "$common_connstr $connstr"
-	];
-
-	command_ok($cmd, $test_name);
-	return;
-}
-
-sub test_connect_fails
-{
-	local $Test::Builder::Level = $Test::Builder::Level + 1;
-
-	my ($common_connstr, $connstr, $expected_stderr, $test_name) = @_;
-
-	my $cmd = [
-		'psql', '-X', '-A', '-t', '-c',
-		"SELECT \$\$connected with $connstr\$\$",
-		'-d', "$common_connstr $connstr"
-	];
-
-	command_fails_like($cmd, $expected_stderr, $test_name);
-	return;
-}
-
 # Copy a set of files, taking into account wildcards
 sub copy_files
 {