From 7a17fc7d57ba84d5597a480781cd51a576a9d6e2 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Wed, 28 Dec 2022 19:15:51 +0300 Subject: [PATCH] fix(staged-dockerfile): correction for ENV and ARG instructions handling 1. ARG should affect all RUN instruction digests which defined after ARG. 2. ARG could be used in the ENV instruction expansion. 3. ARG should not be expanded in the RUN instruction statically, instead expand it in runtime. Internally args should be passed as env vars when running RUN instructions. Signed-off-by: Timofey Kirillov --- pkg/build/image/image.go | 68 +++++++++++++++---- pkg/build/stage/base.go | 4 ++ pkg/build/stage/instruction/add.go | 5 +- pkg/build/stage/instruction/base.go | 62 ++++++++++++++--- pkg/build/stage/instruction/cmd.go | 5 +- pkg/build/stage/instruction/copy.go | 13 ++-- pkg/build/stage/instruction/entrypoint.go | 6 +- pkg/build/stage/instruction/env.go | 6 +- pkg/build/stage/instruction/expose.go | 6 +- pkg/build/stage/instruction/healthcheck.go | 6 +- pkg/build/stage/instruction/label.go | 6 +- pkg/build/stage/instruction/maintainer.go | 6 +- pkg/build/stage/instruction/on_build.go | 6 +- pkg/build/stage/instruction/run.go | 30 ++++++-- pkg/build/stage/instruction/shell.go | 6 +- pkg/build/stage/instruction/stop_signal.go | 6 +- pkg/build/stage/instruction/user.go | 6 +- pkg/build/stage/instruction/volume.go | 6 +- pkg/build/stage/instruction/workdir.go | 6 +- pkg/build/stage/interface.go | 1 + pkg/build/stages_iterator.go | 6 +- pkg/container_backend/buildah_backend.go | 1 + pkg/container_backend/instruction/run.go | 6 +- pkg/docker_registry/api.go | 2 + .../frontend/buildkit_dockerfile.go | 51 ++++++++------ pkg/dockerfile/instruction.go | 42 ++++++++---- pkg/image/info.go | 4 ++ pkg/storage/manager/storage_manager.go | 2 + 28 files changed, 242 insertions(+), 132 deletions(-) diff --git a/pkg/build/image/image.go b/pkg/build/image/image.go index 29b67f5b5a..d2a14b1627 100644 --- a/pkg/build/image/image.go +++ b/pkg/build/image/image.go @@ -3,6 +3,7 @@ package image import ( "context" "fmt" + "strings" "github.com/gookit/color" @@ -203,12 +204,22 @@ func (i *Image) GetRebuilt() bool { return i.rebuilt } +func (i *Image) ExpandDependencies(ctx context.Context, baseEnv map[string]string) error { + for _, stg := range i.stages { + if err := stg.ExpandDependencies(ctx, i.Conveyor, baseEnv); err != nil { + return fmt.Errorf("unable to expand dependencies for stage %q: %w", stg.Name(), err) + } + } + return nil +} + func (i *Image) SetupBaseImage(ctx context.Context, storageManager manager.StorageManagerInterface, storageOpts manager.StorageOptions) error { switch i.baseImageType { case StageAsBaseImage: i.stageAsBaseImage = i.Conveyor.GetImage(i.baseImageName).GetLastNonEmptyStage() i.baseImageReference = i.stageAsBaseImage.GetStageImage().Image.Name() i.baseStageImage = i.stageAsBaseImage.GetStageImage() + case ImageFromRegistryAsBaseImage: if i.IsDockerfileImage && i.dockerfileExpanderFactory != nil { dependenciesArgs := stage.ResolveDependenciesArgs(i.DockerfileImageConfig.Dependencies, i.Conveyor) @@ -218,27 +229,33 @@ func (i *Image) SetupBaseImage(ctx context.Context, storageManager manager.Stora } i.baseImageReference = ref } + i.baseStageImage = i.Conveyor.GetOrCreateStageImage(i.baseImageReference, nil, nil, i) + + info, err := storageManager.GetImageInfo(ctx, i.baseImageReference, storageOpts) + if err != nil { + return fmt.Errorf("unable to get base image %q manifest: %w", i.baseImageReference, err) + } + + i.baseStageImage.Image.SetStageDescription(&image.StageDescription{ + StageID: nil, // this is not a stage actually, TODO + Info: info, + }) + + // for _, expression := range info.OnBuild { + // fmt.Printf(">> %q\n", expression) + // } + case NoBaseImage: + default: panic(fmt.Sprintf("unknown base image type %q", i.baseImageType)) } - if i.IsDockerfileImage && i.DockerfileImageConfig.Staged { - switch i.baseImageType { - case StageAsBaseImage, ImageFromRegistryAsBaseImage: - - fmt.Printf("-- %s SetupBaseImage %q\n", i.Name, i.baseImageReference) - - info, err := storageManager.GetImageInfo(ctx, i.baseImageReference, storageOpts) - if err != nil { - return fmt.Errorf("unable to get base image %q manifest: %w", i.baseImageReference, err) - } - - fmt.Printf("-- %s SetupBaseImage %q -> %#v\n", i.Name, i.baseImageReference, info) - for _, expression := range info.OnBuild { - fmt.Printf(">> %q\n", expression) - } + switch i.baseImageType { + case StageAsBaseImage, ImageFromRegistryAsBaseImage: + if err := i.ExpandDependencies(ctx, EnvToMap(i.baseStageImage.Image.GetStageDescription().Info.Env)); err != nil { + return err } } @@ -257,6 +274,8 @@ func (i *Image) GetBaseImageReference() string { func (i *Image) FetchBaseImage(ctx context.Context) error { switch i.baseImageType { case ImageFromRegistryAsBaseImage: + // TODO: Refactor, move manifest fetching into SetupBaseImage, only pull image in FetchBaseImage method + if info, err := i.ContainerBackend.GetImageInfo(ctx, i.baseStageImage.Image.Name(), container_backend.GetImageInfoOpts{}); err != nil { return fmt.Errorf("unable to inspect local image %s: %w", i.baseStageImage.Image.Name(), err) } else if info != nil { @@ -348,3 +367,22 @@ func (i *Image) getFromBaseImageIdFromRegistry(ctx context.Context, reference st return i.baseImageRepoId, nil } + +func EnvToMap(env []string) map[string]string { + res := make(map[string]string) + for _, kv := range env { + k, v := parseKeyValue(kv) + res[k] = v + } + return res +} + +func parseKeyValue(env string) (string, string) { + parts := strings.SplitN(env, "=", 2) + v := "" + if len(parts) > 1 { + v = parts[1] + } + + return parts[0], v +} diff --git a/pkg/build/stage/base.go b/pkg/build/stage/base.go index ed3e1f07fb..ddc3ad705a 100644 --- a/pkg/build/stage/base.go +++ b/pkg/build/stage/base.go @@ -144,6 +144,10 @@ func (s *BaseStage) Name() StageName { panic("name must be defined!") } +func (s *BaseStage) ExpandDependencies(ctx context.Context, c Conveyor, baseEnv map[string]string) error { + return nil +} + func (s *BaseStage) FetchDependencies(_ context.Context, _ Conveyor, _ container_backend.ContainerBackend, _ docker_registry.ApiInterface) error { return nil } diff --git a/pkg/build/stage/instruction/add.go b/pkg/build/stage/instruction/add.go index 3ec0d5b89a..6ae43aac1b 100644 --- a/pkg/build/stage/instruction/add.go +++ b/pkg/build/stage/instruction/add.go @@ -24,10 +24,7 @@ func NewAdd(i *dockerfile.DockerfileStageInstruction[*instructions.AddCommand], } func (stg *Add) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Sources"}, stg.instruction.Data.Sources()...)...) args = append(args, "Dest", stg.instruction.Data.Dest()) diff --git a/pkg/build/stage/instruction/base.go b/pkg/build/stage/instruction/base.go index 95fc9d309f..caabc0f3bd 100644 --- a/pkg/build/stage/instruction/base.go +++ b/pkg/build/stage/instruction/base.go @@ -17,6 +17,9 @@ type Base[T dockerfile.InstructionDataInterface, BT container_backend.Instructio backendInstruction BT dependencies []*config.Dependency hasPrevStage bool + + baseEnv map[string]string + expandedEnv map[string]string } func NewBase[T dockerfile.InstructionDataInterface, BT container_backend.InstructionInterface](instruction *dockerfile.DockerfileStageInstruction[T], backendInstruction BT, dependencies []*config.Dependency, hasPrevStage bool, opts *stage.BaseStageOptions) *Base[T, BT] { @@ -41,11 +44,18 @@ func (stg *Base[T, BT]) UsesBuildContext() bool { return stg.backendInstruction.UsesBuildContext() } -func (stg *Base[T, BT]) getDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver, expander InstructionExpander) ([]string, error) { - if err := expander.ExpandInstruction(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive); err != nil { - return nil, fmt.Errorf("unable to expand instruction %q: %w", stg.instruction.Data.Name(), err) +func (stg *Base[T, BT]) ExpandDependencies(ctx context.Context, c stage.Conveyor, baseEnv map[string]string) error { + return stg.doExpandDependencies(ctx, c, baseEnv, stg) +} + +func (stg *Base[T, BT]) doExpandDependencies(ctx context.Context, c stage.Conveyor, baseEnv map[string]string, expander InstructionExpander) error { + if err := stg.expandBaseEnv(baseEnv); err != nil { + return fmt.Errorf("2nd stage env expansion failed: %w", err) } - return nil, nil + if err := expander.ExpandInstruction(c, stg.GetExpandedEnv(c)); err != nil { + return fmt.Errorf("2nd stage instruction expansion failed: %w", err) + } + return nil } func (stg *Base[T, BT]) PrepareImage(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevBuiltImage, stageImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) error { @@ -53,12 +63,46 @@ func (stg *Base[T, BT]) PrepareImage(ctx context.Context, c stage.Conveyor, cb c return nil } -func (stg *Base[T, BT]) ExpandInstruction(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevBuiltImage, stageImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) error { - dependenciesArgs := stage.ResolveDependenciesArgs(stg.dependencies, c) - // NOTE: do not skip unset envs during second stage expansion - return stg.instruction.Expand(dependenciesArgs, dockerfile.ExpandOptions{SkipUnsetEnv: false}) +func (stg *Base[T, BT]) expandBaseEnv(baseEnv map[string]string) error { + env := make(map[string]string) + + // NOTE: default fallback builtin PATH + env["PATH"] = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + for k, v := range baseEnv { + env[k] = v + } + + // NOTE: this is 2nd stage expansion (without skipping unset envs) + instructionEnv, err := stg.instruction.ExpandEnv(env, dockerfile.ExpandOptions{}) + if err != nil { + return fmt.Errorf("error expanding env: %w", err) + } + for k, v := range instructionEnv { + env[k] = v + } + + stg.baseEnv = baseEnv + stg.expandedEnv = env + + return nil +} + +func (stg *Base[T, BT]) GetExpandedEnv(c stage.Conveyor) map[string]string { + env := make(map[string]string) + for k, v := range stg.expandedEnv { + env[k] = v + } + for k, v := range stage.ResolveDependenciesArgs(stg.dependencies, c) { + env[k] = v + } + return env +} + +func (stg *Base[T, BT]) ExpandInstruction(_ stage.Conveyor, env map[string]string) error { + // NOTE: this is 2nd stage expansion (without skipping unset envs) + return stg.instruction.Expand(env, dockerfile.ExpandOptions{}) } type InstructionExpander interface { - ExpandInstruction(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevBuiltImage, stageImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) error + ExpandInstruction(c stage.Conveyor, env map[string]string) error } diff --git a/pkg/build/stage/instruction/cmd.go b/pkg/build/stage/instruction/cmd.go index 6b7140055e..77df54f362 100644 --- a/pkg/build/stage/instruction/cmd.go +++ b/pkg/build/stage/instruction/cmd.go @@ -23,10 +23,7 @@ func NewCmd(i *dockerfile.DockerfileStageInstruction[*instructions.CmdCommand], } func (stg *Cmd) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Cmd"}, stg.instruction.Data.CmdLine...)...) args = append(args, "PrependShell", fmt.Sprintf("%v", stg.instruction.Data.PrependShell)) diff --git a/pkg/build/stage/instruction/copy.go b/pkg/build/stage/instruction/copy.go index cb5c09cb9a..0762e444aa 100644 --- a/pkg/build/stage/instruction/copy.go +++ b/pkg/build/stage/instruction/copy.go @@ -22,8 +22,12 @@ func NewCopy(i *dockerfile.DockerfileStageInstruction[*instructions.CopyCommand] return &Copy{Base: NewBase(i, backend_instruction.NewCopy(i.Data), dependencies, hasPrevStage, opts)} } -func (stg *Copy) ExpandInstruction(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevBuiltImage, stageImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) error { - if err := stg.Base.ExpandInstruction(ctx, c, cb, prevBuiltImage, stageImage, buildContextArchive); err != nil { +func (stg *Copy) ExpandDependencies(ctx context.Context, c stage.Conveyor, baseEnv map[string]string) error { + return stg.doExpandDependencies(ctx, c, baseEnv, stg) +} + +func (stg *Copy) ExpandInstruction(c stage.Conveyor, env map[string]string) error { + if err := stg.Base.ExpandInstruction(c, env); err != nil { return err } @@ -38,10 +42,7 @@ func (stg *Copy) ExpandInstruction(ctx context.Context, c stage.Conveyor, cb con } func (stg *Copy) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "From", stg.instruction.Data.From) args = append(args, append([]string{"Sources"}, stg.instruction.Data.Sources()...)...) diff --git a/pkg/build/stage/instruction/entrypoint.go b/pkg/build/stage/instruction/entrypoint.go index df9d7c6554..65bc45ecf4 100644 --- a/pkg/build/stage/instruction/entrypoint.go +++ b/pkg/build/stage/instruction/entrypoint.go @@ -23,12 +23,10 @@ func NewEntrypoint(i *dockerfile.DockerfileStageInstruction[*instructions.Entryp } func (stg *Entrypoint) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Entrypoint"}, stg.instruction.Data.CmdLine...)...) args = append(args, "PrependShell", fmt.Sprintf("%v", stg.instruction.Data.PrependShell)) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/env.go b/pkg/build/stage/instruction/env.go index f42855507c..4cfca277c9 100644 --- a/pkg/build/stage/instruction/env.go +++ b/pkg/build/stage/instruction/env.go @@ -22,10 +22,7 @@ func NewEnv(i *dockerfile.DockerfileStageInstruction[*instructions.EnvCommand], } func (stg *Env) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string if len(stg.instruction.Data.Env) > 0 { args = append(args, "Env") @@ -33,5 +30,6 @@ func (stg *Env) GetDependencies(ctx context.Context, c stage.Conveyor, cb contai args = append(args, item.Key, item.Value) } } + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/expose.go b/pkg/build/stage/instruction/expose.go index 266739bd6a..c65b694eb2 100644 --- a/pkg/build/stage/instruction/expose.go +++ b/pkg/build/stage/instruction/expose.go @@ -22,11 +22,9 @@ func NewExpose(i *dockerfile.DockerfileStageInstruction[*instructions.ExposeComm } func (stg *Expose) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Ports"}, stg.instruction.Data.Ports...)...) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/healthcheck.go b/pkg/build/stage/instruction/healthcheck.go index bbcbae0b58..d0f179eba3 100644 --- a/pkg/build/stage/instruction/healthcheck.go +++ b/pkg/build/stage/instruction/healthcheck.go @@ -23,15 +23,13 @@ func NewHealthcheck(i *dockerfile.DockerfileStageInstruction[*instructions.Healt } func (stg *Healthcheck) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Test"}, stg.instruction.Data.Health.Test...)...) args = append(args, "Interval", stg.instruction.Data.Health.Interval.String()) args = append(args, "Timeout", stg.instruction.Data.Health.Timeout.String()) args = append(args, "StartPeriod", stg.instruction.Data.Health.StartPeriod.String()) args = append(args, "Retries", fmt.Sprintf("%d", stg.instruction.Data.Health.Retries)) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/label.go b/pkg/build/stage/instruction/label.go index d4f5b8b14c..989786612e 100644 --- a/pkg/build/stage/instruction/label.go +++ b/pkg/build/stage/instruction/label.go @@ -22,10 +22,7 @@ func NewLabel(i *dockerfile.DockerfileStageInstruction[*instructions.LabelComman } func (stg *Label) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string if len(stg.instruction.Data.Labels) > 0 { args = append(args, "Labels") @@ -33,5 +30,6 @@ func (stg *Label) GetDependencies(ctx context.Context, c stage.Conveyor, cb cont args = append(args, item.Key, item.Value) } } + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/maintainer.go b/pkg/build/stage/instruction/maintainer.go index 3d19a28370..5a60b687b2 100644 --- a/pkg/build/stage/instruction/maintainer.go +++ b/pkg/build/stage/instruction/maintainer.go @@ -22,11 +22,9 @@ func NewMaintainer(i *dockerfile.DockerfileStageInstruction[*instructions.Mainta } func (stg *Maintainer) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "Maintainer", stg.instruction.Data.Maintainer) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/on_build.go b/pkg/build/stage/instruction/on_build.go index f67a62dba9..9b954aa478 100644 --- a/pkg/build/stage/instruction/on_build.go +++ b/pkg/build/stage/instruction/on_build.go @@ -22,11 +22,9 @@ func NewOnBuild(i *dockerfile.DockerfileStageInstruction[*instructions.OnbuildCo } func (stg *OnBuild) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "Expression", stg.instruction.Data.Expression) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/run.go b/pkg/build/stage/instruction/run.go index eefeaffd2f..2de208f095 100644 --- a/pkg/build/stage/instruction/run.go +++ b/pkg/build/stage/instruction/run.go @@ -3,6 +3,7 @@ package instruction import ( "context" "fmt" + "sort" "github.com/moby/buildkit/frontend/dockerfile/instructions" @@ -19,19 +20,30 @@ type Run struct { } func NewRun(i *dockerfile.DockerfileStageInstruction[*instructions.RunCommand], dependencies []*config.Dependency, hasPrevStage bool, opts *stage.BaseStageOptions) *Run { - return &Run{Base: NewBase(i, backend_instruction.NewRun(i.Data), dependencies, hasPrevStage, opts)} + return &Run{Base: NewBase(i, backend_instruction.NewRun(i.Data, nil), dependencies, hasPrevStage, opts)} } -func (stg *Run) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err +func (stg *Run) ExpandDependencies(ctx context.Context, c stage.Conveyor, baseEnv map[string]string) error { + return stg.doExpandDependencies(ctx, c, baseEnv, stg) +} + +func (stg *Run) ExpandInstruction(c stage.Conveyor, env map[string]string) error { + if err := stg.Base.ExpandInstruction(c, env); err != nil { + return err } + // Setup RUN envs after 2nd stage expansion + stg.backendInstruction.Envs = EnvToSortedArr(stg.GetExpandedEnv(c)) + return nil +} + +func (stg *Run) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { + var args []string network := instructions.GetNetwork(stg.instruction.Data) security := instructions.GetSecurity(stg.instruction.Data) mounts := instructions.GetMounts(stg.instruction.Data) + args = append(args, append([]string{"Env"}, EnvToSortedArr(stg.GetExpandedEnv(c))...)...) args = append(args, append([]string{"Command"}, stg.instruction.Data.CmdLine...)...) args = append(args, "PrependShell", fmt.Sprintf("%v", stg.instruction.Data.PrependShell)) args = append(args, "Network", network) @@ -80,3 +92,11 @@ func (stg *Run) GetDependencies(ctx context.Context, c stage.Conveyor, cb contai return util.Sha256Hash(args...), nil } + +func EnvToSortedArr(env map[string]string) (r []string) { + for k, v := range env { + r = append(r, fmt.Sprintf("%s=%s", k, v)) + } + sort.Strings(r) + return +} diff --git a/pkg/build/stage/instruction/shell.go b/pkg/build/stage/instruction/shell.go index 151d640f48..416df205a0 100644 --- a/pkg/build/stage/instruction/shell.go +++ b/pkg/build/stage/instruction/shell.go @@ -22,11 +22,9 @@ func NewShell(i *dockerfile.DockerfileStageInstruction[*instructions.ShellComman } func (stg *Shell) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Shell"}, stg.instruction.Data.Shell...)...) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/stop_signal.go b/pkg/build/stage/instruction/stop_signal.go index f0df4b2a92..8c084d08fb 100644 --- a/pkg/build/stage/instruction/stop_signal.go +++ b/pkg/build/stage/instruction/stop_signal.go @@ -22,11 +22,9 @@ func NewStopSignal(i *dockerfile.DockerfileStageInstruction[*instructions.StopSi } func (stg *StopSignal) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "Signal", stg.instruction.Data.Signal) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/user.go b/pkg/build/stage/instruction/user.go index 890e0c06fe..b27f913b11 100644 --- a/pkg/build/stage/instruction/user.go +++ b/pkg/build/stage/instruction/user.go @@ -22,11 +22,9 @@ func NewUser(i *dockerfile.DockerfileStageInstruction[*instructions.UserCommand] } func (stg *User) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "User", stg.instruction.Data.User) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/volume.go b/pkg/build/stage/instruction/volume.go index 962ebe7e69..6123a8daa2 100644 --- a/pkg/build/stage/instruction/volume.go +++ b/pkg/build/stage/instruction/volume.go @@ -22,11 +22,9 @@ func NewVolume(i *dockerfile.DockerfileStageInstruction[*instructions.VolumeComm } func (stg *Volume) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, append([]string{"Volumes"}, stg.instruction.Data.Volumes...)...) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/instruction/workdir.go b/pkg/build/stage/instruction/workdir.go index 1e8bf8c717..7ae9623cc9 100644 --- a/pkg/build/stage/instruction/workdir.go +++ b/pkg/build/stage/instruction/workdir.go @@ -22,11 +22,9 @@ func NewWorkdir(i *dockerfile.DockerfileStageInstruction[*instructions.WorkdirCo } func (stg *Workdir) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) { - args, err := stg.getDependencies(ctx, c, cb, prevImage, prevBuiltImage, buildContextArchive, stg) - if err != nil { - return "", err - } + var args []string args = append(args, "Path", stg.instruction.Data.Path) + return util.Sha256Hash(args...), nil } diff --git a/pkg/build/stage/interface.go b/pkg/build/stage/interface.go index bf7a4b4700..a9cbd424c6 100644 --- a/pkg/build/stage/interface.go +++ b/pkg/build/stage/interface.go @@ -14,6 +14,7 @@ type Interface interface { IsEmpty(ctx context.Context, c Conveyor, prevBuiltImage *StageImage) (bool, error) + ExpandDependencies(ctx context.Context, c Conveyor, baseEnv map[string]string) error FetchDependencies(ctx context.Context, c Conveyor, cb container_backend.ContainerBackend, dockerRegistry docker_registry.ApiInterface) error GetDependencies(ctx context.Context, c Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) GetNextStageDependencies(ctx context.Context, c Conveyor) (string, error) diff --git a/pkg/build/stages_iterator.go b/pkg/build/stages_iterator.go index 241363a15f..a6f30b19aa 100644 --- a/pkg/build/stages_iterator.go +++ b/pkg/build/stages_iterator.go @@ -26,7 +26,8 @@ func (iterator *StagesIterator) GetPrevImage(img *build_image.Image, stg stage.I if stg.HasPrevStage() { return iterator.PrevNonEmptyStage.GetStageImage() } else if stg.IsStapelStage() && stg.Name() == "from" { - // TODO(staged-dockerfile): this is only for compatibility with stapel-builder logic, and this should be unified with new staged-dockerfile logic + return img.GetBaseStageImage() + } else if img.IsDockerfileImage && img.DockerfileImageConfig.Staged { return img.GetBaseStageImage() } return nil @@ -36,7 +37,8 @@ func (iterator *StagesIterator) GetPrevBuiltImage(img *build_image.Image, stg st if stg.HasPrevStage() { return iterator.PrevBuiltStage.GetStageImage() } else if stg.IsStapelStage() && stg.Name() == "from" { - // TODO(staged-dockerfile): this is only for compatibility with stapel-builder logic, and this should be unified with new staged-dockerfile logic + return img.GetBaseStageImage() + } else if img.IsDockerfileImage && img.DockerfileImageConfig.Staged { return img.GetBaseStageImage() } return nil diff --git a/pkg/container_backend/buildah_backend.go b/pkg/container_backend/buildah_backend.go index 05df428128..befa5899a2 100644 --- a/pkg/container_backend/buildah_backend.go +++ b/pkg/container_backend/buildah_backend.go @@ -578,6 +578,7 @@ func (runtime *BuildahBackend) GetImageInfo(ctx context.Context, ref string, opt CreatedAtUnixNano: inspect.Docker.Created.UnixNano(), RepoDigest: inspect.FromImageDigest, OnBuild: inspect.Docker.Config.OnBuild, + Env: inspect.Docker.Config.Env, ID: inspect.FromImageID, ParentID: parentID, Size: inspect.Docker.Size, diff --git a/pkg/container_backend/instruction/run.go b/pkg/container_backend/instruction/run.go index 3a9b68bd01..dd6fa567a5 100644 --- a/pkg/container_backend/instruction/run.go +++ b/pkg/container_backend/instruction/run.go @@ -14,10 +14,11 @@ import ( type Run struct { *instructions.RunCommand + Envs []string } -func NewRun(i *instructions.RunCommand) *Run { - return &Run{RunCommand: i} +func NewRun(i *instructions.RunCommand, envs []string) *Run { + return &Run{RunCommand: i, Envs: envs} } func (i *Run) UsesBuildContext() bool { @@ -66,6 +67,7 @@ func (i *Run) Apply(ctx context.Context, containerName string, drv buildah.Build AddCapabilities: addCapabilities, NetworkType: i.GetNetwork(), RunMounts: i.GetMounts(), + Envs: i.Envs, }); err != nil { return fmt.Errorf("error running command %v for container %s: %w", i.CmdLine, containerName, err) } diff --git a/pkg/docker_registry/api.go b/pkg/docker_registry/api.go index 163119cf56..38c9ee01f5 100644 --- a/pkg/docker_registry/api.go +++ b/pkg/docker_registry/api.go @@ -161,6 +161,8 @@ func (api *api) GetRepoImage(_ context.Context, reference string) (*image.Info, RepoDigest: digest.String(), ParentID: parentID, Labels: configFile.Config.Labels, + OnBuild: configFile.Config.OnBuild, + Env: configFile.Config.Env, Size: totalSize, } diff --git a/pkg/dockerfile/frontend/buildkit_dockerfile.go b/pkg/dockerfile/frontend/buildkit_dockerfile.go index 8d2b0d4dbb..4b5cacde1c 100644 --- a/pkg/dockerfile/frontend/buildkit_dockerfile.go +++ b/pkg/dockerfile/frontend/buildkit_dockerfile.go @@ -62,15 +62,14 @@ func ParseDockerfileWithBuildkit(dockerfileBytes []byte, opts dockerfile.Dockerf func NewDockerfileStageFromBuildkitStage(index int, stage instructions.Stage, expanderFactory *ShlexExpanderFactory, metaArgs, buildArgs map[string]string, dependenciesArgsKeys []string) (*dockerfile.DockerfileStage, error) { var stageInstructions []dockerfile.DockerfileStageInstructionInterface - env := map[string]string{} - opts := dockerfile.DockerfileStageInstructionOptions{ExpanderFactory: expanderFactory} + env := make(map[string]string) for _, cmd := range stage.Commands { var i dockerfile.DockerfileStageInstructionInterface switch instrData := cmd.(type) { case *instructions.AddCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr @@ -78,7 +77,7 @@ func NewDockerfileStageFromBuildkitStage(index int, stage instructions.Stage, ex case *instructions.ArgCommand: instrData.Args = removeDependenciesArgs(instrData.Args, dependenciesArgsKeys) - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr @@ -102,25 +101,25 @@ func NewDockerfileStageFromBuildkitStage(index int, stage instructions.Stage, ex } } case *instructions.CmdCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.CopyCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.EntrypointCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.EnvCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr @@ -130,67 +129,67 @@ func NewDockerfileStageFromBuildkitStage(index int, stage instructions.Stage, ex } } case *instructions.ExposeCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.HealthCheckCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.LabelCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.MaintainerCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.OnbuildCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.RunCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.ShellCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.StopSignalCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.UserCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.VolumeCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr } case *instructions.WorkdirCommand: - if instr, err := createAndExpandInstruction(instrData, env, opts); err != nil { + if instr, err := createAndExpandInstruction(instrData, expanderFactory, env); err != nil { return nil, err } else { i = instr @@ -203,11 +202,19 @@ func NewDockerfileStageFromBuildkitStage(index int, stage instructions.Stage, ex return dockerfile.NewDockerfileStage(index, stage.BaseName, stage.Name, stageInstructions, stage.Platform, expanderFactory), nil } -func createAndExpandInstruction[T dockerfile.InstructionDataInterface](data T, env map[string]string, opts dockerfile.DockerfileStageInstructionOptions) (*dockerfile.DockerfileStageInstruction[T], error) { +func createAndExpandInstruction[T dockerfile.InstructionDataInterface](data T, expanderFactory dockerfile.ExpanderFactory, env map[string]string) (*dockerfile.DockerfileStageInstruction[T], error) { + opts := dockerfile.DockerfileStageInstructionOptions{ + ExpanderFactory: expanderFactory, + Env: make(map[string]string), + } + for k, v := range env { + opts.Env[k] = v + } + i := dockerfile.NewDockerfileStageInstruction(data, opts) - // NOTE: skip unset envs during first stage expansion - if err := i.Expand(env, dockerfile.ExpandOptions{SkipUnsetEnv: true}); err != nil { + // NOTE: 1st stage expansion (skip unset envs) + if err := i.Expand(opts.Env, dockerfile.ExpandOptions{SkipUnsetEnv: true}); err != nil { return nil, fmt.Errorf("unable to expand instruction %q: %w", i.GetInstructionData().Name(), err) } diff --git a/pkg/dockerfile/instruction.go b/pkg/dockerfile/instruction.go index 706815dfff..5cf41bda09 100644 --- a/pkg/dockerfile/instruction.go +++ b/pkg/dockerfile/instruction.go @@ -19,6 +19,7 @@ type DockerfileStageInstructionInterface interface { GetDependencyByStageRef(ref string) *DockerfileStage GetDependenciesByStageRef() map[string]*DockerfileStage GetInstructionData() InstructionDataInterface + ExpandEnv(baseEnv map[string]string, opts ExpandOptions) (map[string]string, error) Expand(env map[string]string, opts ExpandOptions) error } @@ -32,11 +33,13 @@ type Expander interface { } type DockerfileStageInstructionOptions struct { + Env map[string]string ExpanderFactory ExpanderFactory } type DockerfileStageInstruction[T InstructionDataInterface] struct { Data T + Env map[string]string DependenciesByStageRef map[string]*DockerfileStage ExpanderFactory ExpanderFactory } @@ -46,6 +49,7 @@ func NewDockerfileStageInstruction[T InstructionDataInterface](data T, opts Dock Data: data, DependenciesByStageRef: make(map[string]*DockerfileStage), ExpanderFactory: opts.ExpanderFactory, + Env: opts.Env, } } @@ -71,6 +75,27 @@ func (i *DockerfileStageInstruction[T]) GetInstructionData() InstructionDataInte return i.Data } +func (i *DockerfileStageInstruction[T]) SetEnvVar(key, value string) { + i.Env[key] = value +} + +func (i *DockerfileStageInstruction[T]) ExpandEnv(baseEnv map[string]string, opts ExpandOptions) (map[string]string, error) { + if i.ExpanderFactory == nil { + return i.Env, nil + } + expander := i.ExpanderFactory.GetExpander(opts) + + res := make(map[string]string) + for k, v := range i.Env { + newValue, err := expander.ProcessWordWithMap(v, baseEnv) + if err != nil { + return nil, fmt.Errorf("error processing word %q: %w", v, err) + } + res[k] = newValue + } + return res, nil +} + func (i *DockerfileStageInstruction[T]) Expand(env map[string]string, opts ExpandOptions) error { if i.ExpanderFactory == nil { return nil @@ -80,7 +105,8 @@ func (i *DockerfileStageInstruction[T]) Expand(env map[string]string, opts Expan switch instr := any(i.Data).(type) { case instructions.SupportsSingleWordExpansion: return instr.Expand(func(word string) (string, error) { - return expander.ProcessWordWithMap(word, env) + v, err := expander.ProcessWordWithMap(word, env) + return v, err }) case *instructions.ExposeCommand: @@ -95,20 +121,6 @@ func (i *DockerfileStageInstruction[T]) Expand(env map[string]string, opts Expan ports = append(ports, ps...) } instr.Ports = ports - - case *instructions.RunCommand: - var newCmdLine []string - for _, line := range instr.CmdLine { - exline, err := expander.ProcessWordWithMap(line, env) - if err != nil { - return fmt.Errorf("unable to expand cmd line %q: %w", line, err) - } - newCmdLine = append(newCmdLine, exline) - } - instr.ShellDependantCmdLine = instructions.ShellDependantCmdLine{ - CmdLine: newCmdLine, - PrependShell: instr.PrependShell, - } } return nil diff --git a/pkg/image/info.go b/pkg/image/info.go index 5bf51fb656..63ae40b0d6 100644 --- a/pkg/image/info.go +++ b/pkg/image/info.go @@ -18,6 +18,7 @@ type Info struct { RepoDigest string `json:"repoDigest"` OnBuild []string `json:"onBuild"` + Env []string `json:"env"` ID string `json:"ID"` ParentID string `json:"parentID"` Labels map[string]string `json:"labels"` @@ -44,6 +45,7 @@ func (info *Info) GetCopy() *Info { Tag: info.Tag, RepoDigest: info.RepoDigest, OnBuild: util.CopyArr(info.OnBuild), + Env: util.CopyArr(info.Env), ID: info.ID, ParentID: info.ParentID, Labels: util.CopyMap(info.Labels), @@ -75,6 +77,7 @@ func NewInfoFromInspect(ref string, inspect *types.ImageInspect) *Info { Tag: tag, Labels: inspect.Config.Labels, OnBuild: inspect.Config.OnBuild, + Env: inspect.Config.Env, CreatedAtUnixNano: MustParseTimestampString(inspect.Created).UnixNano(), RepoDigest: repoDigest, ID: inspect.ID, @@ -109,6 +112,7 @@ func NewImageInfoFromRegistryConfig(ref string, cfg *v1.ConfigFile) *Info { Tag: tag, Labels: cfg.Config.Labels, OnBuild: cfg.Config.OnBuild, + Env: cfg.Config.Env, CreatedAtUnixNano: cfg.Created.UnixNano(), RepoDigest: "", // TODO ID: "", // TODO diff --git a/pkg/storage/manager/storage_manager.go b/pkg/storage/manager/storage_manager.go index 6a909df093..f0b6a87230 100644 --- a/pkg/storage/manager/storage_manager.go +++ b/pkg/storage/manager/storage_manager.go @@ -923,6 +923,8 @@ func ConvertStageDescriptionForStagesStorage(stageDesc *image.StageDescription, Labels: stageDesc.Info.Labels, Size: stageDesc.Info.Size, CreatedAtUnixNano: stageDesc.Info.CreatedAtUnixNano, + OnBuild: stageDesc.Info.OnBuild, + Env: stageDesc.Info.Env, }, } }