remote: allow --http-proxy for remote clients

The remote client should be allowed to specify if the container should be run with the proxy env vars. It will still use the proxy vars from the server process and not the client. This makes podman-remote more consistent with the local version and easier to use in environments where a proxy is required. Fixes containers#16520 Signed-off-by: Paul Holzinger <[email protected]>
rhatdan · Dec 8, 2022 · 2dde30b · 2dde30b
1 parent 6e2e9ab
commit 2dde30b
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 19 deletions.
diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
@@ -621,7 +621,6 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 
 		if registry.IsRemote() {
 			_ = createFlags.MarkHidden("env-host")
-			_ = createFlags.MarkHidden("http-proxy")
 			_ = createFlags.MarkHidden(decryptionKeysFlagName)
 		} else {
 			createFlags.StringVar(

diff --git a/cmd/podman/images/build.go b/cmd/podman/images/build.go
@@ -168,16 +168,7 @@ func buildFlags(cmd *cobra.Command) {
 		logrus.Errorf("Setting up build flags: %v", err)
 		os.Exit(1)
 	}
-	// --http-proxy flag
-	// containers.conf defaults to true but we want to force false by default for remote, since settings do not apply
-	if registry.IsRemote() {
-		flag = fromAndBudFlags.Lookup("http-proxy")
-		buildOpts.HTTPProxy = false
-		if err := flag.Value.Set("false"); err != nil {
-			logrus.Errorf("Unable to set --https-proxy to %v: %v", false, err)
-		}
-		flag.DefValue = "false"
-	}
+
 	flags.AddFlagSet(&fromAndBudFlags)
 	// Add the completion functions
 	fromAndBudFlagsCompletions := buildahCLI.GetFromAndBudFlagsCompletions()

diff --git a/docs/source/markdown/options/http-proxy.md b/docs/source/markdown/options/http-proxy.md
@@ -1,5 +1,5 @@
 ####> This option file is used in:
-####>   podman create, run
+####>   podman build, create, run
 ####> If file is edited, make sure the changes
 ####> are applicable to all of those.
 #### **--http-proxy**
@@ -14,6 +14,7 @@ for the container in any other way will override the values that would have
 been passed through from the host. (Other ways to specify the proxy for the
 container include passing the values with the **--env** flag, or hard coding the
 proxy environment at container build time.)
-(This option is not available with the remote Podman client, including Mac and Windows (excluding WSL2) machines)
+When used with the remote client it will use the proxy environment variables
+that are set on the server process.
 
 Defaults to **true**.
diff --git a/docs/source/markdown/podman-build.1.md.in b/docs/source/markdown/podman-build.1.md.in
@@ -303,9 +303,7 @@ For the bind-mount conditions, only mounts explicitly requested by the caller vi
 
 If --hooks-dir is unset for root callers, Buildah will currently default to /usr/share/containers/oci/hooks.d and /etc/containers/oci/hooks.d in order of increasing precedence. Using these defaults is deprecated, and callers should migrate to explicitly setting --hooks-dir.
 
-#### **--http-proxy**
-
-Pass through HTTP Proxy environment variables.
+@@option http-proxy
 
 #### **--identity-label**
 

diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go
@@ -299,7 +299,7 @@ var _ = Describe("Podman build", func() {
 		Expect(session.OutputToString()).To(ContainSubstring("hello"))
 	})
 
-	It("podman build --http_proxy flag", func() {
+	It("podman build http proxy test", func() {
 		if env, found := os.LookupEnv("http_proxy"); found {
 			defer os.Setenv("http_proxy", env)
 		} else {
@@ -309,6 +309,9 @@ var _ = Describe("Podman build", func() {
 		if IsRemote() {
 			podmanTest.StopRemoteService()
 			podmanTest.StartRemoteService()
+			// set proxy env again so it will only effect the client
+			// the remote client should still use the proxy that was set for the server
+			os.Setenv("http_proxy", "127.0.0.2")
 		}
 		podmanTest.AddImageToRWStore(ALPINE)
 		dockerfile := fmt.Sprintf(`FROM %s
@@ -317,10 +320,17 @@ RUN printenv http_proxy`, ALPINE)
 		dockerfilePath := filepath.Join(podmanTest.TempDir, "Dockerfile")
 		err := os.WriteFile(dockerfilePath, []byte(dockerfile), 0755)
 		Expect(err).ToNot(HaveOccurred())
-		session := podmanTest.Podman([]string{"build", "--pull-never", "--http-proxy", "--file", dockerfilePath, podmanTest.TempDir})
+		// --http-proxy should be true by default so we do not set it
+		session := podmanTest.Podman([]string{"build", "--pull-never", "--file", dockerfilePath, podmanTest.TempDir})
 		session.Wait(120)
 		Expect(session).Should(Exit(0))
 		Expect(session.OutputToString()).To(ContainSubstring("1.2.3.4"))
+
+		// this tries to use the cache so we explicitly disable it
+		session = podmanTest.Podman([]string{"build", "--no-cache", "--pull-never", "--http-proxy=false", "--file", dockerfilePath, podmanTest.TempDir})
+		session.Wait(120)
+		Expect(session).Should(Exit(1))
+		Expect(session.ErrorToString()).To(ContainSubstring(`Error: building at STEP "RUN printenv http_proxy"`))
 	})
 
 	It("podman build relay exit code to process", func() {

diff --git a/test/e2e/run_env_test.go b/test/e2e/run_env_test.go
@@ -130,6 +130,9 @@ ENV hello=world
 		if IsRemote() {
 			podmanTest.StopRemoteService()
 			podmanTest.StartRemoteService()
+			// set proxy env again so it will only effect the client
+			// the remote client should still use the proxy that was set for the server
+			os.Setenv("http_proxy", "127.0.0.2")
 		}
 		session := podmanTest.Podman([]string{"run", "--rm", ALPINE, "printenv", "http_proxy"})
 		session.WaitWithDefaultTimeout()

diff --git a/test/system/070-build.bats b/test/system/070-build.bats
@@ -20,7 +20,7 @@ RUN echo $rand_content > /$rand_filename
 EOF
 
     # The 'apk' command can take a long time to fetch files; bump timeout
-    PODMAN_TIMEOUT=240 run_podman build -t build_test --format=docker --http-proxy $tmpdir
+    PODMAN_TIMEOUT=240 run_podman build -t build_test --format=docker $tmpdir
     is "$output" ".*COMMIT" "COMMIT seen in log"
 
     run_podman run --rm build_test cat /$rand_filename