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

2 files changed, 23 insertions, 6 deletions

diff --git a/docker/instructions.go b/docker/instructions.go
index 8463b0e..56c04b9 100644
--- a/docker/instructions.go
+++ b/docker/instructions.go
@@ -26,7 +26,12 @@ func NewInstruction(bi build.Instruction) (Instruction, error) {
 
 		switch bi.(type) {
 		case build.CopyAs:
-			i.flags = []string{"chown"}
+			switch bi.(build.CopyAs).Instruction.(type) {
+			case build.Copy:
+				i.flags = []string{"chown"}
+			case build.CopyFrom:
+				i.flags = []string{"chown", "from"}
+			}
 		case build.CopyFrom:
 			i.flags = []string{"from"}
 		}
diff --git a/docker/instructions_test.go b/docker/instructions_test.go
index 6215841..07b71c7 100644
--- a/docker/instructions_test.go
+++ b/docker/instructions_test.go
@@ -42,13 +42,25 @@ func TestCopy(t *testing.T) {
 }
 
 func TestCopyAs(t *testing.T) {
-	i := build.CopyAs{123, 124, build.Copy{[]string{"foo1", "foo2"}, "bar"}}
+	t.Run("with Copy", func(t *testing.T) {
+		i := build.CopyAs{123, 124, build.Copy{[]string{"foo1", "foo2"}, "bar"}}
 
-	di, err := docker.NewInstruction(i)
+		di, err := docker.NewInstruction(i)
 
-	if assert.NoError(t, err) {
-		assert.Equal(t, "COPY --chown=123:124 [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
-	}
+		if assert.NoError(t, err) {
+			assert.Equal(t, "COPY --chown=123:124 [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+		}
+	})
+
+	t.Run("with CopyFrom", func(t *testing.T) {
+		i := build.CopyAs{123, 124, build.CopyFrom{"foo", build.Copy{[]string{"foo1", "foo2"}, "bar"}}}
+
+		di, err := docker.NewInstruction(i)
+
+		if assert.NoError(t, err) {
+			assert.Equal(t, "COPY --chown=123:124 --from=foo [\"foo1\", \"foo2\", \"bar\"]\n", di.Compile())
+		}
+	})
 }
 
 func TestCopyFrom(t *testing.T) {