refactor: export proxy configuration directly

parts/linux/cloud-init/artifacts/cse_cmd.sh

@@ -131,10 +131,10 @@ CSE_CONFIG_FILEPATH="{{GetCSEConfigScriptFilepath}}"
 AZURE_PRIVATE_REGISTRY_SERVER="{{GetPrivateAzureRegistryServer}}"
 HAS_CUSTOM_SEARCH_DOMAIN="{{HasCustomSearchDomain}}"
 CUSTOM_SEARCH_DOMAIN_FILEPATH="{{GetCustomSearchDomainsCSEScriptFilepath}}"
-HTTP_PROXY_URLS="{{GetHTTPProxy}}"
-HTTPS_PROXY_URLS="{{GetHTTPSProxy}}"
-NO_PROXY_URLS="{{GetNoProxy}}"
-PROXY_VARS="{{GetProxyVariables}}"
+HTTP_PROXY_URLS={{GetVariable "httpProxyShellQuoted"}}
+HTTPS_PROXY_URLS={{GetVariable "httpsProxyShellQuoted"}}
+NO_PROXY_URLS={{GetVariable "noProxyShellQuoted"}}
+PROXY_VARS='{{GetProxyVariables}}'
 ENABLE_SECURE_TLS_BOOTSTRAPPING="{{EnableSecureTLSBootstrapping}}"
 SECURE_TLS_BOOTSTRAPPING_AAD_RESOURCE="{{GetSecureTLSBootstrappingAADResource}}"
 SECURE_TLS_BOOTSTRAPPING_USER_ASSIGNED_IDENTITY_ID="{{GetSecureTLSBootstrappingUserAssignedIdentityID}}"

parts/linux/cloud-init/artifacts/cse_main.sh

@@ -46,6 +46,20 @@ source "${CSE_INSTALL_FILEPATH}"
 source "${CSE_DISTRO_INSTALL_FILEPATH}"
 source "${CSE_CONFIG_FILEPATH}"
 
+# configureEtcEnvironment persists these values, but the current CSE process needs them immediately.
+if [ -n "${HTTP_PROXY_URLS}" ]; then
+    export HTTP_PROXY="${HTTP_PROXY_URLS}"
+    export http_proxy="${HTTP_PROXY_URLS}"
+fi
+if [ -n "${HTTPS_PROXY_URLS}" ]; then
+    export HTTPS_PROXY="${HTTPS_PROXY_URLS}"
+    export https_proxy="${HTTPS_PROXY_URLS}"
+fi
+if [ -n "${NO_PROXY_URLS}" ]; then
+    export NO_PROXY="${NO_PROXY_URLS}"
+    export no_proxy="${NO_PROXY_URLS}"
+fi
+
 # Disable a single kernel module with a known LPE vulnerability.
 # Writes a modprobe blacklist rule and unloads the module if loaded.
 # Safe to run repeatedly during VHD build or provisioning; idempotent (overwrites with same content if already present).
