Fix ownership on artifact copies

Summary:
The implementation of D984 did not include enforcing ownership for
`build.CopyFrom` instruction and so artifacts copied from one image to
another via `copies:` were problematically owned as root.

In order to fix this behavior:
 1. `config.ArtifactConfig` `build.CopyFrom` instructions are now
    injected duration `build.PhaseInstall`
 2. `config.VariantConfig` calls `build.ApplyUser` for these artifact
    instructions as well using the `runs.as` user
 3. `build.CopyAs` was refactored to wrap any `build.Instruction` which
    should only really be used with `build.Copy` or `build.CopyFrom`.

Test Plan:
Run `go test ./...`. Run `blubber` against configuration with a variant that
uses `copies` and verify that the `COPY --from` instructions also include a
`--chown` flag.

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

Reviewed By: thcipriani, #release-engineering-team

Tags: #release-engineering-team

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

4 files changed, 30 insertions, 8 deletions

diff --git a/build/instructions.go b/build/instructions.go
index 295221e..0167a8a 100644
--- a/build/instructions.go
+++ b/build/instructions.go
@@ -81,17 +81,20 @@ func (copy Copy) Compile() []string {
 // CopyAs is a concrete build instruction for copying source
 // files/directories and setting their ownership to the given UID/GID.
 //
+// While it can technically wrap any build.Instruction, it is meant to be used
+// with build.Copy and build.CopyFrom to enforce file/directory ownership.
+//
 type CopyAs struct {
 	UID uint // owner UID
 	GID uint // owner GID
-	Copy
+	Instruction
 }
 
 // Compile returns the variant name unquoted and all quoted CopyAs instruction
 // fields.
 //
 func (ca CopyAs) Compile() []string {
-	return append([]string{fmt.Sprintf("%d:%d", ca.UID, ca.GID)}, ca.Copy.Compile()...)
+	return append([]string{fmt.Sprintf("%d:%d", ca.UID, ca.GID)}, ca.Instruction.Compile()...)
 }
 
 // CopyFrom is a concrete build instruction for copying source
diff --git a/build/instructions_test.go b/build/instructions_test.go
index 29508e2..77938dd 100644
--- a/build/instructions_test.go
+++ b/build/instructions_test.go
@@ -37,9 +37,25 @@ func TestCopy(t *testing.T) {
 }
 
 func TestCopyAs(t *testing.T) {
-	i := build.CopyAs{123, 124, build.Copy{[]string{"source1", "source2"}, "dest"}}
-
-	assert.Equal(t, []string{"123:124", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+	t.Run("wrapping Copy", func(t *testing.T) {
+		i := build.CopyAs{
+			123,
+			124,
+			build.Copy{[]string{"source1", "source2"}, "dest"},
+		}
+
+		assert.Equal(t, []string{"123:124", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+	})
+
+	t.Run("wrapping CopyFrom", func(t *testing.T) {
+		i := build.CopyAs{
+			123,
+			124,
+			build.CopyFrom{"foo", build.Copy{[]string{"source1", "source2"}, "dest"}},
+		}
+
+		assert.Equal(t, []string{"123:124", "foo", `"source1"`, `"source2"`, `"dest"`}, i.Compile())
+	})
 }
 
 func TestCopyFrom(t *testing.T) {
diff --git a/build/macros.go b/build/macros.go
index 5d3422e..08556d1 100644
--- a/build/macros.go
+++ b/build/macros.go
@@ -11,9 +11,10 @@ func ApplyUser(uid uint, gid uint, instructions []Instruction) []Instruction {
 	applied := make([]Instruction, len(instructions))
 
 	for i, instruction := range instructions {
-		if copy, iscopy := instruction.(Copy); iscopy {
-			applied[i] = CopyAs{uid, gid, copy}
-		} else {
+		switch instruction.(type) {
+		case Copy, CopyFrom:
+			applied[i] = CopyAs{uid, gid, instruction}
+		default:
 			applied[i] = instruction
 		}
 	}
diff --git a/build/macros_test.go b/build/macros_test.go
index e47cf8d..c5066a6 100644
--- a/build/macros_test.go
+++ b/build/macros_test.go
@@ -12,12 +12,14 @@ func TestApplyUser(t *testing.T) {
 	instructions := []build.Instruction{
 		build.Copy{[]string{"foo"}, "bar"},
 		build.Copy{[]string{"baz"}, "qux"},
+		build.CopyFrom{"foo", build.Copy{[]string{"a"}, "b"}},
 	}
 
 	assert.Equal(t,
 		[]build.Instruction{
 			build.CopyAs{123, 223, build.Copy{[]string{"foo"}, "bar"}},
 			build.CopyAs{123, 223, build.Copy{[]string{"baz"}, "qux"}},
+			build.CopyAs{123, 223, build.CopyFrom{"foo", build.Copy{[]string{"a"}, "b"}}},
 		},
 		build.ApplyUser(123, 223, instructions),
 	)