Making src/test/ssl more robust

Started by Michael Paquieralmost 11 years ago4 messages
#1Michael Paquier
michael.paquier@gmail.com
1 attachment(s)

Hi all,

I noticed two things while looking at the SSL test suite:
1) When running the tests, some logs are generated in client-log, but
this log file has no entry in .gitignore... A patch is attached.
2) cp is used with a wildcard and system_or_bail in ServerSetup.pm:
system_or_bail "cp ssl/server-*.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/server-*.key '$tempdir'/pgdata";
system_or_bail "chmod 0600 '$tempdir'/pgdata/server-*.key";
system_or_bail "cp ssl/root+client_ca.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/root+client.crl '$tempdir'/pgdata";
This does not look very portable to me. Wouldn't it be better to use
glob to get a list of the files and then copy each matching entry?
Thoughts?
--
Michael

Attachments:

20150408_ssl_tap_gitignore.patchtext/x-patch; charset=US-ASCII; name=20150408_ssl_tap_gitignore.patchDownload
diff --git a/src/test/ssl/.gitignore b/src/test/ssl/.gitignore
new file mode 100644
index 0000000..3bc46b5
--- /dev/null
+++ b/src/test/ssl/.gitignore
@@ -0,0 +1,2 @@
+# Generated by tests
+/client-log
#2Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#1)
2 attachment(s)
Re: Making src/test/ssl more robust

On Wed, Apr 8, 2015 at 9:57 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

I noticed two things while looking at the SSL test suite:
1) When running the tests, some logs are generated in client-log, but
this log file has no entry in .gitignore... A patch is attached.
2) cp is used with a wildcard and system_or_bail in ServerSetup.pm:
system_or_bail "cp ssl/server-*.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/server-*.key '$tempdir'/pgdata";
system_or_bail "chmod 0600 '$tempdir'/pgdata/server-*.key";
system_or_bail "cp ssl/root+client_ca.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/root+client.crl '$tempdir'/pgdata";
This does not look very portable to me. Wouldn't it be better to use
glob to get a list of the files and then copy each matching entry?
Thoughts?

Here are patches on top of those words. Instead of cp, glob is used
with File::Copy and File::Basename to make the code more portable,
something useful for Windows for example where cp is not directly
available.
--
Michael

Attachments:

0001-Ignore-content-generated-by-TAP-test-suite-of-SSL.patchtext/x-patch; charset=US-ASCII; name=0001-Ignore-content-generated-by-TAP-test-suite-of-SSL.patchDownload
From 47b832a1fee639e49dd0065da710b064275423c4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Thu, 9 Apr 2015 15:47:42 +0000
Subject: [PATCH 1/2] Ignore content generated by TAP test suite of SSL

---
 src/test/ssl/.gitignore | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 src/test/ssl/.gitignore

diff --git a/src/test/ssl/.gitignore b/src/test/ssl/.gitignore
new file mode 100644
index 0000000..61352c1
--- /dev/null
+++ b/src/test/ssl/.gitignore
@@ -0,0 +1,3 @@
+# Generated by tests
+/client-log
+/tmp_check/
-- 
2.3.5

0002-Make-TAP-tests-of-SSL-more-portable-by-avoiding-cp.patchtext/x-patch; charset=US-ASCII; name=0002-Make-TAP-tests-of-SSL-more-portable-by-avoiding-cp.patchDownload
From 12ce72438f609fed0a22248ab0945466252dc936 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Thu, 9 Apr 2015 16:02:08 +0000
Subject: [PATCH 2/2] Make TAP tests of SSL more portable by avoiding cp

Instead, glob takes care of the wildcard, and is used with File::Copy
to copy the set of files necessary for the tests in PGDATA.
---
 src/test/ssl/ServerSetup.pm | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index 1579dc9..ee4daf0 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -19,6 +19,8 @@ package ServerSetup;
 use strict;
 use warnings;
 use TestLib;
+use File::Basename;
+use File::Copy;
 use Test::More;
 
 use Exporter 'import';
@@ -26,6 +28,20 @@ our @EXPORT = qw(
   configure_test_server_for_ssl switch_server_cert
 );
 
+# Copy a set of files, taking into account wildcards
+sub copy_files
+{
+	my $orig = shift;
+	my $dest = shift;
+
+	my @orig_files = glob $orig;
+	foreach my $orig_file (@orig_files)
+	{
+		my $base_file = basename($orig_file);
+		copy($orig_file, "$dest/$base_file") or die "Could not copy $orig_file to $dest";
+	}
+}
+
 sub configure_test_server_for_ssl
 {
   my $tempdir = $_[0];
@@ -48,13 +64,12 @@ sub configure_test_server_for_ssl
 
   close CONF;
 
-
   # Copy all server certificates and keys, and client root cert, to the data dir
-  system_or_bail "cp ssl/server-*.crt '$tempdir'/pgdata";
-  system_or_bail "cp ssl/server-*.key '$tempdir'/pgdata";
+  copy_files("ssl/server-*.crt", "$tempdir/pgdata");
+  copy_files("ssl/server-*.key", "$tempdir/pgdata");
   system_or_bail "chmod 0600 '$tempdir'/pgdata/server-*.key";
-  system_or_bail "cp ssl/root+client_ca.crt '$tempdir'/pgdata";
-  system_or_bail "cp ssl/root+client.crl '$tempdir'/pgdata";
+  copy_files("ssl/root+client_ca.crt", "$tempdir/pgdata");
+  copy_files("ssl/root+client.crl", "$tempdir/pgdata");
 
   # Only accept SSL connections from localhost. Our tests don't depend on this
   # but seems best to keep it as narrow as possible for security reasons.
-- 
2.3.5

#3Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#2)
Re: Making src/test/ssl more robust

On 04/09/2015 10:06 AM, Michael Paquier wrote:

On Wed, Apr 8, 2015 at 9:57 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

I noticed two things while looking at the SSL test suite:
1) When running the tests, some logs are generated in client-log, but
this log file has no entry in .gitignore... A patch is attached.
2) cp is used with a wildcard and system_or_bail in ServerSetup.pm:
system_or_bail "cp ssl/server-*.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/server-*.key '$tempdir'/pgdata";
system_or_bail "chmod 0600 '$tempdir'/pgdata/server-*.key";
system_or_bail "cp ssl/root+client_ca.crt '$tempdir'/pgdata";
system_or_bail "cp ssl/root+client.crl '$tempdir'/pgdata";
This does not look very portable to me. Wouldn't it be better to use
glob to get a list of the files and then copy each matching entry?
Thoughts?

Here are patches on top of those words. Instead of cp, glob is used
with File::Copy and File::Basename to make the code more portable,
something useful for Windows for example where cp is not directly
available.

Pushed, thanks!

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael.paquier@gmail.com
In reply to: Heikki Linnakangas (#3)
Re: Making src/test/ssl more robust

On Fri, Apr 10, 2015 at 4:08 AM, Heikki Linnakangas wrote:

Pushed, thanks!

Thanks.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers