Merge branch 'fix/regression-in-docker-executor' into 'master'

Fix allowed_images behavior

Closes #2624

See merge request !635

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
 	}

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
 	}

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

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
 	}

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")
 }