test: add coverage for removeComments CSE stripping logic by djsly · Pull Request #8489 · Azure/AgentBaker

djsly · 2026-05-11T14:45:31Z

Summary

Add Go unit tests that validate all CSE shell scripts survive the removeComments → gzip → base64 → decode → gunzip pipeline without breaking bash syntax.

Background

PR #8475 (DirtyFrag kernel module blacklist) was dead on arrival because removeComments in pkg/agent/utils.go stripped a # description line from inside a printf format string, breaking the script. PR #8486 fixed the immediate issue, but there was no test to prevent this class of regression.

What this PR adds

`TestRemoveComments_ShellPatterns` — 6 table-driven subtests

Pure comment removal
Hash inside quoted grep pattern (with # in input)
Trailing comment trimming
Shebang preservation
Hash in variable expansions (${#array[@]})
DOA regression pattern: documents that # inside string literals IS stripped (the root cause)

`TestCSEScriptRoundTrip` — 34 CSE pipeline scripts

Script list derived from getBase64EncodedGzippedCustomScript() calls in variables.go
Full pipeline: removeComments → gzip → base64 → decode → gunzip
Byte-for-byte round-trip integrity check
bash -n syntax validation on each decoded script (with extglob enabled)
Scripts with Go template directives ({{}}) skip bash -n (not valid bash pre-execution)

Add unit tests and integration test for the removeComments function that strips comments from shell scripts during CSE assembly. The existing tests only covered simple comment patterns and did not exercise realistic CSE script content. New coverage includes: - disableVulnerableKernelModule function body survives stripping - Function call sites are preserved after stripping - grep patterns with hash characters are not mistaken for comments - Hash in variable expansions (${#array[@]}) is preserved - Documents the known limitation where comment-like lines inside string literals get stripped (root cause of DirtyFrag DOA bug PR #8475) - Integration test reads actual cse_main.sh and validates critical patterns survive removeComments intact AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds additional Go test coverage around removeComments (used during Linux CSE assembly) to prevent regressions where comment-stripping heuristics accidentally remove functionally significant shell content (notably around the DirtyFrag/CVE kernel module blacklist mitigation added recently in cse_main.sh).

Changes:

Adds table-driven unit tests covering several shell-script patterns that removeComments must handle safely.
Adds an integration-style test that loads parts/linux/cloud-init/artifacts/cse_main.sh, runs removeComments, and validates key disableVulnerableKernelModule patterns and call sites survive stripping.
Introduces a repoRoot() helper to locate cse_main.sh without hardcoded absolute paths.

Replace the removeComments-only integration test with comprehensive full-pipeline round-trip tests that exercise the exact same path used in production: removeComments → gzip → base64 → decode → gunzip. TestCSEScriptRoundTrip validates all 6 embedded CSE shell scripts: - Byte-for-byte round-trip integrity (decoded == stripped input) - bash -n syntax check on decoded output (skipped for cse_cmd.sh which contains Go template directives pre-execution) TestCSEScriptRoundTrip_CSEMainShCriticalContent validates cse_main.sh critical content survives the full pipeline (function declaration, call sites, printf patterns, modprobe commands, grep patterns, and no orphaned install/blacklist lines). This would have caught the PR #8475 DOA bug where removeComments broke a printf format string, producing invalid bash that only failed at runtime on the VM. AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Dynamically discover all .sh files in artifacts/ (55+ scripts, including subdirs) - Full pipeline: removeComments → gzip → base64 → decode → gunzip → bash -n - Remove DirtyFrag-specific content assertions — the bash -n check is sufficient - Enable extglob for syntax check (scripts use @() patterns) - Document known removeComments limitation for aks-localdns-hosts-setup.sh AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

- Extract cseRoundTrip and cseValidateBashSyntax helpers to reduce cognitive complexity (gocognit: 38 → under 20) - Fix err shadow (govet) by using writeErr variable - Fix goimports formatting - Rename 'grep with hash pattern' test to clarify intent AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

The aks-localdns-hosts-setup.sh fix is being addressed in a separate PR. This test should have zero skip exceptions. AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use authoritative script list from getBase64EncodedGzippedCustomScript() calls in variables.go instead of walking all .sh files. Only scripts that flow through removeComments in production are tested — no skip lists needed. AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Add test documenting known limitation: '# ' inside string literals gets stripped by removeComments (the PR #8475 DOA root cause) - Fix grep test to actually include '#' in the pattern AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Replace hardcoded script list with discoverCSEScripts() that parses variables.go for getBase64EncodedGzippedCustomScript() calls, resolves constant names from const.go, and filters to .sh files. New scripts added to the CSE pipeline are automatically covered. AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 11, 2026 14:45

djsly requested review from AbelHu, Devinwong, SriHarsha001, awesomenix, calvin197, cameronmeissner, ganeshkumarashok, junjiezhang1997, lilypan26, mxj220, pdamianov-dev, phealy, r2k1, sulixu, surajssd, timmy-wright and zachary-bailey as code owners May 11, 2026 14:45

djsly temporarily deployed to test May 11, 2026 14:45 — with GitHub Actions Inactive

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go Outdated

djsly temporarily deployed to test May 11, 2026 14:56 — with GitHub Actions Inactive

Copilot AI review requested due to automatic review settings May 11, 2026 15:04

djsly temporarily deployed to test May 11, 2026 15:04 — with GitHub Actions Inactive

Copilot started reviewing on behalf of djsly May 11, 2026 15:06 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go Outdated

djsly temporarily deployed to test May 11, 2026 15:26 — with GitHub Actions Inactive

fix: clarify test comment — not full pipeline, just stripping + encoding

6e17a53

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 11, 2026 15:42

djsly temporarily deployed to test May 11, 2026 15:42 — with GitHub Actions Inactive

Copilot started reviewing on behalf of djsly May 11, 2026 15:43 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread pkg/agent/utils_test.go

Comment thread pkg/agent/utils_test.go Outdated

Comment thread pkg/agent/utils_test.go

Comment thread pkg/agent/utils_test.go Outdated

fix: remove knownStripBroken skip list — no exceptions

925209e

The aks-localdns-hosts-setup.sh fix is being addressed in a separate PR. This test should have zero skip exceptions. AB#37912827 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

djsly temporarily deployed to test May 11, 2026 16:05 — with GitHub Actions Inactive

djsly mentioned this pull request May 11, 2026

fix: use variable expansion for comment prefix in aks-localdns-hosts-setup.sh #8490

Closed

Copilot AI review requested due to automatic review settings May 11, 2026 16:16

djsly temporarily deployed to test May 11, 2026 16:16 — with GitHub Actions Inactive

Copilot started reviewing on behalf of djsly May 11, 2026 16:17 View session

djsly temporarily deployed to test May 11, 2026 16:21 — with GitHub Actions Inactive

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread pkg/agent/utils_test.go

Comment thread pkg/agent/utils_test.go

Comment thread pkg/agent/utils_test.go

Comment thread pkg/agent/utils_test.go

djsly temporarily deployed to test May 11, 2026 16:29 — with GitHub Actions Inactive

cameronmeissner approved these changes May 11, 2026

View reviewed changes

cameronmeissner merged commit 18560af into main May 11, 2026
23 checks passed

cameronmeissner deleted the djsly/37912827 branch May 11, 2026 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add coverage for removeComments CSE stripping logic#8489

test: add coverage for removeComments CSE stripping logic#8489
cameronmeissner merged 9 commits into
mainfrom
djsly/37912827

djsly commented May 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

djsly commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

What this PR adds

TestRemoveComments_ShellPatterns — 6 table-driven subtests

TestCSEScriptRoundTrip — 34 CSE pipeline scripts

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

djsly commented May 11, 2026 •

edited

Loading

`TestRemoveComments_ShellPatterns` — 6 table-driven subtests

`TestCSEScriptRoundTrip` — 34 CSE pipeline scripts