Refactor existing index path functions to support multiple indexes la…

…ter on (#510)

* Refactor existing index path functions

Add new functionality behind env var

* Add unit test behind env var

* Code review changes

* Code review updates

* Use default instead of empty string

* Remove unnecessary empty string check

* Code review changes

Use constants variable for "default", update godoc, and remove
unnecessary tests

* Update old paths functions

cmd/krew/cmd/install.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/krew/cmd/krew/cmd/internal"
 	"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"
 )
 
@@ -91,7 +92,7 @@ Remarks:
 
 			var install []index.Plugin
 			for _, name := range pluginNames {
-				plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(), name)
+				plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), name)
 				if err != nil {
 					if os.IsNotExist(err) {
 						return errors.Errorf("plugin %q does not exist in the plugin index", name)

cmd/krew/cmd/root.go

@@ -139,7 +139,7 @@ func preRun(cmd *cobra.Command, _ []string) error {
 		return errors.New("krew home outdated")
 	}
 
-	if _, ok := os.LookupEnv("X_KREW_ENABLE_MULTI_INDEX"); ok {
+	if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
 		isMigrated, err = indexmigration.Done(paths)
 		if err != nil {
 			return errors.Wrap(err, "error getting file info")
@@ -199,7 +199,7 @@ func cleanupStaleKrewInstallations() error {
 }
 
 func checkIndex(_ *cobra.Command, _ []string) error {
-	if ok, err := gitutil.IsGitCloned(paths.IndexPath()); err != nil {
+	if ok, err := gitutil.IsGitCloned(paths.IndexPath(constants.DefaultIndexName)); err != nil {
 		return errors.Wrap(err, "failed to check local index git repository")
 	} else if !ok {
 		return errors.New(`krew local plugin index is not initialized (run "kubectl krew update")`)

cmd/krew/cmd/search.go

@@ -25,6 +25,7 @@ import (
 
 	"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"
 )
 
@@ -42,7 +43,7 @@ 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())
+		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")
 		}

cmd/krew/cmd/system.go

@@ -21,6 +21,7 @@ import (
 
 	"sigs.k8s.io/krew/internal/indexmigration"
 	"sigs.k8s.io/krew/internal/receiptsmigration"
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
 // todo(corneliusweig) remove migration code with v0.4
@@ -68,7 +69,7 @@ This command will be removed without further notice from future versions of krew
 }
 
 func init() {
-	if _, ok := os.LookupEnv("X_KREW_ENABLE_MULTI_INDEX"); ok {
+	if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
 		systemCmd.AddCommand(indexUpgradeCmd)
 	}
 	systemCmd.AddCommand(receiptsUpgradeCmd)

cmd/krew/cmd/update.go

@@ -103,10 +103,10 @@ func showUpdatedPlugins(out io.Writer, preUpdate, posUpdate []index.Plugin, inst
 }
 
 func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
-	preUpdateIndex, _ := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath())
+	preUpdateIndex, _ := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName))
 
-	klog.V(1).Infof("Updating the local copy of plugin index (%s)", paths.IndexPath())
-	if err := gitutil.EnsureUpdated(constants.IndexURI, paths.IndexPath()); err != nil {
+	klog.V(1).Infof("Updating the local copy of plugin index (%s)", paths.IndexPath(constants.DefaultIndexName))
+	if err := gitutil.EnsureUpdated(constants.IndexURI, paths.IndexPath(constants.DefaultIndexName)); err != nil {
 		return errors.Wrap(err, "failed to update the local index")
 	}
 	fmt.Fprintln(os.Stderr, "Updated the local copy of plugin index.")
@@ -115,7 +115,7 @@ func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
 		return nil
 	}
 
-	posUpdateIndex, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath())
+	posUpdateIndex, err := indexscanner.LoadPluginListFromFS(paths.IndexPluginsPath(constants.DefaultIndexName))
 	if err != nil {
 		return errors.Wrap(err, "failed to load plugin index after update")
 	}

cmd/krew/cmd/upgrade.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/krew/cmd/krew/cmd/internal"
 	"sigs.k8s.io/krew/internal/index/indexscanner"
 	"sigs.k8s.io/krew/internal/installation"
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
 func init() {
@@ -62,7 +63,7 @@ kubectl krew upgrade foo bar"`,
 
 			var nErrors int
 			for _, name := range pluginNames {
-				plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(), name)
+				plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), name)
 				if err != nil {
 					if !os.IsNotExist(err) {
 						return errors.Wrapf(err, "failed to load the plugin manifest for plugin %s", name)

cmd/krew/cmd/version.go

@@ -44,7 +44,7 @@ Remarks:
 			{"GitCommit", version.GitCommit()},
 			{"IndexURI", constants.IndexURI},
 			{"BasePath", paths.BasePath()},
-			{"IndexPath", paths.IndexPath()},
+			{"IndexPath", paths.IndexPath(constants.DefaultIndexName)},
 			{"InstallPath", paths.InstallPath()},
 			{"BinPath", paths.BinPath()},
 			{"DetectedPlatform", installation.OSArch().String()},

integration_test/uninstall_test.go

@@ -64,7 +64,7 @@ func TestKrewRemove_ManifestRemovedFromIndex(t *testing.T) {
 	defer cleanup()
 
 	test = test.WithIndex()
-	manifestDir := environment.NewPaths(test.Root()).IndexPluginsPath()
+	manifestDir := environment.NewPaths(test.Root()).IndexPluginsPath(constants.DefaultIndexName)
 	localManifest := filepath.Join(manifestDir, validPlugin+constants.ManifestExtension)
 	if _, err := os.Stat(localManifest); err != nil {
 		t.Fatalf("could not read local manifest file at %s: %v", localManifest, err)

integration_test/update_test.go

@@ -56,7 +56,7 @@ func TestKrewUpdateListsNewPlugins(t *testing.T) {
 
 	test = test.WithIndex()
 
-	pluginManifest := filepath.Join(environment.NewPaths(test.Root()).IndexPluginsPath(), validPlugin+constants.ManifestExtension)
+	pluginManifest := filepath.Join(environment.NewPaths(test.Root()).IndexPluginsPath(constants.DefaultIndexName), validPlugin+constants.ManifestExtension)
 	if err := os.Remove(pluginManifest); err != nil {
 		t.Fatalf("failed to delete manifest of an existing plugin: %v", err)
 	}
@@ -78,7 +78,7 @@ func TestKrewUpdateListsUpgradesAvailable(t *testing.T) {
 	test = test.WithIndex()
 
 	// set version of some manifests to v0.0.0
-	pluginManifest := filepath.Join(environment.NewPaths(test.Root()).IndexPluginsPath(), validPlugin+constants.ManifestExtension)
+	pluginManifest := filepath.Join(environment.NewPaths(test.Root()).IndexPluginsPath(constants.DefaultIndexName), validPlugin+constants.ManifestExtension)
 	modifyManifestVersion(t, pluginManifest, "v0.0.0")
 
 	test.Krew("install", validPlugin, "--no-update-index").RunOrFail()  // has updates available

internal/environment/environment.go

@@ -54,15 +54,30 @@ func NewPaths(base string) Paths {
 // BasePath returns krew base directory.
 func (p Paths) BasePath() string { return p.base }
 
-// IndexPath returns the base directory where plugin index repository is cloned.
-//
-// e.g. {BasePath}/index/
-func (p Paths) IndexPath() string { return filepath.Join(p.base, "index") }
+// IndexBase returns the krew index directory. This directory contains the default
+// index and custom ones.
+func (p Paths) IndexBase() string {
+	return filepath.Join(p.base, "index")
+}
 
-// IndexPluginsPath returns the plugins directory of the index repository.
-//
-// e.g. {BasePath}/index/plugins/
-func (p Paths) IndexPluginsPath() string { return filepath.Join(p.base, "index", "plugins") }
+// IndexPath returns the directory where a plugin index repository is cloned.
+// When constants.EnableMultiIndexSwitch var is unset it just returns the krew
+// index base.
+// e.g. {BasePath}/index/default or {BasePath}/index
+func (p Paths) IndexPath(name string) string {
+	if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
+		return filepath.Join(p.base, "index", name)
+	}
+	return p.IndexBase()
+}
+
+// IndexPluginsPath returns the plugins directory of an index repository.
+// When constants.EnableMultiIndexSwitch var is unset it just returns the old
+// structure krew-index plugins.
+// e.g. {BasePath}/index/default/plugins/ or {BasePath}/index/plugins/
+func (p Paths) IndexPluginsPath(name string) string {
+	return filepath.Join(p.IndexPath(name), "plugins")
+}
 
 // InstallReceiptsPath returns the base directory where plugin receipts are stored.
 //

internal/environment/environment_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/client-go/util/homedir"
 
 	"sigs.k8s.io/krew/internal/testutil"
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
 func TestMustGetKrewPaths_resolvesToHomeDir(t *testing.T) {
@@ -49,31 +50,45 @@ func TestPaths(t *testing.T) {
 	base := filepath.FromSlash("/foo")
 	p := NewPaths(base)
 	if got := p.BasePath(); got != base {
-		t.Fatalf("BasePath()=%s; expected=%s", got, base)
+		t.Errorf("BasePath()=%s; expected=%s", got, base)
 	}
 	if got, expected := p.BinPath(), filepath.FromSlash("/foo/bin"); got != expected {
-		t.Fatalf("BinPath()=%s; expected=%s", got, expected)
-	}
-	if got, expected := p.IndexPath(), filepath.FromSlash("/foo/index"); got != expected {
-		t.Fatalf("IndexPath()=%s; expected=%s", got, expected)
-	}
-	if got, expected := p.IndexPluginsPath(), filepath.FromSlash("/foo/index/plugins"); got != expected {
-		t.Fatalf("IndexPluginsPath()=%s; expected=%s", got, expected)
+		t.Errorf("BinPath()=%s; expected=%s", got, expected)
 	}
+
+	t.Run("with EnableMultiIndexSwitch", func(t *testing.T) {
+		os.Setenv(constants.EnableMultiIndexSwitch, "1")
+		defer os.Unsetenv(constants.EnableMultiIndexSwitch)
+		if got, expected := p.IndexPath(constants.DefaultIndexName), filepath.FromSlash("/foo/index/default"); got != expected {
+			t.Errorf("IndexPath(\"%s\")=%s; expected=%s", constants.DefaultIndexName, got, expected)
+		}
+		if got, expected := p.IndexPluginsPath(constants.DefaultIndexName), filepath.FromSlash("/foo/index/default/plugins"); got != expected {
+			t.Errorf("IndexPluginsPath(\"%s\")=%s; expected=%s", constants.DefaultIndexName, got, expected)
+		}
+	})
+	t.Run("without EnableMultiIndexSwitch", func(t *testing.T) {
+		if got, expected := p.IndexPath(constants.DefaultIndexName), filepath.FromSlash("/foo/index"); got != expected {
+			t.Errorf("IndexPath(\"%s\")=%s; expected=%s", constants.DefaultIndexName, got, expected)
+		}
+		if got, expected := p.IndexPluginsPath(constants.DefaultIndexName), filepath.FromSlash("/foo/index/plugins"); got != expected {
+			t.Errorf("IndexPluginsPath(\"%s\")=%s; expected=%s", constants.DefaultIndexName, got, expected)
+		}
+	})
+
 	if got, expected := p.InstallPath(), filepath.FromSlash("/foo/store"); got != expected {
-		t.Fatalf("InstallPath()=%s; expected=%s", got, expected)
+		t.Errorf("InstallPath()=%s; expected=%s", got, expected)
 	}
 	if got, expected := p.PluginInstallPath("my-plugin"), filepath.FromSlash("/foo/store/my-plugin"); got != expected {
-		t.Fatalf("PluginInstallPath()=%s; expected=%s", got, expected)
+		t.Errorf("PluginInstallPath()=%s; expected=%s", got, expected)
 	}
 	if got, expected := p.PluginVersionInstallPath("my-plugin", "v1"), filepath.FromSlash("/foo/store/my-plugin/v1"); got != expected {
-		t.Fatalf("PluginVersionInstallPath()=%s; expected=%s", got, expected)
+		t.Errorf("PluginVersionInstallPath()=%s; expected=%s", got, expected)
 	}
 	if got := p.InstallReceiptsPath(); !strings.HasSuffix(got, filepath.FromSlash("receipts")) {
-		t.Fatalf("InstallReceiptsPath()=%s; expected suffix 'receipts'", got)
+		t.Errorf("InstallReceiptsPath()=%s; expected suffix 'receipts'", got)
 	}
 	if got := p.PluginInstallReceiptPath("my-plugin"); !strings.HasSuffix(got, filepath.FromSlash("receipts/my-plugin.yaml")) {
-		t.Fatalf("PluginInstallReceiptPath()=%s; expected suffix 'receipts/my-plugin.yaml'", got)
+		t.Errorf("PluginInstallReceiptPath()=%s; expected suffix 'receipts/my-plugin.yaml'", got)
 	}
 }
 

internal/indexmigration/migration.go

@@ -27,7 +27,7 @@ import (
 // Done checks if the krew installation requires a migration to support multiple indexes.
 // A migration is necessary when the index directory contains a ".git" directory.
 func Done(paths environment.Paths) (bool, error) {
-	_, err := os.Stat(filepath.Join(paths.IndexPath(), ".git"))
+	_, err := os.Stat(filepath.Join(paths.IndexBase(), ".git"))
 	if err != nil && os.IsNotExist(err) {
 		return true, nil
 	}
@@ -44,9 +44,9 @@ func Migrate(paths environment.Paths) error {
 		klog.V(2).Infoln("Already migrated")
 		return nil
 	}
-	indexPath := paths.IndexPath()
+	indexPath := paths.IndexBase()
 	tmpPath := filepath.Join(paths.BasePath(), "tmp_index_migration")
-	newPath := filepath.Join(paths.IndexPath(), "default")
+	newPath := filepath.Join(paths.IndexBase(), "default")
 
 	if err := os.Rename(indexPath, tmpPath); err != nil {
 		return errors.Wrapf(err, "could not move index directory %q to temporary location %q", indexPath, tmpPath)

internal/info/info.go

@@ -22,6 +22,7 @@ import (
 
 	"sigs.k8s.io/krew/internal/environment"
 	"sigs.k8s.io/krew/internal/index/indexscanner"
+	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 )
 
@@ -40,5 +41,5 @@ func LoadManifestFromReceiptOrIndex(p environment.Paths, name string) (index.Plu
 	}
 
 	klog.V(3).Infof("Plugin manifest for %q not found in the receipts dir", name)
-	return indexscanner.LoadPluginByName(p.IndexPluginsPath(), name)
+	return indexscanner.LoadPluginByName(p.IndexPluginsPath(constants.DefaultIndexName), name)
 }

internal/info/info_test.go

@@ -24,6 +24,7 @@ import (
 
 	"sigs.k8s.io/krew/internal/environment"
 	"sigs.k8s.io/krew/internal/testutil"
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
 func TestLoadManifestFromReceiptOrIndex(t *testing.T) {
@@ -51,7 +52,7 @@ func TestLoadManifestFromReceiptOrIndex(t *testing.T) {
 		{
 			name: "manifest in index",
 			prepare: func(paths environment.Paths, tmpDir *testutil.TempDir) {
-				path := filepath.Join(paths.IndexPluginsPath(), pluginName+".yaml")
+				path := filepath.Join(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName+".yaml")
 				tmpDir.Write(path, yamlBytes)
 			},
 		},
@@ -66,7 +67,7 @@ func TestLoadManifestFromReceiptOrIndex(t *testing.T) {
 		{
 			name: "invalid manifest in index",
 			prepare: func(paths environment.Paths, tmpDir *testutil.TempDir) {
-				path := filepath.Join(paths.IndexPluginsPath(), pluginName+".yaml")
+				path := filepath.Join(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName+".yaml")
 				tmpDir.Write(path, []byte("invalid yaml file"))
 			},
 			shouldErr: true,

internal/receiptsmigration/migration.go

@@ -76,7 +76,7 @@ func Migrate(newPaths environment.Paths) error {
 	klog.Infoln("These plugins will be reinstalled: ", installed)
 
 	// krew must be skipped by the normal migration logic
-	if err := copyKrewManifest(newPaths.IndexPluginsPath(), newPaths.InstallReceiptsPath()); err != nil {
+	if err := copyKrewManifest(newPaths.IndexPluginsPath(constants.DefaultIndexName), newPaths.InstallReceiptsPath()); err != nil {
 		return errors.Wrapf(err, "failed to copy krew manifest")
 	}
 
@@ -138,7 +138,7 @@ func getPluginsToReinstall(oldPaths oldenvironment.Paths, newPaths environment.P
 
 // isAvailableInIndex checks that the given plugin is available in the index
 func isAvailableInIndex(paths environment.Paths, plugin string) bool {
-	pluginYaml := filepath.Join(paths.IndexPluginsPath(), plugin+constants.ManifestExtension)
+	pluginYaml := filepath.Join(paths.IndexPluginsPath(constants.DefaultIndexName), plugin+constants.ManifestExtension)
 	_, err := os.Lstat(pluginYaml)
 	return err == nil
 }

pkg/constants/constants.go

@@ -24,4 +24,7 @@ const (
 	IndexURI = "https://github.com/kubernetes-sigs/krew-index.git"
 	// DefaultIndexName is a magic string that's used for a plugin name specified without an index.
 	DefaultIndexName = "default"
+	// EnableMultiIndexSwitch is the name of the environment variable that needs to be set to use
+	// the features around multiple indexes (this will be removed later on).
+	EnableMultiIndexSwitch = "X_KREW_ENABLE_MULTI_INDEX"
 )