From 6ee30febc0ea5bc2631e180f3903d14a460c8bb0 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Mon, 24 Jul 2017 17:01:24 +0200
Subject: [PATCH] Fix allowed_images behavior

---
 executors/docker/executor_docker.go           | 12 +++----
 executors/docker/executor_docker_command.go   |  4 +--
 .../docker/executor_docker_command_test.go    | 34 +++++++++++++++++++
 executors/docker/executor_docker_ssh.go       |  2 +-
 executors/docker/executor_docker_test.go      |  4 +--
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/executors/docker/executor_docker.go b/executors/docker/executor_docker.go
index ac9b362e8..71d97e870 100644
--- a/executors/docker/executor_docker.go
+++ b/executors/docker/executor_docker.go
@@ -713,14 +713,13 @@ func (s *executor) createServices() (err error) {
 	}
 
 	s.waitForServices()
-	fmt.Println(linksMap)
 
 	s.links = s.buildServiceLinks(linksMap)
 	return
 }
 
-func (s *executor) createContainer(containerType string, imageDefinition common.Image, cmd []string) (*types.ContainerJSON, error) {
-	imageName, err := s.expandImageName(imageDefinition.Name)
+func (s *executor) createContainer(containerType string, imageDefinition common.Image, cmd []string, allowedInternalImages []string) (*types.ContainerJSON, error) {
+	imageName, err := s.expandImageName(imageDefinition.Name, allowedInternalImages)
 	if err != nil {
 		return nil, err
 	}
@@ -992,10 +991,11 @@ func (s *executor) verifyAllowedImage(image, optionName string, allowedImages []
 	return errors.New("invalid image")
 }
 
-func (s *executor) expandImageName(imageName string) (string, error) {
+func (s *executor) expandImageName(imageName string, allowedInternalImages []string) (string, error) {
 	if imageName != "" {
 		image := s.Build.GetAllVariables().ExpandValue(imageName)
-		err := s.verifyAllowedImage(imageName, "images", s.Config.Docker.AllowedImages, []string{s.Config.Docker.Image})
+		allowedInternalImages = append(allowedInternalImages, s.Config.Docker.Image)
+		err := s.verifyAllowedImage(image, "images", s.Config.Docker.AllowedImages, allowedInternalImages)
 		if err != nil {
 			return "", err
 		}
@@ -1074,7 +1074,7 @@ func (s *executor) Prepare(options common.ExecutorPrepareOptions) error {
 	}
 
 	s.SetCurrentStage(DockerExecutorStagePrepare)
-	imageName, err := s.expandImageName(s.Build.Image.Name)
+	imageName, err := s.expandImageName(s.Build.Image.Name, []string{})
 	if err != nil {
 		return err
 	}
diff --git a/executors/docker/executor_docker_command.go b/executors/docker/executor_docker_command.go
index cdeb11dce..7d387b07b 100644
--- a/executors/docker/executor_docker_command.go
+++ b/executors/docker/executor_docker_command.go
@@ -37,13 +37,13 @@ func (s *commandExecutor) Prepare(options common.ExecutorPrepareOptions) error {
 	}
 
 	// Start pre-build container which will git clone changes
-	s.predefinedContainer, err = s.createContainer("predefined", buildImage, []string{"gitlab-runner-build"})
+	s.predefinedContainer, err = s.createContainer("predefined", buildImage, []string{"gitlab-runner-build"}, []string{prebuildImage.ID})
 	if err != nil {
 		return err
 	}
 
 	// Start build container which will run actual build
-	s.buildContainer, err = s.createContainer("build", s.Build.Image, s.BuildShell.DockerCommand)
+	s.buildContainer, err = s.createContainer("build", s.Build.Image, s.BuildShell.DockerCommand, []string{})
 	if err != nil {
 		return err
 	}
diff --git a/executors/docker/executor_docker_command_test.go b/executors/docker/executor_docker_command_test.go
index ffbbd3f73..6071cfe25 100644
--- a/executors/docker/executor_docker_command_test.go
+++ b/executors/docker/executor_docker_command_test.go
@@ -70,6 +70,40 @@ func TestDockerCommandBuildFail(t *testing.T) {
 	assert.Contains(t, err.Error(), "exit code 1")
 }
 
+func TestDockerCommandWithAllowedImagesRun(t *testing.T) {
+	if helpers.SkipIntegrationTests(t, "docker", "info") {
+		return
+	}
+
+	successfulBuild, err := common.GetRemoteSuccessfulBuild()
+	successfulBuild.Image = common.Image{Name: "$IMAGE_NAME"}
+	successfulBuild.Variables = append(successfulBuild.Variables, common.JobVariable{
+		Key:      "IMAGE_NAME",
+		Value:    "alpine",
+		Public:   true,
+		Internal: false,
+		File:     false,
+	})
+	successfulBuild.Services = append(successfulBuild.Services, common.Image{Name: "docker:dind"})
+	assert.NoError(t, err)
+	build := &common.Build{
+		JobResponse: successfulBuild,
+		Runner: &common.RunnerConfig{
+			RunnerSettings: common.RunnerSettings{
+				Executor: "docker",
+				Docker: &common.DockerConfig{
+					AllowedImages:   []string{"alpine"},
+					AllowedServices: []string{"docker:dind"},
+					Privileged:      true,
+				},
+			},
+		},
+	}
+
+	err = build.Run(&common.Config{}, &common.Trace{Writer: os.Stdout})
+	assert.NoError(t, err)
+}
+
 func TestDockerCommandMissingImage(t *testing.T) {
 	if helpers.SkipIntegrationTests(t, "docker", "info") {
 		return
diff --git a/executors/docker/executor_docker_ssh.go b/executors/docker/executor_docker_ssh.go
index 7951aeb1b..6c169df2d 100644
--- a/executors/docker/executor_docker_ssh.go
+++ b/executors/docker/executor_docker_ssh.go
@@ -28,7 +28,7 @@ func (s *sshExecutor) Prepare(options common.ExecutorPrepareOptions) error {
 	s.Debugln("Starting SSH command...")
 
 	// Start build container which will run actual build
-	container, err := s.createContainer("build", s.Build.Image, []string{})
+	container, err := s.createContainer("build", s.Build.Image, []string{}, []string{})
 	if err != nil {
 		return err
 	}
diff --git a/executors/docker/executor_docker_test.go b/executors/docker/executor_docker_test.go
index 0ce6b426c..90d540008 100644
--- a/executors/docker/executor_docker_test.go
+++ b/executors/docker/executor_docker_test.go
@@ -793,7 +793,7 @@ func TestDockerWatchOn_1_12_4(t *testing.T) {
 	err := e.connectDocker()
 	assert.NoError(t, err)
 
-	container, err := e.createContainer("build", common.Image{Name: "alpine"}, []string{"/bin/sh"})
+	container, err := e.createContainer("build", common.Image{Name: "alpine"}, []string{"/bin/sh"}, []string{})
 	assert.NoError(t, err)
 	assert.NotNil(t, container)
 
@@ -871,7 +871,7 @@ func testDockerConfigurationWithJobContainer(t *testing.T, dockerConfig *common.
 	c.On("ContainerInspect", mock.Anything, "abc").
 		Return(types.ContainerJSON{}, nil).Once()
 
-	_, err := e.createContainer("build", common.Image{Name: "alpine"}, []string{"/bin/sh"})
+	_, err := e.createContainer("build", common.Image{Name: "alpine"}, []string{"/bin/sh"}, []string{})
 	assert.NoError(t, err, "Should create container without errors")
 }
 
-- 
GitLab