Support `copies` config entry for multi-stage builds

Summary:
Support a `copies` variant config entry that will result in a
multi-stage build, copying both shared library files and application
directory from a previously defined variant. This is essentially a
shorthand for two `artifacts` entries that are likely to be idiomatic to
multi-stage build/prod configurations.

Defined a new abstract `build.CopyFrom` instruction and corresponding
`docker.DockerCopyFrom` instruction and refactored the writing of these
Dockerfile lines to be accomplished using an `InstructionsForPhase`
method on `config.ArtifactsConfig`.

Implemented new support for `copies` configuration in
`config.VariantConfig` and an `InstructionsForPhase` method that returns
`build.CopyFrom` instructions for both the shared library and
application directories.

Fixes T174622
Depends on D759

Test Plan:
Run `go test ./...`. Run `blubber blubber.example.yaml production` and ensure
the right `COPY --from` lines are included for the final stage.

Reviewers: thcipriani, mobrovac, hashar, mmodell, #release-engineering-team

Reviewed By: thcipriani, #release-engineering-team

Tags: #release-engineering-team

Maniphest Tasks: T174622

Differential Revision: https://phabricator.wikimedia.org/D768

11 files changed, 234 insertions, 45 deletions

diff --git a/README.md b/README.md
index 38cdcae..b8018a7 100644
--- a/README.md
+++ b/README.md
@@ -14,8 +14,6 @@ running ad-hoc commands.
 base: debian:jessie
 apt:
   packages: [libjpeg, libyaml]
-node:
-  dependencies: true
 runs:
   in: /srv/service
   as: runuser
@@ -26,13 +24,18 @@ runs:
     BAR: baz
 
 variants:
-  development:
+  build:
     apt:
       packages: [libjpeg-dev, libyaml-dev]
+    node:
+      dependencies: true
+
+  development:
+    includes: [build]
     sharedvolume: true
 
   test:
-    includes: [development]
+    includes: [build]
     apt:
       packages: [chromium]
     entrypoint: [npm, test]
@@ -41,10 +44,7 @@ variants:
     base: debian:jessie-slim
     node:
       env: production
