fix: ensure image oci config is included when building actor oci spec

cmd/atelet/internal/memorypullcache/memorypullcache.go

@@ -22,6 +22,7 @@ import (
 	"log/slog"
 	"net"
 	"runtime"
+	"slices"
 	"strings"
 
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -37,11 +38,15 @@ type MemoryPullCache struct {
 
 	localhostRegistryReplacement string
 
-	// Map from hexadecimal sha256 hash of image to byte contents of composed
-	// tarball
+	// Map from hexadecimal sha256 hash of image to the cached image (tarball + config env).
 	cache *lru.Cache
 }
 
+type cachedImage struct {
+	tar []byte
+	env []string
+}
+
 func NewMemoryPullCache(ctx context.Context, gcpAuthenticator authn.Authenticator, localhostRegistryReplacement string) (*MemoryPullCache, error) {
 	c := &MemoryPullCache{
 		// TODO: Need a smarter cache with bounds on total consumed size, not
@@ -65,7 +70,8 @@ func NewMemoryPullCache(ctx context.Context, gcpAuthenticator authn.Authenticato
 	return c, nil
 }
 
-func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser, error) {
+// Fetch returns the image's extracted filesystem tarball and its image ENV.
+func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser, []string, error) {
 	// when running in kind we need to rewrite the registry endpoint similar to the
 	// containerd mirror config used in https://kind.sigs.k8s.io/docs/user/local-registry/
 	// for now we have simple opt-in support to rewrite local registries
@@ -86,7 +92,7 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser,
 
 	parsedRef, err := name.ParseReference(ref, nameOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("while parsing reference: %w", err)
+		return nil, nil, fmt.Errorf("while parsing reference: %w", err)
 	}
 
 	// If the image ref included a digest, check for a hit in the pull cache.
@@ -106,7 +112,8 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser,
 				slog.String("ref", ref),
 				slog.String("digest", requestedDigest.DigestStr()),
 			)
-			return io.NopCloser(bytes.NewReader(vAny.([]byte))), nil
+			img := vAny.(*cachedImage)
+			return io.NopCloser(bytes.NewReader(img.tar)), img.env, nil
 		}
 	}
 
@@ -142,36 +149,43 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser,
 
 	img, err := remote.Image(parsedRef, remoteOptions...)
 	if err != nil {
-		return nil, fmt.Errorf("in remote.Image: %w", err)
+		return nil, nil, fmt.Errorf("in remote.Image: %w", err)
+	}
+
+	var imageEnv []string
+	if cfg, cfgErr := img.ConfigFile(); cfgErr != nil {
+		return nil, nil, fmt.Errorf("while reading image config: %w", cfgErr)
+	} else if cfg != nil {
+		imageEnv = slices.Clone(cfg.Config.Env)
 	}
 
 	size, err := img.Size()
 	if err != nil {
-		return nil, fmt.Errorf("in img.Size(): %w", err)
+		return nil, nil, fmt.Errorf("in img.Size(): %w", err)
 	}
 	if size > 100*1024*1024 {
 		slog.InfoContext(ctx,
 			"Image is too large to cache",
 			slog.String("ref", ref),
 			slog.Int64("size", size),
 		)
-		return mutate.Extract(img), err
+		return mutate.Extract(img), imageEnv, err
 	}
 
 	tarData := mutate.Extract(img)
 	defer tarData.Close()
 
 	memData, err := io.ReadAll(tarData)
 	if err != nil {
-		return nil, fmt.Errorf("while reading image: %w", err)
+		return nil, nil, fmt.Errorf("while reading image: %w", err)
 	}
 
 	if digestWasIncluded {
 		// If the user requested multi-arch image, the digest they request will
 		// not be the same as the digest of the image we actually downloaded
 		// from the registry.  We need to place the cache entry under the digest
 		// they requested.
-		c.cache.Add(requestedDigest.DigestStr(), memData)
+		c.cache.Add(requestedDigest.DigestStr(), &cachedImage{tar: memData, env: imageEnv})
 		slog.InfoContext(
 			ctx,
 			"Populated image cache",
@@ -180,7 +194,7 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser,
 		)
 	}
 
-	return io.NopCloser(bytes.NewReader(memData)), nil
+	return io.NopCloser(bytes.NewReader(memData)), imageEnv, nil
 }
 
 func registryHost(ref string) string {

cmd/atelet/oci.go

@@ -69,7 +69,7 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 		return fmt.Errorf("in os.MkdirAll for container bundle dir: %w", err)
 	}
 
