Add enhanced parameter options to provision (openshift#1785)

* Modification to provision parameters This PR adds a --params flag to the provision operation, enabling the user to provision with parameters encoded as JSON rather than individual -p or --param options. This is useful when the parameters cannot be represented as simple key=value pairs. This PR additionally fixes two typos in the parameters.go file. Closes: openshift#1767 * Modification to provision parameters This PR adds a --params flag to the provision operation, enabling the user to provision with parameters encoded as JSON rather than individual -p or --param options. This is useful when the parameters cannot be represented as simple key=value pairs. This PR additionally fixes two typos in the parameters.go file. Closes: openshift#1767 * Updated per review
jboyd01 · Mar 6, 2018 · a777af5 · a777af5
1 parent fd1a0b9
commit a777af5
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 11 deletions.
diff --git a/cmd/svcat/README.md b/cmd/svcat/README.md
@@ -139,6 +139,29 @@ Additional parameters and secrets can be provided using the `--param` and `--sec
 --param p1=foo --param p2=bar --secret creds[db]
 ```
 
+You can also provide provision parameters in the form of a JSON string using the `--params-json` flag:
+
+```
+svcat provision secure-instance --class mysqldb --plan secureDB --params-json '{
+    "encrypt" : true,
+    "firewallRules" : [
+        {
+            "name": "AllowSome",
+            "startIPAddress": "75.70.113.50",
+            "endIPAddress" : "75.70.113.131"
+        },
+        {
+            "name": "AllowMore",
+            "startIPAddress": "13.54.0.0",
+            "endIPAddress" : "13.56.0.0"
+        }
+    ]
+}
+'
+```
+
+Note: You may not combine the `--params-json` flag with individual `--param` flags.
+
 ## View all instances of a service plan on the cluster
 
 ```console

diff --git a/cmd/svcat/instance/provision_cmd.go b/cmd/svcat/instance/provision_cmd.go
@@ -32,7 +32,8 @@ type provisonCmd struct {
 	className    string
 	planName     string
 	rawParams    []string
-	params       map[string]string
+	jsonParams   string
+	params       interface{}
 	rawSecrets   []string
 	secrets      map[string]string
 }
@@ -46,6 +47,22 @@ func NewProvisionCmd(cxt *command.Context) *cobra.Command {
 		Example: `
   svcat provision wordpress-mysql-instance --class mysqldb --plan free -p location=eastus -p sslEnforcement=disabled
   svcat provision wordpress-mysql-instance --class mysqldb --plan free -s mysecret[dbparams]
+  svcat provision secure-instance --class mysqldb --plan secureDB --params-json '{
+    "encrypt" : true,
+    "firewallRules" : [
+        {
+            "name": "AllowSome",
+            "startIPAddress": "75.70.113.50",
+            "endIPAddress" : "75.70.113.131"
+        },
+        {
+            "name": "AllowMore",
+            "startIPAddress": "13.54.0.0",
+            "endIPAddress" : "13.56.0.0"
+        }
+    ]
+}
+'
 `,
 		PreRunE: command.PreRunE(provisionCmd),
 		RunE:    command.RunE(provisionCmd),
@@ -59,9 +76,11 @@ func NewProvisionCmd(cxt *command.Context) *cobra.Command {
 		"The plan name (Required)")
 	cmd.MarkFlagRequired("plan")
 	cmd.Flags().StringSliceVarP(&provisionCmd.rawParams, "param", "p", nil,
-		"Additional parameter to use when provisioning the service, format: NAME=VALUE")
+		"Additional parameter to use when provisioning the service, format: NAME=VALUE. Cannot be combined with --params-json")
 	cmd.Flags().StringSliceVarP(&provisionCmd.rawSecrets, "secret", "s", nil,
 		"Additional parameter, whose value is stored in a secret, to use when provisioning the service, format: SECRET[KEY]")
+	cmd.Flags().StringVar(&provisionCmd.jsonParams, "params-json", "",
+		"Additional parameters to use when provisioning the service, provided as a JSON object. Cannot be combined with --param")
 	return cmd
 }
 
@@ -73,9 +92,20 @@ func (c *provisonCmd) Validate(args []string) error {
 
 	var err error
 
-	c.params, err = parameters.ParseVariableAssignments(c.rawParams)
-	if err != nil {
-		return fmt.Errorf("invalid --param value (%s)", err)
+	if c.jsonParams != "" && len(c.rawParams) > 0 {
+		return fmt.Errorf("--params-json cannot be used with --param")
+	}
+
+	if c.jsonParams != "" {
+		c.params, err = parameters.ParseVariableJSON(c.jsonParams)
+		if err != nil {
+			return fmt.Errorf("invalid --params value (%s)", err)
+		}
+	} else {
+		c.params, err = parameters.ParseVariableAssignments(c.rawParams)
+		if err != nil {
+			return fmt.Errorf("invalid --param value (%s)", err)
+		}
 	}
 
 	c.secrets, err = parameters.ParseKeyMaps(c.rawSecrets)

diff --git a/cmd/svcat/parameters/parameters.go b/cmd/svcat/parameters/parameters.go
@@ -17,29 +17,42 @@ limitations under the License.
 package parameters
 
 import (
+	"encoding/json"
 	"fmt"
 	"regexp"
 	"strings"
 )
 
 var keymapRegex = regexp.MustCompile(`^([^\[]+)\[(.+)\]\s*$`)
 
+// ParseVariableJSON converts a JSON object into a map of keys and values
+// Example:
+// `{ "location" : "east", "group" : "demo" }' becomes map[location:east group:demo]
+func ParseVariableJSON(params string) (map[string]interface{}, error) {
+	var p map[string]interface{}
+	err := json.Unmarshal([]byte(params), &p)
+	if err != nil {
+		return nil, fmt.Errorf("invalid parameters (%s)", params)
+	}
+	return p, nil
+}
+
 // ParseVariableAssignments converts a string array of variable assignments
 // into a map of keys and values
 // Example:
 // [a=b c=abc1232===] becomes map[a:b c:abc1232===]
 func ParseVariableAssignments(params []string) (map[string]string, error) {
 	variables := map[string]string{}
-
 	for _, p := range params {
+
 		parts := strings.SplitN(p, "=", 2)
 		if len(parts) < 2 {
 			return nil, fmt.Errorf("invalid parameter (%s), must be in name=value format", p)
 		}
 
 		variable := strings.TrimSpace(parts[0])
 		if variable == "" {
-			return nil, fmt.Errorf("invalid parameter (%s), variable name is requried", p)
+			return nil, fmt.Errorf("invalid parameter (%s), variable name is required", p)
 		}
 		value := strings.TrimSpace(parts[1])
 
@@ -64,7 +77,7 @@ func ParseKeyMaps(params []string) (map[string]string, error) {
 
 		mapName := strings.TrimSpace(parts[1])
 		if mapName == "" {
-			return nil, fmt.Errorf("invalid parameter (%s), map is requried", p)
+			return nil, fmt.Errorf("invalid parameter (%s), map is required", p)
 		}
 
 		key := strings.TrimSpace(parts[2])

diff --git a/cmd/svcat/svcat_test.go b/cmd/svcat/svcat_test.go
@@ -55,6 +55,9 @@ func TestCommandValidation(t *testing.T) {
 		{"unbind requires arg", "unbind", "instance or binding name is required"},
 		{"sync requires names", "sync broker", "name is required"},
 		{"deprovision requires name", "deprovision", "name is required"},
+		{"provision does not accept --param and --params-json",
+			`provision name --class class --plan plan --params-json '{}' --param k=v`,
+			"--params-json cannot be used with --param"},
 	}
 
 	for _, tc := range testcases {

diff --git a/cmd/svcat/testdata/plugin.yaml b/cmd/svcat/testdata/plugin.yaml
@@ -119,7 +119,11 @@ tree:
     desc: The class name (Required)
   - name: param
     shorthand: p
-    desc: 'Additional parameter to use when provisioning the service, format: NAME=VALUE'
+    desc: 'Additional parameter to use when provisioning the service, format: NAME=VALUE.
+      Cannot be combined with --params-json'
+  - name: params-json
+    desc: Additional parameters to use when provisioning the service, provided as
+      a JSON object. Cannot be combined with --param
   - name: plan
     desc: The plan name (Required)
   - name: secret

diff --git a/pkg/svcat/service-catalog/instance.go b/pkg/svcat/service-catalog/instance.go
@@ -143,7 +143,7 @@ func (sdk *SDK) InstanceToServiceClassAndPlan(instance *v1beta1.ServiceInstance,
 
 // Provision creates an instance of a service class and plan.
 func (sdk *SDK) Provision(namespace, instanceName, className, planName string,
-	params map[string]string, secrets map[string]string) (*v1beta1.ServiceInstance, error) {
+	params interface{}, secrets map[string]string) (*v1beta1.ServiceInstance, error) {
 
 	request := &v1beta1.ServiceInstance{
 		ObjectMeta: v1.ObjectMeta{

diff --git a/pkg/svcat/service-catalog/parameters.go b/pkg/svcat/service-catalog/parameters.go
@@ -26,7 +26,7 @@ import (
 
 // BuildParameters converts a map of variable assignments to a byte encoded json document,
 // which is what the ServiceCatalog API consumes.
-func BuildParameters(params map[string]string) *runtime.RawExtension {
+func BuildParameters(params interface{}) *runtime.RawExtension {
 	paramsJSON, err := json.Marshal(params)
 	if err != nil {
 		// This should never be hit because marshalling a map[string]string is pretty safe