Skip to content

Commit aee93e9

Browse files
vdicevdice
authored
fix(*): update parameter set logic to support file types (#1137)
* fix(*): update parameter set logic to support file types Signed-off-by: Vaughn Dice <vadice@microsoft.com> * move applyDefaultOptions to sharedOptions.Validate() Signed-off-by: Vaughn Dice <vadice@microsoft.com>
1 parent a355e30 commit aee93e9

15 files changed

+134
-27
lines changed

‎pkg/porter/cnab.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ func (o *sharedOptions) Validate(args []string, p *Porter) error {
9393
return err
9494
}
9595

96+
err = p.applyDefaultOptions(o)
97+
if err != nil {
98+
return err
99+
}
100+
96101
err = o.validateParams(p)
97102
if err != nil {
98103
return err
@@ -234,11 +239,13 @@ func (o *sharedOptions) parseParams() error {
234239

235240
// parseParamSets parses the variable assignments in ParameterSets.
236241
func (o *sharedOptions) parseParamSets(p *Porter) error {
237-
parsed, err := p.loadParameterSets(o.ParameterSets)
238-
if err != nil {
239-
return errors.Wrapf(err, "unable to process provided parameter sets: %v", o.ParameterSets)
242+
if len(o.ParameterSets) > 0 {
243+
parsed, err := p.loadParameterSets(o.ParameterSets)
244+
if err != nil {
245+
return errors.Wrapf(err, "unable to process provided parameter sets: %v", o.ParameterSets)
246+
}
247+
o.parsedParamSets = parsed
240248
}
241-
o.parsedParamSets = parsed
242249
return nil
243250
}
244251

‎pkg/porter/cnab_test.go

+31-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,10 @@ func TestParseParamSets_viaPathOrName(t *testing.T) {
104104
},
105105
}
106106

107-
err := opts.parseParamSets(p.Porter)
107+
err := opts.Validate([]string{}, p.Porter)
108+
assert.NoError(t, err)
109+
110+
err = opts.parseParamSets(p.Porter)
108111
assert.NoError(t, err)
109112

110113
wantParams := map[string]string{
@@ -114,6 +117,33 @@ func TestParseParamSets_viaPathOrName(t *testing.T) {
114117
assert.Equal(t, wantParams, opts.parsedParamSets, "resolved unexpected parameter values")
115118
}
116119

120+
func TestParseParamSets_FileType(t *testing.T) {
121+
p := NewTestPorter(t)
122+
123+
p.TestConfig.TestContext.AddTestFile("testdata/porter-with-file-param.yaml", "porter.yaml")
124+
p.TestConfig.TestContext.AddTestFile("testdata/paramset-with-file-param.json", "/paramset.json")
125+
126+
opts := sharedOptions{
127+
ParameterSets: []string{
128+
"/paramset.json",
129+
},
130+
bundleFileOptions: bundleFileOptions{
131+
File: "porter.yaml",
132+
},
133+
}
134+
135+
err := opts.Validate([]string{}, p.Porter)
136+
assert.NoError(t, err)
137+
138+
err = opts.parseParamSets(p.Porter)
139+
assert.NoError(t, err)
140+
141+
wantParams := map[string]string{
142+
"my-file-param": "/local/path/to/my-file-param",
143+
}
144+
assert.Equal(t, wantParams, opts.parsedParamSets, "resolved unexpected parameter values")
145+
}
146+
117147
func TestCombineParameters(t *testing.T) {
118148
t.Run("no override present, no parameter set present", func(t *testing.T) {
119149
opts := sharedOptions{}

‎pkg/porter/install.go

-5
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ func (p *Porter) InstallBundle(opts InstallOptions) error {
2222
return errors.Wrap(err, "unable to pull bundle before installation")
2323
}
2424

25-
err = p.applyDefaultOptions(&opts.sharedOptions)
26-
if err != nil {
27-
return err
28-
}
29-
3025
err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
3126
if err != nil {
3227
return err

‎pkg/porter/invoke.go

-5
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@ func (p *Porter) InvokeBundle(opts InvokeOptions) error {
3232
return errors.Wrap(err, "unable to pull bundle before invoking the custom action")
3333
}
3434

35-
err = p.applyDefaultOptions(&opts.sharedOptions)
36-
if err != nil {
37-
return err
38-
}
39-
4035
err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
4136
if err != nil {
4237
return err

‎pkg/porter/lifecycle_test.go

+6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"path/filepath"
55
"testing"
66

7+
"get.porter.sh/porter/pkg/config"
78
"github.com/stretchr/testify/assert"
89
"github.com/stretchr/testify/require"
910
)
@@ -84,6 +85,10 @@ func TestInstallFromTagIgnoresCurrentBundle(t *testing.T) {
8485

8586
func TestBundleLifecycleOpts_ToActionArgs(t *testing.T) {
8687
p := NewTestPorter(t)
88+
cxt := p.TestConfig.TestContext
89+
90+
// Add manifest which is used to parse parameter sets
91+
cxt.AddTestFile("testdata/porter.yaml", config.Name)
8792

8893
deps := &dependencyExecutioner{}
8994

@@ -118,6 +123,7 @@ func TestBundleLifecycleOpts_ToActionArgs(t *testing.T) {
118123
sharedOptions: sharedOptions{
119124
bundleFileOptions: bundleFileOptions{
120125
RelocationMapping: "relocation-mapping.json",
126+
File: config.Name,
121127
},
122128
Name: "MyClaim",
123129
Params: []string{

‎pkg/porter/options.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package porter
22

3-
import "get.porter.sh/porter/pkg/manifest"
3+
import (
4+
"get.porter.sh/porter/pkg/manifest"
5+
)
46

57
// applyDefaultOptions applies more advanced defaults to the options
68
// based on values that beyond just what was supplied by the user

‎pkg/porter/parameters.go

+17
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,23 @@ func (p *Porter) loadParameterSets(params []string) (valuesource.Set, error) {
307307
return nil, err
308308
}
309309

310+
// A parameter may correspond to a Porter-specific parameter type of 'file'
311+
// If so, add value (filepath) directly to map and remove from pset
312+
for _, paramDef := range p.Manifest.Parameters {
313+
if paramDef.Type == "file" {
314+
for i, param := range pset.Parameters {
315+
if param.Name == paramDef.Name {
316+
// Pass through value (filepath) directly to resolvedParameters
317+
resolvedParameters[param.Name] = param.Source.Value
318+
// Eliminate this param from pset to prevent its resolution by
319+
// the cnab-go library, which doesn't support this parameter type
320+
pset.Parameters[i] = pset.Parameters[len(pset.Parameters)-1]
321+
pset.Parameters = pset.Parameters[:len(pset.Parameters)-1]
322+
}
323+
}
324+
}
325+
}
326+
310327
rc, err := p.Parameters.ResolveAll(pset)
311328
if err != nil {
312329
return nil, err

‎pkg/porter/testdata/paramset-with-file-param.json

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"name": "mypset",
3+
"created": "1983-04-18T01:02:03.000000004Z",
4+
"modified": "1983-04-18T01:02:03.000000004Z",
5+
"parameters": [
6+
{
7+
"name": "my-file-param",
8+
"source": {
9+
"path": "/local/path/to/my-file-param"
10+
}
11+
}
12+
]
13+
}

‎pkg/porter/testdata/porter-with-file-param.yaml

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
name: HELLO_CUSTOM
2+
version: 0.1.0
3+
description: "A bundle with a custom action"
4+
tag: getporter/porter-hello:v0.1.0
5+
6+
parameters:
7+
- name: my-file-param
8+
description: "My file param"
9+
type: file
10+
path: /container/path/to/file
11+
12+
mixins:
13+
- exec
14+
15+
install:
16+
- exec:
17+
description: "cat file"
18+
command: bash
19+
flags:
20+
c: '"cat /container/path/to/file"'
21+
22+
uninstall:
23+
- exec:
24+
description: "cat file"
25+
command: bash
26+
flags:
27+
c: '"cat /container/path/to/file"'

‎pkg/porter/uninstall.go

-5
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ func (p *Porter) UninstallBundle(opts UninstallOptions) error {
2222
return errors.Wrap(err, "unable to pull bundle before uninstall")
2323
}
2424

25-
err = p.applyDefaultOptions(&opts.sharedOptions)
26-
if err != nil {
27-
return err
28-
}
29-
3025
err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
3126
if err != nil {
3227
return err

‎pkg/porter/upgrade.go

-5
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ func (p *Porter) UpgradeBundle(opts UpgradeOptions) error {
2222
return errors.Wrap(err, "unable to pull bundle before upgrade")
2323
}
2424

25-
err = p.applyDefaultOptions(&opts.sharedOptions)
26-
if err != nil {
27-
return err
28-
}
29-
3025
err = p.ensureLocalBundleIsUpToDate(opts.bundleFileOptions)
3126
if err != nil {
3227
return err

‎tests/install_test.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ func TestInstall_fileParam(t *testing.T) {
5151
p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/bundle-with-file-params.yaml"), "porter.yaml")
5252
p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/helpers.sh"), "helpers.sh")
5353
p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/myfile"), "./myfile")
54+
p.TestConfig.TestContext.AddTestFile(filepath.Join(p.TestDir, "testdata/myotherfile"), "./myotherfile")
5455

5556
installOpts := porter.InstallOptions{}
5657
installOpts.Params = []string{"myfile=./myfile"}
58+
installOpts.ParameterSets = []string{filepath.Join(p.TestDir, "testdata/parameter-set-with-file-param.json")}
5759

5860
err := installOpts.Validate([]string{}, p.Porter)
5961
require.NoError(t, err)
@@ -67,5 +69,6 @@ func TestInstall_fileParam(t *testing.T) {
6769

6870
claim, err := p.Claims.Read(p.Manifest.Name)
6971
require.NoError(t, err, "could not fetch claim")
70-
require.Equal(t, "Hello World!", claim.Outputs["myfile"], "expected output to match the decoded file contents")
72+
require.Equal(t, "Hello World!", claim.Outputs["myfile"], "expected output 'myfile' to match the decoded file contents")
73+
require.Equal(t, "Hello Other World!", claim.Outputs["myotherfile"], "expected output 'myotherfile' to match the decoded file contents")
7174
}

‎tests/testdata/bundle-with-file-params.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ parameters:
1010
- name: myfile
1111
type: file
1212
path: /root/myfile
13+
- name: myotherfile
14+
type: file
15+
path: /root/myotherfile
1316
# This is added to cover bug fix for https://github.com/deislabs/porter/issues/835
1417
# It will be inherently exercised in install_test.go via a default bundle installation
1518
- name: onlyUpgrade
@@ -22,6 +25,11 @@ outputs:
2225
type: string
2326
applyTo:
2427
- install
28+
- name: myotherfile
29+
type: file
30+
path: /root/myotherfile
31+
applyTo:
32+
- install
2533

2634
install:
2735
- exec:

‎tests/testdata/myotherfile

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Hello Other World!

‎tests/testdata/parameter-set-with-file-param.json

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"name": "mybun",
3+
"created": "2020-06-23T13:31:06.22727-06:00",
4+
"modified": "2020-06-23T13:31:44.692834-06:00",
5+
"parameters": [
6+
{
7+
"name": "myotherfile",
8+
"source": {
9+
"path": "./myotherfile"
10+
}
11+
}
12+
]
13+
}

0 commit comments

Comments
 (0)