Skip to content

feat: TargetArch() utility and arch-aware path resolution #2258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jakubno merged 5 commits into from
Mar 30, 2026
Merged

feat: TargetArch() utility and arch-aware path resolution #2258

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20a4968
feat: add TargetArch() utility and arch-aware path resolution
Mar 29, 2026
74d7e74
fix: emit TargetArch warning once, use ErrNotExist for path fallback
Mar 29, 2026
867d4c2
Merge branch 'main' into feat/target-arch-path-resolution
tomassrnka Mar 30, 2026
52e9646
fix: remove legacy fallback from kernel/FC path resolution
Mar 30, 2026
e7543ce
Revert "fix: remove legacy fallback from kernel/FC path resolution"
Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/orchestrator/pkg/sandbox/fc/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"path/filepath"

"github.com/e2b-dev/infra/packages/orchestrator/pkg/cfg"
"github.com/e2b-dev/infra/packages/shared/pkg/utils"
)

const (
Expand Down Expand Up @@ -31,11 +32,11 @@ func (t Config) SandboxKernelDir() string {
}

func (t Config) HostKernelPath(config cfg.BuilderConfig) string {
return filepath.Join(config.HostKernelsDir, t.KernelVersion, SandboxKernelFile)
return filepath.Join(config.HostKernelsDir, t.KernelVersion, utils.TargetArch(), SandboxKernelFile)
}

func (t Config) FirecrackerPath(config cfg.BuilderConfig) string {
return filepath.Join(config.FirecrackerVersionsDir, t.FirecrackerVersion, FirecrackerBinaryName)
return filepath.Join(config.FirecrackerVersionsDir, t.FirecrackerVersion, utils.TargetArch(), FirecrackerBinaryName)
}

type RootfsPaths struct {
Expand Down
37 changes: 37 additions & 0 deletions packages/orchestrator/pkg/sandbox/fc/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package fc

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"

"github.com/e2b-dev/infra/packages/orchestrator/pkg/cfg"
"github.com/e2b-dev/infra/packages/shared/pkg/utils"
)

func TestFirecrackerPath(t *testing.T) {
t.Parallel()
dir := t.TempDir()
arch := utils.TargetArch()

config := cfg.BuilderConfig{FirecrackerVersionsDir: dir}
fc := Config{FirecrackerVersion: "v1.12.0"}

result := fc.FirecrackerPath(config)

assert.Equal(t, filepath.Join(dir, "v1.12.0", arch, "firecracker"), result)
}

func TestHostKernelPath(t *testing.T) {
t.Parallel()
dir := t.TempDir()
arch := utils.TargetArch()

config := cfg.BuilderConfig{HostKernelsDir: dir}
fc := Config{KernelVersion: "vmlinux-6.1.102"}

result := fc.HostKernelPath(config)

assert.Equal(t, filepath.Join(dir, "vmlinux-6.1.102", arch, "vmlinux.bin"), result)
}
15 changes: 9 additions & 6 deletions packages/orchestrator/pkg/template/build/core/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ func (e *ImageTooLargeError) Error() string {
)
}

