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: containers#15897 Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg · May 15, 2023 · d01ae43 · d01ae43
1 parent 493aac6
commit d01ae43
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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})