Small refactorings for better code structure

cmd/krew/cmd/info.go

@@ -22,12 +22,11 @@ import (
 	"strings"
 	"unicode"
 
-	"sigs.k8s.io/krew/pkg/index"
-	"sigs.k8s.io/krew/pkg/index/indexscanner"
-	"sigs.k8s.io/krew/pkg/installation"
-
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
+
+	"sigs.k8s.io/krew/pkg/index"
+	"sigs.k8s.io/krew/pkg/index/indexscanner"
 )
 
 // infoCmd represents the info command
@@ -57,7 +56,7 @@ Example:
 
 func printPluginInfo(out io.Writer, plugin index.Plugin) {
 	fmt.Fprintf(out, "NAME: %s\n", plugin.Name)
-	if platform, ok, err := installation.GetMatchingPlatform(plugin); err == nil && ok {
+	if platform, ok, err := plugin.Spec.GetMatchingPlatform(); err == nil && ok {
 		if platform.URI != "" {
 			fmt.Fprintf(out, "URI: %s\n", platform.URI)
 			fmt.Fprintf(out, "SHA256: %s\n", platform.Sha256)

cmd/krew/cmd/search.go

@@ -79,7 +79,7 @@ Examples:
 			var status string
 			if _, ok := installed[name]; ok {
 				status = "installed"
-			} else if _, ok, err := installation.GetMatchingPlatform(plugin); err != nil {
+			} else if _, ok, err := plugin.Spec.GetMatchingPlatform(); err != nil {
 				return errors.Wrapf(err, "failed to get the matching platform for plugin %s", name)
 			} else if ok {
 				status = "available"

cmd/validate-krew-manifest/main.go

@@ -25,13 +25,14 @@ import (
 	"path/filepath"
 	"strings"
 
-	"sigs.k8s.io/krew/pkg/index"
-	"sigs.k8s.io/krew/pkg/index/indexscanner"
-
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+
+	"sigs.k8s.io/krew/pkg/constants"
+	"sigs.k8s.io/krew/pkg/index"
+	"sigs.k8s.io/krew/pkg/index/indexscanner"
 )
 
 var flManifest string
@@ -63,8 +64,8 @@ func validateManifestFile(path string) error {
 	}
 	filename := filepath.Base(path)
 	manifestExtension := filepath.Ext(filename)
-	if manifestExtension != ".yaml" {
-		return fmt.Errorf("expected manifest extension '.yaml' but found '%s'", manifestExtension)
+	if manifestExtension != constants.ManifestExtension {
+		return fmt.Errorf("expected manifest extension %q but found %q", constants.ManifestExtension, manifestExtension)
 	}
 	pluginNameFromFileName := strings.TrimSuffix(filename, manifestExtension)
 	glog.V(4).Infof("inferred plugin name as %s", pluginNameFromFileName)

cmd/validate-krew-manifest/main_test.go

@@ -51,7 +51,7 @@ func TestValidateManifestFile(t *testing.T) {
 				},
 			},
 			shouldErr: true,
-			errMsg:    "expected manifest extension '.yaml'",
+			errMsg:    "expected manifest extension \".yaml\"",
 		},
 		{
 			name:     "manifest validation fails",

hack/verify-code-patterns.sh

@@ -22,3 +22,14 @@ if [[ -n "$out" ]]; then
   echo >&2 "$out"
   exit 1
 fi
+
+out="$(grep --include '*.go' \
+            --exclude "*_test.go" \
+            --exclude 'constants.go' \
+            --exclude-dir 'vendor/' \
+            -EIrn '\.yaml"' || true)"
+if [[ -n "$out" ]]; then
+  echo >&2 'You used ".yaml" in production, use constants.ManifestExtension instead:'
+  echo >&2 "$out"
+  exit 1
+fi

pkg/constants/constants.go

@@ -17,6 +17,7 @@ package constants
 const (
 	CurrentAPIVersion = "krew.googlecontainertools.github.com/v1alpha2"
 	PluginKind        = "Plugin"
+	ManifestExtension = ".yaml"
 
 	// IndexURI points to the upstream index.
 	IndexURI = "https://github.com/kubernetes-sigs/krew-index.git"

pkg/index/indexscanner/scanner.go

@@ -27,6 +27,7 @@ import (
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/yaml"
 
+	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 )
 
@@ -76,7 +77,7 @@ func LoadPluginFileFromFS(indexDir, pluginName string) (index.Plugin, error) {
 	if err != nil {
 		return index.Plugin{}, err
 	}
-	p, err := ReadPluginFile(filepath.Join(indexDir, pluginName+".yaml"))
+	p, err := ReadPluginFile(filepath.Join(indexDir, pluginName+constants.ManifestExtension))
 	if os.IsNotExist(err) {
 		return index.Plugin{}, err
 	} else if err != nil {

pkg/index/platform.go

@@ -0,0 +1,67 @@
+// 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"
+	"runtime"
+
+	"github.com/golang/glog"
+	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+)
+
+// 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) {
+	os, arch := osArch()
+	glog.V(4).Infof("Using os=%s arch=%s", os, arch)
+	return s.matchPlatformToSystemEnvs(os, arch)
+}
+
+func (s *PluginSpec) matchPlatformToSystemEnvs(os, arch string) (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 {
+		sel, err := metav1.LabelSelectorAsSelector(platform.Selector)
+		if err != nil {
+			return 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
+}
+
+// osArch returns the OS/arch combination to be used on the current system. It
+// can be overridden by setting KREW_OS and/or KREW_ARCH environment variables.
+func osArch() (string, string) {
+	goos, goarch := runtime.GOOS, runtime.GOARCH
+	envOS, envArch := os.Getenv("KREW_OS"), os.Getenv("KREW_ARCH")
+	if envOS != "" {
+		goos = envOS
+	}
+	if envArch != "" {
+		goarch = envArch
+	}
+	return goos, goarch
+}

pkg/index/platform_test.go

@@ -0,0 +1,134 @@
+// 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)
+			}
+		})
+	}
+}

pkg/installation/util.go

@@ -18,60 +18,15 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 
 	"sigs.k8s.io/krew/pkg/index"
 	"sigs.k8s.io/krew/pkg/pathutil"
 )
 
-// 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 GetMatchingPlatform(p index.Plugin) (index.Platform, bool, error) {
-	os, arch := osArch()
-	glog.V(4).Infof("Using os=%s arch=%s", os, arch)
-	return matchPlatformToSystemEnvs(p, os, arch)
-}
-
-// osArch returns the OS/arch combination to be used on the current system. It
-// can be overridden by setting KREW_OS and/or KREW_ARCH environment variables.
-func osArch() (string, string) {
-	goos, goarch := runtime.GOOS, runtime.GOARCH
-	envOS, envArch := os.Getenv("KREW_OS"), os.Getenv("KREW_ARCH")
-	if envOS != "" {
-		goos = envOS
-	}
-	if envArch != "" {
-		goarch = envArch
-	}
-	return goos, goarch
-}
-
-func matchPlatformToSystemEnvs(p index.Plugin, 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 p.Spec.Platforms {
-		sel, err := metav1.LabelSelectorAsSelector(platform.Selector)
-		if err != nil {
-			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 index.Platform{}, false, nil
-}
-
 func findInstalledPluginVersion(installPath, binDir, pluginName string) (name string, installed bool, err error) {
 	if !index.IsSafePluginName(pluginName) {
 		return "", false, errors.Errorf("the plugin name %q is not allowed", pluginName)
@@ -111,7 +66,7 @@ func getPluginVersion(p index.Platform) (version, uri string) {
 }
 
 func getDownloadTarget(index index.Plugin) (version, uri string, fos []index.FileOperation, bin string, err error) {
-	p, ok, err := GetMatchingPlatform(index)
+	p, ok, err := index.Spec.GetMatchingPlatform()
 	if err != nil {
 		return "", "", nil, p.Bin, errors.Wrap(err, "failed to get matching platforms")
 	}