From ccd72ff57c044f8afa181db75fed3a0bdb996262 Mon Sep 17 00:00:00 2001 From: Zethan Date: Sun, 6 Mar 2022 00:48:06 +0100 Subject: [PATCH] Fix: plugin list throwing Nil Pointer exception when plugin with empty .yaml file was loaded Signed-off-by: Zethan --- pkg/plugin/plugin.go | 20 +++++++++- pkg/plugin/plugin_test.go | 37 +++++++++++++++++++ .../plugdir/bad/empty-config/plugin.yaml | 0 .../bad/skip-some/downloader/plugin.yaml | 11 ++++++ .../plugdir/bad/skip-some/echo/plugin.yaml | 8 ++++ .../bad/skip-some/empty-config/plugin.yaml | 0 .../plugdir/bad/skip-some/hello/hello.sh | 9 +++++ .../plugdir/bad/skip-some/hello/plugin.yaml | 9 +++++ .../testdata/plugdir/bad/skip/plugin.yaml | 0 9 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 pkg/plugin/testdata/plugdir/bad/empty-config/plugin.yaml create mode 100644 pkg/plugin/testdata/plugdir/bad/skip-some/downloader/plugin.yaml create mode 100644 pkg/plugin/testdata/plugdir/bad/skip-some/echo/plugin.yaml create mode 100644 pkg/plugin/testdata/plugdir/bad/skip-some/empty-config/plugin.yaml create mode 100644 pkg/plugin/testdata/plugdir/bad/skip-some/hello/hello.sh create mode 100644 pkg/plugin/testdata/plugdir/bad/skip-some/hello/plugin.yaml create mode 100644 pkg/plugin/testdata/plugdir/bad/skip/plugin.yaml diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 1399b7116..32fddfbd9 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -172,7 +172,13 @@ func (p *Plugin) PrepareCommand(extraArgs []string) (string, []string, error) { var validPluginName = regexp.MustCompile("^[A-Za-z0-9_-]+$") // validatePluginData validates a plugin's YAML data. +// IF the plugin metadata is non-existent(the YAML data does not +// exist -> empty plugin YAML file), the plugin cannot be loaded +// because of missing metadata. func validatePluginData(plug *Plugin, filepath string) error { + if plug.Metadata == nil { + return fmt.Errorf("plugin metadata is missing at %q", filepath) + } if !validPluginName.MatchString(plug.Metadata.Name) { return fmt.Errorf("invalid plugin name at %q", filepath) } @@ -225,7 +231,14 @@ func LoadDir(dirname string) (*Plugin, error) { if err := yaml.UnmarshalStrict(data, &plug.Metadata); err != nil { return nil, errors.Wrapf(err, "failed to load plugin at %q", pluginfile) } - return plug, validatePluginData(plug, pluginfile) + if plug.Metadata == nil { + return plug, errors.New(fmt.Sprintf("configuration data missing at %q", pluginfile)) + } + validationError := validatePluginData(plug, pluginfile) + if validationError != nil { + plug = nil + } + return plug, validationError } // LoadAll loads all plugins found beneath the base directory. @@ -247,7 +260,10 @@ func LoadAll(basedir string) ([]*Plugin, error) { for _, yaml := range matches { dir := filepath.Dir(yaml) p, err := LoadDir(dir) - if err != nil { + if p != nil && err != nil { + // Plugin was found but could not be loaded due to missing configuration + continue + } else if err != nil { return plugins, err } plugins = append(plugins, p) diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 3b44a6eb5..16f01d898 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -212,6 +212,13 @@ func TestLoadDirDuplicateEntries(t *testing.T) { } } +func TestLoadDirNoMetadata(t *testing.T) { + dirname := "testdata/plugdir/bad/empty-config" + if plug, err := LoadDir(dirname); err == nil || plug == nil { + t.Errorf("successfully loaded empty plugin") + } +} + func TestDownloader(t *testing.T) { dirname := "testdata/plugdir/good/downloader" plug, err := LoadDir(dirname) @@ -272,6 +279,36 @@ func TestLoadAll(t *testing.T) { } } +func TestLoadAllAndSkipEmptyOnes(t *testing.T) { + + // Verify that empty dir loads: + if plugs, err := LoadAll("testdata"); err != nil { + t.Fatalf("error loading dir with no plugins: %s", err) + } else if len(plugs) > 0 { + t.Fatalf("expected empty dir to have 0 plugins") + } + + basedir := "testdata/plugdir/bad/skip-some" + plugs, err := LoadAll(basedir) + if err != nil { + t.Fatalf("Could not load %q: %s", basedir, err) + } + + if l := len(plugs); l != 3 { + t.Fatalf("expected 3 plugins, found %d", l) + } + + if plugs[0].Metadata.Name != "downloader" { + t.Errorf("Expected first plugin to be echo, got %q", plugs[0].Metadata.Name) + } + if plugs[1].Metadata.Name != "echo" { + t.Errorf("Expected first plugin to be echo, got %q", plugs[0].Metadata.Name) + } + if plugs[2].Metadata.Name != "hello" { + t.Errorf("Expected second plugin to be hello, got %q", plugs[1].Metadata.Name) + } +} + func TestFindPlugins(t *testing.T) { cases := []struct { name string diff --git a/pkg/plugin/testdata/plugdir/bad/empty-config/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/empty-config/plugin.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/plugin/testdata/plugdir/bad/skip-some/downloader/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/skip-some/downloader/plugin.yaml new file mode 100644 index 000000000..c0b90379b --- /dev/null +++ b/pkg/plugin/testdata/plugdir/bad/skip-some/downloader/plugin.yaml @@ -0,0 +1,11 @@ +name: "downloader" +version: "1.2.3" +usage: "usage" +description: |- + download something +command: "echo Hello" +downloaders: + - protocols: + - "myprotocol" + - "myprotocols" + command: "echo Download" diff --git a/pkg/plugin/testdata/plugdir/bad/skip-some/echo/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/skip-some/echo/plugin.yaml new file mode 100644 index 000000000..8baa35b6d --- /dev/null +++ b/pkg/plugin/testdata/plugdir/bad/skip-some/echo/plugin.yaml @@ -0,0 +1,8 @@ +name: "echo" +version: "1.2.3" +usage: "echo something" +description: |- + This is a testing fixture. +command: "echo Hello" +hooks: + install: "echo Installing" diff --git a/pkg/plugin/testdata/plugdir/bad/skip-some/empty-config/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/skip-some/empty-config/plugin.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/plugin/testdata/plugdir/bad/skip-some/hello/hello.sh b/pkg/plugin/testdata/plugdir/bad/skip-some/hello/hello.sh new file mode 100644 index 000000000..dcfd58876 --- /dev/null +++ b/pkg/plugin/testdata/plugdir/bad/skip-some/hello/hello.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +echo "Hello from a Helm plugin" + +echo "PARAMS" +echo $* + +$HELM_BIN ls --all + diff --git a/pkg/plugin/testdata/plugdir/bad/skip-some/hello/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/skip-some/hello/plugin.yaml new file mode 100644 index 000000000..b857b55ee --- /dev/null +++ b/pkg/plugin/testdata/plugdir/bad/skip-some/hello/plugin.yaml @@ -0,0 +1,9 @@ +name: "hello" +version: "0.1.0" +usage: "usage" +description: |- + description +command: "$HELM_PLUGIN_DIR/hello.sh" +ignoreFlags: true +hooks: + install: "echo installing..." diff --git a/pkg/plugin/testdata/plugdir/bad/skip/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/skip/plugin.yaml new file mode 100644 index 000000000..e69de29bb