Move handler into ConfigurableAction struct and add tests

action/configurable_action.go

@@ -0,0 +1,23 @@
+package action
+
+import (
+	"github.com/deislabs/cnab-go/driver"
+)
+
+type OperationConfigFunc func(op *driver.Operation) error
+
+// ConfigurableAction is used to define actions which can configure operations before they are executed.
+type ConfigurableAction struct {
+	// OperationConfig is an optional handler that applies additional
+	// configuration to an operation before it is executed.
+	OperationConfig OperationConfigFunc
+}
+
+// ApplyConfig safely applies the configuration function to the operation, if defined,
+// and returns any error.
+func (a ConfigurableAction) ApplyConfig(op *driver.Operation) error {
+	if a.OperationConfig != nil {
+		return a.OperationConfig(op)
+	}
+	return nil
+}

action/configurable_action_test.go

@@ -0,0 +1,46 @@
+package action
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/deislabs/cnab-go/driver"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestConfigurableAction_ApplyConfig(t *testing.T) {
+	t.Run("no config defined", func(t *testing.T) {
+		a := ConfigurableAction{}
+		op := &driver.Operation{}
+		err := a.ApplyConfig(op)
+		assert.NoError(t, err, "ApplyConfig should not have returned an error")
+	})
+
+	t.Run("config is persisted", func(t *testing.T) {
+		a := ConfigurableAction{
+			OperationConfig: func(op *driver.Operation) error {
+				if op.Files == nil {
+					op.Files = make(map[string]string, 1)
+				}
+				op.Files["a"] = "b"
+				return nil
+			},
+		}
+		op := &driver.Operation{}
+		err := a.ApplyConfig(op)
+		assert.NoError(t, err, "ApplyConfig should not have returned an error")
+		assert.Contains(t, op.Files, "a", "Changes from the config function were not persisted")
+	})
+
+	t.Run("error is returned", func(t *testing.T) {
+		a := ConfigurableAction{
+			OperationConfig: func(op *driver.Operation) error {
+				return errors.New("oops")
+			},
+		}
+		op := &driver.Operation{}
+		err := a.ApplyConfig(op)
+		require.EqualError(t, err, "oops")
+	})
+}

action/install.go

@@ -10,11 +10,8 @@ import (
 
 // Install describes an installation action
 type Install struct {
+	ConfigurableAction
 	Driver driver.Driver // Needs to be more than a string
-
-	// OperationConfig is an optional handler that applies additional
-	// configuration to the operation before it is executed.
-	OperationConfig func(operation *driver.Operation)
 }
 
 // Run performs an installation and updates the Claim accordingly
@@ -29,8 +26,9 @@ func (i *Install) Run(c *claim.Claim, creds credentials.Set, w io.Writer) error
 		return err
 	}
 
-	if i.OperationConfig != nil {
-		i.OperationConfig(op)
+	err = i.ApplyConfig(op)
+	if err != nil {
+		return err
 	}
 
 	opResult, err := i.Driver.Run(op)

action/install_test.go

@@ -34,6 +34,45 @@ func TestInstall_Run(t *testing.T) {
 		assert.Equal(t, map[string]interface{}{"some-output": "SOME CONTENT"}, c.Outputs)
 	})
 
+	t.Run("configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &Install{Driver: d}
+
+		inst.OperationConfig = func(op *driver.Operation) error {
+			op.Files["/tmp/another/path"] = "ANOTHER FILE"
+			return nil
+		}
+		require.NoError(t, inst.Run(c, mockSet, out))
+		assert.Contains(t, d.Operation.Files, "/tmp/another/path")
+	})
+
+	t.Run("error case: configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &Install{Driver: d}
+		inst.OperationConfig = func(op *driver.Operation) error {
+			return errors.New("oops")
+		}
+		require.EqualError(t, inst.Run(c, mockSet, out), "oops")
+	})
+
 	t.Run("when the bundle has no outputs", func(t *testing.T) {
 		c := newClaim()
 		c.Bundle.Outputs = nil
@@ -80,23 +119,4 @@ func TestInstall_Run(t *testing.T) {
 		assert.Equal(t, map[string]interface{}{"some-output": "SOME CONTENT"}, c.Outputs)
 	})
 
-	t.Run("configure-operation", func(t *testing.T) {
-		c := newClaim()
-		d := &mockDriver{
-			shouldHandle: true,
-			Result: driver.OperationResult{
-				Outputs: map[string]string{
-					"/tmp/some/path": "SOME CONTENT",
-				},
-			},
-			Error: nil,
-		}
-		inst := &Install{Driver: d}
-
-		inst.OperationConfig = func(op *driver.Operation) {
-			op.Files["/tmp/another/path"] = "ANOTHER FILE"
-		}
-		require.NoError(t, inst.Run(c, mockSet, out))
-		assert.Contains(t, d.Operation.Files, "/tmp/another/path")
-	})
 }

action/run_custom.go

