Merge pull request #18563 from vrothberg/fix-15897

remote-save: fix permissions and dir formats
containers · May 15, 2023 · e0890ca · e0890ca
2 parents 2f6cb5e + d01ae43
commit e0890ca
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
@@ -66,31 +66,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})
@@ -99,9 +98,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})