From d01ae436ee26535a302bee42a357b12f51d19fde Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 15 May 2023 14:58:21 +0200 Subject: [PATCH] remote-save: fix permissions and dir formats Make sure that the directory formats are not just substituted with their archive counterparts but actually tar'ed up directories. Also make sure that the clients don't get chown errors by setting rootless user and group ID instead of O when running in the user namespace. Fixes: #15897 Signed-off-by: Valentin Rothberg --- pkg/api/handlers/libpod/images.go | 38 ++++++++++++++++++++----------- pkg/domain/infra/tunnel/images.go | 5 ++-- test/e2e/save_test.go | 20 +++++++--------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index 412532954b..b1ef91a68d 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -23,9 +23,13 @@ import ( "github.com/containers/podman/v4/pkg/domain/infra/abi" domainUtils "github.com/containers/podman/v4/pkg/domain/utils" "github.com/containers/podman/v4/pkg/errorhandling" + "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/util" utils2 "github.com/containers/podman/v4/utils" "github.com/containers/storage" + "github.com/containers/storage/pkg/archive" + "github.com/containers/storage/pkg/chrootarchive" + "github.com/containers/storage/pkg/idtools" "github.com/gorilla/schema" ) @@ -266,16 +270,6 @@ func ExportImages(w http.ResponseWriter, r *http.Request) { return } - // if format is dir, server will save to an archive - // the client will unArchive after receive the archive file - // so must convert is at here - switch query.Format { - case define.OCIManifestDir: - query.Format = define.OCIArchive - case define.V2s2ManifestDir: - query.Format = define.V2s2Archive - } - switch query.Format { case define.V2s2Archive, define.OCIArchive: tmpfile, err := os.CreateTemp("", "api.tar") @@ -316,13 +310,31 @@ func ExportImages(w http.ResponseWriter, r *http.Request) { return } - rdr, err := os.Open(output) + // If we already produced a tar archive, let's stream that directly. + switch query.Format { + case define.V2s2Archive, define.OCIArchive: + rdr, err := os.Open(output) + if err != nil { + utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to read the exported tarfile: %w", err)) + return + } + defer rdr.Close() + utils.WriteResponse(w, http.StatusOK, rdr) + return + } + + tarOptions := &archive.TarOptions{ + ChownOpts: &idtools.IDPair{ + UID: rootless.GetRootlessUID(), + GID: rootless.GetRootlessGID(), + }, + } + tar, err := chrootarchive.Tar(output, tarOptions, output) if err != nil { utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to read the exported tarfile: %w", err)) return } - defer rdr.Close() - utils.WriteResponse(w, http.StatusOK, rdr) + utils.WriteResponse(w, http.StatusOK, tar) } func ImagesLoad(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index a05909b307..e2e6a24511 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -19,7 +19,7 @@ import ( "github.com/containers/podman/v4/pkg/domain/entities/reports" "github.com/containers/podman/v4/pkg/domain/utils" "github.com/containers/podman/v4/pkg/errorhandling" - utils2 "github.com/containers/podman/v4/utils" + "github.com/containers/storage/pkg/archive" ) func (ir *ImageEngine) Exists(_ context.Context, nameOrID string) (*entities.BoolReport, error) { @@ -329,7 +329,8 @@ func (ir *ImageEngine) Save(ctx context.Context, nameOrID string, tags []string, default: return err } - return utils2.UntarToFileSystem(opts.Output, f, nil) + + return archive.Untar(f, opts.Output, nil) } func (ir *ImageEngine) Search(ctx context.Context, term string, opts entities.ImageSearchOptions) ([]entities.ImageSearchReport, error) { diff --git a/test/e2e/save_test.go b/test/e2e/save_test.go index 56b1b5d014..d29cbf8f33 100644 --- a/test/e2e/save_test.go +++ b/test/e2e/save_test.go @@ -87,31 +87,30 @@ var _ = Describe("Podman save", func() { }) It("podman save to directory with oci format", func() { - if isRootless() { - Skip("Requires a fix in containers image for chown/lchown") - } outdir := filepath.Join(podmanTest.TempDir, "save") save := podmanTest.Podman([]string{"save", "--format", "oci-dir", "-o", outdir, ALPINE}) save.WaitWithDefaultTimeout() Expect(save).Should(Exit(0)) + + // Smoke test if it looks like an OCI dir + Expect(filepath.Join(outdir, "oci-layout")).Should(BeAnExistingFile()) + Expect(filepath.Join(outdir, "index.json")).Should(BeAnExistingFile()) + Expect(filepath.Join(outdir, "blobs")).Should(BeAnExistingFile()) }) It("podman save to directory with v2s2 docker format", func() { - if isRootless() { - Skip("Requires a fix in containers image for chown/lchown") - } outdir := filepath.Join(podmanTest.TempDir, "save") save := podmanTest.Podman([]string{"save", "--format", "docker-dir", "-o", outdir, ALPINE}) save.WaitWithDefaultTimeout() Expect(save).Should(Exit(0)) + + // Smoke test if it looks like a docker dir + Expect(filepath.Join(outdir, "version")).Should(BeAnExistingFile()) }) It("podman save to directory with docker format and compression", func() { - if isRootless() && podmanTest.RemoteTest { - Skip("Requires a fix in containers image for chown/lchown") - } outdir := filepath.Join(podmanTest.TempDir, "save") save := podmanTest.Podman([]string{"save", "--compress", "--format", "docker-dir", "-o", outdir, ALPINE}) @@ -120,9 +119,6 @@ var _ = Describe("Podman save", func() { }) It("podman save to directory with --compress but not use docker-dir and oci-dir", func() { - if isRootless() && podmanTest.RemoteTest { - Skip("Requires a fix in containers image for chown/lchown") - } outdir := filepath.Join(podmanTest.TempDir, "save") save := podmanTest.Podman([]string{"save", "--compress", "--format", "docker-archive", "-o", outdir, ALPINE})