fix(*): update parameter set logic to support file types (#1137)

* fix(*): update parameter set logic to support file types Signed-off-by: Vaughn Dice <[email protected]> * move applyDefaultOptions to sharedOptions.Validate() Signed-off-by: Vaughn Dice <[email protected]>
getporter · Jul 21, 2020 · aee93e9 · aee93e9
1 parent a355e30
commit aee93e9
Show file tree

Hide file tree

Showing 15 changed files with 134 additions and 27 deletions.
diff --git a/pkg/porter/cnab.go b/pkg/porter/cnab.go
@@ -93,6 +93,11 @@ func (o *sharedOptions) Validate(args []string, p *Porter) error {
 		return err
 	}
 
+	err = p.applyDefaultOptions(o)
+	if err != nil {
+		return err
+	}
+
 	err = o.validateParams(p)
 	if err != nil {
 		return err
@@ -234,11 +239,13 @@ func (o *sharedOptions) parseParams() error {
 
 // parseParamSets parses the variable assignments in ParameterSets.
 func (o *sharedOptions) parseParamSets(p *Porter) error {
-	parsed, err := p.loadParameterSets(o.ParameterSets)
-	if err != nil {
-		return errors.Wrapf(err, "unable to process provided parameter sets: %v", o.ParameterSets)
+	if len(o.ParameterSets) > 0 {
+		parsed, err := p.loadParameterSets(o.ParameterSets)
+		if err != nil {
+			return errors.Wrapf(err, "unable to process provided parameter sets: %v", o.ParameterSets)
+		}
+		o.parsedParamSets = parsed
 	}
-	o.parsedParamSets = parsed
 	return nil
 }
 

diff --git a/pkg/porter/cnab_test.go b/pkg/porter/cnab_test.go
@@ -104,7 +104,10 @@ func TestParseParamSets_viaPathOrName(t *testing.T) {
 		},
 	}
 
-	err := opts.parseParamSets(p.Porter)
+	err := opts.Validate([]string{}, p.Porter)
+	assert.NoError(t, err)
+
+	err = opts.parseParamSets(p.Porter)
 	assert.NoError(t, err)
 
 	wantParams := map[string]string{
@@ -114,6 +117,33 @@ func TestParseParamSets_viaPathOrName(t *testing.T) {
 	assert.Equal(t, wantParams, opts.parsedParamSets, "resolved unexpected parameter values")
 }
 
+func TestParseParamSets_FileType(t *testing.T) {
+	p := NewTestPorter(t)
+
+	p.TestConfig.TestContext.AddTestFile("testdata/porter-with-file-param.yaml", "porter.yaml")
+	p.TestConfig.TestContext.AddTestFile("testdata/paramset-with-file-param.json", "/paramset.json")
+
+	opts := sharedOptions{
+		ParameterSets: []string{
+			"/paramset.json",
+		},
+		bundleFileOptions: bundleFileOptions{
+			File: "porter.yaml",
+		},
+	}
+
+	err := opts.Validate([]string{}, p.Porter)
+	assert.NoError(t, err)
+
+	err = opts.parseParamSets(p.Porter)
+	assert.NoError(t, err)
+
+	wantParams := map[string]string{
+		"my-file-param": "/local/path/to/my-file-param",
+	}
+	assert.Equal(t, wantParams, opts.parsedParamSets, "resolved unexpected parameter values")
+}
+
 func TestCombineParameters(t *testing.T) {
 	t.Run("no override present, no parameter set present", func(t *testing.T) {
 		opts := sharedOptions{}

diff --git a/pkg/porter/install.go b/pkg/porter/install.go
@@ -22,11 +22,6 @@ func (p *Porter) InstallBundle(opts InstallOptions) error {
 		return errors.Wrap(err, "unable to pull bundle before installation")
 	}
 
-	err = p.applyDefaultOptions(&opts.sharedOptions)
-	if err != nil {
-		return err
-	}
-
 	err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
 	if err != nil {
 		return err

diff --git a/pkg/porter/invoke.go b/pkg/porter/invoke.go
@@ -32,11 +32,6 @@ func (p *Porter) InvokeBundle(opts InvokeOptions) error {
 		return errors.Wrap(err, "unable to pull bundle before invoking the custom action")
 	}
 
-	err = p.applyDefaultOptions(&opts.sharedOptions)
-	if err != nil {
-		return err
-	}
-
 	err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
 	if err != nil {
 		return err

diff --git a/pkg/porter/lifecycle_test.go b/pkg/porter/lifecycle_test.go
@@ -4,6 +4,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"get.porter.sh/porter/pkg/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -84,6 +85,10 @@ func TestInstallFromTagIgnoresCurrentBundle(t *testing.T) {
 
 func TestBundleLifecycleOpts_ToActionArgs(t *testing.T) {
 	p := NewTestPorter(t)
+	cxt := p.TestConfig.TestContext
+
+	// Add manifest which is used to parse parameter sets
+	cxt.AddTestFile("testdata/porter.yaml", config.Name)
 
 	deps := &dependencyExecutioner{}
 
@@ -118,6 +123,7 @@ func TestBundleLifecycleOpts_ToActionArgs(t *testing.T) {
 			sharedOptions: sharedOptions{
 				bundleFileOptions: bundleFileOptions{
 					RelocationMapping: "relocation-mapping.json",
+					File:              config.Name,
 				},
 				Name: "MyClaim",
 				Params: []string{

diff --git a/pkg/porter/options.go b/pkg/porter/options.go
@@ -1,6 +1,8 @@
 package porter
 
-import "get.porter.sh/porter/pkg/manifest"
+import (
+	"get.porter.sh/porter/pkg/manifest"
+)
 
 // applyDefaultOptions applies more advanced defaults to the options
 // based on values that beyond just what was supplied by the user

diff --git a/pkg/porter/parameters.go b/pkg/porter/parameters.go
@@ -307,6 +307,23 @@ func (p *Porter) loadParameterSets(params []string) (valuesource.Set, error) {
 			return nil, err
 		}
 
+		// A parameter may correspond to a Porter-specific parameter type of 'file'
+		// If so, add value (filepath) directly to map and remove from pset
+		for _, paramDef := range p.Manifest.Parameters {
+			if paramDef.Type == "file" {
+				for i, param := range pset.Parameters {
+					if param.Name == paramDef.Name {
+						// Pass through value (filepath) directly to resolvedParameters
+						resolvedParameters[param.Name] = param.Source.Value
+						// Eliminate this param from pset to prevent its resolution by
+						// the cnab-go library, which doesn't support this parameter type
+						pset.Parameters[i] = pset.Parameters[len(pset.Parameters)-1]
+						pset.Parameters = pset.Parameters[:len(pset.Parameters)-1]
+					}
+				}
+			}
+		}
+
 		rc, err := p.Parameters.ResolveAll(pset)
 		if err != nil {
 			return nil, err

diff --git a/pkg/porter/testdata/paramset-with-file-param.json b/pkg/porter/testdata/paramset-with-file-param.json
@@ -0,0 +1,13 @@
+{
+  "name": "mypset",
+  "created": "1983-04-18T01:02:03.000000004Z",
+  "modified": "1983-04-18T01:02:03.000000004Z",
+  "parameters": [
+    {
+      "name": "my-file-param",
+      "source": {
+        "path": "/local/path/to/my-file-param"
+      }
+    }
+  ]
+}
diff --git a/pkg/porter/testdata/porter-with-file-param.yaml b/pkg/porter/testdata/porter-with-file-param.yaml
@@ -0,0 +1,27 @@
+name: HELLO_CUSTOM
+version: 0.1.0
+description: "A bundle with a custom action"
+tag: getporter/porter-hello:v0.1.0
+
+parameters:
+  - name: my-file-param
+    description: "My file param"
+    type: file
+    path: /container/path/to/file
+
+mixins:
+  - exec
+
+install:
+  - exec:
+      description: "cat file"
+      command: bash
+      flags:
+        c: '"cat /container/path/to/file"'
+
+uninstall:
+  - exec:
+      description: "cat file"
+      command: bash
+      flags:
+        c: '"cat /container/path/to/file"'
diff --git a/pkg/porter/uninstall.go b/pkg/porter/uninstall.go
@@ -22,11 +22,6 @@ func (p *Porter) UninstallBundle(opts UninstallOptions) error {
 		return errors.Wrap(err, "unable to pull bundle before uninstall")
 	}
 
-	err = p.applyDefaultOptions(&opts.sharedOptions)
-	if err != nil {
-		return err
-	}
-
 	err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
 	if err != nil {
 		return err

diff --git a/pkg/porter/upgrade.go b/pkg/porter/upgrade.go
@@ -22,11 +22,6 @@ func (p *Porter) UpgradeBundle(opts UpgradeOptions) error {
 		return errors.Wrap(err, "unable to pull bundle before upgrade")
 	}
 
-	err = p.applyDefaultOptions(&opts.sharedOptions)
-	if err != nil {
-		return err
-	}
-
 	err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
 	if err != nil {
 		return err

diff --git a/tests/install_test.go b/tests/install_test.go
@@ -51,9 +51,11 @@ func TestInstall_fileParam(t *testing.T) {
 	p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/bundle-with-file-params.yaml"), "porter.yaml")
 	p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/helpers.sh"), "helpers.sh")
 	p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/myfile"), "./myfile")
+	p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/myotherfile"), "./myotherfile")
 
 	installOpts := porter.InstallOptions{}
 	installOpts.Params = []string{"myfile=./myfile"}
+	installOpts.ParameterSets = []string{filepath.Join(p.TestDir, "testdata/parameter-set-with-file-param.json")}
 
 	err := installOpts.Validate([]string{}, p.Porter)
 	require.NoError(t, err)
@@ -67,5 +69,6 @@ func TestInstall_fileParam(t *testing.T) {
 
 	claim, err := p.Claims.Read(p.Manifest.Name)
 	require.NoError(t, err, "could not fetch claim")
-	require.Equal(t, "Hello World!", claim.Outputs["myfile"], "expected output to match the decoded file contents")
+	require.Equal(t, "Hello World!", claim.Outputs["myfile"], "expected output 'myfile' to match the decoded file contents")
+	require.Equal(t, "Hello Other World!", claim.Outputs["myotherfile"], "expected output 'myotherfile' to match the decoded file contents")
 }
diff --git a/tests/testdata/bundle-with-file-params.yaml b/tests/testdata/bundle-with-file-params.yaml
@@ -10,6 +10,9 @@ parameters:
   - name: myfile
     type: file
     path: /root/myfile
+  - name: myotherfile
+    type: file
+    path: /root/myotherfile
   # This is added to cover bug fix for https://github.com/deislabs/porter/issues/835
   # It will be inherently exercised in install_test.go via a default bundle installation
   - name: onlyUpgrade
@@ -22,6 +25,11 @@ outputs:
     type: string
     applyTo:
       - install
+  - name: myotherfile
+    type: file
+    path: /root/myotherfile
+    applyTo:
+      - install
 
 install:
   - exec:

diff --git a/tests/testdata/myotherfile b/tests/testdata/myotherfile
@@ -0,0 +1 @@
+Hello Other World!
diff --git a/tests/testdata/parameter-set-with-file-param.json b/tests/testdata/parameter-set-with-file-param.json
@@ -0,0 +1,13 @@
+{
+  "name": "mybun",
+  "created": "2020-06-23T13:31:06.22727-06:00",
+  "modified": "2020-06-23T13:31:44.692834-06:00",
+  "parameters": [
+    {
+      "name": "myotherfile",
+      "source": {
+        "path": "./myotherfile"
+      }
+    }
+  ]
+}