Refactor builder to support file requirements and run pre-install

The builder configuration has proven useful for supporting generic
pre-entrypoint commands such as dependency managers not otherwise
supported by specific Blubber configuration. Adding additional
`builder.requirements` config expands support for such commands by
allowing the user to specify files that should be copied into the image
before the builder command runs.

To support this extra configuration, `builder` had to be changed from a
simple string to a mapping. The builder command must now by given as
`builder.command`.

The pattern of creating parent directories, copying files, and executing
one or more commands prior to the entrypoint has become a common
one. Some of the implementation of this pattern was moved from
`PythonConfig` into shared build macros `build.SortFilesByDir` and
`build.SyncFiles`. All config types that must have requirements files
copied over independently of the entire source tree (`PythonConfig`,
`BuilderConfig`, `NodeConfig`) now delegate to these functions.

Change-Id: I67f33034f22cee2851ec866cfb07ab20c23eba8c

7 files changed, 95 insertions, 136 deletions

diff --git a/config/builder.go b/config/builder.go
index 98c560c..c2e95b2 100644
--- a/config/builder.go
+++ b/config/builder.go
@@ -5,42 +5,49 @@ import (
 )
 
 // BuilderConfig contains configuration for the definition of an arbitrary
-// build command.
+// build command and the files required to successfully execute the command.
 //
 type BuilderConfig struct {
-	Builder []string `yaml:"builder"`
+	Command      []string `yaml:"command"`
+	Requirements []string `yaml:"requirements"`
 }
 
 // Merge takes another BuilderConfig and merges its fields into this one's,
 // overwriting the builder command.
 //
 func (bc *BuilderConfig) Merge(bc2 BuilderConfig) {
-	if len(bc2.Builder) > 0 {
-		bc.Builder = bc2.Builder
+	if bc2.Command != nil {
+		bc.Command = bc2.Command
+	}
+
+	if bc2.Requirements != nil {
+		bc.Requirements = bc2.Requirements
 	}
 }
 
 // InstructionsForPhase injects instructions into the build related to
-// builder commands.
+// builder commands and required files.
 //
-// PhasePostInstall
+// PhasePreInstall
 //
-// Runs build command provided for the builder
+// Creates directories for requirements files, copies in requirements files,
+// and runs the builder command.
 //
 func (bc BuilderConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
-	switch phase {
-	case build.PhasePostInstall:
-		if len(bc.Builder) == 0 {
-			return []build.Instruction{}
-		}
+	if len(bc.Command) == 0 {
+		return []build.Instruction{}
+	}
 
-		run := build.Run{Command: bc.Builder[0]}
+	switch phase {
+	case build.PhasePreInstall:
+		syncs := build.SyncFiles(bc.Requirements, ".")
+		run := build.Run{Command: bc.Command[0]}
 
-		if len(bc.Builder) > 1 {
-			run.Arguments = bc.Builder[1:]
+		if len(bc.Command) > 1 {
+			run.Arguments = bc.Command[1:]
 		}
 
-		return []build.Instruction{run}
+		return append(syncs, run)
 	}
 
 	return []build.Instruction{}
diff --git a/config/builder_test.go b/config/builder_test.go
index a35221e..773ddd1 100644
--- a/config/builder_test.go
+++ b/config/builder_test.go
@@ -13,23 +13,37 @@ func TestBuilderConfigYAML(t *testing.T) {
 	cfg, err := config.ReadConfig([]byte(`---
     version: v2
     base: foo
+    builder:
+      command: [make, -f, Makefile, test]
+      requirements: [Makefile]
     variants:
+      test: {}
       build:
-        builder: [make, -f, Makefile]`))
+        builder:
+          command: [make]
+          requirements: []`))
 
 	if assert.NoError(t, err) {
-		variant, err := config.ExpandVariant(cfg, "build")
+		variant, err := config.ExpandVariant(cfg, "test")
 
-		assert.Equal(t, []string{"make", "-f", "Makefile"}, variant.Builder)
+		if assert.NoError(t, err) {
+			assert.Equal(t, []string{"make", "-f", "Makefile", "test"}, variant.Builder.Command)
+			assert.Equal(t, []string{"Makefile"}, variant.Builder.Requirements)
+		}
 
-		assert.Nil(t, err)
+		variant, err = config.ExpandVariant(cfg, "build")
+
+		if assert.NoError(t, err) {
+			assert.Equal(t, []string{"make"}, variant.Builder.Command)
+			assert.Equal(t, []string{}, variant.Builder.Requirements)
+		}
 	}
 }
 
 func TestBuilderConfigInstructions(t *testing.T) {
-	cfg := config.BuilderConfig{Builder: []string{"make", "-f", "Makefile"}}
+	cfg := config.BuilderConfig{Command: []string{"make", "-f", "Makefile"}}
 
-	t.Run("PhasePostInstall", func(t *testing.T) {
+	t.Run("PhasePreInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
 				build.Run{
@@ -37,7 +51,29 @@ func TestBuilderConfigInstructions(t *testing.T) {
 					[]string{"-f", "Makefile"},
 				},
 			},
-			cfg.InstructionsForPhase(build.PhasePostInstall),
+			cfg.InstructionsForPhase(build.PhasePreInstall),
+		)
+	})
+}
+
+func TestBuilderConfigInstructionsWithRequirements(t *testing.T) {
+	cfg := config.BuilderConfig{
+		Command:      []string{"make", "-f", "Makefile", "foo"},
+		Requirements: []string{"Makefile", "foo", "bar/baz"},
+	}
+
+	t.Run("PhasePreInstall", func(t *testing.T) {
+		assert.Equal(t,
+			[]build.Instruction{
+				build.Run{"mkdir -p", []string{"bar/"}},
+				build.Copy{[]string{"Makefile", "foo"}, "./"},
+				build.Copy{[]string{"bar/baz"}, "bar/"},
+				build.Run{
+					"make",
+					[]string{"-f", "Makefile", "foo"},
+				},
+			},
+			cfg.InstructionsForPhase(build.PhasePreInstall),
 		)
 	})
 }
diff --git a/config/common.go b/config/common.go
index b3a9401..0bdd4e4 100644
--- a/config/common.go
+++ b/config/common.go
@@ -8,15 +8,15 @@ import (
 // and each configured variant.
 //
 type CommonConfig struct {
-	Base          string           `yaml:"base" validate:"omitempty,baseimage"` // name/path to base image
-	Apt           AptConfig        `yaml:"apt"`                                 // APT related
-	Node          NodeConfig       `yaml:"node"`                                // Node related
-	Python        PythonConfig     `yaml:"python"`                              // Python related
-	BuilderConfig `yaml:",inline"` // Builder related
-	Lives         LivesConfig      `yaml:"lives"`        // application owner/dir
-	Runs          RunsConfig       `yaml:"runs"`         // runtime environment
-	SharedVolume  Flag             `yaml:"sharedvolume"` // use volume for app
-	EntryPoint    []string         `yaml:"entrypoint"`   // entry-point executable
+	Base         string        `yaml:"base" validate:"omitempty,baseimage"` // name/path to base image
+	Apt          AptConfig     `yaml:"apt"`                                 // APT related
+	Node         NodeConfig    `yaml:"node"`                                // Node related
+	Python       PythonConfig  `yaml:"python"`                              // Python related
+	Builder      BuilderConfig `yaml:"builder"`                             // Builder related
+	Lives        LivesConfig   `yaml:"lives"`                               // application owner/dir
+	Runs         RunsConfig    `yaml:"runs"`                                // runtime environment
+	SharedVolume Flag          `yaml:"sharedvolume"`                        // use volume for app
+	EntryPoint   []string      `yaml:"entrypoint"`                          // entry-point executable
 }
 
 // Merge takes another CommonConfig and merges its fields this one's.
@@ -29,7 +29,7 @@ func (cc *CommonConfig) Merge(cc2 CommonConfig) {
 	cc.Apt.Merge(cc2.Apt)
 	cc.Node.Merge(cc2.Node)
 	cc.Python.Merge(cc2.Python)
-	cc.BuilderConfig.Merge(cc2.BuilderConfig)
+	cc.Builder.Merge(cc2.Builder)
 	cc.Lives.Merge(cc2.Lives)
 	cc.Runs.Merge(cc2.Runs)
 	cc.SharedVolume.Merge(cc2.SharedVolume)
@@ -44,7 +44,7 @@ func (cc *CommonConfig) Merge(cc2 CommonConfig) {
 // injected.
 //
 func (cc *CommonConfig) PhaseCompileableConfig() []build.PhaseCompileable {
-	return []build.PhaseCompileable{cc.Apt, cc.Node, cc.Python, cc.BuilderConfig, cc.Lives, cc.Runs}
+	return []build.PhaseCompileable{cc.Apt, cc.Node, cc.Python, cc.Builder, cc.Lives, cc.Runs}
 }
 
 // InstructionsForPhase injects instructions into the given build phase for
diff --git a/config/node.go b/config/node.go
index 2800e61..438e715 100644
--- a/config/node.go
+++ b/config/node.go
@@ -63,10 +63,10 @@ func (nc NodeConfig) InstructionsForPhase(phase build.Phase) []build.Instruction
 				)
 			}
 
-			return []build.Instruction{
-				build.Copy{nc.Requirements, LocalLibPrefix},
+			return append(
+				build.SyncFiles(nc.Requirements, LocalLibPrefix),
 				npmInstall,
-			}
+			)
 		}
 	case build.PhasePostInstall:
 		if nc.Env != "" || len(nc.Requirements) > 0 {
diff --git a/config/node_test.go b/config/node_test.go
index 6f9e3f4..9771251 100644
--- a/config/node_test.go
+++ b/config/node_test.go
@@ -69,7 +69,7 @@ func TestNodeConfigInstructionsNonProduction(t *testing.T) {
 	t.Run("PhasePreInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Copy{[]string{"package.json"}, "/opt/lib"},
+				build.Copy{[]string{"package.json"}, "/opt/lib/"},
 				build.RunAll{[]build.Run{
 					{"cd", []string{"/opt/lib"}},
 					{"npm install", []string{}},
@@ -107,7 +107,7 @@ func TestNodeConfigInstructionsProduction(t *testing.T) {
 	t.Run("PhasePreInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Copy{[]string{"package.json", "package-lock.json"}, "/opt/lib"},
+				build.Copy{[]string{"package.json", "package-lock.json"}, "/opt/lib/"},
 				build.RunAll{[]build.Run{
 					{"cd", []string{"/opt/lib"}},
 					{"npm install", []string{"--production"}},
diff --git a/config/python.go b/config/python.go
index 50bfa99..c2927e3 100644
--- a/config/python.go
+++ b/config/python.go
@@ -1,9 +1,6 @@
 package config
 
 import (
-	"path"
-	"sort"
-
 	"gerrit.wikimedia.org/r/blubber/build"
 )
 
@@ -73,42 +70,23 @@ func (pc PythonConfig) InstructionsForPhase(phase build.Phase) []build.Instructi
 		switch phase {
 		case build.PhasePrivileged:
 			if pc.Requirements != nil {
-				return []build.Instruction{
-					build.RunAll{[]build.Run{
-						{pc.version(), []string{"-m", "easy_install", "pip"}},
-						{pc.version(), []string{"-m", "pip", "install", "-U", "setuptools", "wheel", "tox"}},
-					}},
-				}
+				return []build.Instruction{build.RunAll{[]build.Run{
+					{pc.version(), []string{"-m", "easy_install", "pip"}},
+					{pc.version(), []string{"-m", "pip", "install", "-U", "setuptools", "wheel", "tox"}},
+				}}}
 			}
 
 		case build.PhasePreInstall:
 			if pc.Requirements != nil {
-				envs := build.Env{map[string]string{
-					"PIP_WHEEL_DIR":  PythonLibPrefix,
-					"PIP_FIND_LINKS": "file://" + PythonLibPrefix,
-				}}
-
-				mkdirs := build.RunAll{
-					Runs: []build.Run{
-						build.CreateDirectory(PythonLibPrefix),
-					},
-				}
-
-				dirs, bydir := pc.RequirementsByDir()
-				copies := make([]build.Instruction, len(dirs))
-
-				// make project subdirectories for requirements files if necessary, and
-				// copy in requirements files
-				for i, dir := range dirs {
-					if dir != "./" {
-						mkdirs.Runs = append(mkdirs.Runs, build.CreateDirectory(dir))
-					}
-
-					copies[i] = build.Copy{bydir[dir], dir}
+				ins := []build.Instruction{
+					build.Env{map[string]string{
+						"PIP_WHEEL_DIR":  PythonLibPrefix,
+						"PIP_FIND_LINKS": "file://" + PythonLibPrefix,
+					}},
+					build.CreateDirectory(PythonLibPrefix),
 				}
 
-				ins := []build.Instruction{envs, mkdirs}
-				ins = append(ins, copies...)
+				ins = append(ins, build.SyncFiles(pc.Requirements, ".")...)
 
 				if args := pc.RequirementsArgs(); len(args) > 0 {
 					ins = append(ins, build.RunAll{[]build.Run{
@@ -150,38 +128,6 @@ func (pc PythonConfig) RequirementsArgs() []string {
 	return args
 }
 
-// RequirementsByDir returns both the configured requirements files indexed by
-// parent directory and a sorted slice of those parent directories. The latter
-// is useful in ensuring deterministic iteration since the ordering of map
-// keys is not guaranteed.
-//
-func (pc PythonConfig) RequirementsByDir() ([]string, map[string][]string) {
-	bydir := make(map[string][]string)
-
-	for _, reqpath := range pc.Requirements {
-		dir := path.Dir(reqpath) + "/"
-		reqpath = path.Clean(reqpath)
-
-		if reqs, found := bydir[dir]; found {
-			bydir[dir] = append(reqs, reqpath)
-		} else {
-			bydir[dir] = []string{reqpath}
-		}
-	}
-
-	dirs := make([]string, len(bydir))
-	i := 0
-
-	for dir := range bydir {
-		dirs[i] = dir
-		i++
-	}
-
-	sort.Strings(dirs)
-
-	return dirs, bydir
-}
-
 func (pc PythonConfig) version() string {
 	if pc.Version == "" {
 		return "python"
diff --git a/config/python_test.go b/config/python_test.go
index 3eb6e72..03abad0 100644
--- a/config/python_test.go
+++ b/config/python_test.go
@@ -158,10 +158,8 @@ func TestPythonConfigInstructionsWithRequirements(t *testing.T) {
 					"PIP_WHEEL_DIR":  "/opt/lib/python",
 					"PIP_FIND_LINKS": "file:///opt/lib/python",
 				}},
-				build.RunAll{[]build.Run{
-					{"mkdir -p", []string{"/opt/lib/python"}},
-					{"mkdir -p", []string{"docs/"}},
-				}},
+				build.Run{"mkdir -p", []string{"/opt/lib/python"}},
+				build.Run{"mkdir -p", []string{"docs/"}},
 				build.Copy{[]string{"requirements.txt", "requirements-test.txt"}, "./"},
 				build.Copy{[]string{"docs/requirements.txt"}, "docs/"},
 				build.RunAll{[]build.Run{
@@ -196,34 +194,6 @@ func TestPythonConfigInstructionsWithRequirements(t *testing.T) {
 	})
 }
 
-func TestPythonConfigRequirementsByDir(t *testing.T) {
-	cfg := config.PythonConfig{
-		Requirements: []string{"foo", "./bar", "./c/c-foo", "b/b-foo", "b/b-bar", "a/a-foo"},
-	}
-
-	sortedDirs, reqsByDir := cfg.RequirementsByDir()
-
-	assert.Equal(t,
-		[]string{
-			"./",
-			"a/",
-			"b/",
-			"c/",
-		},
-		sortedDirs,
-	)
-
-	assert.Equal(t,
-		map[string][]string{
-			"./": []string{"foo", "bar"},
-			"c/": []string{"c/c-foo"},
-			"b/": []string{"b/b-foo", "b/b-bar"},
-			"a/": []string{"a/a-foo"},
-		},
-		reqsByDir,
-	)
-}
-
 func TestPythonConfigRequirementsArgs(t *testing.T) {
 	cfg := config.PythonConfig{
 		Requirements: []string{"foo", "bar", "baz/qux"},