From 42f69171e502bbab49b225d01c188d5bb2de1a34 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Fri, 20 Mar 2020 18:33:25 -0700 Subject: [PATCH 1/8] Create receipt earlier to pass to Store function --- cmd/krew/cmd/install.go | 13 ++++++++----- cmd/krew/cmd/upgrade.go | 7 +++++-- internal/installation/install.go | 2 +- internal/installation/receipt/receipt.go | 13 ++++++++++++- internal/installation/receipt/receipt_test.go | 6 +++--- internal/installation/upgrade.go | 4 ++-- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 4f5f2af2..1e8c45b4 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -27,6 +27,8 @@ 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/internal/installation/receipt" + "sigs.k8s.io/krew/internal/pathutil" "sigs.k8s.io/krew/pkg/constants" "sigs.k8s.io/krew/pkg/index" ) @@ -90,16 +92,17 @@ Remarks: return errors.New("--archive can be specified only with --manifest or --manifest-url") } - var install []index.Plugin + var install []index.Receipt for _, name := range pluginNames { - plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), name) + indexName, pluginName := pathutil.CanonicalPluginName(name) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName) if err != nil { if os.IsNotExist(err) { return errors.Errorf("plugin %q does not exist in the plugin index", name) } return errors.Wrapf(err, "failed to load plugin %q from the index", name) } - install = append(install, plugin) + install = append(install, receipt.New(plugin, indexName)) } if *manifest != "" { @@ -107,13 +110,13 @@ Remarks: if err != nil { return errors.Wrap(err, "failed to load plugin manifest from file") } - install = append(install, plugin) + install = append(install, receipt.New(plugin, constants.DefaultIndexName)) } else if *manifestURL != "" { plugin, err := readPluginFromURL(*manifestURL) if err != nil { return errors.Wrap(err, "failed to read plugin manifest file from url") } - install = append(install, plugin) + install = append(install, receipt.New(plugin, constants.DefaultIndexName)) } if len(install) == 0 { diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go index 74dbbd7d..0abcfa3e 100644 --- a/cmd/krew/cmd/upgrade.go +++ b/cmd/krew/cmd/upgrade.go @@ -25,6 +25,8 @@ 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/internal/installation/receipt" + "sigs.k8s.io/krew/internal/pathutil" "sigs.k8s.io/krew/pkg/constants" ) @@ -63,7 +65,8 @@ kubectl krew upgrade foo bar"`, var nErrors int for _, name := range pluginNames { - plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), name) + indexName, pluginName := pathutil.CanonicalPluginName(name) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName) if err != nil { if !os.IsNotExist(err) { return errors.Wrapf(err, "failed to load the plugin manifest for plugin %s", name) @@ -74,7 +77,7 @@ kubectl krew upgrade foo bar"`, if err == nil { fmt.Fprintf(os.Stderr, "Upgrading plugin: %s\n", name) - err = installation.Upgrade(paths, plugin) + err = installation.Upgrade(paths, receipt.New(plugin, indexName)) if ignoreUpgraded && err == installation.ErrIsAlreadyUpgraded { fmt.Fprintf(os.Stderr, "Skipping plugin %s, it is already on the newest version\n", name) continue diff --git a/internal/installation/install.go b/internal/installation/install.go index 4d7ab8ca..963d1a53 100644 --- a/internal/installation/install.go +++ b/internal/installation/install.go @@ -54,7 +54,7 @@ var ( // Install will download and install a plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Install(p environment.Paths, plugin index.Plugin, opts InstallOpts) error { +func Install(p environment.Paths, plugin index.Receipt, opts InstallOpts) error { klog.V(2).Infof("Looking for installed versions") _, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err == nil { diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go index 6ebd5a30..b49a7e67 100644 --- a/internal/installation/receipt/receipt.go +++ b/internal/installation/receipt/receipt.go @@ -26,7 +26,7 @@ import ( // Store saves the given plugin receipt at the destination. // The caller has to ensure that the destination directory exists. -func Store(plugin index.Plugin, dest string) error { +func Store(plugin index.Receipt, dest string) error { yamlBytes, err := yaml.Marshal(plugin) if err != nil { return errors.Wrapf(err, "convert to yaml") @@ -41,3 +41,14 @@ func Store(plugin index.Plugin, dest string) error { func Load(path string) (index.Receipt, error) { return indexscanner.ReadReceiptFromFile(path) } + +func New(plugin index.Plugin, indexName string) index.Receipt { + return index.Receipt{ + Plugin: plugin, + Status: index.ReceiptStatus{ + Source: index.SourceIndex{ + Name: indexName, + }, + }, + } +} diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go index 01f98da5..9f6bfed3 100644 --- a/internal/installation/receipt/receipt_test.go +++ b/internal/installation/receipt/receipt_test.go @@ -32,7 +32,7 @@ func TestStore(t *testing.T) { testReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() dest := tmpDir.Path("some-plugin.yaml") - if err := Store(testPlugin, dest); err != nil { + if err := Store(testReceipt, dest); err != nil { t.Fatal(err) } @@ -51,7 +51,8 @@ func TestLoad(t *testing.T) { defer cleanup() testPlugin := testutil.NewPlugin().WithName("foo").WithPlatforms(testutil.NewPlatform().V()).V() - if err := Store(testPlugin, tmpDir.Path("foo.yaml")); err != nil { + testPluginReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() + if err := Store(testPluginReceipt, tmpDir.Path("foo.yaml")); err != nil { t.Fatal(err) } @@ -59,7 +60,6 @@ func TestLoad(t *testing.T) { if err != nil { t.Fatal(err) } - testPluginReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() if diff := cmp.Diff(&gotPlugin, &testPluginReceipt); diff != "" { t.Fatal(diff) } diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go index 6d18426b..489ee02c 100644 --- a/internal/installation/upgrade.go +++ b/internal/installation/upgrade.go @@ -29,7 +29,7 @@ import ( // Upgrade will reinstall and delete the old plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Upgrade(p environment.Paths, plugin index.Plugin) error { +func Upgrade(p environment.Paths, plugin index.Receipt) error { installReceipt, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err != nil { return errors.Wrapf(err, "failed to load install receipt for plugin %q", plugin.Name) @@ -92,7 +92,7 @@ func Upgrade(p environment.Paths, plugin index.Plugin) error { // Krew on Windows needs special care because active directories can't be // deleted. This method will mark old krew versions and during next run clean // the directory. -func cleanupInstallation(p environment.Paths, plugin index.Plugin, oldVersion string) error { +func cleanupInstallation(p environment.Paths, plugin index.Receipt, oldVersion string) error { if plugin.Name == constants.KrewPluginName && IsWindows() { klog.V(1).Infof("not removing old version of krew during upgrade on windows (should be cleaned up on the next run)") return nil From 6eaaf35a29f3675fc50b2f2f221e44ee5ba30c64 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Sat, 21 Mar 2020 11:53:18 -0700 Subject: [PATCH 2/8] Add godoc to receipt function --- internal/installation/receipt/receipt.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go index b49a7e67..ff14ca61 100644 --- a/internal/installation/receipt/receipt.go +++ b/internal/installation/receipt/receipt.go @@ -42,6 +42,7 @@ func Load(path string) (index.Receipt, error) { return indexscanner.ReadReceiptFromFile(path) } +// New returns a new receipt with the given plugin and index name. func New(plugin index.Plugin, indexName string) index.Receipt { return index.Receipt{ Plugin: plugin, From d79dbb5479c28e9fc5f2e8c8fd0fa70b58f02375 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Sat, 21 Mar 2020 11:59:10 -0700 Subject: [PATCH 3/8] Add simple test --- internal/installation/receipt/receipt_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go index 9f6bfed3..6c44bae0 100644 --- a/internal/installation/receipt/receipt_test.go +++ b/internal/installation/receipt/receipt_test.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/krew/internal/index/indexscanner" "sigs.k8s.io/krew/internal/testutil" + "sigs.k8s.io/krew/pkg/constants" ) func TestStore(t *testing.T) { @@ -71,3 +72,13 @@ func TestLoad_preservesNonExistsError(t *testing.T) { t.Fatalf("returned error is not ENOENT: %+v", err) } } + +func TestNew(t *testing.T) { + testPlugin := testutil.NewPlugin().WithName("foo").WithPlatforms(testutil.NewPlatform().V()).V() + wantReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() + + gotReceipt := New(testPlugin, constants.DefaultIndexName) + if diff := cmp.Diff(gotReceipt, wantReceipt); diff != "" { + t.Fatalf("expected receipts to match: %s", diff) + } +} From 7ec4a79c0e2214fddc23e8cc6f807121a84d5521 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Sat, 21 Mar 2020 12:07:10 -0700 Subject: [PATCH 4/8] Update some variable names to make more sense --- cmd/krew/cmd/install.go | 9 +++++---- internal/installation/install.go | 5 +++-- internal/installation/upgrade.go | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 1e8c45b4..2c655ee8 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -123,15 +123,16 @@ Remarks: return cmd.Help() } - for _, plugin := range install { - klog.V(2).Infof("Will install plugin: %s\n", plugin.Name) + for _, pluginReceipt := range install { + klog.V(2).Infof("Will install plugin: %s\n", pluginReceipt.Name) } var failed []string var returnErr error - for _, plugin := range install { + for _, pluginReceipt := range install { + plugin := pluginReceipt.Plugin fmt.Fprintf(os.Stderr, "Installing plugin: %s\n", plugin.Name) - err := installation.Install(paths, plugin, installation.InstallOpts{ + err := installation.Install(paths, pluginReceipt, installation.InstallOpts{ ArchiveFileOverride: *archiveFileOverride, }) if err == installation.ErrIsAlreadyInstalled { diff --git a/internal/installation/install.go b/internal/installation/install.go index 963d1a53..b5d520bd 100644 --- a/internal/installation/install.go +++ b/internal/installation/install.go @@ -54,7 +54,8 @@ var ( // Install will download and install a plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Install(p environment.Paths, plugin index.Receipt, opts InstallOpts) error { +func Install(p environment.Paths, pluginReceipt index.Receipt, opts InstallOpts) error { + plugin := pluginReceipt.Plugin klog.V(2).Infof("Looking for installed versions") _, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err == nil { @@ -85,7 +86,7 @@ func Install(p environment.Paths, plugin index.Receipt, opts InstallOpts) error return errors.Wrap(err, "install failed") } klog.V(3).Infof("Storing install receipt for plugin %s", plugin.Name) - err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name)) + err = receipt.Store(pluginReceipt, p.PluginInstallReceiptPath(plugin.Name)) return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail") } diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go index 489ee02c..479eb889 100644 --- a/internal/installation/upgrade.go +++ b/internal/installation/upgrade.go @@ -29,7 +29,8 @@ import ( // Upgrade will reinstall and delete the old plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Upgrade(p environment.Paths, plugin index.Receipt) error { +func Upgrade(p environment.Paths, pluginReceipt index.Receipt) error { + plugin := pluginReceipt.Plugin installReceipt, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err != nil { return errors.Wrapf(err, "failed to load install receipt for plugin %q", plugin.Name) @@ -78,7 +79,7 @@ func Upgrade(p environment.Paths, plugin index.Receipt) error { } klog.V(2).Infof("Upgrading install receipt for plugin %s", plugin.Name) - if err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name)); err != nil { + if err = receipt.Store(pluginReceipt, p.PluginInstallReceiptPath(plugin.Name)); err != nil { return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail") } @@ -92,7 +93,7 @@ func Upgrade(p environment.Paths, plugin index.Receipt) error { // Krew on Windows needs special care because active directories can't be // deleted. This method will mark old krew versions and during next run clean // the directory. -func cleanupInstallation(p environment.Paths, plugin index.Receipt, oldVersion string) error { +func cleanupInstallation(p environment.Paths, plugin index.Plugin, oldVersion string) error { if plugin.Name == constants.KrewPluginName && IsWindows() { klog.V(1).Infof("not removing old version of krew during upgrade on windows (should be cleaned up on the next run)") return nil From d7cb1082cf191974d029dfd28185549749616ad0 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Sat, 21 Mar 2020 12:16:44 -0700 Subject: [PATCH 5/8] Use default index name in new receipts --- internal/installation/receipt/receipt.go | 7 ++++++- internal/installation/receipt/receipt_test.go | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go index ff14ca61..c2e92543 100644 --- a/internal/installation/receipt/receipt.go +++ b/internal/installation/receipt/receipt.go @@ -21,6 +21,7 @@ import ( "sigs.k8s.io/yaml" "sigs.k8s.io/krew/internal/index/indexscanner" + "sigs.k8s.io/krew/pkg/constants" "sigs.k8s.io/krew/pkg/index" ) @@ -44,11 +45,15 @@ func Load(path string) (index.Receipt, error) { // New returns a new receipt with the given plugin and index name. func New(plugin index.Plugin, indexName string) index.Receipt { + name := indexName + if name == "" { + name = constants.DefaultIndexName + } return index.Receipt{ Plugin: plugin, Status: index.ReceiptStatus{ Source: index.SourceIndex{ - Name: indexName, + Name: name, }, }, } diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go index 6c44bae0..8e7881ff 100644 --- a/internal/installation/receipt/receipt_test.go +++ b/internal/installation/receipt/receipt_test.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/krew/internal/index/indexscanner" "sigs.k8s.io/krew/internal/testutil" - "sigs.k8s.io/krew/pkg/constants" ) func TestStore(t *testing.T) { @@ -77,7 +76,7 @@ func TestNew(t *testing.T) { testPlugin := testutil.NewPlugin().WithName("foo").WithPlatforms(testutil.NewPlatform().V()).V() wantReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() - gotReceipt := New(testPlugin, constants.DefaultIndexName) + gotReceipt := New(testPlugin, "") if diff := cmp.Diff(gotReceipt, wantReceipt); diff != "" { t.Fatalf("expected receipts to match: %s", diff) } From fd6b506624238ca736b6f879f88320d0b034b77c Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Sat, 21 Mar 2020 12:30:57 -0700 Subject: [PATCH 6/8] Use index name in plugin lookup --- cmd/krew/cmd/install.go | 2 +- cmd/krew/cmd/upgrade.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 2c655ee8..06947071 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -95,7 +95,7 @@ Remarks: var install []index.Receipt for _, name := range pluginNames { indexName, pluginName := pathutil.CanonicalPluginName(name) - plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(indexName), pluginName) if err != nil { if os.IsNotExist(err) { return errors.Errorf("plugin %q does not exist in the plugin index", name) diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go index 0abcfa3e..238516d3 100644 --- a/cmd/krew/cmd/upgrade.go +++ b/cmd/krew/cmd/upgrade.go @@ -27,7 +27,6 @@ import ( "sigs.k8s.io/krew/internal/installation" "sigs.k8s.io/krew/internal/installation/receipt" "sigs.k8s.io/krew/internal/pathutil" - "sigs.k8s.io/krew/pkg/constants" ) func init() { @@ -66,7 +65,7 @@ kubectl krew upgrade foo bar"`, var nErrors int for _, name := range pluginNames { indexName, pluginName := pathutil.CanonicalPluginName(name) - plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(constants.DefaultIndexName), pluginName) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(indexName), pluginName) if err != nil { if !os.IsNotExist(err) { return errors.Wrapf(err, "failed to load the plugin manifest for plugin %s", name) From 78c9800e9fe6dcedba403fd9b5c04ccd17b772d3 Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Wed, 25 Mar 2020 13:10:24 -0700 Subject: [PATCH 7/8] Code review changes --- cmd/krew/cmd/install.go | 33 +++++++++++++------ cmd/krew/cmd/upgrade.go | 3 +- internal/installation/install.go | 11 +++++-- internal/installation/receipt/receipt.go | 13 +++----- internal/installation/receipt/receipt_test.go | 3 +- internal/installation/upgrade.go | 5 ++- 6 files changed, 40 insertions(+), 28 deletions(-) diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 06947071..e8999111 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -27,12 +27,16 @@ 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/internal/installation/receipt" "sigs.k8s.io/krew/internal/pathutil" "sigs.k8s.io/krew/pkg/constants" "sigs.k8s.io/krew/pkg/index" ) +type pluginEntry struct { + p index.Plugin + indexName string +} + func init() { var ( manifest, manifestURL, archiveFileOverride *string @@ -92,7 +96,7 @@ Remarks: return errors.New("--archive can be specified only with --manifest or --manifest-url") } - var install []index.Receipt + var install []pluginEntry for _, name := range pluginNames { indexName, pluginName := pathutil.CanonicalPluginName(name) plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(indexName), pluginName) @@ -102,7 +106,10 @@ Remarks: } return errors.Wrapf(err, "failed to load plugin %q from the index", name) } - install = append(install, receipt.New(plugin, indexName)) + install = append(install, pluginEntry{ + p: plugin, + indexName: indexName, + }) } if *manifest != "" { @@ -110,29 +117,35 @@ Remarks: if err != nil { return errors.Wrap(err, "failed to load plugin manifest from file") } - install = append(install, receipt.New(plugin, constants.DefaultIndexName)) + install = append(install, pluginEntry{ + p: plugin, + indexName: constants.DefaultIndexName, + }) } else if *manifestURL != "" { plugin, err := readPluginFromURL(*manifestURL) if err != nil { return errors.Wrap(err, "failed to read plugin manifest file from url") } - install = append(install, receipt.New(plugin, constants.DefaultIndexName)) + install = append(install, pluginEntry{ + p: plugin, + indexName: constants.DefaultIndexName, + }) } if len(install) == 0 { return cmd.Help() } - for _, pluginReceipt := range install { - klog.V(2).Infof("Will install plugin: %s\n", pluginReceipt.Name) + for _, pluginEntry := range install { + klog.V(2).Infof("Will install plugin: %s\n", pluginEntry.p.Name) } var failed []string var returnErr error - for _, pluginReceipt := range install { - plugin := pluginReceipt.Plugin + for _, entry := range install { + plugin := entry.p fmt.Fprintf(os.Stderr, "Installing plugin: %s\n", plugin.Name) - err := installation.Install(paths, pluginReceipt, installation.InstallOpts{ + err := installation.Install(paths, plugin, entry.indexName, installation.InstallOpts{ ArchiveFileOverride: *archiveFileOverride, }) if err == installation.ErrIsAlreadyInstalled { diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go index 238516d3..8df98060 100644 --- a/cmd/krew/cmd/upgrade.go +++ b/cmd/krew/cmd/upgrade.go @@ -25,7 +25,6 @@ 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/internal/installation/receipt" "sigs.k8s.io/krew/internal/pathutil" ) @@ -76,7 +75,7 @@ kubectl krew upgrade foo bar"`, if err == nil { fmt.Fprintf(os.Stderr, "Upgrading plugin: %s\n", name) - err = installation.Upgrade(paths, receipt.New(plugin, indexName)) + err = installation.Upgrade(paths, plugin, indexName) if ignoreUpgraded && err == installation.ErrIsAlreadyUpgraded { fmt.Fprintf(os.Stderr, "Skipping plugin %s, it is already on the newest version\n", name) continue diff --git a/internal/installation/install.go b/internal/installation/install.go index b5d520bd..c7c61978 100644 --- a/internal/installation/install.go +++ b/internal/installation/install.go @@ -45,6 +45,12 @@ type installOperation struct { binDir string } +// PluginEntry describes a plugin and the index it comes from. +type PluginEntry struct { + Plugin index.Plugin + IndexName string +} + // Plugin lifecycle errors var ( ErrIsAlreadyInstalled = errors.New("can't install, the newest version is already installed") @@ -54,8 +60,7 @@ var ( // Install will download and install a plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Install(p environment.Paths, pluginReceipt index.Receipt, opts InstallOpts) error { - plugin := pluginReceipt.Plugin +func Install(p environment.Paths, plugin index.Plugin, indexName string, opts InstallOpts) error { klog.V(2).Infof("Looking for installed versions") _, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err == nil { @@ -86,7 +91,7 @@ func Install(p environment.Paths, pluginReceipt index.Receipt, opts InstallOpts) return errors.Wrap(err, "install failed") } klog.V(3).Infof("Storing install receipt for plugin %s", plugin.Name) - err = receipt.Store(pluginReceipt, p.PluginInstallReceiptPath(plugin.Name)) + err = receipt.Store(receipt.New(plugin, indexName), p.PluginInstallReceiptPath(plugin.Name)) return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail") } diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go index c2e92543..83a27351 100644 --- a/internal/installation/receipt/receipt.go +++ b/internal/installation/receipt/receipt.go @@ -21,14 +21,13 @@ import ( "sigs.k8s.io/yaml" "sigs.k8s.io/krew/internal/index/indexscanner" - "sigs.k8s.io/krew/pkg/constants" "sigs.k8s.io/krew/pkg/index" ) -// Store saves the given plugin receipt at the destination. +// Store saves the given receipt at the destination. // The caller has to ensure that the destination directory exists. -func Store(plugin index.Receipt, dest string) error { - yamlBytes, err := yaml.Marshal(plugin) +func Store(receipt index.Receipt, dest string) error { + yamlBytes, err := yaml.Marshal(receipt) if err != nil { return errors.Wrapf(err, "convert to yaml") } @@ -45,15 +44,11 @@ func Load(path string) (index.Receipt, error) { // New returns a new receipt with the given plugin and index name. func New(plugin index.Plugin, indexName string) index.Receipt { - name := indexName - if name == "" { - name = constants.DefaultIndexName - } return index.Receipt{ Plugin: plugin, Status: index.ReceiptStatus{ Source: index.SourceIndex{ - Name: name, + Name: indexName, }, }, } diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go index 8e7881ff..6c44bae0 100644 --- a/internal/installation/receipt/receipt_test.go +++ b/internal/installation/receipt/receipt_test.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/krew/internal/index/indexscanner" "sigs.k8s.io/krew/internal/testutil" + "sigs.k8s.io/krew/pkg/constants" ) func TestStore(t *testing.T) { @@ -76,7 +77,7 @@ func TestNew(t *testing.T) { testPlugin := testutil.NewPlugin().WithName("foo").WithPlatforms(testutil.NewPlatform().V()).V() wantReceipt := testutil.NewReceipt().WithPlugin(testPlugin).V() - gotReceipt := New(testPlugin, "") + gotReceipt := New(testPlugin, constants.DefaultIndexName) if diff := cmp.Diff(gotReceipt, wantReceipt); diff != "" { t.Fatalf("expected receipts to match: %s", diff) } diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go index 479eb889..bac6d32a 100644 --- a/internal/installation/upgrade.go +++ b/internal/installation/upgrade.go @@ -29,8 +29,7 @@ import ( // Upgrade will reinstall and delete the old plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. -func Upgrade(p environment.Paths, pluginReceipt index.Receipt) error { - plugin := pluginReceipt.Plugin +func Upgrade(p environment.Paths, plugin index.Plugin, indexName string) error { installReceipt, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err != nil { return errors.Wrapf(err, "failed to load install receipt for plugin %q", plugin.Name) @@ -79,7 +78,7 @@ func Upgrade(p environment.Paths, pluginReceipt index.Receipt) error { } klog.V(2).Infof("Upgrading install receipt for plugin %s", plugin.Name) - if err = receipt.Store(pluginReceipt, p.PluginInstallReceiptPath(plugin.Name)); err != nil { + if err = receipt.Store(receipt.New(plugin, indexName), p.PluginInstallReceiptPath(plugin.Name)); err != nil { return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail") } From fcc6505fa783e056b2a550f3888870422800d20d Mon Sep 17 00:00:00 2001 From: Chris Kim Date: Thu, 26 Mar 2020 22:52:48 -0700 Subject: [PATCH 8/8] Add integration test --- cmd/krew/cmd/install.go | 2 +- integration_test/install_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index e8999111..4dd1e66a 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -137,7 +137,7 @@ Remarks: } for _, pluginEntry := range install { - klog.V(2).Infof("Will install plugin: %s\n", pluginEntry.p.Name) + klog.V(2).Infof("Will install plugin: %s/%s\n", pluginEntry.indexName, pluginEntry.p.Name) } var failed []string diff --git a/integration_test/install_test.go b/integration_test/install_test.go index 21c75898..06d9b9fc 100644 --- a/integration_test/install_test.go +++ b/integration_test/install_test.go @@ -95,6 +95,33 @@ func TestKrewInstall_StdinAndPositionalArguments(t *testing.T) { test.AssertExecutableNotInPATH("kubectl-" + validPlugin2) } +func TestKrewInstall_ExplicitDefaultIndex(t *testing.T) { + skipShort(t) + + test, cleanup := NewTest(t) + defer cleanup() + + test.Krew("install", "default/"+validPlugin).RunOrFail() + test.AssertExecutableInPATH("kubectl-" + validPlugin) +} + +func TestKrewInstall_CustomIndex(t *testing.T) { + skipShort(t) + + test, cleanup := NewTest(t) + defer cleanup() + + test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex() + test.Krew("index", "add", "foo", test.TempDir().Path("index/"+constants.DefaultIndexName)).RunOrFail() + test.Krew("install", "foo/"+validPlugin).RunOrFail() + test.AssertExecutableInPATH("kubectl-" + validPlugin) + + if err := test.Krew("install", "invalid/"+validPlugin2).Run(); err == nil { + t.Fatal("expected install from invalid index to fail") + } + test.AssertExecutableNotInPATH("kubectl-" + validPlugin2) +} + func TestKrewInstall_Manifest(t *testing.T) { skipShort(t)