-	tarData, err := pullCache.Fetch(ctx, ref)
+	tarData, imageEnv, err := pullCache.Fetch(ctx, ref)
 	if err != nil {
 		return fmt.Errorf("in pullCache.Fetch: %w", err)
 	}
@@ -88,7 +88,7 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 		}
 	}
 
-	ociSpec := buildActorOCISpec(args, env, annotations, netns, identityDir)
+	ociSpec := buildActorOCISpec(imageEnv, args, env, annotations, netns, identityDir)
 	ociSpecBytes, err := json.MarshalIndent(ociSpec, "", "  ")
 	if err != nil {
 		return fmt.Errorf("while marshaling OCI spec: %w", err)
@@ -101,15 +101,36 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 	return nil
 }
 
+// mergeActorEnv merges the ActorTemplate env and the image's ENV, with the template taking precedence.
+// duplicated keys are removed in favor of the following precedence template env > image env.
+func mergeActorEnv(imageEnv, templateEnv []string) []string {
+	seen := make(map[string]struct{})
+	var out []string
+	add := func(entries ...string) {
+		for _, e := range entries {
+			key, _, _ := strings.Cut(e, "=")
+			if key == "" {
+				continue
+			}
+			if _, ok := seen[key]; ok {
+				continue
+			}
+			seen[key] = struct{}{}
+			out = append(out, e)
+		}
+	}
+
+	add(templateEnv...)
+	add(imageEnv...)
+	return out
+}
+
 // buildActorOCISpec assembles the OCI runtime spec for an actor container.
 // When identityDir is non-empty it adds a read-only bind mount of that host
 // directory at IdentityMountPath so the actor can read its own ID (see
 // IdentityMountPath for why this is a bind mount rather than env vars).
-func buildActorOCISpec(args []string, env []string, annotations map[string]string, netns string, identityDir string) *specs.Spec {
-	envVars := []string{
-		"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
-	}
-	envVars = append(envVars, env...)
+func buildActorOCISpec(imageEnv []string, args []string, env []string, annotations map[string]string, netns string, identityDir string) *specs.Spec {
+	envVars := mergeActorEnv(imageEnv, env)
 
 	mounts := []specs.Mount{
 		{

cmd/atelet/oci_test.go

@@ -84,6 +84,7 @@ func runUntar(t *testing.T, entries []tarEntry) (string, error) {
 // With an identity dir, a read-only bind mount appears at IdentityMountPath.
 func TestBuildActorOCISpec_IdentityMount(t *testing.T) {
 	spec := buildActorOCISpec(
+		nil,
 		[]string{"/app"},
 		[]string{"FOO=bar"},
 		map[string]string{"k": "v"},
@@ -111,9 +112,49 @@ func TestBuildActorOCISpec_IdentityMount(t *testing.T) {
 	}
 }
 
+// mergeActorEnv test template env overrides image env.
+func TestMergeActorEnv(t *testing.T) {
+	t.Run("template overrides image by key", func(t *testing.T) {
+		imageEnv := []string{"FOO=image"}
+		templateEnv := []string{"FOO=template"}
+		got := mergeActorEnv(imageEnv, templateEnv)
+		if c := countKey(got, "FOO"); c != 1 {
+			t.Fatalf("FOO appears %d times, want 1 (no duplicates): %v", c, got)
+		}
+		if !slices.Contains(got, "FOO=template") {
+			t.Errorf("template value must win over image: %v", got)
+		}
+	})
+
+	t.Run("blank/keyless entries are dropped", func(t *testing.T) {
+		imageEnv := []string{"", "=novalue"}
+		var templateEnv []string
+		mergedEnv := mergeActorEnv(imageEnv, templateEnv)
+		for _, env := range mergedEnv {
+			if env == "" || strings.HasPrefix(env, "=") {
+				t.Errorf("blank entry allowed: %q in %v", env, mergedEnv)
+			}
+		}
+	})
+}
+
+func countKey(env []string, key string) int {
+	n := 0
+	for _, e := range env {
+		k := e
+		if i := strings.IndexByte(e, '='); i >= 0 {
+			k = e[:i]
+		}
+		if k == key {
+			n++
+		}
+	}
+	return n
+}
+
 // Without an identity dir (the pause container), no identity mount appears.
 func TestBuildActorOCISpec_NoIdentityMountForPause(t *testing.T) {
-	bare := buildActorOCISpec([]string{"/pause"}, nil, nil, "/run/netns/x", "")
+	bare := buildActorOCISpec(nil, []string{"/pause"}, nil, nil, "/run/netns/x", "")
 	for _, m := range bare.Mounts {
 		if m.Destination == IdentityMountPath {
 			t.Errorf("identity mount must be absent when identityDir is empty")