Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate process -v/--value in favor of -p/--param #12001

Merged
merged 1 commit into from
Nov 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions contrib/completions/bash/oc
Original file line number Diff line number Diff line change
Expand Up @@ -11493,16 +11493,16 @@ _oc_process()
local_nonpersistent_flags+=("--output=")
flags+=("--output-version=")
local_nonpersistent_flags+=("--output-version=")
flags+=("--param=")
two_word_flags+=("-p")
local_nonpersistent_flags+=("--param=")
flags+=("--parameters")
local_nonpersistent_flags+=("--parameters")
flags+=("--raw")
local_nonpersistent_flags+=("--raw")
flags+=("--template=")
two_word_flags+=("-t")
local_nonpersistent_flags+=("--template=")
flags+=("--value=")
two_word_flags+=("-v")
local_nonpersistent_flags+=("--value=")
flags+=("--as=")
flags+=("--certificate-authority=")
flags_with_completion+=("--certificate-authority")
Expand Down
6 changes: 3 additions & 3 deletions contrib/completions/bash/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -16139,16 +16139,16 @@ _openshift_cli_process()
local_nonpersistent_flags+=("--output=")
flags+=("--output-version=")
local_nonpersistent_flags+=("--output-version=")
flags+=("--param=")
two_word_flags+=("-p")
local_nonpersistent_flags+=("--param=")
flags+=("--parameters")
local_nonpersistent_flags+=("--parameters")
flags+=("--raw")
local_nonpersistent_flags+=("--raw")
flags+=("--template=")
two_word_flags+=("-t")
local_nonpersistent_flags+=("--template=")
flags+=("--value=")
two_word_flags+=("-v")
local_nonpersistent_flags+=("--value=")
flags+=("--as=")
flags+=("--certificate-authority=")
flags_with_completion+=("--certificate-authority")
Expand Down
6 changes: 3 additions & 3 deletions contrib/completions/zsh/oc
Original file line number Diff line number Diff line change
Expand Up @@ -11654,16 +11654,16 @@ _oc_process()
local_nonpersistent_flags+=("--output=")
flags+=("--output-version=")
local_nonpersistent_flags+=("--output-version=")
flags+=("--param=")
two_word_flags+=("-p")
local_nonpersistent_flags+=("--param=")
flags+=("--parameters")
local_nonpersistent_flags+=("--parameters")
flags+=("--raw")
local_nonpersistent_flags+=("--raw")
flags+=("--template=")
two_word_flags+=("-t")
local_nonpersistent_flags+=("--template=")
flags+=("--value=")
two_word_flags+=("-v")
local_nonpersistent_flags+=("--value=")
flags+=("--as=")
flags+=("--certificate-authority=")
flags_with_completion+=("--certificate-authority")
Expand Down
6 changes: 3 additions & 3 deletions contrib/completions/zsh/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -16300,16 +16300,16 @@ _openshift_cli_process()
local_nonpersistent_flags+=("--output=")
flags+=("--output-version=")
local_nonpersistent_flags+=("--output-version=")
flags+=("--param=")
two_word_flags+=("-p")
local_nonpersistent_flags+=("--param=")
flags+=("--parameters")
local_nonpersistent_flags+=("--parameters")
flags+=("--raw")
local_nonpersistent_flags+=("--raw")
flags+=("--template=")
two_word_flags+=("-t")
local_nonpersistent_flags+=("--template=")
flags+=("--value=")
two_word_flags+=("-v")
local_nonpersistent_flags+=("--value=")
flags+=("--as=")
flags+=("--certificate-authority=")
flags_with_completion+=("--certificate-authority")
Expand Down
8 changes: 4 additions & 4 deletions docs/man/man1/oc-process.1
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ The output of the process command is always a list of one or more resources. You
\fB\-\-output\-version\fP=""
Output the formatted object with the given version (default api\-version).

.PP
\fB\-p\fP, \fB\-\-param\fP=[]
Specify a key\-value pair (eg. \-p FOO=BAR) to set/override a parameter value in the template.

