Skip to content

Commit

Permalink
Return error when passing canonical name to upgrade/uninstall
Browse files Browse the repository at this point in the history
  • Loading branch information
chriskim06 committed Apr 28, 2020
1 parent 382fc40 commit 9b8752b
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 6 deletions.
8 changes: 8 additions & 0 deletions cmd/krew/cmd/namingutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
package cmd

import (
"regexp"

"sigs.k8s.io/krew/pkg/constants"
"sigs.k8s.io/krew/pkg/index"
)

var canonicalNameRegex = regexp.MustCompile(`^[\w-]+/[\w-]+$`)

// indexOf returns the index name of a receipt.
func indexOf(r index.Receipt) string {
if r.Status.Source.Name == "" {
Expand Down Expand Up @@ -48,3 +52,7 @@ func canonicalName(p index.Plugin, indexName string) string {
}
return indexName + "/" + p.Name
}

func isCanonicalName(s string) bool {
return canonicalNameRegex.MatchString(s)
}
12 changes: 12 additions & 0 deletions cmd/krew/cmd/namingutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,15 @@ func Test_canonicalName(t *testing.T) {
t.Errorf("expected=%q; got=%q", expected, got)
}
}

func Test_isCanonicalName(t *testing.T) {
if isCanonicalName("foo") {
t.Error("expected foo to not be considered a canonical name")
}
if isCanonicalName("../index/foo") {
t.Error("expected ../index/foo to not be considered a canonical name")
}
if !isCanonicalName("index/foo") {
t.Error("expected index/foo to be considered a canonical name")
}
}
5 changes: 2 additions & 3 deletions cmd/krew/cmd/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package cmd
import (
"fmt"
"os"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand All @@ -41,8 +40,8 @@ Remarks:
RunE: func(cmd *cobra.Command, args []string) error {
for _, name := range args {
if !validation.IsSafePluginName(name) {
if strings.Contains(name, "/") {
klog.Warningf("uninstall command does not support INDEX/PLUGIN syntax; just specify PLUGIN")
if isCanonicalName(name) {
return errors.New("uninstall command does not support INDEX/PLUGIN syntax; just specify PLUGIN")
}
return unsafePluginNameErr(name)
}
Expand Down
5 changes: 2 additions & 3 deletions cmd/krew/cmd/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package cmd
import (
"fmt"
"os"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -63,8 +62,8 @@ kubectl krew upgrade foo bar"`,
// Upgrade certain plugins
for _, arg := range args {
if !validation.IsSafePluginName(arg) {
if strings.Contains(arg, "/") {
klog.Warning("upgrade command does not support INDEX/PLUGIN syntax; just specify PLUGIN")
if isCanonicalName(arg) {
return errors.New("upgrade command does not support INDEX/PLUGIN syntax; just specify PLUGIN")
}
return unsafePluginNameErr(arg)
}
Expand Down
17 changes: 17 additions & 0 deletions integration_test/uninstall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ func TestKrewUninstall(t *testing.T) {
}
}

func TestKrewUninstallPluginsFromCustomIndex(t *testing.T) {
skipShort(t)

test, cleanup := NewTest(t)
defer cleanup()

test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex()
test.Krew("index", "add", "foo", test.TempDir().Path("index/"+constants.DefaultIndexName)).RunOrFail()
test.Krew("install", "foo/"+validPlugin).RunOrFail()

if _, err := test.Krew("uninstall", "foo/"+validPlugin).Run(); err == nil {
t.Error("expected error when uninstalling by canonical name")
}
test.Krew("uninstall", validPlugin).RunOrFail()
test.AssertExecutableNotInPATH("kubectl-" + validPlugin)
}

func TestKrewRemove_AliasSupported(t *testing.T) {
skipShort(t)

Expand Down

0 comments on commit 9b8752b

Please sign in to comment.