Skip to content

Commit 72fae25

Browse files
committed
test(remote-worker): guard probed capabilities against the images (#252 review)
Nothing in CI builds either leaf Dockerfile -- build.yaml's matrix is the harness, the OCP sandbox and the echo target, and it only fires on push to main; ci.yml's remote-worker job is the Go build; hadolint parses these files but does not resolve packages. So the exact mismatch this PR exists to fix -- `probed` naming a tool the image does not install, which shipped as `caps=[bash base64 file]` on a real VM -- had no guard, and neither did the two Dockerfiles' agreement with each other. A Go test rather than the suggested vitest parse of main.go: in-package it can reference `probed` directly, so the list it checks cannot drift from the list the worker advertises. It runs in the `go test -race ./...` step CI already has, needs no daemon, no network and no build. `provides` maps each probed tool to what actually puts it on PATH -- base64 comes from coreutils-single, rg from the vendored tarball -- because a test that grepped for the tool's own name would pass while the image stayed broken. `knownGap` is deliberately empty: an entry there means the agent's tools built on that binary fail at runtime in every leaf sandbox, so adding one should require saying so in the Dockerfile and the PR. Three properties, each negative-tested rather than assumed green: - against the pre-fix tree it reports rg, git and python3 missing (exit 1) - dropping git from Dockerfile.runtime only trips the drift check (exit 1) - removing the digest verification trips the pinning check (exit 1) and all three pass on this branch. The regex asserts a match exists, so a stale pattern fails loudly instead of vacuously passing -- the failure mode verify-sandbox-inventory.sh documents. Static analysis cannot prove a package still resolves in a future UBI 9 minor; only building the image does. verify-sandbox-inventory.sh is the in-image counterpart, and is the pattern to follow if these images are ever published to GHCR. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent 187e7bf commit 72fae25

1 file changed

Lines changed: 123 additions & 0 deletions

File tree

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"regexp"
7+
"strings"
8+
"testing"
9+
)
10+
11+
// Nothing in CI builds either leaf Dockerfile -- build.yaml's matrix is the harness, the OCP sandbox
12+
// and the echo target, and it only fires on push to main; ci.yml's remote-worker job is the Go build.
13+
// hadolint parses these files but does not resolve packages. So the one mismatch that actually bit us
14+
// -- `probed` naming a tool the image does not install, which shipped as `caps=[bash base64 file]` on
15+
// a real VM -- had no guard, and neither did the two Dockerfiles' agreement with each other.
16+
//
17+
// This is that guard, and it is static on purpose: it relates the Go list to the image recipes without
18+
// a daemon, a network or a build, so it runs in the `go test -race ./...` step that already exists.
19+
// What it CANNOT do is prove a package still resolves in a future UBI 9 minor; only building the image
20+
// does that. deploy/knative/verify-sandbox-inventory.sh is the in-image counterpart for the OCP
21+
// sandbox image, and is the pattern to follow if these images ever get published to GHCR.
22+
//
23+
// provides maps each probed tool to the thing in the Dockerfile that puts it on PATH. The indirection
24+
// is the point: `base64` comes from coreutils-single and `rg` from a vendored tarball, so a test that
25+
// merely grepped for the tool's own name would pass while the image stayed broken.
26+
var provides = map[string]string{
27+
"bash": "bash",
28+
"base64": "coreutils-single",
29+
"file": "file",
30+
"git": "git",
31+
"python3": "python3",
32+
"rg": "ripgrep-${RG_VERSION}-${target}.tar.gz",
33+
}
34+
35+
// knownGap is for a probed tool deliberately left out of the images. It must stay EMPTY unless the
36+
// omission is also written into the Dockerfile comments and the PR that adds it, because an entry
37+
// here means the agent's tools built on that binary fail at runtime in every leaf sandbox.
38+
var knownGap = map[string]string{}
39+
40+
var installLine = regexp.MustCompile(`(?m)^RUN microdnf install -y --nodocs (.+?)\s*\\?$`)
41+
42+
func dockerfiles(t *testing.T) map[string]string {
43+
t.Helper()
44+
out := map[string]string{}
45+
for _, name := range []string{"Dockerfile", "Dockerfile.runtime"} {
46+
// The test binary runs in cmd/worker; both Dockerfiles sit at the module root.
47+
path := filepath.Join("..", "..", name)
48+
b, err := os.ReadFile(path)
49+
if err != nil {
50+
t.Fatalf("read %s: %v", path, err)
51+
}
52+
out[name] = string(b)
53+
}
54+
return out
55+
}
56+
57+
// Every tool the worker advertises must be installed by both images that can serve as its sandbox.
58+
func TestDockerfilesProvideEveryProbedCapability(t *testing.T) {
59+
for name, body := range dockerfiles(t) {
60+
for _, tool := range probed {
61+
if why, ok := knownGap[tool]; ok {
62+
t.Logf("%s: %q is a declared gap (%s)", name, tool, why)
63+
continue
64+
}
65+
token, ok := provides[tool]
66+
if !ok {
67+
t.Errorf("%s: probed tool %q has no entry in the `provides` map. Add the package or "+
68+
"vendoring step that puts it on PATH, or record it in knownGap and say so in the "+
69+
"Dockerfile -- an unmapped tool is exactly the drift this test exists to catch.",
70+
name, tool)
71+
continue
72+
}
73+
if !strings.Contains(body, token) {
74+
t.Errorf("%s does not install %q: `probed` advertises it as a capability, but %q "+
75+
"appears nowhere in the file. The worker would report caps without it and the "+
76+
"agent's tools built on it would fail with exit 127 inside a healthy sandbox.",
77+
name, tool, token)
78+
}
79+
}
80+
}
81+
}
82+
83+
// The two files are related only by a "see the same block in ./Dockerfile" comment, so nothing but a
84+
// test keeps their package sets in step. Drift here means one sandbox image silently differs from the
85+
// other depending on which build path an operator happened to use.
86+
func TestBothDockerfilesInstallTheSamePackages(t *testing.T) {
87+
files := dockerfiles(t)
88+
sets := map[string][]string{}
89+
for name, body := range files {
90+
m := installLine.FindAllStringSubmatch(body, -1)
91+
if len(m) == 0 {
92+
t.Fatalf("%s: no `RUN microdnf install` line found -- this test's regex has gone stale, "+
93+
"which would make it pass vacuously", name)
94+
}
95+
// The last match is the runtime stage's: the rg fetch stage installs only tar/gzip.
96+
pkgs := strings.Fields(m[len(m)-1][1])
97+
sets[name] = pkgs
98+
}
99+
a, b := sets["Dockerfile"], sets["Dockerfile.runtime"]
100+
if strings.Join(a, " ") != strings.Join(b, " ") {
101+
t.Errorf("the two leaf Dockerfiles install different package sets, so the image an operator "+
102+
"gets depends on which build path they used:\n Dockerfile: %v\n Dockerfile.runtime: %v", a, b)
103+
}
104+
}
105+
106+
// A vendored binary is only safe if the build fails closed on a bad download, so the digest and the
107+
// verification step must both survive edits to these files.
108+
func TestVendoredRipgrepIsPinnedAndChecksummed(t *testing.T) {
109+
for name, body := range dockerfiles(t) {
110+
for _, want := range []string{
111+
"ARG RG_VERSION=", // pinned release, not "latest"
112+
"ARG RG_SHA256_X86_64=", // both arches carry a digest
113+
"ARG RG_SHA256_AARCH64=", //
114+
"sha256sum -c /tmp/rg.sha256", // and the digest is actually enforced
115+
} {
116+
if !strings.Contains(body, want) {
117+
t.Errorf("%s: vendored ripgrep is missing %q. Without it the build would accept "+
118+
"whatever bytes the network returned for a binary that then executes "+
119+
"model-authored commands.", name, want)
120+
}
121+
}
122+
}
123+
}

0 commit comments

Comments
 (0)