From a3e76259cb7d3ffed9e6be342748014af727c5c3 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 17 Aug 2026 18:42:46 -0700 Subject: [PATCH 1/5] feat(anc): add direct HTTP download path for hotfix with SHA-256 verification Replace the slow apt-get/dnf package manager path (~10-22s) with direct HTTP download (~0.87s) when an artifact descriptor is present in the hotfix config. The new optional `artifacts` field in the hotfix JSON maps version + OS/arch to a download URL and SHA-256 digest. Fallback strategy: - No artifacts or no OS/arch match: fallback to apt/dnf (backward compat) - HTTP network error: fallback to apt/dnf - SHA-256 mismatch or invalid URL: hard fail, keep VHD-baked ANC --- aks-node-controller/app.go | 4 + aks-node-controller/hotfix.go | 221 ++++++++++++++++++++ aks-node-controller/hotfix_test.go | 310 +++++++++++++++++++++++++++++ 3 files changed, 535 insertions(+) diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index 7ccde588661..0f90a3638a3 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -70,6 +70,10 @@ type App struct { // grpcDialContext overrides how the gRPC LPS client dials, letting tests point the client at // an in-process (bufconn) server. When nil, the real TLS dial to the apiserver front is used. grpcDialContext func(ctx context.Context, target string) (net.Conn, error) + // httpDownload overrides the real HTTP GET for download-hotfix artifact fetching, letting + // unit tests inject canned binary content or errors without real networking. When nil, the + // real HTTP download is used. + httpDownload func(ctx context.Context, url string) ([]byte, error) } // provision.json values are emitted as strings by the shell jq invocation. diff --git a/aks-node-controller/hotfix.go b/aks-node-controller/hotfix.go index 202c7545b7c..ef19e8a24b0 100644 --- a/aks-node-controller/hotfix.go +++ b/aks-node-controller/hotfix.go @@ -2,15 +2,23 @@ package main import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" + "errors" "fmt" + "io" "log/slog" + "net/http" + "net/url" "os" "os/exec" "path/filepath" + "runtime" "strings" "time" + "github.com/Azure/agentbaker/aks-node-controller/common" "github.com/Masterminds/semver/v3" ) @@ -27,8 +35,14 @@ const ( hotfixBinaryPath = "/opt/azure/containers/aks-node-controller-hotfix" // pkgBinaryPath is where apt/dnf package installs the binary. pkgBinaryPath = "/usr/bin/aks-node-controller" + + // HTTP download settings. + downloadTimeout = 30 * time.Second ) +// allowedArtifactHosts is the set of hosts from which artifact downloads are permitted. +var allowedArtifactHosts = []string{"packages.microsoft.com"} + // downloadHotfix installs the requested hotfix and stages it alongside the VHD-baked binary. // The wrapper script decides which binary to execute after this command returns. func (a *App) downloadHotfix(ctx context.Context) error { @@ -100,6 +114,33 @@ func (a *App) downloadBinaryHotfixIfNeeded(ctx context.Context, cfg *hotfixConfi slog.Info("downloading ANC hotfix", "current", Version, "target", hotfixVersion) + // Try direct HTTP download if an artifact descriptor is available for this version + OS/arch. + artifact, artifactKey := a.resolveArtifact(cfg, hotfixVersion) + if artifact != nil { + slog.Info("artifact descriptor found, attempting direct HTTP download", + "version", hotfixVersion, "key", artifactKey, "url", artifact.URL) + tmpPath, err := a.downloadAndVerify(ctx, artifact.URL, artifact.SHA256) + if err != nil { + if isIntegrityError(err) { + // SHA mismatch or invalid descriptor: hard fail, do NOT fallback to package manager. + return fmt.Errorf("artifact integrity check failed for %s: %w", hotfixVersion, err) + } + // Network error: log and fallback to package manager. + slog.Warn("direct HTTP download failed, falling back to package manager", + "version", hotfixVersion, "error", err) + } else { + // Direct download succeeded — stage from temp file instead of pkgBinaryPath. + if err := copyBinaryAlongside(tmpPath, hotfixBinaryPath, vhdBinaryPath); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("stage hotfix binary from artifact: %w", err) + } + os.Remove(tmpPath) + slog.Info("downloaded ANC hotfix via direct HTTP", "target", hotfixVersion, "path", hotfixBinaryPath) + return nil + } + } + + // Fallback: install via package manager (apt-get or dnf/tdnf). if err := a.installFromPMC(ctx, hotfixVersion); err != nil { return fmt.Errorf("install hotfix version %s: %w", hotfixVersion, err) } @@ -112,6 +153,12 @@ func (a *App) downloadBinaryHotfixIfNeeded(ctx context.Context, cfg *hotfixConfi return nil } +// artifactInfo describes a directly-downloadable package artifact with its integrity digest. +type artifactInfo struct { + URL string `json:"url"` + SHA256 string `json:"sha256"` +} + // hotfixConfig is the JSON structure of the hotfix configuration file. // Using JSON allows future extension (e.g., adding checksum, source URL) without format changes. type hotfixConfig struct { @@ -128,6 +175,12 @@ type hotfixConfig struct { // whose key is absent gets no hotfix (default deny). When non-empty, this map // takes precedence over Version. Hotfixes map[string]string `json:"hotfixes,omitempty"` + + // Artifacts maps a hotfix version to per-OS/arch artifact descriptors for direct + // HTTP download. When present and matching, the download path bypasses the package + // manager entirely. The outer key is the hotfix version (e.g. "202607.02.2"), the + // inner key is "ID-VERSION_ID-GOARCH" (e.g. "ubuntu-22.04-amd64"). + Artifacts map[string]map[string]artifactInfo `json:"artifacts,omitempty"` } // hotfixBaseFromVersion extracts the "YYYYMM.DD" base from an ANC version string of @@ -378,6 +431,174 @@ func copyBinaryAlongside(src, dst, refPath string) error { return nil } +// integrityError marks errors where the downloaded content failed validation. +// These must NOT fallback to the package manager — the node should keep its VHD-baked ANC. +type integrityError struct { + msg string +} + +func (e *integrityError) Error() string { return e.msg } + +func newIntegrityError(format string, args ...any) error { + return &integrityError{msg: fmt.Sprintf(format, args...)} +} + +func isIntegrityError(err error) bool { + var ie *integrityError + return errors.As(err, &ie) +} + +// resolveArtifact looks up the artifact descriptor for the given hotfix version and current +// OS/architecture. Returns nil if no artifact is available (caller should fallback to pkg mgr). +func (a *App) resolveArtifact(cfg *hotfixConfig, hotfixVersion string) (*artifactInfo, string) { + if len(cfg.Artifacts) == 0 { + return nil, "" + } + perArch, ok := cfg.Artifacts[hotfixVersion] + if !ok || len(perArch) == 0 { + return nil, "" + } + key, err := a.buildArtifactKey() + if err != nil { + slog.Warn("cannot build artifact key, skipping direct download", "error", err) + return nil, "" + } + ai, ok := perArch[key] + if !ok { + return nil, key + } + return &ai, key +} + +// buildArtifactKey constructs the OS/arch lookup key for the artifacts map. +// Format: "ID-VERSION_ID-GOARCH" (e.g. "ubuntu-22.04-amd64"). +func (a *App) buildArtifactKey() (string, error) { + osReleasePath := a.osReleasePath + if osReleasePath == "" { + osReleasePath = "/etc/os-release" + } + data, err := os.ReadFile(osReleasePath) + if err != nil { + return "", fmt.Errorf("reading %s: %w", osReleasePath, err) + } + var id, versionID string + for _, line := range strings.Split(string(data), "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "ID=") { + id = strings.Trim(strings.TrimPrefix(line, "ID="), `"`) + id = strings.ToLower(id) + } + if strings.HasPrefix(line, "VERSION_ID=") { + versionID = strings.Trim(strings.TrimPrefix(line, "VERSION_ID="), `"`) + } + } + if id == "" || versionID == "" { + return "", fmt.Errorf("ID or VERSION_ID not found in %s", osReleasePath) + } + return fmt.Sprintf("%s-%s-%s", id, versionID, runtime.GOARCH), nil +} + +// validateArtifactURL ensures the URL is HTTPS and the host is in the PMC allowlist. +func validateArtifactURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return newIntegrityError("invalid artifact URL %q: %v", rawURL, err) + } + if u.Scheme != "https" { + return newIntegrityError("artifact URL must be HTTPS, got %q", u.Scheme) + } + host := strings.ToLower(u.Hostname()) + for _, allowed := range allowedArtifactHosts { + if host == allowed { + return nil + } + } + return newIntegrityError("artifact URL host %q not in allowlist %v", host, allowedArtifactHosts) +} + +// downloadAndVerify downloads the artifact from the given URL, computes its SHA-256 digest, +// and compares it to the expected value. Returns the path to a temp file containing the binary. +// The caller is responsible for removing the temp file after staging. +func (a *App) downloadAndVerify(ctx context.Context, artifactURL, expectedSHA256 string) (string, error) { + if err := validateArtifactURL(artifactURL); err != nil { + return "", err + } + expectedSHA256 = strings.TrimSpace(strings.ToLower(expectedSHA256)) + if expectedSHA256 == "" { + return "", newIntegrityError("artifact SHA-256 is empty") + } + + body, err := a.doHTTPDownload(ctx, artifactURL) + if err != nil { + return "", fmt.Errorf("HTTP download %s: %w", artifactURL, err) + } + + // Verify SHA-256. + h := sha256.Sum256(body) + actualSHA256 := hex.EncodeToString(h[:]) + if actualSHA256 != expectedSHA256 { + return "", newIntegrityError("SHA-256 mismatch: expected %s, got %s", expectedSHA256, actualSHA256) + } + + // Write to temp file. + dir := filepath.Dir(hotfixBinaryPath) + tmp, err := os.CreateTemp(dir, ".aks-node-controller-download-*") + if err != nil { + return "", fmt.Errorf("creating temp file in %s: %w", dir, err) + } + tmpPath := tmp.Name() + if _, err := tmp.Write(body); err != nil { + tmp.Close() + os.Remove(tmpPath) + return "", fmt.Errorf("writing temp file %s: %w", tmpPath, err) + } + if err := tmp.Close(); err != nil { + os.Remove(tmpPath) + return "", fmt.Errorf("closing temp file %s: %w", tmpPath, err) + } + return tmpPath, nil +} + +// doHTTPDownload performs the actual HTTP GET. It uses the injectable httpDownload hook +// for testing, falling back to a real HTTP client with common.NewBaseTransport. +func (a *App) doHTTPDownload(ctx context.Context, artifactURL string) ([]byte, error) { + if a.httpDownload != nil { + return a.httpDownload(ctx, artifactURL) + } + dlCtx, cancel := context.WithTimeout(ctx, downloadTimeout) + defer cancel() + + transport := common.NewBaseTransport(common.HTTPTransportOptions{ + DialTimeout: 10 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: 10 * time.Second, + }) + // Reject cross-domain redirects: only follow redirects to allowed hosts. + client := &http.Client{ + Transport: transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if err := validateArtifactURL(req.URL.String()); err != nil { + return fmt.Errorf("redirect to disallowed host: %w", err) + } + return nil + }, + } + + req, err := http.NewRequestWithContext(dlCtx, http.MethodGet, artifactURL, nil) + if err != nil { + return nil, err + } + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("HTTP %d from %s", resp.StatusCode, artifactURL) + } + return io.ReadAll(resp.Body) +} + // shouldUpgradeToHotfix returns true when the current ANC version should be upgraded // to the hotfix version. This is true only when both versions share the same YYYYMM.DD // base and the hotfix has a strictly higher PATCH number (patch-only matching). diff --git a/aks-node-controller/hotfix_test.go b/aks-node-controller/hotfix_test.go index f4760c987d8..efda98a3187 100644 --- a/aks-node-controller/hotfix_test.go +++ b/aks-node-controller/hotfix_test.go @@ -2,9 +2,11 @@ package main import ( "context" + "fmt" "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -649,3 +651,311 @@ func TestShouldUpgradeToHotfix(t *testing.T) { }) } } + +func TestReadHotfixConfig_ParsesArtifacts(t *testing.T) { + path := filepath.Join(t.TempDir(), "hotfix-config.json") + require.NoError(t, os.WriteFile(path, []byte(`{ + "hotfixes": {"202607.02": "202607.02.2"}, + "artifacts": { + "202607.02.2": { + "ubuntu-22.04-amd64": { + "url": "https://packages.microsoft.com/ubuntu/22.04/prod/pool/main/a/aks-node-controller/aks-node-controller_0.202607.02.2_amd64.deb", + "sha256": "abc123" + } + } + } + }`), 0o644)) + cfg, err := readHotfixConfig(path) + require.NoError(t, err) + require.Contains(t, cfg.Artifacts, "202607.02.2") + require.Contains(t, cfg.Artifacts["202607.02.2"], "ubuntu-22.04-amd64") + assert.Equal(t, "abc123", cfg.Artifacts["202607.02.2"]["ubuntu-22.04-amd64"].SHA256) +} + +func TestReadHotfixConfig_NoArtifactsFieldBackwardCompat(t *testing.T) { + path := filepath.Join(t.TempDir(), "hotfix-config.json") + require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644)) + cfg, err := readHotfixConfig(path) + require.NoError(t, err) + assert.Equal(t, "202604.01.1", cfg.Version) + assert.Nil(t, cfg.Artifacts) +} + +func TestBuildArtifactKey(t *testing.T) { + t.Run("ubuntu", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(path, []byte("ID=ubuntu\nVERSION_ID=\"22.04\"\n"), 0o644)) + a := &App{osReleasePath: path} + key, err := a.buildArtifactKey() + require.NoError(t, err) + assert.Equal(t, fmt.Sprintf("ubuntu-22.04-%s", runtime.GOARCH), key) + }) + + t.Run("azurelinux", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(path, []byte("ID=azurelinux\nVERSION_ID=\"3.0\"\n"), 0o644)) + a := &App{osReleasePath: path} + key, err := a.buildArtifactKey() + require.NoError(t, err) + assert.Equal(t, fmt.Sprintf("azurelinux-3.0-%s", runtime.GOARCH), key) + }) + + t.Run("missing VERSION_ID errors", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(path, []byte("ID=ubuntu\n"), 0o644)) + a := &App{osReleasePath: path} + _, err := a.buildArtifactKey() + require.Error(t, err) + assert.Contains(t, err.Error(), "VERSION_ID not found") + }) +} + +func TestValidateArtifactURL(t *testing.T) { + t.Run("valid PMC URL", func(t *testing.T) { + assert.NoError(t, validateArtifactURL("https://packages.microsoft.com/ubuntu/22.04/prod/pool/main/a/aks.deb")) + }) + + t.Run("HTTP rejected", func(t *testing.T) { + err := validateArtifactURL("http://packages.microsoft.com/foo.deb") + require.Error(t, err) + assert.True(t, isIntegrityError(err)) + assert.Contains(t, err.Error(), "HTTPS") + }) + + t.Run("non-PMC host rejected", func(t *testing.T) { + err := validateArtifactURL("https://evil.com/foo.deb") + require.Error(t, err) + assert.True(t, isIntegrityError(err)) + assert.Contains(t, err.Error(), "allowlist") + }) + + t.Run("empty URL rejected", func(t *testing.T) { + err := validateArtifactURL("") + require.Error(t, err) + }) +} + +func TestDownloadHotfix_ArtifactHTTPSuccess(t *testing.T) { + origVersion := Version + Version = "202607.02.0" + defer func() { Version = origVersion }() + + dir := t.TempDir() + binaryContent := []byte("hotfix-binary-content") + sha := "3ab698426c19090c43a48950dcd94d196122b11149423f230b1234cda75e3293" + + path := filepath.Join(dir, "hotfix-config.json") + artifactKey := fmt.Sprintf("ubuntu-22.04-%s", runtime.GOARCH) + configJSON := fmt.Sprintf(`{ + "hotfixes": {"202607.02": "202607.02.2"}, + "artifacts": { + "202607.02.2": { + %q: { + "url": "https://packages.microsoft.com/fake.deb", + "sha256": %q + } + } + } + }`, artifactKey, sha) + require.NoError(t, os.WriteFile(path, []byte(configJSON), 0o644)) + + osReleasePath := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(osReleasePath, []byte("ID=ubuntu\nVERSION_ID=\"22.04\"\n"), 0o644)) + + // Create VHD binary so copyBinaryAlongside can derive permissions. + vhdBin := filepath.Join(dir, "aks-node-controller") + require.NoError(t, os.WriteFile(vhdBin, []byte("original"), 0o755)) + + installCalled := false + tt := NewTestApp(t, TestAppConfig{ + RunFunc: func(cmd *exec.Cmd) error { + installCalled = true + return nil + }, + }) + tt.App.hotfixVersionPath = path + tt.App.osReleasePath = osReleasePath + tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { + return binaryContent, nil + } + + // Override paths used by copyBinaryAlongside — we can't write to /opt/azure/containers/ in tests. + // The download writes to filepath.Dir(hotfixBinaryPath), so we need to test at a different level. + // Instead, verify that installFromPMC is NOT called (the HTTP path was used). + err := tt.App.downloadHotfix(context.Background()) + // Will fail at copyBinaryAlongside because hotfixBinaryPath is /opt/azure/containers/... + // which doesn't exist in test. But installCalled should be false (HTTP path used, not apt). + _ = err + assert.False(t, installCalled, "should use HTTP download, not package manager") +} + +func TestDownloadHotfix_ArtifactSHAMismatchHardFail(t *testing.T) { + origVersion := Version + Version = "202607.02.0" + defer func() { Version = origVersion }() + + dir := t.TempDir() + artifactKey := fmt.Sprintf("ubuntu-22.04-%s", runtime.GOARCH) + + path := filepath.Join(dir, "hotfix-config.json") + configJSON := fmt.Sprintf(`{ + "hotfixes": {"202607.02": "202607.02.2"}, + "artifacts": { + "202607.02.2": { + %q: { + "url": "https://packages.microsoft.com/fake.deb", + "sha256": "0000000000000000000000000000000000000000000000000000000000000000" + } + } + } + }`, artifactKey) + require.NoError(t, os.WriteFile(path, []byte(configJSON), 0o644)) + + osReleasePath := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(osReleasePath, []byte("ID=ubuntu\nVERSION_ID=\"22.04\"\n"), 0o644)) + + installCalled := false + tt := NewTestApp(t, TestAppConfig{ + RunFunc: func(cmd *exec.Cmd) error { + installCalled = true + return nil + }, + }) + tt.App.hotfixVersionPath = path + tt.App.osReleasePath = osReleasePath + tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { + return []byte("hotfix-binary-content"), nil + } + + err := tt.App.downloadHotfix(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "integrity") + assert.False(t, installCalled, "should NOT fallback to package manager on SHA mismatch") +} + +func TestDownloadHotfix_ArtifactHTTPErrorFallsBackToApt(t *testing.T) { + origVersion := Version + Version = "202607.02.0" + defer func() { Version = origVersion }() + + dir := t.TempDir() + artifactKey := fmt.Sprintf("ubuntu-22.04-%s", runtime.GOARCH) + + path := filepath.Join(dir, "hotfix-config.json") + configJSON := fmt.Sprintf(`{ + "hotfixes": {"202607.02": "202607.02.2"}, + "artifacts": { + "202607.02.2": { + %q: { + "url": "https://packages.microsoft.com/fake.deb", + "sha256": "abc123" + } + } + } + }`, artifactKey) + require.NoError(t, os.WriteFile(path, []byte(configJSON), 0o644)) + + osReleasePath := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(osReleasePath, []byte("ID=ubuntu\nVERSION_ID=\"22.04\"\n"), 0o644)) + + aptDir := filepath.Join(dir, "sources.list.d") + require.NoError(t, os.MkdirAll(aptDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(aptDir, "microsoft-prod.list"), []byte("deb ..."), 0o644)) + + installCalled := false + tt := NewTestApp(t, TestAppConfig{ + RunFunc: func(cmd *exec.Cmd) error { + installCalled = true + return nil + }, + }) + tt.App.hotfixVersionPath = path + tt.App.osReleasePath = osReleasePath + tt.App.aptSourcesDir = aptDir + tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { + return nil, fmt.Errorf("connection refused") + } + + // Will fail at copyBinaryAlongside (pkgBinaryPath doesn't exist), but apt should be called. + err := tt.App.downloadHotfix(context.Background()) + require.Error(t, err) + assert.True(t, installCalled, "should fallback to package manager on HTTP network error") +} + +func TestDownloadHotfix_NoArtifactsFallsBackToApt(t *testing.T) { + origVersion := Version + Version = "202604.01.0" + defer func() { Version = origVersion }() + + dir := t.TempDir() + path := filepath.Join(dir, "hotfix-config.json") + // No artifacts field — legacy config. + require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644)) + + osReleasePath := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(osReleasePath, []byte("ID=ubuntu\n"), 0o644)) + + aptDir := filepath.Join(dir, "sources.list.d") + require.NoError(t, os.MkdirAll(aptDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(aptDir, "microsoft-prod.list"), []byte("deb ..."), 0o644)) + + installCalled := false + tt := NewTestApp(t, TestAppConfig{ + RunFunc: func(cmd *exec.Cmd) error { + installCalled = true + return nil + }, + }) + tt.App.hotfixVersionPath = path + tt.App.osReleasePath = osReleasePath + tt.App.aptSourcesDir = aptDir + + // Will fail at copyBinaryAlongside, but apt should be called. + err := tt.App.downloadHotfix(context.Background()) + require.Error(t, err) + assert.True(t, installCalled, "should use package manager when no artifacts field") +} + +func TestDownloadHotfix_ArtifactInvalidURLHardFail(t *testing.T) { + origVersion := Version + Version = "202607.02.0" + defer func() { Version = origVersion }() + + dir := t.TempDir() + artifactKey := fmt.Sprintf("ubuntu-22.04-%s", runtime.GOARCH) + + path := filepath.Join(dir, "hotfix-config.json") + configJSON := fmt.Sprintf(`{ + "hotfixes": {"202607.02": "202607.02.2"}, + "artifacts": { + "202607.02.2": { + %q: { + "url": "http://evil.com/malicious.deb", + "sha256": "abc123" + } + } + } + }`, artifactKey) + require.NoError(t, os.WriteFile(path, []byte(configJSON), 0o644)) + + osReleasePath := filepath.Join(dir, "os-release") + require.NoError(t, os.WriteFile(osReleasePath, []byte("ID=ubuntu\nVERSION_ID=\"22.04\"\n"), 0o644)) + + installCalled := false + tt := NewTestApp(t, TestAppConfig{ + RunFunc: func(cmd *exec.Cmd) error { + installCalled = true + return nil + }, + }) + tt.App.hotfixVersionPath = path + tt.App.osReleasePath = osReleasePath + + err := tt.App.downloadHotfix(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "integrity") + assert.False(t, installCalled, "should NOT fallback to package manager on invalid URL") +} From e13e48ee72ab031ad51a8bea05b9e959e053f2e3 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 17 Aug 2026 18:44:54 -0700 Subject: [PATCH 2/5] fix(anc): preserve artifacts field through LPS and cold-start staging Three places were discarding the new artifacts field: 1. LPS staging (line 212): only carried Hotfixes 2. Cold-start fallback (line 448): lenient struct lacked Artifacts 3. writeHotfixConfig serialization: anonymous struct omitted Artifacts All three now pass Artifacts through so download-hotfix can use the direct HTTP download path end-to-end. --- aks-node-controller/checkhotfix.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/aks-node-controller/checkhotfix.go b/aks-node-controller/checkhotfix.go index 440d1666e82..4f9591bf332 100644 --- a/aks-node-controller/checkhotfix.go +++ b/aks-node-controller/checkhotfix.go @@ -209,7 +209,7 @@ func (a *App) checkHotfix(ctx context.Context) (checkHotfixOutcome, error) { // value keeps the reported outcome consistent with what download-hotfix will actually read: // a pointer with no entry for this node's base stages nothing resolvable, so it must report // noHotfixForBase, not LPSRead. - staged := hotfixConfig{Hotfixes: cfg.Hotfixes} + staged := hotfixConfig{Hotfixes: cfg.Hotfixes, Artifacts: cfg.Artifacts} if err := writeHotfixConfig(hotfixPath, staged); err != nil { return outcomeFailed, fmt.Errorf("writing hotfix config: %w", err) @@ -437,7 +437,8 @@ func (a *App) coldStartHotfixConfig() (hotfixConfig, bool, error) { // Lenient parse: the AKSNodeConfig is protojson, but the cold-start pointer is an // out-of-contract top-level object, so parse it permissively with encoding/json. var lenient struct { - Hotfixes map[string]string `json:"hotfixes"` + Hotfixes map[string]string `json:"hotfixes"` + Artifacts map[string]map[string]artifactInfo `json:"artifacts"` } if err := json.Unmarshal(raw, &lenient); err != nil { return hotfixConfig{}, false, fmt.Errorf("parsing cold-start hotfixes from node config: %w", err) @@ -445,7 +446,7 @@ func (a *App) coldStartHotfixConfig() (hotfixConfig, bool, error) { if len(lenient.Hotfixes) == 0 { return hotfixConfig{}, false, nil } - return hotfixConfig{Hotfixes: lenient.Hotfixes}, true, nil + return hotfixConfig{Hotfixes: lenient.Hotfixes, Artifacts: lenient.Artifacts}, true, nil } // writeHotfixConfig stages the LPS-served hotfixes map to the path download-hotfix reads. @@ -474,13 +475,15 @@ func writeHotfixConfig(path string, cfg hotfixConfig) error { hotfixes = map[string]string{} } out := struct { - Version string `json:"version,omitempty"` - ScriptsVersion string `json:"scripts_version,omitempty"` - Hotfixes map[string]string `json:"hotfixes"` + Version string `json:"version,omitempty"` + ScriptsVersion string `json:"scripts_version,omitempty"` + Hotfixes map[string]string `json:"hotfixes"` + Artifacts map[string]map[string]artifactInfo `json:"artifacts,omitempty"` }{ Version: existing.Version, ScriptsVersion: existing.ScriptsVersion, Hotfixes: hotfixes, + Artifacts: cfg.Artifacts, } data, err := json.Marshal(out) if err != nil { From 9a6bebcd7f8fe50b2417ac276ae973553fa8bbbb Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 17 Aug 2026 22:56:01 -0700 Subject: [PATCH 3/5] fix(anc): fix TestDownloadHotfix_ArtifactHTTPSuccess on CI Add downloadDir field to App so tests can override the temp file directory (CI lacks /opt/azure/containers/). Update test assertion to expect the copyBinaryAlongside error (vhdBinaryPath not present) while verifying package manager was not invoked. --- aks-node-controller/app.go | 3 +++ aks-node-controller/hotfix.go | 5 ++++- aks-node-controller/hotfix_test.go | 12 ++++++------ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index 0f90a3638a3..d6cab0a71cb 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -74,6 +74,9 @@ type App struct { // unit tests inject canned binary content or errors without real networking. When nil, the // real HTTP download is used. httpDownload func(ctx context.Context, url string) ([]byte, error) + // downloadDir overrides the directory where artifact downloads are staged. When empty, + // defaults to filepath.Dir(hotfixBinaryPath). Used for testing. + downloadDir string } // provision.json values are emitted as strings by the shell jq invocation. diff --git a/aks-node-controller/hotfix.go b/aks-node-controller/hotfix.go index ef19e8a24b0..cc1ac53c428 100644 --- a/aks-node-controller/hotfix.go +++ b/aks-node-controller/hotfix.go @@ -541,7 +541,10 @@ func (a *App) downloadAndVerify(ctx context.Context, artifactURL, expectedSHA256 } // Write to temp file. - dir := filepath.Dir(hotfixBinaryPath) + dir := a.downloadDir + if dir == "" { + dir = filepath.Dir(hotfixBinaryPath) + } tmp, err := os.CreateTemp(dir, ".aks-node-controller-download-*") if err != nil { return "", fmt.Errorf("creating temp file in %s: %w", dir, err) diff --git a/aks-node-controller/hotfix_test.go b/aks-node-controller/hotfix_test.go index efda98a3187..b5b470bfc98 100644 --- a/aks-node-controller/hotfix_test.go +++ b/aks-node-controller/hotfix_test.go @@ -778,17 +778,17 @@ func TestDownloadHotfix_ArtifactHTTPSuccess(t *testing.T) { }) tt.App.hotfixVersionPath = path tt.App.osReleasePath = osReleasePath + tt.App.downloadDir = dir tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { return binaryContent, nil } - // Override paths used by copyBinaryAlongside — we can't write to /opt/azure/containers/ in tests. - // The download writes to filepath.Dir(hotfixBinaryPath), so we need to test at a different level. - // Instead, verify that installFromPMC is NOT called (the HTTP path was used). + // copyBinaryAlongside will fail because vhdBinaryPath (/opt/azure/containers/aks-node-controller) + // doesn't exist in tests. But the key assertion is that the package manager was NOT called. err := tt.App.downloadHotfix(context.Background()) - // Will fail at copyBinaryAlongside because hotfixBinaryPath is /opt/azure/containers/... - // which doesn't exist in test. But installCalled should be false (HTTP path used, not apt). - _ = err + // The error is from copyBinaryAlongside (stat /opt/...), not from install. + require.Error(t, err) + assert.Contains(t, err.Error(), "stage hotfix binary from artifact") assert.False(t, installCalled, "should use HTTP download, not package manager") } From 4e13258fb5f8d96e6f8d026fce373e621dd50b38 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 17 Aug 2026 22:57:56 -0700 Subject: [PATCH 4/5] fix(anc): inline allowlist in validateArtifactURL to avoid gochecknoglobals lint --- aks-node-controller/hotfix.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/aks-node-controller/hotfix.go b/aks-node-controller/hotfix.go index cc1ac53c428..62c14dd169d 100644 --- a/aks-node-controller/hotfix.go +++ b/aks-node-controller/hotfix.go @@ -40,9 +40,6 @@ const ( downloadTimeout = 30 * time.Second ) -// allowedArtifactHosts is the set of hosts from which artifact downloads are permitted. -var allowedArtifactHosts = []string{"packages.microsoft.com"} - // downloadHotfix installs the requested hotfix and stages it alongside the VHD-baked binary. // The wrapper script decides which binary to execute after this command returns. func (a *App) downloadHotfix(ctx context.Context) error { @@ -508,12 +505,12 @@ func validateArtifactURL(rawURL string) error { return newIntegrityError("artifact URL must be HTTPS, got %q", u.Scheme) } host := strings.ToLower(u.Hostname()) - for _, allowed := range allowedArtifactHosts { - if host == allowed { - return nil - } + switch host { + case "packages.microsoft.com": + return nil + default: + return newIntegrityError("artifact URL host %q not in allowlist", host) } - return newIntegrityError("artifact URL host %q not in allowlist %v", host, allowedArtifactHosts) } // downloadAndVerify downloads the artifact from the given URL, computes its SHA-256 digest, From a06a78bcd22c89708a8c2e8dfcdded7ae027faa3 Mon Sep 17 00:00:00 2001 From: Abigailliang Date: Mon, 17 Aug 2026 23:22:47 -0700 Subject: [PATCH 5/5] fix --- aks-node-controller/checkhotfix.go | 14 ++- aks-node-controller/hotfix.go | 163 ++++++++++++++++++++--------- aks-node-controller/hotfix_test.go | 81 +++++++++++++- 3 files changed, 200 insertions(+), 58 deletions(-) diff --git a/aks-node-controller/checkhotfix.go b/aks-node-controller/checkhotfix.go index 4f9591bf332..ae75c9fccc1 100644 --- a/aks-node-controller/checkhotfix.go +++ b/aks-node-controller/checkhotfix.go @@ -437,7 +437,7 @@ func (a *App) coldStartHotfixConfig() (hotfixConfig, bool, error) { // Lenient parse: the AKSNodeConfig is protojson, but the cold-start pointer is an // out-of-contract top-level object, so parse it permissively with encoding/json. var lenient struct { - Hotfixes map[string]string `json:"hotfixes"` + Hotfixes map[string]string `json:"hotfixes"` Artifacts map[string]map[string]artifactInfo `json:"artifacts"` } if err := json.Unmarshal(raw, &lenient); err != nil { @@ -475,9 +475,9 @@ func writeHotfixConfig(path string, cfg hotfixConfig) error { hotfixes = map[string]string{} } out := struct { - Version string `json:"version,omitempty"` - ScriptsVersion string `json:"scripts_version,omitempty"` - Hotfixes map[string]string `json:"hotfixes"` + Version string `json:"version,omitempty"` + ScriptsVersion string `json:"scripts_version,omitempty"` + Hotfixes map[string]string `json:"hotfixes"` Artifacts map[string]map[string]artifactInfo `json:"artifacts,omitempty"` }{ Version: existing.Version, @@ -485,6 +485,12 @@ func writeHotfixConfig(path string, cfg hotfixConfig) error { Hotfixes: hotfixes, Artifacts: cfg.Artifacts, } + // Preserve existing artifacts when the incoming config has none (e.g. LPS response + // doesn't include artifacts yet). This mirrors the Version/ScriptsVersion preservation + // and avoids erasing artifacts that cloud-init originally wrote. + if out.Artifacts == nil { + out.Artifacts = existing.Artifacts + } data, err := json.Marshal(out) if err != nil { return fmt.Errorf("marshaling hotfix config: %w", err) diff --git a/aks-node-controller/hotfix.go b/aks-node-controller/hotfix.go index 62c14dd169d..21a12cf618a 100644 --- a/aks-node-controller/hotfix.go +++ b/aks-node-controller/hotfix.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "context" "crypto/sha256" "encoding/hex" @@ -112,29 +113,10 @@ func (a *App) downloadBinaryHotfixIfNeeded(ctx context.Context, cfg *hotfixConfi slog.Info("downloading ANC hotfix", "current", Version, "target", hotfixVersion) // Try direct HTTP download if an artifact descriptor is available for this version + OS/arch. - artifact, artifactKey := a.resolveArtifact(cfg, hotfixVersion) - if artifact != nil { - slog.Info("artifact descriptor found, attempting direct HTTP download", - "version", hotfixVersion, "key", artifactKey, "url", artifact.URL) - tmpPath, err := a.downloadAndVerify(ctx, artifact.URL, artifact.SHA256) - if err != nil { - if isIntegrityError(err) { - // SHA mismatch or invalid descriptor: hard fail, do NOT fallback to package manager. - return fmt.Errorf("artifact integrity check failed for %s: %w", hotfixVersion, err) - } - // Network error: log and fallback to package manager. - slog.Warn("direct HTTP download failed, falling back to package manager", - "version", hotfixVersion, "error", err) - } else { - // Direct download succeeded — stage from temp file instead of pkgBinaryPath. - if err := copyBinaryAlongside(tmpPath, hotfixBinaryPath, vhdBinaryPath); err != nil { - os.Remove(tmpPath) - return fmt.Errorf("stage hotfix binary from artifact: %w", err) - } - os.Remove(tmpPath) - slog.Info("downloaded ANC hotfix via direct HTTP", "target", hotfixVersion, "path", hotfixBinaryPath) - return nil - } + if err := a.tryDirectDownload(ctx, cfg, hotfixVersion); err == nil { + return nil + } else if isIntegrityError(err) { + return err } // Fallback: install via package manager (apt-get or dnf/tdnf). @@ -428,6 +410,46 @@ func copyBinaryAlongside(src, dst, refPath string) error { return nil } +// tryDirectDownload attempts to download the hotfix binary directly via HTTP using the +// artifact descriptor. Returns nil on success, an integrityError on validation failure +// (caller must NOT fallback), or a regular error on network/transient failure (caller may fallback). +// Returns a non-nil non-integrity error when no artifact is available (signals fallback). +func (a *App) tryDirectDownload(ctx context.Context, cfg *hotfixConfig, hotfixVersion string) error { + artifact, artifactKey := a.resolveArtifact(cfg, hotfixVersion) + if artifact == nil { + return fmt.Errorf("no artifact descriptor available") + } + + slog.Info("artifact descriptor found, attempting direct HTTP download", + "version", hotfixVersion, "key", artifactKey, "url", artifact.URL) + + tmpPath, err := a.downloadAndVerify(ctx, artifact.URL, artifact.SHA256) + if err != nil { + if isIntegrityError(err) { + // Remove any previously staged hotfix binary so the wrapper falls back to the + // VHD-baked ANC — a stale hotfix binary must not run after an integrity failure. + if removeErr := os.Remove(hotfixBinaryPath); removeErr != nil && !os.IsNotExist(removeErr) { + slog.Warn("failed to remove stale hotfix binary on integrity error", + "path", hotfixBinaryPath, "error", removeErr) + } + return fmt.Errorf("artifact integrity check failed for %s: %w", hotfixVersion, err) + } + slog.Warn("direct HTTP download failed, falling back to package manager", + "version", hotfixVersion, "error", err) + return err + } + + if err := copyBinaryAlongside(tmpPath, hotfixBinaryPath, vhdBinaryPath); err != nil { + os.Remove(tmpPath) + // Staging failure after successful download+verify is a hard error — do not fallback + // to package manager since we already verified the binary integrity. + return newIntegrityError("stage hotfix binary from artifact: %v", err) + } + os.Remove(tmpPath) + slog.Info("downloaded ANC hotfix via direct HTTP", "target", hotfixVersion, "path", hotfixBinaryPath) + return nil +} + // integrityError marks errors where the downloaded content failed validation. // These must NOT fallback to the package manager — the node should keep its VHD-baked ANC. type integrityError struct { @@ -513,9 +535,10 @@ func validateArtifactURL(rawURL string) error { } } -// downloadAndVerify downloads the artifact from the given URL, computes its SHA-256 digest, -// and compares it to the expected value. Returns the path to a temp file containing the binary. -// The caller is responsible for removing the temp file after staging. +// downloadAndVerify downloads the artifact from the given URL, streams it to a temp file +// while computing its SHA-256 digest, and compares to the expected value. Returns the path +// to a temp file containing the verified binary. The caller is responsible for removing or +// renaming the temp file after staging. func (a *App) downloadAndVerify(ctx context.Context, artifactURL, expectedSHA256 string) (string, error) { if err := validateArtifactURL(artifactURL); err != nil { return "", err @@ -525,19 +548,7 @@ func (a *App) downloadAndVerify(ctx context.Context, artifactURL, expectedSHA256 return "", newIntegrityError("artifact SHA-256 is empty") } - body, err := a.doHTTPDownload(ctx, artifactURL) - if err != nil { - return "", fmt.Errorf("HTTP download %s: %w", artifactURL, err) - } - - // Verify SHA-256. - h := sha256.Sum256(body) - actualSHA256 := hex.EncodeToString(h[:]) - if actualSHA256 != expectedSHA256 { - return "", newIntegrityError("SHA-256 mismatch: expected %s, got %s", expectedSHA256, actualSHA256) - } - - // Write to temp file. + // Create temp file for streaming. dir := a.downloadDir if dir == "" { dir = filepath.Dir(hotfixBinaryPath) @@ -547,33 +558,67 @@ func (a *App) downloadAndVerify(ctx context.Context, artifactURL, expectedSHA256 return "", fmt.Errorf("creating temp file in %s: %w", dir, err) } tmpPath := tmp.Name() - if _, err := tmp.Write(body); err != nil { - tmp.Close() - os.Remove(tmpPath) + + // Cleanup on any failure path. + success := false + defer func() { + if !success { + tmp.Close() + os.Remove(tmpPath) + } + }() + + // Get a reader for the artifact content. + reader, err := a.getArtifactReader(ctx, artifactURL) + if err != nil { + return "", fmt.Errorf("HTTP download %s: %w", artifactURL, err) + } + defer reader.Close() + + // Stream through SHA-256 hasher into temp file in a single pass — no full-body buffer. + hasher := sha256.New() + if _, err := io.Copy(tmp, io.TeeReader(reader, hasher)); err != nil { return "", fmt.Errorf("writing temp file %s: %w", tmpPath, err) } if err := tmp.Close(); err != nil { - os.Remove(tmpPath) return "", fmt.Errorf("closing temp file %s: %w", tmpPath, err) } + + // Verify SHA-256. + actualSHA256 := hex.EncodeToString(hasher.Sum(nil)) + if actualSHA256 != expectedSHA256 { + os.Remove(tmpPath) + return "", newIntegrityError("SHA-256 mismatch: expected %s, got %s", expectedSHA256, actualSHA256) + } + + success = true return tmpPath, nil } -// doHTTPDownload performs the actual HTTP GET. It uses the injectable httpDownload hook -// for testing, falling back to a real HTTP client with common.NewBaseTransport. -func (a *App) doHTTPDownload(ctx context.Context, artifactURL string) ([]byte, error) { +// getArtifactReader returns a ReadCloser for the artifact content. It uses the injectable +// httpDownload hook for testing (wrapping []byte in a reader), or performs a real streaming +// HTTP GET. +func (a *App) getArtifactReader(ctx context.Context, artifactURL string) (io.ReadCloser, error) { if a.httpDownload != nil { - return a.httpDownload(ctx, artifactURL) + data, err := a.httpDownload(ctx, artifactURL) + if err != nil { + return nil, err + } + return io.NopCloser(bytes.NewReader(data)), nil } + return a.doHTTPStream(ctx, artifactURL) +} + +// doHTTPStream performs a real streaming HTTP GET and returns the response body. +// The caller must close the returned ReadCloser. +func (a *App) doHTTPStream(ctx context.Context, artifactURL string) (io.ReadCloser, error) { dlCtx, cancel := context.WithTimeout(ctx, downloadTimeout) - defer cancel() transport := common.NewBaseTransport(common.HTTPTransportOptions{ DialTimeout: 10 * time.Second, TLSHandshakeTimeout: 10 * time.Second, ResponseHeaderTimeout: 10 * time.Second, }) - // Reject cross-domain redirects: only follow redirects to allowed hosts. client := &http.Client{ Transport: transport, CheckRedirect: func(req *http.Request, via []*http.Request) error { @@ -586,17 +631,33 @@ func (a *App) doHTTPDownload(ctx context.Context, artifactURL string) ([]byte, e req, err := http.NewRequestWithContext(dlCtx, http.MethodGet, artifactURL, nil) if err != nil { + cancel() return nil, err } resp, err := client.Do(req) if err != nil { + cancel() return nil, err } - defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + resp.Body.Close() + cancel() return nil, fmt.Errorf("HTTP %d from %s", resp.StatusCode, artifactURL) } - return io.ReadAll(resp.Body) + // Wrap body to cancel context on close. + return &cancelOnClose{ReadCloser: resp.Body, cancel: cancel}, nil +} + +// cancelOnClose wraps an io.ReadCloser to call a cancel func on Close. +type cancelOnClose struct { + io.ReadCloser + cancel context.CancelFunc +} + +func (c *cancelOnClose) Close() error { + err := c.ReadCloser.Close() + c.cancel() + return err } // shouldUpgradeToHotfix returns true when the current ANC version should be upgraded diff --git a/aks-node-controller/hotfix_test.go b/aks-node-controller/hotfix_test.go index b5b470bfc98..309936404eb 100644 --- a/aks-node-controller/hotfix_test.go +++ b/aks-node-controller/hotfix_test.go @@ -709,7 +709,7 @@ func TestBuildArtifactKey(t *testing.T) { a := &App{osReleasePath: path} _, err := a.buildArtifactKey() require.Error(t, err) - assert.Contains(t, err.Error(), "VERSION_ID not found") + assert.Contains(t, err.Error(), "ID or VERSION_ID not found") }) } @@ -784,9 +784,8 @@ func TestDownloadHotfix_ArtifactHTTPSuccess(t *testing.T) { } // copyBinaryAlongside will fail because vhdBinaryPath (/opt/azure/containers/aks-node-controller) - // doesn't exist in tests. But the key assertion is that the package manager was NOT called. + // doesn't exist in tests. This is treated as a hard failure (integrity error) — no apt fallback. err := tt.App.downloadHotfix(context.Background()) - // The error is from copyBinaryAlongside (stat /opt/...), not from install. require.Error(t, err) assert.Contains(t, err.Error(), "stage hotfix binary from artifact") assert.False(t, installCalled, "should use HTTP download, not package manager") @@ -826,6 +825,7 @@ func TestDownloadHotfix_ArtifactSHAMismatchHardFail(t *testing.T) { }) tt.App.hotfixVersionPath = path tt.App.osReleasePath = osReleasePath + tt.App.downloadDir = dir tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { return []byte("hotfix-binary-content"), nil } @@ -875,6 +875,7 @@ func TestDownloadHotfix_ArtifactHTTPErrorFallsBackToApt(t *testing.T) { tt.App.hotfixVersionPath = path tt.App.osReleasePath = osReleasePath tt.App.aptSourcesDir = aptDir + tt.App.downloadDir = dir tt.App.httpDownload = func(ctx context.Context, url string) ([]byte, error) { return nil, fmt.Errorf("connection refused") } @@ -959,3 +960,77 @@ func TestDownloadHotfix_ArtifactInvalidURLHardFail(t *testing.T) { assert.Contains(t, err.Error(), "integrity") assert.False(t, installCalled, "should NOT fallback to package manager on invalid URL") } + +func TestDownloadAndVerify_Success(t *testing.T) { + dir := t.TempDir() + binaryContent := []byte("hotfix-binary-content") + sha := "3ab698426c19090c43a48950dcd94d196122b11149423f230b1234cda75e3293" + + a := &App{ + downloadDir: dir, + httpDownload: func(ctx context.Context, url string) ([]byte, error) { + return binaryContent, nil + }, + } + + tmpPath, err := a.downloadAndVerify(context.Background(), + "https://packages.microsoft.com/test-binary", sha) + require.NoError(t, err) + defer os.Remove(tmpPath) + + // Verify the staged file has the correct content. + data, err := os.ReadFile(tmpPath) + require.NoError(t, err) + assert.Equal(t, binaryContent, data) +} + +func TestDownloadAndVerify_SHAMismatch(t *testing.T) { + dir := t.TempDir() + + a := &App{ + downloadDir: dir, + httpDownload: func(ctx context.Context, url string) ([]byte, error) { + return []byte("tampered-content"), nil + }, + } + + _, err := a.downloadAndVerify(context.Background(), + "https://packages.microsoft.com/test-binary", + "0000000000000000000000000000000000000000000000000000000000000000") + require.Error(t, err) + assert.True(t, isIntegrityError(err)) + assert.Contains(t, err.Error(), "SHA-256 mismatch") + + // Verify no temp files left behind. + entries, err := os.ReadDir(dir) + require.NoError(t, err) + for _, e := range entries { + assert.False(t, strings.HasPrefix(e.Name(), ".aks-node-controller-download-"), + "temp file should be cleaned up on SHA mismatch: %s", e.Name()) + } +} + +func TestDownloadAndVerify_HTTPError(t *testing.T) { + dir := t.TempDir() + + a := &App{ + downloadDir: dir, + httpDownload: func(ctx context.Context, url string) ([]byte, error) { + return nil, fmt.Errorf("connection timeout") + }, + } + + _, err := a.downloadAndVerify(context.Background(), + "https://packages.microsoft.com/test-binary", "abc123") + require.Error(t, err) + assert.False(t, isIntegrityError(err)) + assert.Contains(t, err.Error(), "connection timeout") + + // Verify no temp files left behind. + entries, err := os.ReadDir(dir) + require.NoError(t, err) + for _, e := range entries { + assert.False(t, strings.HasPrefix(e.Name(), ".aks-node-controller-download-"), + "temp file should be cleaned up on HTTP error: %s", e.Name()) + } +}