From 5282cfed39909230f77906fc1fd469d2fb0aec5c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:37:36 +0000 Subject: [PATCH] chore: remove dead function buildArcDindChrootConfigInjectScript The function was only used by its own test (TestArcDindChrootConfigInjection). In production, the patch body is embedded directly inside BuildAWFCommand's arcDindPrefixProbe if-block via buildArcDindChrootConfigPatchBody(), so the standalone wrapper was never called from any main entrypoint. Remove the function, its exclusive test, and clean up dangling references in comments in awf_config.go and awf_helpers.go. Detected by: deadcode ./cmd/... ./internal/tools/... Run ID: 27559409182 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/workflow/awf_config.go | 3 +- pkg/workflow/awf_helpers.go | 14 ---- pkg/workflow/awf_helpers_test.go | 130 ------------------------------- 3 files changed, 1 insertion(+), 146 deletions(-) diff --git a/pkg/workflow/awf_config.go b/pkg/workflow/awf_config.go index 04d7cce739d..b299980686b 100644 --- a/pkg/workflow/awf_config.go +++ b/pkg/workflow/awf_config.go @@ -167,8 +167,7 @@ type AWFConfigFile struct { Container *AWFContainerConfig `json:"container,omitempty"` // Chroot contains chroot execution overrides for split-filesystem ARC/DinD runners. - // This field is not populated at compile time; it is injected at runtime by - // buildArcDindChrootConfigInjectScript when DinD topology is detected. + // This field is not populated at compile time; it is injected at runtime when DinD topology is detected. Chroot *AWFChrootConfig `json:"chroot,omitempty"` } diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index a0420e0afb3..e438367440e 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -956,7 +956,6 @@ func awfSupportsChrootConfig(firewallConfig *FirewallConfig) bool { // buildArcDindChrootConfigPatchBody returns the Python heredoc that patches the AWF // config file with chroot.binariesSourcePath and chroot.identity.*. It is designed to be // embedded inside a bash if-block that already guards on DOCKER_HOST=tcp://... -// (see buildArcDindChrootConfigInjectScript for standalone use and tests). // // The Python is intentionally kept compact to minimise script size and stay within // GitHub Actions' 21 KB per-step expression limit. @@ -977,16 +976,3 @@ except Exception as e: raise SystemExit(f"chroot config patch failed: {e}") from e PY`, awfArcDindChrootBinariesSourcePath, awfArcDindChrootIdentityHome, awfArcDindChrootBinariesSourcePath) } - -// buildArcDindChrootConfigInjectScript returns a standalone bash+Python script that -// patches the AWF config file with chroot.binariesSourcePath and chroot.identity.* when the -// runner is in an ARC/DinD topology (detected via DOCKER_HOST=tcp://...). -// -// This standalone form is used by tests. In production, the patch body is embedded -// inside the arcDindPrefixProbe if-block (see BuildAWFCommand) so there is only one -// DOCKER_HOST condition check in the generated script. -func buildArcDindChrootConfigInjectScript() string { - return fmt.Sprintf(`if [[ "${DOCKER_HOST:-}" =~ %s ]]; then -%s -fi`, awfArcDindDockerHostRegex, buildArcDindChrootConfigPatchBody()) -} diff --git a/pkg/workflow/awf_helpers_test.go b/pkg/workflow/awf_helpers_test.go index d0bcfdc1722..7cb54aee689 100644 --- a/pkg/workflow/awf_helpers_test.go +++ b/pkg/workflow/awf_helpers_test.go @@ -3,10 +3,7 @@ package workflow import ( - "encoding/json" - "errors" "fmt" - "os" "os/exec" "strconv" "strings" @@ -1463,133 +1460,6 @@ func TestAWFSupportsChrootConfig(t *testing.T) { } } -// TestArcDindChrootConfigInjection exercises the Python script generated by -// buildArcDindChrootConfigInjectScript. It writes a minimal AWF config JSON, -// runs the script with a controlled DOCKER_HOST, and verifies that the chroot -// section is added with the expected static paths and runtime identity values. -func TestArcDindChrootConfigInjection(t *testing.T) { - if _, err := exec.LookPath("python3"); err != nil { - t.Skip("python3 not available") - } - if _, err := exec.LookPath("id"); err != nil { - t.Skip("id command not available") - } - - tests := []struct { - name string - dockerHost string - wantChroot bool - }{ - {"tcp://localhost:2375 triggers injection", "tcp://localhost:2375", true}, - {"tcp://dind:2375 triggers injection", "tcp://dind:2375", true}, - {"tcp://172.30.0.5:2376 triggers injection", "tcp://172.30.0.5:2376", true}, - {"unix socket skips injection", "unix:///var/run/docker.sock", false}, - {"bare path skips injection", "/var/run/docker.sock", false}, - {"empty DOCKER_HOST skips injection", "", false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tmpDir := t.TempDir() - // The Python script reads from ${RUNNER_TEMP}/gh-aw/awf-config.json, - // so create the gh-aw subdirectory and write the config there. - ghAwDir := tmpDir + "/gh-aw" - configPath := ghAwDir + "/awf-config.json" - - // Write a minimal AWF config file (simulating the printf step). - // Use json.Marshal of a real AWFConfigFile to stay in sync with the struct. - minimalConfig := &AWFConfigFile{ - APIProxy: &AWFAPIProxyConfig{ - Enabled: true, - MaxRuns: 100, - }, - } - initialConfigBytes, err := json.Marshal(minimalConfig) - require.NoError(t, err, "failed to marshal minimal AWF config") - initialConfig := string(initialConfigBytes) - writeScript := fmt.Sprintf(`mkdir -p %s && printf '%%s\n' '%s' > %s`, ghAwDir, initialConfig, configPath) - - script := buildArcDindChrootConfigInjectScript() - - // The Python patch also writes to awfArcDindChrootBinariesSourcePath/awf-config.json - // (/tmp/gh-aw/awf-config.json). Pre-create that directory so the write succeeds, - // and schedule cleanup of the file after the test. - if tt.wantChroot { - if err := os.MkdirAll(awfArcDindChrootBinariesSourcePath, 0o755); err != nil { - t.Skipf("cannot create %s: %v", awfArcDindChrootBinariesSourcePath, err) - } - t.Cleanup(func() { os.Remove(awfArcDindChrootBinariesSourcePath + "/awf-config.json") }) - } - - fullScript := fmt.Sprintf(`#!/bin/bash -export RUNNER_TEMP=%q -export DOCKER_HOST=%q -%s -%s -cat %s -`, tmpDir, tt.dockerHost, writeScript, script, configPath) - - cmd := exec.Command("bash", "-c", fullScript) - out, err := cmd.Output() - var stderrMsg string - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - stderrMsg = string(exitErr.Stderr) - } - require.NoError(t, err, "script should succeed (stderr: %s)", stderrMsg) - - var result map[string]any - require.NoError(t, json.Unmarshal(out, &result), "output must be valid JSON: %s", string(out)) - - if !tt.wantChroot { - assert.NotContains(t, string(out), `"chroot"`, - "chroot section should NOT be injected for DOCKER_HOST=%s", tt.dockerHost) - return - } - - require.Contains(t, string(out), `"chroot"`, - "chroot section should be injected for DOCKER_HOST=%s", tt.dockerHost) - - // Verify /tmp/gh-aw/awf-config.json was also written (matches the runner_temp copy). - tmpConfigBytes, readErr := os.ReadFile(awfArcDindChrootBinariesSourcePath + "/awf-config.json") - require.NoError(t, readErr, "%s/awf-config.json should have been written by inject script", awfArcDindChrootBinariesSourcePath) - assert.Equal(t, string(out), string(tmpConfigBytes), - "%s/awf-config.json should match %s/gh-aw/awf-config.json", awfArcDindChrootBinariesSourcePath, tmpDir) - - chrootRaw, ok := result["chroot"].(map[string]any) - require.True(t, ok, "chroot must be an object") - - // Verify binariesSourcePath is the expected constant. - assert.Equal(t, awfArcDindChrootBinariesSourcePath, chrootRaw["binariesSourcePath"], - "binariesSourcePath should be %s", awfArcDindChrootBinariesSourcePath) - - // Verify identity section is present. - identityRaw, ok := chrootRaw["identity"].(map[string]any) - require.True(t, ok, "chroot.identity must be an object") - - // Verify home is the expected constant. - assert.Equal(t, awfArcDindChrootIdentityHome, identityRaw["home"], - "identity.home should be %s", awfArcDindChrootIdentityHome) - - // Verify user is a non-empty string. - user, ok := identityRaw["user"].(string) - assert.True(t, ok && user != "", "identity.user should be a non-empty string") - - // Verify uid/gid are positive numbers. - uid, ok := identityRaw["uid"].(float64) - assert.True(t, ok && uid > 0, "identity.uid should be a positive number, got %v", identityRaw["uid"]) - - gid, ok := identityRaw["gid"].(float64) - assert.True(t, ok && gid > 0, "identity.gid should be a positive number, got %v", identityRaw["gid"]) - - // Verify original config fields survived the patch (round-trip correctness). - apiProxy, ok := result["apiProxy"].(map[string]any) - require.True(t, ok, "apiProxy section must be preserved after chroot injection") - assert.Equal(t, true, apiProxy["enabled"], "apiProxy.enabled should be unchanged after chroot injection") - }) - } -} - // TestBuildAWFCommand_IncludesChrootInjectScript verifies that BuildAWFCommand // includes the chroot injection script in the generated run step when the AWF // version supports it.