fix(dockerfile): validate base image resolved to non-empty image

Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
werf · Jan 31, 2022 · e6f90c1 · e6f90c1
1 parent b58eb07
commit e6f90c1
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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
@@ -13,51 +13,61 @@ 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) {
 			ctx := context.Background()
 
 			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
@@ -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
@@ -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
@@ -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)
+}