refactor: export proxy configuration directly - #9387
refactor: export proxy configuration directly#9387Martin Heberling (Bickor) wants to merge 4 commits into
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Windows Unit Test Results 3 files 14 suites 52s ⏱️ Results for commit b618956. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
🟡 Changes recommended
The proxy compatibility regression and ineffective ordering assertions must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors Linux provisioning to export proxy variables directly instead of generating PROXY_VARS.
Changes:
- Exports uppercase and lowercase proxy variables directly.
- Removes legacy
PROXY_VARSgeneration. - Updates proxy documentation and test coverage.
Blocking findings remain: removing PROXY_VARS breaks compatibility when the ANC script overlay fails on older VHDs, and the ordering test can pass when no proxy export is found.
File summaries
| File | Description |
|---|---|
spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh |
Adds proxy export and ordering tests; ordering assertions need strengthening. |
pkg/agent/variables.go |
Removes proxy command generation. |
pkg/agent/baker.go |
Removes the proxy template helper. |
parts/linux/cloud-init/artifacts/cse_main.sh |
Exports proxy variables directly; introduces an older-VHD compatibility risk. |
parts/linux/cloud-init/artifacts/cse_cmd.sh |
Stops rendering PROXY_VARS. |
aks-node-controller/proto/README.md |
Updates proxy output documentation. |
aks-node-controller/parser/parser.go |
Removes PROXY_VARS from controller environments. |
aks-node-controller/parser/helper.go |
Removes proxy command generation. |
aks-node-controller/app.go |
Removes obsolete comparison commentary. |
aks-node-controller/app_test.go |
Generalizes quoted-value parsing coverage. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Scope note: this PR is being narrowed to the three reported legacy AgentBaker locations only: The scriptless path under The narrowing follow-up is staged locally and will update the PR diff after the signed commit is pushed. |
There was a problem hiding this comment.
🟡 Changes recommended
Removing PROXY_VARS breaks compatibility with supported older VHDs, and the stated proxy regression coverage is missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
PROXY_VARS differs between provisioning paths, causing false migration parity mismatches.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
|
|
||
| // 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; ` + |
There was a problem hiding this comment.
why not use the values from nbc.HTTPProxyConfig?
is the assumption that HTTP_PROXY_URLS is available and exported already here ?
one return uses nbc.HTTPProxyConfig while the other return uses ENV VARs
There was a problem hiding this comment.
Yes, the assumption is that HTTP_PROXY_URLS, HTTPS_PROXY_URLS, and NO_PROXY_URLS are available when this payload is evaluated. cse_cmd.sh assigns them in the contiguous command prefix before the nohup invocation, so Bash places them in that command's environment and they are inherited by provision_start.sh and cse_main.sh; they do not need a separate export before nohup.
Their values still originate from nbc.HTTPProxyConfig, but they are shell-quoted at that initial assignment boundary. We intentionally do not insert those values directly into PROXY_VARS, because older VHDs execute eval $PROXY_VARS; embedding customer-controlled values in the evaluated string would recreate the command-injection issue. The nbc.HTTPProxyConfig check in getProxyVariables only determines whether the backward-compatibility payload is needed, while the fixed payload reads the safely assigned runtime environment variables.
Changes cached containers or packages on windows VHDsPlease get a Windows SIG member to approve. The following dif file shows any additions or deletions from what will be cached on windows VHDs organised by VHD type.
diff --git a/vhd_files/2022-containerd-gen2.txt b/vhd_files/2022-containerd-gen2.txt
index 4e05767..17b79ef 100644
--- a/vhd_files/2022-containerd-gen2.txt
+++ b/vhd_files/2022-containerd-gen2.txt
@@ -18,6 +17,0 @@ c:\akse-cache\csi-proxy\: https://packages.aks.azure.com/csi-proxy/v1.1.2-hotfix
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.102-akslts/windowszip/v1.28.102-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.103-akslts/windowszip/v1.28.103-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.100-akslts/windowszip/v1.29.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.101-akslts/windowszip/v1.29.101-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.100-akslts/windowszip/v1.30.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.101-akslts/windowszip/v1.30.101-akslts-1int.zip
@@ -138 +131,0 @@ mcr.microsoft.com/windows/nanoserver:ltsc2022
-mcr.microsoft.com/windows/servercore:10.0.20348.5386
@@ -139,0 +133 @@ mcr.microsoft.com/windows/servercore:10.0.20348.5499
+mcr.microsoft.com/windows/servercore:10.0.20348.5622
@@ -145 +139 @@ Windows 2022-containerd-gen2 base image sku: 2022-datacenter-core-smalldisk-g2
-Windows 2022-containerd-gen2 base version: 20348.5499.260809
+Windows 2022-containerd-gen2 base version: 20348.5622.260906
diff --git a/vhd_files/2022-containerd.txt b/vhd_files/2022-containerd.txt
index a8c76d7..786c54d 100644
--- a/vhd_files/2022-containerd.txt
+++ b/vhd_files/2022-containerd.txt
@@ -18,6 +17,0 @@ c:\akse-cache\csi-proxy\: https://packages.aks.azure.com/csi-proxy/v1.1.2-hotfix
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.102-akslts/windowszip/v1.28.102-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.103-akslts/windowszip/v1.28.103-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.100-akslts/windowszip/v1.29.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.101-akslts/windowszip/v1.29.101-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.100-akslts/windowszip/v1.30.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.101-akslts/windowszip/v1.30.101-akslts-1int.zip
@@ -138 +131,0 @@ mcr.microsoft.com/windows/nanoserver:ltsc2022
-mcr.microsoft.com/windows/servercore:10.0.20348.5386
@@ -139,0 +133 @@ mcr.microsoft.com/windows/servercore:10.0.20348.5499
+mcr.microsoft.com/windows/servercore:10.0.20348.5622
@@ -145 +139 @@ Windows 2022-containerd base image sku: 2022-Datacenter-Core-smalldisk
-Windows 2022-containerd base version: 20348.5499.260809
+Windows 2022-containerd base version: 20348.5622.260906
diff --git a/vhd_files/2025-gen2-tl.txt b/vhd_files/2025-gen2-tl.txt
index e49ede0..18b19fc 100644
--- a/vhd_files/2025-gen2-tl.txt
+++ b/vhd_files/2025-gen2-tl.txt
@@ -16,6 +15,0 @@ c:\akse-cache\wcn\: mcr.microsoft.com/wcn/package:1.7.1-cpu-arch
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.102-akslts/windowszip/v1.28.102-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.103-akslts/windowszip/v1.28.103-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.100-akslts/windowszip/v1.29.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.101-akslts/windowszip/v1.29.101-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.100-akslts/windowszip/v1.30.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.101-akslts/windowszip/v1.30.101-akslts-1int.zip
@@ -70 +63,0 @@ mcr.microsoft.com/windows/nanoserver:ltsc2025
-mcr.microsoft.com/windows/servercore:10.0.20348.5386
@@ -72 +65 @@ mcr.microsoft.com/windows/servercore:10.0.20348.5499
-mcr.microsoft.com/windows/servercore:10.0.26100.33158
+mcr.microsoft.com/windows/servercore:10.0.20348.5622
@@ -73,0 +67 @@ mcr.microsoft.com/windows/servercore:10.0.26100.33296
+mcr.microsoft.com/windows/servercore:10.0.26100.33438
@@ -80 +74 @@ Windows 2025-gen2-tl base image sku: 2025-datacenter-core-smalldisk-g2
-Windows 2025-gen2-tl base version: 26100.33296.260809
+Windows 2025-gen2-tl base version: 26100.33438.260905
diff --git a/vhd_files/2025-gen2.txt b/vhd_files/2025-gen2.txt
index eec1d89..86b88ab 100644
--- a/vhd_files/2025-gen2.txt
+++ b/vhd_files/2025-gen2.txt
@@ -16,6 +15,0 @@ c:\akse-cache\wcn\: mcr.microsoft.com/wcn/package:1.7.1-cpu-arch
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.102-akslts/windowszip/v1.28.102-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.103-akslts/windowszip/v1.28.103-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.100-akslts/windowszip/v1.29.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.101-akslts/windowszip/v1.29.101-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.100-akslts/windowszip/v1.30.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.101-akslts/windowszip/v1.30.101-akslts-1int.zip
@@ -70 +63,0 @@ mcr.microsoft.com/windows/nanoserver:ltsc2025
-mcr.microsoft.com/windows/servercore:10.0.20348.5386
@@ -72 +65 @@ mcr.microsoft.com/windows/servercore:10.0.20348.5499
-mcr.microsoft.com/windows/servercore:10.0.26100.33158
+mcr.microsoft.com/windows/servercore:10.0.20348.5622
@@ -73,0 +67 @@ mcr.microsoft.com/windows/servercore:10.0.26100.33296
+mcr.microsoft.com/windows/servercore:10.0.26100.33438
@@ -80 +74 @@ Windows 2025-gen2 base image sku: 2025-datacenter-core-smalldisk-g2
-Windows 2025-gen2 base version: 26100.33296.260809
+Windows 2025-gen2 base version: 26100.33438.260905
diff --git a/vhd_files/2025.txt b/vhd_files/2025.txt
index 5997647..3eaa402 100644
--- a/vhd_files/2025.txt
+++ b/vhd_files/2025.txt
@@ -16,6 +15,0 @@ c:\akse-cache\wcn\: mcr.microsoft.com/wcn/package:1.7.1-cpu-arch
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.102-akslts/windowszip/v1.28.102-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.28.103-akslts/windowszip/v1.28.103-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.100-akslts/windowszip/v1.29.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.29.101-akslts/windowszip/v1.29.101-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.100-akslts/windowszip/v1.30.100-akslts-1int.zip
-c:\akse-cache\win-k8s\: https://packages.aks.azure.com/kubernetes/v1.30.101-akslts/windowszip/v1.30.101-akslts-1int.zip
@@ -70 +63,0 @@ mcr.microsoft.com/windows/nanoserver:ltsc2025
-mcr.microsoft.com/windows/servercore:10.0.20348.5386
@@ -72 +65 @@ mcr.microsoft.com/windows/servercore:10.0.20348.5499
-mcr.microsoft.com/windows/servercore:10.0.26100.33158
+mcr.microsoft.com/windows/servercore:10.0.20348.5622
@@ -73,0 +67 @@ mcr.microsoft.com/windows/servercore:10.0.26100.33296
+mcr.microsoft.com/windows/servercore:10.0.26100.33438
@@ -80 +74 @@ Windows 2025 base image sku: 2025-datacenter-core-smalldisk
-Windows 2025 base version: 26100.33296.260809
+Windows 2025 base version: 26100.33438.260905 |
Failed gate run
Detective summaryUbuntu2404Gen2 was the only failed scenario after 123 passed and 70 skipped. The first failing operation was pod exec for Likely causeSignature: Recommended owner/actionNode lifecycle/E2E owner: continue tracking under repair #39614265; no direct PR action unless this reproduces deterministically on rerun or local validation. Strongest alternativePR-caused proxy compatibility regression is the strongest alternative because this PR touches proxy export paths; it is less likely because the failing command reaches Kubernetes pod exec and fails on localhost:9443 to kubelet 10250 proxying, not during CSE proxy configuration. Evidence
Wiki signature |
Summary
PROXY_VARSas fixed trusted compatibility code for older baked VHD scripts, without embedding customer-controlled values in evaluated shell codecse_main.shand remove itseval $PROXY_VARScallsaks-node-controller/parserimplementation unchanged for follow-up workTesting
go test ./...cd aks-node-controller && go test ./parser$(), backticks, globs, and backslashes; values round-trip without executing marker commandseval $PROXY_VARSbehavior exercised using the generated compatibility payloadbash -n parts/linux/cloud-init/artifacts/cse_main.shgit diff --check