Merge pull request #206 from vdice/fix/container-config

ref(docker.go): container config fields on Driver no longer pointers
cnabio · May 2, 2020 · 1db08a3 · 1db08a3
2 parents bc0cd79 + 5395191
commit 1db08a3
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 26 deletions.
diff --git a/driver/docker/docker.go b/driver/docker/docker.go
@@ -21,7 +21,7 @@ import (
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/docker/registry"
-	copystructure "github.com/mitchellh/copystructure"
+	"github.com/mitchellh/copystructure"
 )
 
 // Driver is capable of running Docker invocation images using Docker itself.
@@ -33,8 +33,8 @@ type Driver struct {
 	dockerConfigurationOptions []ConfigurationOption
 	containerOut               io.Writer
 	containerErr               io.Writer
-	containerHostCfg           *container.HostConfig
-	containerCfg               *container.Config
+	containerHostCfg           container.HostConfig
+	containerCfg               container.Config
 }
 
 // Run executes the Docker driver
@@ -55,33 +55,33 @@ func (d *Driver) AddConfigurationOptions(opts ...ConfigurationOption) {
 // GetContainerConfig returns a copy of the container configuration
 // used by the driver during container exec
 func (d *Driver) GetContainerConfig() (container.Config, error) {
-	cpy, err := copystructure.Copy(*d.containerCfg)
+	cpy, err := copystructure.Copy(d.containerCfg)
 	if err != nil {
 		return container.Config{}, err
 	}
 
-	containerCfg, ok := cpy.(container.Config)
+	cfg, ok := cpy.(container.Config)
 	if !ok {
 		return container.Config{}, errors.New("unable to process container config")
 	}
 
-	return containerCfg, nil
+	return cfg, nil
 }
 
 // GetContainerHostConfig returns a copy of the container host configuration
 // used by the driver during container exec
 func (d *Driver) GetContainerHostConfig() (container.HostConfig, error) {
-	cpy, err := copystructure.Copy(*d.containerHostCfg)
+	cpy, err := copystructure.Copy(d.containerHostCfg)
 	if err != nil {
 		return container.HostConfig{}, err
 	}
 
-	hostCfg, ok := cpy.(container.HostConfig)
+	cfg, ok := cpy.(container.HostConfig)
 	if !ok {
 		return container.HostConfig{}, errors.New("unable to process container host config")
 	}
 
-	return hostCfg, nil
+	return cfg, nil
 }
 
 // Config returns the Docker driver configuration options
@@ -191,27 +191,27 @@ func (d *Driver) exec(op *driver.Operation) (driver.OperationResult, error) {
 		env = append(env, fmt.Sprintf("%s=%v", k, v))
 	}
 
-	d.containerCfg = &container.Config{
+	d.containerCfg = container.Config{
 		Image:        op.Image.Image,
 		Env:          env,
 		Entrypoint:   strslice.StrSlice{"/cnab/app/run"},
 		AttachStderr: true,
 		AttachStdout: true,
 	}
 
-	d.containerHostCfg = &container.HostConfig{}
-	if err := d.applyConfigurationOptions(); err != nil {
+	d.containerHostCfg = container.HostConfig{}
+	if err := d.ApplyConfigurationOptions(); err != nil {
 		return driver.OperationResult{}, err
 	}
 
-	resp, err := cli.Client().ContainerCreate(ctx, d.containerCfg, d.containerHostCfg, nil, "")
+	resp, err := cli.Client().ContainerCreate(ctx, &d.containerCfg, &d.containerHostCfg, nil, "")
 	switch {
 	case client.IsErrNotFound(err):
 		fmt.Fprintf(cli.Err(), "Unable to find image '%s' locally\n", op.Image.Image)
 		if err := pullImage(ctx, cli, op.Image.Image); err != nil {
 			return driver.OperationResult{}, err
 		}
-		if resp, err = cli.Client().ContainerCreate(ctx, d.containerCfg, d.containerHostCfg, nil, ""); err != nil {
+		if resp, err = cli.Client().ContainerCreate(ctx, &d.containerCfg, &d.containerHostCfg, nil, ""); err != nil {
 			return driver.OperationResult{}, fmt.Errorf("cannot create container: %v", err)
 		}
 	case err != nil:
@@ -293,9 +293,10 @@ func (d *Driver) exec(op *driver.Operation) (driver.OperationResult, error) {
 	return opResult, err
 }
 
-func (d *Driver) applyConfigurationOptions() error {
+// ApplyConfigurationOptions applies the configuration options set on the driver
+func (d *Driver) ApplyConfigurationOptions() error {
 	for _, opt := range d.dockerConfigurationOptions {
-		if err := opt(d.containerCfg, d.containerHostCfg); err != nil {
+		if err := opt(&d.containerCfg, &d.containerHostCfg); err != nil {
 			return err
 		}
 	}

diff --git a/driver/docker/docker_test.go b/driver/docker/docker_test.go
@@ -5,20 +5,19 @@ import (
 
 	"github.com/cnabio/cnab-go/driver"
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/strslice"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestDriver_GetConfigurationOptions(t *testing.T) {
-	d := &Driver{}
 	is := assert.New(t)
-	is.NotNil(d)
-	is.True(d.Handles(driver.ImageTypeDocker))
 
-	t.Run("no configuration options", func(t *testing.T) {
-		d.containerCfg = &container.Config{}
-		d.containerHostCfg = &container.HostConfig{}
+	t.Run("empty configuration options", func(t *testing.T) {
+		d := &Driver{}
+		is.NotNil(d)
+		is.True(d.Handles(driver.ImageTypeDocker))
 
-		err := d.applyConfigurationOptions()
+		err := d.ApplyConfigurationOptions()
 		is.NoError(err)
 
 		cfg, err := d.GetContainerConfig()
@@ -31,15 +30,15 @@ func TestDriver_GetConfigurationOptions(t *testing.T) {
 	})
 
 	t.Run("configuration options", func(t *testing.T) {
-		d.containerCfg = &container.Config{}
-		d.containerHostCfg = &container.HostConfig{}
+		d := &Driver{}
+
 		d.AddConfigurationOptions(func(cfg *container.Config, hostCfg *container.HostConfig) error {
 			cfg.User = "cnabby"
 			hostCfg.Privileged = true
 			return nil
 		})
 
-		err := d.applyConfigurationOptions()
+		err := d.ApplyConfigurationOptions()
 		is.NoError(err)
 
 		expectedContainerCfg := container.Config{
@@ -57,4 +56,30 @@ func TestDriver_GetConfigurationOptions(t *testing.T) {
 		is.NoError(err)
 		is.Equal(expectedHostCfg, hostCfg)
 	})
+
+	t.Run("configuration options - no unintentional modification", func(t *testing.T) {
+		d := &Driver{}
+
+		d.AddConfigurationOptions(func(cfg *container.Config, hostCfg *container.HostConfig) error {
+			hostCfg.CapAdd = strslice.StrSlice{"SUPER_POWERS"}
+			return nil
+		})
+
+		err := d.ApplyConfigurationOptions()
+		is.NoError(err)
+
+		expectedHostCfg := container.HostConfig{
+			CapAdd: strslice.StrSlice{"SUPER_POWERS"},
+		}
+
+		hostCfg, err := d.GetContainerHostConfig()
+		is.NoError(err)
+		is.Equal(expectedHostCfg, hostCfg)
+
+		hostCfg.CapAdd[0] = "NORMAL_POWERS"
+
+		hostCfg, err = d.GetContainerHostConfig()
+		is.NoError(err)
+		is.Equal(expectedHostCfg, hostCfg)
+	})
 }