From 35a6bd418c48eabf7ca213cd0f421d23357c5b80 Mon Sep 17 00:00:00 2001 From: Nishchay Date: Wed, 28 Oct 2026 14:41:50 -0700 Subject: [PATCH 1/2] test: add minification verification test --- .github/workflows/go-test.yml | 10 ++ .../script_minification_artifacts_test.go | 94 +++++++++++++++++++ vhdbuilder/packer/README.md | 17 ++++ 3 files changed, 121 insertions(+) create mode 100644 pkg/agent/script_minification_artifacts_test.go diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 248c9cb084c..ee175c7a279 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -25,6 +25,8 @@ jobs: with: go-version: '1.27' - name: Run unit tests for go code in the repository (with coverage) + env: + CSE_SCRIPT_DIFF_DIR: ${{ runner.temp }}/cse-minification-diffs # Single instrumented pass instead of `make test` followed by a second coverage run: # this builds and tests each module once, mirroring what `make test` does, while also # producing coverage profiles. @@ -37,6 +39,14 @@ jobs: (cd aks-node-controller && go build -mod=readonly ./... && go test ./... -coverprofile=../coverage/aks-node-controller.out -covermode=atomic) (cd aks-live-patching && go build -mod=readonly ./... && go test ./... -coverprofile=../coverage/aks-live-patching.out -covermode=atomic) (cd vhdbuilder/cosi-uploader && go build -mod=readonly ./... && go test ./... -coverprofile=../../coverage/cosi-uploader.out -covermode=atomic) + - name: Upload CSE minification diffs + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + if: (!cancelled()) + with: + name: cse-minification-diffs + path: ${{ runner.temp }}/cse-minification-diffs + if-no-files-found: error + retention-days: 5 - name: Strip generated/mock code from coverage profiles run: | set -ex diff --git a/pkg/agent/script_minification_artifacts_test.go b/pkg/agent/script_minification_artifacts_test.go new file mode 100644 index 00000000000..b8897482196 --- /dev/null +++ b/pkg/agent/script_minification_artifacts_test.go @@ -0,0 +1,94 @@ +package agent + +import ( + "bytes" + "encoding/base64" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "text/template" + + "github.com/Azure/agentbaker/parts" + "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/stretchr/testify/require" +) + +func TestCSEScriptMinificationArtifacts(t *testing.T) { + outputDir := os.Getenv("CSE_SCRIPT_DIFF_DIR") + if outputDir == "" { + t.Skip("set CSE_SCRIPT_DIFF_DIR to export rendered and minified script diffs") + } + require.True(t, filepath.IsAbs(outputDir), "CSE_SCRIPT_DIFF_DIR must be an absolute path") + scripts := discoverCSEScripts(t) + require.NotEmpty(t, scripts) + + for _, distro := range []datamodel.Distro{ + datamodel.AKSUbuntuContainerd2204Gen2, + datamodel.AKSUbuntuContainerd2404Gen2, + datamodel.AKSUbuntuMinimalContainerd2604Gen2, + datamodel.AKSCBLMarinerV2Gen2, + datamodel.AKSAzureLinuxV2Gen2, + datamodel.AKSAzureLinuxV3Gen2, + } { + for _, cloud := range []string{"public", "custom"} { + t.Run(string(distro)+"/"+cloud, func(t *testing.T) { + config := newNodeCustomDataRenderConfig(distro) + config.EnableKubeletConfigFile = true + config.AgentPoolProfile.LocalDNSProfile = &datamodel.LocalDNSProfile{EnableLocalDNS: true} + if cloud == "custom" { + config.ContainerService.Properties.CustomCloudEnv = &datamodel.CustomCloudEnv{Name: "akscustom"} + } + dir := filepath.Join(outputDir, string(distro), cloud) + for _, script := range scripts { + t.Run(script, func(t *testing.T) { + // Mirror production preprocessing without using the shell parser/printer + // on the baseline: otherwise normalization could hide minifier changes. + raw, err := parts.Templates.ReadFile(script) + require.NoError(t, err) + templ, err := template.New("ContainerService template"). + Option("missingkey=error"). + Funcs(getContainerServiceFuncMap(config)). + Parse(string(removeComments(raw))) + require.NoError(t, err) + var rendered bytes.Buffer + require.NoError(t, templ.Execute(&rendered, config.ContainerService)) + before := []byte(strings.ReplaceAll(rendered.String(), "\r\n", "\n")) + beforePath := filepath.Join("before", filepath.FromSlash(script)) + writeMinificationArtifact(t, filepath.Join(dir, beforePath), before) + + encoded := getBase64EncodedGzippedCustomScript(script, config) + compressed, err := base64.StdEncoding.Strict().DecodeString(encoded) + require.NoError(t, err) + after, err := getGzipDecodedValue(compressed) + require.NoError(t, err) + afterPath := filepath.Join("after", filepath.FromSlash(script)) + writeMinificationArtifact(t, filepath.Join(dir, afterPath), after) + + cmd := exec.Command("git", "--no-pager", "diff", "--no-index", + "--no-ext-diff", "--no-textconv", "--no-color", "--", beforePath, afterPath) + cmd.Dir = dir + diff, err := cmd.CombinedOutput() + if err != nil { + var exitErr *exec.ExitError + require.True(t, errors.As(err, &exitErr), "git diff: %v\n%s", err, diff) + require.Equal(t, 1, exitErr.ExitCode(), "git diff: %s", diff) + } + writeMinificationArtifact(t, filepath.Join(dir, "diff", filepath.FromSlash(script)+".diff"), diff) + cseValidateBashSyntax(t, script, before) + cseValidateBashSyntax(t, script, after) + }) + } + }) + } + } + t.Logf("script diffs written to %s", outputDir) +} + +func writeMinificationArtifact(t *testing.T, path string, content []byte) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0700)) + require.NoError(t, os.WriteFile(path, content, 0600)) +} diff --git a/vhdbuilder/packer/README.md b/vhdbuilder/packer/README.md index 6686ef9be1c..98cb19845ec 100644 --- a/vhdbuilder/packer/README.md +++ b/vhdbuilder/packer/README.md @@ -145,6 +145,23 @@ Rendered shell payloads are minified with the `mvdan.cc/sh/v3/syntax` Bash parse before gzip and base64 encoding. Trailing `#HELPERSEOF` and `#EOF` delivery markers are preserved. VHD source scripts and non-shell payloads are not minified. +The Go unit-test workflow uploads `cse-minification-diffs` for review. To generate +the same artifacts locally, run from the repository root with an absolute output +directory: + +```bash +CSE_SCRIPT_DIFF_DIR="$(mktemp -d)" \ + go test ./pkg/agent -run '^TestCSEScriptMinificationArtifacts$' -count=1 -v +``` + +Each OS/cloud fixture contains `before/` (after comment stripping and template +rendering), `after/` (decoded production payload scripts), and `diff/` (unmodified +Git diffs). The baseline is not reformatted with the minifier. These synthetic +fixtures cover six OS variants, public/custom cloud, and LocalDNS enabled; all +discovered scripts are exported, including ones not delivered by that fixture's +OS/cloud. The test checks Bash syntax without executing provisioning scripts. +Artifact inspection is not proof of semantic equivalence or dependency safety. + When adding a module, also register its source-to-variable mapping in `hotfix/hotfix_generate.py`. The current keys are `provisionConfigsGPU`, `provisionConfigsLocalDNS`, `provisionConfigsKubelet`, `provisionConfigsNetwork`, From 2b3b9f336e2314586263ea926a686880ddf8442b Mon Sep 17 00:00:00 2001 From: cameronmeissner Date: Wed, 28 Oct 2026 16:13:28 -0700 Subject: [PATCH 2/2] chore: harden tests --- .../script_minification_artifacts_test.go | 201 ++++++++++++++++-- 1 file changed, 180 insertions(+), 21 deletions(-) diff --git a/pkg/agent/script_minification_artifacts_test.go b/pkg/agent/script_minification_artifacts_test.go index b8897482196..02a5336d99a 100644 --- a/pkg/agent/script_minification_artifacts_test.go +++ b/pkg/agent/script_minification_artifacts_test.go @@ -13,15 +13,17 @@ import ( "github.com/Azure/agentbaker/parts" "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" + "mvdan.cc/sh/v3/syntax" ) func TestCSEScriptMinificationArtifacts(t *testing.T) { outputDir := os.Getenv("CSE_SCRIPT_DIFF_DIR") - if outputDir == "" { - t.Skip("set CSE_SCRIPT_DIFF_DIR to export rendered and minified script diffs") + if outputDir != "" { + require.True(t, filepath.IsAbs(outputDir), "CSE_SCRIPT_DIFF_DIR must be an absolute path") } - require.True(t, filepath.IsAbs(outputDir), "CSE_SCRIPT_DIFF_DIR must be an absolute path") scripts := discoverCSEScripts(t) require.NotEmpty(t, scripts) @@ -44,8 +46,8 @@ func TestCSEScriptMinificationArtifacts(t *testing.T) { dir := filepath.Join(outputDir, string(distro), cloud) for _, script := range scripts { t.Run(script, func(t *testing.T) { - // Mirror production preprocessing without using the shell parser/printer - // on the baseline: otherwise normalization could hide minifier changes. + // Existing comment removal is part of the accepted baseline. Render + // without the production minifier or encoder so their changes remain visible. raw, err := parts.Templates.ReadFile(script) require.NoError(t, err) templ, err := template.New("ContainerService template"). @@ -56,27 +58,16 @@ func TestCSEScriptMinificationArtifacts(t *testing.T) { var rendered bytes.Buffer require.NoError(t, templ.Execute(&rendered, config.ContainerService)) before := []byte(strings.ReplaceAll(rendered.String(), "\r\n", "\n")) - beforePath := filepath.Join("before", filepath.FromSlash(script)) - writeMinificationArtifact(t, filepath.Join(dir, beforePath), before) - encoded := getBase64EncodedGzippedCustomScript(script, config) compressed, err := base64.StdEncoding.Strict().DecodeString(encoded) require.NoError(t, err) after, err := getGzipDecodedValue(compressed) require.NoError(t, err) - afterPath := filepath.Join("after", filepath.FromSlash(script)) - writeMinificationArtifact(t, filepath.Join(dir, afterPath), after) - - cmd := exec.Command("git", "--no-pager", "diff", "--no-index", - "--no-ext-diff", "--no-textconv", "--no-color", "--", beforePath, afterPath) - cmd.Dir = dir - diff, err := cmd.CombinedOutput() - if err != nil { - var exitErr *exec.ExitError - require.True(t, errors.As(err, &exitErr), "git diff: %v\n%s", err, diff) - require.Equal(t, 1, exitErr.ExitCode(), "git diff: %s", diff) + if outputDir != "" { + writeMinificationDiff(t, dir, script, before, after) } - writeMinificationArtifact(t, filepath.Join(dir, "diff", filepath.FromSlash(script)+".diff"), diff) + require.Empty(t, shellCodeDiff(t, script, before, after), + "production rendering changed shell code (-source +production)") cseValidateBashSyntax(t, script, before) cseValidateBashSyntax(t, script, after) }) @@ -84,7 +75,175 @@ func TestCSEScriptMinificationArtifacts(t *testing.T) { }) } } - t.Logf("script diffs written to %s", outputDir) + if outputDir != "" { + t.Logf("script diffs written to %s", outputDir) + } +} + +func shellCodeDiff(t *testing.T, name string, before, after []byte) string { + t.Helper() + type shellCode struct { + Shebang string + File *syntax.File + } + parse := func(script []byte) shellCode { + t.Helper() + // Discard comments, but retain literal content (including heredocs). + file, err := syntax.NewParser(syntax.Variant(syntax.LangBash)).Parse(bytes.NewReader(script), name) + require.NoError(t, err) + code := shellCode{File: file} + if bytes.HasPrefix(script, []byte("#!")) { + code.Shebang, _, _ = strings.Cut(string(script), "\n") + } + return code + } + return cmp.Diff(parse(before), parse(after), + cmpopts.IgnoreTypes(syntax.Pos{}), + // Minification may spell ${name} as $name; the parsed parameter and + // all expansion operators must still match. + cmpopts.IgnoreFields(syntax.ParamExp{}, "Short"), + ) +} + +func TestShellCodeDiff(t *testing.T) { + tests := []struct { + name string + before string + after string + equal bool + }{ + { + name: "comments whitespace and statement separators", + before: "#!/bin/bash\n# comment\n\nif true; then\n echo ok # trailing comment\nfi\n", + after: "#!/bin/bash\nif true;then echo ok;fi\n", + equal: true, + }, + { + name: "short parameter expansion", + before: `echo "${value}"` + "\n", + after: `echo "$value"` + "\n", + equal: true, + }, + { + name: "preserved heredoc and multiline literal", + before: "# comment\ncat <<'EOF'\n# literal\n content\n\nEOF\n\n echo 'first\n\nlast'\n", + after: "cat <<'EOF'\n# literal\n content\n\nEOF\necho 'first\n\nlast'\n", + equal: true, + }, + { + name: "changed parameter boundary", + before: "echo ${value}suffix\n", + after: "echo $valuesuffix\n", + }, + { + name: "changed positional parameter", + before: "echo ${10}\n", + after: "echo $10\n", + }, + { + name: "added command", + before: "echo ok\n", + after: "echo ok; unexpected_command\n", + }, + { + name: "removed command", + before: "check_configuration\necho ok\n", + after: "echo ok\n", + }, + { + name: "changed argument", + before: "echo ok\n", + after: "echo unexpected\n", + }, + { + name: "injected command substitution", + before: `echo "ok"` + "\n", + after: `echo "$(unexpected_command)"` + "\n", + }, + { + name: "injected process substitution", + before: "cat input\n", + after: "cat <(unexpected_command)\n", + }, + { + name: "changed control flow", + before: "check_configuration && start_service\n", + after: "check_configuration; start_service\n", + }, + { + name: "background command", + before: "check_configuration; start_service\n", + after: "check_configuration & start_service\n", + }, + { + name: "changed redirect", + before: "echo ok > output\n", + after: "echo ok >> output\n", + }, + { + name: "changed function body", + before: "setup() { echo ok; }\n", + after: "setup() { echo ok; unexpected_command; }\n", + }, + { + name: "removed quoting", + before: `echo "$value"` + "\n", + after: "echo $value\n", + }, + { + name: "changed whitespace inside literal", + before: "echo 'a b'\n", + after: "echo 'a b'\n", + }, + { + name: "removed comment-like literal content", + before: "echo 'first\n# literal\nlast'\n", + after: "echo 'first\nlast'\n", + }, + { + name: "changed heredoc payload", + before: "cat <<'EOF'\n# literal\n content\n\nEOF\n", + after: "cat <<'EOF'\ncontent\nEOF\n", + }, + { + name: "enabled heredoc expansion", + before: "cat <<'EOF'\n$variable\nEOF\n", + after: "cat <