Merge pull request #5422 from localghost/upload_owner_fix

Fix owner of files uploaded to docker container run as non-root.

builder/docker/communicator.go

@@ -18,12 +18,13 @@ import (
 )
 
 type Communicator struct {
-	ContainerID  string
-	HostDir      string
-	ContainerDir string
-	Version      *version.Version
-	Config       *Config
-	lock         sync.Mutex
+	ContainerID   string
+	HostDir       string
+	ContainerDir  string
+	Version       *version.Version
+	Config        *Config
+	ContainerUser string
+	lock          sync.Mutex
 }
 
 func (c *Communicator) Start(remote *packer.RemoteCmd) error {
@@ -154,6 +155,10 @@ func (c *Communicator) uploadFile(dst string, src io.Reader, fi *os.FileInfo) er
 		return fmt.Errorf("Failed to upload to '%s' in container: %s. %s.", dst, stderrOut, err)
 	}
 
+	if err := c.fixDestinationOwner(dst); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -207,6 +212,10 @@ func (c *Communicator) UploadDir(dst string, src string, exclude []string) error
 		return fmt.Errorf("Failed to upload to '%s' in container: %s. %s.", dst, stderrOut, err)
 	}
 
+	if err := c.fixDestinationOwner(dst); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -310,3 +319,25 @@ func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin io.Wri
 	// Set the exit status which triggers waiters
 	remote.SetExited(exitStatus)
 }
+
+// TODO Workaround for #5307. Remove once #5409 is fixed.
+func (c *Communicator) fixDestinationOwner(destination string) error {
+	if !c.Config.FixUploadOwner {
+		return nil
+	}
+
+	owner := c.ContainerUser
+	if owner == "" {
+		owner = "root"
+	}
+
+	chownArgs := []string{
+		"docker", "exec", "--user", "root", c.ContainerID, "/bin/sh", "-c",
+		fmt.Sprintf("chown -R %s %s", owner, destination),
+	}
+	if output, err := exec.Command(chownArgs[0], chownArgs[1:]...).CombinedOutput(); err != nil {
+		return fmt.Errorf("Failed to set owner of the uploaded file: %s, %s", err, output)
+	}
+
+	return nil
+}

builder/docker/communicator_test.go

@@ -209,6 +209,82 @@ func TestLargeDownload(t *testing.T) {
 
 }
 
