From cbc77cc4a6463e4c6db1a72b7ca5c7fbe4abf9f8 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Sat, 28 Mar 2020 19:15:46 -0700 Subject: [PATCH 1/3] cmd: make krew search work with multiple indexes Mostly just refactoring. Added test to make sure search output is alphabetically sorted (even in case of multiple indexes). Signed-off-by: Ahmet Alp Balkan --- cmd/krew/cmd/search.go | 80 +++++++++++++++++++++++---------- integration_test/search_test.go | 53 ++++++++++++++++++++++ 2 files changed, 110 insertions(+), 23 deletions(-) diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go index 8a8570c2..16f5e36f 100644 --- a/cmd/krew/cmd/search.go +++ b/cmd/krew/cmd/search.go @@ -22,11 +22,12 @@ import ( "github.com/pkg/errors" "github.com/sahilm/fuzzy" "github.com/spf13/cobra" + "k8s.io/klog" + "sigs.k8s.io/krew/internal/index/indexoperations" "sigs.k8s.io/krew/internal/index/indexscanner" "sigs.k8s.io/krew/internal/installation" "sigs.k8s.io/krew/pkg/constants" - "sigs.k8s.io/krew/pkg/index" ) // searchCmd represents the search command @@ -43,58 +44,91 @@ Examples: To fuzzy search plugins with a keyword: kubectl krew search KEYWORD`, RunE: func(cmd *cobra.Command, args []string) error { - plugins, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName)) - if err != nil { - return errors.Wrap(err, "failed to load the list of plugins from the index") + indexes := []indexoperations.Index{ + { + Name: constants.DefaultIndexName, + URL: constants.IndexURI, // unused here but providing for completeness + }, } - names := make([]string, len(plugins)) - pluginMap := make(map[string]index.Plugin, len(plugins)) - for i, p := range plugins { - names[i] = p.Name - pluginMap[p.Name] = p + if os.Getenv(constants.EnableMultiIndexSwitch) != "" { + out, err := indexoperations.ListIndexes(paths) + if err != nil { + return errors.Wrapf(err, "failed to list plugin indexes available") + } + indexes = out } + klog.V(3).Infof("found %d indexes", len(indexes)) + + var plugins []pluginEntry + for _, idx := range indexes { + ps, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName)) + if err != nil { + return errors.Wrap(err, "failed to load the list of plugins from the index") + } + for _, p := range ps { + plugins = append(plugins, pluginEntry{p, idx.Name}) + } + } + + keys := func(v map[string]pluginEntry) []string { + out := make([]string, 0, len(v)) + for k := range v { + out = append(out, k) + } + return out + } + + pluginMap := make(map[string]pluginEntry, len(plugins)) + for _, p := range plugins { + pluginMap[canonicalName(p.p, p.indexName)] = p + } + + installed := make(map[string]string) receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath()) if err != nil { return errors.Wrap(err, "failed to load installed plugins") } - - // TODO(chriskim06) include index name when refactoring for custom indexes - installed := make(map[string]string) for _, receipt := range receipts { - installed[receipt.Name] = receipt.Spec.Version + index := receipt.Status.Source.Name + if index == "" { + index = constants.DefaultIndexName + } + installed[receipt.Name] = index } - var matchNames []string + corpus := keys(pluginMap) + var searchResults []string if len(args) > 0 { - matches := fuzzy.Find(strings.Join(args, ""), names) + matches := fuzzy.Find(strings.Join(args, ""), corpus) for _, m := range matches { - matchNames = append(matchNames, m.Str) + searchResults = append(searchResults, m.Str) } } else { - matchNames = names + searchResults = corpus } // No plugins found - if len(matchNames) == 0 { + if len(searchResults) == 0 { return nil } var rows [][]string cols := []string{"NAME", "DESCRIPTION", "INSTALLED"} - for _, name := range matchNames { - plugin := pluginMap[name] + for _, name := range searchResults { + v := pluginMap[name] var status string - if _, ok := installed[name]; ok { + if index := installed[v.p.Name]; index == v.indexName { status = "yes" - } else if _, ok, err := installation.GetMatchingPlatform(plugin.Spec.Platforms); err != nil { + } else if _, ok, err := installation.GetMatchingPlatform(v.p.Spec.Platforms); err != nil { return errors.Wrapf(err, "failed to get the matching platform for plugin %s", name) } else if ok { status = "no" } else { status = "unavailable on " + runtime.GOOS } - rows = append(rows, []string{name, limitString(plugin.Spec.ShortDescription, 50), status}) + + rows = append(rows, []string{displayName(v.p, v.indexName), limitString(v.p.Spec.ShortDescription, 50), status}) } rows = sortByFirstColumn(rows) return printTable(os.Stdout, cols, rows) diff --git a/integration_test/search_test.go b/integration_test/search_test.go index 44489bec..a44de314 100644 --- a/integration_test/search_test.go +++ b/integration_test/search_test.go @@ -15,8 +15,12 @@ package integrationtest import ( + "regexp" + "sort" "strings" "testing" + + "sigs.k8s.io/krew/pkg/constants" ) func TestKrewSearchAll(t *testing.T) { @@ -46,3 +50,52 @@ func TestKrewSearchOne(t *testing.T) { t.Errorf("The first match should be krew") } } + +func TestKrewSearchMultiIndex(t *testing.T) { + skipShort(t) + test, cleanup := NewTest(t) + test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex() + defer cleanup() + + // alias default plugin index to another + localIndex := test.TempDir().Path("index/" + constants.DefaultIndexName) + test.Krew("index", "add", "foo", localIndex).RunOrFailOutput() + + test.Krew("install", validPlugin).RunOrFail() + test.Krew("install", "foo/"+validPlugin2).RunOrFail() + + output := string(test.Krew("search").RunOrFailOutput()) + wantPatterns := []*regexp.Regexp{ + regexp.MustCompile(`(?m)^` + validPlugin + `\b.*\byes`), + regexp.MustCompile(`(?m)^` + validPlugin2 + `\b.*\bno`), + regexp.MustCompile(`(?m)^foo/` + validPlugin + `\b.*\bno$`), + regexp.MustCompile(`(?m)^foo/` + validPlugin2 + `\b.*\byes$`), + } + for _, p := range wantPatterns { + if !p.MatchString(output) { + t.Fatalf("pattern %s not found in search output=%s", p, output) + } + } +} + +func TestKrewSearchMultiIndexSortedByDisplayName(t *testing.T) { + skipShort(t) + test, cleanup := NewTest(t) + test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex() + defer cleanup() + + // alias default plugin index to another + localIndex := test.TempDir().Path("index/" + constants.DefaultIndexName) + test.Krew("index", "add", "foo", localIndex).RunOrFailOutput() + + output := string(test.Krew("search").RunOrFailOutput()) + + // match first column that is not NAME by matching everything up until a space + names := regexp.MustCompile(`(?m)^[^\s|NAME]+\b`).FindAllString(output, -1) + if len(names) < 10 { + t.Fatalf("could not capture names") + } + if !sort.StringsAreSorted(names) { + t.Fatalf("names are not sorted: [%s]", strings.Join(names, ", ")) + } +} From 8d77f840c5aae15a4c9ae6ce5db571bf2f0e8c79 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Mon, 30 Mar 2020 17:49:21 -0700 Subject: [PATCH 2/3] fix bug Signed-off-by: Ahmet Alp Balkan --- cmd/krew/cmd/search.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go index 16f5e36f..7131fee2 100644 --- a/cmd/krew/cmd/search.go +++ b/cmd/krew/cmd/search.go @@ -62,7 +62,7 @@ Examples: var plugins []pluginEntry for _, idx := range indexes { - ps, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName)) + ps, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(idx.Name)) if err != nil { return errors.Wrap(err, "failed to load the list of plugins from the index") } From 0a343ce5b47d379bf3da37a6090bc583ef95fb35 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Fri, 3 Apr 2020 16:25:50 -0700 Subject: [PATCH 3/3] address code review feedback Signed-off-by: Ahmet Alp Balkan --- cmd/krew/cmd/search.go | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go index 7131fee2..313f1c42 100644 --- a/cmd/krew/cmd/search.go +++ b/cmd/krew/cmd/search.go @@ -71,41 +71,32 @@ Examples: } } - keys := func(v map[string]pluginEntry) []string { - out := make([]string, 0, len(v)) - for k := range v { - out = append(out, k) - } - return out - } - - pluginMap := make(map[string]pluginEntry, len(plugins)) - for _, p := range plugins { - pluginMap[canonicalName(p.p, p.indexName)] = p + pluginCanonicalNames := make([]string, len(plugins)) + pluginCanonicalNameMap := make(map[string]pluginEntry, len(plugins)) + for i, p := range plugins { + cn := canonicalName(p.p, p.indexName) + pluginCanonicalNames[i] = cn + pluginCanonicalNameMap[cn] = p } - installed := make(map[string]string) + installed := make(map[string]bool) receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath()) if err != nil { return errors.Wrap(err, "failed to load installed plugins") } for _, receipt := range receipts { - index := receipt.Status.Source.Name - if index == "" { - index = constants.DefaultIndexName - } - installed[receipt.Name] = index + cn := canonicalName(receipt.Plugin, indexOf(receipt)) + installed[cn] = true } - corpus := keys(pluginMap) var searchResults []string if len(args) > 0 { - matches := fuzzy.Find(strings.Join(args, ""), corpus) + matches := fuzzy.Find(strings.Join(args, ""), pluginCanonicalNames) for _, m := range matches { searchResults = append(searchResults, m.Str) } } else { - searchResults = corpus + searchResults = pluginCanonicalNames } // No plugins found @@ -115,13 +106,13 @@ Examples: var rows [][]string cols := []string{"NAME", "DESCRIPTION", "INSTALLED"} - for _, name := range searchResults { - v := pluginMap[name] + for _, canonicalName := range searchResults { + v := pluginCanonicalNameMap[canonicalName] var status string - if index := installed[v.p.Name]; index == v.indexName { + if installed[canonicalName] { status = "yes" } else if _, ok, err := installation.GetMatchingPlatform(v.p.Spec.Platforms); err != nil { - return errors.Wrapf(err, "failed to get the matching platform for plugin %s", name) + return errors.Wrapf(err, "failed to get the matching platform for plugin %s", canonicalName) } else if ok { status = "no" } else {