Svcat bind params now supports --params-json #1889
Conversation
…g and orphan mitigation) (kubernetes-retired#1803)" This reverts commit 4b5d159.
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
@jberkhahn any thoughts? |
| @@ -97,7 +97,7 @@ func (c *provisonCmd) Validate(args []string) error { | |||
| if c.jsonParams != "" { | |||
| c.params, err = parameters.ParseVariableJSON(c.jsonParams) | |||
| if err != nil { | |||
| return fmt.Errorf("invalid --params value (%s)", err) | |||
| return fmt.Errorf("invalid --params-json value (%s)", err) | |||
| @@ -137,7 +137,7 @@ func (sdk *SDK) Unbind(ns, instanceName string) ([]v1beta1.ServiceBinding, error | |||
| } | |||
|
|
|||
| //Range over the deleted bindings to build a slice to return | |||
| deleted := []v1beta1.ServiceBinding{} | |||
| deleted := []v1beta1.ServiceBinding(nil) | |||
There was a problem hiding this comment.
I feel silly asking but what does this change do?
There was a problem hiding this comment.
[]v1beta1.ServiceBinding{} is creating an empty slice with the literal initialized.
[]v1beta1.ServiceBinding(nil) creates with the explicate nil initializer.
TBH: goland tells me using the (nil) way is better than {}
There was a problem hiding this comment.
I bow to the superior skills of Goland. Good to know what that means, thanks! 👍
| cmd.Flags().StringSliceVarP(&bindCmd.rawSecrets, "secret", "s", nil, | ||
| "Additional parameter, whose value is stored in a secret, to use when binding the instance, format: SECRET[KEY]") | ||
|
|
||
| cmd.Flags().StringVar(&bindCmd.jsonParams, "params-json", "", |
There was a problem hiding this comment.
I like it.
I would like a version of this that was --secret-params-json and let you put a json blob into a secret that was referenced.
There was a problem hiding this comment.
Would the result of that command create a new secret and then use the secret just like we currently do with --secret-name NAME ?
We would have to have some way to clean that secret up after the operation has finished. I think this sounds like a good idea and I am guessing most users would not understand that sending params via svcat exposes those to the other users of k8s.
update: Oh I needed to read better. We would be updating the contents of the secret with the blob, and we pass in the name of the secret.
Yeah! nice. made and issue: #1898
There was a problem hiding this comment.
Just to make sure I understand, the difference between --secret and this new flag would be that it would handle making the secret for you and setting the secret paramsfrom on the instance for you?
There was a problem hiding this comment.
@carolynvs I misunderstood the suggestion at first. I think @pmorie was suggesting the simpler case of supporting exactly what the current --secret does except provide the secret pairs as a json blob provided by --secret-json
There was a problem hiding this comment.
Oh! @pmorie can you weigh in when you get a chance? 😀
| cmd.Flags().StringSliceVarP(&bindCmd.rawSecrets, "secret", "s", nil, | ||
| "Additional parameter, whose value is stored in a secret, to use when binding the instance, format: SECRET[KEY]") | ||
|
|
||
| cmd.Flags().StringVar(&bindCmd.jsonParams, "params-json", "", | ||
| "Additional parameters to use when binding the instance, provided as a JSON object. Cannot be combined with --param") |
There was a problem hiding this comment.
We should have a message (fine to be in a follow up) warning people not to use this for secret information.
* master: Support provisioning by external id (kubernetes-retired#1756) Add namespaced ServiceBroker API (kubernetes-retired#1881) Enable verbose logging in svcat (kubernetes-retired#1822) Adding ServicePlan (namespaced plans) API (kubernetes-retired#1894) Svcat bind params now supports --params-json (kubernetes-retired#1889) words discussing the automated builds (kubernetes-retired#1885) v0.1.12 release changes don't overwrite generated files before verifying (kubernetes-retired#1891) Update registry code from broker to clusterservicebroker (kubernetes-retired#1880) Credentials remapping (kubernetes-retired#1868) Rename/move existing "ServicePlan" things (kubernetes-retired#1883) Start orphan mitigation after receiving a last operation status for async provisioning (kubernetes-retired#1886) Update of name in example ServiceClass (kubernetes-retired#1878) Cluster-id configmap (kubernetes-retired#1658) Add namespaced ServiceClass API (kubernetes-retired#1859) Fix common validations regression (kubernetes-retired#1882)
fixes #1871