Feat: add options for addon parameter (#5166)

* Feat: add options for addon parameter

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

* Fix: wrongly report disbaled when addon is not existed

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

pkg/addon/helper.go

@@ -188,8 +188,8 @@ func GetAddonStatus(ctx context.Context, cli client.Client, name string) (Status
 	}
 }
 
-// FindWholeAddonPackagesFromRegistry find addons' WholeInstallPackage from registries, empty registryName indicates matching all
-func FindWholeAddonPackagesFromRegistry(ctx context.Context, k8sClient client.Client, addonNames []string, registryNames []string) ([]*WholeAddonPackage, error) {
+// FindAddonPackagesDetailFromRegistry find addons' WholeInstallPackage from registries, empty registryName indicates matching all
+func FindAddonPackagesDetailFromRegistry(ctx context.Context, k8sClient client.Client, addonNames []string, registryNames []string) ([]*WholeAddonPackage, error) {
 	var addons []*WholeAddonPackage
 	var registries []Registry
 

pkg/addon/helper_test.go

@@ -29,21 +29,21 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
+var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() {
 	Describe("when no registry is added, no matter what you do, it will just return error", func() {
 		Context("when empty addonNames and registryNames is supplied", func() {
 			It("should return error", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{})
 				Expect(err).To(HaveOccurred())
 			})
 			It("should return error", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, nil)
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, nil)
 				Expect(err).To(HaveOccurred())
 			})
 		})
 		Context("when non-empty addonNames and registryNames is supplied", func() {
 			It("should return error saying ErrRegistryNotExist", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"fluxcd"}, []string{"some-registry"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"fluxcd"}, []string{"some-registry"})
 				Expect(errors.Is(err, ErrRegistryNotExist)).To(BeTrue())
 			})
 		})
@@ -70,18 +70,18 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("when empty addonNames and registryNames is supplied", func() {
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{"KubeVela"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{"KubeVela"})
 				Expect(err).To(HaveOccurred())
 			})
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, []string{"KubeVela"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"KubeVela"})
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("one existing addon name provided", func() {
 			It("should return one valid result, matching all registries", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("velaux"))
@@ -89,7 +89,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 				Expect(res[0].APISchema).ToNot(BeNil())
 			})
 			It("should return one valid result, matching one registry", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux"}, []string{"KubeVela"})
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, []string{"KubeVela"})
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("velaux"))
@@ -100,7 +100,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("one non-existent addon name provided", func() {
 			It("should return error as ErrNotExist", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil)
 				Expect(errors.Is(err, ErrNotExist)).To(BeTrue())
 				Expect(res).To(BeNil())
 			})
