Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd: Add 'index remove' command #552

Merged
merged 7 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 62 additions & 4 deletions cmd/krew/cmd/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,22 @@ package cmd

import (
"os"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
"k8s.io/klog"

"sigs.k8s.io/krew/cmd/krew/cmd/internal"
"sigs.k8s.io/krew/internal/index/indexoperations"
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/pkg/constants"
)

var (
forceIndexDelete *bool
)

// indexCmd represents the index command
var indexCmd = &cobra.Command{
Use: "index",
Expand All @@ -42,7 +50,7 @@ This command prints a list of indexes. It shows the name and the remote URL for
each configured index in table format.`,
Args: cobra.NoArgs,
RunE: func(_ *cobra.Command, _ []string) error {
indexes, err := indexoperations.ListIndexes(paths.IndexBase())
indexes, err := indexoperations.ListIndexes(paths)
if err != nil {
return errors.Wrap(err, "failed to list indexes")
}
Expand All @@ -62,14 +70,64 @@ var indexAddCmd = &cobra.Command{
Example: "kubectl krew index add default " + constants.IndexURI,
Args: cobra.ExactArgs(2),
RunE: func(_ *cobra.Command, args []string) error {
return indexoperations.AddIndex(paths.IndexBase(), args[0], args[1])
return indexoperations.AddIndex(paths, args[0], args[1])
},
}

var indexDeleteCmd = &cobra.Command{
Use: "remove",
Short: "Remove a configured index",
Long: `Removes a configured plugin index

It is only safe to remove indexes without installed plugins. Removing an index
while there are plugins installed will result in an error, unless the --force
option is used ( not recommended).`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
option is used ( not recommended).`,
option is used (not recommended).`,


Args: cobra.ExactArgs(1),
RunE: indexDelete,
}

func indexDelete(_ *cobra.Command, args []string) error {
name := args[0]

ps, err := installation.InstalledPluginsFromIndex(paths.InstallReceiptsPath(), name)
if err != nil {
return errors.Wrap(err, "failed querying plugins installed from the index")
}
klog.V(4).Infof("Found %d plugins from index", len(ps))

if len(ps) > 0 && !*forceIndexDelete {
var names []string
for _, pl := range ps {
names = append(names, pl.Name)
}

internal.PrintWarning(os.Stderr, `Plugins [%s] are still installed from index %q!
Removing indexes while there are plugins installed from is not recommended
(you can use --force to ignore this check).`+"\n", strings.Join(names, ", "), name)
return errors.Errorf("there are still plugins installed from this index")
}

err = indexoperations.DeleteIndex(paths, name)
if os.IsNotExist(err) {
if *forceIndexDelete {
klog.V(4).Infof("Index not found, but --force is used, so not returning an error")
return nil // success if --force specified and index does not exist.
}
return errors.Errorf("index %q does not exist", name)
}
return errors.Wrap(err, "error while removing the plugin index")
}

func init() {
forceIndexDelete = indexDeleteCmd.Flags().Bool("force", false,
"Remove index even if it has plugins currently installed (may result in unsupported behavior)")

indexCmd.AddCommand(indexAddCmd)
indexCmd.AddCommand(indexListCmd)
indexCmd.AddCommand(indexDeleteCmd)
corneliusweig marked this conversation as resolved.
Show resolved Hide resolved

if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
indexCmd.AddCommand(indexListCmd)
indexCmd.AddCommand(indexAddCmd)
rootCmd.AddCommand(indexCmd)
}
}
9 changes: 2 additions & 7 deletions cmd/krew/cmd/internal/security_notice.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,18 @@
package internal

import (
"fmt"
"os"

"github.com/fatih/color"

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

const securityNotice = `You installed plugin %q from the krew-index plugin repository.
const securityNoticeFmt = `You installed plugin %q from the krew-index plugin repository.
These plugins are not audited for security by the Krew maintainers.
Run them at your own risk.`

func PrintSecurityNotice(plugin string) {
if plugin == constants.KrewPluginName {
return // do not warn for krew itself
}
boldRed := color.New(color.FgRed, color.Bold).SprintfFunc()
msg := fmt.Sprintf(securityNotice, plugin)
fmt.Fprintf(os.Stderr, "%s: %s\n", boldRed("WARNING"), msg)
PrintWarning(os.Stderr, securityNoticeFmt+"\n", plugin)
}
27 changes: 27 additions & 0 deletions cmd/krew/cmd/internal/warning.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2020 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 internal

import (
"fmt"
"io"

"github.com/fatih/color"
)

func PrintWarning(w io.Writer, format string, a ...interface{}) {
color.New(color.FgRed, color.Bold).Fprint(w, "WARNING: ")
fmt.Fprintf(w, format, a...)
}
3 changes: 1 addition & 2 deletions cmd/krew/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ func init() {
func preRun(cmd *cobra.Command, _ []string) error {
// check must be done before ensureDirs, to detect krew's self-installation
if !internal.IsBinDirInPATH(paths) {
boldRed := color.New(color.FgRed, color.Bold).SprintfFunc()
fmt.Fprintf(os.Stderr, "%s: %s\n\n", boldRed("WARNING"), internal.SetupInstructions())
internal.PrintWarning(os.Stderr, internal.SetupInstructions()+"\n\n")
}

if err := ensureDirs(paths.BasePath(),
Expand Down
51 changes: 50 additions & 1 deletion integration_test/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func TestKrewIndexList(t *testing.T) {
t.Fatal("expected at least 1 index in output")
}

test.Krew("index", "add", "foo", test.TempDir().Path("index/"+constants.DefaultIndexName)).RunOrFail()
localIndex := test.TempDir().Path("index/" + constants.DefaultIndexName)
test.Krew("index", "add", "foo", localIndex).RunOrFail()
out = test.Krew("index", "list").RunOrFailOutput()
if indexes := lines(out); len(indexes) < 3 {
// the first line is the header
Expand All @@ -84,3 +85,51 @@ func TestKrewIndexList_NoIndexes(t *testing.T) {
t.Fatalf("expected index list to be empty:\n%s", string(out))
}
}

func TestKrewIndexRemove_nonExisting(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1)
defer cleanup()

err := test.Krew("index", "remove", "non-existing").Run()
if err == nil {
t.Fatal("expected error")
}
}

func TestKrewIndexRemove_ok(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex()
defer cleanup()

localIndex := test.TempDir().Path("index/" + constants.DefaultIndexName)
test.Krew("index", "add", "foo", localIndex).RunOrFail()
test.Krew("index", "remove", "foo").RunOrFail()
}

func TestKrewIndexRemoveFailsWhenPluginsInstalled(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1)
defer cleanup()

test.Krew("install", validPlugin).RunOrFailOutput()
if err := test.Krew("index", "remove", "default").Run(); err == nil {
t.Fatal("expected error while removing index when there are installed plugins")
}

// using --force skips the check
test.Krew("index", "remove", "--force", "default").RunOrFail()
}

func TestKrewIndexRemoveForce_nonExisting(t *testing.T) {
skipShort(t)
test, cleanup := NewTest(t)
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1)
defer cleanup()

// --force returns success for non-existing indexes
test.Krew("index", "remove", "--force", "non-existing").RunOrFail()
}
27 changes: 20 additions & 7 deletions internal/index/indexoperations/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ package indexoperations
import (
"io/ioutil"
"os"
"path/filepath"
"regexp"

"github.com/pkg/errors"

"sigs.k8s.io/krew/internal/environment"
"sigs.k8s.io/krew/internal/gitutil"
)

Expand All @@ -35,16 +35,16 @@ type Index struct {

// ListIndexes returns a slice of Index objects. The path argument is used as
// the base path of the index.
func ListIndexes(path string) ([]Index, error) {
dirs, err := ioutil.ReadDir(path)
func ListIndexes(paths environment.Paths) ([]Index, error) {
dirs, err := ioutil.ReadDir(paths.IndexBase())
if err != nil {
return nil, errors.Wrapf(err, "failed to read directory %s", path)
return nil, errors.Wrap(err, "failed to list directory")
}

indexes := []Index{}
for _, dir := range dirs {
indexName := dir.Name()
remote, err := gitutil.GetRemoteURL(filepath.Join(path, indexName))
remote, err := gitutil.GetRemoteURL(paths.IndexPath(indexName))
if err != nil {
return nil, errors.Wrapf(err, "failed to list the remote URL for index %s", indexName)
}
Expand All @@ -58,12 +58,12 @@ func ListIndexes(path string) ([]Index, error) {
}

// AddIndex initializes a new index to install plugins from.
func AddIndex(path, name, url string) error {
func AddIndex(paths environment.Paths, name, url string) error {
if !IsValidIndexName(name) {
return errors.New("invalid index name")
}

dir := filepath.Join(path, name)
dir := paths.IndexPath(name)
if _, err := os.Stat(dir); os.IsNotExist(err) {
return gitutil.EnsureCloned(url, dir)
} else if err != nil {
Expand All @@ -72,6 +72,19 @@ func AddIndex(path, name, url string) error {
return errors.New("index already exists")
}

// DeleteIndex removes specified index name. If index does not exist, returns an error that can be tested by os.IsNotExist.
func DeleteIndex(paths environment.Paths, name string) error {
if !IsValidIndexName(name) {
return errors.New("invalid index name")
}

dir := paths.IndexPath(name)
if _, err := os.Stat(dir); err != nil {
return err
}
return os.RemoveAll(dir)
corneliusweig marked this conversation as resolved.
Show resolved Hide resolved
}

// IsValidIndexName validates if an index name contains invalid characters
func IsValidIndexName(name string) bool {
return validNamePattern.MatchString(name)
Expand Down
Loading