@@ -180,13 +194,6 @@ function basePrep {
         systemctl restart systemd-timesyncd
     fi
 
-    # Eval proxy vars to ensure curl commands use proxy if configured.
-    # e.g. PROXY_VARS=`export HTTPS_PROXY="https://proxy.example.com:8080"; export http_proxy="http://proxy.example.com:8080"; export NO_PROXY="127.0.0.1,localhost";`
-    # Setting vars in etc environment (configureEtcEnvironment) won't take effect in current shell session.
-    if [ -n "${PROXY_VARS}" ]; then
-        eval $PROXY_VARS
-    fi
-
     resolve_packages_source_url
     logs_to_events "AKS.CSE.setPackagesBaseURL" "echo $PACKAGE_DOWNLOAD_BASE_URL"
 
@@ -446,9 +453,6 @@ function nodePrep {
     fi
 
     if [ -n "${OUTBOUND_COMMAND}" ]; then
-        if [ -n "${PROXY_VARS}" ]; then
-            eval $PROXY_VARS
-        fi
         retrycmd_if_failure 20 1 15 $OUTBOUND_COMMAND >> /var/log/azure/cluster-provision-cse-output.log 2>&1 || exit $ERR_OUTBOUND_CONN_FAIL;
     fi
     if [ -n "${BOOTSTRAP_PROFILE_CONTAINER_REGISTRY_SERVER}" ]; then

pkg/agent/baker_test.go

@@ -9,6 +9,9 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -1172,6 +1175,59 @@ var _ = Describe("getLinuxNodeCSECommand", func() {
 		Expect(cseCmd).To(ContainSubstring("bash"))
 	})
 
+	It("should safely preserve proxy values for older VHD scripts in scriptless mode", func() {
+		tempDir, err := os.MkdirTemp("", "agentbaker-proxy-test")
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(tempDir)
+
+		httpMarker := filepath.Join(tempDir, "http-injected")
+		httpsMarker := filepath.Join(tempDir, "https-injected")
+		noProxyMarker := filepath.Join(tempDir, "no-proxy-injected")
+		httpProxy := `http://user:p'ass"word/$(touch ` + httpMarker + ");`touch " + httpMarker + "`/*?[x]\\value"
+		httpsProxy := `https://proxy.example/$(touch ` + httpsMarker + ")"
+		noProxyValues := []string{"localhost", `$(touch ` + noProxyMarker + ")", ".svc"}
+		baseConfig.HTTPProxyConfig = &datamodel.HTTPProxyConfig{
+			HTTPProxy:  &httpProxy,
+			HTTPSProxy: &httpsProxy,
+			NoProxy:    &noProxyValues,
+		}
+
+		var encodedNBCCmd string
+		for _, file := range templateGenerator.getScriptlessConfiguration(baseConfig) {
+			if file.path == aksNbcCmdFilepath {
+				encodedNBCCmd = file.content
+				break
+			}
+		}
+		Expect(encodedNBCCmd).NotTo(BeEmpty())
+		compressedNBCCmd, err := base64.StdEncoding.DecodeString(encodedNBCCmd)
+		Expect(err).NotTo(HaveOccurred())
+		cseCmdBytes, err := getGzipDecodedValue(compressedNBCCmd)
+		Expect(err).NotTo(HaveOccurred())
+		cseCmd := string(cseCmdBytes)
+		start := strings.Index(cseCmd, "HTTP_PROXY_URLS=")
+		Expect(start).To(BeNumerically(">=", 0))
+		end := strings.Index(cseCmd[start:], " ENABLE_SECURE_TLS_BOOTSTRAPPING=")
+		Expect(end).To(BeNumerically(">", 0))
+		proxyAssignments := cseCmd[start : start+end]
+		command := proxyAssignments + ` /bin/bash -c 'eval $PROXY_VARS; printf "%s\n" "$HTTP_PROXY" "$http_proxy" "$HTTPS_PROXY" "$https_proxy" "$NO_PROXY" "$no_proxy"'`
+
+		output, err := exec.Command("/bin/bash", "-c", command).CombinedOutput()
+		Expect(err).NotTo(HaveOccurred(), string(output))
+		Expect(strings.Split(strings.TrimSuffix(string(output), "\n"), "\n")).To(Equal([]string{
+			httpProxy,
+			httpProxy,
+			httpsProxy,
+			httpsProxy,
+			strings.Join(noProxyValues, ","),
+			strings.Join(noProxyValues, ","),
+		}))
+		Expect(httpMarker).NotTo(BeAnExistingFile())
+		Expect(httpsMarker).NotTo(BeAnExistingFile())
+		Expect(noProxyMarker).NotTo(BeAnExistingFile())
+		Expect(getProxyVariables(baseConfig)).NotTo(ContainSubstring(tempDir))
+	})
+
 	It("should embed cloud-init status checks when custom data is enabled", func() {
 		Expect(baseConfig.DisableCustomData).To(BeFalse())
 

pkg/agent/variables.go

@@ -4,7 +4,6 @@
 package agent
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -87,6 +86,18 @@ func getWindowsCustomDataVariables(config *datamodel.NodeBootstrappingConfigurat
 func getCSECommandVariables(config *datamodel.NodeBootstrappingConfiguration) paramsMap {
 	cs := config.ContainerService
 	profile := config.AgentPoolProfile
+	httpProxy, httpsProxy, noProxy := "", "", ""
+	if config.HTTPProxyConfig != nil {
+		if config.HTTPProxyConfig.HTTPProxy != nil {
+			httpProxy = *config.HTTPProxyConfig.HTTPProxy
+		}
+		if config.HTTPProxyConfig.HTTPSProxy != nil {
+			httpsProxy = *config.HTTPProxyConfig.HTTPSProxy
+		}
+		if config.HTTPProxyConfig.NoProxy != nil {
+			noProxy = strings.Join(*config.HTTPProxyConfig.NoProxy, ",")
+		}
+	}
 
 	// this method is called for both windows and linux. If there's no windows profile, then let's just
 	// use a blank one.
@@ -145,6 +156,9 @@ func getCSECommandVariables(config *datamodel.NodeBootstrappingConfiguration) pa
 		"serviceAccountImagePullDefaultClientID": getServiceAccountImagePullDefaultClientID(cs),
 		"serviceAccountImagePullDefaultTenantID": getServiceAccountImagePullDefaultTenantID(cs),
 		"identityBindingsLocalAuthoritySNI":      getServiceAccountImagePullLocalAuthoritySNI(cs),
+		"httpProxyShellQuoted":                   shellQuote(httpProxy),
+		"httpsProxyShellQuoted":                  shellQuote(httpsProxy),
+		"noProxyShellQuoted":                     shellQuote(noProxy),
 	}
 }
 
@@ -230,20 +244,19 @@ func getOutBoundCmd(nbc *datamodel.NodeBootstrappingConfiguration, cloudSpecConf
 	return connectivityCheckCommand
 }
 
+func shellQuote(value string) string {
+	return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'"
+}
+
 func getProxyVariables(nbc *datamodel.NodeBootstrappingConfiguration) string {
-	// only use https proxy, if user doesn't specify httpsProxy we autofill it with value from httpProxy.
-	proxyVars := ""
-	if nbc.HTTPProxyConfig != nil {
-		if nbc.HTTPProxyConfig.HTTPProxy != nil {
-			// from https://curl.se/docs/manual.html, curl uses http_proxy but uppercase for others?
-			proxyVars = fmt.Sprintf("export http_proxy=\"%s\";", *nbc.HTTPProxyConfig.HTTPProxy)
-		}
-		if nbc.HTTPProxyConfig.HTTPSProxy != nil {
-			proxyVars = fmt.Sprintf("export HTTPS_PROXY=\"%s\"; %s", *nbc.HTTPProxyConfig.HTTPSProxy, proxyVars)
-		}
-		if nbc.HTTPProxyConfig.NoProxy != nil {
-			proxyVars = fmt.Sprintf("export NO_PROXY=\"%s\"; %s", strings.Join(*nbc.HTTPProxyConfig.NoProxy, ","), proxyVars)
-		}
+	if nbc.HTTPProxyConfig == nil ||
+		(nbc.HTTPProxyConfig.HTTPProxy == nil && nbc.HTTPProxyConfig.HTTPSProxy == nil && nbc.HTTPProxyConfig.NoProxy == nil) {
+		return ""
 	}
-	return proxyVars
+
+	// Older VHDs evaluate PROXY_VARS. Keep this payload free of customer-controlled values;
+	// those values are shell-quoted separately and referenced only through variables here.
+	return `if [ -n "${HTTP_PROXY_URLS}" ]; then export HTTP_PROXY="${HTTP_PROXY_URLS}" http_proxy="${HTTP_PROXY_URLS}"; fi; ` +
+		`if [ -n "${HTTPS_PROXY_URLS}" ]; then export HTTPS_PROXY="${HTTPS_PROXY_URLS}" https_proxy="${HTTPS_PROXY_URLS}"; fi; ` +
+		`if [ -n "${NO_PROXY_URLS}" ]; then export NO_PROXY="${NO_PROXY_URLS}" no_proxy="${NO_PROXY_URLS}"; fi`
 }

spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh

@@ -127,7 +127,118 @@ Describe 'select_localdns_corefile()'
     End
 End
 
+Describe 'proxy environment exports'
+    setup() {
+        unset HTTP_PROXY http_proxy HTTPS_PROXY https_proxy NO_PROXY no_proxy
+        HTTP_PROXY_URLS=""
+        HTTPS_PROXY_URLS=""
+        NO_PROXY_URLS=""
+        proxy_exports="$(sed -n '/^# configureEtcEnvironment persists/,/^# Disable a single kernel module/p' parts/linux/cloud-init/artifacts/cse_main.sh)"
+    }
+
+    cleanup() {
+        unset HTTP_PROXY http_proxy HTTPS_PROXY https_proxy NO_PROXY no_proxy
+        unset HTTP_PROXY_URLS HTTPS_PROXY_URLS NO_PROXY_URLS
+    }
+
+    exported_proxy_environment() {
+        eval "${proxy_exports}"
+        /bin/bash -c 'printf "%s\n" "${HTTP_PROXY-<unset>}" "${http_proxy-<unset>}" "${HTTPS_PROXY-<unset>}" "${https_proxy-<unset>}" "${NO_PROXY-<unset>}" "${no_proxy-<unset>}"'
+    }
+
+    proxy_environment_matches() {
+        actual="$(exported_proxy_environment)"
+        expected="$(printf '%s\n' "$@")"
+        [ "${actual}" = "${expected}" ]
+    }
+
+    BeforeEach 'setup'
+    AfterEach 'cleanup'
+
+    It 'exports HTTP proxy values only'
+        HTTP_PROXY_URLS="http://proxy.example.com:8080"
+
+        When call proxy_environment_matches \
+            "http://proxy.example.com:8080" "http://proxy.example.com:8080" \
+            "<unset>" "<unset>" "<unset>" "<unset>"
+        The status should be success
+    End
+
+    It 'exports HTTPS proxy values only'
+        HTTPS_PROXY_URLS="https://proxy.example.com:8443"
+
+        When call proxy_environment_matches \
+            "<unset>" "<unset>" \
+            "https://proxy.example.com:8443" "https://proxy.example.com:8443" \
+            "<unset>" "<unset>"
+        The status should be success
+    End
+
+    It 'exports no-proxy values only'
+        NO_PROXY_URLS="127.0.0.1,localhost,.svc"
+
+        When call proxy_environment_matches \
+            "<unset>" "<unset>" "<unset>" "<unset>" \
+            "127.0.0.1,localhost,.svc" "127.0.0.1,localhost,.svc"
+        The status should be success
+    End
+
+    It 'exports all proxy values simultaneously'
+        HTTP_PROXY_URLS="http://proxy.example.com:8080"
+        HTTPS_PROXY_URLS="https://proxy.example.com:8443"
+        NO_PROXY_URLS="127.0.0.1,localhost,.svc"
+
+        When call proxy_environment_matches \
+            "http://proxy.example.com:8080" "http://proxy.example.com:8080" \
+            "https://proxy.example.com:8443" "https://proxy.example.com:8443" \
+            "127.0.0.1,localhost,.svc" "127.0.0.1,localhost,.svc"
+        The status should be success
+    End
+
+    It 'leaves existing values unchanged when proxy URLs are empty'
+        export HTTP_PROXY="existing-http-upper"
+        export http_proxy="existing-http-lower"
+        export HTTPS_PROXY="existing-https-upper"
+        export https_proxy="existing-https-lower"
+        export NO_PROXY="existing-no-proxy-upper"
+        export no_proxy="existing-no-proxy-lower"
+
+        When call proxy_environment_matches \
+            "existing-http-upper" "existing-http-lower" \
+            "existing-https-upper" "existing-https-lower" \
+            "existing-no-proxy-upper" "existing-no-proxy-lower"
+        The status should be success
+    End
+End
+
 Describe 'connectivity preflight timeouts'
+    It 'exports proxy values before package resolution and the outbound check'
+        proxy_consumer_order() {
+            awk '
+                /export HTTP_PROXY=/ && !proxy_exports { proxy_exports = NR }
+                /^[[:space:]]*resolve_packages_source_url$/ { package_resolution = NR }
+                /retrycmd_if_failure 20 1 15 \$OUTBOUND_COMMAND/ { outbound_check = NR }
+                END {
+                    if (proxy_exports > 0 && package_resolution > 0 && outbound_check > 0 &&
+                        proxy_exports < package_resolution && proxy_exports < outbound_check) {
+                        print "true"
+                    } else {
+                        print "false"
+                    }
+                }
+            ' parts/linux/cloud-init/artifacts/cse_main.sh
+        }
+
+        When call proxy_consumer_order
+        The output should equal "true"
+    End
+
+    It 'does not evaluate PROXY_VARS'
+        When run grep -F 'eval $PROXY_VARS' parts/linux/cloud-init/artifacts/cse_main.sh
+        The status should be failure
+        The output should equal ""
+    End
+
     It 'allows DNS failover during the outbound check'
         When run awk '/retrycmd_if_failure [0-9]+ [0-9]+ [0-9]+ \$OUTBOUND_COMMAND/ { print $2, $3, $4 }' parts/linux/cloud-init/artifacts/cse_main.sh
         The output should eq "20 1 15"