From ff802ad2c5b764d72022a1c198d92ea5b3af6904 Mon Sep 17 00:00:00 2001 From: wyike Date: Wed, 11 Jan 2023 11:45:41 +0800 Subject: [PATCH] Fix: more explicit error when addon package hasn't a metadata.yaml (#5298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * more explicit error when addon package hasn't a metadata.yaml Signed-off-by: 楚岳 fix checkdiff Signed-off-by: 楚岳 * fix commets Signed-off-by: 楚岳 * fix test Signed-off-by: 楚岳 Signed-off-by: 楚岳 --- pkg/addon/addon.go | 1 - pkg/addon/helper.go | 6 ++++++ pkg/addon/utils.go | 14 ++++++++++++++ pkg/addon/utils_test.go | 26 ++++++++++++++++++++++++++ references/cli/addon.go | 4 ++-- 5 files changed, 48 insertions(+), 3 deletions(-) diff --git a/pkg/addon/addon.go b/pkg/addon/addon.go index 85bd742af..2dc6d376d 100644 --- a/pkg/addon/addon.go +++ b/pkg/addon/addon.go @@ -911,7 +911,6 @@ func NewAddonInstaller(ctx context.Context, cli client.Client, discoveryClient * func (h *Installer) enableAddon(addon *InstallPackage) (string, error) { var err error h.addon = addon - if !h.skipVersionValidate { err = checkAddonVersionMeetRequired(h.ctx, addon.SystemRequirements, h.cli, h.dc) if err != nil { diff --git a/pkg/addon/helper.go b/pkg/addon/helper.go index b6a906f21..91576ea52 100644 --- a/pkg/addon/helper.go +++ b/pkg/addon/helper.go @@ -58,6 +58,9 @@ func EnableAddon(ctx context.Context, name string, version string, cli client.Cl if err != nil { return "", err } + if err := validateAddonPackage(pkg); err != nil { + return "", errors.Wrap(err, fmt.Sprintf("failed to enable addon: %s", name)) + } return h.enableAddon(pkg) } @@ -106,6 +109,9 @@ func EnableAddonByLocalDir(ctx context.Context, name string, dir string, cli cli if err != nil { return "", err } + if err := validateAddonPackage(pkg); err != nil { + return "", errors.Wrap(err, fmt.Sprintf("failed to enable addon by local dir: %s", dir)) + } h := NewAddonInstaller(ctx, cli, dc, applicator, config, &Registry{Name: LocalAddonRegistryName}, args, nil, nil, opts...) needEnableAddonNames, err := h.checkDependency(pkg) if err != nil { diff --git a/pkg/addon/utils.go b/pkg/addon/utils.go index 86dc6d813..2e772fec2 100644 --- a/pkg/addon/utils.go +++ b/pkg/addon/utils.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "strings" "github.com/pkg/errors" @@ -503,6 +504,19 @@ func checkBondComponentExist(u unstructured.Unstructured, app v1beta1.Applicatio return false } +func validateAddonPackage(addonPkg *InstallPackage) error { + if reflect.DeepEqual(addonPkg.Meta, Meta{}) { + return fmt.Errorf("the addon package doesn't have `metadata.yaml`") + } + if addonPkg.Name == "" { + return fmt.Errorf("`matadata.yaml` must define the name of addon") + } + if addonPkg.Version == "" { + return fmt.Errorf("`matadata.yaml` must define the version of addon") + } + return nil +} + // FilterDependencyRegistries will return all registries besides the target registry itself func FilterDependencyRegistries(i int, rs []Registry) []Registry { if i >= len(rs) { diff --git a/pkg/addon/utils_test.go b/pkg/addon/utils_test.go index 9e9a7d79f..dbcc5535d 100644 --- a/pkg/addon/utils_test.go +++ b/pkg/addon/utils_test.go @@ -17,9 +17,11 @@ limitations under the License. package addon import ( + "fmt" "net/http/httptest" "os" "path/filepath" + "reflect" "strings" "testing" @@ -402,6 +404,30 @@ func TestFilterDependencyRegistries(t *testing.T) { } } +func TestCheckAddonPackageValid(t *testing.T) { + testCases := []struct { + testCase Meta + err error + }{{ + testCase: Meta{}, + err: fmt.Errorf("the addon package doesn't have `metadata.yaml`"), + }, { + testCase: Meta{Version: "v1.4.0"}, + err: fmt.Errorf("`matadata.yaml` must define the name of addon"), + }, { + testCase: Meta{Name: "test-addon"}, + err: fmt.Errorf("`matadata.yaml` must define the version of addon"), + }, { + testCase: Meta{Name: "test-addon", Version: "1.4.5"}, + err: nil, + }, + } + for _, testCase := range testCases { + err := validateAddonPackage(&InstallPackage{Meta: testCase.testCase}) + assert.Equal(t, reflect.DeepEqual(err, testCase.err), true) + } +} + const ( compDefYaml = ` apiVersion: core.oam.dev/v1beta1 diff --git a/references/cli/addon.go b/references/cli/addon.go index b003242d3..214cc03b9 100644 --- a/references/cli/addon.go +++ b/references/cli/addon.go @@ -185,7 +185,7 @@ func NewAddonEnableCommand(c common.Args, ioStream cmdutil.IOStreams) *cobra.Com if !file.IsDir() { return fmt.Errorf("%s is not addon dir", addonOrDir) } - ioStream.Infof("enable addon by local dir: %s \n", addonOrDir) + ioStream.Infof(color.New(color.FgYellow).Sprintf("enabling addon by local dir: %s \n", addonOrDir)) // args[0] is a local path install with local dir, use base dir name as addonName abs, err := filepath.Abs(addonOrDir) if err != nil { @@ -314,7 +314,7 @@ non-empty new arg if !file.IsDir() { return fmt.Errorf("%s is not addon dir", addonOrDir) } - ioStream.Infof("enable addon by local dir: %s \n", addonOrDir) + ioStream.Infof(color.New(color.FgYellow).Sprintf("enabling addon by local dir: %s \n", addonOrDir)) // args[0] is a local path install with local dir abs, err := filepath.Abs(addonOrDir) if err != nil {