Refactor mergeBundleParameters to take the Claim

Instead of a `bundle`, this allows it to `c.Parameters` itself as a
precursor to having it update them.

Signed-off-by: Ian Campbell <[email protected]>

Author: Ian Campbell

internal/commands/cnab.go

@@ -324,14 +324,12 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
 	}
 	c.Bundle = bundle
 
-	parameters, err := mergeBundleParameters(c.Bundle,
+	if err := mergeBundleParameters(c,
 		withFileParameters(paramsOpts.parametersFiles),
 		withCommandLineParameters(paramsOpts.overrides),
-	)
-	if err != nil {
+	); err != nil {
 		return nil, nil, nil, err
 	}
-	c.Parameters = parameters
 
 	a := &action.RunCustom{
 		Action: actionName,

internal/commands/install.go

@@ -98,13 +98,12 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
 	}
 	c.Bundle = bndl
 
-	c.Parameters, err = mergeBundleParameters(bndl,
+	if err := mergeBundleParameters(c,
 		withFileParameters(opts.parametersFiles),
 		withCommandLineParameters(opts.overrides),
 		withOrchestratorParameters(opts.orchestrator, opts.kubeNamespace),
 		withSendRegistryAuth(opts.sendRegistryAuth),
-	)
-	if err != nil {
+	); err != nil {
 		return err
 	}
 	creds, err := prepareCredentialSet(bndl,

internal/commands/parameters.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/deislabs/duffle/pkg/bundle"
+	"github.com/deislabs/duffle/pkg/claim"
 	"github.com/docker/app/internal"
 	"github.com/docker/app/types/parameters"
 	cliopts "github.com/docker/cli/opts"
@@ -60,18 +61,20 @@ func withOrchestratorParameters(orchestrator string, kubeNamespace string) param
 	}
 }
 
-func mergeBundleParameters(bndl *bundle.Bundle, ops ...parameterOperation) (map[string]interface{}, error) {
+func mergeBundleParameters(c *claim.Claim, ops ...parameterOperation) error {
+	bndl := c.Bundle
 	userParams := map[string]string{}
 	for _, op := range ops {
 		if err := op(bndl, userParams); err != nil {
-			return nil, err
+			return err
 		}
 	}
 	convertedParams, err := matchParametersDefinition(userParams, bndl.Parameters)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	return bundle.ValuesOrDefaults(convertedParams, bndl)
+	c.Parameters, err = bundle.ValuesOrDefaults(convertedParams, bndl)
+	return err
 }
 
 func matchParametersDefinition(parameterValues map[string]string, parameterDefinitions map[string]bundle.ParameterDefinition) (map[string]interface{}, error) {

internal/commands/parameters_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/deislabs/duffle/pkg/bundle"
+	"github.com/deislabs/duffle/pkg/claim"
 	"github.com/docker/app/internal"
 	"gotest.tools/assert"
 	"gotest.tools/assert/cmp"
@@ -105,16 +106,16 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-		actual, err := mergeBundleParameters(bundle,
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c,
 			first,
 			second,
 		)
-
 		assert.NilError(t, err)
 		expected := map[string]interface{}{
 			"param": "second",
 		}
-		assert.Assert(t, cmp.DeepEqual(actual, expected))
+		assert.Assert(t, cmp.DeepEqual(c.Parameters, expected))
 	})
 
 	t.Run("Default values", func(t *testing.T) {
@@ -126,13 +127,13 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-		actual, err := mergeBundleParameters(bundle)
-
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c)
 		assert.NilError(t, err)
 		expected := map[string]interface{}{
 			"param": "default",
 		}
-		assert.Assert(t, cmp.DeepEqual(actual, expected))
+		assert.Assert(t, cmp.DeepEqual(c.Parameters, expected))
 	})
 
 	t.Run("Converting values", func(t *testing.T) {
@@ -148,14 +149,13 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-
-		actual, err := mergeBundleParameters(bundle, withIntValue)
-
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c, withIntValue)
 		assert.NilError(t, err)
 		expected := map[string]interface{}{
 			"param": 1,
 		}
-		assert.Assert(t, cmp.DeepEqual(actual, expected))
+		assert.Assert(t, cmp.DeepEqual(c.Parameters, expected))
 	})
 
 	t.Run("Default values", func(t *testing.T) {
@@ -167,13 +167,13 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-		actual, err := mergeBundleParameters(bundle)
-
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c)
 		assert.NilError(t, err)
 		expected := map[string]interface{}{
 			"param": "default",
 		}
-		assert.Assert(t, cmp.DeepEqual(actual, expected))
+		assert.Assert(t, cmp.DeepEqual(c.Parameters, expected))
 	})
 
 	t.Run("Undefined parameter is rejected", func(t *testing.T) {
@@ -184,7 +184,8 @@ func TestMergeBundleParameters(t *testing.T) {
 		bundle := &bundle.Bundle{
 			Parameters: map[string]bundle.ParameterDefinition{},
 		}
-		_, err := mergeBundleParameters(bundle, withUndefined)
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c, withUndefined)
 		assert.ErrorContains(t, err, "is not defined in the bundle")
 	})
 
@@ -200,7 +201,8 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-		_, err := mergeBundleParameters(bundle, withIntValue)
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c, withIntValue)
 		assert.ErrorContains(t, err, "invalid value for parameter")
 	})
 
@@ -217,7 +219,8 @@ func TestMergeBundleParameters(t *testing.T) {
 				},
 			},
 		}
-		_, err := mergeBundleParameters(bundle, withIntValue)
+		c := &claim.Claim{Bundle: bundle}
+		err := mergeBundleParameters(c, withIntValue)
 		assert.ErrorContains(t, err, "invalid value for parameter")
 	})
 }

internal/commands/upgrade.go

@@ -55,12 +55,11 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
 		}
 		c.Bundle = b
 	}
-	c.Parameters, err = mergeBundleParameters(c.Bundle,
+	if err := mergeBundleParameters(&c,
 		withFileParameters(opts.parametersFiles),
 		withCommandLineParameters(opts.overrides),
 		withSendRegistryAuth(opts.sendRegistryAuth),
-	)
-	if err != nil {
+	); err != nil {
 		return err
 	}