-    artifacts:
-      - from: test
-        source: /srv/service
-        destination: .
+    copies: test
     entrypoint: [node, server.js]
 ```
 
diff --git a/blubber.example.yaml b/blubber.example.yaml
index 4cea7c3..87a4cf1 100644
--- a/blubber.example.yaml
+++ b/blubber.example.yaml
@@ -2,8 +2,6 @@
 base: debian:jessie
 apt:
   packages: [libjpeg, libyaml]
-node:
-  dependencies: true
 runs:
   in: /srv/service
   as: runuser
@@ -14,13 +12,18 @@ runs:
     BAR: baz
 
 variants:
-  development:
+  build:
     apt:
       packages: [libjpeg-dev, libyaml-dev]
+    node:
+      dependencies: true
+
+  development:
+    includes: [build]
     sharedvolume: true
 
   test:
-    includes: [development]
+    includes: [build]
     apt:
       packages: [chromium]
     entrypoint: [npm, test]
@@ -29,8 +32,5 @@ variants:
     base: debian:jessie-slim
     node:
       env: production
-    artifacts:
-      - from: test
-        source: /srv/service
-        destination: .
+    copies: test
     entrypoint: [node, server.js]
diff --git a/build/instructions.go b/build/instructions.go
index 5c12796..1b954a2 100644
--- a/build/instructions.go
+++ b/build/instructions.go
@@ -50,6 +50,15 @@ func (copy Copy) Compile() []string {
 	return append(quoteAll(copy.Sources), quote(copy.Destination))
 }
 
+type CopyFrom struct {
+	From string
+	Copy
+}
+
+func (cf CopyFrom) Compile() []string {
+	return append([]string{cf.From}, cf.Copy.Compile()...)
+}
+
 type Env struct {
 	Definitions map[string]string
 }
diff --git a/build/instructions_test.go b/build/instructions_test.go
index dbab4b5..8f9470c 100644
--- a/build/instructions_test.go
+++ b/build/instructions_test.go
@@ -36,6 +36,12 @@ func TestCopy(t *testing.T) {
 	assert.Equal(t, []string{`"source1"`, `"source2"`, `"dest"`}, i.Compile())
 }
 
+func TestCopyFrom(t *testing.T) {
+	i := build.CopyFrom{"foo", build.Copy{[]string{"source1", "source2"}, "dest"}}
+
+	assert.Equal(t, []string{"foo", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+}
+
 func TestEnv(t *testing.T) {
 	i := build.Env{map[string]string{
 		"fooname": "foovalue",
diff --git a/config/artifacts.go b/config/artifacts.go
index a8108d0..c3fad69 100644
--- a/config/artifacts.go
+++ b/config/artifacts.go
@@ -1,7 +1,22 @@
 package config
 
+import (
+	"phabricator.wikimedia.org/source/blubber.git/build"
+)
+
 type ArtifactsConfig struct {
 	From        string `yaml:"from"`
 	Source      string `yaml:"source"`
 	Destination string `yaml:"destination"`
 }
+
+func (ac ArtifactsConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
+	switch phase {
+	case build.PhasePostInstall:
+		return []build.Instruction{
+			build.CopyFrom{ac.From, build.Copy{[]string{ac.Source}, ac.Destination}},
+		}
+	}
+
+	return []build.Instruction{}
+}
diff --git a/config/artifacts_test.go b/config/artifacts_test.go
index 09f2ae4..9198d96 100644
--- a/config/artifacts_test.go
+++ b/config/artifacts_test.go
@@ -5,6 +5,7 @@ import (
 
 	"gopkg.in/stretchr/testify.v1/assert"
 
+	"phabricator.wikimedia.org/source/blubber.git/build"
 	"phabricator.wikimedia.org/source/blubber.git/config"
 )
 
@@ -38,3 +39,33 @@ func TestArtifactsConfig(t *testing.T) {
 		config.ArtifactsConfig{From: "build", Source: "/bar/src", Destination: "/bar/dst"},
 	)
 }
+
+func TestArtifactsConfigInstructions(t *testing.T) {
+	cfg := config.ArtifactsConfig{
+		From:        "foo",
+		Source:      "/source/path",
+		Destination: "/destination/path",
+	}
+
+	t.Run("PhasePrivileged", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
+	})
+
+	t.Run("PhasePrivilegeDropped", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivilegeDropped))
+	})
+
+	t.Run("PhasePreInstall", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePreInstall))
+	})
+
+	t.Run("PhasePostInstall", func(t *testing.T) {
+		assert.Equal(t,
+			[]build.Instruction{build.CopyFrom{
+				"foo",
+				build.Copy{[]string{"/source/path"}, "/destination/path"},
+			}},
+			cfg.InstructionsForPhase(build.PhasePostInstall),
+		)
+	})
+}
diff --git a/config/variant.go b/config/variant.go
index 9309b9d..cc2cbdb 100644
--- a/config/variant.go
+++ b/config/variant.go
@@ -1,12 +1,67 @@
 package config
 
+import (
+	"phabricator.wikimedia.org/source/blubber.git/build"
+)
+
 type VariantConfig struct {
 	Includes     []string          `yaml:"includes"`
+	Copies       string            `yaml:"copies"`
 	Artifacts    []ArtifactsConfig `yaml:"artifacts"`
 	CommonConfig `yaml:",inline"`
 }
 
 func (vc *VariantConfig) Merge(vc2 VariantConfig) {
+	vc.Copies = vc2.Copies
 	vc.Artifacts = append(vc.Artifacts, vc2.Artifacts...)
 	vc.CommonConfig.Merge(vc2.CommonConfig)
 }
+
+func (vc *VariantConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
+	instructions := vc.CommonConfig.InstructionsForPhase(phase)
+	ainstructions := []build.Instruction{}
+
+	for _, artifact := range vc.allArtifacts() {
+		ainstructions = append(ainstructions, artifact.InstructionsForPhase(phase)...)
+	}
+
+	return append(ainstructions, instructions...)
+}
+
+func (vc *VariantConfig) VariantDependencies() []string {
+	// get unique set of variant dependencies based on artifacts
+	existing := map[string]bool{}
+	dependencies := []string{}
+
+	for _, artifact := range vc.allArtifacts() {
+		if dependency := artifact.From; dependency != "" && !existing[dependency] {
+			existing[dependency] = true
+			dependencies = append(dependencies, dependency)
+		}
+	}
+
+	return dependencies
+}
+
+func (vc *VariantConfig) allArtifacts() []ArtifactsConfig {
+	return append(vc.defaultArtifacts(), vc.Artifacts...)
+}
+
+func (vc *VariantConfig) defaultArtifacts() []ArtifactsConfig {
+	if vc.Copies != "" {
+		return []ArtifactsConfig{
+			{
+				From:        vc.Copies,
+				Source:      vc.CommonConfig.Runs.In,
+				Destination: vc.CommonConfig.Runs.In,
+			},
+			{
+				From:        vc.Copies,
+				Source:      LocalLibPrefix,
+				Destination: LocalLibPrefix,
+			},
+		}
+	}
+
+	return []ArtifactsConfig{}
+}
diff --git a/config/variant_test.go b/config/variant_test.go
new file mode 100644
index 0000000..0a98828
--- /dev/null
+++ b/config/variant_test.go
@@ -0,0 +1,66 @@
+package config_test
+
+import (
+	"testing"
+
+	"gopkg.in/stretchr/testify.v1/assert"
+
+	"phabricator.wikimedia.org/source/blubber.git/build"
+	"phabricator.wikimedia.org/source/blubber.git/config"
+)
+
+func TestVariantConfig(t *testing.T) {
+	cfg, err := config.ReadConfig([]byte(`---
+    variants:
+      build: {}
+      production:
+        copies: build
+        artifacts:
+          - from: build
+            source: /foo/src
+            destination: /foo/dst
+          - from: build
+            source: /bar/src
+            destination: /bar/dst`))
+
+	assert.Nil(t, err)
+
+	variant, err := config.ExpandVariant(cfg, "production")
+
+	assert.Nil(t, err)
+
+	assert.Equal(t, "build", variant.Copies)
+	assert.Len(t, variant.Artifacts, 2)
+}
+
+func TestVariantDependencies(t *testing.T) {
+	cfg := config.VariantConfig{
+		Copies: "foo",
+		Artifacts: []config.ArtifactsConfig{
+			{From: "build", Source: "/foo/src", Destination: "/foo/dst"},
+		},
+	}
+
+	assert.Equal(t, []string{"foo", "build"}, cfg.VariantDependencies())
+}
+
+func TestVariantConfigInstructions(t *testing.T) {
+	cfg := config.VariantConfig{
+		CommonConfig: config.CommonConfig{Runs: config.RunsConfig{In: "/srv/service"}},
+		Copies:       "foo",
+		Artifacts: []config.ArtifactsConfig{
+			{From: "build", Source: "/foo/src", Destination: "/foo/dst"},
+		},
+	}
+
+	t.Run("PhasePostInstall", func(t *testing.T) {
+		assert.Equal(t,
+			[]build.Instruction{
+				build.CopyFrom{"foo", build.Copy{[]string{"/srv/service"}, "/srv/service"}},
+				build.CopyFrom{"foo", build.Copy{[]string{config.LocalLibPrefix}, config.LocalLibPrefix}},
+				build.CopyFrom{"build", build.Copy{[]string{"/foo/src"}, "/foo/dst"}},
+			},
+			cfg.InstructionsForPhase(build.PhasePostInstall),
+		)
+	})
+}
diff --git a/docker/compiler.go b/docker/compiler.go
index e77691f..2891140 100644
--- a/docker/compiler.go
+++ b/docker/compiler.go
@@ -17,25 +17,13 @@ func Compile(cfg *config.Config, variant string) *bytes.Buffer {
 		// omit the main stage name unless multi-stage is required below
 		mainStage := ""
 
-		// get unique set of artifact/variant dependencies
-		existing := map[string]bool{}
-		stages := []string{}
-
-		for _, artifact := range vcfg.Artifacts {
-			if stage := artifact.From; stage != "" && !existing[stage] {
-				existing[stage] = true
-				stages = append(stages, stage)
-
-				mainStage = variant
-			}
-		}
-
-		// write multi-stage sections for each artifact/variant dependency
-		for _, stage := range stages {
+		// write multi-stage sections for each variant dependency
+		for _, stage := range vcfg.VariantDependencies() {
 			dependency, err := config.ExpandVariant(cfg, stage)
 
 			if err == nil {
 				CompileStage(buffer, stage, dependency)
+				mainStage = variant
 			}
 		}
 
@@ -76,17 +64,6 @@ func CompileStage(buffer *bytes.Buffer, stage string, vcfg *config.VariantConfig
 		Writeln(buffer, "COPY . .")
 	}
 
-	// Artifact copying
-	for _, artifact := range vcfg.Artifacts {
-		Write(buffer, "COPY ")
-
-		if artifact.From != "" {
-			Write(buffer, "--from=", artifact.From, " ")
-		}
-
-		Writeln(buffer, "[\"", artifact.Source, "\", \"", artifact.Destination, "\"]")
-	}
-
 	CompilePhase(buffer, vcfg, build.PhasePostInstall)
 
 	if len(vcfg.EntryPoint) > 0 {
diff --git a/docker/instructions.go b/docker/instructions.go
index a93ad11..2b00c1b 100644
--- a/docker/instructions.go
+++ b/docker/instructions.go
@@ -18,11 +18,16 @@ func NewDockerInstruction(instruction build.Instruction) (DockerInstruction, err
 		var dockerInstruction DockerCopy
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
+	case build.CopyFrom:
+		var dockerInstruction DockerCopyFrom
+		dockerInstruction.arguments = instruction.Compile()
+		return dockerInstruction, nil
 	case build.Env:
 		var dockerInstruction DockerEnv
 		dockerInstruction.arguments = instruction.Compile()
 		return dockerInstruction, nil
 	}
+
 	return nil, errors.New("Unable to create DockerInstruction")
 }
 
@@ -44,7 +49,7 @@ type DockerRun struct{ abstractDockerInstruction }
 func (dr DockerRun) Compile() string {
 	return fmt.Sprintf(
 		"RUN %s\n",
-		removeNewlines(strings.Join(dr.arguments, "")))
+		join(dr.arguments, ""))
 }
 
 type DockerCopy struct{ abstractDockerInstruction }
@@ -52,7 +57,16 @@ type DockerCopy struct{ abstractDockerInstruction }
 func (dc DockerCopy) Compile() string {
 	return fmt.Sprintf(
 		"COPY [%s]\n",
-		removeNewlines(strings.Join(dc.arguments, ", ")))
+		join(dc.arguments, ", "))
+}
+
+type DockerCopyFrom struct{ abstractDockerInstruction }
+
+func (dcf DockerCopyFrom) Compile() string {
+	return fmt.Sprintf(
+		"COPY --from=%s [%s]\n",
+		dcf.arguments[0],
+		join(dcf.arguments[1:], ", "))
 }
 
 type DockerEnv struct{ abstractDockerInstruction }
@@ -60,7 +74,11 @@ type DockerEnv struct{ abstractDockerInstruction }
 func (de DockerEnv) Compile() string {
 	return fmt.Sprintf(
 		"ENV %s\n",
-		removeNewlines(strings.Join(de.arguments, " ")))
+		join(de.arguments, " "))
+}
+
+func join(arguments []string, delimiter string) string {
+	return removeNewlines(strings.Join(arguments, delimiter))
 }
 
 func removeNewlines(instructions string) string {
diff --git a/docker/instructions_test.go b/docker/instructions_test.go
index c03d48b..fb9116b 100644
--- a/docker/instructions_test.go
+++ b/docker/instructions_test.go
@@ -46,6 +46,18 @@ func TestCopy(t *testing.T) {
 	assert.Equal(t, "COPY [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
 }
 
+func TestCopyFrom(t *testing.T) {
+	i := build.CopyFrom{"foo", build.Copy{[]string{"foo1", "foo2"}, "bar"}}
+
+	di, err := docker.NewDockerInstruction(i)
+
+	var dockerCopyFrom docker.DockerCopyFrom
+
+	assert.Nil(t, err)
+	assert.IsType(t, dockerCopyFrom, di)
+	assert.Equal(t, "COPY --from=foo [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+}
+
 func TestEnv(t *testing.T) {
 	i := build.Env{map[string]string{"foo": "bar", "bar": "foo"}}