From ae31c04afdadc3e52fdee62eb48874401e7b421a Mon Sep 17 00:00:00 2001 From: Devin Wong Date: Tue, 20 Oct 2026 15:49:13 -0700 Subject: [PATCH 1/4] fix: preserve custom-cloud eligibility in hotfix payloads Render and select AKS custom-cloud payload variants from live provisioning inputs. Avoid reinjecting canonical scriptless entries that already exist. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b42051a-f79c-4dbf-8460-a4772d54e01f --- aks-node-controller/README.md | 19 ++- aks-node-controller/app.go | 2 +- aks-node-controller/embeddednodecustomdata.go | 48 +++++++- .../embeddednodecustomdata_test.go | 116 +++++++++++++++++- hotfix/hotfix_generate.py | 29 +++-- hotfix/hotfix_generate_test.py | 111 ++++++++++++++++- hotfix/render-nodecustomdata/main.go | 50 +++++--- 7 files changed, 338 insertions(+), 37 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index df2e1d5c8de..9671d9e72c1 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -250,10 +250,21 @@ updates to the ANC binary itself. The generated files live under `rendered_nodecustomdata_.yml`. The generator selects only changed hotfixable entries from `nodecustomdata.yml`, then renders only Ubuntu and standard Azure Linux variants through AgentBaker's production Go-template -functions. Azure Linux retains the `mariner` payload filename; the legacy +functions. When custom-cloud rendering differs, each platform also has a +`rendered_nodecustomdata__akscustom.yml` variant that preserves +`IsAKSCustomCloud` eligibility. ANC follows provisioning's input precedence: +when an NBC command exists, it reads `IS_CUSTOM_CLOUD` without executing the +command; otherwise it reads the live node config's +`custom_cloud_config.custom_cloud_env_name` and compares it to `akscustom` +(case-insensitive, matching the production renderer). Malformed or unavailable eligibility +inputs return an error before any payload is written; the caller logs the error. +Existing packages without cloud variants retain distro-only application. +Azure Linux retains the `mariner` payload filename; the legacy `ID=mariner` OS is no longer supported. OS Guard, ACL, and Flatcar are explicitly skipped during embedded application, including variants that share the `azurelinux` OS ID. Their distro-specific source changes do not trigger payload generation. +For cloud-init delivery, generation does not inject a second copy of a canonical +block already present in the scriptless branch (such as `init-aks-cloud.sh`). The repository keeps a README placeholder so `go:embed` builds without any script hotfix payloads. Generation replaces it with Ubuntu and Azure Linux YAMLs. @@ -266,9 +277,9 @@ is retired explicitly. The existing applier writes entries sequentially and creates missing destination files and parent directories. There is no transactional rollback: if an entry -fails, earlier writes remain. Generation selects by distro only; hotfix authors -must separately account for non-distro template conditions such as custom-image -exclusions. +fails, earlier writes remain. Generation selects by distro and AKS custom-cloud +eligibility; hotfix authors must separately account for other template conditions +such as custom-image exclusions. `cse_start.sh` (`provision_start.sh` on the node) is excluded from embedded hotfixes to preserve custom-image wrappers. Generation fails explicitly if this diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index fbbdd616f1c..a69bee0f91e 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -271,7 +271,7 @@ func (a *App) runApplyHotfixCommand(context.Context) error { applyHotfix := a.applyEmbeddedHotfix if applyHotfix == nil { applyHotfix = func(osReleasePath string) error { - return applyEmbeddedNodeCustomData(embeddedGeneratedNodeCustomData, osReleasePath, embeddedNodeCustomDataPath) + return applyEmbeddedNodeCustomData(embeddedGeneratedNodeCustomData, osReleasePath, embeddedNodeCustomDataPath, a.isEmbeddedHotfixCustomCloud) } } if err := applyHotfix(a.osReleasePath); err != nil { diff --git a/aks-node-controller/embeddednodecustomdata.go b/aks-node-controller/embeddednodecustomdata.go index 67615d54fcb..934314de4a7 100644 --- a/aks-node-controller/embeddednodecustomdata.go +++ b/aks-node-controller/embeddednodecustomdata.go @@ -9,6 +9,9 @@ import ( "os" "path/filepath" "strings" + + "github.com/Azure/agentbaker/aks-node-controller/helpers" + "github.com/Azure/agentbaker/aks-node-controller/pkg/nodeconfigutils" ) const defaultOSReleasePath = "/etc/os-release" @@ -34,7 +37,7 @@ const ( //go:embed generated var embeddedGeneratedNodeCustomData embed.FS -func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath string) error { +func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath string, isCustomCloud func() (bool, error)) error { if osReleasePath == "" { osReleasePath = defaultOSReleasePath } @@ -47,6 +50,18 @@ func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath stri return nil } renderedPath := fmt.Sprintf("generated/rendered_nodecustomdata_%s.yml", platform) + customCloudPath := strings.TrimSuffix(renderedPath, ".yml") + "_akscustom.yml" + if _, err := fs.Stat(payloadFS, customCloudPath); err == nil { + customCloud, err := isCustomCloud() + if err != nil { + return fmt.Errorf("resolve embedded hotfix cloud eligibility: %w", err) + } + if customCloud { + renderedPath = customCloudPath + } + } else if !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("stat embedded nodecustomdata %s: %w", customCloudPath, err) + } data, err := fs.ReadFile(payloadFS, renderedPath) if errors.Is(err, fs.ErrNotExist) { return nil @@ -67,6 +82,37 @@ func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath stri return nil } +func (a *App) isEmbeddedHotfixCustomCloud() (bool, error) { + // Match Provision's precedence when both live configuration forms are present. + commandPath := a.getNBCCmdPath() + data, err := os.ReadFile(commandPath) + if err == nil { + value := parseEnvVarsFromNBCCmdContent(string(data))["IS_CUSTOM_CLOUD"] + switch value { + case "true": + return true, nil + case "false": + return false, nil + default: + return false, fmt.Errorf("NBC command %s has missing or invalid IS_CUSTOM_CLOUD", commandPath) + } + } + if !errors.Is(err, fs.ErrNotExist) { + return false, fmt.Errorf("read NBC command %s: %w", commandPath, err) + } + + configPath := a.getNodeConfigPath() + data, err = os.ReadFile(configPath) + if err != nil { + return false, fmt.Errorf("read node config %s: %w", configPath, err) + } + config, err := nodeconfigutils.UnmarshalConfigurationV1(data) + if err != nil { + return false, fmt.Errorf("decode node config %s: %w", configPath, err) + } + return strings.EqualFold(config.GetCustomCloudConfig().GetCustomCloudEnvName(), helpers.AksCustomCloudName), nil +} + func classifyNodeCustomDataPlatform(osReleasePath string) (nodeCustomDataPlatform, error) { data, err := os.ReadFile(osReleasePath) if err != nil { diff --git a/aks-node-controller/embeddednodecustomdata_test.go b/aks-node-controller/embeddednodecustomdata_test.go index c57aaae51a4..c5bab3436f7 100644 --- a/aks-node-controller/embeddednodecustomdata_test.go +++ b/aks-node-controller/embeddednodecustomdata_test.go @@ -2,6 +2,7 @@ package main import ( "encoding/base64" + "errors" "io/fs" "os" "path/filepath" @@ -14,6 +15,113 @@ import ( "gopkg.in/yaml.v3" ) +func TestEmbeddedHotfixCloudEligibility(t *testing.T) { + tests := []struct { + name, config, command, wantError string + wantCustom bool + }{ + {name: "custom", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, wantCustom: true}, + {name: "case insensitive", config: `{"customCloudConfig":{"customCloudEnvName":"AKSCustom"}}`, wantCustom: true}, + {name: "public", config: `{}`}, + {name: "other cloud", config: `{"custom_cloud_config":{"custom_cloud_env_name":"AzureChinaCloud"}}`}, + {name: "command authoritative", config: `{}`, command: `IS_CUSTOM_CLOUD=true`, wantCustom: true}, + {name: "command excludes custom config", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `IS_CUSTOM_CLOUD=false`}, + {name: "command ignores invalid config", config: `{`, command: `IS_CUSTOM_CLOUD=true`, wantCustom: true}, + {name: "invalid config", config: `{`, wantError: "decode node config"}, + {name: "invalid command does not fall back", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `echo test`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, + {name: "legacy custom", command: `IS_CUSTOM_CLOUD="true" /bin/bash provision.sh`, wantCustom: true}, + {name: "legacy public", command: `IS_CUSTOM_CLOUD=false /bin/bash provision.sh`}, + {name: "missing marker", command: `echo IS_CUSTOM_CLOUD`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, + {name: "invalid marker", command: `IS_CUSTOM_CLOUD=TRUE`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, + {name: "missing inputs", wantError: "read node config"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + directory := t.TempDir() + app := &App{ + nodeConfigPath: filepath.Join(directory, "config.json"), + nbcCmdPath: filepath.Join(directory, "nbc-cmd.sh"), + } + if test.config != "" { + require.NoError(t, os.WriteFile(app.nodeConfigPath, []byte(test.config), 0o600)) + } + if test.command != "" { + require.NoError(t, os.WriteFile(app.nbcCmdPath, []byte(test.command), 0o600)) + } + actual, err := app.isEmbeddedHotfixCustomCloud() + if test.wantError != "" { + require.ErrorContains(t, err, test.wantError) + } else { + require.NoError(t, err) + assert.Equal(t, test.wantCustom, actual) + } + }) + } +} + +func TestApplyEmbeddedNodeCustomDataSelectsCloudVariant(t *testing.T) { + for _, platform := range []string{"ubuntu", "mariner"} { + for _, cloud := range []string{"public", "custom", "unknown"} { + t.Run(platform+"/"+cloud, func(t *testing.T) { + directory := t.TempDir() + script := filepath.Join(directory, "init-aks-cloud.sh") + common := filepath.Join(directory, "provision_source_distro.sh") + require.NoError(t, os.WriteFile(script, []byte("original"), 0o600)) + publicFiles := []nodeCustomDataWriteFile{{Path: common, Content: platform}} + customFiles := append(append([]nodeCustomDataWriteFile{}, publicFiles...), nodeCustomDataWriteFile{ + Path: script, Encoding: encodingBase64, Content: base64.StdEncoding.EncodeToString([]byte("hotfix")), + }) + publicYAML, err := yaml.Marshal(nodeCustomData{WriteFiles: publicFiles}) + require.NoError(t, err) + customYAML, err := yaml.Marshal(nodeCustomData{WriteFiles: customFiles}) + require.NoError(t, err) + name := "generated/rendered_nodecustomdata_" + platform + files := fstest.MapFS{ + name + ".yml": &fstest.MapFile{Data: publicYAML}, + name + "_akscustom.yml": &fstest.MapFile{Data: customYAML}, + } + releasePath := filepath.Join(directory, "os-release") + id := platform + if platform == "mariner" { + id = "azurelinux" + } + require.NoError(t, os.WriteFile(releasePath, []byte("ID="+id), 0o600)) + outputPath := filepath.Join(directory, "embedded.yml") + err = applyEmbeddedNodeCustomData(files, releasePath, outputPath, func() (bool, error) { + if cloud == "unknown" { + return false, errors.New("unavailable config") + } + return cloud == "custom", nil + }) + if cloud == "unknown" { + require.ErrorContains(t, err, "resolve embedded hotfix cloud eligibility") + assert.NoFileExists(t, common) + assert.NoFileExists(t, outputPath) + } else { + require.NoError(t, err) + actual, err := os.ReadFile(common) + require.NoError(t, err) + assert.Equal(t, platform, string(actual)) + retained, err := os.ReadFile(outputPath) + require.NoError(t, err) + if cloud == "custom" { + assert.Equal(t, customYAML, retained) + } else { + assert.Equal(t, publicYAML, retained) + } + } + actual, err := os.ReadFile(script) + require.NoError(t, err) + if cloud == "custom" { + assert.Equal(t, "hotfix", string(actual)) + } else { + assert.Equal(t, "original", string(actual), "ineligible nodes must not overwrite the script") + } + }) + } + } +} + func TestClassifyNodeCustomDataPlatform(t *testing.T) { tests := []struct { name string @@ -37,7 +145,7 @@ func TestClassifyNodeCustomDataPlatform(t *testing.T) { assert.Equal(t, test.expected, actual) if actual == nodeCustomDataPlatformUnsupported { outputPath := filepath.Join(t.TempDir(), "embedded-nodecustomdata.yml") - require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath), + require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath, nil), "unsupported platforms must skip without reading any payload") assert.NoFileExists(t, outputPath) } @@ -72,7 +180,7 @@ func TestApplyEmbeddedNodeCustomData(t *testing.T) { files := fstest.MapFS{ "generated/rendered_nodecustomdata_" + platform + ".yml": &fstest.MapFile{Data: data}, } - require.NoError(t, applyEmbeddedNodeCustomData(files, releasePath, outputPath)) + require.NoError(t, applyEmbeddedNodeCustomData(files, releasePath, outputPath, nil)) for _, destination := range []string{existing, missing} { actual, readErr := os.ReadFile(destination) require.NoError(t, readErr) @@ -84,7 +192,7 @@ func TestApplyEmbeddedNodeCustomData(t *testing.T) { legacy, err := os.ReadFile(legacyPath) require.NoError(t, err) assert.Equal(t, "legacy payload", string(legacy)) - require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath)) + require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath, nil)) retained, err = os.ReadFile(outputPath) require.NoError(t, err) assert.Equal(t, data, retained, "a skipped application must preserve the last payload") @@ -142,7 +250,7 @@ func TestApplyEmbeddedNodeCustomDataErrorsAndRetention(t *testing.T) { if test.missing == "payload-file" { files["generated/rendered_nodecustomdata_ubuntu.yml"].Mode = fs.ModeDir } - err := applyEmbeddedNodeCustomData(files, releasePath, outputPath) + err := applyEmbeddedNodeCustomData(files, releasePath, outputPath, nil) if test.wantError == "" { require.NoError(t, err) } else { diff --git a/hotfix/hotfix_generate.py b/hotfix/hotfix_generate.py index 2623604bac3..8ec95d89cc3 100644 --- a/hotfix/hotfix_generate.py +++ b/hotfix/hotfix_generate.py @@ -13,8 +13,9 @@ (the release tag the VHD was built from, derived from linux_sig_version.json), selects their write_files entries from parts/linux/cloud-init/nodecustomdata.yml, injects those entries into the scriptless section of nodecustomdata.yml, and - renders self-contained ANC payloads for Ubuntu and Mariner with AgentBaker's - canonical Go-template renderer. Diffing against the frozen baseline keeps both + renders self-contained ANC payloads for Ubuntu and Mariner, each with default + and AKS custom-cloud variants, with AgentBaker's canonical Go-template renderer. + Diffing against the frozen baseline keeps both delivery paths cumulative. 3. Writes `scripts_version` for script hotfixes. With --use-anc-for-scripts, also @@ -467,7 +468,7 @@ def update_nodecustomdata(target_varkeys): return lines = clean_content.splitlines(keepends=True) - _, else_line, end_line = find_block_boundaries(lines) + scriptless_start, else_line, end_line = find_block_boundaries(lines) if else_line is None or end_line is None: raise GenerationError( f"could not find traditional write_files section in {TEMPLATE}" @@ -480,10 +481,22 @@ def update_nodecustomdata(target_varkeys): if not selected_blocks: raise GenerationError("no matching write_files blocks found for nodecustomdata") - injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] - for block_lines in selected_blocks: - injected_lines.extend(block_lines) - injected_lines.append(f"{SCRIPTS_END}\n") + existing_blocks = { + "".join(block_lines).strip() + for _, block_lines in parse_write_files_blocks( + lines[scriptless_start + 1:else_line] + ) + } + selected_blocks = [ + block_lines for block_lines in selected_blocks + if "".join(block_lines).strip() not in existing_blocks + ] + injected_lines = [] + if selected_blocks: + injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] + for block_lines in selected_blocks: + injected_lines.extend(block_lines) + injected_lines.append(f"{SCRIPTS_END}\n") with open(TEMPLATE, "w") as template_file: template_file.writelines(lines[:else_line] + injected_lines + lines[else_line:]) @@ -494,7 +507,7 @@ def update_nodecustomdata(target_varkeys): def write_rendered_payload(target_varkeys, traditional_lines): - """Render platform-specific YAML through AgentBaker's production template path.""" + """Render distro/cloud YAML through AgentBaker's production template path.""" if not target_varkeys: print( f"No new script hotfixes; preserving {GENERATED_DIR}", diff --git a/hotfix/hotfix_generate_test.py b/hotfix/hotfix_generate_test.py index 23b5ab5b11e..174c1d25cb8 100644 --- a/hotfix/hotfix_generate_test.py +++ b/hotfix/hotfix_generate_test.py @@ -1,6 +1,9 @@ +import base64 +import gzip import io import json import os +import re from pathlib import Path import subprocess import tempfile @@ -302,6 +305,71 @@ def render(command, check): {path.name for path in generated.iterdir()}, ) + def test_custom_cloud_and_distro_scripts_survive_production_rendering(self): + repository = Path(__file__).resolve().parents[1] + lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines( + keepends=True + ) + _, outer_else, end = hotfix_generate.find_block_boundaries(lines) + traditional = lines[outer_else + 1:end] + selections = ( + {"initAKSCloud"}, + {"initAKSCloud", "provisionSourceUbuntu", "provisionSourceMariner"}, + {"provisionSourceUbuntu", "provisionSourceMariner"}, + ) + for selected in selections: + with self.subTest(selected=selected), tempfile.TemporaryDirectory() as directory: + generated = Path(directory) / "generated" + with mock.patch.object( + hotfix_generate, "GENERATED_DIR", str(generated) + ): + hotfix_generate.write_rendered_payload(selected, traditional) + for platform in ("ubuntu", "mariner"): + for custom in (False, True): + suffix = "_akscustom" if custom else "" + path = ( + generated / f"rendered_nodecustomdata_{platform}{suffix}.yml" + ) + if custom and "initAKSCloud" not in selected: + self.assertFalse(path.exists()) + continue + content = path.read_text() + self.assertNotIn("{{", content) + entries = re.findall( + r"- path: ([^\n]+)\n" + r' permissions: "([0-7]+)"\n' + r" encoding: gzip\n owner: root\n" + r" content: !!binary \|\n ([A-Za-z0-9+/=]+)", + content, + ) + self.assertEqual( + int(custom) + int("provisionSourceUbuntu" in selected), + len(entries), + ) + for path, permissions, encoded in entries: + if path.endswith("/init-aks-cloud.sh"): + self.assertTrue(custom) + source = "init-aks-cloud.sh" + else: + self.assertTrue( + path.endswith("/provision_source_distro.sh") + ) + source = f"{platform}/cse_helpers_{platform}.sh" + self.assertEqual("0744", permissions) + decoded = gzip.decompress(base64.b64decode(encoded)) + expected = ( + repository / hotfix_generate.ARTIFACTS_DIR / source + ).read_bytes() + self.assertTrue(decoded.startswith(b"#!/bin/bash\n")) + self.assertGreater(len(decoded), 1000) + # The production renderer strips comments from scripts. + functions = re.findall( + rb"^\w+\(\) \{", expected, re.MULTILINE + ) + self.assertTrue(functions) + for function in functions: + self.assertIn(function, decoded) + def test_write_rendered_payload_preserves_placeholder_without_hotfix(self): with tempfile.TemporaryDirectory() as temp_dir: generated = Path(temp_dir) / "generated" @@ -326,7 +394,7 @@ def test_write_rendered_payload_preserves_previous_hotfix_when_unchanged(self): with tempfile.TemporaryDirectory() as temp_dir: generated = Path(temp_dir) / "generated" generated.mkdir() - platforms = ("ubuntu", "mariner") + platforms = ("ubuntu", "mariner", "ubuntu_akscustom", "mariner_akscustom") for platform in platforms: (generated / f"rendered_nodecustomdata_{platform}.yml").write_text( f"write_files:\n- path: /{platform}-existing\n" @@ -377,6 +445,47 @@ def test_update_nodecustomdata_injects_and_removes_selected_blocks(self): self.assertNotIn(hotfix_generate.SCRIPTS_BEGIN, cleaned) self.assertEqual(1, cleaned.count("provisionSourceUbuntu")) + def test_update_nodecustomdata_does_not_duplicate_custom_cloud_entry(self): + repository = Path(__file__).resolve().parents[1] + original = (repository / hotfix_generate.TEMPLATE).read_text() + lines = original.splitlines(keepends=True) + _, outer_else, end = hotfix_generate.find_block_boundaries(lines) + duplicate = hotfix_generate.build_hotfix_template( + {"initAKSCloud"}, lines[outer_else + 1:end] + ).removeprefix("#cloud-config\nwrite_files:\n") + # Start with an asset produced by the old generator, including its duplicate. + original = "".join( + lines[:outer_else] + + [f"\n{hotfix_generate.SCRIPTS_BEGIN}\n", duplicate, + f"{hotfix_generate.SCRIPTS_END}\n"] + + lines[outer_else:] + ) + with tempfile.TemporaryDirectory() as directory: + template = Path(directory) / "nodecustomdata.yml" + template.write_text(original) + with mock.patch.object(hotfix_generate, "TEMPLATE", str(template)): + for selected in ( + {"initAKSCloud"}, + {"initAKSCloud", "provisionSource"}, + {"initAKSCloud", "provisionSource"}, + {"initAKSCloud"}, + ): + hotfix_generate.update_nodecustomdata(selected) + lines = template.read_text().splitlines(keepends=True) + start, outer_else, end = hotfix_generate.find_block_boundaries( + lines + ) + for section in ( + lines[start + 1:outer_else], lines[outer_else + 1:end] + ): + content = "".join(section) + self.assertEqual(1, content.count("{{if IsAKSCustomCloud}}")) + self.assertEqual(1, content.count('"initAKSCloud"')) + self.assertEqual( + "provisionSource" in selected, + hotfix_generate.SCRIPTS_BEGIN in "".join(lines), + ) + def test_write_hotfix_file_uses_hotfixes_and_preserves_scripts_version(self): with tempfile.TemporaryDirectory() as temp_dir: target = Path(temp_dir) / "hotfix.json" diff --git a/hotfix/render-nodecustomdata/main.go b/hotfix/render-nodecustomdata/main.go index 798f8d36875..5a83231f2f4 100644 --- a/hotfix/render-nodecustomdata/main.go +++ b/hotfix/render-nodecustomdata/main.go @@ -42,38 +42,48 @@ func main() { } for _, target := range platforms { - rendered, err := agent.RenderLinuxNodeCustomDataTemplate( - templateContent, - newRenderConfig(target.distro), - ) - if err != nil { - fatalf("render %s nodecustomdata: %v", target.name, err) - } - outputPath := filepath.Join( - *outputDir, - "rendered_nodecustomdata_"+target.name+".yml", - ) - //nolint:gosec // The caller intentionally selects the output directory; filenames are fixed by this tool. - if err := os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil { - fatalf("write %s nodecustomdata: %v", target.name, err) + var defaultRendered string + for _, customCloud := range []bool{false, true} { + rendered, err := agent.RenderLinuxNodeCustomDataTemplate( + templateContent, + newRenderConfig(target.distro, customCloud), + ) + if err != nil { + fatalf("render %s nodecustomdata (custom cloud %t): %v", target.name, customCloud, err) + } + if customCloud && rendered == defaultRendered { + continue + } + if !customCloud { + defaultRendered = rendered + } + name := "rendered_nodecustomdata_" + target.name + if customCloud { + name += "_akscustom" + } + outputPath := filepath.Join(*outputDir, name+".yml") + //nolint:gosec // The caller intentionally selects the output directory; filenames are fixed by this tool. + if err := os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil { + fatalf("write %s nodecustomdata: %v", target.name, err) + } } } } // newRenderConfig builds the minimal NodeBootstrappingConfiguration required to -// run agent.RenderLinuxNodeCustomDataTemplate. Only the Distro selector affects -// the hotfix write_files blocks we render; the remaining fields (orchestrator +// run agent.RenderLinuxNodeCustomDataTemplate. Distro and custom-cloud eligibility +// select the hotfix write_files blocks; the remaining fields (orchestrator // version, location, FQDN, KubernetesConfig, etc.) are placeholders whose sole // purpose is to satisfy the renderer's unconditional dereferences of // OrchestratorProfile/KubernetesConfig so rendering doesn't nil-panic. Their // values are not meaningful and are not consumed by the embedded output. -func newRenderConfig(distro datamodel.Distro) *datamodel.NodeBootstrappingConfiguration { +func newRenderConfig(distro datamodel.Distro, customCloud bool) *datamodel.NodeBootstrappingConfiguration { profile := &datamodel.AgentPoolProfile{ Name: "hotfix-render", OSType: datamodel.Linux, Distro: distro, } - return &datamodel.NodeBootstrappingConfiguration{ + config := &datamodel.NodeBootstrappingConfiguration{ ContainerService: &datamodel.ContainerService{ Location: "eastus", Properties: &datamodel.Properties{ @@ -95,6 +105,10 @@ func newRenderConfig(distro datamodel.Distro) *datamodel.NodeBootstrappingConfig K8sComponents: &datamodel.K8sComponents{}, KubeletConfig: map[string]string{}, } + if customCloud { + config.ContainerService.Properties.CustomCloudEnv = &datamodel.CustomCloudEnv{Name: "akscustom"} + } + return config } func fatalf(format string, args ...interface{}) { From 118ae2d635e4ea81bf4a5357efdab3206f55cc55 Mon Sep 17 00:00:00 2001 From: Devin Wong Date: Tue, 20 Oct 2026 15:54:53 -0700 Subject: [PATCH 2/4] refactor: minimize custom-cloud hotfix changes Keep the existing renderer loop with explicit cloud targets and consolidate regression cases while retaining runtime eligibility protection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b42051a-f79c-4dbf-8460-a4772d54e01f --- aks-node-controller/README.md | 18 ++-- .../embeddednodecustomdata_test.go | 95 +++++-------------- hotfix/hotfix_generate_test.py | 50 ++-------- hotfix/render-nodecustomdata/main.go | 59 ++++++------ 4 files changed, 67 insertions(+), 155 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index 9671d9e72c1..9f90e969cca 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -250,21 +250,15 @@ updates to the ANC binary itself. The generated files live under `rendered_nodecustomdata_.yml`. The generator selects only changed hotfixable entries from `nodecustomdata.yml`, then renders only Ubuntu and standard Azure Linux variants through AgentBaker's production Go-template -functions. When custom-cloud rendering differs, each platform also has a -`rendered_nodecustomdata__akscustom.yml` variant that preserves -`IsAKSCustomCloud` eligibility. ANC follows provisioning's input precedence: -when an NBC command exists, it reads `IS_CUSTOM_CLOUD` without executing the -command; otherwise it reads the live node config's -`custom_cloud_config.custom_cloud_env_name` and compares it to `akscustom` -(case-insensitive, matching the production renderer). Malformed or unavailable eligibility -inputs return an error before any payload is written; the caller logs the error. -Existing packages without cloud variants retain distro-only application. -Azure Linux retains the `mariner` payload filename; the legacy +functions. Distinct custom-cloud output uses an `_akscustom.yml` variant, selected +using the NBC command's `IS_CUSTOM_CLOUD` or, when that command file is absent, the node config's +`custom_cloud_config.custom_cloud_env_name` (case-insensitive `akscustom`). +Unresolvable eligibility returns an error before writes. Packages without cloud +variants retain distro-only application. Azure Linux retains the `mariner` payload filename; the legacy `ID=mariner` OS is no longer supported. OS Guard, ACL, and Flatcar are explicitly skipped during embedded application, including variants that share the `azurelinux` OS ID. Their distro-specific source changes do not trigger payload generation. -For cloud-init delivery, generation does not inject a second copy of a canonical -block already present in the scriptless branch (such as `init-aks-cloud.sh`). +Generation does not duplicate blocks already present in the scriptless branch. The repository keeps a README placeholder so `go:embed` builds without any script hotfix payloads. Generation replaces it with Ubuntu and Azure Linux YAMLs. diff --git a/aks-node-controller/embeddednodecustomdata_test.go b/aks-node-controller/embeddednodecustomdata_test.go index c5bab3436f7..2ffdcc891dc 100644 --- a/aks-node-controller/embeddednodecustomdata_test.go +++ b/aks-node-controller/embeddednodecustomdata_test.go @@ -2,7 +2,6 @@ package main import ( "encoding/base64" - "errors" "io/fs" "os" "path/filepath" @@ -15,24 +14,21 @@ import ( "gopkg.in/yaml.v3" ) -func TestEmbeddedHotfixCloudEligibility(t *testing.T) { +func TestApplyEmbeddedNodeCustomDataCloudEligibility(t *testing.T) { tests := []struct { name, config, command, wantError string wantCustom bool }{ {name: "custom", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, wantCustom: true}, - {name: "case insensitive", config: `{"customCloudConfig":{"customCloudEnvName":"AKSCustom"}}`, wantCustom: true}, + {name: "case insensitive protojson", config: `{"customCloudConfig":{"customCloudEnvName":"AKSCustom"}}`, wantCustom: true}, {name: "public", config: `{}`}, {name: "other cloud", config: `{"custom_cloud_config":{"custom_cloud_env_name":"AzureChinaCloud"}}`}, {name: "command authoritative", config: `{}`, command: `IS_CUSTOM_CLOUD=true`, wantCustom: true}, {name: "command excludes custom config", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `IS_CUSTOM_CLOUD=false`}, - {name: "command ignores invalid config", config: `{`, command: `IS_CUSTOM_CLOUD=true`, wantCustom: true}, {name: "invalid config", config: `{`, wantError: "decode node config"}, {name: "invalid command does not fall back", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `echo test`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, {name: "legacy custom", command: `IS_CUSTOM_CLOUD="true" /bin/bash provision.sh`, wantCustom: true}, {name: "legacy public", command: `IS_CUSTOM_CLOUD=false /bin/bash provision.sh`}, - {name: "missing marker", command: `echo IS_CUSTOM_CLOUD`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, - {name: "invalid marker", command: `IS_CUSTOM_CLOUD=TRUE`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, {name: "missing inputs", wantError: "read node config"}, } for _, test := range tests { @@ -48,80 +44,37 @@ func TestEmbeddedHotfixCloudEligibility(t *testing.T) { if test.command != "" { require.NoError(t, os.WriteFile(app.nbcCmdPath, []byte(test.command), 0o600)) } - actual, err := app.isEmbeddedHotfixCustomCloud() + script := filepath.Join(directory, "init-aks-cloud.sh") + require.NoError(t, os.WriteFile(script, []byte("original"), 0o600)) + data, err := yaml.Marshal(nodeCustomData{WriteFiles: []nodeCustomDataWriteFile{ + {Path: script, Content: "hotfix"}, + }}) + require.NoError(t, err) + files := fstest.MapFS{ + "generated/rendered_nodecustomdata_ubuntu.yml": &fstest.MapFile{Data: []byte("write_files: []")}, + "generated/rendered_nodecustomdata_ubuntu_akscustom.yml": &fstest.MapFile{Data: data}, + } + release := filepath.Join(directory, "os-release") + require.NoError(t, os.WriteFile(release, []byte("ID=ubuntu"), 0o600)) + output := filepath.Join(directory, "embedded.yml") + err = applyEmbeddedNodeCustomData(files, release, output, app.isEmbeddedHotfixCustomCloud) if test.wantError != "" { require.ErrorContains(t, err, test.wantError) + assert.NoFileExists(t, output) } else { require.NoError(t, err) - assert.Equal(t, test.wantCustom, actual) } + actual, err := os.ReadFile(script) + require.NoError(t, err) + expected := "original" + if test.wantCustom { + expected = "hotfix" + } + assert.Equal(t, expected, string(actual)) }) } } -func TestApplyEmbeddedNodeCustomDataSelectsCloudVariant(t *testing.T) { - for _, platform := range []string{"ubuntu", "mariner"} { - for _, cloud := range []string{"public", "custom", "unknown"} { - t.Run(platform+"/"+cloud, func(t *testing.T) { - directory := t.TempDir() - script := filepath.Join(directory, "init-aks-cloud.sh") - common := filepath.Join(directory, "provision_source_distro.sh") - require.NoError(t, os.WriteFile(script, []byte("original"), 0o600)) - publicFiles := []nodeCustomDataWriteFile{{Path: common, Content: platform}} - customFiles := append(append([]nodeCustomDataWriteFile{}, publicFiles...), nodeCustomDataWriteFile{ - Path: script, Encoding: encodingBase64, Content: base64.StdEncoding.EncodeToString([]byte("hotfix")), - }) - publicYAML, err := yaml.Marshal(nodeCustomData{WriteFiles: publicFiles}) - require.NoError(t, err) - customYAML, err := yaml.Marshal(nodeCustomData{WriteFiles: customFiles}) - require.NoError(t, err) - name := "generated/rendered_nodecustomdata_" + platform - files := fstest.MapFS{ - name + ".yml": &fstest.MapFile{Data: publicYAML}, - name + "_akscustom.yml": &fstest.MapFile{Data: customYAML}, - } - releasePath := filepath.Join(directory, "os-release") - id := platform - if platform == "mariner" { - id = "azurelinux" - } - require.NoError(t, os.WriteFile(releasePath, []byte("ID="+id), 0o600)) - outputPath := filepath.Join(directory, "embedded.yml") - err = applyEmbeddedNodeCustomData(files, releasePath, outputPath, func() (bool, error) { - if cloud == "unknown" { - return false, errors.New("unavailable config") - } - return cloud == "custom", nil - }) - if cloud == "unknown" { - require.ErrorContains(t, err, "resolve embedded hotfix cloud eligibility") - assert.NoFileExists(t, common) - assert.NoFileExists(t, outputPath) - } else { - require.NoError(t, err) - actual, err := os.ReadFile(common) - require.NoError(t, err) - assert.Equal(t, platform, string(actual)) - retained, err := os.ReadFile(outputPath) - require.NoError(t, err) - if cloud == "custom" { - assert.Equal(t, customYAML, retained) - } else { - assert.Equal(t, publicYAML, retained) - } - } - actual, err := os.ReadFile(script) - require.NoError(t, err) - if cloud == "custom" { - assert.Equal(t, "hotfix", string(actual)) - } else { - assert.Equal(t, "original", string(actual), "ineligible nodes must not overwrite the script") - } - }) - } - } -} - func TestClassifyNodeCustomDataPlatform(t *testing.T) { tests := []struct { name string diff --git a/hotfix/hotfix_generate_test.py b/hotfix/hotfix_generate_test.py index 174c1d25cb8..ce31443dfeb 100644 --- a/hotfix/hotfix_generate_test.py +++ b/hotfix/hotfix_generate_test.py @@ -335,40 +335,18 @@ def test_custom_cloud_and_distro_scripts_survive_production_rendering(self): continue content = path.read_text() self.assertNotIn("{{", content) - entries = re.findall( - r"- path: ([^\n]+)\n" - r' permissions: "([0-7]+)"\n' - r" encoding: gzip\n owner: root\n" - r" content: !!binary \|\n ([A-Za-z0-9+/=]+)", - content, - ) + entries = re.findall(r"- path: ([^\n]+)", content) self.assertEqual( int(custom) + int("provisionSourceUbuntu" in selected), len(entries), ) - for path, permissions, encoded in entries: - if path.endswith("/init-aks-cloud.sh"): - self.assertTrue(custom) - source = "init-aks-cloud.sh" - else: - self.assertTrue( - path.endswith("/provision_source_distro.sh") - ) - source = f"{platform}/cse_helpers_{platform}.sh" - self.assertEqual("0744", permissions) + self.assertEqual( + custom, "/opt/azure/containers/init-aks-cloud.sh" in entries + ) + for encoded in re.findall(r"content: !!binary \|\n (\S+)", content): decoded = gzip.decompress(base64.b64decode(encoded)) - expected = ( - repository / hotfix_generate.ARTIFACTS_DIR / source - ).read_bytes() self.assertTrue(decoded.startswith(b"#!/bin/bash\n")) self.assertGreater(len(decoded), 1000) - # The production renderer strips comments from scripts. - functions = re.findall( - rb"^\w+\(\) \{", expected, re.MULTILINE - ) - self.assertTrue(functions) - for function in functions: - self.assertIn(function, decoded) def test_write_rendered_payload_preserves_placeholder_without_hotfix(self): with tempfile.TemporaryDirectory() as temp_dir: @@ -447,8 +425,7 @@ def test_update_nodecustomdata_injects_and_removes_selected_blocks(self): def test_update_nodecustomdata_does_not_duplicate_custom_cloud_entry(self): repository = Path(__file__).resolve().parents[1] - original = (repository / hotfix_generate.TEMPLATE).read_text() - lines = original.splitlines(keepends=True) + lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines(keepends=True) _, outer_else, end = hotfix_generate.find_block_boundaries(lines) duplicate = hotfix_generate.build_hotfix_template( {"initAKSCloud"}, lines[outer_else + 1:end] @@ -471,19 +448,12 @@ def test_update_nodecustomdata_does_not_duplicate_custom_cloud_entry(self): {"initAKSCloud"}, ): hotfix_generate.update_nodecustomdata(selected) - lines = template.read_text().splitlines(keepends=True) - start, outer_else, end = hotfix_generate.find_block_boundaries( - lines - ) - for section in ( - lines[start + 1:outer_else], lines[outer_else + 1:end] - ): - content = "".join(section) - self.assertEqual(1, content.count("{{if IsAKSCustomCloud}}")) - self.assertEqual(1, content.count('"initAKSCloud"')) + content = template.read_text() + self.assertEqual(2, content.count("{{if IsAKSCustomCloud}}")) + self.assertEqual(2, content.count('"initAKSCloud"')) self.assertEqual( "provisionSource" in selected, - hotfix_generate.SCRIPTS_BEGIN in "".join(lines), + hotfix_generate.SCRIPTS_BEGIN in content, ) def test_write_hotfix_file_uses_hotfixes_and_preserves_scripts_version(self): diff --git a/hotfix/render-nodecustomdata/main.go b/hotfix/render-nodecustomdata/main.go index 5a83231f2f4..ed1e65b903c 100644 --- a/hotfix/render-nodecustomdata/main.go +++ b/hotfix/render-nodecustomdata/main.go @@ -14,14 +14,17 @@ import ( ) type platform struct { - name string - distro datamodel.Distro + name string + distro datamodel.Distro + customCloud bool } func main() { platforms := []platform{ {name: "ubuntu", distro: datamodel.AKSUbuntuContainerd2204Gen2}, {name: "mariner", distro: datamodel.AKSAzureLinuxV3Gen2}, + {name: "ubuntu_akscustom", distro: datamodel.AKSUbuntuContainerd2204Gen2, customCloud: true}, + {name: "mariner_akscustom", distro: datamodel.AKSAzureLinuxV3Gen2, customCloud: true}, } templatePath := flag.String("template", "", "path to the hotfix nodecustomdata template") @@ -41,31 +44,27 @@ func main() { fatalf("create output directory: %v", err) } + defaults := make(map[datamodel.Distro]string) for _, target := range platforms { - var defaultRendered string - for _, customCloud := range []bool{false, true} { - rendered, err := agent.RenderLinuxNodeCustomDataTemplate( - templateContent, - newRenderConfig(target.distro, customCloud), - ) - if err != nil { - fatalf("render %s nodecustomdata (custom cloud %t): %v", target.name, customCloud, err) - } - if customCloud && rendered == defaultRendered { - continue - } - if !customCloud { - defaultRendered = rendered - } - name := "rendered_nodecustomdata_" + target.name - if customCloud { - name += "_akscustom" - } - outputPath := filepath.Join(*outputDir, name+".yml") - //nolint:gosec // The caller intentionally selects the output directory; filenames are fixed by this tool. - if err := os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil { - fatalf("write %s nodecustomdata: %v", target.name, err) - } + config := newRenderConfig(target.distro) + if target.customCloud { + config.ContainerService.Properties.CustomCloudEnv = &datamodel.CustomCloudEnv{Name: "akscustom"} + } + rendered, err := agent.RenderLinuxNodeCustomDataTemplate(templateContent, config) + if err != nil { + fatalf("render %s nodecustomdata: %v", target.name, err) + } + if target.customCloud && rendered == defaults[target.distro] { + continue + } + defaults[target.distro] = rendered + outputPath := filepath.Join( + *outputDir, + "rendered_nodecustomdata_"+target.name+".yml", + ) + //nolint:gosec // The caller intentionally selects the output directory; filenames are fixed by this tool. + if err := os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil { + fatalf("write %s nodecustomdata: %v", target.name, err) } } } @@ -77,13 +76,13 @@ func main() { // purpose is to satisfy the renderer's unconditional dereferences of // OrchestratorProfile/KubernetesConfig so rendering doesn't nil-panic. Their // values are not meaningful and are not consumed by the embedded output. -func newRenderConfig(distro datamodel.Distro, customCloud bool) *datamodel.NodeBootstrappingConfiguration { +func newRenderConfig(distro datamodel.Distro) *datamodel.NodeBootstrappingConfiguration { profile := &datamodel.AgentPoolProfile{ Name: "hotfix-render", OSType: datamodel.Linux, Distro: distro, } - config := &datamodel.NodeBootstrappingConfiguration{ + return &datamodel.NodeBootstrappingConfiguration{ ContainerService: &datamodel.ContainerService{ Location: "eastus", Properties: &datamodel.Properties{ @@ -105,10 +104,6 @@ func newRenderConfig(distro datamodel.Distro, customCloud bool) *datamodel.NodeB K8sComponents: &datamodel.K8sComponents{}, KubeletConfig: map[string]string{}, } - if customCloud { - config.ContainerService.Properties.CustomCloudEnv = &datamodel.CustomCloudEnv{Name: "akscustom"} - } - return config } func fatalf(format string, args ...interface{}) { From a405924e2d1205b7794f9f5a8162ff455d740cd8 Mon Sep 17 00:00:00 2001 From: Devin Wong Date: Tue, 20 Oct 2026 16:00:08 -0700 Subject: [PATCH 3/4] fix: scope hotfix generation to omitting empty payloads Remove runtime cloud selection and duplicate-block changes. Skip rendered YAML without write_files entries, remove stale empty outputs, and retain an embed placeholder when all platforms are empty. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b42051a-f79c-4dbf-8460-a4772d54e01f --- aks-node-controller/README.md | 18 ++- aks-node-controller/app.go | 2 +- aks-node-controller/embeddednodecustomdata.go | 48 +------ .../embeddednodecustomdata_test.go | 69 +--------- hotfix/hotfix_generate.py | 29 ++-- hotfix/hotfix_generate_test.py | 128 +++++++----------- hotfix/render-nodecustomdata/main.go | 47 ++++--- 7 files changed, 100 insertions(+), 241 deletions(-) diff --git a/aks-node-controller/README.md b/aks-node-controller/README.md index 9f90e969cca..51337747968 100644 --- a/aks-node-controller/README.md +++ b/aks-node-controller/README.md @@ -250,18 +250,16 @@ updates to the ANC binary itself. The generated files live under `rendered_nodecustomdata_.yml`. The generator selects only changed hotfixable entries from `nodecustomdata.yml`, then renders only Ubuntu and standard Azure Linux variants through AgentBaker's production Go-template -functions. Distinct custom-cloud output uses an `_akscustom.yml` variant, selected -using the NBC command's `IS_CUSTOM_CLOUD` or, when that command file is absent, the node config's -`custom_cloud_config.custom_cloud_env_name` (case-insensitive `akscustom`). -Unresolvable eligibility returns an error before writes. Packages without cloud -variants retain distro-only application. Azure Linux retains the `mariner` payload filename; the legacy +functions. Azure Linux retains the `mariner` payload filename; the legacy `ID=mariner` OS is no longer supported. OS Guard, ACL, and Flatcar are explicitly skipped during embedded application, including variants that share the `azurelinux` OS ID. Their distro-specific source changes do not trigger payload generation. -Generation does not duplicate blocks already present in the scriptless branch. The repository keeps a README placeholder so `go:embed` builds without any -script hotfix payloads. Generation replaces it with Ubuntu and Azure Linux YAMLs. +script hotfix payloads. Generation omits any rendered YAML with no `write_files` +entries (including missing, null, or empty lists) and logs the skipped platform. +If all platforms are empty, it writes a README placeholder to keep `go:embed` valid. +This does not add support for non-distro template conditions. ANC skips application when the local platform's YAML is absent; no separate activation flag is needed. Other payload read errors are logged. @@ -271,9 +269,9 @@ is retired explicitly. The existing applier writes entries sequentially and creates missing destination files and parent directories. There is no transactional rollback: if an entry -fails, earlier writes remain. Generation selects by distro and AKS custom-cloud -eligibility; hotfix authors must separately account for other template conditions -such as custom-image exclusions. +fails, earlier writes remain. Generation selects by distro only; hotfix authors +must separately account for non-distro template conditions such as custom-image +exclusions. `cse_start.sh` (`provision_start.sh` on the node) is excluded from embedded hotfixes to preserve custom-image wrappers. Generation fails explicitly if this diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index a69bee0f91e..fbbdd616f1c 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -271,7 +271,7 @@ func (a *App) runApplyHotfixCommand(context.Context) error { applyHotfix := a.applyEmbeddedHotfix if applyHotfix == nil { applyHotfix = func(osReleasePath string) error { - return applyEmbeddedNodeCustomData(embeddedGeneratedNodeCustomData, osReleasePath, embeddedNodeCustomDataPath, a.isEmbeddedHotfixCustomCloud) + return applyEmbeddedNodeCustomData(embeddedGeneratedNodeCustomData, osReleasePath, embeddedNodeCustomDataPath) } } if err := applyHotfix(a.osReleasePath); err != nil { diff --git a/aks-node-controller/embeddednodecustomdata.go b/aks-node-controller/embeddednodecustomdata.go index 934314de4a7..67615d54fcb 100644 --- a/aks-node-controller/embeddednodecustomdata.go +++ b/aks-node-controller/embeddednodecustomdata.go @@ -9,9 +9,6 @@ import ( "os" "path/filepath" "strings" - - "github.com/Azure/agentbaker/aks-node-controller/helpers" - "github.com/Azure/agentbaker/aks-node-controller/pkg/nodeconfigutils" ) const defaultOSReleasePath = "/etc/os-release" @@ -37,7 +34,7 @@ const ( //go:embed generated var embeddedGeneratedNodeCustomData embed.FS -func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath string, isCustomCloud func() (bool, error)) error { +func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath string) error { if osReleasePath == "" { osReleasePath = defaultOSReleasePath } @@ -50,18 +47,6 @@ func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath stri return nil } renderedPath := fmt.Sprintf("generated/rendered_nodecustomdata_%s.yml", platform) - customCloudPath := strings.TrimSuffix(renderedPath, ".yml") + "_akscustom.yml" - if _, err := fs.Stat(payloadFS, customCloudPath); err == nil { - customCloud, err := isCustomCloud() - if err != nil { - return fmt.Errorf("resolve embedded hotfix cloud eligibility: %w", err) - } - if customCloud { - renderedPath = customCloudPath - } - } else if !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("stat embedded nodecustomdata %s: %w", customCloudPath, err) - } data, err := fs.ReadFile(payloadFS, renderedPath) if errors.Is(err, fs.ErrNotExist) { return nil @@ -82,37 +67,6 @@ func applyEmbeddedNodeCustomData(payloadFS fs.FS, osReleasePath, outputPath stri return nil } -func (a *App) isEmbeddedHotfixCustomCloud() (bool, error) { - // Match Provision's precedence when both live configuration forms are present. - commandPath := a.getNBCCmdPath() - data, err := os.ReadFile(commandPath) - if err == nil { - value := parseEnvVarsFromNBCCmdContent(string(data))["IS_CUSTOM_CLOUD"] - switch value { - case "true": - return true, nil - case "false": - return false, nil - default: - return false, fmt.Errorf("NBC command %s has missing or invalid IS_CUSTOM_CLOUD", commandPath) - } - } - if !errors.Is(err, fs.ErrNotExist) { - return false, fmt.Errorf("read NBC command %s: %w", commandPath, err) - } - - configPath := a.getNodeConfigPath() - data, err = os.ReadFile(configPath) - if err != nil { - return false, fmt.Errorf("read node config %s: %w", configPath, err) - } - config, err := nodeconfigutils.UnmarshalConfigurationV1(data) - if err != nil { - return false, fmt.Errorf("decode node config %s: %w", configPath, err) - } - return strings.EqualFold(config.GetCustomCloudConfig().GetCustomCloudEnvName(), helpers.AksCustomCloudName), nil -} - func classifyNodeCustomDataPlatform(osReleasePath string) (nodeCustomDataPlatform, error) { data, err := os.ReadFile(osReleasePath) if err != nil { diff --git a/aks-node-controller/embeddednodecustomdata_test.go b/aks-node-controller/embeddednodecustomdata_test.go index 2ffdcc891dc..c57aaae51a4 100644 --- a/aks-node-controller/embeddednodecustomdata_test.go +++ b/aks-node-controller/embeddednodecustomdata_test.go @@ -14,67 +14,6 @@ import ( "gopkg.in/yaml.v3" ) -func TestApplyEmbeddedNodeCustomDataCloudEligibility(t *testing.T) { - tests := []struct { - name, config, command, wantError string - wantCustom bool - }{ - {name: "custom", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, wantCustom: true}, - {name: "case insensitive protojson", config: `{"customCloudConfig":{"customCloudEnvName":"AKSCustom"}}`, wantCustom: true}, - {name: "public", config: `{}`}, - {name: "other cloud", config: `{"custom_cloud_config":{"custom_cloud_env_name":"AzureChinaCloud"}}`}, - {name: "command authoritative", config: `{}`, command: `IS_CUSTOM_CLOUD=true`, wantCustom: true}, - {name: "command excludes custom config", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `IS_CUSTOM_CLOUD=false`}, - {name: "invalid config", config: `{`, wantError: "decode node config"}, - {name: "invalid command does not fall back", config: `{"custom_cloud_config":{"custom_cloud_env_name":"akscustom"}}`, command: `echo test`, wantError: "missing or invalid IS_CUSTOM_CLOUD"}, - {name: "legacy custom", command: `IS_CUSTOM_CLOUD="true" /bin/bash provision.sh`, wantCustom: true}, - {name: "legacy public", command: `IS_CUSTOM_CLOUD=false /bin/bash provision.sh`}, - {name: "missing inputs", wantError: "read node config"}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - directory := t.TempDir() - app := &App{ - nodeConfigPath: filepath.Join(directory, "config.json"), - nbcCmdPath: filepath.Join(directory, "nbc-cmd.sh"), - } - if test.config != "" { - require.NoError(t, os.WriteFile(app.nodeConfigPath, []byte(test.config), 0o600)) - } - if test.command != "" { - require.NoError(t, os.WriteFile(app.nbcCmdPath, []byte(test.command), 0o600)) - } - script := filepath.Join(directory, "init-aks-cloud.sh") - require.NoError(t, os.WriteFile(script, []byte("original"), 0o600)) - data, err := yaml.Marshal(nodeCustomData{WriteFiles: []nodeCustomDataWriteFile{ - {Path: script, Content: "hotfix"}, - }}) - require.NoError(t, err) - files := fstest.MapFS{ - "generated/rendered_nodecustomdata_ubuntu.yml": &fstest.MapFile{Data: []byte("write_files: []")}, - "generated/rendered_nodecustomdata_ubuntu_akscustom.yml": &fstest.MapFile{Data: data}, - } - release := filepath.Join(directory, "os-release") - require.NoError(t, os.WriteFile(release, []byte("ID=ubuntu"), 0o600)) - output := filepath.Join(directory, "embedded.yml") - err = applyEmbeddedNodeCustomData(files, release, output, app.isEmbeddedHotfixCustomCloud) - if test.wantError != "" { - require.ErrorContains(t, err, test.wantError) - assert.NoFileExists(t, output) - } else { - require.NoError(t, err) - } - actual, err := os.ReadFile(script) - require.NoError(t, err) - expected := "original" - if test.wantCustom { - expected = "hotfix" - } - assert.Equal(t, expected, string(actual)) - }) - } -} - func TestClassifyNodeCustomDataPlatform(t *testing.T) { tests := []struct { name string @@ -98,7 +37,7 @@ func TestClassifyNodeCustomDataPlatform(t *testing.T) { assert.Equal(t, test.expected, actual) if actual == nodeCustomDataPlatformUnsupported { outputPath := filepath.Join(t.TempDir(), "embedded-nodecustomdata.yml") - require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath, nil), + require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath), "unsupported platforms must skip without reading any payload") assert.NoFileExists(t, outputPath) } @@ -133,7 +72,7 @@ func TestApplyEmbeddedNodeCustomData(t *testing.T) { files := fstest.MapFS{ "generated/rendered_nodecustomdata_" + platform + ".yml": &fstest.MapFile{Data: data}, } - require.NoError(t, applyEmbeddedNodeCustomData(files, releasePath, outputPath, nil)) + require.NoError(t, applyEmbeddedNodeCustomData(files, releasePath, outputPath)) for _, destination := range []string{existing, missing} { actual, readErr := os.ReadFile(destination) require.NoError(t, readErr) @@ -145,7 +84,7 @@ func TestApplyEmbeddedNodeCustomData(t *testing.T) { legacy, err := os.ReadFile(legacyPath) require.NoError(t, err) assert.Equal(t, "legacy payload", string(legacy)) - require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath, nil)) + require.NoError(t, applyEmbeddedNodeCustomData(fstest.MapFS{}, releasePath, outputPath)) retained, err = os.ReadFile(outputPath) require.NoError(t, err) assert.Equal(t, data, retained, "a skipped application must preserve the last payload") @@ -203,7 +142,7 @@ func TestApplyEmbeddedNodeCustomDataErrorsAndRetention(t *testing.T) { if test.missing == "payload-file" { files["generated/rendered_nodecustomdata_ubuntu.yml"].Mode = fs.ModeDir } - err := applyEmbeddedNodeCustomData(files, releasePath, outputPath, nil) + err := applyEmbeddedNodeCustomData(files, releasePath, outputPath) if test.wantError == "" { require.NoError(t, err) } else { diff --git a/hotfix/hotfix_generate.py b/hotfix/hotfix_generate.py index 8ec95d89cc3..2623604bac3 100644 --- a/hotfix/hotfix_generate.py +++ b/hotfix/hotfix_generate.py @@ -13,9 +13,8 @@ (the release tag the VHD was built from, derived from linux_sig_version.json), selects their write_files entries from parts/linux/cloud-init/nodecustomdata.yml, injects those entries into the scriptless section of nodecustomdata.yml, and - renders self-contained ANC payloads for Ubuntu and Mariner, each with default - and AKS custom-cloud variants, with AgentBaker's canonical Go-template renderer. - Diffing against the frozen baseline keeps both + renders self-contained ANC payloads for Ubuntu and Mariner with AgentBaker's + canonical Go-template renderer. Diffing against the frozen baseline keeps both delivery paths cumulative. 3. Writes `scripts_version` for script hotfixes. With --use-anc-for-scripts, also @@ -468,7 +467,7 @@ def update_nodecustomdata(target_varkeys): return lines = clean_content.splitlines(keepends=True) - scriptless_start, else_line, end_line = find_block_boundaries(lines) + _, else_line, end_line = find_block_boundaries(lines) if else_line is None or end_line is None: raise GenerationError( f"could not find traditional write_files section in {TEMPLATE}" @@ -481,22 +480,10 @@ def update_nodecustomdata(target_varkeys): if not selected_blocks: raise GenerationError("no matching write_files blocks found for nodecustomdata") - existing_blocks = { - "".join(block_lines).strip() - for _, block_lines in parse_write_files_blocks( - lines[scriptless_start + 1:else_line] - ) - } - selected_blocks = [ - block_lines for block_lines in selected_blocks - if "".join(block_lines).strip() not in existing_blocks - ] - injected_lines = [] - if selected_blocks: - injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] - for block_lines in selected_blocks: - injected_lines.extend(block_lines) - injected_lines.append(f"{SCRIPTS_END}\n") + injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] + for block_lines in selected_blocks: + injected_lines.extend(block_lines) + injected_lines.append(f"{SCRIPTS_END}\n") with open(TEMPLATE, "w") as template_file: template_file.writelines(lines[:else_line] + injected_lines + lines[else_line:]) @@ -507,7 +494,7 @@ def update_nodecustomdata(target_varkeys): def write_rendered_payload(target_varkeys, traditional_lines): - """Render distro/cloud YAML through AgentBaker's production template path.""" + """Render platform-specific YAML through AgentBaker's production template path.""" if not target_varkeys: print( f"No new script hotfixes; preserving {GENERATED_DIR}", diff --git a/hotfix/hotfix_generate_test.py b/hotfix/hotfix_generate_test.py index ce31443dfeb..31ed80b03d3 100644 --- a/hotfix/hotfix_generate_test.py +++ b/hotfix/hotfix_generate_test.py @@ -1,9 +1,6 @@ -import base64 -import gzip import io import json import os -import re from pathlib import Path import subprocess import tempfile @@ -59,6 +56,53 @@ class HotfixGenerateTests(unittest.TestCase): + def test_production_renderer_omits_empty_platform_payloads(self): + repository = Path(__file__).resolve().parents[1] + lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines( + keepends=True + ) + _, outer_else, end = hotfix_generate.find_block_boundaries(lines) + cases = [ + ("custom cloud only", hotfix_generate.build_hotfix_template( + {"initAKSCloud"}, lines[outer_else + 1:end] + ), set()), + ("missing", "#cloud-config\n", set()), + ("null", "write_files:\n", set()), + ("empty list", "write_files: []\n", set()), + ("ubuntu only", 'write_files:\n{{if not IsMariner}}\n' + '- path: /test\n content: hotfix\n{{end}}\n', {"ubuntu"}), + ("both", "write_files:\n- path: /test\n content: hotfix\n", + {"ubuntu", "mariner"}), + ] + for name, template, platforms in cases: + with self.subTest(name=name), tempfile.TemporaryDirectory() as directory: + template_path = Path(directory) / "template.yml" + template_path.write_text(template) + output = Path(directory) / "generated" + output.mkdir() + for platform in ("ubuntu", "mariner"): + (output / f"rendered_nodecustomdata_{platform}.yml").write_text( + "write_files:\n" # Stale empty files must also be removed. + ) + subprocess.run( + ["go", "run", "./hotfix/render-nodecustomdata", "--template", + str(template_path), "--output-dir", str(output)], + cwd=repository, check=True, capture_output=True, + ) + expected = { + f"rendered_nodecustomdata_{platform}.yml" for platform in platforms + } or {"README"} + self.assertEqual(expected, {path.name for path in output.iterdir()}) + if name == "custom cloud only": + probe = Path(directory) / "main.go" + probe.write_text('package main\nimport "embed"\n' + '//go:embed generated\nvar files embed.FS\n' + 'func main() {}\n') + subprocess.run( + ["go", "build", "-o", str(Path(directory) / "probe"), str(probe)], + cwd=repository, check=True, capture_output=True, + ) + def test_config_modules_are_independently_hotfixable(self): repository = Path(__file__).resolve().parents[1] lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines( @@ -305,49 +349,6 @@ def render(command, check): {path.name for path in generated.iterdir()}, ) - def test_custom_cloud_and_distro_scripts_survive_production_rendering(self): - repository = Path(__file__).resolve().parents[1] - lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines( - keepends=True - ) - _, outer_else, end = hotfix_generate.find_block_boundaries(lines) - traditional = lines[outer_else + 1:end] - selections = ( - {"initAKSCloud"}, - {"initAKSCloud", "provisionSourceUbuntu", "provisionSourceMariner"}, - {"provisionSourceUbuntu", "provisionSourceMariner"}, - ) - for selected in selections: - with self.subTest(selected=selected), tempfile.TemporaryDirectory() as directory: - generated = Path(directory) / "generated" - with mock.patch.object( - hotfix_generate, "GENERATED_DIR", str(generated) - ): - hotfix_generate.write_rendered_payload(selected, traditional) - for platform in ("ubuntu", "mariner"): - for custom in (False, True): - suffix = "_akscustom" if custom else "" - path = ( - generated / f"rendered_nodecustomdata_{platform}{suffix}.yml" - ) - if custom and "initAKSCloud" not in selected: - self.assertFalse(path.exists()) - continue - content = path.read_text() - self.assertNotIn("{{", content) - entries = re.findall(r"- path: ([^\n]+)", content) - self.assertEqual( - int(custom) + int("provisionSourceUbuntu" in selected), - len(entries), - ) - self.assertEqual( - custom, "/opt/azure/containers/init-aks-cloud.sh" in entries - ) - for encoded in re.findall(r"content: !!binary \|\n (\S+)", content): - decoded = gzip.decompress(base64.b64decode(encoded)) - self.assertTrue(decoded.startswith(b"#!/bin/bash\n")) - self.assertGreater(len(decoded), 1000) - def test_write_rendered_payload_preserves_placeholder_without_hotfix(self): with tempfile.TemporaryDirectory() as temp_dir: generated = Path(temp_dir) / "generated" @@ -372,7 +373,7 @@ def test_write_rendered_payload_preserves_previous_hotfix_when_unchanged(self): with tempfile.TemporaryDirectory() as temp_dir: generated = Path(temp_dir) / "generated" generated.mkdir() - platforms = ("ubuntu", "mariner", "ubuntu_akscustom", "mariner_akscustom") + platforms = ("ubuntu", "mariner") for platform in platforms: (generated / f"rendered_nodecustomdata_{platform}.yml").write_text( f"write_files:\n- path: /{platform}-existing\n" @@ -423,39 +424,6 @@ def test_update_nodecustomdata_injects_and_removes_selected_blocks(self): self.assertNotIn(hotfix_generate.SCRIPTS_BEGIN, cleaned) self.assertEqual(1, cleaned.count("provisionSourceUbuntu")) - def test_update_nodecustomdata_does_not_duplicate_custom_cloud_entry(self): - repository = Path(__file__).resolve().parents[1] - lines = (repository / hotfix_generate.TEMPLATE).read_text().splitlines(keepends=True) - _, outer_else, end = hotfix_generate.find_block_boundaries(lines) - duplicate = hotfix_generate.build_hotfix_template( - {"initAKSCloud"}, lines[outer_else + 1:end] - ).removeprefix("#cloud-config\nwrite_files:\n") - # Start with an asset produced by the old generator, including its duplicate. - original = "".join( - lines[:outer_else] - + [f"\n{hotfix_generate.SCRIPTS_BEGIN}\n", duplicate, - f"{hotfix_generate.SCRIPTS_END}\n"] - + lines[outer_else:] - ) - with tempfile.TemporaryDirectory() as directory: - template = Path(directory) / "nodecustomdata.yml" - template.write_text(original) - with mock.patch.object(hotfix_generate, "TEMPLATE", str(template)): - for selected in ( - {"initAKSCloud"}, - {"initAKSCloud", "provisionSource"}, - {"initAKSCloud", "provisionSource"}, - {"initAKSCloud"}, - ): - hotfix_generate.update_nodecustomdata(selected) - content = template.read_text() - self.assertEqual(2, content.count("{{if IsAKSCustomCloud}}")) - self.assertEqual(2, content.count('"initAKSCloud"')) - self.assertEqual( - "provisionSource" in selected, - hotfix_generate.SCRIPTS_BEGIN in content, - ) - def test_write_hotfix_file_uses_hotfixes_and_preserves_scripts_version(self): with tempfile.TemporaryDirectory() as temp_dir: target = Path(temp_dir) / "hotfix.json" diff --git a/hotfix/render-nodecustomdata/main.go b/hotfix/render-nodecustomdata/main.go index ed1e65b903c..3fa4de0657d 100644 --- a/hotfix/render-nodecustomdata/main.go +++ b/hotfix/render-nodecustomdata/main.go @@ -11,20 +11,18 @@ import ( "github.com/Azure/agentbaker/pkg/agent" "github.com/Azure/agentbaker/pkg/agent/datamodel" + "gopkg.in/yaml.v3" ) type platform struct { - name string - distro datamodel.Distro - customCloud bool + name string + distro datamodel.Distro } func main() { platforms := []platform{ {name: "ubuntu", distro: datamodel.AKSUbuntuContainerd2204Gen2}, {name: "mariner", distro: datamodel.AKSAzureLinuxV3Gen2}, - {name: "ubuntu_akscustom", distro: datamodel.AKSUbuntuContainerd2204Gen2, customCloud: true}, - {name: "mariner_akscustom", distro: datamodel.AKSAzureLinuxV3Gen2, customCloud: true}, } templatePath := flag.String("template", "", "path to the hotfix nodecustomdata template") @@ -44,34 +42,49 @@ func main() { fatalf("create output directory: %v", err) } - defaults := make(map[datamodel.Distro]string) + written := 0 for _, target := range platforms { - config := newRenderConfig(target.distro) - if target.customCloud { - config.ContainerService.Properties.CustomCloudEnv = &datamodel.CustomCloudEnv{Name: "akscustom"} - } - rendered, err := agent.RenderLinuxNodeCustomDataTemplate(templateContent, config) + rendered, err := agent.RenderLinuxNodeCustomDataTemplate( + templateContent, + newRenderConfig(target.distro), + ) if err != nil { fatalf("render %s nodecustomdata: %v", target.name, err) } - if target.customCloud && rendered == defaults[target.distro] { - continue - } - defaults[target.distro] = rendered outputPath := filepath.Join( *outputDir, "rendered_nodecustomdata_"+target.name+".yml", ) + var payload struct { + WriteFiles []yaml.Node `yaml:"write_files"` + } + if err := yaml.Unmarshal([]byte(rendered), &payload); err != nil { + fatalf("parse %s nodecustomdata: %v", target.name, err) + } + if len(payload.WriteFiles) == 0 { + fmt.Fprintf(os.Stderr, "Skipping %s nodecustomdata: no write_files entries\n", target.name) + if err := os.Remove(outputPath); err != nil && !os.IsNotExist(err) { + fatalf("remove empty %s nodecustomdata: %v", target.name, err) + } + continue + } //nolint:gosec // The caller intentionally selects the output directory; filenames are fixed by this tool. if err := os.WriteFile(outputPath, []byte(rendered), 0o600); err != nil { fatalf("write %s nodecustomdata: %v", target.name, err) } + written++ + } + if written == 0 { + // go:embed requires at least one file even when every platform is empty. + if err := os.WriteFile(filepath.Join(*outputDir, "README"), []byte("No applicable script hotfix payloads.\n"), 0o600); err != nil { + fatalf("write empty payload placeholder: %v", err) + } } } // newRenderConfig builds the minimal NodeBootstrappingConfiguration required to -// run agent.RenderLinuxNodeCustomDataTemplate. Distro and custom-cloud eligibility -// select the hotfix write_files blocks; the remaining fields (orchestrator +// run agent.RenderLinuxNodeCustomDataTemplate. Only the Distro selector affects +// the hotfix write_files blocks we render; the remaining fields (orchestrator // version, location, FQDN, KubernetesConfig, etc.) are placeholders whose sole // purpose is to satisfy the renderer's unconditional dereferences of // OrchestratorProfile/KubernetesConfig so rendering doesn't nil-panic. Their From 244cadb23edf011c5ca5045ea5bf08131e7f8f7b Mon Sep 17 00:00:00 2001 From: Devin Wong Date: Tue, 20 Oct 2026 16:07:18 -0700 Subject: [PATCH 4/4] fix: avoid directive syntax in embed placeholder comment Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b42051a-f79c-4dbf-8460-a4772d54e01f --- hotfix/render-nodecustomdata/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hotfix/render-nodecustomdata/main.go b/hotfix/render-nodecustomdata/main.go index 3fa4de0657d..9d213fda857 100644 --- a/hotfix/render-nodecustomdata/main.go +++ b/hotfix/render-nodecustomdata/main.go @@ -75,7 +75,7 @@ func main() { written++ } if written == 0 { - // go:embed requires at least one file even when every platform is empty. + // Keep at least one file for embedding when every platform is empty. if err := os.WriteFile(filepath.Join(*outputDir, "README"), []byte("No applicable script hotfix payloads.\n"), 0o600); err != nil { fatalf("write empty payload placeholder: %v", err) }