+// TestFixUploadOwner verifies that owner of uploaded files is the user the container is running as.
+func TestFixUploadOwner(t *testing.T) {
+	ui := packer.TestUi(t)
+	cache := &packer.FileCache{CacheDir: os.TempDir()}
+
+	tpl, err := template.Parse(strings.NewReader(testFixUploadOwnerTemplate))
+	if err != nil {
+		t.Fatalf("Unable to parse config: %s", err)
+	}
+
+	if os.Getenv("PACKER_ACC") == "" {
+		t.Skip("This test is only run with PACKER_ACC=1")
+	}
+	cmd := exec.Command("docker", "-v")
+	cmd.Run()
+	if !cmd.ProcessState.Success() {
+		t.Error("docker command not found; please make sure docker is installed")
+	}
+
+	// Setup the builder
+	builder := &Builder{}
+	warnings, err := builder.Prepare(tpl.Builders["docker"].Config)
+	if err != nil {
+		t.Fatalf("Error preparing configuration %s", err)
+	}
+	if len(warnings) > 0 {
+		t.Fatal("Encountered configuration warnings; aborting")
+	}
+
+	// Setup the provisioners
+	fileProvisioner := &file.Provisioner{}
+	err = fileProvisioner.Prepare(tpl.Provisioners[0].Config)
+	if err != nil {
+		t.Fatalf("Error preparing single file upload provisioner: %s", err)
+	}
+
+	dirProvisioner := &file.Provisioner{}
+	err = dirProvisioner.Prepare(tpl.Provisioners[1].Config)
+	if err != nil {
+		t.Fatalf("Error preparing directory upload provisioner: %s", err)
+	}
+
+	shellProvisioner := &shell.Provisioner{}
+	err = shellProvisioner.Prepare(tpl.Provisioners[2].Config)
+	if err != nil {
+		t.Fatalf("Error preparing shell provisioner: %s", err)
+	}
+
+	verifyProvisioner := &shell.Provisioner{}
+	err = verifyProvisioner.Prepare(tpl.Provisioners[3].Config)
+	if err != nil {
+		t.Fatalf("Error preparing verification provisioner: %s", err)
+	}
+
+	// Add hooks so the provisioners run during the build
+	hooks := map[string][]packer.Hook{}
+	hooks[packer.HookProvision] = []packer.Hook{
+		&packer.ProvisionHook{
+			Provisioners: []packer.Provisioner{
+				fileProvisioner,
+				dirProvisioner,
+				shellProvisioner,
+				verifyProvisioner,
+			},
+			ProvisionerTypes: []string{"", "", "", ""},
+		},
+	}
+	hook := &packer.DispatchHook{Mapping: hooks}
+
+	artifact, err := builder.Run(ui, hook, cache)
+	if err != nil {
+		t.Fatalf("Error running build %s", err)
+	}
+	defer artifact.Destroy()
+}
+
 const dockerBuilderConfig = `
 {
   "builders": [
@@ -269,3 +345,40 @@ const dockerLargeBuilderConfig = `
   ]
 }
 `
+
+const testFixUploadOwnerTemplate = `
+{
+  "builders": [
+    {
+      "type": "docker",
+      "image": "ubuntu",
+      "discard": true,
+      "run_command": ["-d", "-i", "-t", "-u", "42", "{{.Image}}", "/bin/sh"]
+    }
+  ],
+  "provisioners": [
+    {
+      "type": "file",
+      "source": "test-fixtures/onecakes/strawberry",
+      "destination": "/tmp/strawberry-cake"
+    },
+    {
+      "type": "file",
+      "source": "test-fixtures/manycakes",
+      "destination": "/tmp/"
+    },
+    {
+      "type": "shell",
+      "inline": "touch /tmp/testUploadOwner"
+    },
+    {
+      "type": "shell",
+      "inline": [
+        "[ $(stat -c %u /tmp/strawberry-cake) -eq 42 ] || (echo 'Invalid owner of /tmp/strawberry-cake' && exit 1)",
+        "[ $(stat -c %u /tmp/testUploadOwner) -eq 42 ] || (echo 'Invalid owner of /tmp/testUploadOwner' && exit 1)",
+        "find /tmp/manycakes | xargs -n1 -IFILE /bin/sh -c '[ $(stat -c %u FILE) -eq 42 ] || (echo \"Invalid owner of FILE\" && exit 1)'"
+      ]
+    }
+  ]
+}
+`

builder/docker/config.go

@@ -23,20 +23,21 @@ type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
 	Comm                communicator.Config `mapstructure:",squash"`
 
-	Author       string
-	Changes      []string
-	Commit       bool
-	ContainerDir string `mapstructure:"container_dir"`
-	Discard      bool
-	ExecUser     string `mapstructure:"exec_user"`
-	ExportPath   string `mapstructure:"export_path"`
-	Image        string
-	Message      string
-	Privileged   bool `mapstructure:"privileged"`
-	Pty          bool
-	Pull         bool
-	RunCommand   []string `mapstructure:"run_command"`
-	Volumes      map[string]string
+	Author         string
+	Changes        []string
+	Commit         bool
+	ContainerDir   string `mapstructure:"container_dir"`
+	Discard        bool
+	ExecUser       string `mapstructure:"exec_user"`
+	ExportPath     string `mapstructure:"export_path"`
+	Image          string
+	Message        string
+	Privileged     bool `mapstructure:"privileged"`
+	Pty            bool
+	Pull           bool
+	RunCommand     []string `mapstructure:"run_command"`
+	Volumes        map[string]string
+	FixUploadOwner bool `mapstructure:"fix_upload_owner"`
 
 	// This is used to login to dockerhub to pull a private base container. For
 	// pushing to dockerhub, see the docker post-processors
@@ -54,6 +55,8 @@ type Config struct {
 func NewConfig(raws ...interface{}) (*Config, []string, error) {
 	c := new(Config)
 
+	c.FixUploadOwner = true
+
 	var md mapstructure.Metadata
 	err := config.Decode(c, &config.DecodeOpts{
 		Metadata:           &md,

builder/docker/step_connect_docker.go

@@ -1,7 +1,10 @@
 package docker
 
 import (
+	"fmt"
 	"github.com/mitchellh/multistep"
+	"os/exec"
+	"strings"
 )
 
 type StepConnectDocker struct{}
@@ -19,14 +22,21 @@ func (s *StepConnectDocker) Run(state multistep.StateBag) multistep.StepAction {
 		return multistep.ActionHalt
 	}
 
+	containerUser, err := getContainerUser(containerId)
+	if err != nil {
+		state.Put("error", err)
+		return multistep.ActionHalt
+	}
+
 	// Create the communicator that talks to Docker via various
 	// os/exec tricks.
 	comm := &Communicator{
-		ContainerID:  containerId,
-		HostDir:      tempDir,
-		ContainerDir: config.ContainerDir,
-		Version:      version,
-		Config:       config,
+		ContainerID:   containerId,
+		HostDir:       tempDir,
+		ContainerDir:  config.ContainerDir,
+		Version:       version,
+		Config:        config,
+		ContainerUser: containerUser,
 	}
 
 	state.Put("communicator", comm)
@@ -34,3 +44,16 @@ func (s *StepConnectDocker) Run(state multistep.StateBag) multistep.StepAction {
 }
 
 func (s *StepConnectDocker) Cleanup(state multistep.StateBag) {}
+
+func getContainerUser(containerId string) (string, error) {
+	inspectArgs := []string{"docker", "inspect", "--format", "{{.Config.User}}", containerId}
+	stdout, err := exec.Command(inspectArgs[0], inspectArgs[1:]...).Output()
+	if err != nil {
+		errStr := fmt.Sprintf("Failed to inspect the container: %s", err)
+		if ee, ok := err.(*exec.ExitError); ok {
+			errStr = fmt.Sprintf("%s, %s", errStr, ee.Stderr)
+		}
+		return "", fmt.Errorf(errStr)
+	}
+	return strings.TrimSpace(string(stdout)), nil
+}

website/source/docs/builders/docker.html.md

@@ -211,6 +211,10 @@ You must specify (only) one of `commit`, `discard`, or `export_path`.
 -   `container_dir` (string) - The directory inside container to mount
      temp directory from host server for work [file provisioner](/docs/provisioners/file.html).
      By default this is set to `/packer-files`.
+     
+-   `fix_upload_owner` (boolean) - If true, files uploaded to the container will
+    be owned by the user the container is running as. If false, the owner will depend
+    on the version of docker installed in the system. Defaults to true.
 
 ## Using the Artifact: Export