diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 66b7fd9518..2b230ff472 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -17,6 +17,7 @@ package cmd import ( "bufio" "fmt" + "net/http" "os" "github.com/pkg/errors" @@ -25,15 +26,14 @@ import ( "sigs.k8s.io/krew/cmd/krew/cmd/internal" "sigs.k8s.io/krew/internal/index/indexscanner" - "sigs.k8s.io/krew/internal/index/validation" "sigs.k8s.io/krew/internal/installation" "sigs.k8s.io/krew/pkg/index" ) func init() { var ( - manifest, archiveFileOverride *string - noUpdateIndex *bool + manifest, manifestURL, archiveFileOverride *string + noUpdateIndex *bool ) // installCmd represents the install command @@ -49,9 +49,9 @@ Examples: To install plugins from a newline-delimited file, run: kubectl krew install < file.txt - (For developers) To provide a custom plugin manifest, use the --manifest - argument. Similarly, instead of downloading files from a URL, you can specify a - local --archive file: + (For developers) To provide a custom plugin manifest, use the --manifest or + --manifest-url arguments. Similarly, instead of downloading files from a URL, + you can specify a local --archive file: kubectl krew install --manifest=FILE [--archive=FILE] Remarks: @@ -77,17 +77,21 @@ Remarks: } } - if len(pluginNames) != 0 && *manifest != "" { - return errors.New("must specify either specify either plugin names (via positional arguments or STDIN), or --manifest; not both") + if *manifest != "" && *manifestURL != "" { + return errors.New("cannot specify --manifest and --manifest-url at the same time") } - if *archiveFileOverride != "" && *manifest == "" { - return errors.New("--archive can be specified only with --manifest") + if len(pluginNames) != 0 && (*manifest != "" || *manifestURL != "") { + return errors.New("must specify either specify either plugin names (via positional arguments or STDIN), or --manifest/--manifest-url; not both") + } + + if *archiveFileOverride != "" && *manifest == "" && *manifestURL == "" { + return errors.New("--archive can be specified only with --manifest or --manifest-url") } var install []index.Plugin for _, name := range pluginNames { - plugin, err := indexscanner.LoadPluginFileFromFS(paths.IndexPluginsPath(), name) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(), name) if err != nil { if os.IsNotExist(err) { return errors.Errorf("plugin %q does not exist in the plugin index", name) @@ -98,12 +102,15 @@ Remarks: } if *manifest != "" { - plugin, err := indexscanner.ReadPluginFile(*manifest) + plugin, err := indexscanner.ReadPluginFromFile(*manifest) if err != nil { - return errors.Wrap(err, "failed to load custom manifest file") + return errors.Wrap(err, "failed to load plugin manifest from file") } - if err := validation.ValidatePlugin(plugin.Name, plugin); err != nil { - return errors.Wrap(err, "plugin manifest validation error") + install = append(install, plugin) + } 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) } @@ -164,9 +171,23 @@ Remarks: }, } - manifest = installCmd.Flags().String("manifest", "", "(Development-only) specify plugin manifest directly.") + manifest = installCmd.Flags().String("manifest", "", "(Development-only) specify local plugin manifest file") + manifestURL = installCmd.Flags().String("manifest-url", "", "(Development-only) specify plugin manifest file from url") archiveFileOverride = installCmd.Flags().String("archive", "", "(Development-only) force all downloads to use the specified file") noUpdateIndex = installCmd.Flags().Bool("no-update-index", false, "(Experimental) do not update local copy of plugin index before installing") rootCmd.AddCommand(installCmd) } + +func readPluginFromURL(url string) (index.Plugin, error) { + klog.V(4).Infof("downloading manifest from url %s", url) + resp, err := http.Get(url) + if err != nil { + return index.Plugin{}, errors.Wrapf(err, "request to url failed (%s)", url) + } + klog.V(4).Infof("manifest downloaded from url, status=%v headers=%v", resp.Status, resp.Header) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return index.Plugin{}, errors.Errorf("unexpected status code (http %d) from url", resp.StatusCode) + } + return indexscanner.ReadPlugin(resp.Body) +} diff --git a/cmd/krew/cmd/install_test.go b/cmd/krew/cmd/install_test.go new file mode 100644 index 0000000000..69916faf0d --- /dev/null +++ b/cmd/krew/cmd/install_test.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 cmd + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "testing" +) + +func Test_readPluginFromURL(t *testing.T) { + server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join("../../../integration_test/testdata")))) + defer server.Close() + + tests := []struct { + name string + url string + wantName string + wantErr bool + }{ + { + name: "successful request", + url: server.URL + "/konfig.yaml", + wantName: "konfig", + }, + { + name: "invalid server", + url: "http://example.invalid:80/foo.yaml", + wantErr: true, + }, + { + name: "bad response", + url: server.URL + "/404.yaml", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := readPluginFromURL(tt.url) + if (err != nil) != tt.wantErr { + t.Fatalf("readPluginFromURL() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got.Name != tt.wantName { + t.Fatalf("readPluginFromURL() returned name=%v; want=%v", got.Name, tt.wantName) + } + }) + } +} diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go index d00415f8a7..98a8ef5cf4 100644 --- a/cmd/krew/cmd/upgrade.go +++ b/cmd/krew/cmd/upgrade.go @@ -62,7 +62,7 @@ kubectl krew upgrade foo bar"`, var nErrors int for _, name := range pluginNames { - plugin, err := indexscanner.LoadPluginFileFromFS(paths.IndexPluginsPath(), name) + plugin, err := indexscanner.LoadPluginByName(paths.IndexPluginsPath(), name) if err != nil { if os.IsNotExist(err) { return errors.Errorf("plugin %q does not exist in the plugin index", name) diff --git a/cmd/validate-krew-manifest/main.go b/cmd/validate-krew-manifest/main.go index 513cca10a8..d0b749a499 100644 --- a/cmd/validate-krew-manifest/main.go +++ b/cmd/validate-krew-manifest/main.go @@ -64,7 +64,7 @@ func main() { func validateManifestFile(path string) error { klog.V(4).Infof("reading file %s", path) - p, err := indexscanner.ReadPluginFile(path) + p, err := indexscanner.ReadPluginFromFile(path) if err != nil { return errors.Wrap(err, "failed to read plugin file") } diff --git a/integration_test/install_test.go b/integration_test/install_test.go index cb0119c77f..21c75898d0 100644 --- a/integration_test/install_test.go +++ b/integration_test/install_test.go @@ -15,6 +15,8 @@ package integrationtest import ( + "net/http" + "net/http/httptest" "path/filepath" "strings" "testing" @@ -105,6 +107,20 @@ func TestKrewInstall_Manifest(t *testing.T) { test.AssertExecutableInPATH("kubectl-" + validPlugin) } +func TestKrewInstall_ManifestURL(t *testing.T) { + skipShort(t) + + test, cleanup := NewTest(t) + defer cleanup() + srv, shutdown := localTestServer() + defer shutdown() + + test.Krew("install", + "--manifest-url", srv+"/"+validPlugin+constants.ManifestExtension). + RunOrFail() + test.AssertExecutableInPATH("kubectl-" + validPlugin) +} + func TestKrewInstall_ManifestAndArchive(t *testing.T) { skipShort(t) @@ -132,7 +148,23 @@ func TestKrewInstall_OnlyArchive(t *testing.T) { } } -func TestKrewInstall_PositionalArgumentsAndManifest(t *testing.T) { +func TestKrewInstall_ManifestArgsAreMutuallyExclusive(t *testing.T) { + skipShort(t) + + test, cleanup := NewTest(t) + defer cleanup() + srv, shutdown := localTestServer() + defer shutdown() + + if err := test.Krew("install", + "--manifest", filepath.Join("testdata", fooPlugin+constants.ManifestExtension), + "--manifest-url", srv+"/"+validPlugin+constants.ManifestExtension). + Run(); err == nil { + t.Fatal("expected mutually exclusive arguments to cause failure") + } +} + +func TestKrewInstall_NoManifestArgsWhenPositionalArgsSpecified(t *testing.T) { skipShort(t) test, cleanup := NewTest(t) @@ -143,6 +175,17 @@ func TestKrewInstall_PositionalArgumentsAndManifest(t *testing.T) { "--archive", filepath.Join("testdata", fooPlugin+".tar.gz")). Run() if err == nil { - t.Fatal("expected failure") + t.Fatal("expected failure when positional args and --manifest specified") + } + + err = test.Krew("install", validPlugin, + "--manifest-url", filepath.Join("testdata", fooPlugin+constants.ManifestExtension)).Run() + if err == nil { + t.Fatal("expected failure when positional args and --manifest-url specified") } } + +func localTestServer() (string, func()) { + s := httptest.NewServer(http.FileServer(http.Dir("testdata"))) + return s.URL, s.Close +} diff --git a/internal/index/indexscanner/scanner.go b/internal/index/indexscanner/scanner.go index 6a43145068..e8cec59936 100644 --- a/internal/index/indexscanner/scanner.go +++ b/internal/index/indexscanner/scanner.go @@ -60,7 +60,7 @@ func LoadPluginListFromFS(indexDir string) ([]index.Plugin, error) { list := make([]index.Plugin, 0, len(files)) for _, file := range files { pluginName := strings.TrimSuffix(file, filepath.Ext(file)) - p, err := LoadPluginFileFromFS(indexDir, pluginName) + p, err := LoadPluginByName(indexDir, pluginName) if err != nil { // Index loading shouldn't fail because of one plugin. // Show error instead. @@ -72,38 +72,37 @@ func LoadPluginListFromFS(indexDir string) ([]index.Plugin, error) { return list, nil } -// LoadPluginFileFromFS loads a plugins index file by its name. When plugin +// LoadPluginByName loads a plugins index file by its name. When plugin // file not found, it returns an error that can be checked with os.IsNotExist. -func LoadPluginFileFromFS(pluginsDir, pluginName string) (index.Plugin, error) { +func LoadPluginByName(pluginsDir, pluginName string) (index.Plugin, error) { if !validation.IsSafePluginName(pluginName) { return index.Plugin{}, errors.Errorf("plugin name %q not allowed", pluginName) } klog.V(4).Infof("Reading plugin %q", pluginName) - pluginsDir, err := filepath.EvalSymlinks(pluginsDir) - if err != nil { - return index.Plugin{}, err - } - p, err := ReadPluginFile(filepath.Join(pluginsDir, pluginName+constants.ManifestExtension)) - if os.IsNotExist(err) { - return index.Plugin{}, err - } else if err != nil { - return index.Plugin{}, errors.Wrap(err, "failed to read the plugin manifest") - } - return p, validation.ValidatePlugin(pluginName, p) + return ReadPluginFromFile(filepath.Join(pluginsDir, pluginName+constants.ManifestExtension)) } -// ReadPluginFile loads a file from the FS. When plugin file not found, it +// ReadPluginFromFile loads a file from the FS. When plugin file not found, it // returns an error that can be checked with os.IsNotExist. -func ReadPluginFile(indexFilePath string) (index.Plugin, error) { - f, err := os.Open(indexFilePath) +func ReadPluginFromFile(path string) (index.Plugin, error) { + f, err := os.Open(path) if os.IsNotExist(err) { + // TODO(ahmetb): we should use go1.13+ errors.Is construct at call sites to evaluate if an error is os.IsNotExist return index.Plugin{}, err } else if err != nil { return index.Plugin{}, errors.Wrap(err, "failed to open index file") } + return ReadPlugin(f) +} + +func ReadPlugin(f io.ReadCloser) (index.Plugin, error) { defer f.Close() - return DecodePluginFile(f) + p, err := DecodePluginFile(f) + if err != nil { + return p, errors.Wrap(err, "failed to decode plugin manifest") + } + return p, errors.Wrap(validation.ValidatePlugin(p.Name, p), "plugin manifest validation error") } // DecodePluginFile tries to decodes a plugin manifest from r. diff --git a/internal/index/indexscanner/scanner_test.go b/internal/index/indexscanner/scanner_test.go index 25683c2327..de6da1a2ed 100644 --- a/internal/index/indexscanner/scanner_test.go +++ b/internal/index/indexscanner/scanner_test.go @@ -23,7 +23,7 @@ import ( "k8s.io/apimachinery/pkg/labels" ) -func Test_readIndexFile(t *testing.T) { +func TestReadPluginFile(t *testing.T) { type args struct { indexFilePath string } @@ -64,32 +64,42 @@ func Test_readIndexFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ReadPluginFile(tt.args.indexFilePath) + got, err := ReadPluginFromFile(tt.args.indexFilePath) if (err != nil) != tt.wantErr { - t.Errorf("readIndexFile() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("ReadPluginFromFile() error = %v, wantErr %v", err, tt.wantErr) return } if err != nil { return } if got.Name != "foo" && got.Kind != "Plugin" { - t.Errorf("readIndexFile() has not parsed the metainformations %v", got) + t.Errorf("ReadPluginFromFile() has not parsed the metainformations %v", got) return } sel, err := metav1.LabelSelectorAsSelector(got.Spec.Platforms[0].Selector) if err != nil { - t.Errorf("readIndexFile() error parsing label err: %v", err) + t.Errorf("ReadPluginFromFile() error parsing label err: %v", err) return } if !sel.Matches(tt.matchFirst) || sel.Matches(neverMatch) { - t.Errorf("readIndexFile() didn't parse label selector properly: %##v", sel) + t.Errorf("ReadPluginFromFile() didn't parse label selector properly: %##v", sel) return } }) } } +func TestReadPluginFile_preservesNotFoundErr(t *testing.T) { + _, err := ReadPluginFromFile(filepath.Join(testdataPath(t), "does-not-exist.yaml")) + if err == nil { + t.Fatal("expected error") + } + if !os.IsNotExist(err) { + t.Fatalf("returned error is not IsNotExist type: %v", err) + } +} + func TestLoadIndexListFromFS(t *testing.T) { type args struct { indexDir string @@ -161,7 +171,7 @@ func TestLoadIndexFileFromFS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := LoadPluginFileFromFS(tt.args.indexDir, tt.args.pluginName) + got, err := LoadPluginByName(tt.args.indexDir, tt.args.pluginName) if (err != nil) != tt.wantErr { t.Errorf("LoadIndexFileFromFS() got = %##v,error = %v, wantErr %v", got, err, tt.wantErr) return diff --git a/internal/info/info.go b/internal/info/info.go index 01e21b480a..5fe68947b0 100644 --- a/internal/info/info.go +++ b/internal/info/info.go @@ -28,7 +28,7 @@ import ( // LoadManifestFromReceiptOrIndex tries to load a plugin manifest from the // receipts directory or from the index directory if the former fails. func LoadManifestFromReceiptOrIndex(p environment.Paths, name string) (index.Plugin, error) { - receipt, err := indexscanner.LoadPluginFileFromFS(p.InstallReceiptsPath(), name) + receipt, err := indexscanner.LoadPluginByName(p.InstallReceiptsPath(), name) if err == nil { klog.V(3).Infof("Found plugin manifest for %q in the receipts dir", name) @@ -40,5 +40,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.LoadPluginFileFromFS(p.IndexPluginsPath(), name) + return indexscanner.LoadPluginByName(p.IndexPluginsPath(), name) } diff --git a/internal/installation/receipt/receipt.go b/internal/installation/receipt/receipt.go index 4fb356c639..e4495cfa1a 100644 --- a/internal/installation/receipt/receipt.go +++ b/internal/installation/receipt/receipt.go @@ -39,5 +39,5 @@ func Store(plugin index.Plugin, dest string) error { // Load reads the plugin receipt at the specified destination. // If not found, it returns os.IsNotExist error. func Load(path string) (index.Plugin, error) { - return indexscanner.ReadPluginFile(path) + return indexscanner.ReadPluginFromFile(path) } diff --git a/internal/installation/receipt/receipt_test.go b/internal/installation/receipt/receipt_test.go index 3786edb334..f0adddc101 100644 --- a/internal/installation/receipt/receipt_test.go +++ b/internal/installation/receipt/receipt_test.go @@ -35,7 +35,7 @@ func TestStore(t *testing.T) { t.Fatal(err) } - actual, err := indexscanner.LoadPluginFileFromFS(tmpDir.Root(), "some-plugin") + actual, err := indexscanner.LoadPluginByName(tmpDir.Root(), "some-plugin") if err != nil { t.Fatal(err) }