.PP
\fB\-\-parameters\fP=false
Do not process but only print available parameters
Expand All @@ -52,10 +56,6 @@ The output of the process command is always a list of one or more resources. You
Template string or path to template file to use when \-o=template or \-o=templatefile. The template format is golang templates [
\[la]http://golang.org/pkg/text/template/#pkg-overview\[ra]]

.PP
\fB\-v\fP, \fB\-\-value\fP=[]
Specify a key\-value pair (eg. \-v FOO=BAR) to set/override a parameter value in the template.


.SH OPTIONS INHERITED FROM PARENT COMMANDS
.PP
Expand Down
8 changes: 4 additions & 4 deletions docs/man/man1/openshift-cli-process.1
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ The output of the process command is always a list of one or more resources. You
\fB\-\-output\-version\fP=""
Output the formatted object with the given version (default api\-version).

.PP
\fB\-p\fP, \fB\-\-param\fP=[]
Specify a key\-value pair (eg. \-p FOO=BAR) to set/override a parameter value in the template.

.PP
\fB\-\-parameters\fP=false
Do not process but only print available parameters
Expand All @@ -52,10 +56,6 @@ The output of the process command is always a list of one or more resources. You
Template string or path to template file to use when \-o=template or \-o=templatefile. The template format is golang templates [
\[la]http://golang.org/pkg/text/template/#pkg-overview\[ra]]

.PP
\fB\-v\fP, \fB\-\-value\fP=[]
Specify a key\-value pair (eg. \-v FOO=BAR) to set/override a parameter value in the template.


.SH OPTIONS INHERITED FROM PARENT COMMANDS
.PP
Expand Down
22 changes: 12 additions & 10 deletions pkg/cmd/cli/cmd/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ func NewCmdProcess(fullName string, f *clientcmd.Factory, out, errout io.Writer)
}
cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to read a template")
cmd.MarkFlagFilename("filename", "yaml", "yml", "json")
cmd.Flags().StringArrayP("value", "v", nil, "Specify a key-value pair (eg. -v FOO=BAR) to set/override a parameter value in the template.")
params := cmd.Flags().StringArrayP("value", "v", nil, "Specify a key-value pair (eg. -v FOO=BAR) to set/override a parameter value in the template.")
cmd.Flags().MarkDeprecated("value", "Use -p, --param instead.")
cmd.Flags().MarkHidden("value")
cmd.Flags().StringArrayVarP(params, "param", "p", nil, "Specify a key-value pair (eg. -p FOO=BAR) to set/override a parameter value in the template.")
cmd.Flags().BoolP("parameters", "", false, "Do not process but only print available parameters")
cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this template")

Expand Down Expand Up @@ -102,12 +105,11 @@ func RunProcess(f *clientcmd.Factory, out, errout io.Writer, cmd *cobra.Command,
duplicatedKeys := sets.NewString()

var flagValues []string
if cmd.Flag("value").Changed {
flagValues = getFlagStringArray(cmd, "value")
if cmd.Flag("value").Changed || cmd.Flag("param").Changed {
flagValues = getFlagStringArray(cmd, "param")
cmdutil.WarnAboutCommaSeparation(errout, flagValues, "--param")
}

cmdutil.WarnAboutCommaSeparation(errout, flagValues, "--value")

for _, value := range flagValues {
key := strings.Split(value, "=")[0]
if keys.Has(key) {
Expand All @@ -134,7 +136,7 @@ func RunProcess(f *clientcmd.Factory, out, errout io.Writer, cmd *cobra.Command,
}

if kcmdutil.GetFlagBool(cmd, "parameters") {
for _, flag := range []string{"value", "labels", "output", "output-version", "raw", "template"} {
for _, flag := range []string{"value", "param", "labels", "output", "output-version", "raw", "template"} {
if f := cmd.Flags().Lookup(flag); f != nil && f.Changed {
return kcmdutil.UsageError(cmd, "The --parameters flag does not process the template, can't be used with --%v", flag)
}
Expand Down Expand Up @@ -238,10 +240,10 @@ func RunProcess(f *clientcmd.Factory, out, errout io.Writer, cmd *cobra.Command,
}
}

// Override the values for the current template parameters
// when user specify the --value
if cmd.Flag("value").Changed {
values := getFlagStringArray(cmd, "value")
// Override the parameter values for the current template parameters
// when user specifies --param
if cmd.Flag("value").Changed || cmd.Flag("param").Changed {
values := getFlagStringArray(cmd, "param")
if errs := injectUserVars(values, obj); errs != nil {
return kerrors.NewAggregate(errs)
}
Expand Down
29 changes: 25 additions & 4 deletions test/cmd/process.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ os::test::junit::declare_suite_start "cmd/process"
os::cmd::expect_failure_and_text 'oc process name1 name2' 'template name must be specified only once'
# fail to pass a filename or template by name
os::cmd::expect_failure_and_text 'oc process' 'Must pass a filename or name of stored template'
# can't ask for parameters and try process the template
# can't ask for parameters and try process the template (include tests for deprecated -v/--value)
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'
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'
os::cmd::expect_failure_and_text 'oc process template-name --parameters --param=someval' '\-\-parameters flag does not process the template, can.t be used with \-\-param'
os::cmd::expect_failure_and_text 'oc process template-name --parameters -p someval' '\-\-parameters flag does not process the template, can.t be used with \-\-param'
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'
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'
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'
Expand All @@ -29,11 +31,14 @@ os::cmd::expect_failure_and_text 'oc process template-name --parameters --raw' '
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'
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'

# providing a value more than once should fail
# providing a value more than once should fail (include tests for deprecated -v/--value)
os::cmd::expect_failure_and_text 'oc process template-name key=value key=value' 'provided more than once: key'
os::cmd::expect_failure_and_text 'oc process template-name --value=key=value --value=key=value' 'provided more than once: key'
os::cmd::expect_failure_and_text 'oc process template-name --param=key=value --param=key=value' 'provided more than once: key'
os::cmd::expect_failure_and_text 'oc process template-name key=value --value=key=value' 'provided more than once: key'
os::cmd::expect_failure_and_text 'oc process template-name key=value --param=key=value' 'provided more than once: key'
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'
os::cmd::expect_failure_and_text 'oc process template-name key=value other=foo --param=key=value --param=other=baz' 'provided more than once: key, other'

required_params="${OS_ROOT}/test/testdata/template_required_params.yaml"

Expand All @@ -43,29 +48,45 @@ os::cmd::expect_failure_and_text "oc process -f '${OS_ROOT}/test/testdata/basic-
# not providing required parameter should fail
os::cmd::expect_failure_and_text "oc process -f '${required_params}'" 'parameter required_param is required and must be specified'
# not providing an optional param is OK
os::cmd::expect_success "oc process -f '${required_params}' --value=required_param=someval | oc create -f -"
os::cmd::expect_success "oc process -f '${required_params}' --value=required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' -v required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' --param=required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' -p required_param=someval | oc create -f -"
# parameters with multiple equal signs are OK
os::cmd::expect_success "oc process -f '${required_params}' required_param=someval=moreval | oc create -f -"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm interested in this test both -v and -p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deads2k 27067a9 ptal

that works, thanks.

os::cmd::expect_success "oc process -f '${required_params}' -v required_param=someval=moreval2 | oc create -f -"
os::cmd::expect_success "oc process -f '${required_params}' -p required_param=someval=moreval3 | oc create -f -"
# we should have overwritten the template param
os::cmd::expect_success_and_text 'oc get user someval -o jsonpath={.Name}' 'someval'
os::cmd::expect_success_and_text 'oc get user someval=moreval -o jsonpath={.Name}' 'someval=moreval'
os::cmd::expect_success_and_text 'oc get user someval=moreval2 -o jsonpath={.Name}' 'someval=moreval2'
os::cmd::expect_success_and_text 'oc get user someval=moreval3 -o jsonpath={.Name}' 'someval=moreval3'
# providing a value not in the template should fail
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=other_param=otherval" 'unknown parameter name "other_param"'
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --param=required_param=someval --param=other_param=otherval" 'unknown parameter name "other_param"'
# failure on values fails the entire call
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=optional_param" 'invalid parameter assignment in'
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --param=required_param=someval --param=optional_param" 'invalid parameter assignment in'
# failure on labels fails the entire call
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --labels======" 'error parsing labels'
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --param=required_param=someval --labels======" 'error parsing labels'

# values are not split on commas, required parameter is not recognized
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=optional_param=a,required_param=b" 'parameter required_param is required and must be specified'
# warning is printed iff --value looks like two k-v pairs separated by comma
os::cmd::expect_failure_and_text "oc process -f '${required_params}' --param=optional_param=a,required_param=b" 'parameter required_param is required and must be specified'
# warning is printed iff --value/--param looks like two k-v pairs separated by comma
os::cmd::expect_success_and_text "oc process -f '${required_params}' --value=required_param=a,b=c,d" 'no longer accepts comma-separated list'
os::cmd::expect_success_and_not_text "oc process -f '${required_params}' --value=required_param=a_b_c_d" 'no longer accepts comma-separated list'
os::cmd::expect_success_and_not_text "oc process -f '${required_params}' --value=required_param=a,b,c,d" 'no longer accepts comma-separated list'
os::cmd::expect_success_and_text "oc process -f '${required_params}' --param=required_param=a,b=c,d" 'no longer accepts comma-separated list'
os::cmd::expect_success_and_not_text "oc process -f '${required_params}' --param=required_param=a_b_c_d" 'no longer accepts comma-separated list'
os::cmd::expect_success_and_not_text "oc process -f '${required_params}' --param=required_param=a,b,c,d" 'no longer accepts comma-separated list'
# warning is not printed for template values passed as positional arguments
os::cmd::expect_success_and_not_text "oc process -f '${required_params}' required_param=a,b=c,d" 'no longer accepts comma-separated list'

# set template parameter to contents of file
os::cmd::expect_success_and_text "oc process -f '${required_params}' --value=required_param='`cat ${OS_ROOT}/test/testdata/multiline.txt`'" 'also,with=commas'
os::cmd::expect_success_and_text "oc process -f '${required_params}' --param=required_param='`cat ${OS_ROOT}/test/testdata/multiline.txt`'" 'also,with=commas'


echo "process: ok"
Expand Down
8 changes: 4 additions & 4 deletions test/cmd/templates.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ os::test::junit::declare_suite_end

os::test::junit::declare_suite_start "cmd/templates/parameters"
# Individually specified parameter values are honored
os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser -v ADMIN_PASSWORD=mypassword' '"myuser"'
os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -v ADMIN_USERNAME=myuser -v ADMIN_PASSWORD=mypassword' '"mypassword"'
os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -p ADMIN_USERNAME=myuser -p ADMIN_PASSWORD=mypassword' '"myuser"'
os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json -p ADMIN_USERNAME=myuser -p ADMIN_PASSWORD=mypassword' '"mypassword"'
# Argument values are honored
os::cmd::expect_success_and_text 'oc process ADMIN_USERNAME=myuser ADMIN_PASSWORD=mypassword -f test/templates/testdata/guestbook.json' '"myuser"'
os::cmd::expect_success_and_text 'oc process -f test/templates/testdata/guestbook.json ADMIN_USERNAME=myuser ADMIN_PASSWORD=mypassword' '"mypassword"'
# Argument values with commas are honored
os::cmd::expect_success 'oc create -f examples/sample-app/application-template-stibuild.json'
os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample MYSQL_USER=myself MYSQL_PASSWORD=my,1%pa=s' '"myself"'
os::cmd::expect_success_and_text 'oc process MYSQL_USER=myself MYSQL_PASSWORD=my,1%pa=s ruby-helloworld-sample' '"my,1%pa=s"'
os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample -v MYSQL_USER=myself -v MYSQL_PASSWORD=my,1%pa=s' '"myself"'
os::cmd::expect_success_and_text 'oc process -v MYSQL_USER=myself -v MYSQL_PASSWORD=my,1%pa=s ruby-helloworld-sample' '"my,1%pa=s"'
os::cmd::expect_success_and_text 'oc process ruby-helloworld-sample -p MYSQL_USER=myself -p MYSQL_PASSWORD=my,1%pa=s' '"myself"'
os::cmd::expect_success_and_text 'oc process -p MYSQL_USER=myself -p MYSQL_PASSWORD=my,1%pa=s ruby-helloworld-sample' '"my,1%pa=s"'
os::cmd::expect_success 'oc delete template ruby-helloworld-sample'
echo "template+parameters: ok"
os::test::junit::declare_suite_end
Expand Down
Loading