diff --git a/cmd/atelet/internal/memorypullcache/memorypullcache.go b/cmd/atelet/internal/memorypullcache/memorypullcache.go index 481db59fb..a106c4533 100644 --- a/cmd/atelet/internal/memorypullcache/memorypullcache.go +++ b/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,12 +149,19 @@ 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, @@ -155,7 +169,7 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser, slog.String("ref", ref), slog.Int64("size", size), ) - return mutate.Extract(img), err + return mutate.Extract(img), imageEnv, err } tarData := mutate.Extract(img) @@ -163,7 +177,7 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser, 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 { @@ -171,7 +185,7 @@ func (c *MemoryPullCache) Fetch(ctx context.Context, ref string) (io.ReadCloser, // 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 { diff --git a/cmd/atelet/oci.go b/cmd/atelet/oci.go index 4451f6362..f0d1447bd 100644 --- a/cmd/atelet/oci.go +++ b/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{ { diff --git a/cmd/atelet/oci_test.go b/cmd/atelet/oci_test.go index 5b53f2520..55aa381b2 100644 --- a/cmd/atelet/oci_test.go +++ b/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")