On 24.07.24 23:03, Andreas Karlsson wrote:
On 7/24/24 10:35 PM, Tom Lane wrote:
Andreas Karlsson <andr...@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.
From 2fe3e9020fe11ed5e86432d98f1c7283668654e4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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
From 9322b87aad3d207731dc71853f39cb9651ea8a18 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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-0c8453a4e...@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
