Repository: kubernetes-sigs/krew
Pull request #290 (2019-07-22 19:16:18)

pkg/installation: unused pluginVersionFromPath
Removing unused method.

Aside: It would be good to have a lint check that validates that unexposed
methods are not used (maybe even for external methods).

/kind cleanup
/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/290#" 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":[]} -->

Patch:
diff --git a/cmd/krew/cmd/info.go b/cmd/krew/cmd/info.go
index 4ff2885..7aba59e 100644
--- a/cmd/krew/cmd/info.go
+++ b/cmd/krew/cmd/info.go
@@ -27,6 +27,7 @@ import (
 
 	"sigs.k8s.io/krew/pkg/index"
 	"sigs.k8s.io/krew/pkg/info"
+	"sigs.k8s.io/krew/pkg/installation"
 )
 
 // infoCmd represents the info command
@@ -56,7 +57,7 @@ Example:
 
 func printPluginInfo(out io.Writer, plugin index.Plugin) {
 	fmt.Fprintf(out, "NAME: %s\n", plugin.Name)
-	if platform, ok, err := plugin.Spec.GetMatchingPlatform(); err == nil && ok {
+	if platform, ok, err := installation.GetMatchingPlatform(plugin.Spec.Platforms); err == nil && ok {
 		if platform.URI != "" {
 			fmt.Fprintf(out, "URI: %s\n", platform.URI)
 			fmt.Fprintf(out, "SHA256: %s\n", platform.Sha256)
diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go
index b6ace8e..caae74c 100644
--- a/cmd/krew/cmd/search.go
+++ b/cmd/krew/cmd/search.go
@@ -80,7 +80,7 @@ Examples:
 			var status string
 			if _, ok := installed[name]; ok {
 				status = "yes"
-			} else if _, ok, err := plugin.Spec.GetMatchingPlatform(); err != nil {
+			} else if _, ok, err := installation.GetMatchingPlatform(plugin.Spec.Platforms); err != nil {
 				return errors.Wrapf(err, "failed to get the matching platform for plugin %s", name)
 			} else if ok {
 				status = "no"
diff --git a/hack/verify-code-patterns.sh b/hack/verify-code-patterns.sh
index 73701b3..d6b5477 100755
--- a/hack/verify-code-patterns.sh
+++ b/hack/verify-code-patterns.sh
@@ -43,3 +43,13 @@ if [[ -n "$out" ]]; then
   echo >&2 "$out"
   exit 1
 fi
+
+# Do not initialize index.{Plugin,Platform} structs in test code.
+out="$(grep --include '*_test.go' --exclude-dir 'vendor/' -EIrn '[^]](index\.)(Plugin|Platform){' || true)"
+if [[ -n "$out" ]]; then
+  echo >&2 "Do not use index.Platform or index.Plugin structs directly in tests,"
+  echo >&2 "use testutil.NewPlugin() or testutil.NewPlatform() instead:"
+  echo >&2 "-----"
+  echo >&2 "$out"
+  exit 1
+fi
diff --git a/pkg/index/validation/validate.go b/pkg/index/validation/validate.go
index 1a3d9e1..113f4cb 100644
--- a/pkg/index/validation/validate.go
+++ b/pkg/index/validation/validate.go
@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
@@ -88,15 +89,15 @@ func ValidatePlugin(name string, p index.Plugin) error {
 		return errors.Wrap(err, "failed to parse plugin version")
 	}
 	for _, pl := range p.Spec.Platforms {
-		if err := ValidatePlatform(pl); err != nil {
+		if err := validatePlatform(pl); err != nil {
 			return errors.Wrapf(err, "platform (%+v) is badly constructed", pl)
 		}
 	}
 	return nil
 }
 
-// ValidatePlatform checks Platform for structural validity.
-func ValidatePlatform(p index.Platform) error {
+// validatePlatform checks Platform for structural validity.
+func validatePlatform(p index.Platform) error {
 	if p.URI == "" {
 		return errors.New("URI has to be set")
 	}
@@ -112,5 +113,41 @@ func ValidatePlatform(p index.Platform) error {
 	if len(p.Files) == 0 {
 		return errors.New("can't have a plugin without specifying file operations")
 	}
+	if err := validateSelector(p.Selector); err != nil {
+		return errors.Wrap(err, "invalid platform selector")
+	}
+	return nil
+}
+
+// validateSelector checks if the platform selector uses supported keys and is not empty or nil.
+func validateSelector(sel *metav1.LabelSelector) error {
+	if sel == nil {
+		return errors.New("nil selector is not supported")
+	}
+	if sel.MatchLabels == nil && len(sel.MatchExpressions) == 0 {
+		return errors.New("empty selector is not supported")
+	}
+
+	// check for unsupported keys
+	keys := []string{}
+	for k := range sel.MatchLabels {
+		keys = append(keys, k)
+	}
+	for _, expr := range sel.MatchExpressions {
+		keys = append(keys, expr.Key)
+	}
+	for _, key := range keys {
+		if key != "os" && key != "arch" {
+			return errors.Errorf("key %q not supported", key)
+		}
+	}
+
+	if sel.MatchLabels != nil && len(sel.MatchLabels) == 0 {
+		return errors.New("matchLabels specified but empty")
+	}
+	if sel.MatchExpressions != nil && len(sel.MatchExpressions) == 0 {
+		return errors.New("matchExpressions specified but empty")
+	}
+
 	return nil
 }
diff --git a/pkg/index/platform.go b/pkg/installation/platform.go
similarity index 77%
rename from pkg/index/platform.go
rename to pkg/installation/platform.go
index 2321dc0..1640142 100644
--- a/pkg/index/platform.go
+++ b/pkg/installation/platform.go
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package index
+package installation
 
 import (
 	"os"
@@ -22,34 +22,37 @@ import (
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+
+	"sigs.k8s.io/krew/pkg/index"
 )
 
 // GetMatchingPlatform finds the platform spec in the specified plugin that
 // matches the OS/arch of the current machine (can be overridden via KREW_OS
 // and/or KREW_ARCH).
-func (s *PluginSpec) GetMatchingPlatform() (Platform, bool, error) {
+func GetMatchingPlatform(platforms []index.Platform) (index.Platform, bool, error) {
 	os, arch := osArch()
-	glog.V(4).Infof("Using os=%s arch=%s", os, arch)
-	return s.matchPlatformToSystemEnvs(os, arch)
+	return matchPlatform(platforms, os, arch)
 }
 
-func (s *PluginSpec) matchPlatformToSystemEnvs(os, arch string) (Platform, bool, error) {
+// matchPlatform returns the first matching platform to given os/arch.
+func matchPlatform(platforms []index.Platform, os, arch string) (index.Platform, bool, error) {
 	envLabels := labels.Set{
 		"os":   os,
 		"arch": arch,
 	}
 	glog.V(2).Infof("Matching platform for labels(%v)", envLabels)
-	for i, platform := range s.Platforms {
+
+	for i, platform := range platforms {
 		sel, err := metav1.LabelSelectorAsSelector(platform.Selector)
 		if err != nil {
-			return Platform{}, false, errors.Wrap(err, "failed to compile label selector")
+			return index.Platform{}, false, errors.Wrap(err, "failed to compile label selector")
 		}
 		if sel.Matches(envLabels) {
 			glog.V(2).Infof("Found matching platform with index (%d)", i)
 			return platform, true, nil
 		}
 	}
-	return Platform{}, false, nil
+	return index.Platform{}, false, nil
 }
 
 // osArch returns the OS/arch combination to be used on the current system. It
diff --git a/pkg/installation/util.go b/pkg/installation/util.go
index 690ff30..22b3444 100644
--- a/pkg/installation/util.go
+++ b/pkg/installation/util.go
@@ -23,24 +23,14 @@ import (
 	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 	"sigs.k8s.io/krew/pkg/installation/receipt"
-	"sigs.k8s.io/krew/pkg/pathutil"
 )
 
-func pluginVersionFromPath(installPath, pluginPath string) (string, error) {
-	// plugin path: {install_path}/{plugin_name}/{version}/...
-	elems, ok := pathutil.IsSubPath(installPath, pluginPath)
-	if !ok || len(elems) < 2 {
-		return "", errors.Errorf("failed to get the version from execution path=%q, with install path=%q", pluginPath, installPath)
-	}
-	return elems[1], nil
-}
-
 func getDownloadTarget(index index.Plugin) (version, sha256sum, uri string, fos []index.FileOperation, bin string, err error) {
 	// TODO(ahmetb): We have many return values from this method, indicating
 	// code smell. More specifically we return all-or-nothing, so ideally this
 	// should be converted into a struct, like InstallOperation{} contains all
 	// the data needed to install a plugin.
-	p, ok, err := index.Spec.GetMatchingPlatform()
+	p, ok, err := GetMatchingPlatform(index.Spec.Platforms)
 	if err != nil {
 		return "", "", "", nil, p.Bin, errors.Wrap(err, "failed to get matching platforms")
 	}


Test patch:
diff --git a/cmd/validate-krew-manifest/main_test.go b/cmd/validate-krew-manifest/main_test.go
index ab2297e..a93c574 100644
--- a/cmd/validate-krew-manifest/main_test.go
+++ b/cmd/validate-krew-manifest/main_test.go
@@ -166,13 +166,8 @@ func Test_findAnyMatchingPlatform(t *testing.T) {
 }
 
 func Test_isOverlappingPlatformSelectors_noOverlap(t *testing.T) {
-	p1 := index.Platform{
-		Selector: &metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"}}}}}
-	p2 := index.Platform{
-		Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "windows"}}}
+	p1 := testutil.NewPlatform().WithOSes("darwin", "linux").V()
+	p2 := testutil.NewPlatform().WithOSes("windows").V()
 
 	err := isOverlappingPlatformSelectors([]index.Platform{p1, p2})
 	if err != nil {
@@ -181,14 +176,8 @@ func Test_isOverlappingPlatformSelectors_noOverlap(t *testing.T) {
 }
 
 func Test_isOverlappingPlatformSelectors_overlap(t *testing.T) {
-	p1 := index.Platform{
-		Selector: &metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"}}}}}
-	p2 := index.Platform{
-		Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}}
-
+	p1 := testutil.NewPlatform().WithOS("darwin").V()
+	p2 := testutil.NewPlatform().WithOSes("darwin", "linux").V()
 	err := isOverlappingPlatformSelectors([]index.Platform{p1, p2})
 	if err == nil {
 		t.Fatal("expected overlap")
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
index c9b5dbf..362f174 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
@@ -24,9 +24,15 @@ spec:
     uri: https://example.com
     sha256: deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
     bin: kubectl-bar
+    selector:
+      matchLabels:
+        os: windows
   - files:
     - from: "*"
     uri: https://example.com
     sha256: deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
     bin: kubectl-bar
+    selector:
+      matchLabels:
+        os: linux
   shortDescription: "exists"
diff --git a/pkg/index/platform_test.go b/pkg/index/platform_test.go
deleted file mode 100644
index 5285a7f..0000000
--- a/pkg/index/platform_test.go
+++ /dev/null
@@ -1,134 +0,0 @@
-// 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 index
-
-import (
-	"os"
-	"reflect"
-	"runtime"
-	"testing"
-
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-)
-
-func Test_osArch_default(t *testing.T) {
-	inOS, inArch := runtime.GOOS, runtime.GOARCH
-	outOS, outArch := osArch()
-	if inOS != outOS {
-		t.Fatalf("returned OS=%q; expected=%q", outOS, inOS)
-	}
-	if inArch != outArch {
-		t.Fatalf("returned Arch=%q; expected=%q", outArch, inArch)
-	}
-}
-func Test_osArch_override(t *testing.T) {
-	customOS, customArch := "dragons", "metav1"
-	os.Setenv("KREW_OS", customOS)
-	os.Setenv("KREW_ARCH", customArch)
-	defer func() {
-		os.Unsetenv("KREW_ARCH")
-		os.Unsetenv("KREW_OS")
-	}()
-
-	outOS, outArch := osArch()
-	if customOS != outOS {
-		t.Fatalf("returned OS=%q; expected=%q", outOS, customOS)
-	}
-	if customArch != outArch {
-		t.Fatalf("returned Arch=%q; expected=%q", outArch, customArch)
-	}
-}
-
-func Test_matchPlatformToSystemEnvs(t *testing.T) {
-	matchingPlatform := Platform{
-		URI: "A",
-		Selector: &metav1.LabelSelector{
-			MatchLabels: map[string]string{
-				"os": "foo",
-			},
-		},
-		Files: nil,
-	}
-
-	type args struct {
-		i Plugin
-	}
-	tests := []struct {
-		name         string
-		args         args
-		wantPlatform Platform
-		wantFound    bool
-		wantErr      bool
-	}{
-		{
-			name: "Test Matching Index",
-			args: args{
-				i: Plugin{
-					Spec: PluginSpec{
-						Platforms: []Platform{
-							matchingPlatform, {
-								URI: "B",
-								Selector: &metav1.LabelSelector{
-									MatchLabels: map[string]string{
-										"os": "None",
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			wantPlatform: matchingPlatform,
-			wantFound:    true,
-			wantErr:      false,
-		}, {
-			name: "Test Matching Index Not Found",
-			args: args{
-				i: Plugin{
-					Spec: PluginSpec{
-						Platforms: []Platform{
-							{
-								URI: "B",
-								Selector: &metav1.LabelSelector{
-									MatchLabels: map[string]string{
-										"os": "None",
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			wantPlatform: Platform{},
-			wantFound:    false,
-			wantErr:      false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			gotPlatform, gotFound, err := tt.args.i.Spec.matchPlatformToSystemEnvs("foo", "amdBar")
-			if (err != nil) != tt.wantErr {
-				t.Errorf("GetMatchingPlatform() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(gotPlatform, tt.wantPlatform) {
-				t.Errorf("GetMatchingPlatform() gotPlatform = %v, want %v", gotPlatform, tt.wantPlatform)
-			}
-			if gotFound != tt.wantFound {
-				t.Errorf("GetMatchingPlatform() gotFound = %v, want %v", gotFound, tt.wantFound)
-			}
-		})
-	}
-}
diff --git a/pkg/index/validation/validate_test.go b/pkg/index/validation/validate_test.go
index 7d7a2a8..db3068d 100644
--- a/pkg/index/validation/validate_test.go
+++ b/pkg/index/validation/validate_test.go
@@ -25,47 +25,36 @@ import (
 )
 
 func Test_IsSafePluginName(t *testing.T) {
-	type args struct {
-		name string
-	}
 	tests := []struct {
-		name string
-		args args
-		want bool
+		name       string
+		pluginName string
+		want       bool
 	}{
 		{
-			name: "secure name",
-			args: args{
-				name: "foo-bar",
-			},
-			want: true,
+			name:       "secure name",
+			pluginName: "foo-bar",
+			want:       true,
 		},
 		{
-			name: "insecure path name",
-			args: args{
-				name: "/foo-bar",
-			},
-			want: false,
+			name:       "insecure path name",
+			pluginName: "/foo-bar",
+			want:       false,
 		},
 		{
-			name: "relative name",
-			args: args{
-				name: "..foo-bar",
-			},
-			want: false,
+			name:       "relative name",
+			pluginName: "..foo-bar",
+			want:       false,
 		},
 		{
-			name: "bad windows name",
-			args: args{
-				name: "nul",
-			},
-			want: false,
+			name:       "bad windows name",
+			pluginName: "nul",
+			want:       false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := IsSafePluginName(tt.args.name); got != tt.want {
-				t.Errorf("IsSafePluginName() = %v, want %v", got, tt.want)
+			if got := IsSafePluginName(tt.pluginName); got != tt.want {
+				t.Errorf("IsSafePluginName(%s) = %v, want %v", tt.pluginName, got, tt.want)
 			}
 		})
 	}
@@ -150,6 +139,12 @@ func TestValidatePlugin(t *testing.T) {
 			plugin:     testutil.NewPlugin().WithName("foo").WithVersion("v01.02.3-a").V(),
 			wantErr:    true,
 		},
+		{
+			name:       "no platform specified",
+			pluginName: "foo",
+			plugin:     testutil.NewPlugin().WithName("foo").WithPlatforms().V(),
+			wantErr:    true,
+		},
 		{
 			name:       "no file operations",
 			pluginName: "foo",
@@ -204,14 +199,85 @@ func TestValidatePlatform(t *testing.T) {
 			platform: testutil.NewPlatform().WithBin("").V(),
 			wantErr:  true,
 		},
+		{
+			name: "invalid platform selector",
+			platform: testutil.NewPlatform().WithSelector(&metav1.LabelSelector{
+				MatchLabels: map[string]string{"unsupported-field": "orange"}}).V(),
+			wantErr: true,
+		},
 		// TODO(ahmetb): add test case "bin field outside the plugin installation directory"
 		// by testing .WithBin("foo/../../../malicious-file").
 		// It appears like currently we're allowing this.
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := ValidatePlatform(tt.platform); (err != nil) != tt.wantErr {
-				t.Errorf("ValidatePlatform() error = %v, wantErr %v", err, tt.wantErr)
+			if err := validatePlatform(tt.platform); (err != nil) != tt.wantErr {
+				t.Errorf("validatePlatform() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func Test_validateSelector(t *testing.T) {
+	var tests = []struct {
+		name    string
+		sel     *metav1.LabelSelector
+		wantErr bool
+	}{
+		{
+			name:    "nil selector",
+			sel:     nil,
+			wantErr: true,
+		},
+		{
+			name:    "empty (wildcard) selector",
+			sel:     &metav1.LabelSelector{},
+			wantErr: true,
+		},
+		{
+			name:    "valid matchLabels",
+			sel:     &metav1.LabelSelector{MatchLabels: map[string]string{"os": "foo", "arch": "bar"}},
+			wantErr: false,
+		},
+		{
+			name: "valid matchExpressions",
+			sel: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{Key: "os",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"apple", "orange"},
+					}}},
+			wantErr: false,
+		},
+		{
+			name:    "empty matchLabels",
+			sel:     &metav1.LabelSelector{MatchLabels: map[string]string{}},
+			wantErr: true,
+		},
+		{
+			name:    "empty matchExpressions",
+			sel:     &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{}},
+			wantErr: true,
+		},
+		{
+			name:    "unsupported key in matchLabels",
+			sel:     &metav1.LabelSelector{MatchLabels: map[string]string{"unsupported-key": "value"}},
+			wantErr: true,
+		},
+		{
+			name: "unsupported key in matchExpressions",
+			sel: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{Key: "unsupported-key",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"apple", "orange"}}}},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := validateSelector(tt.sel); (err != nil) != tt.wantErr {
+				t.Errorf("validateSelector() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
 	}
diff --git a/pkg/installation/platform_test.go b/pkg/installation/platform_test.go
new file mode 100644
index 0000000..da2b6d0
--- /dev/null
+++ b/pkg/installation/platform_test.go
@@ -0,0 +1,81 @@
+// 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 installation
+
+import (
+	"os"
+	"runtime"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+
+	"sigs.k8s.io/krew/pkg/index"
+	"sigs.k8s.io/krew/pkg/testutil"
+)
+
+func Test_osArch(t *testing.T) {
+	inOS, inArch := runtime.GOOS, runtime.GOARCH
+	outOS, outArch := osArch()
+	if inOS != outOS {
+		t.Errorf("returned OS=%q; expected=%q", outOS, inOS)
+	}
+	if inArch != outArch {
+		t.Errorf("returned Arch=%q; expected=%q", outArch, inArch)
+	}
+}
+
+func Test_osArch_override(t *testing.T) {
+	customOS, customArch := "dragons", "metav1"
+	os.Setenv("KREW_OS", customOS)
+	os.Setenv("KREW_ARCH", customArch)
+	defer func() {
+		os.Unsetenv("KREW_ARCH")
+		os.Unsetenv("KREW_OS")
+	}()
+
+	outOS, outArch := osArch()
+	if customOS != outOS {
+		t.Errorf("returned OS=%q; expected=%q", outOS, customOS)
+	}
+	if customArch != outArch {
+		t.Errorf("returned Arch=%q; expected=%q", outArch, customArch)
+	}
+}
+
+func Test_matchPlatform(t *testing.T) {
+	const targetOS, targetArch = "foo", "amd64"
+	matchingPlatform := testutil.NewPlatform().WithOSArch(targetOS, targetArch).V()
+	differentOS := testutil.NewPlatform().WithOSArch("other", targetArch).V()
+	differentArch := testutil.NewPlatform().WithOSArch(targetOS, "other").V()
+
+	p, ok, err := matchPlatform([]index.Platform{differentOS, differentArch, matchingPlatform}, targetOS, targetArch)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !ok {
+		t.Fatal("failed to find a match")
+	}
+	if diff := cmp.Diff(p, matchingPlatform); diff != "" {
+		t.Fatalf("got a different object from the matching platform:\n%s", diff)
+	}
+
+	_, ok, err = matchPlatform([]index.Platform{differentOS, differentArch}, targetOS, targetArch)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if ok {
+		t.Fatal("got a matching platform, but was not expecting")
+	}
+}
diff --git a/pkg/installation/util_test.go b/pkg/installation/util_test.go
index 34e9a1a..f1395c9 100644
--- a/pkg/installation/util_test.go
+++ b/pkg/installation/util_test.go
@@ -96,38 +96,3 @@ func testdataPath(t *testing.T) string {
 	}
 	return filepath.Join(pwd, "testdata")
 }
-
-func Test_pluginVersionFromPath(t *testing.T) {
-	type args struct {
-		installPath string
-		pluginPath  string
-	}
-	tests := []struct {
-		name    string
-		args    args
-		want    string
-		wantErr bool
-	}{
-		{
-			name: "normal version",
-			args: args{
-				installPath: filepath.FromSlash("install/"),
-				pluginPath:  filepath.FromSlash("install/foo/HEAD/kubectl-foo"),
-			},
-			want:    "HEAD",
-			wantErr: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := pluginVersionFromPath(tt.args.installPath, tt.args.pluginPath)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("pluginVersionFromPath() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if got != tt.want {
-				t.Errorf("pluginVersionFromPath() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
diff --git a/pkg/testutil/plugin.go b/pkg/testutil/plugin.go
index 22ac415..c966f17 100644
--- a/pkg/testutil/plugin.go
+++ b/pkg/testutil/plugin.go
@@ -79,7 +79,15 @@ func NewPlatform() *R {
 }
 
 func (p *R) WithOS(os string) *R {
-	p.v.Selector.MatchLabels = map[string]string{"os": os}
+	p.v.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"os": os}}
+	return p
+}
+
+func (p *R) WithOSes(os ...string) *R {
+	p.v.Selector = &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
+		Key:      "os",
+		Operator: metav1.LabelSelectorOpIn,
+		Values:   os}}}
 	return p
 }