From e3ac50b2a2973c8a334cef6d48f031387109cc16 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Thu, 13 Jul 2023 19:34:01 +0300 Subject: [PATCH] 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 --- pkg/cli/config_cmd.go | 80 ++++++++++++++++++++++++++++++++--- pkg/cli/config_cmd_test.go | 86 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 157 insertions(+), 9 deletions(-) diff --git a/pkg/cli/config_cmd.go b/pkg/cli/config_cmd.go index 9138d1d2..0460cffd 100644 --- a/pkg/cli/config_cmd.go +++ b/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 ", - Short: "Add configuration for a zot registry", - Long: "Add configuration for a zot registry", - Args: cobra.ExactArgs(twoArgs), + Use: "add ", + Example: " zli config add main https://zot-foo.com:8080", + Short: "Add configuration for a zot registry", + 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 ", + 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 + 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: diff --git a/pkg/cli/config_cmd_test.go b/pkg/cli/config_cmd_test.go index 0b40429f..1ad0c7ba 100644 --- a/pkg/cli/config_cmd_test.go +++ b/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, "") })