feat(zli): add new subcommand to remove a zli config (#1619)

https://github.com/project-zot/zot/issues/1594
- use indent '  ' in the config json

https://github.com/project-zot/zot/issues/1593
- add subcommand `zli config remove`

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

pkg/cli/config_cmd.go

@@ -97,16 +97,18 @@ func NewConfigCommand() *cobra.Command {
 	configCmd.Flags().BoolVar(&isReset, "reset", false, "Reset a variable value")
 	configCmd.SetUsageTemplate(configCmd.UsageTemplate() + supportedOptions)
 	configCmd.AddCommand(NewConfigAddCommand())
+	configCmd.AddCommand(NewConfigRemoveCommand())
 
 	return configCmd
 }
 
 func NewConfigAddCommand() *cobra.Command {
 	configAddCmd := &cobra.Command{
-		Use:   "add <config-name> <url>",
+		Use:     "add <config-name> <url>",
-		Short: "Add configuration for a zot registry",
+		Example: "  zli config add main https://zot-foo.com:8080",
-		Long:  "Add configuration for a zot registry",
+		Short:   "Add configuration for a zot registry",
-		Args:  cobra.ExactArgs(twoArgs),
+		Long:    "Add configuration for a zot registry",
+		Args:    cobra.ExactArgs(twoArgs),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			home, err := os.UserHomeDir()
 			if err != nil {
@@ -124,9 +126,42 @@ func NewConfigAddCommand() *cobra.Command {
 		},
 	}
 
+	// Prevent parent template from overwriting default template
+	configAddCmd.SetUsageTemplate(configAddCmd.UsageTemplate())
+
 	return configAddCmd
 }
 
+func NewConfigRemoveCommand() *cobra.Command {
+	configRemoveCmd := &cobra.Command{
+		Use:     "remove <config-name>",
+		Example: "  zli config remove main",
+		Short:   "Remove configuration for a zot registry",
+		Long:    "Remove configuration for a zot registry",
+		Args:    cobra.ExactArgs(oneArg),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			home, err := os.UserHomeDir()
+			if err != nil {
+				panic(err)
+			}
+
+			configPath := path.Join(home + "/.zot")
+			// zot config add <config-name> <url>
+			err = removeConfig(configPath, args[0])
+			if err != nil {
+				return err
+			}
+
+			return nil
+		},
+	}
+
+	// Prevent parent template from overwriting default template
+	configRemoveCmd.SetUsageTemplate(configRemoveCmd.UsageTemplate())
+
+	return configRemoveCmd
+}
+
 func getConfigMapFromFile(filePath string) ([]interface{}, error) {
 	file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, defaultConfigPerms)
 	if err != nil {
@@ -164,7 +199,7 @@ func saveConfigMapToFile(filePath string, configMap []interface{}) error {
 	listMap := make(map[string]interface{})
 	listMap["configs"] = configMap
 
-	marshalled, err := json.Marshal(&listMap)
+	marshalled, err := json.MarshalIndent(&listMap, "", "  ")
 	if err != nil {
 		return err
 	}
@@ -235,6 +270,38 @@ func addConfig(configPath, configName, url string) error {
 	return nil
 }
 
+func removeConfig(configPath, configName string) error {
+	configs, err := getConfigMapFromFile(configPath)
+	if err != nil {
+		return err
+	}
+
+	for i, val := range configs {
+		configMap, ok := val.(map[string]interface{})
+		if !ok {
+			return zerr.ErrBadConfig
+		}
+
+		name := configMap[nameKey]
+		if name != configName {
+			continue
+		}
+
+		// Remove config from the config list before saving
+		newConfigs := configs[:i]
+		newConfigs = append(newConfigs, configs[i+1:]...)
+
+		err = saveConfigMapToFile(configPath, newConfigs)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}
+
+	return zerr.ErrConfigNotFound
+}
+
 func addDefaultConfigs(config map[string]interface{}) {
 	if _, ok := config[showspinnerConfig]; !ok {
 		config[showspinnerConfig] = true
@@ -411,9 +478,10 @@ func configNameExists(configs []interface{}, configName string) bool {
 
 const (
 	examples = `  zli config add main https://zot-foo.com:8080
+  zli config --list
   zli config main url
   zli config main --list
-  zli config --list`
+  zli config remove main`
 
 	supportedOptions = `
 Useful variables:

pkg/cli/config_cmd_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"log"
 	"os"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -162,6 +163,85 @@ func TestConfigCmdMain(t *testing.T) {
 		So(err, ShouldEqual, zotErrors.ErrInvalidURL)
 	})
 
+	Convey("Test remove config entry successfully", t, func() {
+		args := []string{"remove", "configtest"}
+		configPath := makeConfigFile(`{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`)
+		defer os.Remove(configPath)
+		cmd := NewConfigCommand()
+		buff := bytes.NewBufferString("")
+		cmd.SetOut(buff)
+		cmd.SetErr(buff)
+		cmd.SetArgs(args)
+		err := cmd.Execute()
+		So(err, ShouldBeNil)
+		actual, err := os.ReadFile(configPath)
+		So(err, ShouldBeNil)
+		space := regexp.MustCompile(`\s+`)
+		actualString := space.ReplaceAllString(string(actual), " ")
+		So(actualString, ShouldEqual, `{ "configs": [] }`)
+	})
+
+	Convey("Test remove missing config entry", t, func() {
+		args := []string{"remove", "configtest"}
+		configPath := makeConfigFile(`{"configs":[]`)
+		defer os.Remove(configPath)
+		cmd := NewConfigCommand()
+		buff := bytes.NewBufferString("")
+		cmd.SetOut(buff)
+		cmd.SetErr(buff)
+		cmd.SetArgs(args)
+		err := cmd.Execute()
+		So(err, ShouldNotBeNil)
+		So(buff.String(), ShouldContainSubstring, "does not exist")
+	})
+
+	Convey("Test remove bad config file content", t, func() {
+		args := []string{"remove", "configtest"}
+		configPath := makeConfigFile(`{"asdf":[]`)
+		defer os.Remove(configPath)
+		cmd := NewConfigCommand()
+		buff := bytes.NewBufferString("")
+		cmd.SetOut(buff)
+		cmd.SetErr(buff)
+		cmd.SetArgs(args)
+		err := cmd.Execute()
+		So(err, ShouldNotBeNil)
+		So(buff.String(), ShouldContainSubstring, "config json is empty")
+	})
+
+	Convey("Test remove bad config file entry", t, func() {
+		args := []string{"remove", "configtest"}
+		configPath := makeConfigFile(`{"configs":[asdad]`)
+		defer os.Remove(configPath)
+		cmd := NewConfigCommand()
+		buff := bytes.NewBufferString("")
+		cmd.SetOut(buff)
+		cmd.SetErr(buff)
+		cmd.SetArgs(args)
+		err := cmd.Execute()
+		So(err, ShouldNotBeNil)
+		So(buff.String(), ShouldContainSubstring, "invalid config")
+	})
+
+	Convey("Test remove config bad permissions", t, func() {
+		args := []string{"remove", "configtest"}
+		configPath := makeConfigFile(`{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`)
+		defer func() {
+			_ = os.Chmod(configPath, 0o600)
+			os.Remove(configPath)
+		}()
+		err := os.Chmod(configPath, 0o400) // Read-only, so we fail only on updating the file, not reading
+		So(err, ShouldBeNil)
+		cmd := NewConfigCommand()
+		buff := bytes.NewBufferString("")
+		cmd.SetOut(buff)
+		cmd.SetErr(buff)
+		cmd.SetArgs(args)
+		err = cmd.Execute()
+		So(err, ShouldNotBeNil)
+		So(buff.String(), ShouldContainSubstring, "permission denied")
+	})
+
 	Convey("Test fetch all config", t, func() {
 		args := []string{"--list"}
 		configPath := makeConfigFile(`{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`)
@@ -294,7 +374,7 @@ func TestConfigCmdMain(t *testing.T) {
 		}
 		actualStr := string(actual)
 		So(actualStr, ShouldContainSubstring, "https://test-url.com")
-		So(actualStr, ShouldContainSubstring, `"showspinner":false`)
+		So(actualStr, ShouldContainSubstring, `"showspinner": false`)
 		So(buff.String(), ShouldEqual, "")
 
 		Convey("To an empty file", func() {
@@ -330,7 +410,7 @@ func TestConfigCmdMain(t *testing.T) {
 		}
 		actualStr := string(actual)
 		So(actualStr, ShouldContainSubstring, `https://new-url.com`)
-		So(actualStr, ShouldContainSubstring, `"showspinner":false`)
+		So(actualStr, ShouldContainSubstring, `"showspinner": false`)
 		So(actualStr, ShouldNotContainSubstring, `https://test-url.com`)
 		So(buff.String(), ShouldEqual, "")
 	})
@@ -353,7 +433,7 @@ func TestConfigCmdMain(t *testing.T) {
 		}
 		actualStr := string(actual)
 		So(actualStr, ShouldNotContainSubstring, "showspinner")
-		So(actualStr, ShouldContainSubstring, `"url":"https://test-url.com"`)
+		So(actualStr, ShouldContainSubstring, `"url": "https://test-url.com"`)
 		So(buff.String(), ShouldEqual, "")
 	})