From 54ed20153a43a5567511784751be4a349c0f6c0c Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Tue, 16 Jul 2019 22:29:47 -0700 Subject: [PATCH] Make version field required No semver validation specifically just yet. Making the field required, and planning to make it more strict later by actually parsing it. Signed-off-by: Ahmet Alp Balkan --- cmd/validate-krew-manifest/main_test.go | 2 + docs/DEVELOPER_GUIDE.md | 9 +-- docs/USER_GUIDE.md | 2 +- .../testdata/testindex/plugins/bar.yaml | 1 + .../testdata/testindex/plugins/foo.yaml | 1 + pkg/index/validate.go | 4 +- pkg/index/validate_test.go | 57 +++++++++++-------- pkg/info/info_test.go | 2 +- pkg/receipt/receipt_test.go | 19 +++---- 9 files changed, 54 insertions(+), 43 deletions(-) diff --git a/cmd/validate-krew-manifest/main_test.go b/cmd/validate-krew-manifest/main_test.go index dfac0f0e..ea74c338 100644 --- a/cmd/validate-krew-manifest/main_test.go +++ b/cmd/validate-krew-manifest/main_test.go @@ -80,6 +80,7 @@ func TestValidateManifestFile(t *testing.T) { Name: "test", }, Spec: index.PluginSpec{ + Version: "v1.0.0", ShortDescription: "test", Platforms: []index.Platform{{ URI: "http://test.com", @@ -108,6 +109,7 @@ func TestValidateManifestFile(t *testing.T) { }, Spec: index.PluginSpec{ ShortDescription: "test", + Version: "v1.0.0", Platforms: []index.Platform{ { URI: "http://test.com", diff --git a/docs/DEVELOPER_GUIDE.md b/docs/DEVELOPER_GUIDE.md index d0aa4b83..5a99ad81 100644 --- a/docs/DEVELOPER_GUIDE.md +++ b/docs/DEVELOPER_GUIDE.md @@ -94,7 +94,7 @@ kind: Plugin metadata: name: foo # plugin name must match your manifest file name (e.g. foo.yaml) spec: - version: "v0.0.1" # optional, only for documentation purposes + version: "v0.0.1" # required, must be in semver format, prefixed with "v" platforms: # specify installation script for linux and darwin (macOS) - selector: # a regular Kubernetes selector @@ -299,10 +299,11 @@ needed to run the plugin in the `caveats:` field. ### Updating existing plugins When you have a newer version of your plugin, create a new pull request that -updates `uri` and `sha256` fields of the plugin manifest file. +updates thew `version`, `uri` and `sha256` fields of the plugin manifest file. -Optionally, you can use the `version` field to match to your plugin's released -version string. +Ideally, the `version` specified should match the release tag of the plugin. +This helps users and maintainers to easily identify which version of the plugin +they have installed. [index]: https://github.com/kubernetes-sigs/krew-index [plugins]: https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins/ diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 7ab3b90b..f7d8d389 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -48,7 +48,7 @@ SHA256: 8be8ed348d02285abc46bbf7a4cc83da0ee9d54dc2c5bf86a7b64947811b843c DESCRIPTION:  Pretty print the current cluster certificate.  The plugin formats the certificate in PEM following RFC1421. -VERSION: master +VERSION: v1.0.0 CAVEATS:  This plugin needs the following programs:  * base64 diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml index 253af587..2cc27031 100644 --- a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml +++ b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml @@ -17,6 +17,7 @@ kind: Plugin metadata: name: bar spec: + version: v1.0.0 platforms: - files: - from: "*" diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml index 79aa7207..8c66beee 100644 --- a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml +++ b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml @@ -17,6 +17,7 @@ kind: Plugin metadata: name: foo spec: + version: v1.0.0 platforms: - files: - from: "*" diff --git a/pkg/index/validate.go b/pkg/index/validate.go index 09c8fbb4..1551024b 100644 --- a/pkg/index/validate.go +++ b/pkg/index/validate.go @@ -58,7 +58,6 @@ func (p Plugin) Validate(name string) error { if p.Kind != constants.PluginKind { return errors.Errorf("plugin manifest has kind=%q, but only %q is supported", p.Kind, constants.PluginKind) } - if !IsSafePluginName(name) { return errors.Errorf("the plugin name %q is not allowed, must match %q", name, safePluginRegexp.String()) } @@ -71,6 +70,9 @@ func (p Plugin) Validate(name string) error { if len(p.Spec.Platforms) == 0 { return errors.New("should have a platform specified") } + if p.Spec.Version == "" { + return errors.New("should have a version specified") + } for _, pl := range p.Spec.Platforms { if err := pl.Validate(); err != nil { return errors.Wrapf(err, "platform (%+v) is badly constructed", pl) diff --git a/pkg/index/validate_test.go b/pkg/index/validate_test.go index f5fb91f4..c81cf97b 100644 --- a/pkg/index/validate_test.go +++ b/pkg/index/validate_test.go @@ -105,7 +105,7 @@ func TestPlugin_Validate(t *testing.T) { wantErr bool }{ { - name: "validate success", + name: "success", fields: fields{ TypeMeta: metav1.TypeMeta{ APIVersion: constants.CurrentAPIVersion, @@ -113,11 +113,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", - Homepage: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -139,10 +136,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -164,11 +159,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", - Homepage: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -182,7 +174,7 @@ func TestPlugin_Validate(t *testing.T) { wantErr: true, }, { - name: "no short description", + name: "shortDescription unspecified", fields: fields{ TypeMeta: metav1.TypeMeta{ APIVersion: constants.CurrentAPIVersion, @@ -190,10 +182,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "", - Description: "", - Caveats: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -207,7 +197,7 @@ func TestPlugin_Validate(t *testing.T) { wantErr: true, }, { - name: "no file operations", + name: "version unspecified", fields: fields{ TypeMeta: metav1.TypeMeta{ APIVersion: constants.CurrentAPIVersion, @@ -217,8 +207,29 @@ func TestPlugin_Validate(t *testing.T) { Spec: PluginSpec{ Version: "", ShortDescription: "short", - Description: "", - Caveats: "", + Platforms: []Platform{{ + URI: "http://example.com", + Sha256: "deadbeef", + Selector: nil, + Files: []FileOperation{{"", ""}}, + Bin: "foo", + }}, + }, + }, + pluginName: "foo", + wantErr: true, + }, + { + name: "no file operations", + fields: fields{ + TypeMeta: metav1.TypeMeta{ + APIVersion: constants.CurrentAPIVersion, + Kind: constants.PluginKind, + }, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: PluginSpec{ + Version: "v1.0.0", + ShortDescription: "short", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -240,10 +251,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "wrong-name"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -265,10 +274,8 @@ func TestPlugin_Validate(t *testing.T) { }, ObjectMeta: metav1.ObjectMeta{Name: "../foo"}, Spec: PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", Platforms: []Platform{{ URI: "http://example.com", Sha256: "deadbeef", diff --git a/pkg/info/info_test.go b/pkg/info/info_test.go index 9b8f889d..8aa53995 100644 --- a/pkg/info/info_test.go +++ b/pkg/info/info_test.go @@ -39,7 +39,7 @@ var ( }, ObjectMeta: metav1.ObjectMeta{Name: pluginName}, Spec: index.PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", Description: "", Caveats: "", diff --git a/pkg/receipt/receipt_test.go b/pkg/receipt/receipt_test.go index c4af6005..37bf8456 100644 --- a/pkg/receipt/receipt_test.go +++ b/pkg/receipt/receipt_test.go @@ -26,21 +26,18 @@ import ( "sigs.k8s.io/krew/pkg/testutil" ) -const pluginName = "some" +const testPluginName = "some" var ( - plugin = index.Plugin{ + testPlugin = index.Plugin{ TypeMeta: metav1.TypeMeta{ APIVersion: constants.CurrentAPIVersion, Kind: constants.PluginKind, }, - ObjectMeta: metav1.ObjectMeta{Name: pluginName}, + ObjectMeta: metav1.ObjectMeta{Name: testPluginName}, Spec: index.PluginSpec{ - Version: "", + Version: "v1.0.0", ShortDescription: "short", - Description: "", - Caveats: "", - Homepage: "", Platforms: []index.Platform{{ URI: "http://example.com", Sha256: "deadbeef", @@ -58,16 +55,16 @@ func TestStore(t *testing.T) { dest := tmpDir.Path("some.yaml") - if err := Store(plugin, dest); err != nil { + if err := Store(testPlugin, dest); err != nil { t.Error(err) } - actual, err := indexscanner.LoadPluginFileFromFS(tmpDir.Root(), pluginName) + actual, err := indexscanner.LoadPluginFileFromFS(tmpDir.Root(), testPluginName) if err != nil { t.Fatal(err) } - if diff := cmp.Diff(&plugin, &actual); diff != "" { - t.Error(diff) + if diff := cmp.Diff(&testPlugin, &actual); diff != "" { + t.Fatal(diff) } }