From 850289fe8fb9a31fb21077210f2634907d103356 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:18:42 +0800 Subject: [PATCH] fix(orchestrator): propagate template Docker image ENV vars to sandbox runtime A template built from a Docker image records the image's ENV directives in its metadata (meta.Context.EnvVars), but on sandbox create only the user-provided env vars from the SDK create call reach the runtime: Create set config.Envd.Vars = req.GetSandbox().GetEnvVars() and never merged the template's ENV. So ENV FOO=bar baked into the image was lost in sandbox processes (issue #2268). This mirrors the gap the fs-only cold boot already closed for DefaultUser / DefaultWorkdir - those are restored from template metadata, but EnvVars was not. Merge once in Server.Create, right after reading template metadata and before the reboot/resume branch: the template ENV is the base layer and the user-provided vars override it on conflict (per-command vars from process.start override both, later, in envd). Because both branches receive the same config and each sends config.Envd.Vars to envd via POST /init (newEnvdInitRequest, alongside DefaultUser/DefaultWorkdir), this single merge covers both the memory resume and the filesystem-only cold boot. Scoped deliberately to the user-facing create path. The factory functions (RebootSandbox/ResumeSandbox/CreateSandbox) have other callers - template build, checkpoint, prefetch - where the image ENV is being defined, not consumed by user code, so merging at the factory layer would alter build-time behavior; the Create-only merge avoids that. (The original attempt #3148 merged in both Server.Create and reboot.go; the reboot.go merge is redundant once Create merges into the shared config, and is dropped here.) Extracted as mergeTemplateEnvVars for isolation; tested for precedence, disjoint merge, nil handling, and that the shared template metadata map is not mutated. Closes #2268. Supersedes #3148. --- packages/orchestrator/pkg/server/env_merge.go | 22 +++++++++++ .../orchestrator/pkg/server/env_merge_test.go | 38 +++++++++++++++++++ packages/orchestrator/pkg/server/sandboxes.go | 8 ++++ 3 files changed, 68 insertions(+) create mode 100644 packages/orchestrator/pkg/server/env_merge.go create mode 100644 packages/orchestrator/pkg/server/env_merge_test.go diff --git a/packages/orchestrator/pkg/server/env_merge.go b/packages/orchestrator/pkg/server/env_merge.go new file mode 100644 index 0000000000..dd0ba32ded --- /dev/null +++ b/packages/orchestrator/pkg/server/env_merge.go @@ -0,0 +1,22 @@ +//go:build linux + +package server + +import "maps" + +// mergeTemplateEnvVars layers env vars for a sandbox start: the template's +// Docker image ENV vars are the base, and user-provided env vars (from the SDK +// create call) override them on conflict. Per-command env vars from +// process.start override both, applied later in envd. A nil/empty template map +// returns userVars unchanged (preserving nil), so the common no-image-ENV case +// adds nothing. +func mergeTemplateEnvVars(templateVars, userVars map[string]string) map[string]string { + if len(templateVars) == 0 { + return userVars + } + + merged := maps.Clone(templateVars) + maps.Copy(merged, userVars) + + return merged +} diff --git a/packages/orchestrator/pkg/server/env_merge_test.go b/packages/orchestrator/pkg/server/env_merge_test.go new file mode 100644 index 0000000000..01afd23db9 --- /dev/null +++ b/packages/orchestrator/pkg/server/env_merge_test.go @@ -0,0 +1,38 @@ +//go:build linux + +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeTemplateEnvVars(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + tmpl map[string]string + user map[string]string + want map[string]string + }{ + "no template, no user": {nil, nil, nil}, + "no template, user only": {nil, map[string]string{"A": "1"}, map[string]string{"A": "1"}}, + "template only": {map[string]string{"PYTHONPATH": "/lib"}, nil, map[string]string{"PYTHONPATH": "/lib"}}, + "disjoint merged": {map[string]string{"P": "/lib"}, map[string]string{"K": "v"}, map[string]string{"P": "/lib", "K": "v"}}, + "user overrides template": {map[string]string{"L": "info", "P": "/lib"}, map[string]string{"L": "debug"}, map[string]string{"L": "debug", "P": "/lib"}}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, mergeTemplateEnvVars(tc.tmpl, tc.user)) + }) + } +} + +// the template map must not be mutated (it is shared template metadata). +func TestMergeTemplateEnvVars_DoesNotMutateTemplate(t *testing.T) { + t.Parallel() + tmpl := map[string]string{"P": "/lib"} + _ = mergeTemplateEnvVars(tmpl, map[string]string{"P": "/override", "K": "v"}) + assert.Equal(t, map[string]string{"P": "/lib"}, tmpl, "template metadata map must be left unchanged") +} diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go index 6abe238390..f36a088164 100644 --- a/packages/orchestrator/pkg/server/sandboxes.go +++ b/packages/orchestrator/pkg/server/sandboxes.go @@ -381,6 +381,14 @@ func (s *Server) Create(ctx context.Context, req *orchestrator.SandboxCreateRequ return nil, fmt.Errorf("failed to read template metadata: %w", err) } + // Propagate the template's Docker image ENV vars to the sandbox runtime. + // They form the base layer; user-provided env vars (already in config from + // the SDK create call) override them, and per-command env vars from + // process.start override both. Mutating config here reaches both the resume + // and the filesystem-only cold-boot branch below, since each sends + // config.Envd.Vars to envd via POST /init (newEnvdInitRequest). + config.Envd.Vars = mergeTemplateEnvVars(meta.Context.EnvVars, config.Envd.Vars) + fsOnly = meta.IsFilesystemOnly() filesystemBooted = filesystemBoot(meta, req) if filesystemBooted {