var DefaultPlatform = containerregistry.Platform{
OS: "linux",
Architecture: "amd64",
// DefaultPlatform returns the OCI platform for image pulls, respecting TARGET_ARCH.
func DefaultPlatform() containerregistry.Platform {
return containerregistry.Platform{
OS: "linux",
Architecture: utils.TargetArch(),
}
}

// wrapImagePullError converts technical Docker registry errors into user-friendly messages.
Expand Down Expand Up @@ -96,7 +99,7 @@ func GetPublicImage(ctx context.Context, dockerhubRepository dockerhub.RemoteRep
return nil, fmt.Errorf("invalid image reference '%s': %w", tag, err)
}

platform := DefaultPlatform
platform := DefaultPlatform()

// When no auth provider is provided and the image is from the default registry
// use docker remote repository proxy with cached images
Expand Down Expand Up @@ -149,7 +152,7 @@ func GetImage(ctx context.Context, artifactRegistry artifactsregistry.ArtifactsR
childCtx, childSpan := tracer.Start(ctx, "pull-docker-image")
defer childSpan.End()

platform := DefaultPlatform
platform := DefaultPlatform()

img, err := artifactRegistry.GetImage(childCtx, templateId, buildId, platform)
if err != nil {
Expand Down Expand Up @@ -469,7 +472,7 @@ func verifyImagePlatform(img containerregistry.Image, platform containerregistry
return fmt.Errorf("error getting image config file: %w", err)
}
if config.Architecture != platform.Architecture {
return fmt.Errorf("image is not %s", platform.Architecture)
return fmt.Errorf("image architecture %q does not match expected %q", config.Architecture, platform.Architecture)
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion packages/orchestrator/pkg/template/build/core/oci/oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/e2b-dev/infra/packages/shared/pkg/dockerhub"
templatemanager "github.com/e2b-dev/infra/packages/shared/pkg/grpc/template-manager"
"github.com/e2b-dev/infra/packages/shared/pkg/logger"
"github.com/e2b-dev/infra/packages/shared/pkg/utils"
)

func createFileTar(t *testing.T, fileName string) *bytes.Buffer {
Expand Down Expand Up @@ -213,7 +214,7 @@ func TestGetPublicImageWithGeneralAuth(t *testing.T) {
// Set the config to include the proper platform
configFile, err := testImage.ConfigFile()
require.NoError(t, err)
configFile.Architecture = "amd64"
configFile.Architecture = utils.TargetArch()
configFile.OS = "linux"
testImage, err = mutate.ConfigFile(testImage, configFile)
require.NoError(t, err)
Expand Down
31 changes: 31 additions & 0 deletions packages/shared/pkg/utils/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,40 @@ package utils
import (
"fmt"
"os"
"runtime"
"strings"
"sync"
)

// archAliases normalizes common architecture names to Go convention.
var archAliases = map[string]string{
"amd64": "amd64",
"x86_64": "amd64",
"arm64": "arm64",
"aarch64": "arm64",
}

var archWarningOnce sync.Once

// TargetArch returns the target architecture for binary paths and OCI platform.
// If TARGET_ARCH is set, it is normalized to Go convention ("amd64" or "arm64");
// otherwise defaults to the host architecture (runtime.GOARCH).
func TargetArch() string {
if arch := os.Getenv("TARGET_ARCH"); arch != "" {
if normalized, ok := archAliases[arch]; ok {
return normalized
}

archWarningOnce.Do(func() {
fmt.Fprintf(os.Stderr, "WARNING: unrecognized TARGET_ARCH=%q, falling back to %s\n", arch, runtime.GOARCH)
})
Comment on lines +20 to +32
Copy link
Copy Markdown

@claude claude bot Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The archAliases map lookup in TargetArch() is case-sensitive, so TARGET_ARCH=AMD64 or TARGET_ARCH=ARM64 (uppercase, common in some CI/CD environments) fails the lookup, emits a misleading warning, and returns runtime.GOARCH instead of the intended architecture. On a cross-compilation host (e.g., arm64 node with TARGET_ARCH=AMD64), this silently selects the wrong architecture for kernel paths and OCI platform. Fix: normalize with strings.ToLower(arch) before the map lookup in env.go:25.

Extended reasoning...

What the bug is and how it manifests

The TargetArch() function in packages/shared/pkg/utils/env.go reads the TARGET_ARCH environment variable and normalizes common aliases (e.g., x86_64->amd64, aarch64->arm64) via the archAliases map. However, all map keys are lowercase, so any uppercase or mixed-case value (e.g., AMD64, ARM64, X86_64) will fail the lookup.

The specific code path that triggers it

In env.go around line 25:

if normalized, ok := archAliases[arch]; ok {
    return normalized
}
// falls through to warning + runtime.GOARCH fallback
fmt.Fprintf(os.Stderr, "WARNING: unrecognized TARGET_ARCH=%q, falling back to %s\n", arch, runtime.GOARCH)
return runtime.GOARCH

When arch is "AMD64", the map lookup misses (key "AMD64" does not exist; only "amd64" does), the warning is printed, and runtime.GOARCH (the host arch) is returned.

Why existing code does not prevent it

The fix needs to be applied before the map lookup: archAliases[strings.ToLower(arch)]. The strings package is already imported in this file (used by RequiredEnv/OptionalEnv), so this is a trivial one-line change.

Impact

On a cross-compilation host -- for example, an arm64 CI runner with TARGET_ARCH=AMD64 to build amd64 binaries -- the function returns "arm64" instead of "amd64". This propagates to: (1) the Firecracker binary path (FirecrackerPath builds an arch-prefixed path using TargetArch()), (2) the kernel path (HostKernelPath), and (3) the OCI DefaultPlatform(). The operator does see a warning, so this is not completely silent -- but a WARNING to stderr is easy to overlook in noisy CI logs, and the downstream behavior (wrong arch binary, wrong OCI platform) is incorrect.

Step-by-step proof

  1. CI runner is arm64; operator sets TARGET_ARCH=AMD64 (uppercase, intending amd64 cross-compile).
  2. TargetArch() calls os.Getenv("TARGET_ARCH") and gets "AMD64".
  3. archAliases["AMD64"] returns ok == false (map has only "amd64").
  4. Function emits: WARNING: unrecognized TARGET_ARCH="AMD64", falling back to arm64.
  5. Returns "arm64" (runtime.GOARCH on the arm64 runner) -- the wrong value.
  6. DefaultPlatform() returns {OS: "linux", Architecture: "arm64"} -- the wrong OCI platform.
  7. FirecrackerPath checks {version}/arm64/firecracker and falls back to the legacy flat path, potentially using the arm64 binary when amd64 was intended.

How to fix it

Change env.go line 25 from:

if normalized, ok := archAliases[arch]; ok {

to:

if normalized, ok := archAliases[strings.ToLower(arch)]; ok {

This makes TargetArch() case-insensitive and robustly handles the full set of real-world CI inputs.

Copy link
Copy Markdown
Member Author

@tomassrnka tomassrnka Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't fix — Go/Docker/Debian arch conventions are always lowercase (amd64, arm64). Adding strings.ToLower would mask actual typos rather than surfacing them via the warning.


return runtime.GOARCH
}

return runtime.GOARCH
}

// RequiredEnv returns the value of the environment variable for key if it is set, non-empty and not only whitespace.
// It panics otherwise.
//
Expand Down
66 changes: 66 additions & 0 deletions packages/shared/pkg/utils/env_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package utils

import (
"runtime"
"testing"

"github.com/stretchr/testify/assert"
)

func TestTargetArch_DefaultsToHostArch(t *testing.T) {
t.Setenv("TARGET_ARCH", "")

result := TargetArch()

assert.Equal(t, runtime.GOARCH, result)
}

func TestTargetArch_RespectsValidOverride(t *testing.T) {
tests := []struct {
name string
arch string
expected string
}{
{name: "amd64", arch: "amd64", expected: "amd64"},
{name: "arm64", arch: "arm64", expected: "arm64"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("TARGET_ARCH", tt.arch)

result := TargetArch()

assert.Equal(t, tt.expected, result)
})
}
}

func TestTargetArch_NormalizesAliases(t *testing.T) {
tests := []struct {
name string
arch string
expected string
}{
{name: "x86_64 → amd64", arch: "x86_64", expected: "amd64"},
{name: "aarch64 → arm64", arch: "aarch64", expected: "arm64"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Setenv("TARGET_ARCH", tt.arch)

result := TargetArch()

assert.Equal(t, tt.expected, result)
})
}
}

func TestTargetArch_FallsBackOnUnknown(t *testing.T) {
t.Setenv("TARGET_ARCH", "mips")

result := TargetArch()

assert.Equal(t, runtime.GOARCH, result)
}
Loading