Merge pull request #112 from carolynvs/op-config-func

Support tweaking the operation before running it

action/action.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"math"
 	"strings"
 
@@ -28,7 +27,7 @@ const stateful = false
 // - status
 type Action interface {
 	// Run an action, and record the status in the given claim
-	Run(*claim.Claim, credentials.Set, io.Writer) error
+	Run(*claim.Claim, credentials.Set, ...OperationConfigFunc) error
 }
 
 func golangTypeToJSONType(value interface{}) (string, error) {
@@ -188,7 +187,7 @@ func appliesToAction(action string, parameter bundle.Parameter) bool {
 	return false
 }
 
-func opFromClaim(action string, stateless bool, c *claim.Claim, ii bundle.InvocationImage, creds credentials.Set, w io.Writer) (*driver.Operation, error) {
+func opFromClaim(action string, stateless bool, c *claim.Claim, ii bundle.InvocationImage, creds credentials.Set) (*driver.Operation, error) {
 	env, files, err := creds.Expand(c.Bundle, stateless)
 	if err != nil {
 		return nil, err
@@ -232,7 +231,6 @@ func opFromClaim(action string, stateless bool, c *claim.Claim, ii bundle.Invoca
 		Environment:  env,
 		Files:        files,
 		Outputs:      outputs,
-		Out:          w,
 	}, nil
 }
 

action/action_test.go

@@ -3,7 +3,6 @@ package action
 import (
 	"encoding/json"
 	"errors"
-	"os"
 	"strings"
 	"testing"
 	"time"
@@ -20,6 +19,7 @@ import (
 
 type mockDriver struct {
 	shouldHandle bool
+	Operation    *driver.Operation
 	Result       driver.OperationResult
 	Error        error
 }
@@ -28,6 +28,7 @@ func (d *mockDriver) Handles(imageType string) bool {
 	return d.shouldHandle
 }
 func (d *mockDriver) Run(op *driver.Operation) (driver.OperationResult, error) {
+	d.Operation = op
 	return d.Result, d.Error
 }
 
@@ -190,7 +191,7 @@ func TestOpFromClaim(t *testing.T) {
 	}
 	invocImage := c.Bundle.InvocationImages[0]
 
-	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -216,7 +217,7 @@ func TestOpFromClaim(t *testing.T) {
 	is.NoError(json.Unmarshal([]byte(op.Files["/cnab/app/image-map.json"]), &imgMap))
 	is.Equal(c.Bundle.Images, imgMap)
 	is.Len(op.Parameters, 7)
-	is.Equal(os.Stdout, op.Out)
+	is.Nil(op.Out)
 }
 
 func TestOpFromClaim_NoOutputsOnBundle(t *testing.T) {
@@ -225,7 +226,7 @@ func TestOpFromClaim_NoOutputsOnBundle(t *testing.T) {
 	c.Bundle.Outputs = nil
 	invocImage := c.Bundle.InvocationImages[0]
 
-	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -243,7 +244,7 @@ func TestOpFromClaim_NoOutputsOnBundle(t *testing.T) {
 	is.NoError(json.Unmarshal([]byte(op.Files["/cnab/app/image-map.json"]), &imgMap))
 	is.Equal(c.Bundle.Images, imgMap)
 	is.Len(op.Parameters, 0)
-	is.Equal(os.Stdout, op.Out)
+	is.Nil(op.Out)
 }
 
 func TestOpFromClaim_NoParameter(t *testing.T) {
@@ -252,7 +253,7 @@ func TestOpFromClaim_NoParameter(t *testing.T) {
 	c.Bundle.Parameters = nil
 	invocImage := c.Bundle.InvocationImages[0]
 
-	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -270,7 +271,7 @@ func TestOpFromClaim_NoParameter(t *testing.T) {
 	is.NoError(json.Unmarshal([]byte(op.Files["/cnab/app/image-map.json"]), &imgMap))
 	is.Equal(c.Bundle.Images, imgMap)
 	is.Len(op.Parameters, 0)
-	is.Equal(os.Stdout, op.Out)
+	is.Nil(op.Out)
 }
 
 func TestOpFromClaim_UndefinedParams(t *testing.T) {
@@ -283,7 +284,7 @@ func TestOpFromClaim_UndefinedParams(t *testing.T) {
 	}
 	invocImage := c.Bundle.InvocationImages[0]
 
-	_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+	_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 	assert.Error(t, err)
 }
 
@@ -298,13 +299,13 @@ func TestOpFromClaim_MissingRequiredParameter(t *testing.T) {
 	invocImage := c.Bundle.InvocationImages[0]
 
 	t.Run("missing required parameter fails", func(t *testing.T) {
-		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 		assert.EqualError(t, err, `missing required parameter "param_one" for action "install"`)
 	})
 
 	t.Run("fill the missing parameter", func(t *testing.T) {
 		c.Parameters["param_one"] = "oneval"
-		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 		assert.Nil(t, err)
 	})
 }
@@ -326,18 +327,18 @@ func TestOpFromClaim_MissingRequiredParamSpecificToAction(t *testing.T) {
 	invocImage := c.Bundle.InvocationImages[0]
 
 	t.Run("if param is not required for this action, succeed", func(t *testing.T) {
-		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet, os.Stdout)
+		_, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, mockSet)
 		assert.Nil(t, err)
 	})
 
 	t.Run("if param is required for this action and is missing, error", func(t *testing.T) {
-		_, err := opFromClaim("test", stateful, c, invocImage, mockSet, os.Stdout)
+		_, err := opFromClaim("test", stateful, c, invocImage, mockSet)
 		assert.EqualError(t, err, `missing required parameter "param_test" for action "test"`)
 	})
 
 	t.Run("if param is required for this action and is set, succeed", func(t *testing.T) {
 		c.Parameters["param_test"] = "only for test action"
-		_, err := opFromClaim("test", stateful, c, invocImage, mockSet, os.Stdout)
+		_, err := opFromClaim("test", stateful, c, invocImage, mockSet)
 		assert.Nil(t, err)
 	})
 }

action/install.go

@@ -1,8 +1,6 @@
 package action
 
 import (
-	"io"
-
 	"github.com/deislabs/cnab-go/claim"
 	"github.com/deislabs/cnab-go/credentials"
 	"github.com/deislabs/cnab-go/driver"
@@ -14,13 +12,18 @@ type Install struct {
 }
 
 // Run performs an installation and updates the Claim accordingly
-func (i *Install) Run(c *claim.Claim, creds credentials.Set, w io.Writer) error {
+func (i *Install) Run(c *claim.Claim, creds credentials.Set, opCfgs ...OperationConfigFunc) error {
 	invocImage, err := selectInvocationImage(i.Driver, c)
 	if err != nil {
 		return err
 	}
 
-	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, creds, w)
+	op, err := opFromClaim(claim.ActionInstall, stateful, c, invocImage, creds)
+	if err != nil {
+		return err
+	}
+
+	err = OperationConfigs(opCfgs).ApplyConfig(op)
 	if err != nil {
 		return err
 	}

action/install_test.go

@@ -7,15 +7,18 @@ 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 Install implements Action interface
 var _ Action = &Install{}
 
 func TestInstall_Run(t *testing.T) {
-	out := ioutil.Discard
+	out := func(op *driver.Operation) error {
+		op.Out = ioutil.Discard
+		return nil
+	}
 
 	t.Run("happy-path", func(t *testing.T) {
 		c := newClaim()
@@ -34,6 +37,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}
+
+		addFile := func(op *driver.Operation) error {
+			op.Files["/tmp/another/path"] = "ANOTHER FILE"
+			return nil
+		}
+		require.NoError(t, inst.Run(c, mockSet, out, addFile))
+		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}
+		sabotage := func(op *driver.Operation) error {
+			return errors.New("oops")
+		}
+		require.EqualError(t, inst.Run(c, mockSet, out, sabotage), "oops")
+	})
+
 	t.Run("when the bundle has no outputs", func(t *testing.T) {
 		c := newClaim()
 		c.Bundle.Outputs = nil
@@ -79,4 +121,5 @@ func TestInstall_Run(t *testing.T) {
 		assert.Equal(t, claim.ActionInstall, c.Result.Action)
 		assert.Equal(t, map[string]interface{}{"some-output": "SOME CONTENT"}, c.Outputs)
 	})
+
 }

action/operation_configs.go

@@ -0,0 +1,26 @@
+package action
+
+import (
+	"github.com/deislabs/cnab-go/driver"
+)
+
+// OperationConfigFunc is a configuration function that can be applied to an
+// operation.
+type OperationConfigFunc func(op *driver.Operation) error
+
+// OperationConfigs is a set of configuration functions that can be applied as a
+// unit to an operation.
+type OperationConfigs []OperationConfigFunc
+
+// ApplyConfig safely applies the configuration function to the operation, if
+// defined, and stops immediately upon the first error.
+func (cfgs OperationConfigs) ApplyConfig(op *driver.Operation) error {
+	var err error
+	for _, cfg := range cfgs {
+		err = cfg(op)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}

action/operation_configs_test.go

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

action/run_custom.go

@@ -2,7 +2,6 @@ package action
 
 import (
 	"errors"
-	"io"
 
 	"github.com/deislabs/cnab-go/claim"
 	"github.com/deislabs/cnab-go/credentials"
@@ -28,7 +27,7 @@ type RunCustom struct {
 var blockedActions = map[string]struct{}{"install": {}, "uninstall": {}, "upgrade": {}}
 
 // Run executes a status action in an image
-func (i *RunCustom) Run(c *claim.Claim, creds credentials.Set, w io.Writer) error {
+func (i *RunCustom) Run(c *claim.Claim, creds credentials.Set, opCfgs ...OperationConfigFunc) error {
 	if _, ok := blockedActions[i.Action]; ok {
 		return ErrBlockedAction
 	}
@@ -43,7 +42,12 @@ func (i *RunCustom) Run(c *claim.Claim, creds credentials.Set, w io.Writer) erro
 		return err
 	}
 
-	op, err := opFromClaim(i.Action, actionDef.Stateless, c, invocImage, creds, w)
+	op, err := opFromClaim(i.Action, actionDef.Stateless, c, invocImage, creds)
+	if err != nil {
+		return err
+	}
+
+	err = OperationConfigs(opCfgs).ApplyConfig(op)
 	if err != nil {
 		return err
 	}