From e12915dc4d1059d72cced556bc62f760ad817a65 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 20 Feb 2020 19:59:42 -0800 Subject: [PATCH 1/9] Add index migration code behind an environment variable This only changes the directory structure that will be needed to support custom indexes. The other changes will be in subsequent PRs. --- cmd/krew/cmd/system.go | 20 +++++++ internal/environment/environment.go | 3 ++ internal/indexmigration/migration.go | 48 +++++++++++++++++ internal/indexmigration/migration_test.go | 64 +++++++++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 internal/indexmigration/migration.go create mode 100644 internal/indexmigration/migration_test.go diff --git a/cmd/krew/cmd/system.go b/cmd/krew/cmd/system.go index a7b6cded..d7c126f7 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/environment/environment.go b/internal/environment/environment.go index 9ebbf890..5105140d 100644 --- a/internal/environment/environment.go +++ b/internal/environment/environment.go @@ -59,6 +59,9 @@ func (p Paths) BasePath() string { return p.base } // e.g. {BasePath}/index/ func (p Paths) IndexPath() string { return filepath.Join(p.base, "index") } +// DefaultIndexPath returns the base directory where the default plugin index repository is cloned. +func (p Paths) DefaultIndexPath() string { return filepath.Join(p.base, "index", "default") } + // IndexPluginsPath returns the plugins directory of the index repository. // // e.g. {BasePath}/index/plugins/ diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go new file mode 100644 index 00000000..8403bb6c --- /dev/null +++ b/internal/indexmigration/migration.go @@ -0,0 +1,48 @@ +// 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" + + "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 doesn't contain a "default" directory. +func Done(paths environment.Paths) (bool, error) { + f, err := os.Stat(paths.DefaultIndexPath()) + if err == nil { + return f.IsDir(), nil + } else if os.IsNotExist(err) { + return false, 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 { + err := os.RemoveAll(paths.IndexPath()) + if err != nil { + return err + } + err = gitutil.EnsureCloned(constants.IndexURI, paths.DefaultIndexPath()) + if err != nil { + return err + } + return nil +} diff --git a/internal/indexmigration/migration_test.go b/internal/indexmigration/migration_test.go new file mode 100644 index 00000000..ab5bf80c --- /dev/null +++ b/internal/indexmigration/migration_test.go @@ -0,0 +1,64 @@ +// 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", + expected: true, + }, + { + name: "Not migrated", + dirPath: "index", + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tmpDir, cleanup := testutil.NewTempDir(t) + defer cleanup() + + _ = os.MkdirAll(tmpDir.Path(test.dirPath), os.ModePerm) + + 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) + } + }) + } +} From 31cdb6fa4cadc1641148ef6cebf2359276d7672f Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 20 Feb 2020 20:07:10 -0800 Subject: [PATCH 2/9] Check to see if the migration has already happened --- internal/indexmigration/migration.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go index 8403bb6c..422ffc9b 100644 --- a/internal/indexmigration/migration.go +++ b/internal/indexmigration/migration.go @@ -17,6 +17,7 @@ package indexmigration import ( "os" + "k8s.io/klog" "sigs.k8s.io/krew/internal/environment" "sigs.k8s.io/krew/internal/gitutil" "sigs.k8s.io/krew/pkg/constants" @@ -36,6 +37,15 @@ func Done(paths environment.Paths) (bool, error) { // 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 err + } + if isMigrated { + klog.Infoln("Already migrated") + return nil + } + err := os.RemoveAll(paths.IndexPath()) if err != nil { return err From 317df5cd53427d8b9e94a39fd1ad54d6281c357a Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 20 Feb 2020 20:11:12 -0800 Subject: [PATCH 3/9] Fix error and linting issue --- internal/indexmigration/migration.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go index 422ffc9b..2013f179 100644 --- a/internal/indexmigration/migration.go +++ b/internal/indexmigration/migration.go @@ -18,6 +18,7 @@ import ( "os" "k8s.io/klog" + "sigs.k8s.io/krew/internal/environment" "sigs.k8s.io/krew/internal/gitutil" "sigs.k8s.io/krew/pkg/constants" @@ -46,7 +47,7 @@ func Migrate(paths environment.Paths) error { return nil } - err := os.RemoveAll(paths.IndexPath()) + err = os.RemoveAll(paths.IndexPath()) if err != nil { return err } From aeeb8a22d06421cd3911a029b6aa617b400dce00 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 20 Feb 2020 21:02:58 -0800 Subject: [PATCH 4/9] Code review changes --- internal/environment/environment.go | 3 --- internal/indexmigration/migration.go | 20 +++++++++++--------- internal/indexmigration/migration_test.go | 9 ++++++--- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/internal/environment/environment.go b/internal/environment/environment.go index 5105140d..9ebbf890 100644 --- a/internal/environment/environment.go +++ b/internal/environment/environment.go @@ -59,9 +59,6 @@ func (p Paths) BasePath() string { return p.base } // e.g. {BasePath}/index/ func (p Paths) IndexPath() string { return filepath.Join(p.base, "index") } -// DefaultIndexPath returns the base directory where the default plugin index repository is cloned. -func (p Paths) DefaultIndexPath() string { return filepath.Join(p.base, "index", "default") } - // IndexPluginsPath returns the plugins directory of the index repository. // // e.g. {BasePath}/index/plugins/ diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go index 2013f179..549e5c18 100644 --- a/internal/indexmigration/migration.go +++ b/internal/indexmigration/migration.go @@ -16,7 +16,9 @@ package indexmigration import ( "os" + "path/filepath" + "github.com/pkg/errors" "k8s.io/klog" "sigs.k8s.io/krew/internal/environment" @@ -25,13 +27,13 @@ import ( ) // Done checks if the krew installation requires a migration to support multiple indexes. -// A migration is necessary when the index directory doesn't contain a "default" directory. +// A migration is necessary when the index directory contains a ".git" directory. func Done(paths environment.Paths) (bool, error) { - f, err := os.Stat(paths.DefaultIndexPath()) + _, err := os.Stat(filepath.Join(paths.IndexPath(), ".git")) if err == nil { - return f.IsDir(), nil - } else if os.IsNotExist(err) { return false, nil + } else if os.IsNotExist(err) { + return true, nil } return false, err } @@ -40,20 +42,20 @@ func Done(paths environment.Paths) (bool, error) { func Migrate(paths environment.Paths) error { isMigrated, err := Done(paths) if err != nil { - return err + return errors.Wrap(err, "failed to check if index migration is complete") } if isMigrated { - klog.Infoln("Already migrated") + klog.V(2).Infoln("Already migrated") return nil } err = os.RemoveAll(paths.IndexPath()) if err != nil { - return err + return errors.Wrapf(err, "could not remove index directory %q", paths.IndexPath()) } - err = gitutil.EnsureCloned(constants.IndexURI, paths.DefaultIndexPath()) + err = gitutil.EnsureCloned(constants.IndexURI, filepath.Join(paths.IndexPath(), "default")) if err != nil { - return err + return errors.Wrap(err, "failed to clone index") } return nil } diff --git a/internal/indexmigration/migration_test.go b/internal/indexmigration/migration_test.go index ab5bf80c..0e7dc105 100644 --- a/internal/indexmigration/migration_test.go +++ b/internal/indexmigration/migration_test.go @@ -34,12 +34,12 @@ func TestIsMigrated(t *testing.T) { }{ { name: "Already migrated", - dirPath: "index/default", + dirPath: "index", expected: true, }, { name: "Not migrated", - dirPath: "index", + dirPath: "index/.git", expected: false, }, } @@ -49,7 +49,10 @@ func TestIsMigrated(t *testing.T) { tmpDir, cleanup := testutil.NewTempDir(t) defer cleanup() - _ = os.MkdirAll(tmpDir.Path(test.dirPath), os.ModePerm) + err := os.MkdirAll(tmpDir.Path(test.dirPath), os.ModePerm) + if err != nil { + t.Fatal(err) + } newPaths := environment.NewPaths(tmpDir.Root()) actual, err := Done(newPaths) From dfa0d054775e5e778002ddbc16f803349c1242b2 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 20 Feb 2020 21:06:42 -0800 Subject: [PATCH 5/9] Make migrated test case a little more explicit --- internal/indexmigration/migration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/indexmigration/migration_test.go b/internal/indexmigration/migration_test.go index 0e7dc105..eaaf5ff9 100644 --- a/internal/indexmigration/migration_test.go +++ b/internal/indexmigration/migration_test.go @@ -34,7 +34,7 @@ func TestIsMigrated(t *testing.T) { }{ { name: "Already migrated", - dirPath: "index", + dirPath: "index/default/.git", expected: true, }, { From 10f05029a994b085a0571a908b105e27d4b95625 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Fri, 21 Feb 2020 10:38:31 -0800 Subject: [PATCH 6/9] Add migration check to root behind env var --- cmd/krew/cmd/root.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go index ff5d91c2..3bcd1173 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 home outdated") + } + } + if installation.IsWindows() { klog.V(4).Infof("detected windows, will check for old krew installations to clean up") err := cleanupStaleKrewInstallations() From 521f8402380e9219f7d2b683f0987821a6cd3a71 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Fri, 21 Feb 2020 10:47:00 -0800 Subject: [PATCH 7/9] Update wording around out of date index message --- cmd/krew/cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go index 3bcd1173..bb7a0629 100644 --- a/cmd/krew/cmd/root.go +++ b/cmd/krew/cmd/root.go @@ -146,7 +146,7 @@ func preRun(cmd *cobra.Command, _ []string) error { } 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 home outdated") + return errors.New("krew index outdated") } } From 9d0c164e6544607ae0d0aea7eb4b0bfe267c1324 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Fri, 21 Feb 2020 13:06:53 -0800 Subject: [PATCH 8/9] Add success message after migrate --- internal/indexmigration/migration.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go index 549e5c18..cf635adf 100644 --- a/internal/indexmigration/migration.go +++ b/internal/indexmigration/migration.go @@ -57,5 +57,6 @@ func Migrate(paths environment.Paths) error { if err != nil { return errors.Wrap(err, "failed to clone index") } + klog.Infof("Migration completed succesfully.") return nil } From 4a0c3e03e50971a35f2bde822180cc59c767d670 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Fri, 21 Feb 2020 13:08:32 -0800 Subject: [PATCH 9/9] Fix typo --- internal/indexmigration/migration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/indexmigration/migration.go b/internal/indexmigration/migration.go index cf635adf..d8685c51 100644 --- a/internal/indexmigration/migration.go +++ b/internal/indexmigration/migration.go @@ -57,6 +57,6 @@ func Migrate(paths environment.Paths) error { if err != nil { return errors.Wrap(err, "failed to clone index") } - klog.Infof("Migration completed succesfully.") + klog.Infof("Migration completed successfully.") return nil }