fix: fix register cluster create panic (#5198)

Co-authored-by: 1aal <[email protected]>

deploy/risingwave-cluster/values.schema.json

@@ -31,21 +31,26 @@
                                     "type": "object",
                                     "properties": {
                                         "accessKey": {
-                                            "$ref": "#/definitions/nonEmptyString"
+                                            "$ref": "#/definitions/nonEmptyString",
+                                            "description": "Specify the S3 access key."
                                         },
                                         "secretAccessKey": {
-                                            "$ref": "#/definitions/nonEmptyString"
+                                            "$ref": "#/definitions/nonEmptyString",
+                                            "description": "Specify the S3 secret access key."
                                         }
                                     }
                                 },
                                 "bucket": {
-                                    "$ref": "#/definitions/nonEmptyString"
+                                    "$ref": "#/definitions/nonEmptyString",
+                                    "description": "Specify the S3 bucket."
                                 },
                                 "endpoint": {
-                                    "$ref": "#/definitions/nonEmptyString"
+                                    "$ref": "#/definitions/nonEmptyString",
+                                    "description": "Specify the S3 endpoint."
                                 },
                                 "region": {
-                                    "$ref": "#/definitions/nonEmptyString"
+                                    "$ref": "#/definitions/nonEmptyString",
+                                    "description": "Specify the S3 region."
                                 }
                             }
                         }

docs/user_docs/cli/kbcli_cluster_register.md

@@ -22,7 +22,7 @@ kbcli cluster register [NAME] --source [CHART-URL] [flags]
 
 ```
       --alias string    Set the cluster type alias
-      --auto-approve    Skip interactive approval when register an existed cluster type
+      --auto-approve    Skip interactive approval when registering an existed cluster type
   -h, --help            help for register
   -S, --source string   Specify the cluster type chart source, support a URL or a local file path
 ```

internal/cli/cluster/external_charts.go

@@ -27,12 +27,14 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/spf13/cobra"
 	"gopkg.in/yaml.v2"
 	"helm.sh/helm/v3/pkg/chart/loader"
 	"k8s.io/klog"
 
 	"github.com/apecloud/kubeblocks/internal/cli/types"
 	"github.com/apecloud/kubeblocks/internal/cli/util"
+	"github.com/apecloud/kubeblocks/internal/cli/util/flags"
 )
 
 // CliClusterChartConfig is $HOME/.kbcli/cluster_types by default
