Skip to content

Commit 6eaf7a2

Browse files
stevekuznetsovstevekuznetsov
committed
update failure conditions for oc process
Signed-off-by: Steve Kuznetsov <[email protected]>
1 parent 054059a commit 6eaf7a2

File tree

3 files changed

+172
-62
lines changed

3 files changed

+172
-62
lines changed

‎pkg/cmd/cli/cmd/process.go

+94-62
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
1616
"k8s.io/kubernetes/pkg/kubectl/resource"
1717
"k8s.io/kubernetes/pkg/runtime"
18+
kerrors "k8s.io/kubernetes/pkg/util/errors"
19+
"k8s.io/kubernetes/pkg/util/sets"
1820

1921
"github.com/openshift/origin/pkg/cmd/cli/describe"
2022
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
@@ -96,6 +98,34 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
9698
}
9799
}
98100

101+
keys := sets.NewString()
102+
duplicatedKeys := sets.NewString()
103+
104+
var flagValues []string
105+
if cmd.Flag("value").Changed {
106+
flagValues = kcmdutil.GetFlagStringSlice(cmd, "value")
107+
}
108+
109+
for _, value := range flagValues {
110+
key := strings.Split(value, "=")[0]
111+
if keys.Has(key) {
112+
duplicatedKeys.Insert(key)
113+
}
114+
keys.Insert(key)
115+
}
116+
117+
for _, value := range valueArgs {
118+
key := strings.Split(value, "=")[0]
119+
if keys.Has(key) {
120+
duplicatedKeys.Insert(key)
121+
}
122+
keys.Insert(key)
123+
}
124+
125+
if len(duplicatedKeys) != 0 {
126+
return kcmdutil.UsageError(cmd, fmt.Sprintf("The following values were provided more than once: %s", strings.Join(duplicatedKeys.List(), ", ")))
127+
}
128+
99129
filename := kcmdutil.GetFlagString(cmd, "filename")
100130
if len(templateName) == 0 && len(filename) == 0 {
101131
return kcmdutil.UsageError(cmd, "Must pass a filename or name of stored template")
@@ -171,73 +201,74 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
171201

172202
outputFormat := kcmdutil.GetFlagString(cmd, "output")
173203

174-
for i := range infos {
175-
obj, ok := infos[i].Object.(*templateapi.Template)
176-
if !ok {
177-
sourceName := filename
178-
if len(templateName) > 0 {
179-
sourceName = namespace + "/" + templateName
180-
}
181-
fmt.Fprintf(cmd.Out(), "unable to parse %q, not a valid Template but %s\n", sourceName, reflect.TypeOf(infos[i].Object))
182-
continue
183-
}
204+
if len(infos) > 1 {
205+
// in order to run validation on the input given to us by a user, we only support the processing
206+
// of one template in a list. For instance, we want to be able to fail when a user does not give
207+
// a parameter that the template wants or when they give a parameter the template doesn't need,
208+
// as this may indicate that they have mis-used `oc process`. This is much less complicated when
209+
// we process at most one template.
210+
fmt.Fprintf(out, "%d input templates found, but only the first will be processed", len(infos))
211+
}
184212

185-
// If 'parameters' flag is set it does not do processing but only print
186-
// the template parameters to console for inspection.
187-
// If multiple templates are passed, this will print combined output for all
188-
// templates.
189-
if kcmdutil.GetFlagBool(cmd, "parameters") {
190-
if len(infos) > 1 {
191-
fmt.Fprintf(out, "\n%s:\n", obj.Name)
192-
}
193-
if err := describe.PrintTemplateParameters(obj.Parameters, out); err != nil {
194-
fmt.Fprintf(cmd.Out(), "error printing parameters for %q: %v\n", obj.Name, err)
195-
}
196-
continue
213+
obj, ok := infos[0].Object.(*templateapi.Template)
214+
if !ok {
215+
sourceName := filename
216+
if len(templateName) > 0 {
217+
sourceName = namespace + "/" + templateName
197218
}
219+
return fmt.Errorf("unable to parse %q, not a valid Template but %s\n", sourceName, reflect.TypeOf(infos[0].Object))
220+
}
198221

199-
if label := kcmdutil.GetFlagString(cmd, "labels"); len(label) > 0 {
200-
lbl, err := kubectl.ParseLabels(label)
201-
if err != nil {
202-
fmt.Fprintf(cmd.Out(), "error parsing labels: %v\n", err)
203-
continue
204-
}
205-
if obj.ObjectLabels == nil {
206-
obj.ObjectLabels = make(map[string]string)
207-
}
208-
for key, value := range lbl {
209-
obj.ObjectLabels[key] = value
210-
}
211-
}
222+
// If 'parameters' flag is set it does not do processing but only print
223+
// the template parameters to console for inspection.
224+
if kcmdutil.GetFlagBool(cmd, "parameters") {
225+
return describe.PrintTemplateParameters(obj.Parameters, out)
226+
}
212227

213-
// Override the values for the current template parameters
214-
// when user specify the --value
215-
if cmd.Flag("value").Changed {
216-
values := kcmdutil.GetFlagStringSlice(cmd, "value")
217-
injectUserVars(values, cmd, obj)
228+
if label := kcmdutil.GetFlagString(cmd, "labels"); len(label) > 0 {
229+
lbl, err := kubectl.ParseLabels(label)
230+
if err != nil {
231+
return fmt.Errorf("error parsing labels: %v\n", err)
232+
}
233+
if obj.ObjectLabels == nil {
234+
obj.ObjectLabels = make(map[string]string)
235+
}
236+
for key, value := range lbl {
237+
obj.ObjectLabels[key] = value
218238
}
219-
injectUserVars(valueArgs, cmd, obj)
239+
}
220240

221-
resultObj, err := client.TemplateConfigs(namespace).Create(obj)
222-
if err != nil {
223-
fmt.Fprintf(cmd.Out(), "error processing the template %q: %v\n", obj.Name, err)
224-
continue
241+
// Override the values for the current template parameters
242+
// when user specify the --value
243+
if cmd.Flag("value").Changed {
244+
values := kcmdutil.GetFlagStringSlice(cmd, "value")
245+
if errs := injectUserVars(values, obj); errs != nil {
246+
return kerrors.NewAggregate(errs)
225247
}
248+
}
226249

227-
if outputFormat == "describe" {
228-
if s, err := (&describe.TemplateDescriber{
229-
MetadataAccessor: meta.NewAccessor(),
230-
ObjectTyper: kapi.Scheme,
231-
ObjectDescriber: nil,
232-
}).DescribeTemplate(resultObj); err != nil {
233-
fmt.Fprintf(cmd.Out(), "error describing %q: %v\n", obj.Name, err)
234-
} else {
235-
fmt.Fprintf(out, s)
236-
}
237-
continue
250+
if errs := injectUserVars(valueArgs, obj); errs != nil {
251+
return kerrors.NewAggregate(errs)
252+
}
253+
254+
resultObj, err := client.TemplateConfigs(namespace).Create(obj)
255+
if err != nil {
256+
return fmt.Errorf("error processing the template %q: %v\n", obj.Name, err)
257+
}
258+
259+
if outputFormat == "describe" {
260+
if s, err := (&describe.TemplateDescriber{
261+
MetadataAccessor: meta.NewAccessor(),
262+
ObjectTyper: kapi.Scheme,
263+
ObjectDescriber: nil,
264+
}).DescribeTemplate(resultObj); err != nil {
265+
return fmt.Errorf("error describing %q: %v\n", obj.Name, err)
266+
} else {
267+
_, err := fmt.Fprintf(out, s)
268+
return err
238269
}
239-
objects = append(objects, resultObj.Objects...)
240270
}
271+
objects = append(objects, resultObj.Objects...)
241272

242273
// Do not print the processed templates when asked to only show parameters or
243274
// describe.
@@ -271,19 +302,20 @@ func RunProcess(f *clientcmd.Factory, out io.Writer, cmd *cobra.Command, args []
271302
}
272303

273304
// injectUserVars injects user specified variables into the Template
274-
func injectUserVars(values []string, cmd *cobra.Command, t *templateapi.Template) {
305+
func injectUserVars(values []string, t *templateapi.Template) []error {
306+
var errors []error
275307
for _, keypair := range values {
276-
p := strings.SplitN(keypair, "=", 2)
308+
p := strings.Split(keypair, "=")
277309
if len(p) != 2 {
278-
fmt.Fprintf(cmd.Out(), "invalid parameter assignment in %q: %q\n", t.Name, keypair)
279-
continue
310+
errors = append(errors, fmt.Errorf("invalid parameter assignment in %q: %q\n", t.Name, keypair))
280311
}
281312
if v := template.GetParameterByName(t, p[0]); v != nil {
282313
v.Value = p[1]
283314
v.Generate = ""
284315
template.AddParameter(t, *v)
285316
} else {
286-
fmt.Fprintf(cmd.Out(), "unknown parameter name %q\n", p[0])
317+
errors = append(errors, fmt.Errorf("unknown parameter name %q\n", p[0]))
287318
}
288319
}
320+
return errors
289321
}

‎test/cmd/process.sh

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#!/bin/bash
2+
3+
set -o errexit
4+
set -o nounset
5+
set -o pipefail
6+
7+
OS_ROOT=$(dirname "${BASH_SOURCE}")/../..
8+
source "${OS_ROOT}/hack/common.sh"
9+
source "${OS_ROOT}/hack/util.sh"
10+
source "${OS_ROOT}/hack/cmd_util.sh"
11+
os::log::install_errexit
12+
13+
# Cleanup cluster resources created by this test
14+
(
15+
set +e
16+
oc delete all,templates --all
17+
oc delete user someval
18+
exit 0
19+
) &>/dev/null
20+
21+
os::test::junit::declare_suite_start "cmd/process"
22+
# This test validates oc process
23+
24+
# fail to process two templates by name
25+
os::cmd::expect_failure_and_text 'oc process name1 name2' 'template name must be specified only once'
26+
# fail to pass a filename or template by name
27+
os::cmd::expect_failure_and_text 'oc process' 'Must pass a filename or name of stored template'
28+
# can't ask for parameters and try process the template
29+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --value=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-value'
30+
os::cmd::expect_failure_and_text 'oc process template-name --parameters -v someval' '\-\-parameters flag does not process the template, can.t be used with \-\-value'
31+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --labels=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-labels'
32+
os::cmd::expect_failure_and_text 'oc process template-name --parameters -l someval' '\-\-parameters flag does not process the template, can.t be used with \-\-labels'
33+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --output=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-output'
34+
os::cmd::expect_failure_and_text 'oc process template-name --parameters -o someval' '\-\-parameters flag does not process the template, can.t be used with \-\-output'
35+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --output-version=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-output-version'
36+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --raw' '\-\-parameters flag does not process the template, can.t be used with \-\-raw'
37+
os::cmd::expect_failure_and_text 'oc process template-name --parameters --template=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-template'
38+
os::cmd::expect_failure_and_text 'oc process template-name --parameters -t someval' '\-\-parameters flag does not process the template, can.t be used with \-\-template'
39+
40+
# providing a value more than once should fail
41+
os::cmd::expect_failure_and_text 'oc process template-name key=value key=value' 'provided more than once: key'
42+
os::cmd::expect_failure_and_text 'oc process template-name --value=key=value --value=key=value' 'provided more than once: key'
43+
os::cmd::expect_failure_and_text 'oc process template-name key=value --value=key=value' 'provided more than once: key'
44+
os::cmd::expect_failure_and_text 'oc process template-name key=value other=foo --value=key=value --value=other=baz' 'provided more than once: key, other'
45+
46+
required_params="${OS_ROOT}/test/fixtures/template_required_params.yaml"
47+
48+
# providing something other than a template is not OK
49+
os::cmd::expect_failure_and_text "oc process -f '${OS_ROOT}/test/fixtures/basic-user.json'" 'not a valid Template but'
50+
51+
# not providing required parameter should fail
52+
os::cmd::expect_failure_and_text "oc process -f '${required_params}'" 'parameter required_param is required and must be specified'
53+
# not providiing an optional param is OK
54+
os::cmd::expect_success "oc process -f '${required_params}' --value=required_param=someval | oc create -f -"
55+
# we should have overwritten the template param
56+
os::cmd::expect_success_and_text 'oc get user someval -o jsonpath={.Name}' 'someval'
57+
# providing a value not in the template should fail
58+
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=other_param=otherval" 'unknown parameter name "other_param"'
59+
# failure on values fails the entire call
60+
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=optional_param=some=series=of=values=" 'invalid parameter assignment in'
61+
# failure on labels fails the entire call
62+
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --labels======" 'error parsing labels'
63+
64+
echo "process: ok"
65+
os::test::junit::declare_suite_end

‎test/fixtures/template_required_params.yaml

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
kind: Template
2+
apiVersion: v1
3+
metadata:
4+
name: template
5+
objects:
6+
- kind: User
7+
apiVersion: v1
8+
metadata:
9+
name: ${required_param}
10+
parameters:
11+
- name: optional_param
12+
- name: required_param
13+
required: true

0 commit comments

Comments
 (0)