From 493179be456c880c55ca6434200a0847e337fb49 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 21 Mar 2024 12:49:42 +0100 Subject: [PATCH] fix remote build isolation when server runs as root I am really not sure why the caller even should have the option to set this. We should always use the correct isolation type based on the privileges the server runs under never the client. podman-remote build seems to send the default based on its local privs which was wrong as well. To fix this I also changed the client to send the default if the isolation flag is not set. Fixes #22109 Signed-off-by: Paul Holzinger --- cmd/podman/common/build.go | 11 ++++++++--- pkg/api/handlers/compat/images_build.go | 15 ++++++++++++--- pkg/machine/e2e/basic_test.go | 10 ++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cmd/podman/common/build.go b/cmd/podman/common/build.go index 9fd1a4f7c0..25be5154e8 100644 --- a/cmd/podman/common/build.go +++ b/cmd/podman/common/build.go @@ -400,9 +400,14 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *Buil compression = buildahDefine.Uncompressed } - isolation, err := parse.IsolationOption(flags.Isolation) - if err != nil { - return nil, err + isolation := buildahDefine.IsolationDefault + // Only parse the isolation when it is actually needed as we do not want to send a wrong default + // to the server in the remote case (root vs rootless). + if flags.Isolation != "" { + isolation, err = parse.IsolationOption(flags.Isolation) + if err != nil { + return nil, err + } } usernsOption, idmappingOptions, err := parse.IDMappingOptions(c, isolation) diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 7d2cc37a9b..df6b3518bf 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -383,10 +383,19 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { return } - // make sure to force rootless as rootless otherwise buildah runs code which is intended to be run only as root. - if isolation == buildah.IsolationOCI && rootless.IsRootless() { - isolation = buildah.IsolationOCIRootless + // Make sure to force rootless as rootless otherwise buildah runs code which is intended to be run only as root. + // Same the other way around: https://github.com/containers/podman/issues/22109 + switch isolation { + case buildah.IsolationOCI: + if rootless.IsRootless() { + isolation = buildah.IsolationOCIRootless + } + case buildah.IsolationOCIRootless: + if !rootless.IsRootless() { + isolation = buildah.IsolationOCI + } } + registry = "" format = query.OutputFormat } else { diff --git a/pkg/machine/e2e/basic_test.go b/pkg/machine/e2e/basic_test.go index 7daea646f5..3429f78e41 100644 --- a/pkg/machine/e2e/basic_test.go +++ b/pkg/machine/e2e/basic_test.go @@ -54,6 +54,16 @@ var _ = Describe("run basic podman commands", func() { Expect(runAlp).To(Exit(0)) Expect(runAlp.outputToString()).To(ContainSubstring("Alpine Linux")) + contextDir := GinkgoT().TempDir() + cfile := filepath.Join(contextDir, "Containerfile") + err = os.WriteFile(cfile, []byte("FROM quay.io/libpod/alpine_nginx\nRUN ip addr\n"), 0o644) + Expect(err).ToNot(HaveOccurred()) + + build, err := mb.setCmd(bm.withPodmanCommand([]string{"build", contextDir})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(build).To(Exit(0)) + Expect(build.outputToString()).To(ContainSubstring("COMMIT")) + rmCon, err := mb.setCmd(bm.withPodmanCommand([]string{"rm", "-a"})).run() Expect(err).ToNot(HaveOccurred()) Expect(rmCon).To(Exit(0))