From 0ec389c4e3984cd0332f912586f2e57a1aea1d09 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 23 Aug 2026 17:02:06 +0200 Subject: [PATCH] :bug: fix(dev): select envtest assets by host OS/arch Four envtest suites resolved their control-plane binaries by returning the first directory under bin/k8s. os.ReadDir returns that listing sorted, so on an Apple-silicon workstation whose bin/ also holds linux downloads the pick was 1.35.0-linux-amd64 and every suite died in BeforeSuite with fork/exec bin/k8s/1.35.0-linux-amd64/etcd: exec format error with the correct 1.36.2-darwin-arm64 assets sitting right next to it. This is a selection bug, not a procurement one: local green on internal/collect, internal/controller, internal/pipeline and internal/webhook/v1alpha1 has been unobtainable on a non-linux host for as long as this has been true. internal/envtestassets now resolves the assets once, for all four suites: KUBEBUILDER_ASSETS first (the order controller-runtime itself uses, so an exporting harness stays authoritative), then the bin/k8s entry whose name carries the -- suffix. Only suffix matches are considered, which also skips non-asset directories such as the stray bin/k8s/k8s a --bin-dir typo leaves behind, and among matches the highest version wins compared field by numeric field so 1.36.10 outranks 1.36.2. CI is unaffected: hack/coverage.sh and `make test` export KUBEBUILDER_ASSETS from `setup-envtest ... -p path`, which resolves the host platform itself, and controller-runtime's BinPathFinder consults that variable before any BinaryAssetsDirectory. The linux-amd64 selection is pinned by its own table case rather than left to CI to discover. The helper joins internal/digest and internal/errors under the arch-lint `shared` component because all four suites, in four different components, need the same resolution; each still passes its own base paths, since the webhook suite sits one directory deeper than the others. --- .go-arch-lint.yml | 1 + docs/development/setup.md | 5 + internal/collect/scale_envtest_test.go | 37 +--- internal/controller/suite_test.go | 36 ++-- internal/envtestassets/assets.go | 111 ++++++++++ internal/envtestassets/assets_test.go | 193 ++++++++++++++++++ internal/pipeline/integration_test.go | 32 +-- .../webhook/v1alpha1/setup_manager_test.go | 2 +- internal/webhook/v1alpha1/suite_test.go | 24 +-- 9 files changed, 347 insertions(+), 94 deletions(-) create mode 100644 internal/envtestassets/assets.go create mode 100644 internal/envtestassets/assets_test.go diff --git a/.go-arch-lint.yml b/.go-arch-lint.yml index f457f844..084734db 100644 --- a/.go-arch-lint.yml +++ b/.go-arch-lint.yml @@ -75,6 +75,7 @@ components: shared: in: - internal/digest/** + - internal/envtestassets/** - internal/errors/** - internal/metrics/** - internal/pathvalidate/** diff --git a/docs/development/setup.md b/docs/development/setup.md index 103c298b..665c5090 100644 --- a/docs/development/setup.md +++ b/docs/development/setup.md @@ -236,6 +236,11 @@ Test pyramid (L0–L5), coverage floors, and CI gates: for controller-runtime envtest. First run may take a minute. Controller tests live under `internal/controller/` (`suite_test.go` sets up envtest). +The envtest suites fall back to `bin/k8s/--` when `KUBEBUILDER_ASSETS` is unset, +so a bare `go test ./internal/...` works too. They select the directory matching the **host** +OS/arch (`internal/envtestassets`), which matters when `bin/` holds downloads for more than one +platform — picking the wrong one fails every suite in `BeforeSuite` with `exec format error`. + E2E scripts, nightly workflows, multi-tenant fixtures, and tenantMode RBAC asserts are documented in [testing.md](testing.md) and `hack/kind/README.md`. diff --git a/internal/collect/scale_envtest_test.go b/internal/collect/scale_envtest_test.go index 316f02b3..8cc82b27 100644 --- a/internal/collect/scale_envtest_test.go +++ b/internal/collect/scale_envtest_test.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + goruntime "runtime" "strconv" "sync" "testing" @@ -25,6 +26,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" kollectdevv1alpha1 "github.com/platformrelay/kollect/api/v1alpha1" + "github.com/platformrelay/kollect/internal/envtestassets" ) // scaleTestMaxObjects is the ADR-0603 default synthetic object cap for task test. @@ -285,41 +287,16 @@ func stopScaleEnvtest(t *testing.T, testEnv *envtest.Environment) { } } +// resolveEnvtestAssetsDir returns KUBEBUILDER_ASSETS when the harness exported it (`task test`, +// hack/coverage.sh), and otherwise the local setup-envtest download built for this host. Selecting +// by host OS/arch matters because a checkout can hold assets for several platforms at once. func resolveEnvtestAssetsDir() string { - if assets := os.Getenv("KUBEBUILDER_ASSETS"); assets != "" { - if abs, err := filepath.Abs(assets); err == nil { - return abs - } - - return assets - } - - return scaleEnvtestBinaryDir() -} - -func scaleEnvtestBinaryDir() string { - for _, basePath := range []string{ + basePaths := []string{ filepath.Join("bin", "k8s"), filepath.Join("..", "..", "bin", "k8s"), - } { - entries, err := os.ReadDir(basePath) - if err != nil { - continue - } - - for _, entry := range entries { - if entry.IsDir() { - abs, err := filepath.Abs(filepath.Join(basePath, entry.Name())) - if err != nil { - continue - } - - return abs - } - } } - return "" + return envtestassets.Resolve(basePaths, goruntime.GOOS, goruntime.GOARCH) } func seedScaleNamespace(ctx context.Context, cfg *rest.Config) error { diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index e714e8f6..94fa17b0 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -7,6 +7,7 @@ import ( "context" "os" "path/filepath" + "runtime" "testing" "time" @@ -21,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" kollectdevv1alpha1 "github.com/platformrelay/kollect/api/v1alpha1" + "github.com/platformrelay/kollect/internal/envtestassets" "github.com/platformrelay/kollect/internal/sink" // +kubebuilder:scaffold:imports ) @@ -66,9 +68,10 @@ var _ = BeforeSuite(func() { ErrorIfCRDPathMissing: true, } - // Retrieve the first found binary directory to allow running tests from IDEs - if getFirstFoundEnvTestBinaryDir() != "" { - testEnv.BinaryAssetsDirectory = getFirstFoundEnvTestBinaryDir() + // Resolve the binaries built for this host so the suite also runs from an IDE, and on a + // developer machine whose OS/arch differs from CI's. + if dir := envtestassets.Resolve(envtestAssetBasePaths(), runtime.GOOS, runtime.GOARCH); dir != "" { + testEnv.BinaryAssetsDirectory = dir } // cfg is defined in this file globally. @@ -96,25 +99,10 @@ var _ = AfterSuite(func() { }, time.Minute, time.Second).Should(Succeed()) }) -// getFirstFoundEnvTestBinaryDir locates the first binary in the specified path. -// ENVTEST-based tests depend on specific binaries, usually located in paths set by -// controller-runtime. When running tests directly (e.g., via an IDE) without using -// Makefile targets, the 'BinaryAssetsDirectory' must be explicitly configured. -// -// This function streamlines the process by finding the required binaries, similar to -// setting the 'KUBEBUILDER_ASSETS' environment variable. To ensure the binaries are -// properly set up, run 'make setup-envtest' beforehand. -func getFirstFoundEnvTestBinaryDir() string { - basePath := filepath.Join("..", "..", "bin", "k8s") - entries, err := os.ReadDir(basePath) - if err != nil { - logf.Log.Error(err, "Failed to read directory", "path", basePath) - return "" - } - for _, entry := range entries { - if entry.IsDir() { - return filepath.Join(basePath, entry.Name()) - } - } - return "" +// envtestAssetBasePaths lists the directories that may hold setup-envtest downloads, relative to +// this package. ENVTEST-based tests depend on those binaries, and when the tests run directly +// (e.g. from an IDE) rather than through a Makefile target that exports KUBEBUILDER_ASSETS, the +// 'BinaryAssetsDirectory' must be configured explicitly. Run 'make setup-envtest' beforehand. +func envtestAssetBasePaths() []string { + return []string{filepath.Join("..", "..", "bin", "k8s")} } diff --git a/internal/envtestassets/assets.go b/internal/envtestassets/assets.go new file mode 100644 index 00000000..c2af48ec --- /dev/null +++ b/internal/envtestassets/assets.go @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Konrad Heimel + +// Package envtestassets resolves the envtest control-plane binaries that match the running host. +// +// setup-envtest lays its downloads out as /--, and a checkout can +// easily end up holding several of them at once — a bin/ shared between machines, a stale download, +// or a directory left behind by an earlier --bin-dir. The kubebuilder scaffold picks the first +// directory it finds, which on an Apple-silicon host hands envtest linux-amd64 binaries and fails +// every suite in BeforeSuite with "exec format error". Selecting by host OS/arch instead keeps a +// bare `go test ./internal/...` usable off linux without changing what CI resolves. +package envtestassets + +import ( + "os" + "path/filepath" + "strconv" + "strings" +) + +// EnvVar is the environment variable `make test` and hack/coverage.sh export, and the one +// controller-runtime consults before an envtest.Environment's BinaryAssetsDirectory. +const EnvVar = "KUBEBUILDER_ASSETS" + +// Dir returns the envtest asset directory built for goos/goarch, or "" when there is none. +// +// basePaths are searched in order and the first one holding a match wins; a base path that cannot +// be read is skipped. Only directories whose name ends in -- are considered, which +// also rules out non-asset directories. Among the matches the highest version wins, compared field +// by numeric field so that 1.36.10 outranks 1.36.2 — plain lexical order gets that backwards. +// +// The result is absolute so it survives a caller changing directory. An empty result means callers +// should leave BinaryAssetsDirectory unset and let controller-runtime fall back to EnvVar or to its +// own default location. +func Dir(basePaths []string, goos, goarch string) string { + suffix := "-" + goos + "-" + goarch + + for _, basePath := range basePaths { + entries, err := os.ReadDir(basePath) + if err != nil { + continue + } + + best, bestVersion := "", "" + + for _, entry := range entries { + name := entry.Name() + if !entry.IsDir() || !strings.HasSuffix(name, suffix) { + continue + } + + version := strings.TrimSuffix(name, suffix) + if best == "" || newer(version, bestVersion) { + best, bestVersion = name, version + } + } + + if best != "" { + return abs(filepath.Join(basePath, best)) + } + } + + return "" +} + +// Resolve returns EnvVar when it is set and Dir(basePaths, goos, goarch) otherwise. +// +// EnvVar wins because that is the order controller-runtime itself resolves binaries in, so a +// harness that exports it — `make test`, hack/coverage.sh, CI — stays authoritative over whatever +// else happens to sit in bin/k8s. +func Resolve(basePaths []string, goos, goarch string) string { + if assets := os.Getenv(EnvVar); assets != "" { + return abs(assets) + } + + return Dir(basePaths, goos, goarch) +} + +// newer reports whether version a ranks above version b. Dot-separated fields are compared in +// order, numerically where both parse and lexically where either does not, and a version that +// extends another as a prefix ranks above it. +func newer(a, b string) bool { + fieldsA := strings.Split(a, ".") + fieldsB := strings.Split(b, ".") + + for i := 0; i < len(fieldsA) && i < len(fieldsB); i++ { + if fieldsA[i] == fieldsB[i] { + continue + } + + numA, errA := strconv.Atoi(fieldsA[i]) + numB, errB := strconv.Atoi(fieldsB[i]) + + if errA != nil || errB != nil { + return fieldsA[i] > fieldsB[i] + } + + return numA > numB + } + + return len(fieldsA) > len(fieldsB) +} + +// abs resolves path against the working directory, falling back to path when that is unavailable. +func abs(path string) string { + if resolved, err := filepath.Abs(path); err == nil { + return resolved + } + + return path +} diff --git a/internal/envtestassets/assets_test.go b/internal/envtestassets/assets_test.go new file mode 100644 index 00000000..a74d54be --- /dev/null +++ b/internal/envtestassets/assets_test.go @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Konrad Heimel + +package envtestassets_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/platformrelay/kollect/internal/envtestassets" +) + +// realWorldListing is what bin/k8s actually holds on an Apple-silicon workstation whose checkout +// has downloaded assets for more than one platform: two linux-amd64 versions sorting ahead of the +// darwin-arm64 one, plus a stray non-asset directory left behind by a `--bin-dir bin/k8s` +// invocation. Returning the first entry here is what produced +// "fork/exec bin/k8s/1.35.0-linux-amd64/etcd: exec format error". +var realWorldListing = []string{ + "1.35.0-linux-amd64", + "1.36.0-linux-amd64", + "1.36.2-darwin-arm64", + "k8s", +} + +// makeBase builds a throwaway bin/k8s-shaped directory containing dirs and files. +func makeBase(t *testing.T, dirs, files []string) string { + t.Helper() + + base := t.TempDir() + + for _, dir := range dirs { + if err := os.MkdirAll(filepath.Join(base, dir), 0o750); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } + } + + for _, name := range files { + if err := os.WriteFile(filepath.Join(base, name), []byte("x"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + return base +} + +func TestDir(t *testing.T) { + tests := []struct { + name string + dirs []string + files []string + goos string + goarch string + want string + }{ + { + name: "apple silicon skips the linux assets that sort first", + dirs: realWorldListing, + goos: "darwin", + goarch: "arm64", + want: "1.36.2-darwin-arm64", + }, + { + name: "linux amd64 still resolves the linux assets CI depends on", + dirs: realWorldListing, + goos: "linux", + goarch: "amd64", + want: "1.36.0-linux-amd64", + }, + { + name: "the highest matching version wins, compared numerically not lexically", + dirs: []string{"1.36.2-linux-arm64", "1.34.0-linux-arm64", "1.36.10-linux-arm64"}, + goos: "linux", + goarch: "arm64", + want: "1.36.10-linux-arm64", + }, + { + name: "a longer version outranks the prefix it extends", + dirs: []string{"1.36-linux-arm64", "1.36.1-linux-arm64"}, + goos: "linux", + goarch: "arm64", + want: "1.36.1-linux-arm64", + }, + { + name: "unparseable version fields fall back to a deterministic lexical order", + dirs: []string{"1.36.2-linux-arm64", "1.36.beta-linux-arm64"}, + goos: "linux", + goarch: "arm64", + want: "1.36.beta-linux-arm64", + }, + { + name: "a matching name that is a file is not an asset directory", + dirs: []string{"k8s"}, + files: []string{"1.36.2-darwin-arm64"}, + goos: "darwin", + goarch: "arm64", + want: "", + }, + { + name: "no host match leaves the choice to controller-runtime", + dirs: realWorldListing, + goos: "windows", + goarch: "amd64", + want: "", + }, + { + name: "an empty base directory matches nothing", + goos: "darwin", + goarch: "arm64", + want: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + base := makeBase(t, tc.dirs, tc.files) + + want := "" + if tc.want != "" { + want = filepath.Join(base, tc.want) + } + + if got := envtestassets.Dir([]string{base}, tc.goos, tc.goarch); got != want { + t.Fatalf("Dir = %q, want %q", got, want) + } + }) + } +} + +func TestDirTriesEveryBasePathInOrder(t *testing.T) { + base := makeBase(t, realWorldListing, nil) + missing := filepath.Join(t.TempDir(), "does-not-exist") + + got := envtestassets.Dir([]string{missing, base}, "darwin", "arm64") + if want := filepath.Join(base, "1.36.2-darwin-arm64"); got != want { + t.Fatalf("Dir = %q, want %q", got, want) + } + + if got := envtestassets.Dir(nil, "darwin", "arm64"); got != "" { + t.Fatalf("Dir with no base paths = %q, want empty", got) + } +} + +func TestDirReturnsAnAbsolutePath(t *testing.T) { + base := makeBase(t, realWorldListing, nil) + + rel, err := filepath.Rel(mustGetwd(t), base) + if err != nil { + t.Skipf("no relative path from cwd to %s: %v", base, err) + } + + got := envtestassets.Dir([]string{rel}, "darwin", "arm64") + if !filepath.IsAbs(got) { + t.Fatalf("Dir = %q, want an absolute path", got) + } + + if want := filepath.Join(base, "1.36.2-darwin-arm64"); got != want { + t.Fatalf("Dir = %q, want %q", got, want) + } +} + +func TestResolvePrefersKubebuilderAssets(t *testing.T) { + base := makeBase(t, realWorldListing, nil) + assets := filepath.Join(base, "1.35.0-linux-amd64") + + t.Setenv(envtestassets.EnvVar, assets) + + if got := envtestassets.Resolve([]string{base}, "darwin", "arm64"); got != assets { + t.Fatalf("Resolve = %q, want %q", got, assets) + } +} + +func TestResolveFallsBackToTheHostMatchingDir(t *testing.T) { + base := makeBase(t, realWorldListing, nil) + + t.Setenv(envtestassets.EnvVar, "") + + got := envtestassets.Resolve([]string{base}, "darwin", "arm64") + if want := filepath.Join(base, "1.36.2-darwin-arm64"); got != want { + t.Fatalf("Resolve = %q, want %q", got, want) + } +} + +func mustGetwd(t *testing.T) string { + t.Helper() + + wd, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + + return wd +} diff --git a/internal/pipeline/integration_test.go b/internal/pipeline/integration_test.go index a74d2054..80796c41 100644 --- a/internal/pipeline/integration_test.go +++ b/internal/pipeline/integration_test.go @@ -10,6 +10,7 @@ import ( "io/fs" "os" "path/filepath" + "runtime" "strings" "testing" @@ -23,6 +24,7 @@ import ( clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "sigs.k8s.io/controller-runtime/pkg/envtest" + "github.com/platformrelay/kollect/internal/envtestassets" "github.com/platformrelay/kollect/internal/sink" ) @@ -324,34 +326,14 @@ func countFiles(t *testing.T, root string) int { } // pipelineEnvtestAssetsDir resolves the envtest binary assets directory the same way the collect -// scale test does: KUBEBUILDER_ASSETS first (set by the Makefile `test` target), then a local -// bin/k8s/ directory. Returns "" when neither is present so the caller can skip. +// scale test does: KUBEBUILDER_ASSETS first (set by the Makefile `test` target), then the local +// bin/k8s/-- directory built for this host. Returns "" when neither is present +// so the caller can skip. func pipelineEnvtestAssetsDir() string { - if assets := os.Getenv("KUBEBUILDER_ASSETS"); assets != "" { - if abs, err := filepath.Abs(assets); err == nil { - return abs - } - - return assets - } - - for _, basePath := range []string{ + basePaths := []string{ filepath.Join("bin", "k8s"), filepath.Join("..", "..", "bin", "k8s"), - } { - entries, err := os.ReadDir(basePath) - if err != nil { - continue - } - - for _, entry := range entries { - if entry.IsDir() { - if abs, err := filepath.Abs(filepath.Join(basePath, entry.Name())); err == nil { - return abs - } - } - } } - return "" + return envtestassets.Resolve(basePaths, runtime.GOOS, runtime.GOARCH) } diff --git a/internal/webhook/v1alpha1/setup_manager_test.go b/internal/webhook/v1alpha1/setup_manager_test.go index 9df6e5fc..31e9179b 100644 --- a/internal/webhook/v1alpha1/setup_manager_test.go +++ b/internal/webhook/v1alpha1/setup_manager_test.go @@ -35,7 +35,7 @@ func startWebhookManager(t *testing.T, tenantMode bool) { Paths: []string{filepath.Join("..", "..", "..", "config", "webhook", "manifests.yaml")}, }, } - if dir := firstEnvtestBinaryDir(); dir != "" { + if dir := hostEnvtestBinaryDir(); dir != "" { env.BinaryAssetsDirectory = dir } diff --git a/internal/webhook/v1alpha1/suite_test.go b/internal/webhook/v1alpha1/suite_test.go index 857758b4..b8f10156 100644 --- a/internal/webhook/v1alpha1/suite_test.go +++ b/internal/webhook/v1alpha1/suite_test.go @@ -5,8 +5,8 @@ package webhookv1alpha1 import ( "context" - "os" "path/filepath" + "runtime" "testing" "time" @@ -24,6 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" kollectdevv1alpha1 "github.com/platformrelay/kollect/api/v1alpha1" + "github.com/platformrelay/kollect/internal/envtestassets" ) func TestWebhookEnvtest(t *testing.T) { @@ -55,7 +56,7 @@ var _ = BeforeSuite(func() { }, } - if dir := firstEnvtestBinaryDir(); dir != "" { + if dir := hostEnvtestBinaryDir(); dir != "" { webhookEnv.BinaryAssetsDirectory = dir } @@ -90,16 +91,11 @@ var _ = AfterSuite(func() { }, time.Minute, time.Second).Should(Succeed()) }) -func firstEnvtestBinaryDir() string { - basePath := filepath.Join("..", "..", "..", "bin", "k8s") - entries, err := os.ReadDir(basePath) - if err != nil { - return "" - } - for _, entry := range entries { - if entry.IsDir() { - return filepath.Join(basePath, entry.Name()) - } - } - return "" +// hostEnvtestBinaryDir resolves the envtest assets for this suite: KUBEBUILDER_ASSETS when the +// harness exported it, and otherwise the local setup-envtest download built for this host. Note the +// three-level base path — this package sits one directory deeper than the other envtest suites. +func hostEnvtestBinaryDir() string { + basePaths := []string{filepath.Join("..", "..", "..", "bin", "k8s")} + + return envtestassets.Resolve(basePaths, runtime.GOOS, runtime.GOARCH) }