[PATCH v2 0/5] Sandbox test suite and enable podman tests on more hosts

[PATCH v2 0/5] Sandbox test suite and enable podman tests on more hosts

[David Gibson]

I noticed the podman tests weren't actually running on my Fedora host.
This turns out to be because cataonit is not in the path on Fedora
(it's in /usr/libexec).

While attempting to get this working with my "test in a box" script I
ran into some additional problems: the podman tests downloaded and
built podman, which requires external network access.  That doesn't
work in an isolated network environment.

Changes since v1:
 * Test that podman is using the correct pasta binary
 * Added patch to prevent make cppcheck from checking the downloaded
   podman source as well.

David Gibson (5):
  cppcheck: Explicitly give files to check
  test: Make sure to update mbuto repository
  test: Build and download podman as a test asset
  test: catatonit may not be in $PATH
  test: Verify that podman tests are using the pasta binary we expect

 Makefile               |  2 +-
 seccomp.sh             |  4 ++--
 test/.gitignore        |  1 +
 test/Makefile          | 20 +++++++++++++++++---
 test/pasta_podman/bats | 15 ++++++++++-----
 5 files changed, 31 insertions(+), 11 deletions(-)

-- 
2.44.0

[PATCH v2 1/5] cppcheck: Explicitly give files to check

[David Gibson]

Currently "make cppcheck" invokes cppcheck on ".", so it will check all the
.c and .h files it can find in the source tree.  This isn't ideal, because
it can find files that aren't actually part of the real build, or even
stale files which aren't in git.

More practically, some upcoming changes are looking at downloading other
source trees for some tests.  Static errors in there is Not Our Problem,
so checking them is both slow and pointless.

So, change the Makefile to invoke cppcheck only on the specific source
files that are part of the build.  For some reason in this format the
badBitmaskCheck warnings in seccomp.h which were suppressed by 5beb3472e
("cppcheck: Avoid errors due to zeroes in bitwise ORs") no longer trigger.
That means we get unmatchedSuppression warnings instead.  We add an
unmatchedSuppression suppression instead of simply removing the original
suppressions, just in case this odd behaviour isn't the same for all
cppcheck versions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile   | 2 +-
 seccomp.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 84280520..c1e1f062 100644
--- a/Makefile
+++ b/Makefile
@@ -308,4 +308,4 @@ cppcheck: $(SRCS) $(HEADERS)
 	--inline-suppr							\
 	--suppress=unusedStructMember					\
 	$(filter -D%,$(FLAGS) $(CFLAGS) $(CPPFLAGS))			\
-	.
+	$(SRCS) $(HEADERS)
diff --git a/seccomp.sh b/seccomp.sh
index e1224e0d..052e1c8c 100755
--- a/seccomp.sh
+++ b/seccomp.sh
@@ -29,11 +29,11 @@ HEADER="/* This file was automatically generated by $(basename ${0}) */
 # Prefix for each profile: check that 'arch' in seccomp_data is matching
 PRE='
 struct sock_filter filter_@PROFILE@[] = {
-	/* cppcheck-suppress badBitmaskCheck */
+	/* cppcheck-suppress [badBitmaskCheck, unmatchedSuppression] */
 	BPF_STMT(BPF_LD | BPF_W | BPF_ABS,
 		 (offsetof(struct seccomp_data, arch))),
 	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, PASST_AUDIT_ARCH, 0, @KILL@),
-	/* cppcheck-suppress badBitmaskCheck */
+	/* cppcheck-suppress [badBitmaskCheck, unmatchedSuppression] */
 	BPF_STMT(BPF_LD | BPF_W | BPF_ABS,
 		 (offsetof(struct seccomp_data, nr))),
 
-- 
@@ -29,11 +29,11 @@ HEADER="/* This file was automatically generated by $(basename ${0}) */
 # Prefix for each profile: check that 'arch' in seccomp_data is matching
 PRE='
 struct sock_filter filter_@PROFILE@[] = {
-	/* cppcheck-suppress badBitmaskCheck */
+	/* cppcheck-suppress [badBitmaskCheck, unmatchedSuppression] */
 	BPF_STMT(BPF_LD | BPF_W | BPF_ABS,
 		 (offsetof(struct seccomp_data, arch))),
 	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, PASST_AUDIT_ARCH, 0, @KILL@),
-	/* cppcheck-suppress badBitmaskCheck */
+	/* cppcheck-suppress [badBitmaskCheck, unmatchedSuppression] */
 	BPF_STMT(BPF_LD | BPF_W | BPF_ABS,
 		 (offsetof(struct seccomp_data, nr))),
 
-- 
2.44.0

[PATCH v2 2/5] test: Make sure to update mbuto repository

[David Gibson]

We download and use mbuto to build trivial boot images for our VM tests.
However, if mbuto is already cloned, we won't update it to the current
version.  Add some make logic to ensure that we do this.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/Makefile b/test/Makefile
index 7b00bef4..711c61c1 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -67,13 +67,19 @@ CFLAGS = -Wall -Werror -Wextra -pedantic -std=c99
 
 assets: $(ASSETS)
 
+.PHONY: pull-%
+pull-%: %
+	git -C $* pull
+
 mbuto:
 	git clone git://mbuto.sh/mbuto
 
+mbuto/mbuto: pull-mbuto
+
 guest-key guest-key.pub:
 	ssh-keygen -f guest-key -N ''
 
-mbuto.img: passt.mbuto mbuto guest-key.pub $(TESTDATA_ASSETS)
+mbuto.img: passt.mbuto mbuto/mbuto guest-key.pub $(TESTDATA_ASSETS)
 	./mbuto/mbuto -p ./$< -c lz4 -f $@
 
 mbuto.mem.img: passt.mem.mbuto mbuto ../passt.avx2
-- 
@@ -67,13 +67,19 @@ CFLAGS = -Wall -Werror -Wextra -pedantic -std=c99
 
 assets: $(ASSETS)
 
+.PHONY: pull-%
+pull-%: %
+	git -C $* pull
+
 mbuto:
 	git clone git://mbuto.sh/mbuto
 
+mbuto/mbuto: pull-mbuto
+
 guest-key guest-key.pub:
 	ssh-keygen -f guest-key -N ''
 
-mbuto.img: passt.mbuto mbuto guest-key.pub $(TESTDATA_ASSETS)
+mbuto.img: passt.mbuto mbuto/mbuto guest-key.pub $(TESTDATA_ASSETS)
 	./mbuto/mbuto -p ./$< -c lz4 -f $@
 
 mbuto.mem.img: passt.mem.mbuto mbuto ../passt.avx2
-- 
2.44.0

[PATCH v2 3/5] test: Build and download podman as a test asset

[David Gibson]

The pasta_podman/bats test scrpt downloads and builds podman, then runs its
pasta specific tests.  Downloading from within a test case has some
drawbacks:
 * It can be very tedious if you have poor connectivity to the server
 * It makes a test that's ostensibly for pasta itself dependent on the
   state of the github server
 * It precludes runnning the tests in an isolated network environment

The same concerns largely apply to building podman too, because it's pretty
common for Go builds to download dependencies themselves.  Therefore move
the download and build of podman from the test itself, to the Makefile
where we prepare other test assets.

To avoid cryptic failures if something went wrong with the build, make
running the test dependent on having the built podman binary.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/.gitignore        |  1 +
 test/Makefile          | 12 ++++++++++--
 test/pasta_podman/bats |  6 ++----
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/test/.gitignore b/test/.gitignore
index 48374028..6dd4790b 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -1,5 +1,6 @@
 test_logs/
 mbuto/
+podman/
 *.img
 QEMU_EFI.fd
 *.qcow2
diff --git a/test/Makefile b/test/Makefile
index 711c61c1..35a3b559 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -52,10 +52,10 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \
 	jammy-server-cloudimg-s390x.img
 UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS)
 
