Convert sepgsql tests to TAP
The sepgsql tests have not been integrated into the Meson build system
yet. I propose to fix that here.
One problem there was that the tests use a very custom construction
where a top-level shell script internally calls make. I have converted
this to a TAP script that does the preliminary checks and then calls
pg_regress directly, without make. This seems to get the job done.
Also, once you have your SELinux environment set up as required, the
test now works fully automatically; you don't have to do any manual prep
work. The whole thing is guarded by PG_TEST_EXTRA=sepgsql now.
Some comments and questions:
- Do we want to keep the old way to run the test? I don't know all the
testing scenarios that people might be interested in, but of course it
would also be good to cut down on the duplication in the test files.
- Strangely, there was apparently so far no way to get to the build
directory from a TAP script. They only ever want to read files from the
source directory. So I had to add that.
- If you go through the pre-test checks in contrib/sepgsql/test_sepgsql,
I have converted most of these checks to the Perl script. Some of the
checks are obsolete, because they check whether the database has been
correctly initialized, which is now done by the TAP script anyway. One
check that I wasn't sure about is the
# 'psql' command must be executable from test domain
The old test was checking the installation tree, which I guess could be
set up in random ways. But do we need this kind of check if we are
using a temporary installation?
As mentioned in the patch, the documentation needs to be updated. This
depends on the outcome of the question above whether we want to keep the
old tests in some way.
Attachments:
v1-0001-Convert-sepgsql-tests-to-TAP.patchtext/plain; charset=UTF-8; name=v1-0001-Convert-sepgsql-tests-to-TAP.patchDownload
From 2f8e73932b1068caf696582487de9e100fcd46be Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 13 May 2024 07:55:55 +0200
Subject: [PATCH v1] Convert sepgsql tests to TAP
Add a TAP test for sepgsql. This automates the previously required
manual setup before the test. The actual tests are still run by
pg_regress, but not called from within the TAP Perl script.
TODO: remove the old test scripts?
---
contrib/sepgsql/.gitignore | 3 +
contrib/sepgsql/Makefile | 3 +
contrib/sepgsql/meson.build | 11 +-
contrib/sepgsql/t/001_sepgsql.pl | 248 +++++++++++++++++++++++++++++++
doc/src/sgml/regress.sgml | 11 ++
doc/src/sgml/sepgsql.sgml | 2 +
meson.build | 1 +
src/Makefile.global.in | 1 +
8 files changed, 279 insertions(+), 1 deletion(-)
create mode 100644 contrib/sepgsql/t/001_sepgsql.pl
diff --git a/contrib/sepgsql/.gitignore b/contrib/sepgsql/.gitignore
index 31613e011f5..7cadb9419e9 100644
--- a/contrib/sepgsql/.gitignore
+++ b/contrib/sepgsql/.gitignore
@@ -1,7 +1,10 @@
/sepgsql.sql
+# FIXME
/sepgsql-regtest.fc
/sepgsql-regtest.if
/sepgsql-regtest.pp
/tmp
# Generated subdirectories
+/log/
/results/
+/tmp_check/
diff --git a/contrib/sepgsql/Makefile b/contrib/sepgsql/Makefile
index afca75b693f..5cc9697736c 100644
--- a/contrib/sepgsql/Makefile
+++ b/contrib/sepgsql/Makefile
@@ -15,6 +15,9 @@ OBJS = \
DATA_built = sepgsql.sql
PGFILEDESC = "sepgsql - SELinux integration"
+TAP_TESTS = 1
+
+# FIXME
# Note: because we don't tell the Makefile there are any regression tests,
# we have to clean those result files explicitly
EXTRA_CLEAN = -r $(pg_regress_clean_files) tmp/ *.pp sepgsql-regtest.if sepgsql-regtest.fc
diff --git a/contrib/sepgsql/meson.build b/contrib/sepgsql/meson.build
index 9544efe0287..5817ba30a58 100644
--- a/contrib/sepgsql/meson.build
+++ b/contrib/sepgsql/meson.build
@@ -40,4 +40,13 @@ contrib_targets += custom_target('sepgsql.sql',
install_dir: contrib_data_args['install_dir'],
)
-# TODO: implement sepgsql tests
+tests += {
+ 'name': 'sepgsql',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_sepgsql.pl',
+ ],
+ },
+}
diff --git a/contrib/sepgsql/t/001_sepgsql.pl b/contrib/sepgsql/t/001_sepgsql.pl
new file mode 100644
index 00000000000..82bba5774ce
--- /dev/null
+++ b/contrib/sepgsql/t/001_sepgsql.pl
@@ -0,0 +1,248 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bsepgsql\b/)
+{
+ plan skip_all =>
+ 'Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA';
+}
+
+note "checking selinux environment";
+
+# matchpathcon must be present to assess whether the installation environment
+# is OK.
+note "checking for matchpathcon";
+if (system('matchpathcon -n . >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The matchpathcon command must be available.
+Please install it or update your PATH to include it
+(it is typically in '/usr/sbin', which might not be in your PATH).
+matchpathcon is typically included in the libselinux-utils package.
+EOS
+ die;
+}
+
+# runcon must be present to launch psql using the correct environment
+note "checking for runcon";
+if (system('runcon --help >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The runcon command must be available.
+runcon is typically included in the coreutils package.
+EOS
+ die;
+}
+
+# check sestatus too, since that lives in yet another package
+note "checking for sestatus";
+if (system('sestatus >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The sestatus command must be available.
+sestatus is typically included in the policycoreutils package.
+EOS
+ die;
+}
+
+# check that the user is running in the unconfined_t domain
+note "checking current user domain";
+my $DOMAIN = `id -Z 2>/dev/null | sed 's/:/ /g' | awk '{print \$3}'`;
+chomp $DOMAIN;
+note "current user domain is '$DOMAIN'";
+if ($DOMAIN ne 'unconfined_t')
+{
+ diag <<'EOS';
+
+The regression tests must be launched from the unconfined_t domain.
+
+The unconfined_t domain is typically the default domain for user
+shell processes. If the default has been changed on your system,
+you can revert the changes like this:
+
+ $ sudo semanage login -d `whoami`
+
+Or, you can add a setting to log in using the unconfined_t domain:
+
+ $ sudo semanage login -a -s unconfined_u -r s0-s0:c0.c255 `whoami`
+
+EOS
+ die;
+}
+
+# SELinux must be configured in enforcing mode
+note "checking selinux operating mode";
+my $CURRENT_MODE =
+ `LANG=C sestatus | grep '^Current mode:' | awk '{print \$3}'`;
+chomp $CURRENT_MODE;
+note "current operating mode is '$CURRENT_MODE'";
+if ($CURRENT_MODE eq 'enforcing')
+{
+ # OK
+}
+elsif ($CURRENT_MODE eq 'permissive' || $CURRENT_MODE eq 'disabled')
+{
+ diag <<'EOS';
+
+Before running the regression tests, SELinux must be enabled and
+must be running in enforcing mode.
+
+If SELinux is currently running in permissive mode, you can
+switch to enforcing mode using the 'setenforce' command.
+
+ $ sudo setenforce 1
+
+The system default setting is configured in /etc/selinux/config,
+or using a kernel boot parameter.
+EOS
+ die;
+}
+else
+{
+ diag <<EOS;
+
+Unable to determine the current selinux operating mode. Please
+verify that the sestatus command is installed and in your PATH.
+EOS
+ die;
+}
+
+# 'sepgsql-regtest' policy module must be loaded
+note "checking for sepgsql-regtest policy";
+my $SELINUX_MNT = `sestatus | grep '^SELinuxfs mount:' | awk '{print \$3}'`;
+chomp $SELINUX_MNT;
+if ($SELINUX_MNT eq "")
+{
+ diag <<EOS;
+
+Unable to find SELinuxfs mount point.
+
+The sestatus command should report the location where SELinuxfs
+is mounted, but did not do so.
+EOS
+ die;
+}
+if (!-e "${SELINUX_MNT}/booleans/sepgsql_regression_test_mode")
+{
+ diag <<'EOS';
+
+The 'sepgsql-regtest' policy module appears not to be installed.
+Without this policy installed, the regression tests will fail.
+You can install this module using the following commands:
+
+ $ make -f /usr/share/selinux/devel/Makefile
+ $ sudo semodule -u sepgsql-regtest.pp
+
+To confirm that the policy package is installed, use this command:
+
+ $ sudo semodule -l | grep sepgsql
+
+EOS
+ die;
+}
+
+# Verify that sepgsql_regression_test_mode is active.
+note "checking whether policy is enabled";
+foreach
+ my $policy ('sepgsql_regression_test_mode', 'sepgsql_enable_users_ddl')
+{
+ my $POLICY_STATUS = `getsebool $policy | awk '{print \$3}'`;
+ chomp $POLICY_STATUS;
+ note "$policy is '$POLICY_STATUS'";
+ if ($POLICY_STATUS ne "on")
+ {
+ diag <<EOS;
+
+The SELinux boolean '$policy' must be
+turned on in order to enable the rules necessary to run the
+regression tests.
+
+EOS
+
+ if ($POLICY_STATUS eq "")
+ {
+ diag <<EOS;
+We attempted to determine the state of this Boolean using
+'getsebool', but that command did not produce the expected
+output. Please verify that getsebool is available and in
+your PATH.
+EOS
+ }
+ else
+ {
+ diag <<EOS;
+You can turn on this variable using the following commands:
+
+ \$ sudo setsebool $policy on
+
+For security reasons, it is suggested that you turn off this
+variable when regression testing is complete and the associated
+rules are no longer needed.
+EOS
+ }
+ die;
+ }
+}
+
+
+#
+# checking complete - let's run the tests
+#
+
+note "running sepgsql regression tests";
+
+my $node;
+
+$node = PostgreSQL::Test::Cluster->new('test');
+$node->init;
+$node->append_conf('postgresql.conf', 'log_statement=none');
+
+{
+ local %ENV = $node->_get_env();
+
+ my $result = run_log(
+ [
+ 'postgres', '--single',
+ '-F', '-c',
+ 'exit_on_error=true', '-D',
+ $node->data_dir, 'template0'
+ ],
+ '<',
+ $ENV{builddir} . '/sepgsql.sql');
+ ok($result, 'sepgsql installation script');
+}
+
+$node->append_conf('postgresql.conf', 'shared_preload_libraries=sepgsql');
+$node->start;
+
+my @tests = qw(label dml ddl alter misc);
+
+# Check if the truncate permission exists in the loaded policy, and if so,
+# run the truncate test
+#
+# Testing the TRUNCATE regression test can be done by manually adding
+# the permission with CIL if necessary:
+# sudo semodule -cE base
+# sudo sed -i -E 's/(class db_table.*?) \)/\1 truncate\)/' base.cil
+# sudo semodule -i base.cil
+push @tests, 'truncate' if -f '/sys/fs/selinux/class/db_table/perms/truncate';
+
+$node->command_ok(
+ [
+ $ENV{PG_REGRESS}, '--bindir=', '--inputdir=.', '--launcher',
+ './launcher', @tests
+ ],
+ 'sepgsql tests');
+
+done_testing();
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d1042e02228..9aebd9f3bed 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -284,6 +284,17 @@ <title>Additional Test Suites</title>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>sepgsql</literal></term>
+ <listitem>
+ <para>
+ Runs the test suite under <filename>contrib/sepgsql</filename>. This
+ requires an SELinux environment that is set up in a specific way; see
+ <xref linkend="sepgsql-regression"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>ssl</literal></term>
<listitem>
diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index 1b848f1977c..a21e2d86279 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -151,6 +151,8 @@ <title>Installation</title>
<sect2 id="sepgsql-regression">
<title>Regression Tests</title>
+ <!-- FIXME: update this section -->
+
<para>
Due to the nature of <productname>SELinux</productname>, running the
regression tests for <filename>sepgsql</filename> requires several extra
diff --git a/meson.build b/meson.build
index 1c0579d5a6b..49e61b33960 100644
--- a/meson.build
+++ b/meson.build
@@ -3375,6 +3375,7 @@ foreach test_dir : tests
# Add temporary install, the build directory for non-installed binaries and
# also test/ for non-installed test binaries built separately.
env = test_env
+ env.set('builddir', test_dir['bd'])
env.prepend('PATH', temp_install_bindir, test_dir['bd'], test_dir['bd'] / 'test')
foreach name, value : t.get('env', {})
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index a00c909681e..9f87cae879b 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -477,6 +477,7 @@ echo "# +++ tap check in $(subdir) +++" && \
rm -rf '$(CURDIR)'/tmp_check && \
$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
+ builddir='$(CURDIR)' \
TESTLOGDIR='$(CURDIR)/tmp_check/log' \
TESTDATADIR='$(CURDIR)/tmp_check' \
$(with_temp_install) \
base-commit: 3ca43dbbb67fbfb96dec8de2e268b96790555148
--
2.44.0
I took a quick look at the patch and I like that we standardize things a
bit. But one thing I am not a fan of are all the use of sed and awk in
the Perl script. I would prefer if that logic happened all in Perl,
especially since we have some of it in Perl (e.g. chomp). Also I wonder
if we should not use IPC::Run to do the tests since we already depend on
it for the other TAP tests.
I have not yet set up an VM with selinux to try the patch out for real
but will do so later.
On 5/13/24 8:16 AM, Peter Eisentraut wrote:
- Do we want to keep the old way to run the test? I don't know all the
testing scenarios that people might be interested in, but of course it
would also be good to cut down on the duplication in the test files.
I cannot see why. Having two ways to run the tests seems only like a bad
thing to me.
- If you go through the pre-test checks in contrib/sepgsql/test_sepgsql,
I have converted most of these checks to the Perl script. Some of the
checks are obsolete, because they check whether the database has been
correctly initialized, which is now done by the TAP script anyway. One
check that I wasn't sure about is the# 'psql' command must be executable from test domain
The old test was checking the installation tree, which I guess could be
set up in random ways. But do we need this kind of check if we are
using a temporary installation?
Yeah, that does not seem necessary.
Andreas
On 7/24/24 4:31 PM, Andreas Karlsson wrote:
I have not yet set up an VM with selinux to try the patch out for real
but will do so later.
I almost got the tests running but it required way too many manual steps
to just get there and I gave up after just getting segfaults. I had to
edit sepgsql-regtest.te because sepgsql-regtest.pp would not build
otherwise on Debian bookworm, but after I had done that instead of
getting test failures as I expected I just got segfaults. Maybe those
are caused by an incorrect sepgsql-regtest.pp but this was not nice at
all to try to get running for someone like me who does not know selinux
well.
Peter, what did you do to get the tests running? And should we fix these
tests to make them more user friendly?
Andreas
On 24.07.24 18:29, Andreas Karlsson wrote:
On 7/24/24 4:31 PM, Andreas Karlsson wrote:
I have not yet set up an VM with selinux to try the patch out for real
but will do so later.I almost got the tests running but it required way too many manual steps
to just get there and I gave up after just getting segfaults. I had to
edit sepgsql-regtest.te because sepgsql-regtest.pp would not build
otherwise on Debian bookworm, but after I had done that instead of
getting test failures as I expected I just got segfaults. Maybe those
are caused by an incorrect sepgsql-regtest.pp but this was not nice at
all to try to get running for someone like me who does not know selinux
well.Peter, what did you do to get the tests running? And should we fix these
tests to make them more user friendly?
In my experience, the tests (both the old and the proposed new) only
work on Red Hat-like platforms. I had also tried on Debian but decided
that it won't work.
On 24.07.24 16:31, Andreas Karlsson wrote:
I took a quick look at the patch and I like that we standardize things a
bit. But one thing I am not a fan of are all the use of sed and awk in
the Perl script. I would prefer if that logic happened all in Perl,
especially since we have some of it in Perl (e.g. chomp). Also I wonder
if we should not use IPC::Run to do the tests since we already depend on
it for the other TAP tests.
In principle yes, but here I tried not rewriting the tests too much but
just port them to a newer environment. I think the adjustments you
describe could be done as a second step.
(I don't really have any expertise in sepgsql or selinux, I'm just doing
this to reduce the dependency on makefiles for testing. So I'm trying
to use as light a touch as possible.)
On 7/24/24 6:31 PM, Peter Eisentraut wrote:
On 24.07.24 18:29, Andreas Karlsson wrote:
Peter, what did you do to get the tests running? And should we fix
these tests to make them more user friendly?In my experience, the tests (both the old and the proposed new) only
work on Red Hat-like platforms. I had also tried on Debian but decided
that it won't work.
Thanks, will try to run them on Rocky Linux when I have calmed down a
bit. :)
Andreas
Peter Eisentraut <peter@eisentraut.org> writes:
In my experience, the tests (both the old and the proposed new) only
work on Red Hat-like platforms. I had also tried on Debian but decided
that it won't work.
Yeah, Red Hat is pretty much the only vendor that has pushed SELinux
far enough to be usable by non-wizards. I'm not surprised if there
are outright bugs in other distros' versions of it, as AFAIK
nobody else turns it on by default.
regards, tom lane
On 7/24/24 12:36, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
In my experience, the tests (both the old and the proposed new) only
work on Red Hat-like platforms. I had also tried on Debian but decided
that it won't work.Yeah, Red Hat is pretty much the only vendor that has pushed SELinux
far enough to be usable by non-wizards. I'm not surprised if there
are outright bugs in other distros' versions of it, as AFAIK
nobody else turns it on by default.
I tried some years ago to get it working on my Debian-derived Linux Mint
desktop and gave up. I think SELinux is a really good tool on RHEL
variants, but I don't think many people use it on anything else. As Tom
says, perhaps there are a few wizards out there though...
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 7/24/24 6:33 PM, Peter Eisentraut wrote:
On 24.07.24 16:31, Andreas Karlsson wrote:
I took a quick look at the patch and I like that we standardize things
a bit. But one thing I am not a fan of are all the use of sed and awk
in the Perl script. I would prefer if that logic happened all in Perl,
especially since we have some of it in Perl (e.g. chomp). Also I
wonder if we should not use IPC::Run to do the tests since we already
depend on it for the other TAP tests.In principle yes, but here I tried not rewriting the tests too much but
just port them to a newer environment. I think the adjustments you
describe could be done as a second step.
That reasoning makes a lot of sense and I am in agreement. Cleaning that
up is best for another patch.
And managed to get the tests running on Rocky Linux 9 with both
autotools and meson and everything work as it should.
So I have two comments:
1) As I said earlier I think we should remove the old code.
2) If we remove the old code I think the launcher script can be merged
into the TAP test instead of being a separate shell script. But I am
fine if you think that is also something for a separate commit.
I like this kind of clean up patch. Good work! :)
Andreas
Andreas Karlsson <andreas@proxel.se> writes:
1) As I said earlier I think we should remove the old code.
I agree that carrying two versions of the test doesn't seem great.
However, a large part of the purpose of test_sepgsql is to help
people debug their sepgsql setup, which is why it goes to great
lengths to print helpful error messages. I'm worried that making
it into a TAP test will degrade the usefulness of that, simply
because the TAP infrastructure is pretty damn unfriendly when it
comes to figuring out why a test failed. You have to know where
to even look for the test logfile, and then you have to ignore
a bunch of useless-to-you chatter. I'm not sure if there is much
we can do to improve that. (Although if we could, it would
yield benefits across the whole tree.)
OTOH, I suspect there are so few people using sepgsql that this
doesn't matter too much. Probably most of them will be advanced
hackers who won't blink at digging through a TAP log. We should
update the docs to explain that though.
regards, tom lane
On 7/24/24 10:35 PM, Tom Lane wrote:
Andreas Karlsson <andreas@proxel.se> writes:
1) As I said earlier I think we should remove the old code.
I agree that carrying two versions of the test doesn't seem great.
However, a large part of the purpose of test_sepgsql is to help
people debug their sepgsql setup, which is why it goes to great
lengths to print helpful error messages. I'm worried that making
it into a TAP test will degrade the usefulness of that, simply
because the TAP infrastructure is pretty damn unfriendly when it
comes to figuring out why a test failed. You have to know where
to even look for the test logfile, and then you have to ignore
a bunch of useless-to-you chatter. I'm not sure if there is much
we can do to improve that. (Although if we could, it would
yield benefits across the whole tree.)
For me personally the output from when running it with meson was good
enough while the output when running with autotools was usable but
annoying to work with. Meson's integration with TAP is pretty good. But
with that said I am a power user and developer used to both meson and
autotools. Unclear what skill we should expect from the target audience
of test_sepgsql.
Andreas
On 24.07.24 23:03, Andreas Karlsson wrote:
On 7/24/24 10:35 PM, Tom Lane wrote:
Andreas Karlsson <andreas@proxel.se> writes:
1) As I said earlier I think we should remove the old code.
I agree that carrying two versions of the test doesn't seem great.
However, a large part of the purpose of test_sepgsql is to help
people debug their sepgsql setup, which is why it goes to great
lengths to print helpful error messages. I'm worried that making
it into a TAP test will degrade the usefulness of that, simply
because the TAP infrastructure is pretty damn unfriendly when it
comes to figuring out why a test failed. You have to know where
to even look for the test logfile, and then you have to ignore
a bunch of useless-to-you chatter. I'm not sure if there is much
we can do to improve that. (Although if we could, it would
yield benefits across the whole tree.)For me personally the output from when running it with meson was good
enough while the output when running with autotools was usable but
annoying to work with. Meson's integration with TAP is pretty good. But
with that said I am a power user and developer used to both meson and
autotools. Unclear what skill we should expect from the target audience
of test_sepgsql.
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an extension
but a loose SQL script, of which it is now the only one. Maybe
something to address separately.)
I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations. I
did change it so that it calls pg_regress directly, without going via
make, so that the dependency on make is removed.
The documentation is also updated a little bit, but I kept it to a
minimum, because I'm not really sure how up to date the existing
documentation was. It lists several steps in the test procedure that I
didn't need to do. Someone who knows more about the whole picture would
need to look at that in more detail.
Attachments:
v2-0001-meson-Fix-sepgsql-installation.patchtext/plain; charset=UTF-8; name=v2-0001-meson-Fix-sepgsql-installation.patchDownload
From 2fe3e9020fe11ed5e86432d98f1c7283668654e4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 27 Aug 2024 09:48:47 +0200
Subject: [PATCH v2 1/2] meson: Fix sepgsql installation
The sepgsql.sql file should be installed under share/contrib/, not
share/extension/, since it is not an extension. This makes it match
what the make install does.
---
contrib/sepgsql/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/sepgsql/meson.build b/contrib/sepgsql/meson.build
index 9544efe0287..acff379b4b6 100644
--- a/contrib/sepgsql/meson.build
+++ b/contrib/sepgsql/meson.build
@@ -37,7 +37,7 @@ contrib_targets += custom_target('sepgsql.sql',
command: [sed, '-e', 's,MODULE_PATHNAME,$libdir/sepgsql,g', '@INPUT@'],
capture: true,
install: true,
- install_dir: contrib_data_args['install_dir'],
+ install_dir: dir_data / 'contrib',
)
# TODO: implement sepgsql tests
base-commit: 7229ebe011dff3f418251a4836f6d098923aa1e1
--
2.46.0
v2-0002-Convert-sepgsql-tests-to-TAP.patchtext/plain; charset=UTF-8; name=v2-0002-Convert-sepgsql-tests-to-TAP.patchDownload
From 9322b87aad3d207731dc71853f39cb9651ea8a18 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 27 Aug 2024 09:52:45 +0200
Subject: [PATCH v2 2/2] Convert sepgsql tests to TAP
Add a TAP test for sepgsql. This automates the previously required
manual setup before the test. The actual tests are still run by
pg_regress, but not called from within the TAP Perl script.
The previous manual test script (test_sepgsql) is left in place, since
its purpose is (also) to test whether a running instance was properly
initialized for sepgsql. But it has been changed to call pg_regress
directly and no longer require make.
Discussion: https://www.postgresql.org/message-id/flat/651a5baf-5c45-4a5a-a202-0c8453a4ebf8@eisentraut.org
---
contrib/sepgsql/.gitignore | 4 +-
contrib/sepgsql/Makefile | 2 +
contrib/sepgsql/meson.build | 11 +-
contrib/sepgsql/t/001_sepgsql.pl | 246 +++++++++++++++++++++++++++++++
contrib/sepgsql/test_sepgsql | 12 +-
doc/src/sgml/regress.sgml | 11 ++
doc/src/sgml/sepgsql.sgml | 17 ++-
meson.build | 2 +
src/Makefile.global.in | 2 +
9 files changed, 294 insertions(+), 13 deletions(-)
create mode 100644 contrib/sepgsql/t/001_sepgsql.pl
diff --git a/contrib/sepgsql/.gitignore b/contrib/sepgsql/.gitignore
index 31613e011f5..b1778d05bbd 100644
--- a/contrib/sepgsql/.gitignore
+++ b/contrib/sepgsql/.gitignore
@@ -3,5 +3,5 @@
/sepgsql-regtest.if
/sepgsql-regtest.pp
/tmp
-# Generated subdirectories
-/results/
+# Generated by test suite
+/tmp_check/
diff --git a/contrib/sepgsql/Makefile b/contrib/sepgsql/Makefile
index afca75b693f..90b4585a9e2 100644
--- a/contrib/sepgsql/Makefile
+++ b/contrib/sepgsql/Makefile
@@ -15,6 +15,8 @@ OBJS = \
DATA_built = sepgsql.sql
PGFILEDESC = "sepgsql - SELinux integration"
+TAP_TESTS = 1
+
# Note: because we don't tell the Makefile there are any regression tests,
# we have to clean those result files explicitly
EXTRA_CLEAN = -r $(pg_regress_clean_files) tmp/ *.pp sepgsql-regtest.if sepgsql-regtest.fc
diff --git a/contrib/sepgsql/meson.build b/contrib/sepgsql/meson.build
index acff379b4b6..56316e4882d 100644
--- a/contrib/sepgsql/meson.build
+++ b/contrib/sepgsql/meson.build
@@ -40,4 +40,13 @@ contrib_targets += custom_target('sepgsql.sql',
install_dir: dir_data / 'contrib',
)
-# TODO: implement sepgsql tests
+tests += {
+ 'name': 'sepgsql',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_sepgsql.pl',
+ ],
+ },
+}
diff --git a/contrib/sepgsql/t/001_sepgsql.pl b/contrib/sepgsql/t/001_sepgsql.pl
new file mode 100644
index 00000000000..cba51403518
--- /dev/null
+++ b/contrib/sepgsql/t/001_sepgsql.pl
@@ -0,0 +1,246 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bsepgsql\b/)
+{
+ plan skip_all =>
+ 'Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA';
+}
+
+note "checking selinux environment";
+
+# matchpathcon must be present to assess whether the installation environment
+# is OK.
+note "checking for matchpathcon";
+if (system('matchpathcon -n . >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The matchpathcon command must be available.
+Please install it or update your PATH to include it
+(it is typically in '/usr/sbin', which might not be in your PATH).
+matchpathcon is typically included in the libselinux-utils package.
+EOS
+ die;
+}
+
+# runcon must be present to launch psql using the correct environment
+note "checking for runcon";
+if (system('runcon --help >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The runcon command must be available.
+runcon is typically included in the coreutils package.
+EOS
+ die;
+}
+
+# check sestatus too, since that lives in yet another package
+note "checking for sestatus";
+if (system('sestatus >/dev/null 2>&1') != 0)
+{
+ diag <<EOS;
+
+The sestatus command must be available.
+sestatus is typically included in the policycoreutils package.
+EOS
+ die;
+}
+
+# check that the user is running in the unconfined_t domain
+note "checking current user domain";
+my $DOMAIN = (split /:/, `id -Z 2>/dev/null`)[2];
+note "current user domain is '$DOMAIN'";
+if ($DOMAIN ne 'unconfined_t')
+{
+ diag <<'EOS';
+
+The regression tests must be launched from the unconfined_t domain.
+
+The unconfined_t domain is typically the default domain for user
+shell processes. If the default has been changed on your system,
+you can revert the changes like this:
+
+ $ sudo semanage login -d `whoami`
+
+Or, you can add a setting to log in using the unconfined_t domain:
+
+ $ sudo semanage login -a -s unconfined_u -r s0-s0:c0.c255 `whoami`
+
+EOS
+ die;
+}
+
+# SELinux must be configured in enforcing mode
+note "checking selinux operating mode";
+my $CURRENT_MODE =
+ (split /: */, `LANG=C sestatus | grep '^Current mode:'`)[1];
+chomp $CURRENT_MODE;
+note "current operating mode is '$CURRENT_MODE'";
+if ($CURRENT_MODE eq 'enforcing')
+{
+ # OK
+}
+elsif ($CURRENT_MODE eq 'permissive' || $CURRENT_MODE eq 'disabled')
+{
+ diag <<'EOS';
+
+Before running the regression tests, SELinux must be enabled and
+must be running in enforcing mode.
+
+If SELinux is currently running in permissive mode, you can
+switch to enforcing mode using the 'setenforce' command.
+
+ $ sudo setenforce 1
+
+The system default setting is configured in /etc/selinux/config,
+or using a kernel boot parameter.
+EOS
+ die;
+}
+else
+{
+ diag <<EOS;
+
+Unable to determine the current selinux operating mode. Please
+verify that the sestatus command is installed and in your PATH.
+EOS
+ die;
+}
+
+# 'sepgsql-regtest' policy module must be loaded
+note "checking for sepgsql-regtest policy";
+my $SELINUX_MNT = (split /: */, `sestatus | grep '^SELinuxfs mount:'`)[1];
+chomp $SELINUX_MNT;
+if ($SELINUX_MNT eq "")
+{
+ diag <<EOS;
+
+Unable to find SELinuxfs mount point.
+
+The sestatus command should report the location where SELinuxfs
+is mounted, but did not do so.
+EOS
+ die;
+}
+if (!-e "${SELINUX_MNT}/booleans/sepgsql_regression_test_mode")
+{
+ diag <<'EOS';
+
+The 'sepgsql-regtest' policy module appears not to be installed.
+Without this policy installed, the regression tests will fail.
+You can install this module using the following commands:
+
+ $ make -f /usr/share/selinux/devel/Makefile
+ $ sudo semodule -u sepgsql-regtest.pp
+
+To confirm that the policy package is installed, use this command:
+
+ $ sudo semodule -l | grep sepgsql
+
+EOS
+ die;
+}
+
+# Verify that sepgsql_regression_test_mode is active.
+note "checking whether policy is enabled";
+foreach
+ my $policy ('sepgsql_regression_test_mode', 'sepgsql_enable_users_ddl')
+{
+ my $POLICY_STATUS = (split ' ', `getsebool $policy`)[2];
+ note "$policy is '$POLICY_STATUS'";
+ if ($POLICY_STATUS ne "on")
+ {
+ diag <<EOS;
+
+The SELinux boolean '$policy' must be
+turned on in order to enable the rules necessary to run the
+regression tests.
+
+EOS
+
+ if ($POLICY_STATUS eq "")
+ {
+ diag <<EOS;
+We attempted to determine the state of this Boolean using
+'getsebool', but that command did not produce the expected
+output. Please verify that getsebool is available and in
+your PATH.
+EOS
+ }
+ else
+ {
+ diag <<EOS;
+You can turn on this variable using the following commands:
+
+ \$ sudo setsebool $policy on
+
+For security reasons, it is suggested that you turn off this
+variable when regression testing is complete and the associated
+rules are no longer needed.
+EOS
+ }
+ die;
+ }
+}
+
+
+#
+# checking complete - let's run the tests
+#
+
+note "running sepgsql regression tests";
+
+my $node;
+
+$node = PostgreSQL::Test::Cluster->new('test');
+$node->init;
+$node->append_conf('postgresql.conf', 'log_statement=none');
+
+{
+ local %ENV = $node->_get_env();
+
+ my $result = run_log(
+ [
+ 'postgres', '--single',
+ '-F', '-c',
+ 'exit_on_error=true', '-D',
+ $node->data_dir, 'template0'
+ ],
+ '<',
+ $ENV{share_contrib_dir} . '/sepgsql.sql');
+ ok($result, 'sepgsql installation script');
+}
+
+$node->append_conf('postgresql.conf', 'shared_preload_libraries=sepgsql');
+$node->start;
+
+my @tests = qw(label dml ddl alter misc);
+
+# Check if the truncate permission exists in the loaded policy, and if so,
+# run the truncate test
+#
+# Testing the TRUNCATE regression test can be done by manually adding
+# the permission with CIL if necessary:
+# sudo semodule -cE base
+# sudo sed -i -E 's/(class db_table.*?) \)/\1 truncate\)/' base.cil
+# sudo semodule -i base.cil
+push @tests, 'truncate' if -f '/sys/fs/selinux/class/db_table/perms/truncate';
+
+$node->command_ok(
+ [
+ $ENV{PG_REGRESS}, '--bindir=', '--inputdir=.', '--launcher',
+ './launcher', @tests
+ ],
+ 'sepgsql tests');
+
+done_testing();
diff --git a/contrib/sepgsql/test_sepgsql b/contrib/sepgsql/test_sepgsql
index 3a29556d1ff..23dae1bf037 100755
--- a/contrib/sepgsql/test_sepgsql
+++ b/contrib/sepgsql/test_sepgsql
@@ -4,10 +4,10 @@
# to try to ensure that the SELinux environment is set up appropriately and
# the database is configured correctly.
#
-# Note that this must be run against an installed Postgres database.
-# There's no equivalent of "make check", and that wouldn't be terribly useful
-# since much of the value is in checking that you installed sepgsql into
-# your database correctly.
+# This must be run against an installed Postgres database. The
+# purpose of this script is in checking that you installed sepgsql
+# into your database correctly. For testing sepgsql during
+# development, "make check", "meson test", etc. are also available.
#
# This must be run in the contrib/sepgsql directory of a Postgres build tree.
#
@@ -302,5 +302,5 @@ if [ -f /sys/fs/selinux/class/db_table/perms/truncate ]; then
tests+=" truncate"
fi
-make REGRESS="$tests" REGRESS_OPTS="--launcher ./launcher" installcheck
-# exit with the exit code provided by "make"
+PGXS=`pg_config --pgxs`
+"$(dirname $PGXS)/../../src/test/regress/pg_regress" --inputdir=. --bindir="$PG_BINDIR" --launcher=./launcher $tests
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d1042e02228..9aebd9f3bed 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -284,6 +284,17 @@ <title>Additional Test Suites</title>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>sepgsql</literal></term>
+ <listitem>
+ <para>
+ Runs the test suite under <filename>contrib/sepgsql</filename>. This
+ requires an SELinux environment that is set up in a specific way; see
+ <xref linkend="sepgsql-regression"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>ssl</literal></term>
<listitem>
diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index bc308e3142d..9217c1b3d68 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -151,14 +151,23 @@ <title>Installation</title>
<sect2 id="sepgsql-regression">
<title>Regression Tests</title>
+ <para>
+ The <filename>sepgsql</filename> test suite is run if
+ <literal>PG_TEST_EXTRA</literal> contains <literal>sepgsql</literal> (see
+ <xref linkend="regress-additional"/>). This method is suitable during
+ development of <productname>PostgreSQL</productname>. Alternatively, there
+ is a way to run the tests to checks whether a database instance has been
+ set up properly for <literal>sepgsql</literal>.
+ </para>
+
<para>
Due to the nature of <productname>SELinux</productname>, running the
regression tests for <filename>sepgsql</filename> requires several extra
configuration steps, some of which must be done as root.
- The regression tests will not be run by an ordinary
- <literal>make check</literal> or <literal>make installcheck</literal> command; you must
- set up the configuration and then invoke the test script manually.
- The tests must be run in the <filename>contrib/sepgsql</filename> directory
+ </para>
+
+ <para>
+ The manual tests must be run in the <filename>contrib/sepgsql</filename> directory
of a configured PostgreSQL build tree. Although they require a build tree,
the tests are designed to be executed against an installed server,
that is they are comparable to <literal>make installcheck</literal> not
diff --git a/meson.build b/meson.build
index ea07126f78e..c18340adc90 100644
--- a/meson.build
+++ b/meson.build
@@ -3518,6 +3518,8 @@ foreach test_dir : tests
# also test/ for non-installed test binaries built separately.
env = test_env
env.prepend('PATH', temp_install_bindir, test_dir['bd'], test_dir['bd'] / 'test')
+ temp_install_datadir = '@0@@1@'.format(test_install_destdir, dir_prefix / dir_data)
+ env.set('share_contrib_dir', temp_install_datadir / 'contrib')
foreach name, value : t.get('env', {})
env.set(name, value)
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 42f50b49761..190f5559acf 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -456,6 +456,7 @@ cd $(srcdir) && \
PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
+ share_contrib_dir='$(DESTDIR)$(datadir)/$(datamoduledir)' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
else # PGXS case
@@ -483,6 +484,7 @@ cd $(srcdir) && \
$(with_temp_install) \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
+ share_contrib_dir='$(abs_top_builddir)/tmp_install$(datadir)/$(datamoduledir)' \
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
endef
--
2.46.0
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an extension
but a loose SQL script, of which it is now the only one. Maybe
something to address separately.)I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations. I
did change it so that it calls pg_regress directly, without going via
make, so that the dependency on make is removed.
This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an
extension but a loose SQL script, of which it is now the only one.
Maybe something to address separately.)I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations.
I did change it so that it calls pg_regress directly, without going
via make, so that the dependency on make is removed.This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.
This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 1/24/25 09:00, Andrew Dunstan wrote:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an
extension but a loose SQL script, of which it is now the only one.
Maybe something to address separately.)I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations.
I did change it so that it calls pg_regress directly, without going
via make, so that the dependency on make is removed.This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)
Sure, I am delinquent on updating my buildfarm client so will do that as
well as pull the fix. It might be the weekend before I have the time though.
This reminds me though that rhino is still running on CentOS 7 -- I need
to upgrade that one of these days...
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Andrew Dunstan <andrew@dunslane.net> writes:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.
This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)
Looks like alligator needs some help here too.
regards, tom lane
On 2025-01-24 Fr 10:57 AM, Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)Looks like alligator needs some help here too.
That's an issue with the new TAP test - alligator isn't running the
TestSepgsql module. lapwing has also had a TAP test failure.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
On 2025-01-24 Fr 10:57 AM, Tom Lane wrote:
Looks like alligator needs some help here too.
That's an issue with the new TAP test - alligator isn't running the
TestSepgsql module. lapwing has also had a TAP test failure.
Hmm. Neither of those animals should be trying to run the sepgsql
test; they are not configured --with-selinux, and probably don't
even have libselinux installed.
Looking at the buildfarm client script, it looks to me like it
will unconditionally try to run TAP tests in every contrib directory
that has a "t" subdirectory. Up to now, none of those needed to
be conditional ... but now we need some more awareness. However,
if this theory is right, then pretty much every BF animal except
rhinoceros should be failing. So there's some additional moving
part that I'm not seeing. In any case, I think there's a BF
client script bug here somewhere, or else these animals are
running an old script version that does the wrong thing.
regards, tom lane
On 2025-01-24 Fr 4:07 PM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
On 2025-01-24 Fr 10:57 AM, Tom Lane wrote:
Looks like alligator needs some help here too.
That's an issue with the new TAP test - alligator isn't running the
TestSepgsql module. lapwing has also had a TAP test failure.Hmm. Neither of those animals should be trying to run the sepgsql
test; they are not configured --with-selinux, and probably don't
even have libselinux installed.Looking at the buildfarm client script, it looks to me like it
will unconditionally try to run TAP tests in every contrib directory
that has a "t" subdirectory. Up to now, none of those needed to
be conditional ... but now we need some more awareness. However,
if this theory is right, then pretty much every BF animal except
rhinoceros should be failing. So there's some additional moving
part that I'm not seeing. In any case, I think there's a BF
client script bug here somewhere, or else these animals are
running an old script version that does the wrong thing.
The new TAP test has:
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bsepgsql\b/)
+{
+ plan skip_all =>
+ 'Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA';
+}It looks like the error on alligator is coming from a NON TAP test:
rm -rf '/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build'/tmp_install
/usr/bin/mkdir -p '/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build'/tmp_install/log
make -C '../..' DESTDIR='/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build'/tmp_install install >'/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build'/tmp_install/log/install.log 2>&1
make -j1 checkprep >>'/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build'/tmp_install/log/install.log 2>&1
make: *** [../../src/Makefile.global:424: temp-install] Error 2
But why is it doing that? On my Ubuntu 22.04 dev instance, this test shows this as expected:
echo "# +++ tap check in contrib/sepgsql +++" && rm -rf '/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql'/tmp_check && /usr/bin/mkdir -p '/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql'/tmp_check && cd /home/andrew/pgl/pg_head/contrib/sepgsql && TESTLOGDIR='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql/tmp_check/log' TESTDATADIR='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql/tmp_check' PATH="/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/tmp_install/home/andrew/pgl/pg_head/root/HEAD/inst/bin:/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql:$PATH" LD_LIBRARY_PATH="/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/tmp_install/home/andrew/pgl/pg_head/root/HEAD/inst/lib:$LD_LIBRARY_PATH" INITDB_TEMPLATE='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build'/tmp_install/initdb-template PGPORT='65678' top_builddir='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql/../..' PG_REGRESS='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql/../../src/test/regress/pg_regress' share_contrib_dir='/home/andrew/pgl/pg_head/root/HEAD/pgsql.build/tmp_install/home/andrew/pgl/pg_head/root/HEAD/inst/share/postgresql/contrib' /usr/bin/prove -I /home/andrew/pgl/pg_head/src/test/perl/ -I /home/andrew/pgl/pg_head/contrib/sepgsql --timer t/*.pl
# +++ tap check in contrib/sepgsql +++
[17:06:18] t/001_sepgsql.pl .. skipped: Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA
[17:06:18]
Files=1, Tests=0, 0 wallclock secs ( 0.00 usr 0.00 sys + 0.03 cusr 0.00 csys = 0.03 CPU)
Result: NOTESTS
log files for step contrib-sepgsqlCheck:
==~_~===-=-===~_~== /home/andrew/pgl/pg_head/root/HEAD/pgsql.build/contrib/sepgsql/tmp_check/log/regress_log_001_sepgsql ==~_~===-=-===~_~==
[17:06:18.231](0.004s) 1..0 # SKIP Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA
As of now I'm confused ...
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
On 2025-01-24 Fr 4:07 PM, Tom Lane wrote:
Looking at the buildfarm client script, it looks to me like it
will unconditionally try to run TAP tests in every contrib directory
that has a "t" subdirectory. Up to now, none of those needed to
be conditional ... but now we need some more awareness.
The new TAP test has:
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bsepgsql\b/) +{ + plan skip_all => + 'Potentially unsafe test sepgsql not enabled in PG_TEST_EXTRA'; +}
Yeah, but to get to that point you have to get past "make install",
which requires compiling the code, which will absolutely not work
on these platforms. alligator and lapwing are not reporting the
relevant log file, but what we do see is an install failure that
could well be down to a compile failure.
But why is it doing that? On my Ubuntu 22.04 dev instance, this test shows this as expected:
I don't understand how you're compiling on Ubuntu ... does it
have selinux installed? On a Mac for instance,
$ cd pgsql/contrib/sepgsql/
$ make install
...
In file included from database.c:21:
./sepgsql.h:17:10: fatal error: 'selinux/selinux.h' file not found
17 | #include <selinux/selinux.h>
| ^~~~~~~~~~~~~~~~~~~
1 error generated.
As of now I'm confused ...
Me too. The way it looks from here, the farm should be all red,
but it isn't.
regards, tom lane
I wrote:
Yeah, but to get to that point you have to get past "make install",
Oh ... wait a second. After further code reading I see that
the BF client sets NO_TEMP_INSTALL if check_install_is_complete
succeeds. So evidently, that is successfully suppressing
"make install" on most animals, but not these two. How come?
regards, tom lane
I wrote:
Oh ... wait a second. After further code reading I see that
the BF client sets NO_TEMP_INSTALL if check_install_is_complete
succeeds. So evidently, that is successfully suppressing
"make install" on most animals, but not these two. How come?
Got it: we can see on alligator that it installs (for instance)
test_parser.so into
/usr/bin/install -c -m 755 test_parser.so '/home/postgres/proj/build-farm-17/buildroot/HEAD/inst/lib/test_parser.so'
but what check_install_is_complete is looking for is
$tmp_loc = "$tmp_loc/$install_dir";
$bindir = "$tmp_loc/bin";
$libdir = "$tmp_loc/lib/postgresql";
...
my $res =
( (-d $tmp_loc)
&& (-f "$bindir/postgres" || -f "$bindir/postgres.exe")
&& (-f "$libdir/hstore$suffix")
&& (-f "$libdir/test_parser$suffix"));
That is, check_install_is_complete expects to see test_parser.so
under installdir/lib/postgresql/, but it's actually getting put
into installdir/lib/ because "postgres" appears earlier in the
path. So we're forcing a bunch of useless "make install"s,
but that was never mission-critical until today.
Unsurprisingly, lapwing is also running under /home/postgres/.
Apparently no other BF animals are.
regards, tom lane
On Sat, 25 Jan 2025 at 08:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
alligator and lapwing are not reporting the
relevant log file, but what we do see is an install failure that
could well be down to a compile failure.
You're probably right about that.
This is what I see in the install.log
(/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build/tmp_install/log/install.log)
make[2]: Entering directory
'/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build/contrib/sepgsql'
ccache 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 -Wmissing-variable-declarations
-fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -g -O2 -fPIC
-fvisibility=hidden -I. -I. -I../../src/include -D_GNU_SOURCE
-I/usr/include/libxml2 -c -o database.o database.c
In file included from database.c:21:
sepgsql.h:17:10: fatal error: selinux/selinux.h: No such file or directory
17 | #include <selinux/selinux.h>
| ^~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [<builtin>: database.o] Error 1
make[2]: Leaving directory
'/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build/contrib/sepgsql'
make[1]: *** [../../src/Makefile.global:435: checkprep] Error 2
make[1]: Leaving directory
'/home/postgres/proj/build-farm-17/buildroot/HEAD/pgsql.build/contrib/sepgsql'
-
robins
On 2025-01-24 Fr 6:08 PM, Tom Lane wrote:
I wrote:
Oh ... wait a second. After further code reading I see that
the BF client sets NO_TEMP_INSTALL if check_install_is_complete
succeeds. So evidently, that is successfully suppressing
"make install" on most animals, but not these two. How come?Got it: we can see on alligator that it installs (for instance)
test_parser.so into/usr/bin/install -c -m 755 test_parser.so '/home/postgres/proj/build-farm-17/buildroot/HEAD/inst/lib/test_parser.so'
but what check_install_is_complete is looking for is
$tmp_loc = "$tmp_loc/$install_dir";
$bindir = "$tmp_loc/bin";
$libdir = "$tmp_loc/lib/postgresql";
...
my $res =
( (-d $tmp_loc)
&& (-f "$bindir/postgres" || -f "$bindir/postgres.exe")
&& (-f "$libdir/hstore$suffix")
&& (-f "$libdir/test_parser$suffix"));That is, check_install_is_complete expects to see test_parser.so
under installdir/lib/postgresql/, but it's actually getting put
into installdir/lib/ because "postgres" appears earlier in the
path. So we're forcing a bunch of useless "make install"s,
but that was never mission-critical until today.Unsurprisingly, lapwing is also running under /home/postgres/.
Apparently no other BF animals are.
Oh, good catch!
I'll go and fix it.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Robins Tharakan <tharakan@gmail.com> writes:
On Sat, 25 Jan 2025 at 08:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
alligator and lapwing are not reporting the
relevant log file, but what we do see is an install failure that
could well be down to a compile failure.
You're probably right about that.
This is what I see in the install.log
In file included from database.c:21:
sepgsql.h:17:10: fatal error: selinux/selinux.h: No such file or directory
17 | #include <selinux/selinux.h>
| ^~~~~~~~~~~~~~~~~~~
compilation terminated.
Yeah, that's about what I expected. As a workaround until Andrew
updates the BF client, you could do
- $libdir = "$tmp_loc/lib/postgresql";
+ $libdir = "$tmp_loc/lib";at about line 429 of PGBuild/Utils.pm
regards, tom lane
On Sat, 25 Jan 2025 at 10:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yeah, that's about what I expected. As a workaround until Andrew
updates the BF client, you could do- $libdir = "$tmp_loc/lib/postgresql"; + $libdir = "$tmp_loc/lib";at about line 429 of PGBuild/Utils.pm
Ack. Triggered a fresh HEAD run to see how it pans out.
postgres@dell:~/proj/build-farm-17$ head -430 PGBuild/Utils.pm | tail -3
$bindir = "$tmp_loc/bin";
$libdir = "$tmp_loc/lib";
}
-
robins
On 2025-01-24 Fr 7:09 PM, Robins Tharakan wrote:
On Sat, 25 Jan 2025 at 10:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yeah, that's about what I expected. As a workaround until Andrew
updates the BF client, you could do- $libdir = "$tmp_loc/lib/postgresql"; + $libdir = "$tmp_loc/lib";at about line 429 of PGBuild/Utils.pm
Ack. Triggered a fresh HEAD run to see how it pans out.
postgres@dell:~/proj/build-farm-17$ head -430 PGBuild/Utils.pm | tail -3
$bindir = "$tmp_loc/bin";
$libdir = "$tmp_loc/lib";
}
Here's the hot fix (which passed my test with a directory with pgsql in
its path):
https://github.com/PGBuildFarm/client-code/commit/f6c6dd52d2959814452454890fb9838429c5c3e8
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
On 2025-01-24 Fr 7:50 PM, Andrew Dunstan wrote:
Here's the hot fix (which passed my test with a directory with pgsql
in its path):https://github.com/PGBuildFarm/client-code/commit/f6c6dd52d2959814452454890fb9838429c5c3e8
Oops, wrong address on previous email. Julien, please update the address
for your animal using the update_personality.pl script, and apply the
fix above.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On Sat, 25 Jan 2025 at 11:57, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2025-01-24 Fr 7:50 PM, Andrew Dunstan wrote:
Here's the hot fix (which passed my test with a directory with pgsql
in its path):
https://github.com/PGBuildFarm/client-code/commit/f6c6dd52d2959814452454890fb9838429c5c3e8
Switching alligator's bf client (from v17 release tarball) to git; I see
HEAD passed fine.
-
robins
Peter Eisentraut <peter@eisentraut.org> writes:
This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.
BTW, shouldn't the CF entry for this get closed now?
regards, tom lane
On 25.01.25 22:55, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.BTW, shouldn't the CF entry for this get closed now?
done
On 1/24/25 09:00, Andrew Dunstan wrote:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an
extension but a loose SQL script, of which it is now the only one.
Maybe something to address separately.)I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations.
I did change it so that it calls pg_regress directly, without going
via make, so that the dependency on make is removed.This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)
Out of curiosity, what should have changed? I have so far done nothing
to rhino, yet it continues to return OK even after Peter's change...
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 2025-01-26 Su 10:29 AM, Joe Conway wrote:
On 1/24/25 09:00, Andrew Dunstan wrote:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I
also fixed "make installcheck". I noticed that meson installs
sepgsql.sql into the wrong directory, so that's fixed also. (Many
of the complications in this patch set are because sepgsql is not
an extension but a loose SQL script, of which it is now the only
one. Maybe something to address separately.)I did end up deciding to keep the old test_sepgsql script, because
it does have the documented purpose of testing existing
installations. I did change it so that it calls pg_regress
directly, without going via make, so that the dependency on make is
removed.This has been committed. And I understand there is a buildfarm
client update available for the affected buildfarm members.This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)Out of curiosity, what should have changed? I have so far done nothing
to rhino, yet it continues to return OK even after Peter's change...
It will run the old test redundantly. Peter left it in place.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 1/26/25 11:36, Andrew Dunstan wrote:
On 2025-01-26 Su 10:29 AM, Joe Conway wrote:
On 1/24/25 09:00, Andrew Dunstan wrote:
On 2025-01-24 Fr 7:25 AM, Peter Eisentraut wrote:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I
also fixed "make installcheck". I noticed that meson installs
sepgsql.sql into the wrong directory, so that's fixed also. (Many
of the complications in this patch set are because sepgsql is not
an extension but a loose SQL script, of which it is now the only
one. Maybe something to address separately.)I did end up deciding to keep the old test_sepgsql script, because
it does have the documented purpose of testing existing
installations. I did change it so that it calls pg_regress
directly, without going via make, so that the dependency on make is
removed.This has been committed. And I understand there is a buildfarm
client update available for the affected buildfarm members.This should only be rhinoceros. Joe can pull this fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db(or just copy the whole file from
https://raw.githubusercontent.com/PGBuildFarm/client-code/refs/heads/main/PGBuild/Modules/TestSepgsql.pm)Out of curiosity, what should have changed? I have so far done nothing
to rhino, yet it continues to return OK even after Peter's change...It will run the old test redundantly. Peter left it in place.
I have pulled and applied the fix:
https://github.com/PGBuildFarm/client-code/commit/60b72787036090c6bf829f5cef2b0b3e60f2a2db
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Peter Eisentraut <peter@eisentraut.org> writes:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an
extension but a loose SQL script, of which it is now the only one.
Maybe something to address separately.)
I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations. I
did change it so that it calls pg_regress directly, without going via
make, so that the dependency on make is removed.This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.
This patch passed the TAP command invocation cleanup patch mid-flight,
so didn't get the memo about command usng the fat comma for line option
arguments. Here's a patch for bringing it in line with the new
convention. I don't have any machines with SELinux enabled, so either
someone who has would need to test it, or we can rely on the buildfarm.
- ilmari
Attachments:
0001-sepgsql-update-TAP-test-to-use-fat-comma-style.patchtext/x-diffDownload
From bc899fbe7a89fcdf198421a9abf608772748c1ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 28 Jan 2025 13:32:35 +0000
Subject: [PATCH] sepgsql: update TAP test to use fat comma style
---
contrib/sepgsql/t/001_sepgsql.pl | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/contrib/sepgsql/t/001_sepgsql.pl b/contrib/sepgsql/t/001_sepgsql.pl
index cba51403518..c5fd7254841 100644
--- a/contrib/sepgsql/t/001_sepgsql.pl
+++ b/contrib/sepgsql/t/001_sepgsql.pl
@@ -211,10 +211,10 @@
my $result = run_log(
[
- 'postgres', '--single',
- '-F', '-c',
- 'exit_on_error=true', '-D',
- $node->data_dir, 'template0'
+ 'postgres', '--single', '-F',
+ '-c' => 'exit_on_error=true',
+ '-D' => $node->data_dir,
+ 'template0'
],
'<',
$ENV{share_contrib_dir} . '/sepgsql.sql');
@@ -238,8 +238,11 @@
$node->command_ok(
[
- $ENV{PG_REGRESS}, '--bindir=', '--inputdir=.', '--launcher',
- './launcher', @tests
+ $ENV{PG_REGRESS},
+ '--bindir' => '',
+ '--inputdir' => '.',
+ '--launcher' => './launcher',
+ @tests
],
'sepgsql tests');
--
2.48.1
On 28.01.25 14:34, Dagfinn Ilmari Mannsåker wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
On 27.08.24 10:12, Peter Eisentraut wrote:
Here is a new patch version.
I simplified the uses of sed and awk inside the Perl script. I also
fixed "make installcheck". I noticed that meson installs sepgsql.sql
into the wrong directory, so that's fixed also. (Many of the
complications in this patch set are because sepgsql is not an
extension but a loose SQL script, of which it is now the only one.
Maybe something to address separately.)
I did end up deciding to keep the old test_sepgsql script, because it
does have the documented purpose of testing existing installations. I
did change it so that it calls pg_regress directly, without going via
make, so that the dependency on make is removed.This has been committed. And I understand there is a buildfarm client
update available for the affected buildfarm members.This patch passed the TAP command invocation cleanup patch mid-flight,
so didn't get the memo about command usng the fat comma for line option
arguments. Here's a patch for bringing it in line with the new
convention. I don't have any machines with SELinux enabled, so either
someone who has would need to test it, or we can rely on the buildfarm.
committed, thanks!