@@ -108,7 +108,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("two existing addon names provided", func() {
 			It("should return two valid result", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux", "traefik"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "traefik"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(2))
 				Expect(res[0].Name).To(Equal("velaux"))
@@ -122,7 +122,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("one existing addon name and one non-existent addon name provided", func() {
 			It("should return only one valid result", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux", "non-existent-addon"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "non-existent-addon"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("velaux"))
@@ -151,25 +151,25 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("when empty addonNames and registryNames is supplied", func() {
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{})
 				Expect(err).To(HaveOccurred())
 			})
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, []string{"testreg"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"testreg"})
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("one existing addon name provided", func() {
 			It("should return one valid result, matching all registries", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("example"))
 				Expect(res[0].InstallPackage).ToNot(BeNil())
 			})
 			It("should return one valid result, matching one registry", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example"}, []string{"testreg"})
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example"}, []string{"testreg"})
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("example"))
@@ -179,7 +179,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("one non-existent addon name provided", func() {
 			It("should return error as ErrNotExist", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil)
 				Expect(errors.Is(err, ErrNotExist)).To(BeTrue())
 				Expect(res).To(BeNil())
 			})
@@ -187,7 +187,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() {
 
 		Context("one existing addon name and one non-existent addon name provided", func() {
 			It("should return only one valid result", func() {
-				res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example", "non-existent-addon"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example", "non-existent-addon"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
 				Expect(res[0].Name).To(Equal("example"))

references/cli/addon.go

@@ -654,6 +654,16 @@ func statusAddon(name string, ioStreams cmdutil.IOStreams, cmd *cobra.Command, c
 	return nil
 }
 
+func addonNotExist(err error) bool {
+	if errors.Is(err, pkgaddon.ErrNotExist) || errors.Is(err, pkgaddon.ErrRegistryNotExist) {
+		return true
+	}
+	if strings.Contains(err.Error(), "not found") {
+		return true
+	}
+	return false
+}
+
 // generateAddonInfo will get addon status, description, version, dependencies (and whether they are installed),
 // and parameters (and their current values).
 // The first return value is the formatted string for printing.
@@ -664,12 +674,21 @@ func generateAddonInfo(c client.Client, name string) (string, pkgaddon.Status, e
 	var installed bool
 	var addonPackage *pkgaddon.WholeAddonPackage
 
+	// Check current addon status
+	status, err := pkgaddon.GetAddonStatus(context.Background(), c, name)
+	if err != nil {
+		return res, status, err
+	}
+
 	// Get addon install package
-	if verboseStatus {
+	if verboseStatus || status.AddonPhase == statusDisabled {
 		// We need the metadata to get descriptions about parameters
-		addonPackages, err := pkgaddon.FindWholeAddonPackagesFromRegistry(context.Background(), c, []string{name}, nil)
-		// Not found error can be ignored, because the user can define their own addon. Others can't.
-		if err != nil && !errors.Is(err, pkgaddon.ErrNotExist) && !errors.Is(err, pkgaddon.ErrRegistryNotExist) {
+		addonPackages, err := pkgaddon.FindAddonPackagesDetailFromRegistry(context.Background(), c, []string{name}, nil)
+		// If the state of addon is not disabled, we don't check the error, because it could be installed from local.
+		if status.AddonPhase == statusDisabled && err != nil {
+			if addonNotExist(err) {
+				return "", pkgaddon.Status{}, fmt.Errorf("addon '%s' not found in cluster or any registry", name)
+			}
 			return "", pkgaddon.Status{}, err
 		}
 		if len(addonPackages) != 0 {
@@ -677,12 +696,6 @@ func generateAddonInfo(c client.Client, name string) (string, pkgaddon.Status, e
 		}
 	}
 
-	// Check current addon status
-	status, err := pkgaddon.GetAddonStatus(context.Background(), c, name)
-	if err != nil {
-		return res, status, err
-	}
-
 	switch status.AddonPhase {
 	case statusEnabled:
 		installed = true
@@ -785,12 +798,23 @@ func generateParameterString(status pkgaddon.Status, addonPackage *pkgaddon.Whol
 	if addonPackage.APISchema == nil {
 		return ret
 	}
-
 	ret = printSchema(addonPackage.APISchema, status.Parameters, 0)
 
 	return ret
 }
 
+func convertInterface2StringList(l []interface{}) []string {
+	var strl []string
+	for _, s := range l {
+		str, ok := s.(string)
+		if !ok {
+			continue
+		}
+		strl = append(strl, str)
+	}
+	return strl
+}
+
 // printSchema prints the parameters in an addon recursively to a string
 // Deeper the parameter is nested, more the indentations.
 func printSchema(ref *openapi3.Schema, currentParams map[string]interface{}, indent int) string {
@@ -853,19 +877,25 @@ func printSchema(ref *openapi3.Schema, currentParams map[string]interface{}, ind
 		// Show current value
 		if currentValue != "" {
 			ret += addedIndent
-			ret += "\tcurrent: " + color.New(color.FgGreen).Sprintf("%s\n", currentValue)
-		}
-		// Show default value
-		if defaultValue != "" {
-			ret += addedIndent
-			ret += "\tdefault: " + fmt.Sprintf("%#v\n", defaultValue)
+			ret += "\tcurrent value: " + color.New(color.FgGreen).Sprintf("%s\n", currentValue)
 		}
+
 		// Show required or not
 		if required {
 			ret += addedIndent
 			ret += "\trequired: "
 			ret += color.GreenString("✔\n")
 		}
+		// Show Enum options
+		if len(propValue.Value.Enum) > 0 {
+			ret += addedIndent
+			ret += "\toptions: \"" + strings.Join(convertInterface2StringList(propValue.Value.Enum), "\", \"") + "\"\n"
+		}
+		// Show default value
+		if defaultValue != "" && currentValue == "" {
+			ret += addedIndent
+			ret += "\tdefault: " + fmt.Sprintf("%#v\n", defaultValue)
+		}
 
 		// Object type param, we will get inside the object.
 		// To show what's inside nested objects.

references/cli/addon_suite_test.go

@@ -269,7 +269,7 @@ var _ = Describe("Addon status or info", func() {
 			BeforeEach(func() {
 				// Delete KubeVela registry
 				ds := pkgaddon.NewRegistryDataStore(k8sClient)
-				Expect(ds.DeleteRegistry(context.Background(), "KubeVela")).To(Succeed())
+				Expect(ds.DeleteRegistry(context.Background(), "KubeVela")).Should(SatisfyAny(Succeed(), util.NotFoundMatcher{}))
 				// Install fluxcd locally
 				Expect(k8sClient.Create(context.Background(), &fluxcd)).Should(SatisfyAny(BeNil(), util.AlreadyExistMatcher{}))
 			})
@@ -287,6 +287,7 @@ var _ = Describe("Addon status or info", func() {
 					if err != nil {
 						return err
 					}
+					fmt.Println(addonName, res, err)
 					// Should include enabled status, like:
 					// fluxcd: enabled (1.1.0)
 					if !strings.Contains(res,
@@ -349,6 +350,11 @@ var _ = Describe("Addon status or info", func() {
 						"KubeVela",
 				))
 			})
+			It("should report addon not exist in any registry name", func() {
+				addonName := "not-exist"
+				_, _, err := generateAddonInfo(k8sClient, addonName)
+				Expect(err.Error()).Should(BeEquivalentTo("addon 'not-exist' not found in cluster or any registry"))
+			})
 		})
 	})
 })

references/cli/addon_test.go

@@ -19,7 +19,6 @@ package cli
 import (
 	"fmt"
 	"os"
-	"strings"
 	"testing"
 
 	. "github.com/onsi/ginkgo"
@@ -27,7 +26,7 @@ import (
 
 	"github.com/fatih/color"
 	"github.com/getkin/kin-openapi/openapi3"
-	"gotest.tools/assert"
+	"github.com/stretchr/testify/assert"
 
 	pkgaddon "github.com/oam-dev/kubevela/pkg/addon"
 	"github.com/oam-dev/kubevela/pkg/utils/common"
@@ -91,8 +90,8 @@ func TestParseMap(t *testing.T) {
 	for _, s := range testcase {
 		r, err := parseAddonArgsToMap(s.args)
 		if s.nilError {
-			assert.NilError(t, err)
-			assert.DeepEqual(t, s.res, r)
+			assert.NoError(t, err)
+			assert.Equal(t, s.res, r)
 		} else {
 			assert.Error(t, err, fmt.Sprintf("%v should be error case", s.args))
 		}
@@ -208,7 +207,7 @@ func TestTransCluster(t *testing.T) {
 		},
 	}
 	for _, s := range testcase {
-		assert.DeepEqual(t, transClusters(s.str), s.res)
+		assert.Equal(t, transClusters(s.str), s.res)
 	}
 }
 
@@ -341,7 +340,7 @@ func TestPackageValidAddon(t *testing.T) {
 	cmd := NewAddonPackageCommand(commandArgs)
 	cmd.SetArgs([]string{"./test-data/addon/sample"})
 	err := cmd.Execute()
-	assert.NilError(t, err)
+	assert.NoError(t, err)
 	defer func() {
 		_ = os.RemoveAll("sample-1.0.1.tgz")
 	}()
@@ -380,13 +379,13 @@ func TestGenerateParameterString(t *testing.T) {
 						"dbURL": &openapi3.SchemaRef{
 							Value: &openapi3.Schema{
 								Description: "Specify the MongoDB URL. it only enabled where DB type is MongoDB.",
-								Default:     nil,
+								Default:     "abc.com",
 							},
 						},
 						"dbType": &openapi3.SchemaRef{
 							Value: &openapi3.Schema{
 								Description: "Specify the database type, current support KubeAPI(default) and MongoDB.",
-								Default:     "kubeapi",
+								Enum:        []interface{}{"kubeapi", "mongodb"},
 							},
 						},
 					},
@@ -397,18 +396,19 @@ func TestGenerateParameterString(t *testing.T) {
 				color.New(color.FgCyan).Sprintf("-> ") +
 					color.New(color.Bold).Sprint("dbType") + ": " +
 					"Specify the database type, current support KubeAPI(default) and MongoDB.\n" +
-					"\tcurrent: " + color.New(color.FgGreen).Sprint("\"kubeapi\"\n") +
-					"\tdefault: " + "\"kubeapi\"\n" +
-					"\trequired: " + color.GreenString("✔\n"),
+					"\tcurrent value: " + color.New(color.FgGreen).Sprint("\"kubeapi\"\n") +
+					"\trequired: " + color.GreenString("✔\n") +
+					"\toptions: \"kubeapi\", \"mongodb\"\n",
 				// dbURL
 				color.New(color.FgCyan).Sprintf("-> ") +
 					color.New(color.Bold).Sprint("dbURL") + ": " +
-					"Specify the MongoDB URL. it only enabled where DB type is MongoDB.",
+					"Specify the MongoDB URL. it only enabled where DB type is MongoDB.\n" +
+					"\tdefault: " + "\"abc.com\"\n",
 				// database
 				color.New(color.FgCyan).Sprintf("-> ") +
 					color.New(color.Bold).Sprint("database") + ": " +
 					"Specify the database name, for the kubeapi db type, it represents namespace.\n" +
-					"\tcurrent: " + color.New(color.FgGreen).Sprint("\"kubevela\""),
+					"\tcurrent value: " + color.New(color.FgGreen).Sprint("\"kubevela\""),
 			},
 		},
 	}
@@ -416,7 +416,7 @@ func TestGenerateParameterString(t *testing.T) {
 	for _, s := range testcase {
 		res := generateParameterString(s.status, s.addonPackage)
 		for _, o := range s.outputs {
-			assert.Check(t, strings.Contains(res, o))
+			assert.Contains(t, res, o)
 		}
 
 	}
@@ -434,12 +434,12 @@ func TestNewAddonCreateCommand(t *testing.T) {
 
 	cmd.SetArgs([]string{"test-addon", "--chart", "a", "--helm-repo", "https://some.com", "--chart-version", "c"})
 	err = cmd.Execute()
-	assert.NilError(t, err)
+	assert.NoError(t, err)
 	_ = os.RemoveAll("test-addon")
 
 	cmd.SetArgs([]string{"test-addon"})
 	err = cmd.Execute()
-	assert.NilError(t, err)
+	assert.NoError(t, err)
 	_ = os.RemoveAll("test-addon")
 
 }