From aee93e9826a5a8191a051e90e22229a426249865 Mon Sep 17 00:00:00 2001 From: Vaughn Dice Date: Tue, 21 Jul 2020 15:49:10 -0600 Subject: [PATCH] fix(*): update parameter set logic to support file types (#1137) * fix(*): update parameter set logic to support file types Signed-off-by: Vaughn Dice * move applyDefaultOptions to sharedOptions.Validate() Signed-off-by: Vaughn Dice --- pkg/porter/cnab.go | 15 ++++++--- pkg/porter/cnab_test.go | 32 ++++++++++++++++++- pkg/porter/install.go | 5 --- pkg/porter/invoke.go | 5 --- pkg/porter/lifecycle_test.go | 6 ++++ pkg/porter/options.go | 4 ++- pkg/porter/parameters.go | 17 ++++++++++ .../testdata/paramset-with-file-param.json | 13 ++++++++ .../testdata/porter-with-file-param.yaml | 27 ++++++++++++++++ pkg/porter/uninstall.go | 5 --- pkg/porter/upgrade.go | 5 --- tests/install_test.go | 5 ++- tests/testdata/bundle-with-file-params.yaml | 8 +++++ tests/testdata/myotherfile | 1 + .../parameter-set-with-file-param.json | 13 ++++++++ 15 files changed, 134 insertions(+), 27 deletions(-) create mode 100644 pkg/porter/testdata/paramset-with-file-param.json create mode 100644 pkg/porter/testdata/porter-with-file-param.yaml create mode 100644 tests/testdata/myotherfile create mode 100644 tests/testdata/parameter-set-with-file-param.json diff --git a/pkg/porter/cnab.go b/pkg/porter/cnab.go index 2e003eaf2..3ca4b6a87 100644 --- 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 index bef422b6e..29cf1b4d3 100644 --- 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 index a96c295a8..e8ba32b1d 100644 --- 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 index ed4b13f89..7ac233de7 100644 --- 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 index 7cdd22d7c..731d9d162 100644 --- 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 index 51d1cc3f1..0bad8e062 100644 --- 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 index 84523fea4..ca26baf00 100644 --- 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 new file mode 100644 index 000000000..6a3856522 --- /dev/null +++ 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 new file mode 100644 index 000000000..f80b04c26 --- /dev/null +++ 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 index e8b629ee6..5af9edb3f 100644 --- 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 index b445cf0af..143458cfe 100644 --- 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 index 20df66c56..ccd5a7bc6 100644 --- 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 index 5ebab2f31..677f65f62 100644 --- 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 new file mode 100644 index 000000000..cbe6fe039 --- /dev/null +++ b/tests/testdata/myotherfile @@ -0,0 +1 @@ +Hello Other World! \ No newline at end of file diff --git a/tests/testdata/parameter-set-with-file-param.json b/tests/testdata/parameter-set-with-file-param.json new file mode 100644 index 000000000..4a98695cd --- /dev/null +++ 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" + } + } + ] +} \ No newline at end of file