Generalize instructions for entrypoint and working directory

Summary:
Introduce new `build.EntryPoint` and `build.WorkingDirectory`
instructions to allow configuration to inject them instead of hard
coding their generation in the Docker compiler.

Simplified the Docker compiler to simply iterate over build phases as
returned by a new function `build.Phases()`.

Depends on D990

Test Plan: Run `go test ./...`.

Reviewers: thcipriani, demon, hashar, #release-engineering-team

Reviewed By: thcipriani, #release-engineering-team

Tags: #release-engineering-team

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

10 files changed, 107 insertions, 18 deletions

diff --git a/build/instructions.go b/build/instructions.go
index 91caa6b..295221e 100644
--- a/build/instructions.go
+++ b/build/instructions.go
@@ -109,6 +109,18 @@ func (cf CopyFrom) Compile() []string {
 	return append([]string{cf.From}, cf.Copy.Compile()...)
 }
 
+// EntryPoint is a build instruction for declaring a container's default
+// runtime process.
+type EntryPoint struct {
+	Command []string // command and arguments
+}
+
+// Compile returns the quoted entrypoint command and arguments.
+//
+func (ep EntryPoint) Compile() []string {
+	return quoteAll(ep.Command)
+}
+
 // Env is a concrete build instruction for declaring a container's runtime
 // environment variables.
 //
@@ -163,6 +175,19 @@ func (vol Volume) Compile() []string {
 	return []string{quote(vol.Path)}
 }
 
+// WorkingDirectory is a build instruction for defining the working directory
+// for future command and entrypoint instructions.
+//
+type WorkingDirectory struct {
+	Path string // working directory path
+}
+
+// Compile returns the quoted working directory path.
+//
+func (wd WorkingDirectory) Compile() []string {
+	return []string{quote(wd.Path)}
+}
+
 func compileSortedKeyValues(keyValues map[string]string) []string {
 	defs := make([]string, 0, len(keyValues))
 	names := make([]string, 0, len(keyValues))
diff --git a/build/instructions_test.go b/build/instructions_test.go
index f6e3628..29508e2 100644
--- a/build/instructions_test.go
+++ b/build/instructions_test.go
@@ -48,6 +48,12 @@ func TestCopyFrom(t *testing.T) {
 	assert.Equal(t, []string{"foo", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
 }
 
+func TestEntryPoint(t *testing.T) {
+	i := build.EntryPoint{[]string{"/bin/foo", "bar", "baz"}}
+
+	assert.Equal(t, []string{`"/bin/foo"`, `"bar"`, `"baz"`}, i.Compile())
+}
+
 func TestEnv(t *testing.T) {
 	i := build.Env{map[string]string{
 		"fooname": "foovalue",
@@ -87,3 +93,9 @@ func TestVolume(t *testing.T) {
 
 	assert.Equal(t, []string{`"/foo/dir"`}, i.Compile())
 }
+
+func TestWorkingDirectory(t *testing.T) {
+	i := build.WorkingDirectory{"/foo/path"}
+
+	assert.Equal(t, []string{`"/foo/path"`}, i.Compile())
+}
diff --git a/build/phases.go b/build/phases.go
index 3480927..a656727 100644
--- a/build/phases.go
+++ b/build/phases.go
@@ -22,3 +22,15 @@ const (
 type PhaseCompileable interface {
 	InstructionsForPhase(phase Phase) []Instruction
 }
+
+// Phases returns all build phases in the order to be compiled.
+//
+func Phases() []Phase {
+	return []Phase{
+		PhasePrivileged,
+		PhasePrivilegeDropped,
+		PhasePreInstall,
+		PhaseInstall,
+		PhasePostInstall,
+	}
+}
diff --git a/config/lives.go b/config/lives.go
index 8d705e7..665a734 100644
--- a/config/lives.go
+++ b/config/lives.go
@@ -48,6 +48,10 @@ func (lives LivesConfig) InstructionsForPhase(phase build.Phase) []build.Instruc
 				build.Chown(lives.UID, lives.GID, LocalLibPrefix),
 			),
 		}}
+	case build.PhasePrivilegeDropped:
+		return []build.Instruction{
+			build.WorkingDirectory{lives.In},
+		}
 	}
 
 	return []build.Instruction{}
diff --git a/config/lives_test.go b/config/lives_test.go
index eeb91ef..dec1f99 100644
--- a/config/lives_test.go
+++ b/config/lives_test.go
@@ -73,7 +73,12 @@ func TestLivesConfigInstructions(t *testing.T) {
 	})
 
 	t.Run("PhasePrivilegeDropped", func(t *testing.T) {
-		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePreInstall))
+		assert.Equal(t,
+			[]build.Instruction{
+				build.WorkingDirectory{"/some/directory"},
+			},
+			cfg.InstructionsForPhase(build.PhasePrivilegeDropped),
+		)
 	})
 
 	t.Run("PhasePreInstall", func(t *testing.T) {
diff --git a/config/variant.go b/config/variant.go
index 0cb3f09..e3562bf 100644
--- a/config/variant.go
+++ b/config/variant.go
@@ -68,6 +68,10 @@ func (vc *VariantConfig) InstructionsForPhase(phase build.Phase) []build.Instruc
 	case build.PhasePostInstall:
 		switchUser = vc.Runs.As
 		instructions = build.ApplyUser(vc.Runs.UID, vc.Runs.GID, instructions)
+
+		if len(vc.EntryPoint) > 0 {
+			instructions = append(instructions, build.EntryPoint{vc.EntryPoint})
+		}
 	}
 
 	if switchUser != "" {
diff --git a/config/variant_test.go b/config/variant_test.go
index 6890f6d..f906e67 100644
--- a/config/variant_test.go
+++ b/config/variant_test.go
@@ -135,6 +135,21 @@ func TestVariantConfigInstructions(t *testing.T) {
 				cfg.InstructionsForPhase(build.PhasePostInstall),
 			)
 		})
