Skip to content

Commit 5282cfe

Browse files
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>
1 parent cb89a18 commit 5282cfe

3 files changed

Lines changed: 1 addition & 146 deletions

File tree

pkg/workflow/awf_config.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,7 @@ type AWFConfigFile struct {
167167
Container *AWFContainerConfig `json:"container,omitempty"`
168168

169169
// Chroot contains chroot execution overrides for split-filesystem ARC/DinD runners.
170-
// This field is not populated at compile time; it is injected at runtime by
171-
// buildArcDindChrootConfigInjectScript when DinD topology is detected.
170+
// This field is not populated at compile time; it is injected at runtime when DinD topology is detected.
172171
Chroot *AWFChrootConfig `json:"chroot,omitempty"`
173172
}
174173

pkg/workflow/awf_helpers.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,6 @@ func awfSupportsChrootConfig(firewallConfig *FirewallConfig) bool {
956956
// buildArcDindChrootConfigPatchBody returns the Python heredoc that patches the AWF
957957
// config file with chroot.binariesSourcePath and chroot.identity.*. It is designed to be
958958
// embedded inside a bash if-block that already guards on DOCKER_HOST=tcp://...
959-
// (see buildArcDindChrootConfigInjectScript for standalone use and tests).
960959
//
961960
// The Python is intentionally kept compact to minimise script size and stay within
962961
// GitHub Actions' 21 KB per-step expression limit.
@@ -977,16 +976,3 @@ except Exception as e:
977976
raise SystemExit(f"chroot config patch failed: {e}") from e
978977
PY`, awfArcDindChrootBinariesSourcePath, awfArcDindChrootIdentityHome, awfArcDindChrootBinariesSourcePath)
979978
}
980-
981-
// buildArcDindChrootConfigInjectScript returns a standalone bash+Python script that
982-
// patches the AWF config file with chroot.binariesSourcePath and chroot.identity.* when the
983-
// runner is in an ARC/DinD topology (detected via DOCKER_HOST=tcp://...).
984-
//
985-
// This standalone form is used by tests. In production, the patch body is embedded
986-
// inside the arcDindPrefixProbe if-block (see BuildAWFCommand) so there is only one
987-
// DOCKER_HOST condition check in the generated script.
988-
func buildArcDindChrootConfigInjectScript() string {
989-
return fmt.Sprintf(`if [[ "${DOCKER_HOST:-}" =~ %s ]]; then
990-
%s
991-
fi`, awfArcDindDockerHostRegex, buildArcDindChrootConfigPatchBody())
992-
}

pkg/workflow/awf_helpers_test.go

Lines changed: 0 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33
package workflow
44

55
import (
6-
"encoding/json"
7-
"errors"
86
"fmt"
9-
"os"
107
"os/exec"
118
"strconv"
129
"strings"
@@ -1463,133 +1460,6 @@ func TestAWFSupportsChrootConfig(t *testing.T) {
14631460
}
14641461
}
14651462

1466-
// TestArcDindChrootConfigInjection exercises the Python script generated by
1467-
// buildArcDindChrootConfigInjectScript. It writes a minimal AWF config JSON,
1468-
// runs the script with a controlled DOCKER_HOST, and verifies that the chroot
1469-
// section is added with the expected static paths and runtime identity values.
1470-
func TestArcDindChrootConfigInjection(t *testing.T) {
1471-
if _, err := exec.LookPath("python3"); err != nil {
1472-
t.Skip("python3 not available")
1473-
}
1474-
if _, err := exec.LookPath("id"); err != nil {
1475-
t.Skip("id command not available")
1476-
}
1477-
1478-
tests := []struct {
1479-
name string
1480-
dockerHost string
1481-
wantChroot bool
1482-
}{
1483-
{"tcp://localhost:2375 triggers injection", "tcp://localhost:2375", true},
1484-
{"tcp://dind:2375 triggers injection", "tcp://dind:2375", true},
1485-
{"tcp://172.30.0.5:2376 triggers injection", "tcp://172.30.0.5:2376", true},
1486-
{"unix socket skips injection", "unix:///var/run/docker.sock", false},
1487-
{"bare path skips injection", "/var/run/docker.sock", false},
1488-
{"empty DOCKER_HOST skips injection", "", false},
1489-
}
1490-
1491-
for _, tt := range tests {
1492-
t.Run(tt.name, func(t *testing.T) {
1493-
tmpDir := t.TempDir()
1494-
// The Python script reads from ${RUNNER_TEMP}/gh-aw/awf-config.json,
1495-
// so create the gh-aw subdirectory and write the config there.
1496-
ghAwDir := tmpDir + "/gh-aw"
1497-
configPath := ghAwDir + "/awf-config.json"
1498-
1499-
// Write a minimal AWF config file (simulating the printf step).
1500-
// Use json.Marshal of a real AWFConfigFile to stay in sync with the struct.
1501-
minimalConfig := &AWFConfigFile{
1502-
APIProxy: &AWFAPIProxyConfig{
1503-
Enabled: true,
1504-
MaxRuns: 100,
1505-
},
1506-
}
1507-
initialConfigBytes, err := json.Marshal(minimalConfig)
1508-
require.NoError(t, err, "failed to marshal minimal AWF config")
1509-
initialConfig := string(initialConfigBytes)
1510-
writeScript := fmt.Sprintf(`mkdir -p %s && printf '%%s\n' '%s' > %s`, ghAwDir, initialConfig, configPath)
1511-
1512-
script := buildArcDindChrootConfigInjectScript()
1513-
1514-
// The Python patch also writes to awfArcDindChrootBinariesSourcePath/awf-config.json
1515-
// (/tmp/gh-aw/awf-config.json). Pre-create that directory so the write succeeds,
1516-
// and schedule cleanup of the file after the test.
1517-
if tt.wantChroot {
1518-
if err := os.MkdirAll(awfArcDindChrootBinariesSourcePath, 0o755); err != nil {
1519-
t.Skipf("cannot create %s: %v", awfArcDindChrootBinariesSourcePath, err)
1520-
}
1521-
t.Cleanup(func() { os.Remove(awfArcDindChrootBinariesSourcePath + "/awf-config.json") })
1522-
}
1523-
1524-
fullScript := fmt.Sprintf(`#!/bin/bash
1525-
export RUNNER_TEMP=%q
1526-
export DOCKER_HOST=%q
1527-
%s
1528-
%s
1529-
cat %s
1530-
`, tmpDir, tt.dockerHost, writeScript, script, configPath)
1531-
1532-
cmd := exec.Command("bash", "-c", fullScript)
1533-
out, err := cmd.Output()
1534-
var stderrMsg string
1535-
var exitErr *exec.ExitError
1536-
if errors.As(err, &exitErr) {
1537-
stderrMsg = string(exitErr.Stderr)
1538-
}
1539-
require.NoError(t, err, "script should succeed (stderr: %s)", stderrMsg)
1540-
1541-
var result map[string]any
1542-
require.NoError(t, json.Unmarshal(out, &result), "output must be valid JSON: %s", string(out))
1543-
1544-
if !tt.wantChroot {
1545-
assert.NotContains(t, string(out), `"chroot"`,
1546-
"chroot section should NOT be injected for DOCKER_HOST=%s", tt.dockerHost)
1547-
return
1548-
}
1549-
1550-
require.Contains(t, string(out), `"chroot"`,
1551-
"chroot section should be injected for DOCKER_HOST=%s", tt.dockerHost)
1552-
1553-
// Verify /tmp/gh-aw/awf-config.json was also written (matches the runner_temp copy).
1554-
tmpConfigBytes, readErr := os.ReadFile(awfArcDindChrootBinariesSourcePath + "/awf-config.json")
1555-
require.NoError(t, readErr, "%s/awf-config.json should have been written by inject script", awfArcDindChrootBinariesSourcePath)
1556-
assert.Equal(t, string(out), string(tmpConfigBytes),
1557-
"%s/awf-config.json should match %s/gh-aw/awf-config.json", awfArcDindChrootBinariesSourcePath, tmpDir)
1558-
1559-
chrootRaw, ok := result["chroot"].(map[string]any)
1560-
require.True(t, ok, "chroot must be an object")
1561-
1562-
// Verify binariesSourcePath is the expected constant.
1563-
assert.Equal(t, awfArcDindChrootBinariesSourcePath, chrootRaw["binariesSourcePath"],
1564-
"binariesSourcePath should be %s", awfArcDindChrootBinariesSourcePath)
1565-
1566-
// Verify identity section is present.
1567-
identityRaw, ok := chrootRaw["identity"].(map[string]any)
1568-
require.True(t, ok, "chroot.identity must be an object")
1569-
1570-
// Verify home is the expected constant.
1571-
assert.Equal(t, awfArcDindChrootIdentityHome, identityRaw["home"],
1572-
"identity.home should be %s", awfArcDindChrootIdentityHome)
1573-
1574-
// Verify user is a non-empty string.
1575-
user, ok := identityRaw["user"].(string)
1576-
assert.True(t, ok && user != "", "identity.user should be a non-empty string")
1577-
1578-
// Verify uid/gid are positive numbers.
1579-
uid, ok := identityRaw["uid"].(float64)
1580-
assert.True(t, ok && uid > 0, "identity.uid should be a positive number, got %v", identityRaw["uid"])
1581-
1582-
gid, ok := identityRaw["gid"].(float64)
1583-
assert.True(t, ok && gid > 0, "identity.gid should be a positive number, got %v", identityRaw["gid"])
1584-
1585-
// Verify original config fields survived the patch (round-trip correctness).
1586-
apiProxy, ok := result["apiProxy"].(map[string]any)
1587-
require.True(t, ok, "apiProxy section must be preserved after chroot injection")
1588-
assert.Equal(t, true, apiProxy["enabled"], "apiProxy.enabled should be unchanged after chroot injection")
1589-
})
1590-
}
1591-
}
1592-
15931463
// TestBuildAWFCommand_IncludesChrootInjectScript verifies that BuildAWFCommand
15941464
// includes the chroot injection script in the generated run step when the AWF
15951465
// version supports it.

0 commit comments

Comments
 (0)