@@ -18,12 +18,9 @@ var (
 
 // RunCustom allows the execution of an arbitrary target in a CNAB bundle.
 type RunCustom struct {
+	ConfigurableAction
 	Driver driver.Driver
 	Action string
-
-	// OperationConfig is an optional handler that applies additional
-	// configuration to the operation before it is executed.
-	OperationConfig func(operation *driver.Operation)
 }
 
 // blockedActions is a list of actions that cannot be run as custom.
@@ -52,8 +49,9 @@ func (i *RunCustom) Run(c *claim.Claim, creds credentials.Set, w io.Writer) erro
 		return err
 	}
 
-	if i.OperationConfig != nil {
-		i.OperationConfig(op)
+	err = i.ApplyConfig(op)
+	if err != nil {
+		return err
 	}
 
 	opResult, err := i.Driver.Run(op)

action/run_custom_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/deislabs/cnab-go/claim"
 	"github.com/deislabs/cnab-go/driver"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // makes sure RunCustom implements Action interface
@@ -38,6 +39,44 @@ func TestRunCustom(t *testing.T) {
 		assert.Equal(t, map[string]interface{}{"some-output": "SOME CONTENT"}, c.Outputs)
 	})
 
+	t.Run("configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &RunCustom{Driver: d, Action: "test"}
+		inst.OperationConfig = func(op *driver.Operation) error {
+			op.Files["/tmp/another/path"] = "ANOTHER FILE"
+			return nil
+		}
+		require.NoError(t, inst.Run(c, mockSet, out))
+		assert.Contains(t, d.Operation.Files, "/tmp/another/path")
+	})
+
+	t.Run("error case: configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &RunCustom{Driver: d, Action: "test"}
+		inst.OperationConfig = func(op *driver.Operation) error {
+			return errors.New("oops")
+		}
+		require.EqualError(t, inst.Run(c, mockSet, out), "oops")
+	})
+
 	t.Run("when there are no outputs in the bundle", func(t *testing.T) {
 		c := newClaim()
 		c.Bundle.Outputs = nil

action/status.go

@@ -10,11 +10,8 @@ import (
 
 // Status runs a status action on a CNAB bundle.
 type Status struct {
+	ConfigurableAction
 	Driver driver.Driver
-
-	// OperationConfig is an optional handler that applies additional
-	// configuration to the operation before it is executed.
-	OperationConfig func(operation *driver.Operation)
 }
 
 // Run executes a status action in an image
@@ -29,8 +26,9 @@ func (i *Status) Run(c *claim.Claim, creds credentials.Set, w io.Writer) error {
 		return err
 	}
 
-	if i.OperationConfig != nil {
-		i.OperationConfig(op)
+	err = i.ApplyConfig(op)
+	if err != nil {
+		return err
 	}
 
 	// Ignore OperationResult because non-modifying actions don't have outputs to save.

action/status_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/deislabs/cnab-go/driver"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // makes sure Status implements Action interface
@@ -34,6 +35,44 @@ func TestStatus_Run(t *testing.T) {
 		assert.Empty(t, c.Outputs)
 	})
 
+	t.Run("configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &Status{Driver: d}
+		inst.OperationConfig = func(op *driver.Operation) error {
+			op.Files["/tmp/another/path"] = "ANOTHER FILE"
+			return nil
+		}
+		require.NoError(t, inst.Run(c, mockSet, out))
+		assert.Contains(t, d.Operation.Files, "/tmp/another/path")
+	})
+
+	t.Run("error case: configure operation", func(t *testing.T) {
+		c := newClaim()
+		d := &mockDriver{
+			shouldHandle: true,
+			Result: driver.OperationResult{
+				Outputs: map[string]string{
+					"/tmp/some/path": "SOME CONTENT",
+				},
+			},
+			Error: nil,
+		}
+		inst := &Status{Driver: d}
+		inst.OperationConfig = func(op *driver.Operation) error {
+			return errors.New("oops")
+		}
+		require.EqualError(t, inst.Run(c, mockSet, out), "oops")
+	})
+
 	t.Run("error case: driver doesn't handle image", func(t *testing.T) {
 		c := newClaim()
 		st := &Status{Driver: &mockDriver{Error: errors.New("I always fail")}}

action/uninstall.go

@@ -10,11 +10,8 @@ import (
 
 // Uninstall runs an uninstall action
 type Uninstall struct {
+	ConfigurableAction
 	Driver driver.Driver
-
-	// OperationConfig is an optional handler that applies additional
-	// configuration to the operation before it is executed.
-	OperationConfig func(operation *driver.Operation)
 }
 
 // Run performs the uninstall steps and updates the Claim
@@ -29,8 +26,9 @@ func (u *Uninstall) Run(c *claim.Claim, creds credentials.Set, w io.Writer) erro
 		return err
 	}
 
-	if u.OperationConfig != nil {
-		u.OperationConfig(op)
+	err = u.ApplyConfig(op)
+	if err != nil {
+		return err
 	}
 
 	opResult, err := u.Driver.Run(op)