Repository: kubernetes-sigs/krew
Pull request #506 (2020-02-21 04:51:12)

add helper for inferring index name from input
To be used in multi-index work.
cc: @chriskim06

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/506#" title="Author self-approved">ahmetb</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov-io: # [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=h1) Report
> Merging [#506](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=desc) into [master](https://codecov.io/gh/kubernetes-sigs/krew/commit/fb5343a7101fe3f7742ef90d70f007f5de70f6d9?src=pr&el=desc) will **increase** coverage by `0.21%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/kubernetes-sigs/krew/pull/506/graphs/tree.svg?width=650&token=J3nFtARI2m&height=150&src=pr)](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #506      +/-   ##
==========================================
+ Coverage   58.51%   58.72%   +0.21%     
==========================================
  Files          22       22              
  Lines         969      974       +5     
==========================================
+ Hits          567      572       +5     
  Misses        346      346              
  Partials       56       56
```


| [Impacted Files](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/pathutil/pathutil.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/506/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGF0aHV0aWwvcGF0aHV0aWwuZ28=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=footer). Last update [fb5343a...ca81ca7](https://codecov.io/gh/kubernetes-sigs/krew/pull/506?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

chriskim06: I can use `constants.DefaultIndexName` in https://github.com/kubernetes-sigs/krew/pull/505, so when this gets merge I'll rebase the other one.
ahmetb: Migration codes ideally should not use external variables like this (we want them to do a rather static job). 
So let’s hardcode that one. 

Patch:
diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go
index ff5d91c..bb7a062 100644
--- a/cmd/krew/cmd/root.go
+++ b/cmd/krew/cmd/root.go
@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/krew/cmd/krew/cmd/internal"
 	"sigs.k8s.io/krew/internal/environment"
 	"sigs.k8s.io/krew/internal/gitutil"
+	"sigs.k8s.io/krew/internal/indexmigration"
 	"sigs.k8s.io/krew/internal/installation"
 	"sigs.k8s.io/krew/internal/installation/receipt"
 	"sigs.k8s.io/krew/internal/installation/semver"
@@ -138,6 +139,17 @@ func preRun(cmd *cobra.Command, _ []string) error {
 		return errors.New("krew home outdated")
 	}
 
+	if _, ok := os.LookupEnv("X_KREW_ENABLE_MULTI_INDEX"); ok {
+		isMigrated, err = indexmigration.Done(paths)
+		if err != nil {
+			return errors.Wrap(err, "error getting file info")
+		}
+		if !isMigrated && cmd.Use != "index-upgrade" {
+			fmt.Fprintln(os.Stderr, "You need to perform a migration to continue using krew.\nPlease run `kubectl krew system index-upgrade`")
+			return errors.New("krew index outdated")
+		}
+	}
+
 	if installation.IsWindows() {
 		klog.V(4).Infof("detected windows, will check for old krew installations to clean up")
 		err := cleanupStaleKrewInstallations()
diff --git a/cmd/krew/cmd/system.go b/cmd/krew/cmd/system.go
index a7b6cde..d7c126f 100644
--- a/cmd/krew/cmd/system.go
+++ b/cmd/krew/cmd/system.go
@@ -15,8 +15,11 @@
 package cmd
 
 import (
+	"os"
+
 	"github.com/spf13/cobra"
 
+	"sigs.k8s.io/krew/internal/indexmigration"
 	"sigs.k8s.io/krew/internal/receiptsmigration"
 )
 
@@ -50,7 +53,24 @@ This command will be removed without further notice from future versions of krew
 	PreRunE: ensureIndexUpdated,
 }
 
+var indexUpgradeCmd = &cobra.Command{
+	Use:   "index-upgrade",
+	Short: "Perform a migration of the krew index",
+	Long: `Krew became more awesome! To use the new features, you need to run this
+one-time migration, which will enable installing plugins from custom indexes.
+
+This command will be removed without further notice from future versions of krew.
+`,
+	Args: cobra.NoArgs,
+	RunE: func(cmd *cobra.Command, args []string) error {
+		return indexmigration.Migrate(paths)
+	},
+}
+
 func init() {
+	if _, ok := os.LookupEnv("X_KREW_ENABLE_MULTI_INDEX"); ok {
+		systemCmd.AddCommand(indexUpgradeCmd)
+	}
 	systemCmd.AddCommand(receiptsUpgradeCmd)
 	rootCmd.AddCommand(systemCmd)
 }
diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go
new file mode 100644
index 0000000..d8685c5
--- /dev/null
+++ b/internal/indexmigration/migration.go
@@ -0,0 +1,62 @@
+// Copyright 2019 The Kubernetes Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package indexmigration
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/pkg/errors"
+	"k8s.io/klog"
+
+	"sigs.k8s.io/krew/internal/environment"
+	"sigs.k8s.io/krew/internal/gitutil"
+	"sigs.k8s.io/krew/pkg/constants"
+)
+
+// 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"))
+	if err == nil {
+		return false, nil
+	} else if os.IsNotExist(err) {
+		return true, nil
+	}
+	return false, err
+}
+
+// Migrate removes the index directory and then clones krew-index to the new default index path.
+func Migrate(paths environment.Paths) error {
+	isMigrated, err := Done(paths)
+	if err != nil {
+		return errors.Wrap(err, "failed to check if index migration is complete")
+	}
+	if isMigrated {
+		klog.V(2).Infoln("Already migrated")
+		return nil
+	}
+
+	err = os.RemoveAll(paths.IndexPath())
+	if err != nil {
+		return errors.Wrapf(err, "could not remove index directory %q", paths.IndexPath())
+	}
+	err = gitutil.EnsureCloned(constants.IndexURI, filepath.Join(paths.IndexPath(), "default"))
+	if err != nil {
+		return errors.Wrap(err, "failed to clone index")
+	}
+	klog.Infof("Migration completed successfully.")
+	return nil
+}
diff --git a/internal/pathutil/pathutil.go b/internal/pathutil/pathutil.go
index b672426..b9fc7bf 100644
--- a/internal/pathutil/pathutil.go
+++ b/internal/pathutil/pathutil.go
@@ -19,6 +19,8 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+
+	"sigs.k8s.io/krew/pkg/constants"
 )
 
 // IsSubPath checks if the extending path is an extension of the basePath, it will return the extending path
@@ -42,3 +44,13 @@ func ReplaceBase(path, old, replacement string) (string, error) {
 	}
 	return filepath.Join(replacement, extendingPath), nil
 }
+
+// CanonicalPluginName resolves a plugin's index and name from input string.
+// If an index is not specified, the default index name is assumed.
+func CanonicalPluginName(in string) (string, string) {
+	if strings.Count(in, "/") == 0 {
+		return constants.DefaultIndexName, in
+	}
+	p := strings.SplitN(in, "/", 2)
+	return p[0], p[1]
+}
diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
index bfa8cd8..04dad80 100644
--- a/pkg/constants/constants.go
+++ b/pkg/constants/constants.go
@@ -22,4 +22,6 @@ const (
 
 	// IndexURI points to the upstream index.
 	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"
 )


Test patch:
diff --git a/internal/indexmigration/migration_test.go b/internal/indexmigration/migration_test.go
new file mode 100644
index 0000000..eaaf5ff
--- /dev/null
+++ b/internal/indexmigration/migration_test.go
@@ -0,0 +1,67 @@
+// Copyright 2019 The Kubernetes Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package indexmigration
+
+import (
+	"os"
+	"testing"
+
+	"sigs.k8s.io/krew/internal/environment"
+	"sigs.k8s.io/krew/internal/testutil"
+)
+
+func TestIsMigrated(t *testing.T) {
+	if _, ok := os.LookupEnv("X_KREW_ENABLE_MULTI_INDEX"); !ok {
+		t.Skip("Set X_KREW_ENABLE_MULTI_INDEX variable to run this test")
+	}
+
+	tests := []struct {
+		name     string
+		dirPath  string
+		expected bool
+	}{
+		{
+			name:     "Already migrated",
+			dirPath:  "index/default/.git",
+			expected: true,
+		},
+		{
+			name:     "Not migrated",
+			dirPath:  "index/.git",
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			tmpDir, cleanup := testutil.NewTempDir(t)
+			defer cleanup()
+
+			err := os.MkdirAll(tmpDir.Path(test.dirPath), os.ModePerm)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			newPaths := environment.NewPaths(tmpDir.Root())
+			actual, err := Done(newPaths)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if actual != test.expected {
+				t.Errorf("Expected %v but found %v", test.expected, actual)
+			}
+		})
+	}
+}
diff --git a/internal/pathutil/pathutil_test.go b/internal/pathutil/pathutil_test.go
index 10c350c..47f7029 100644
--- a/internal/pathutil/pathutil_test.go
+++ b/internal/pathutil/pathutil_test.go
@@ -184,3 +184,27 @@ func TestReplaceBase(t *testing.T) {
 		})
 	}
 }
+
+func TestCanonicalPluginName(t *testing.T) {
+	tests := []struct {
+		in        string
+		wantIndex string
+		wantName  string
+	}{
+		{"foo", "default", "foo"},
+		{"", "default", ""}, // despite unsupported
+		{"a/b", "a", "b"},
+		{"a/b/c", "a", "b/c"}, // despite unsupported
+	}
+	for _, tt := range tests {
+		t.Run(tt.in, func(t *testing.T) {
+			gotIndex, gotName := CanonicalPluginName(tt.in)
+			if gotIndex != tt.wantIndex {
+				t.Errorf("CanonicalPluginName(%q) gotIndex = %q, want = %q", tt.in, gotIndex, tt.wantIndex)
+			}
+			if gotName != tt.wantName {
+				t.Errorf("CanonicalPluginName(%q) gotName = %q, want = %q", tt.in, gotName, tt.wantName)
+			}
+		})
+	}
+}