-DOWNLOAD_ASSETS = mbuto \
+DOWNLOAD_ASSETS = mbuto podman \
 	$(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)
 TESTDATA_ASSETS = small.bin big.bin medium.bin
-LOCAL_ASSETS = mbuto.img mbuto.mem.img QEMU_EFI.fd \
+LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \
 	$(DEBIAN_IMGS:%=prepared-%) $(FEDORA_IMGS:%=prepared-%) \
 	$(UBUNTU_NEW_IMGS:%=prepared-%) \
 	nstool guest-key guest-key.pub \
@@ -76,6 +76,14 @@ mbuto:
 
 mbuto/mbuto: pull-mbuto
 
+podman:
+	git clone https://github.com/containers/podman.git
+
+# To succesfully build podman, you will need gpgme and systemd
+# development packages
+podman/bin/podman: pull-podman
+	$(MAKE) -C podman
+
 guest-key guest-key.pub:
 	ssh-keygen -f guest-key -N ''
 
diff --git a/test/pasta_podman/bats b/test/pasta_podman/bats
index 21446f08..cb88aa41 100644
--- a/test/pasta_podman/bats
+++ b/test/pasta_podman/bats
@@ -11,11 +11,9 @@
 # Copyright (c) 2022 Red Hat GmbH
 # Author: Stefano Brivio <sbrivio@redhat.com>
 
-htools	git make go bats catatonit ip jq socat
+htools	git make go bats catatonit ip jq socat ./test/podman/bin/podman
 
 test	Podman system test with bats
 
-host	git -C __STATEDIR__ clone https://github.com/containers/podman.git
-host	make -C __STATEDIR__/podman
 hout	WD pwd
-host	PODMAN="__STATEDIR__/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats __STATEDIR__/podman/test/system/505-networking-pasta.bats
+host	PODMAN="test/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
-- 
@@ -11,11 +11,9 @@
 # Copyright (c) 2022 Red Hat GmbH
 # Author: Stefano Brivio <sbrivio@redhat.com>
 
