From 198155027d7eb535a5786eddf80a7c103484a7a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 20 Jan 2023 17:21:54 +0100 Subject: [PATCH 1/3] Fix (test-integration), in a container without CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1222 . Signed-off-by: Miloslav Trmač --- Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e5f94867..a29f145f 100644 --- a/Makefile +++ b/Makefile @@ -197,7 +197,11 @@ shell: check: validate test-unit test-integration test-system test-integration: - $(CONTAINER_RUN) $(MAKE) test-integration-local +# This is intended to be equal to $(CONTAINER_RUN), but with --cap-add=cap_mknod. +# --cap-add=cap_mknod is important to allow skopeo to use containers-storage: directly as it exists in the callers’ environment, without +# creating a nested user namespace (which requires /etc/subuid and /etc/subgid to be set up) + $(CONTAINER_CMD) --security-opt label=disable --cap-add=cap_mknod -v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) $(SKOPEO_CIDEV_CONTAINER_FQIN) \ + $(MAKE) test-integration-local # Intended for CI, assumed to be running in quay.io/libpod/skopeo_cidev container. From a98c137243d815af851059e6c65b2d2b1b2268a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 19 Jan 2023 23:40:08 +0100 Subject: [PATCH 2/3] Fix storage.conf setup in test-system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don't do it at all for the CI VM: We can use the VM's global Podman configuration, and use faster overlay instead of vfs, so let's do that. - For the developer-run (make test-system): - Add graphroot and runroot paths to make the configuration minimally valid - Explicitly point CONTAINERS_STORAGE_CONF at the configutation to be certain it will get used. Then drop the (podman pull ...) in runner.sh:_podman_reset that seemed to previously workaround the invalid /etc/containers/storage.conf . Signed-off-by: Miloslav Trmač --- contrib/cirrus/runner.sh | 9 --------- hack/make/test-system | 34 +++++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh index fb128ccb..d74b26b9 100755 --- a/contrib/cirrus/runner.sh +++ b/contrib/cirrus/runner.sh @@ -115,15 +115,6 @@ _run_unit() { _podman_reset() { # Ensure we start with a clean-slate showrun podman system reset --force - # WARNING WARNING WARNING WARNING - # Without running a container, the system tests will inexplicably - # fail with obscure errors/warning messages. I have no idea why - # running a container after a `system reset` fixes/prevents the - # problem. The failures do not reproduce when tests are run manually. - # So unless or until /until somebody develops a better understanding, - # this fix is JFM - just fakking magic. - # WARNING WARNING WARNING WARNING - showrun podman run -it --rm --entrypoint /bin/true quay.io/libpod/alpine:latest } _run_integration() { diff --git a/hack/make/test-system b/hack/make/test-system index 47998742..41c00845 100755 --- a/hack/make/test-system +++ b/hack/make/test-system @@ -5,16 +5,36 @@ set -e # not all storage drivers are supported in a container # environment. Detect this and setup storage when # running in a container. -if ((SKOPEO_CONTAINER_TESTS)) && [[ -r /etc/containers/storage.conf ]]; then - sed -i \ - -e 's/^driver\s*=.*/driver = "vfs"/' \ - -e 's/^mountopt/#mountopt/' \ - /etc/containers/storage.conf -elif ((SKOPEO_CONTAINER_TESTS)); then - cat >> /etc/containers/storage.conf << EOF +# +# Paradoxically (FIXME: clean this up), SKOPEO_CONTAINER_TESTS is set +# both inside a container and without a container (in a CI VM); it actually means +# "it is safe to desctructively modify the system for tests". +# +# On a CI VM, we can just use Podman as it is already configured; the changes below, +# to use VFS, are necessary only inside a container, because overlay-inside-overlay +# does not work. So, make these changes conditional on both +# SKOPEO_CONTAINER_TESTS (for acceptability to do destructive modification) and !CI +# (for necessity to adjust for in-container operation) +if ((SKOPEO_CONTAINER_TESTS)) && [[ "$CI" != true ]]; then + if [[ -r /etc/containers/storage.conf ]]; then + echo "MODIFYING existing storage.conf" + sed -i \ + -e 's/^driver\s*=.*/driver = "vfs"/' \ + -e 's/^mountopt/#mountopt/' \ + /etc/containers/storage.conf + else + echo "CREATING NEW storage.conf" + cat >> /etc/containers/storage.conf << EOF [storage] driver = "vfs" +runroot = "/run/containers/storage" +graphroot = "/var/lib/containers/storage" EOF + fi + # The logic of finding the relevant storage.conf file is convoluted + # and in effect differs between Skopeo and Podman, at least in some versions; + # explicitly point at the file we want to use to hopefully avoid that. + export CONTAINERS_STORAGE_CONF=/etc/containers/storage.conf fi # Build skopeo, install into /usr/bin From 850bc49d27fe0beb5bed32c9c0287cad6d2568b7 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 20 Jan 2023 13:10:14 +0000 Subject: [PATCH 3/3] Update module github.com/containers/storage to v1.45.3 Signed-off-by: Renovate Bot --- go.mod | 2 +- go.sum | 4 ++-- vendor/github.com/containers/storage/VERSION | 2 +- vendor/github.com/containers/storage/types/utils.go | 3 +++ vendor/github.com/containers/storage/userns.go | 10 +++++++--- vendor/modules.txt | 2 +- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 5e8db474..0b8cac59 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/containers/common v0.50.1 github.com/containers/image/v5 v5.23.1-0.20230113185223-cf9ccfb4d9b1 github.com/containers/ocicrypt v1.1.7 - github.com/containers/storage v1.45.1 + github.com/containers/storage v1.45.3 github.com/docker/distribution v2.8.1+incompatible github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc2 diff --git a/go.sum b/go.sum index 0d563a0f..e7ae0a6d 100644 --- a/go.sum +++ b/go.sum @@ -935,8 +935,8 @@ github.com/containers/ocicrypt v1.1.7 h1:thhNr4fu2ltyGz8aMx8u48Ae0Pnbip3ePP9/mzk github.com/containers/ocicrypt v1.1.7/go.mod h1:7CAhjcj2H8AYp5YvEie7oVSK2AhBY8NscCYRawuDNtw= github.com/containers/storage v1.43.0/go.mod h1:uZ147thiIFGdVTjMmIw19knttQnUCl3y9zjreHrg11s= github.com/containers/storage v1.45.0/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g= -github.com/containers/storage v1.45.1 h1:hsItObigGLm77Dn4ebUxQ68EfE6nMrwGcIdMRqzgclI= -github.com/containers/storage v1.45.1/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g= +github.com/containers/storage v1.45.3 h1:GbtTvTtp3GW2/tcFg5VhgHXcYMwVn2KfZKiHjf9FAOM= +github.com/containers/storage v1.45.3/go.mod h1:OdRUYHrq1HP6iAo79VxqtYuJzC5j4eA2I60jKOoCT7g= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= diff --git a/vendor/github.com/containers/storage/VERSION b/vendor/github.com/containers/storage/VERSION index df7da090..53999456 100644 --- a/vendor/github.com/containers/storage/VERSION +++ b/vendor/github.com/containers/storage/VERSION @@ -1 +1 @@ -1.45.1 +1.45.3 diff --git a/vendor/github.com/containers/storage/types/utils.go b/vendor/github.com/containers/storage/types/utils.go index 76ff122b..72c38f86 100644 --- a/vendor/github.com/containers/storage/types/utils.go +++ b/vendor/github.com/containers/storage/types/utils.go @@ -173,6 +173,9 @@ func DefaultConfigFile(rootless bool) (string, error) { return path, nil } if !rootless { + if _, err := os.Stat(defaultOverrideConfigFile); err == nil { + return defaultOverrideConfigFile, nil + } return defaultConfigFile, nil } diff --git a/vendor/github.com/containers/storage/userns.go b/vendor/github.com/containers/storage/userns.go index 720752f8..9c27b5d3 100644 --- a/vendor/github.com/containers/storage/userns.go +++ b/vendor/github.com/containers/storage/userns.go @@ -78,6 +78,10 @@ func (s *store) getAvailableIDs() (*idSet, *idSet, error) { return u, g, nil } +// nobodyUser returns the UID and GID of the "nobody" user. Hardcode its value +// for simplicity. +const nobodyUser = 65534 + // parseMountedFiles returns the maximum UID and GID found in the /etc/passwd and // /etc/group files. func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 { @@ -98,10 +102,10 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 { if u.Name == "nobody" { continue } - if u.Uid > size { + if u.Uid > size && u.Uid != nobodyUser { size = u.Uid } - if u.Gid > size { + if u.Gid > size && u.Gid != nobodyUser { size = u.Gid } } @@ -113,7 +117,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 { if g.Name == "nobody" { continue } - if g.Gid > size { + if g.Gid > size && g.Gid != nobodyUser { size = g.Gid } } diff --git a/vendor/modules.txt b/vendor/modules.txt index 5da2edb8..bf61205c 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -149,7 +149,7 @@ github.com/containers/ocicrypt/keywrap/pkcs7 github.com/containers/ocicrypt/spec github.com/containers/ocicrypt/utils github.com/containers/ocicrypt/utils/keyprovider -# github.com/containers/storage v1.45.1 +# github.com/containers/storage v1.45.3 ## explicit; go 1.17 github.com/containers/storage github.com/containers/storage/drivers