+
+		t.Run("with entrypoint", func(t *testing.T) {
+			cfg := config.VariantConfig{
+				CommonConfig: config.CommonConfig{
+					EntryPoint: []string{"/foo", "bar"},
+				},
+			}
+
+			assert.Equal(t,
+				[]build.Instruction{
+					build.EntryPoint{[]string{"/foo", "bar"}},
+				},
+				cfg.InstructionsForPhase(build.PhasePostInstall),
+			)
+		})
 	})
 }
 
diff --git a/docker/compiler.go b/docker/compiler.go
index 5929692..b0d5dce 100644
--- a/docker/compiler.go
+++ b/docker/compiler.go
@@ -5,7 +5,6 @@ package docker
 
 import (
 	"bytes"
-	"strings"
 
 	"phabricator.wikimedia.org/source/blubber/build"
 	"phabricator.wikimedia.org/source/blubber/config"
@@ -61,22 +60,8 @@ func compileStage(buffer *bytes.Buffer, stage string, vcfg *config.VariantConfig
 
 	writeln(buffer, "FROM ", baseAndStage)
 
-	compilePhase(buffer, vcfg, build.PhasePrivileged)
-
-	compilePhase(buffer, vcfg, build.PhasePrivilegeDropped)
-
-	if vcfg.Lives.In != "" {
-		writeln(buffer, "WORKDIR ", vcfg.Lives.In)
-	}
-
-	compilePhase(buffer, vcfg, build.PhasePreInstall)
-
-	compilePhase(buffer, vcfg, build.PhaseInstall)
-
-	compilePhase(buffer, vcfg, build.PhasePostInstall)
-
-	if len(vcfg.EntryPoint) > 0 {
-		writeln(buffer, "ENTRYPOINT [\"", strings.Join(vcfg.EntryPoint, "\", \""), "\"]")
+	for _, phase := range build.Phases() {
+		compilePhase(buffer, vcfg, phase)
 	}
 }
 
diff --git a/docker/instructions.go b/docker/instructions.go
index c031d01..8463b0e 100644
--- a/docker/instructions.go
+++ b/docker/instructions.go
@@ -31,6 +31,10 @@ func NewInstruction(bi build.Instruction) (Instruction, error) {
 			i.flags = []string{"from"}
 		}
 
+	case build.EntryPoint:
+		i.name = "ENTRYPOINT"
+		i.array = true
+
 	case build.Env:
 		i.name = "ENV"
 		i.separator = " "
@@ -45,6 +49,9 @@ func NewInstruction(bi build.Instruction) (Instruction, error) {
 	case build.Volume:
 		i.name = "VOLUME"
 		i.array = true
+
+	case build.WorkingDirectory:
+		i.name = "WORKDIR"
 	}
 
 	if i.name == "" {
diff --git a/docker/instructions_test.go b/docker/instructions_test.go
index 37eeaf1..6215841 100644
--- a/docker/instructions_test.go
+++ b/docker/instructions_test.go
@@ -61,6 +61,16 @@ func TestCopyFrom(t *testing.T) {
 	}
 }
 
+func TestEntryPoint(t *testing.T) {
+	i := build.EntryPoint{[]string{"foo", "bar"}}
+
+	di, err := docker.NewInstruction(i)
+
+	if assert.NoError(t, err) {
+		assert.Equal(t, "ENTRYPOINT [\"foo\", \"bar\"]\n", di.Compile())
+	}
+}
+
 func TestEnv(t *testing.T) {
 	i := build.Env{map[string]string{"foo": "bar", "bar": "foo"}}
 
@@ -101,6 +111,16 @@ func TestVolume(t *testing.T) {
 	}
 }
 
+func TestWorkingDirectory(t *testing.T) {
+	i := build.WorkingDirectory{"/foo/dir"}
+
+	di, err := docker.NewInstruction(i)
+
+	if assert.NoError(t, err) {
+		assert.Equal(t, "WORKDIR \"/foo/dir\"\n", di.Compile())
+	}
+}
+
 func TestEscapeRun(t *testing.T) {
 	i := build.Run{"/bin/true\nRUN echo HACKED!", []string{}}