Merge pull request openshift#763 from renan-campos/ms_edit

bugfixes for editing managed service

cmd/edit/service/cmd.go

@@ -17,11 +17,11 @@ limitations under the License.
 package service
 
 import (
-	"fmt"
 	"os"
 
 	"github.com/spf13/cobra"
 
+	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/openshift/rosa/pkg/arguments"
 	"github.com/openshift/rosa/pkg/ocm"
 	"github.com/openshift/rosa/pkg/rosa"
@@ -39,17 +39,6 @@ var Cmd = &cobra.Command{
 	Run:                run,
 	Hidden:             true,
 	DisableFlagParsing: true,
-	Args: func(cmd *cobra.Command, argv []string) error {
-		err := arguments.ParseUnknownFlags(cmd, argv)
-		if err != nil {
-			return err
-		}
-
-		if len(cmd.Flags().Args()) > 0 {
-			return fmt.Errorf("Unrecognized command line parameter")
-		}
-		return nil
-	},
 }
 
 func init() {
@@ -67,34 +56,53 @@ func run(cmd *cobra.Command, argv []string) {
 	r := rosa.NewRuntime().WithOCM()
 	defer r.Cleanup()
 
+	err := arguments.ParseKnownFlags(cmd, argv, false)
+	if err != nil {
+		r.Reporter.Errorf("Failed to parse flags: %v", err)
+		os.Exit(1)
+	}
+
 	if args.ID == "" {
 		r.Reporter.Errorf("Service id not specified.")
 		cmd.Help()
 		os.Exit(1)
 	}
 
-	// Try to find the cluster:
+	// Try to find the service:
 	r.Reporter.Debugf("Loading service %q", args.ID)
 	service, err := r.OCMClient.GetManagedService(ocm.DescribeManagedServiceArgs{ID: args.ID})
 	if err != nil {
 		r.Reporter.Errorf("Failed to get service %q: %v", args.ID, err)
 		os.Exit(1)
 	}
 
-	parameters := service.Parameters()
+	addOn, err := r.OCMClient.GetAddOn(service.Service())
+	if err != nil {
+		r.Reporter.Errorf("Failed to get add-on %q: %s", service.Service(), err)
+		os.Exit(1)
+	}
 
-	if len(parameters) == 0 {
-		r.Reporter.Errorf("Service %q has no parameters to edit", args.ID)
+	addonParameters := addOn.Parameters()
+	addonParameters.Each(func(param *cmv1.AddOnParameter) bool {
+		arguments.AddStringFlag(cmd, param.ID())
+		return true
+	})
+
+	err = arguments.ParseKnownFlags(cmd, argv, true)
+	if err != nil {
+		r.Reporter.Errorf("Failed to parse flags: %v", err)
 		os.Exit(1)
 	}
 
 	args.Parameters = map[string]string{}
-	for _, param := range parameters {
+	addonParameters.Each(func(param *cmv1.AddOnParameter) bool {
 		flag := cmd.Flags().Lookup(param.ID())
-		if flag != nil {
+		// Checking if the flag changed to ensure that the user set the value.
+		if flag != nil && flag.Changed {
 			args.Parameters[param.ID()] = flag.Value.String()
 		}
-	}
+		return true
+	})
 
 	r.Reporter.Debugf("Updating parameters for service %q", args.ID)
 	err = r.OCMClient.UpdateManagedService(args)

pkg/arguments/arguments.go

@@ -19,6 +19,7 @@ limitations under the License.
 package arguments
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -101,6 +102,94 @@ func ParseUnknownFlags(cmd *cobra.Command, argv []string) error {
 	return nil
 }
 
+// Parse known flags will take the command line arguments and map the ones that fit with known flags.
+func ParseKnownFlags(cmd *cobra.Command, argv []string, failOnUnknown bool) error {
+	flags := cmd.Flags()
+
+	var validArgs []string = []string{}
+	var upcomingValue bool
+	unknownFlags := ""
+
+	for _, arg := range argv {
+		switch {
+		// A long flag with a space separated value
+		case strings.HasPrefix(arg, "--") && !strings.Contains(arg, "="):
+			flagName := arg[2:]
+			// Skip EOF and known flags
+			if flag := flags.Lookup(flagName); flag != nil {
+				validArgs = append(validArgs, arg)
+				if flag.Value.Type() != "bool" {
+					upcomingValue = true
+				}
+			} else if failOnUnknown {
+				unknownFlags += fmt.Sprintf("%q, ", flagName)
+			}
+		// A long flag with a value after an equal sign
+		case strings.HasPrefix(arg, "--") && strings.Contains(arg, "="):
+			flagName := strings.SplitN(arg[2:], "=", 2)[0]
+			if flags.Lookup(flagName) != nil {
+				validArgs = append(validArgs, arg)
+			} else if failOnUnknown {
+				unknownFlags += fmt.Sprintf("%q, ", flagName)
+			}
+			upcomingValue = false
+		// A short flag with a space separated value
+		case strings.HasPrefix(arg, "-") && !strings.Contains(arg, "="):
+			flagName := arg[1:]
+			if flag := flags.Lookup(flagName); flag != nil {
+				validArgs = append(validArgs, arg)
+				if flag.Value.Type() != "bool" {
+					upcomingValue = true
+				}
+			} else if failOnUnknown {
+				unknownFlags += fmt.Sprintf("%q, ", flagName)
+			}
+		// A short flag with with a value after an equal sign
+		case strings.HasPrefix(arg, "-") && strings.Contains(arg, "="):
+			flagName := strings.SplitN(arg[1:], "=", 2)[0]
+			if flags.Lookup(flagName) != nil {
+				validArgs = append(validArgs, arg)
+			} else if failOnUnknown {
+				unknownFlags += fmt.Sprintf("%q, ", flagName)
+			}
+			upcomingValue = false
+		case upcomingValue:
+			validArgs = append(validArgs, arg)
+			upcomingValue = false
+		}
+	}
+
+	if failOnUnknown && unknownFlags != "" {
+		return fmt.Errorf("Unknown flags passed: %s", unknownFlags[:len(unknownFlags)-2])
+	}
+
+	err := flags.Parse(validArgs)
+	if err != nil {
+		return err
+	}
+
+	// If help is called, regardless of other flags, return we want help.
+	// Also say we need help if the command isn't runnable.
+	helpVal, err := cmd.Flags().GetBool("help")
+	if err != nil {
+		// should be impossible to get here as we always declare a help
+		// flag in InitDefaultHelpFlag()
+		cmd.Println("\"help\" flag declared as non-bool. Please correct your code")
+		return err
+	}
+	if helpVal {
+		return pflag.ErrHelp
+	}
+
+	return nil
+}
+
+func AddStringFlag(cmd *cobra.Command, flagName string) {
+	flags := cmd.Flags()
+	var pStrVal *string = new(string)
+	flags.StringVar(pStrVal, flagName, "", "")
+}
+
 // HasUnknownFlags returns whether the flag parser detected any unknown flags
 func HasUnknownFlags() bool {
 	return hasUnknownFlags