Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

Commit a804fb0

Browse files
rumplrumpl
authored
Merge pull request #570 from jcsirot/warn-on-too-many-params
Print a warning when undefined parameters are passed to docker app
2 parents 60a1f22 + 0a9fcc0 commit a804fb0

File tree

6 files changed

+96
-34
lines changed

6 files changed

+96
-34
lines changed

‎internal/commands/cnab.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
368368
if err := mergeBundleParameters(installation,
369369
withFileParameters(paramsOpts.parametersFiles),
370370
withCommandLineParameters(paramsOpts.overrides),
371+
withStrictMode(paramsOpts.strictMode),
371372
); err != nil {
372373
return nil, nil, nil, err
373374
}

‎internal/commands/install.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
117117
withCommandLineParameters(opts.overrides),
118118
withOrchestratorParameters(opts.orchestrator, opts.kubeNamespace),
119119
withSendRegistryAuth(opts.sendRegistryAuth),
120+
withStrictMode(opts.strictMode),
120121
); err != nil {
121122
return err
122123
}

‎internal/commands/parameters.go‎

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package commands
22

33
import (
44
"fmt"
5+
"io"
6+
"os"
57
"strings"
68

79
"github.com/deislabs/cnab-go/bundle"
@@ -13,80 +15,109 @@ import (
1315
"github.com/pkg/errors"
1416
)
1517

16-
type mergeBundleOpt func(bndl *bundle.Bundle, params map[string]string) error
18+
type mergeBundleConfig struct {
19+
bundle *bundle.Bundle
20+
params map[string]string
21+
strictMode bool
22+
stderr io.Writer
23+
}
24+
25+
type mergeBundleOpt func(c *mergeBundleConfig) error
1726

1827
func withFileParameters(parametersFiles []string) mergeBundleOpt {
19-
return func(bndl *bundle.Bundle, params map[string]string) error {
28+
return func(c *mergeBundleConfig) error {
2029
p, err := parameters.LoadFiles(parametersFiles)
2130
if err != nil {
2231
return err
2332
}
2433
for k, v := range p.Flatten() {
25-
params[k] = v
34+
c.params[k] = v
2635
}
2736
return nil
2837
}
2938
}
3039

3140
func withCommandLineParameters(overrides []string) mergeBundleOpt {
32-
return func(bndl *bundle.Bundle, params map[string]string) error {
41+
return func(c *mergeBundleConfig) error {
3342
d := cliopts.ConvertKVStringsToMap(overrides)
3443
for k, v := range d {
35-
params[k] = v
44+
c.params[k] = v
3645
}
3746
return nil
3847
}
3948
}
4049

4150
func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt {
42-
return func(bndl *bundle.Bundle, params map[string]string) error {
43-
if _, ok := bndl.Definitions[internal.ParameterShareRegistryCredsName]; ok {
51+
return func(c *mergeBundleConfig) error {
52+
if _, ok := c.bundle.Definitions[internal.ParameterShareRegistryCredsName]; ok {
4453
val := "false"
4554
if sendRegistryAuth {
4655
val = "true"
4756
}
48-
params[internal.ParameterShareRegistryCredsName] = val
57+
c.params[internal.ParameterShareRegistryCredsName] = val
4958
}
5059
return nil
5160
}
5261
}
5362

5463
func withOrchestratorParameters(orchestrator string, kubeNamespace string) mergeBundleOpt {
55-
return func(bndl *bundle.Bundle, params map[string]string) error {
56-
if _, ok := bndl.Definitions[internal.ParameterOrchestratorName]; ok {
57-
params[internal.ParameterOrchestratorName] = orchestrator
64+
return func(c *mergeBundleConfig) error {
65+
if _, ok := c.bundle.Definitions[internal.ParameterOrchestratorName]; ok {
66+
c.params[internal.ParameterOrchestratorName] = orchestrator
5867
}
59-
if _, ok := bndl.Definitions[internal.ParameterKubernetesNamespaceName]; ok {
60-
params[internal.ParameterKubernetesNamespaceName] = kubeNamespace
68+
if _, ok := c.bundle.Definitions[internal.ParameterKubernetesNamespaceName]; ok {
69+
c.params[internal.ParameterKubernetesNamespaceName] = kubeNamespace
6170
}
6271
return nil
6372
}
6473
}
6574

75+
func withErrorWriter(w io.Writer) mergeBundleOpt {
76+
return func(c *mergeBundleConfig) error {
77+
c.stderr = w
78+
return nil
79+
}
80+
}
81+
82+
func withStrictMode(strict bool) mergeBundleOpt {
83+
return func(c *mergeBundleConfig) error {
84+
c.strictMode = strict
85+
return nil
86+
}
87+
}
6688
func mergeBundleParameters(installation *store.Installation, ops ...mergeBundleOpt) error {
6789
bndl := installation.Bundle
6890
if installation.Parameters == nil {
6991
installation.Parameters = make(map[string]interface{})
7092
}
7193
userParams := map[string]string{}
94+
cfg := &mergeBundleConfig{
95+
bundle: bndl,
96+
params: userParams,
97+
stderr: os.Stderr,
98+
}
7299
for _, op := range ops {
73-
if err := op(bndl, userParams); err != nil {
100+
if err := op(cfg); err != nil {
74101
return err
75102
}
76103
}
77-
if err := matchAndMergeParametersDefinition(installation.Parameters, userParams, bndl.Definitions); err != nil {
104+
if err := matchAndMergeParametersDefinition(installation.Parameters, cfg.params, bndl.Definitions, cfg.strictMode, cfg.stderr); err != nil {
78105
return err
79106
}
80107
var err error
81108
installation.Parameters, err = bundle.ValuesOrDefaults(installation.Parameters, bndl)
82109
return err
83110
}
84111

85-
func matchAndMergeParametersDefinition(currentValues map[string]interface{}, parameterValues map[string]string, parameterDefinitions definition.Definitions) error {
112+
func matchAndMergeParametersDefinition(currentValues map[string]interface{}, parameterValues map[string]string, parameterDefinitions definition.Definitions, strictMode bool, stderr io.Writer) error {
86113
for k, v := range parameterValues {
87114
definition, ok := parameterDefinitions[k]
88115
if !ok {
89-
return fmt.Errorf("parameter %q is not defined in the bundle", k)
116+
if strictMode {
117+
return fmt.Errorf("parameter %q is not defined in the bundle", k)
118+
}
119+
fmt.Fprintf(stderr, "Warning: parameter %q is not defined in the bundle\n", k)
120+
continue
90121
}
91122
value, err := definition.ConvertValue(v)
92123
if err != nil {

‎internal/commands/parameters_test.go‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package commands
22

33
import (
4+
"bytes"
5+
"strings"
46
"testing"
57

68
"github.com/deislabs/cnab-go/bundle"
@@ -26,7 +28,11 @@ overridden: bar`))
2628
actual := map[string]string{
2729
"overridden": "foo",
2830
}
29-
err := withFileParameters([]string{tmpDir.Join("params.yaml")})(bundle, actual)
31+
err := withFileParameters([]string{tmpDir.Join("params.yaml")})(
32+
&mergeBundleConfig{
33+
bundle: bundle,
34+
params: actual,
35+
})
3036
assert.NilError(t, err)
3137
expected := map[string]string{
3238
"param1.param2": "value1",
@@ -42,7 +48,11 @@ func TestWithCommandLineParameters(t *testing.T) {
4248
"overridden": "foo",
4349
}
4450

45-
err := withCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(bundle, actual)
51+
err := withCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(
52+
&mergeBundleConfig{
53+
bundle: bundle,
54+
params: actual,
55+
})
4656
assert.NilError(t, err)
4757
expected := map[string]string{
4858
"param1.param2": "value1",
@@ -135,7 +145,10 @@ func TestWithOrchestratorParameters(t *testing.T) {
135145
for _, testCase := range testCases {
136146
t.Run(testCase.name, func(t *testing.T) {
137147
actual := map[string]string{}
138-
err := withOrchestratorParameters("kubernetes", "my-namespace")(testCase.bundle, actual)
148+
err := withOrchestratorParameters("kubernetes", "my-namespace")(&mergeBundleConfig{
149+
bundle: testCase.bundle,
150+
params: actual,
151+
})
139152
assert.NilError(t, err)
140153
assert.Assert(t, cmp.DeepEqual(actual, testCase.expected))
141154
})
@@ -144,12 +157,12 @@ func TestWithOrchestratorParameters(t *testing.T) {
144157

145158
func TestMergeBundleParameters(t *testing.T) {
146159
t.Run("Override Order", func(t *testing.T) {
147-
first := func(b *bundle.Bundle, params map[string]string) error {
148-
params["param"] = "first"
160+
first := func(c *mergeBundleConfig) error {
161+
c.params["param"] = "first"
149162
return nil
150163
}
151-
second := func(b *bundle.Bundle, params map[string]string) error {
152-
params["param"] = "second"
164+
second := func(c *mergeBundleConfig) error {
165+
c.params["param"] = "second"
153166
return nil
154167
}
155168
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
@@ -177,8 +190,8 @@ func TestMergeBundleParameters(t *testing.T) {
177190
})
178191

179192
t.Run("Converting values", func(t *testing.T) {
180-
withIntValue := func(b *bundle.Bundle, params map[string]string) error {
181-
params["param"] = "1"
193+
withIntValue := func(c *mergeBundleConfig) error {
194+
c.params["param"] = "1"
182195
return nil
183196
}
184197
bundle := prepareBundle(withParameter("param", "integer"))
@@ -202,20 +215,33 @@ func TestMergeBundleParameters(t *testing.T) {
202215
assert.Assert(t, cmp.DeepEqual(i.Parameters, expected))
203216
})
204217

205-
t.Run("Undefined parameter is rejected", func(t *testing.T) {
206-
withUndefined := func(b *bundle.Bundle, params map[string]string) error {
207-
params["param"] = "1"
218+
t.Run("Undefined parameter throws warning", func(t *testing.T) {
219+
withUndefined := func(c *mergeBundleConfig) error {
220+
c.params["param"] = "1"
221+
return nil
222+
}
223+
bundle := prepareBundle()
224+
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
225+
buf := new(bytes.Buffer)
226+
err := mergeBundleParameters(i, withUndefined, withErrorWriter(buf))
227+
assert.NilError(t, err)
228+
assert.Assert(t, strings.Contains(buf.String(), "is not defined in the bundle"))
229+
})
230+
231+
t.Run("Undefined parameter with strict mode is rejected", func(t *testing.T) {
232+
withUndefined := func(c *mergeBundleConfig) error {
233+
c.params["param"] = "1"
208234
return nil
209235
}
210236
bundle := prepareBundle()
211237
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
212-
err := mergeBundleParameters(i, withUndefined)
238+
err := mergeBundleParameters(i, withUndefined, withStrictMode(true))
213239
assert.ErrorContains(t, err, "is not defined in the bundle")
214240
})
215241

216242
t.Run("Invalid type is rejected", func(t *testing.T) {
217-
withIntValue := func(b *bundle.Bundle, params map[string]string) error {
218-
params["param"] = "foo"
243+
withIntValue := func(c *mergeBundleConfig) error {
244+
c.params["param"] = "foo"
219245
return nil
220246
}
221247
bundle := prepareBundle(withParameter("param", "integer"))
@@ -225,8 +251,8 @@ func TestMergeBundleParameters(t *testing.T) {
225251
})
226252

227253
t.Run("Invalid value is rejected", func(t *testing.T) {
228-
withInvalidValue := func(b *bundle.Bundle, params map[string]string) error {
229-
params["param"] = "invalid"
254+
withInvalidValue := func(c *mergeBundleConfig) error {
255+
c.params["param"] = "invalid"
230256
return nil
231257
}
232258
bundle := prepareBundle(withParameterAndValues("param", "string", []interface{}{"valid"}))

‎internal/commands/root.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,13 @@ func prepareBundleStore() (store.BundleStore, error) {
9494
type parametersOptions struct {
9595
parametersFiles []string
9696
overrides []string
97+
strictMode bool
9798
}
9899

99100
func (o *parametersOptions) addFlags(flags *pflag.FlagSet) {
100101
flags.StringArrayVar(&o.parametersFiles, "parameters-file", []string{}, "Override parameters file")
101102
flags.StringArrayVarP(&o.overrides, "set", "s", []string{}, "Override parameter value")
103+
flags.BoolVar(&o.strictMode, "strict", false, "Fail when a paramater is undefined instead of displaying a warning")
102104
}
103105

104106
type credentialOptions struct {

‎internal/commands/upgrade.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
6767
withFileParameters(opts.parametersFiles),
6868
withCommandLineParameters(opts.overrides),
6969
withSendRegistryAuth(opts.sendRegistryAuth),
70+
withStrictMode(opts.strictMode),
7071
); err != nil {
7172
return err
7273
}

0 commit comments

Comments
 (0)