-htools	git make go bats catatonit ip jq socat
+htools	git make go bats catatonit ip jq socat ./test/podman/bin/podman
 
 test	Podman system test with bats
 
-host	git -C __STATEDIR__ clone https://github.com/containers/podman.git
-host	make -C __STATEDIR__/podman
 hout	WD pwd
-host	PODMAN="__STATEDIR__/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats __STATEDIR__/podman/test/system/505-networking-pasta.bats
+host	PODMAN="test/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
-- 
2.44.0

[PATCH v2 4/5] test: catatonit may not be in $PATH

[David Gibson]

The pasta_podman/bats test script looks for 'catatonit' amongst other tools
to be avaiiliable on the host.  However, while the podman tests do require
catatonit, it doesn't necessarily need to be in the regular path.  For
example Fedora and RHEL place catatonit in /usr/libexec and podman finds it
there fine.

Therefore, remove it as an htools dependency.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/pasta_podman/bats | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/pasta_podman/bats b/test/pasta_podman/bats
index cb88aa41..46a958a9 100644
--- a/test/pasta_podman/bats
+++ b/test/pasta_podman/bats
@@ -11,7 +11,7 @@
 # Copyright (c) 2022 Red Hat GmbH
 # Author: Stefano Brivio <sbrivio@redhat.com>
 
-htools	git make go bats catatonit ip jq socat ./test/podman/bin/podman
+htools	git make go bats ip jq socat ./test/podman/bin/podman
 
 test	Podman system test with bats
 
-- 
@@ -11,7 +11,7 @@
 # Copyright (c) 2022 Red Hat GmbH
 # Author: Stefano Brivio <sbrivio@redhat.com>
 
-htools	git make go bats catatonit ip jq socat ./test/podman/bin/podman
+htools	git make go bats ip jq socat ./test/podman/bin/podman
 
 test	Podman system test with bats
 
-- 
2.44.0

[PATCH v2 5/5] test: Verify that podman tests are using the pasta binary we expect

[David Gibson]

Paul Holzinger pointed out that when we invoke the podman tests inside the
passt testsuite, the way we point podman at the newly built pasta binary
is kind of indirect.  It's therefore prudent to check that podman is
actually using the binary we expect it to - in particular that it is using
the binary built in this tree, not some system installed pasta binary.

Suggested-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/pasta_podman/bats | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/test/pasta_podman/bats b/test/pasta_podman/bats
index 46a958a9..6b1c5751 100644
--- a/test/pasta_podman/bats
+++ b/test/pasta_podman/bats
@@ -13,7 +13,14 @@
 
 htools	git make go bats ip jq socat ./test/podman/bin/podman
 
+set	PODMAN test/podman/bin/podman
+hout	WD pwd
+
+test	Podman pasta path
+
+hout	PASTA_BIN CONTAINERS_HELPER_BINARY_DIR="__WD__" __PODMAN__ info --format "{{.Host.Pasta.Executable}}"
+check	[ "__PASTA_BIN__" = "__WD__/pasta" ]
+
 test	Podman system test with bats
 
-hout	WD pwd
-host	PODMAN="test/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
+host	PODMAN="__PODMAN__" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
-- 
@@ -13,7 +13,14 @@
 
 htools	git make go bats ip jq socat ./test/podman/bin/podman
 
+set	PODMAN test/podman/bin/podman
+hout	WD pwd
+
+test	Podman pasta path
+
+hout	PASTA_BIN CONTAINERS_HELPER_BINARY_DIR="__WD__" __PODMAN__ info --format "{{.Host.Pasta.Executable}}"
+check	[ "__PASTA_BIN__" = "__WD__/pasta" ]
+
 test	Podman system test with bats
 
-hout	WD pwd
-host	PODMAN="test/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
+host	PODMAN="__PODMAN__" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
-- 
2.44.0

Re: [PATCH v2 0/5] Sandbox test suite and enable podman tests on more hosts

[Stefano Brivio]

On Thu, 21 Mar 2024 15:57:37 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> I noticed the podman tests weren't actually running on my Fedora host.
> This turns out to be because cataonit is not in the path on Fedora
> (it's in /usr/libexec).
> 
> While attempting to get this working with my "test in a box" script I
> ran into some additional problems: the podman tests downloaded and
> built podman, which requires external network access.  That doesn't
> work in an isolated network environment.
> 
> Changes since v1:
>  * Test that podman is using the correct pasta binary
>  * Added patch to prevent make cppcheck from checking the downloaded
>    podman source as well.
> 
> David Gibson (5):
>   cppcheck: Explicitly give files to check
>   test: Make sure to update mbuto repository
>   test: Build and download podman as a test asset
>   test: catatonit may not be in $PATH
>   test: Verify that podman tests are using the pasta binary we expect

Applied.

-- 
Stefano