@@ -186,7 +188,16 @@ func (h *TypeInstance) PreCheck() error {
 	if err := chartInfo.buildClusterSchema(); err != nil {
 		return err
 	}
-	return chartInfo.buildClusterDef()
+	if err := chartInfo.buildClusterDef(); err != nil {
+		return err
+	}
+
+	// pre-check build sub-command flags
+	if err := flags.BuildFlagsBySchema(&cobra.Command{}, chartInfo.Schema); err != nil {
+		return err
+	}
+
+	return flags.BuildFlagsBySchema(&cobra.Command{}, chartInfo.SubSchema)
 }
 
 func (h *TypeInstance) loadChart() (io.ReadCloser, error) {

internal/cli/cmd/backuprepo/create.go

@@ -201,7 +201,7 @@ func (o *createOptions) parseProviderFlags(cmd *cobra.Command, args []string, f
 		if err = json.Unmarshal(schemaData, schema); err != nil {
 			return err
 		}
-		if err = flags.BuildFlagsBySchema(cmd, f, schema); err != nil {
+		if err = flags.BuildFlagsBySchema(cmd, schema); err != nil {
 			return err
 		}
 	}

internal/cli/cmd/cluster/config_observer.go

@@ -90,7 +90,7 @@ var (
 
 func (r *configObserverOptions) addCommonFlags(cmd *cobra.Command, f cmdutil.Factory) {
 	cmd.Flags().StringSliceVar(&r.configSpecs, "config-specs", nil, "Specify the name of the configuration template to describe. (e.g. for apecloud-mysql: --config-specs=mysql-3node-tpl)")
-	flags.AddComponentsFlag(f, cmd, true, &r.componentNames, "Specify the name of Component to describe (e.g. for apecloud-mysql: --component=mysql). If the cluster has only one component, unset the parameter.\"")
+	flags.AddComponentsFlag(f, cmd, &r.componentNames, "Specify the name of Component to describe (e.g. for apecloud-mysql: --component=mysql). If the cluster has only one component, unset the parameter.\"")
 }
 
 func (r *configObserverOptions) complete2(args []string) error {

internal/cli/cmd/cluster/config_ops.go

@@ -31,15 +31,14 @@ import (
 	"k8s.io/kubectl/pkg/util/templates"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/apecloud/kubeblocks/internal/configuration/core"
-
 	appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
 	"github.com/apecloud/kubeblocks/internal/cli/printer"
 	"github.com/apecloud/kubeblocks/internal/cli/types"
 	"github.com/apecloud/kubeblocks/internal/cli/util"
 	"github.com/apecloud/kubeblocks/internal/cli/util/flags"
 	"github.com/apecloud/kubeblocks/internal/cli/util/prompt"
 	cfgcm "github.com/apecloud/kubeblocks/internal/configuration/config_manager"
+	"github.com/apecloud/kubeblocks/internal/configuration/core"
 	"github.com/apecloud/kubeblocks/internal/controllerutil"
 )
 
@@ -241,7 +240,7 @@ func (o *configOpsOptions) buildReconfigureCommonFlags(cmd *cobra.Command, f cmd
 		"For available templates and configs, refer to: 'kbcli cluster describe-config'.")
 	cmd.Flags().StringVar(&o.CfgFile, "config-file", "", "Specify the name of the configuration file to be updated (e.g. for mysql: --config-file=my.cnf). "+
 		"For available templates and configs, refer to: 'kbcli cluster describe-config'.")
-	flags.AddComponentsFlag(f, cmd, false, &o.ComponentName, "Specify the name of Component to be updated. If the cluster has only one component, unset the parameter.")
+	flags.AddComponentFlag(f, cmd, &o.ComponentName, "Specify the name of Component to be updated. If the cluster has only one component, unset the parameter.")
 	cmd.Flags().BoolVar(&o.ForceRestart, "force-restart", false, "Boolean flag to restart component. Default with false.")
 	cmd.Flags().StringVar(&o.LocalFilePath, "local-file", "", "Specify the local configuration file to be updated.")
 	cmd.Flags().BoolVar(&o.replaceFile, "replace", false, "Boolean flag to enable replacing config file. Default with false.")

internal/cli/cmd/cluster/connect.go

@@ -113,7 +113,7 @@ func NewConnectCmd(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobr
 		},
 	}
 	cmd.Flags().StringVarP(&o.PodName, "instance", "i", "", "The instance name to connect.")
-	flags.AddComponentsFlag(f, cmd, false, &o.componentName, "The component to connect. If not specified, pick up the first one.")
+	flags.AddComponentFlag(f, cmd, &o.componentName, "The component to connect. If not specified, pick up the first one.")
 	cmd.Flags().BoolVar(&o.showExample, "show-example", false, "Show how to connect to cluster/instance from different clients.")
 	cmd.Flags().BoolVar(&o.showPassword, "show-password", false, "Show password in example.")
 

internal/cli/cmd/cluster/create_subcmds.go

@@ -222,9 +222,8 @@ func (o *createSubCmdsOptions) run() error {
 
 func (o *createSubCmdsOptions) validateVersion() error {
 	var err error
-
-	cv := o.values[cluster.VersionSchemaProp.String()].(string)
-	if cv != "" {
+	cv, ok := o.values[cluster.VersionSchemaProp.String()].(string)
+	if ok && cv != "" {
 		if err = cluster.ValidateClusterVersion(o.Dynamic, o.chartInfo.ClusterDef, cv); err != nil {
 			return fmt.Errorf("cluster version \"%s\" does not exist, run following command to get the available cluster versions\n\tkbcli cv list --cluster-definition=%s",
 				cv, o.chartInfo.ClusterDef)

internal/cli/cmd/cluster/create_util.go

@@ -54,12 +54,12 @@ func addCreateFlags(cmd *cobra.Command, f cmdutil.Factory, c *cluster.ChartInfo)
 	}
 
 	// add the flags for the cluster schema
-	if err := flags.BuildFlagsBySchema(cmd, f, c.Schema); err != nil {
+	if err := flags.BuildFlagsBySchema(cmd, c.Schema); err != nil {
 		return err
 	}
 
 	// add the flags for sub helm chart
-	if err := flags.BuildFlagsBySchema(cmd, f, c.SubSchema); err != nil {
+	if err := flags.BuildFlagsBySchema(cmd, c.SubSchema); err != nil {
 		return err
 	}
 
@@ -81,12 +81,20 @@ func getValuesFromFlags(fs *flag.FlagSet) map[string]interface{} {
 		}
 		var val interface{}
 		switch f.Value.Type() {
-		case "bool":
+		case flags.CobraBool:
 			val, _ = fs.GetBool(f.Name)
-		case "int":
+		case flags.CobraInt:
 			val, _ = fs.GetInt(f.Name)
-		case "float64":
+		case flags.CobraFloat64:
 			val, _ = fs.GetFloat64(f.Name)
+		case flags.CobraStringArray:
+			val, _ = fs.GetStringArray(f.Name)
+		case flags.CobraIntSlice:
+			val, _ = fs.GetIntSlice(f.Name)
+		case flags.CobraFloat64Slice:
+			val, _ = fs.GetFloat64Slice(f.Name)
+		case flags.CobraBoolSlice:
+			val, _ = fs.GetBoolSlice(f.Name)
 		default:
 			val, _ = fs.GetString(f.Name)
 		}
@@ -185,12 +193,25 @@ func buildHelmValues(c *cluster.ChartInfo, values map[string]interface{}) map[st
 	newValues := map[string]interface{}{
 		c.SubChartName: map[string]interface{}{},
 	}
+	var build func(key []string, v interface{}, values *map[string]interface{})
+	build = func(key []string, v interface{}, values *map[string]interface{}) {
+		if len(key) == 1 {
+			(*values)[key[0]] = v
+			return
+		}
+		if (*values)[key[0]] == nil {
+			(*values)[key[0]] = make(map[string]interface{})
+		}
+		nextMap := (*values)[key[0]].(map[string]interface{})
+		build(key[1:], v, &nextMap)
+	}
 
 	for k, v := range values {
 		if slices.Contains(subSchemaKeys, k) {
 			newValues[c.SubChartName].(map[string]interface{})[k] = v
 		} else {
-			newValues[k] = v
+			// todo: fix "."
+			build(strings.Split(k, "."), v, &newValues)
 		}
 	}
 

internal/cli/cmd/cluster/create_util_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/apecloud/kubeblocks/internal/cli/cluster"
 	"github.com/apecloud/kubeblocks/internal/cli/testing"
 	"github.com/apecloud/kubeblocks/internal/cli/types"
+	cmdflags "github.com/apecloud/kubeblocks/internal/cli/util/flags"
 )
 
 var _ = Describe("cluster create util", func() {
@@ -112,35 +113,62 @@ metadata:
 			value interface{}
 		}{
 			{
-				tpe:   "int",
+				tpe:   cmdflags.CobraInt,
 				name:  "int",
 				value: 1,
 			},
 			{
-				tpe:   "bool",
+				tpe:   cmdflags.CobraBool,
 				name:  "bool",
 				value: true,
 			},
 			{
-				tpe:   "float64",
+				tpe:   cmdflags.CobraFloat64,
 				name:  "float64",
 				value: 1.1,
 			},
 			{
-				tpe:   "string",
+				tpe:   cmdflags.CobraSting,
 				name:  "hello",
 				value: "Hello, KubeBlocks",
+			}, {
+				tpe:   cmdflags.CobraStringArray,
+				name:  "clusters",
+				value: []string{"mysql", "etcd"},
+			},
+			{
+				tpe:   cmdflags.CobraIntSlice,
+				name:  "ports",
+				value: []int{3303, 2379},
+			},
+			{
+				tpe:   cmdflags.CobraFloat64Slice,
+				name:  "resource",
+				value: []float64{0.5, 1.8},
+			},
+			{
+				tpe:   cmdflags.CobraBoolSlice,
+				name:  "enable",
+				value: []bool{false, true},
 			},
 		}
 
 		for _, f := range flags {
 			switch f.tpe {
-			case "int":
+			case cmdflags.CobraInt:
 				fs.Int(f.name, f.value.(int), f.name)
-			case "bool":
+			case cmdflags.CobraBool:
 				fs.Bool(f.name, f.value.(bool), f.name)
-			case "float64":
+			case cmdflags.CobraFloat64:
 				fs.Float64(f.name, f.value.(float64), f.name)
+			case cmdflags.CobraStringArray:
+				fs.StringArray(f.name, f.value.([]string), f.name)
+			case cmdflags.CobraIntSlice:
+				fs.IntSlice(f.name, f.value.([]int), f.name)
+			case cmdflags.CobraFloat64Slice:
+				fs.Float64Slice(f.name, f.value.([]float64), f.name)
+			case cmdflags.CobraBoolSlice:
+				fs.BoolSlice(f.name, f.value.([]bool), f.name)
 			default:
 				fs.String(f.name, f.value.(string), f.name)
 			}
@@ -170,5 +198,31 @@ metadata:
 		Expect(helmValues["version"]).Should(Equal("1.0.0"))
 		Expect(helmValues[c.SubChartName]).ShouldNot(BeNil())
 		Expect(helmValues[c.SubChartName].(map[string]interface{})["terminationPolicy"]).Should(Equal("Halt"))
+
+		By("build object helm values")
+		values = map[string]interface{}{
+			"etcd.cluster":   "etcd",
+			"etcd.namespace": "default",
+		}
+		helmValues = buildHelmValues(c, values)
+		Expect(helmValues).ShouldNot(BeNil())
+		Expect(helmValues["etcd"]).ShouldNot(BeNil())
+		Expect(helmValues["etcd"].(map[string]interface{})).Should(Equal(map[string]interface{}{
+			"cluster":   "etcd",
+			"namespace": "default",
+		}))
+
+		By("build array helm values")
+		values = map[string]interface{}{
+			"servers.name": []string{"mysql", "etcd"},
+			"servers.port": []int{3306, 2379},
+		}
+		helmValues = buildHelmValues(c, values)
+		Expect(helmValues).ShouldNot(BeNil())
+		Expect(helmValues["servers"]).ShouldNot(BeNil())
+		Expect(helmValues["servers"].(map[string]interface{})).Should(Equal(map[string]interface{}{
+			"name": []string{"mysql", "etcd"},
+			"port": []int{3306, 2379},
+		}))
 	})
 })

internal/cli/cmd/cluster/operations.go

@@ -141,7 +141,7 @@ func (o *OperationsOptions) addCommonFlags(cmd *cobra.Command, f cmdutil.Factory
 	cmd.Flags().StringVar(&o.DryRun, "dry-run", "none", `Must be "client", or "server". If with client strategy, only print the object that would be sent, and no data is actually sent. If with server strategy, submit the server-side request, but no data is persistent.`)
 	cmd.Flags().Lookup("dry-run").NoOptDefVal = "unchanged"
 	if o.HasComponentNamesFlag {
-		flags.AddComponentsFlag(f, cmd, true, &o.ComponentNames, "Component names to this operations")
+		flags.AddComponentsFlag(f, cmd, &o.ComponentNames, "Component names to this operations")
 	}
 }
 

internal/cli/cmd/cluster/register.go

@@ -26,6 +26,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 	"time"
 
 	"github.com/asaskevich/govalidator"
@@ -35,6 +36,7 @@ import (
 	"k8s.io/kubectl/pkg/util/templates"
 
 	"github.com/apecloud/kubeblocks/internal/cli/cluster"
+	"github.com/apecloud/kubeblocks/internal/cli/util"
 	"github.com/apecloud/kubeblocks/internal/cli/util/helm"
 	"github.com/apecloud/kubeblocks/internal/cli/util/prompt"
 )
@@ -81,11 +83,12 @@ func newRegisterCmd(f cmdutil.Factory, streams genericclioptions.IOStreams) *cob
 			o.clusterType = cluster.ClusterType(args[0])
 			cmdutil.CheckErr(o.validate())
 			cmdutil.CheckErr(o.run())
+			fmt.Fprint(streams.Out, buildRegisterSuccessExamples(o.clusterType))
 		},
 	}
 	cmd.Flags().StringVarP(&o.source, "source", "S", "", "Specify the cluster type chart source, support a URL or a local file path")
 	cmd.Flags().StringVar(&o.alias, "alias", "", "Set the cluster type alias")
-	cmd.Flags().BoolVar(&o.autoApprove, "auto-approve", false, "Skip interactive approval when register an existed cluster type")
+	cmd.Flags().BoolVar(&o.autoApprove, "auto-approve", false, "Skip interactive approval when registering an existed cluster type")
 
 	_ = cmd.MarkFlagRequired("source")
 
@@ -166,7 +169,7 @@ func (o *registerOption) run() error {
 		ChartName: o.cachedName,
 	}
 	if err := instance.PreCheck(); err != nil {
-		return fmt.Errorf("register helm chart pre-check failed: %s", err.Error())
+		return fmt.Errorf("the chart of %s pre-check unsuccssful: %s", o.clusterType, err.Error())
 	}
 
 	if o.replace {
@@ -210,3 +213,18 @@ func copyFile(src, dest string) error {
 	_, err = io.Copy(destFile, sourceFile)
 	return err
 }
+
+// buildCreateSubCmdsExamples builds the creation examples for the specified clusterType type.
+func buildRegisterSuccessExamples(t cluster.ClusterType) string {
+	exampleTpl := `
+
+cluster type "{{ .ClusterType }}" register successful.
+Use "kbcli cluster create {{ .ClusterType }}" to create a {{ .ClusterType }} cluster
+`
+
+	var builder strings.Builder
+	_ = util.PrintGoTemplate(&builder, exampleTpl, map[string]interface{}{
+		"ClusterType": t.String(),
+	})
+	return templates.Examples(builder.String())
+}