Repository: kubernetes-sigs/krew
Pull request #269 (2019-07-17 05:30:53)

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.

/assign @corneliusweig

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

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/269#" 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/269?src=pr&el=h1) Report
> Merging [#269](https://codecov.io/gh/kubernetes-sigs/krew/pull/269?src=pr&el=desc) into [master](https://codecov.io/gh/kubernetes-sigs/krew/commit/6016916dfe8178b95705f278516706d9fcbba650?src=pr&el=desc) will **increase** coverage by `0.1%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff            @@
##           master     #269     +/-   ##
=========================================
+ Coverage   53.76%   53.87%   +0.1%     
=========================================
  Files          18       18             
  Lines         863      865      +2     
=========================================
+ Hits          464      466      +2     
  Misses        342      342             
  Partials       57       57
```


| [Impacted Files](https://codecov.io/gh/kubernetes-sigs/krew/pull/269?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/index/validate.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/269/diff?src=pr&el=tree#diff-cGtnL2luZGV4L3ZhbGlkYXRlLmdv) | `94.73% <100%> (+0.29%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/269?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/269?src=pr&el=footer). Last update [6016916...54ed201](https://codecov.io/gh/kubernetes-sigs/krew/pull/269?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/docs/DEVELOPER_GUIDE.md b/docs/DEVELOPER_GUIDE.md
index d0aa4b8..5a99ad8 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 7ab3b90..f7d8d38 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/hack/run-lint.sh b/hack/run-lint.sh
index 983fe20..06d022c 100755
--- a/hack/run-lint.sh
+++ b/hack/run-lint.sh
@@ -18,11 +18,14 @@ set -euo pipefail
 
 [[ -n "${DEBUG:-}" ]] && set -x
 
-if ! [[ -x "$GOPATH/bin/golangci-lint" ]]
+gopath="$(go env GOPATH)"
+
+if ! [[ -x "$gopath/bin/golangci-lint" ]]
 then
-   echo 'Installing golangci-lint'
-   curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b "$GOPATH/bin" v1.16.0
+   echo >&2 'Installing golangci-lint'
+   curl --silent --fail --location \
+       https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b "$gopath/bin" v1.16.0
 fi
 
 # configured by .golangci.yml
-GO111MODULE=on "$GOPATH/bin/golangci-lint" run
+exec env GO111MODULE=on "$gopath/bin/golangci-lint" run
diff --git a/pkg/index/validate.go b/pkg/index/validate.go
index 09c8fbb..1551024 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)


Test patch:
diff --git a/cmd/validate-krew-manifest/main_test.go b/cmd/validate-krew-manifest/main_test.go
index dfac0f0..ea74c33 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/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
index 253af58..2cc2703 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 79aa720..8c66bee 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_test.go b/pkg/index/validate_test.go
index f5fb91f..c81cf97 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 9b8f889..8aa5399 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 c4af600..37bf845 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)
 	}
 }