From e6f90c1dc78ff2c979ffaece11983d6f1c01a156 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Mon, 31 Jan 2022 22:45:07 +0300 Subject: [PATCH] fix(dockerfile): validate base image resolved to non-empty image Signed-off-by: Timofey Kirillov --- pkg/build/build_phase.go | 3 +- pkg/build/stage/base.go | 3 +- pkg/build/stage/dockerfile.go | 16 ++++- pkg/build/stage/dockerfile_test.go | 110 +++++++++++++++++++---------- pkg/build/stage/interface.go | 3 +- pkg/build/stage/stubs_test.go | 38 ++++++++++ pkg/docker_registry/interface.go | 11 +++ 7 files changed, 141 insertions(+), 43 deletions(-) create mode 100644 pkg/docker_registry/interface.go diff --git a/pkg/build/build_phase.go b/pkg/build/build_phase.go index ca6f310095..022e95fd73 100644 --- a/pkg/build/build_phase.go +++ b/pkg/build/build_phase.go @@ -20,6 +20,7 @@ import ( "github.com/werf/logboek/pkg/types" "github.com/werf/werf/pkg/build/stage" "github.com/werf/werf/pkg/container_runtime" + "github.com/werf/werf/pkg/docker_registry" "github.com/werf/werf/pkg/git_repo" imagePkg "github.com/werf/werf/pkg/image" "github.com/werf/werf/pkg/stapel" @@ -376,7 +377,7 @@ func (phase *BuildPhase) onImageStage(ctx context.Context, img *Image, stg stage return nil } - if err := stg.FetchDependencies(ctx, phase.Conveyor, phase.Conveyor.ContainerRuntime); err != nil { + if err := stg.FetchDependencies(ctx, phase.Conveyor, phase.Conveyor.ContainerRuntime, docker_registry.API()); err != nil { return fmt.Errorf("unable to fetch dependencies for stage %s: %s", stg.LogDetailedName(), err) } diff --git a/pkg/build/stage/base.go b/pkg/build/stage/base.go index 7294e550e2..a0d742d04a 100644 --- a/pkg/build/stage/base.go +++ b/pkg/build/stage/base.go @@ -12,6 +12,7 @@ import ( "github.com/werf/logboek" "github.com/werf/werf/pkg/config" "github.com/werf/werf/pkg/container_runtime" + "github.com/werf/werf/pkg/docker_registry" imagePkg "github.com/werf/werf/pkg/image" "github.com/werf/werf/pkg/slug" "github.com/werf/werf/pkg/util" @@ -121,7 +122,7 @@ func (s *BaseStage) Name() StageName { panic("name must be defined!") } -func (s *BaseStage) FetchDependencies(_ context.Context, _ Conveyor, _ container_runtime.ContainerRuntime) error { +func (s *BaseStage) FetchDependencies(_ context.Context, _ Conveyor, _ container_runtime.ContainerRuntime, _ docker_registry.DockerRegistryInterface) error { return nil } diff --git a/pkg/build/stage/dockerfile.go b/pkg/build/stage/dockerfile.go index 81b98e3cad..b5bc58b46a 100644 --- a/pkg/build/stage/dockerfile.go +++ b/pkg/build/stage/dockerfile.go @@ -27,6 +27,12 @@ import ( "github.com/werf/werf/pkg/util" ) +var ErrInvalidBaseImage = errors.New("invalid base image") + +func IsErrInvalidBaseImage(err error) bool { + return err != nil && errors.Is(err, ErrInvalidBaseImage) +} + func GenerateDockerfileStage(dockerRunArgs *DockerRunArgs, dockerStages *DockerStages, contextChecksum *ContextChecksum, baseStageOptions *NewBaseStageOptions, dependencies []*config.Dependency) *DockerfileStage { return newDockerfileStage(dockerRunArgs, dockerStages, contextChecksum, baseStageOptions, dependencies) } @@ -312,7 +318,7 @@ type dockerfileInstructionInterface interface { Name() string } -func (s *DockerfileStage) FetchDependencies(ctx context.Context, c Conveyor, containerRuntime container_runtime.ContainerRuntime) error { +func (s *DockerfileStage) FetchDependencies(ctx context.Context, c Conveyor, containerRuntime container_runtime.ContainerRuntime, dockerRegistry docker_registry.DockerRegistryInterface) error { resolvedDependenciesArgsHash := resolveDependenciesArgsHash(s.dependencies, c) resolvedDockerMetaArgsHash, err := s.resolveDockerMetaArgs(resolvedDependenciesArgsHash) @@ -327,6 +333,10 @@ outerLoop: return err } + if resolvedBaseName == "" { + return ErrInvalidBaseImage + } + for relatedStageIndex, relatedStage := range s.dockerStages { if ind == relatedStageIndex { continue @@ -356,9 +366,9 @@ outerLoop: } getBaseImageOnBuildRemotely := func() ([]string, error) { - configFile, err := docker_registry.API().GetRepoImageConfigFile(ctx, resolvedBaseName) + configFile, err := dockerRegistry.GetRepoImageConfigFile(ctx, resolvedBaseName) if err != nil { - return nil, fmt.Errorf("get repo image %s config file failed: %s", resolvedBaseName, err) + return nil, fmt.Errorf("get repo image %q config file failed: %s", resolvedBaseName, err) } return configFile.Config.OnBuild, nil diff --git a/pkg/build/stage/dockerfile_test.go b/pkg/build/stage/dockerfile_test.go index fdc24effc6..1f88a31951 100644 --- a/pkg/build/stage/dockerfile_test.go +++ b/pkg/build/stage/dockerfile_test.go @@ -13,6 +13,51 @@ import ( "github.com/werf/werf/pkg/util" ) +func testDockerfileToDockerStages(dockerfile []byte) ([]instructions.Stage, []instructions.ArgCommand) { + p, err := parser.Parse(bytes.NewReader(dockerfile)) + Expect(err).To(Succeed()) + + dockerStages, dockerMetaArgs, err := instructions.Parse(p.AST) + Expect(err).To(Succeed()) + + dockerfile_helpers.ResolveDockerStagesFromValue(dockerStages) + + return dockerStages, dockerMetaArgs +} + +func newTestDockerfileStage(dockerfile []byte, target string, buildArgs map[string]interface{}, dockerStages []instructions.Stage, dockerMetaArgs []instructions.ArgCommand, dependencies []*TestDependency) *DockerfileStage { + dockerTargetIndex, err := dockerfile_helpers.GetDockerTargetStageIndex(dockerStages, target) + Expect(err).To(Succeed()) + + ds := NewDockerStages( + dockerStages, + util.MapStringInterfaceToMapStringString(buildArgs), + dockerMetaArgs, + dockerTargetIndex, + ) + + return newDockerfileStage( + NewDockerRunArgs( + dockerfile, + "no-such-path", + target, + "", + nil, + buildArgs, + nil, + "", + "", + ), + ds, + NewContextChecksum(nil), + &NewBaseStageOptions{ + ImageName: "example-image", + ProjectName: "example-project", + }, + GetConfigDependencies(dependencies), + ) +} + var _ = Describe("DockerfileStage", func() { DescribeTable("configuring images dependencies for dockerfile stage", func(data TestDockerfileDependencies) { @@ -20,44 +65,9 @@ var _ = Describe("DockerfileStage", func() { conveyor := NewConveyorStubForDependencies(NewGiterminismManagerStub(NewLocalGitRepoStub("9d8059842b6fde712c58315ca0ab4713d90761c0")), data.TestDependencies.Dependencies) - p, err := parser.Parse(bytes.NewReader(data.Dockerfile)) - Expect(err).To(Succeed()) - - dockerStages, dockerMetaArgs, err := instructions.Parse(p.AST) - Expect(err).To(Succeed()) + dockerStages, dockerMetaArgs := testDockerfileToDockerStages(data.Dockerfile) - dockerfile_helpers.ResolveDockerStagesFromValue(dockerStages) - - dockerTargetIndex, err := dockerfile_helpers.GetDockerTargetStageIndex(dockerStages, data.Target) - Expect(err).To(Succeed()) - - ds := NewDockerStages( - dockerStages, - util.MapStringInterfaceToMapStringString(data.BuildArgs), - dockerMetaArgs, - dockerTargetIndex, - ) - - stage := newDockerfileStage( - NewDockerRunArgs( - data.Dockerfile, - "no-such-path", - data.Target, - "", - nil, - data.BuildArgs, - nil, - "", - "", - ), - ds, - NewContextChecksum(nil), - &NewBaseStageOptions{ - ImageName: "example-image", - ProjectName: "example-project", - }, - GetConfigDependencies(data.TestDependencies.Dependencies), - ) + stage := newTestDockerfileStage(data.Dockerfile, data.Target, data.BuildArgs, dockerStages, dockerMetaArgs, data.TestDependencies.Dependencies) img := NewLegacyImageStub() @@ -260,6 +270,32 @@ RUN echo hello }, ), ) + + When("Dockerfile uses undefined build argument", func() { + It("should report descriptive error when fetching dockerfile stage dependencies", func() { + dockerfile := []byte(` +ARG BASE_NAME=alpine:latest + +FROM ${BASE_NAME1} +RUN echo hello +`) + + ctx := context.Background() + + conveyor := NewConveyorStubForDependencies(NewGiterminismManagerStub(NewLocalGitRepoStub("9d8059842b6fde712c58315ca0ab4713d90761c0")), nil) + + dockerStages, dockerMetaArgs := testDockerfileToDockerStages(dockerfile) + + stage := newTestDockerfileStage(dockerfile, "", nil, dockerStages, dockerMetaArgs, nil) + + containerRuntime := NewContainerRuntimeMock() + + dockerRegistry := NewDockerRegistryStub() + + err := stage.FetchDependencies(ctx, conveyor, containerRuntime, dockerRegistry) + Expect(IsErrInvalidBaseImage(err)).To(BeTrue()) + }) + }) }) type TestDockerfileDependencies struct { diff --git a/pkg/build/stage/interface.go b/pkg/build/stage/interface.go index ace2a45e12..e899934e6c 100644 --- a/pkg/build/stage/interface.go +++ b/pkg/build/stage/interface.go @@ -4,6 +4,7 @@ import ( "context" "github.com/werf/werf/pkg/container_runtime" + "github.com/werf/werf/pkg/docker_registry" "github.com/werf/werf/pkg/image" ) @@ -13,7 +14,7 @@ type Interface interface { IsEmpty(ctx context.Context, c Conveyor, prevBuiltImage container_runtime.LegacyImageInterface) (bool, error) - FetchDependencies(ctx context.Context, c Conveyor, cr container_runtime.ContainerRuntime) error + FetchDependencies(ctx context.Context, c Conveyor, cr container_runtime.ContainerRuntime, dockerRegistry docker_registry.DockerRegistryInterface) error GetDependencies(ctx context.Context, c Conveyor, prevImage container_runtime.LegacyImageInterface, prevBuiltImage container_runtime.LegacyImageInterface) (string, error) GetNextStageDependencies(ctx context.Context, c Conveyor) (string, error) diff --git a/pkg/build/stage/stubs_test.go b/pkg/build/stage/stubs_test.go index 9506f28e03..36f141428d 100644 --- a/pkg/build/stage/stubs_test.go +++ b/pkg/build/stage/stubs_test.go @@ -3,11 +3,14 @@ package stage import ( "context" + v1 "github.com/google/go-containerregistry/pkg/v1" . "github.com/onsi/gomega" "github.com/werf/werf/pkg/container_runtime" + "github.com/werf/werf/pkg/docker_registry" "github.com/werf/werf/pkg/git_repo" "github.com/werf/werf/pkg/giterminism_manager" + "github.com/werf/werf/pkg/image" ) type LegacyImageStub struct { @@ -167,3 +170,38 @@ func NewGitRepoArchiveStub() *GitRepoArchiveStub { func (archive *GitRepoArchiveStub) GetFilePath() string { return "no-such-file" } + +type ContainerRuntimeMock struct { + container_runtime.ContainerRuntime + + _PulledImages map[string]bool +} + +func NewContainerRuntimeMock() *ContainerRuntimeMock { + return &ContainerRuntimeMock{ + _PulledImages: make(map[string]bool), + } +} + +func (containerRuntime *ContainerRuntimeMock) GetImageInfo(ctx context.Context, ref string, opts container_runtime.GetImageInfoOpts) (*image.Info, error) { + return nil, nil +} + +func (containerRuntime *ContainerRuntimeMock) Pull(ctx context.Context, ref string, opts container_runtime.PullOpts) error { + containerRuntime._PulledImages[ref] = true + return nil +} + +type DockerRegistryStub struct { + docker_registry.DockerRegistryInterface +} + +func NewDockerRegistryStub() *DockerRegistryStub { + return &DockerRegistryStub{} +} + +func (dockerRegistry *DockerRegistryStub) GetRepoImageConfigFile(ctx context.Context, reference string) (*v1.ConfigFile, error) { + return &v1.ConfigFile{ + Config: v1.Config{}, + }, nil +} diff --git a/pkg/docker_registry/interface.go b/pkg/docker_registry/interface.go new file mode 100644 index 0000000000..6293839fdc --- /dev/null +++ b/pkg/docker_registry/interface.go @@ -0,0 +1,11 @@ +package docker_registry + +import ( + "context" + + v1 "github.com/google/go-containerregistry/pkg/v1" +) + +type DockerRegistryInterface interface { + GetRepoImageConfigFile(ctx context.Context, reference string) (*v1.ConfigFile, error) +}