diff --git a/errors/errors.go b/errors/errors.go index 3ff10514..6bf99f68 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -49,6 +49,7 @@ func GetDetails(err error) map[string]string { var ( ErrBadConfig = errors.New("invalid server config") ErrCliBadConfig = errors.New("invalid cli config") + ErrCliMissingConfigsField = errors.New(`missing or null "configs" field`) ErrRepoNotFound = errors.New("repository not found") ErrRepoBadVersion = errors.New("unsupported repository layout version") ErrRepoBadLayout = errors.New("invalid repository layout") diff --git a/pkg/cli/client/client_test.go b/pkg/cli/client/client_test.go index 7bd42613..49087453 100644 --- a/pkg/cli/client/client_test.go +++ b/pkg/cli/client/client_test.go @@ -105,9 +105,9 @@ func TestTLSWithAuth(t *testing.T) { defer cm.StopServer() Convey("Test with htpassw auth", func() { - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) - // Use the HOME that makeConfigFile set (temp directory) for certificates + // Client certs are resolved under $HOME; isolate from the real home directory. home := os.Getenv("HOME") destCertsDir := filepath.Join(home, certsDir1) err := os.MkdirAll(destCertsDir, 0o755) @@ -368,14 +368,9 @@ func TestTLSBadCerts(t *testing.T) { func makeConfigFile(t *testing.T, content string) string { t.Helper() tempDir := t.TempDir() - os.Setenv("HOME", tempDir) + t.Setenv("HOME", tempDir) - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - configPath := path.Join(home, "/.zot") + configPath := filepath.Join(tempDir, ".zot") if err := os.WriteFile(configPath, []byte(content), 0o600); err != nil { panic(err) diff --git a/pkg/cli/client/config.go b/pkg/cli/client/config.go new file mode 100644 index 00000000..7f39c24a --- /dev/null +++ b/pkg/cli/client/config.go @@ -0,0 +1,329 @@ +//go:build search + +package client + +import ( + "bytes" + "errors" + "fmt" + "os" + "slices" + "strconv" + "strings" + "text/tabwriter" + + jsoniter "github.com/json-iterator/go" + + zerr "zotregistry.dev/zot/v2/errors" +) + +const ( + defaultConfigPerms = 0o644 + defaultFilePerms = 0o600 + + nameKey = "_name" + showspinnerConfig = "showspinner" + verifyTLSConfig = "verify-tls" +) + +// ZliConfigFile is the on-disk JSON shape for ~/.zot (zli CLI registry profiles). +type ZliConfigFile struct { + Configs []ZliConfig `json:"configs"` +} + +// ZliConfig is one named registry profile inside ZliConfigFile.Configs. +type ZliConfig struct { + Name string `json:"_name"` //nolint:tagliatelle // persisted ~/.zot schema uses `_name` + URL string `json:"url"` + ShowSpinner any `json:"showspinner,omitempty"` + VerifyTLS any `json:"verify-tls,omitempty"` //nolint:tagliatelle // hyphenated ~/.zot key +} + +// isConfigUnavailable reports errors that mean there is no usable ~/.zot config yet (empty file +// or JSON without a "configs" field). Those cases are treated like an empty config for some commands. +func isConfigUnavailable(err error) bool { + return errors.Is(err, zerr.ErrEmptyJSON) || errors.Is(err, zerr.ErrCliMissingConfigsField) +} + +// ReadZliConfigFile reads and validates ~/.zot JSON from path. +// +// If the path does not exist yet, OpenFile(..., O_CREATE) creates an empty file first so later +// ReadFile sees zero bytes and returns [ErrEmptyJSON] ("fresh" CLI state). Adding entries via +// `zli config add` still works without relying on this side effect. +func ReadZliConfigFile(filePath string) (*ZliConfigFile, error) { + file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, defaultConfigPerms) + if err != nil { + return nil, err + } + + if err := file.Close(); err != nil { + return nil, err + } + + data, err := os.ReadFile(filePath) + if err != nil { + return nil, err + } + + if len(bytes.TrimSpace(data)) == 0 { + return nil, zerr.ErrEmptyJSON + } + + json := jsoniter.ConfigCompatibleWithStandardLibrary + + var jsonMap map[string]any + if unmarshalErr := json.Unmarshal(data, &jsonMap); unmarshalErr != nil { + return nil, fmt.Errorf("%w: %w", zerr.ErrCliBadConfig, unmarshalErr) + } + + if _, ok := jsonMap["configs"]; !ok || jsonMap["configs"] == nil { + return nil, fmt.Errorf("%w: %w", zerr.ErrCliBadConfig, zerr.ErrCliMissingConfigsField) + } + + configsAny, ok := jsonMap["configs"].([]any) + if !ok { + return nil, fmt.Errorf( + `%w: field "configs" must be a JSON array, got %T`, + zerr.ErrCliBadConfig, jsonMap["configs"]) + } + + out := &ZliConfigFile{Configs: make([]ZliConfig, 0, len(configsAny))} + + for i, v := range configsAny { + configMap, ok := v.(map[string]any) + if !ok { + return nil, fmt.Errorf( + `%w: configs[%d] must be a JSON object, got %T`, + zerr.ErrCliBadConfig, i, v) + } + + c, err := zliConfigFromMap(configMap, i) + if err != nil { + return nil, err + } + + out.Configs = append(out.Configs, c) + } + + return out, nil +} + +func zliConfigFromMap(configMap map[string]any, i int) (ZliConfig, error) { + nameRaw, nameOk := configMap[nameKey] + nameStr, nameIsStr := nameRaw.(string) + if !nameOk || !nameIsStr || strings.TrimSpace(nameStr) == "" { + return ZliConfig{}, fmt.Errorf( + `%w: configs[%d]: "_name" must be a non-empty string`, + zerr.ErrCliBadConfig, i) + } + + urlRaw, urlOk := configMap[URLFlag] + urlStr, urlIsStr := urlRaw.(string) + if !urlOk || !urlIsStr || strings.TrimSpace(urlStr) == "" { + return ZliConfig{}, fmt.Errorf( + `%w: configs[%d]: "url" must be a non-empty string`, + zerr.ErrCliBadConfig, i) + } + + profile := ZliConfig{ + Name: nameStr, + URL: urlStr, + } + + if showSpinner, ok := configMap[showspinnerConfig]; ok { + profile.ShowSpinner = showSpinner + } + + if verifyTLS, ok := configMap[verifyTLSConfig]; ok { + profile.VerifyTLS = verifyTLS + } + + return profile, nil +} + +// WriteFile marshals this config to path with standard zli permissions. +func (f *ZliConfigFile) WriteFile(filePath string) error { + json := jsoniter.ConfigCompatibleWithStandardLibrary + + cfg := *f + if cfg.Configs == nil { + cfg.Configs = []ZliConfig{} + } + + marshalled, err := json.MarshalIndent(&cfg, "", " ") + if err != nil { + return err + } + + if err := os.WriteFile(filePath, marshalled, defaultFilePerms); err != nil { + return err + } + + return nil +} + +// Find returns the profile with the given name, applying defaults to the matched entry. +func (f *ZliConfigFile) Find(configName string) (*ZliConfig, error) { + for i := range f.Configs { + if f.Configs[i].Name == configName { + f.Configs[i].ApplyDefaults() + + return &f.Configs[i], nil + } + } + + return nil, zerr.ErrConfigNotFound +} + +// HasEntry reports whether a profile name already exists. +func (f *ZliConfigFile) HasEntry(configName string) bool { + return slices.ContainsFunc(f.Configs, func(c ZliConfig) bool { + return c.Name == configName + }) +} + +// AddEntry appends a new profile after validating URL and duplicate names. +func (f *ZliConfigFile) AddEntry(configName, urlStr string) error { + if err := validateURL(urlStr); err != nil { + return err + } + + if f.HasEntry(configName) { + return zerr.ErrDuplicateConfigName + } + + c := ZliConfig{ + Name: configName, + URL: urlStr, + } + c.ApplyDefaults() + f.Configs = append(f.Configs, c) + + return nil +} + +// RemoveEntry removes a profile by name. +func (f *ZliConfigFile) RemoveEntry(configName string) error { + for i, c := range f.Configs { + if c.Name != configName { + continue + } + + f.Configs = append(f.Configs[:i], f.Configs[i+1:]...) + + return nil + } + + return zerr.ErrConfigNotFound +} + +// FormatNames renders name and URL columns for `zli config --list`. +func (f *ZliConfigFile) FormatNames() (string, error) { + var builder strings.Builder + + writer := tabwriter.NewWriter(&builder, 0, 8, 1, '\t', tabwriter.AlignRight) //nolint:mnd + + for _, c := range f.Configs { + fmt.Fprintf(writer, "%s\t%s\n", c.Name, c.URL) + } + + if err := writer.Flush(); err != nil { + return "", err + } + + return builder.String(), nil +} + +// ApplyDefaults sets omitted boolean fields to their CLI defaults (mutates receiver). +func (c *ZliConfig) ApplyDefaults() { + if c.ShowSpinner == nil { + c.ShowSpinner = true + } + + if c.VerifyTLS == nil { + c.VerifyTLS = true + } +} + +// GetVar returns a single setting as text (after defaults apply via Find). +func (c *ZliConfig) GetVar(key string) (string, error) { + switch key { + case nameKey: + return c.Name, nil + case URLFlag: + return c.URL, nil + case showspinnerConfig: + if c.ShowSpinner == nil { + return "", nil + } + + return fmt.Sprintf("%v", c.ShowSpinner), nil + case verifyTLSConfig: + if c.VerifyTLS == nil { + return "", nil + } + + return fmt.Sprintf("%v", c.VerifyTLS), nil + default: + return "", zerr.ErrIllegalConfigKey + } +} + +// SetVar updates url / showspinner / verify-tls (does not persist). +func (c *ZliConfig) SetVar(key, value string) error { + if key == nameKey { + return zerr.ErrIllegalConfigKey + } + + if key != URLFlag && key != showspinnerConfig && key != verifyTLSConfig { + return zerr.ErrIllegalConfigKey + } + + boolVal, parseErr := strconv.ParseBool(value) + out := any(value) + if parseErr == nil { + out = boolVal + } + + switch key { + case URLFlag: + if err := validateURL(value); err != nil { + return err + } + + c.URL = value + case showspinnerConfig: + c.ShowSpinner = out + case verifyTLSConfig: + c.VerifyTLS = out + } + + return nil +} + +// ResetVar clears optional booleans (does not persist). +func (c *ZliConfig) ResetVar(key string) error { + switch key { + case URLFlag, nameKey: + return zerr.ErrCannotResetConfigKey + case showspinnerConfig: + c.ShowSpinner = nil + case verifyTLSConfig: + c.VerifyTLS = nil + default: + return zerr.ErrIllegalConfigKey + } + + return nil +} + +// FormatListedVars renders lines for `zli config --list`. +func (c *ZliConfig) FormatListedVars() string { + var builder strings.Builder + + fmt.Fprintf(&builder, "%s = %v\n", URLFlag, c.URL) + fmt.Fprintf(&builder, "%s = %v\n", showspinnerConfig, c.ShowSpinner) + fmt.Fprintf(&builder, "%s = %v\n", verifyTLSConfig, c.VerifyTLS) + + return builder.String() +} diff --git a/pkg/cli/client/config_cmd.go b/pkg/cli/client/config_cmd.go index 34a8e779..f24bda7e 100644 --- a/pkg/cli/client/config_cmd.go +++ b/pkg/cli/client/config_cmd.go @@ -3,26 +3,15 @@ package client import ( - "errors" "fmt" "os" - "path" - "slices" - "strconv" - "strings" - "text/tabwriter" + "path/filepath" - jsoniter "github.com/json-iterator/go" "github.com/spf13/cobra" zerr "zotregistry.dev/zot/v2/errors" ) -const ( - defaultConfigPerms = 0o644 - defaultFilePerms = 0o600 -) - func NewConfigCommand() *cobra.Command { var isListing bool @@ -40,11 +29,11 @@ func NewConfigCommand() *cobra.Command { return err } - configPath := path.Join(home, "/.zot") + configPath := filepath.Join(home, ".zot") switch len(args) { case noArgs: - if isListing { // zot config -l + if isListing { // zli config -l res, err := getConfigNames(configPath) if err != nil { return err @@ -57,7 +46,7 @@ func NewConfigCommand() *cobra.Command { return zerr.ErrInvalidArgs case oneArg: - // zot config -l + // zli config -l if isListing { res, err := getAllConfig(configPath, args[0]) if err != nil { @@ -71,17 +60,17 @@ func NewConfigCommand() *cobra.Command { return zerr.ErrInvalidArgs case twoArgs: - if isReset { // zot config --reset + if isReset { // zli config --reset return resetConfigValue(configPath, args[0], args[1]) } - // zot config + // zli config res, err := getConfigValue(configPath, args[0], args[1]) if err != nil { return err } fmt.Fprintln(cmd.OutOrStdout(), res) case threeArgs: - // zot config + // zli config if err := setConfigValue(configPath, args[0], args[1], args[2]); err != nil { return err } @@ -116,8 +105,8 @@ func NewConfigAddCommand() *cobra.Command { return err } - configPath := path.Join(home, "/.zot") - // zot config add + configPath := filepath.Join(home, ".zot") + // zli config add err = addConfig(configPath, args[0], args[1]) if err != nil { return err @@ -146,8 +135,8 @@ func NewConfigRemoveCommand() *cobra.Command { return err } - configPath := path.Join(home, "/.zot") - // zot config add + configPath := filepath.Join(home, ".zot") + // zli config remove err = removeConfig(configPath, args[0]) if err != nil { return err @@ -163,223 +152,95 @@ func NewConfigRemoveCommand() *cobra.Command { return configRemoveCmd } -func getConfigMapFromFile(filePath string) ([]any, error) { - file, err := os.OpenFile(filePath, os.O_RDONLY|os.O_CREATE, defaultConfigPerms) - if err != nil { - return nil, err - } - - file.Close() - - data, err := os.ReadFile(filePath) - if err != nil { - return nil, err - } - - var jsonMap map[string]any - - json := jsoniter.ConfigCompatibleWithStandardLibrary - - _ = json.Unmarshal(data, &jsonMap) - - if jsonMap["configs"] == nil { - return nil, zerr.ErrEmptyJSON - } - - configs, ok := jsonMap["configs"].([]any) - if !ok { - return nil, zerr.ErrCliBadConfig - } - - return configs, nil -} - -func saveConfigMapToFile(filePath string, configMap []any) error { - json := jsoniter.ConfigCompatibleWithStandardLibrary - - listMap := make(map[string]any) - listMap["configs"] = configMap - - marshalled, err := json.MarshalIndent(&listMap, "", " ") - if err != nil { - return err - } - - if err := os.WriteFile(filePath, marshalled, defaultFilePerms); err != nil { - return err - } - - return nil -} - func getConfigNames(configPath string) (string, error) { - configs, err := getConfigMapFromFile(configPath) + cfg, err := ReadZliConfigFile(configPath) if err != nil { - if errors.Is(err, zerr.ErrEmptyJSON) { + if isConfigUnavailable(err) { return "", nil } return "", err } - var builder strings.Builder - - writer := tabwriter.NewWriter(&builder, 0, 8, 1, '\t', tabwriter.AlignRight) //nolint:mnd - - for _, val := range configs { - configMap, ok := val.(map[string]any) - if !ok { - return "", zerr.ErrBadConfig - } - - fmt.Fprintf(writer, "%s\t%s\n", configMap[nameKey], configMap["url"]) - } - - err = writer.Flush() - if err != nil { - return "", err - } - - return builder.String(), nil + return cfg.FormatNames() } func addConfig(configPath, configName, url string) error { - configs, err := getConfigMapFromFile(configPath) - if err != nil && !errors.Is(err, zerr.ErrEmptyJSON) { - return err - } - - if err := validateURL(url); err != nil { - return err - } - - if configNameExists(configs, configName) { - return zerr.ErrDuplicateConfigName - } - - configMap := make(map[string]any) - configMap["url"] = url - configMap[nameKey] = configName - addDefaultConfigs(configMap) - configs = append(configs, configMap) - - err = saveConfigMapToFile(configPath, configs) + cfg, err := ReadZliConfigFile(configPath) if err != nil { - return err - } - - 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]any) - 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 { + if !isConfigUnavailable(err) { return err } - return nil + cfg = &ZliConfigFile{} } - return zerr.ErrConfigNotFound + if err := cfg.AddEntry(configName, url); err != nil { + return err + } + + return cfg.WriteFile(configPath) } -func addDefaultConfigs(config map[string]any) { - if _, ok := config[showspinnerConfig]; !ok { - config[showspinnerConfig] = true - } - - if _, ok := config[verifyTLSConfig]; !ok { - config[verifyTLSConfig] = true - } -} - -func getConfigValue(configPath, configName, key string) (string, error) { - configs, err := getConfigMapFromFile(configPath) +func removeConfig(configPath, configName string) error { + cfg, err := ReadZliConfigFile(configPath) if err != nil { - if errors.Is(err, zerr.ErrEmptyJSON) { - return "", zerr.ErrConfigNotFound - } - - return "", err - } - - for _, val := range configs { - configMap, ok := val.(map[string]any) - if !ok { - return "", zerr.ErrBadConfig - } - - addDefaultConfigs(configMap) - - name := configMap[nameKey] - if name == configName { - if configMap[key] == nil { - return "", nil - } - - return fmt.Sprintf("%v", configMap[key]), nil - } - } - - return "", zerr.ErrConfigNotFound -} - -func resetConfigValue(configPath, configName, key string) error { - if key == "url" || key == nameKey { - return zerr.ErrCannotResetConfigKey - } - - configs, err := getConfigMapFromFile(configPath) - if err != nil { - if errors.Is(err, zerr.ErrEmptyJSON) { + if isConfigUnavailable(err) { return zerr.ErrConfigNotFound } return err } - for _, val := range configs { - configMap, ok := val.(map[string]any) - if !ok { - return zerr.ErrBadConfig - } - - addDefaultConfigs(configMap) - - name := configMap[nameKey] - if name == configName { - delete(configMap, key) - - err = saveConfigMapToFile(configPath, configs) - if err != nil { - return err - } - - return nil - } + if err := cfg.RemoveEntry(configName); err != nil { + return err } - return zerr.ErrConfigNotFound + return cfg.WriteFile(configPath) +} + +func getConfigValue(configPath, configName, key string) (string, error) { + cfg, err := ReadZliConfigFile(configPath) + if err != nil { + if isConfigUnavailable(err) { + return "", zerr.ErrConfigNotFound + } + + return "", err + } + + c, err := cfg.Find(configName) + if err != nil { + return "", err + } + + return c.GetVar(key) +} + +func resetConfigValue(configPath, configName, key string) error { + if key == URLFlag || key == nameKey { + return zerr.ErrCannotResetConfigKey + } + + cfg, err := ReadZliConfigFile(configPath) + if err != nil { + if isConfigUnavailable(err) { + return zerr.ErrConfigNotFound + } + + return err + } + + c, err := cfg.Find(configName) + if err != nil { + return err + } + + if err := c.ResetVar(key); err != nil { + return err + } + + return cfg.WriteFile(configPath) } func setConfigValue(configPath, configName, key, value string) error { @@ -387,90 +248,43 @@ func setConfigValue(configPath, configName, key, value string) error { return zerr.ErrIllegalConfigKey } - configs, err := getConfigMapFromFile(configPath) + cfg, err := ReadZliConfigFile(configPath) if err != nil { - if errors.Is(err, zerr.ErrEmptyJSON) { + if isConfigUnavailable(err) { return zerr.ErrConfigNotFound } return err } - for _, val := range configs { - configMap, ok := val.(map[string]any) - if !ok { - return zerr.ErrBadConfig - } - - addDefaultConfigs(configMap) - - name := configMap[nameKey] - if name == configName { - boolVal, err := strconv.ParseBool(value) - if err == nil { - configMap[key] = boolVal - } else { - configMap[key] = value - } - - err = saveConfigMapToFile(configPath, configs) - if err != nil { - return err - } - - return nil - } + c, err := cfg.Find(configName) + if err != nil { + return err } - return zerr.ErrConfigNotFound + if err := c.SetVar(key, value); err != nil { + return err + } + + return cfg.WriteFile(configPath) } func getAllConfig(configPath, configName string) (string, error) { - configs, err := getConfigMapFromFile(configPath) + cfg, err := ReadZliConfigFile(configPath) if err != nil { - if errors.Is(err, zerr.ErrEmptyJSON) { + if isConfigUnavailable(err) { return "", nil } return "", err } - var builder strings.Builder - - for _, value := range configs { - configMap, ok := value.(map[string]any) - if !ok { - return "", zerr.ErrBadConfig - } - - addDefaultConfigs(configMap) - - name := configMap[nameKey] - if name == configName { - for key, val := range configMap { - if key == nameKey { - continue - } - - fmt.Fprintf(&builder, "%s = %v\n", key, val) - } - - return builder.String(), nil - } + c, err := cfg.Find(configName) + if err != nil { + return "", err } - return "", zerr.ErrConfigNotFound -} - -func configNameExists(configs []any, configName string) bool { - return slices.ContainsFunc(configs, func(val any) bool { - configMap, ok := val.(map[string]any) - if !ok { - return false - } - - return configMap[nameKey] == configName - }) + return c.FormatListedVars(), nil } const ( @@ -487,13 +301,8 @@ Useful variables: verify-tls enable TLS certificate verification of the server [default: true] ` - nameKey = "_name" - noArgs = 0 oneArg = 1 twoArgs = 2 threeArgs = 3 - - showspinnerConfig = "showspinner" - verifyTLSConfig = "verify-tls" ) diff --git a/pkg/cli/client/config_cmd_internal_test.go b/pkg/cli/client/config_cmd_internal_test.go new file mode 100644 index 00000000..6296e261 --- /dev/null +++ b/pkg/cli/client/config_cmd_internal_test.go @@ -0,0 +1,363 @@ +//go:build search + +package client + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + zerr "zotregistry.dev/zot/v2/errors" +) + +func writeTestZotFile(t *testing.T, dir, content string) string { + t.Helper() + + path := filepath.Join(dir, ".zot") + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + + return path +} + +// tempConfigPath returns ~/.zot path under an isolated temp dir; when writeFile is true, writes cfgContents first. +func tempConfigPath(t *testing.T, writeFile bool, cfgContents string) string { + t.Helper() + + dir := t.TempDir() + if writeFile { + return writeTestZotFile(t, dir, cfgContents) + } + + return filepath.Join(dir, ".zot") +} + +func TestGetConfigValue(t *testing.T) { + t.Parallel() + + validProfile := `{"configs":[{"_name":"a","url":"https://example.com"}]}` + + tests := []struct { + name string + cfgContents string // ignored when writeFile is false (missing ~/.zot until read) + writeFile bool + configName string + wantErrIs error + wantCliBadWrap bool // errors.Is(_, ErrCliBadConfig); implies !isConfigUnavailable + }{ + { + name: "fresh_missing_file_ErrConfigNotFound", + writeFile: false, + configName: "any", + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "fresh_empty_configs_ErrConfigNotFound", + cfgContents: `{}`, + writeFile: true, + configName: "any", + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "read_invalid_JSON_ErrCliBadConfig", + cfgContents: `not-json`, + writeFile: true, + configName: "any", + wantCliBadWrap: true, + }, + { + name: "profile_not_found_ErrConfigNotFound", + cfgContents: validProfile, + writeFile: true, + configName: "missing", + wantErrIs: zerr.ErrConfigNotFound, + }, + } + + for _, tableCase := range tests { + t.Run(tableCase.name, func(t *testing.T) { + t.Parallel() + + cfgPath := tempConfigPath(t, tableCase.writeFile, tableCase.cfgContents) + + got, err := getConfigValue(cfgPath, tableCase.configName, URLFlag) + + switch { + case tableCase.wantCliBadWrap: + require.Error(t, err) + require.False(t, isConfigUnavailable(err)) + require.True(t, errors.Is(err, zerr.ErrCliBadConfig)) + require.Equal(t, "", got) + case tableCase.wantErrIs != nil: + require.ErrorIs(t, err, tableCase.wantErrIs) + require.Equal(t, "", got) + default: + require.Failf(t, "table row must set wantErrIs or wantCliBadWrap: %s", tableCase.name) + } + }) + } + + t.Run("success", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + cfgPath := writeTestZotFile(t, dir, validProfile) + + got, err := getConfigValue(cfgPath, "a", URLFlag) + require.NoError(t, err) + require.Equal(t, "https://example.com", got) + }) +} + +func TestResetConfigValue(t *testing.T) { + t.Parallel() + + validProfile := `{"configs":[{"_name":"a","url":"https://example.com"}]}` + + tests := []struct { + name string + cfgContents string + writeFile bool + configName string + key string + wantErrIs error + wantCliBadWrap bool + }{ + { + name: "fresh_ErrConfigNotFound", + writeFile: false, + configName: "any", + key: showspinnerConfig, + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "cannot_reset_URL_before_read", + writeFile: false, + configName: "any", + key: URLFlag, + wantErrIs: zerr.ErrCannotResetConfigKey, + }, + { + name: "read_invalid_JSON_ErrCliBadConfig", + cfgContents: `not-json`, + writeFile: true, + configName: "a", + key: showspinnerConfig, + wantCliBadWrap: true, + }, + { + name: "profile_not_found_ErrConfigNotFound", + cfgContents: validProfile, + writeFile: true, + configName: "nobody", + key: showspinnerConfig, + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "ResetVar_illegal_key_ErrIllegalConfigKey", + cfgContents: validProfile, + writeFile: true, + configName: "a", + key: "bogus-key", + wantErrIs: zerr.ErrIllegalConfigKey, + }, + } + + for _, tableCase := range tests { + t.Run(tableCase.name, func(t *testing.T) { + t.Parallel() + + cfgPath := tempConfigPath(t, tableCase.writeFile, tableCase.cfgContents) + + err := resetConfigValue(cfgPath, tableCase.configName, tableCase.key) + + switch { + case tableCase.wantCliBadWrap: + require.Error(t, err) + require.False(t, isConfigUnavailable(err)) + require.True(t, errors.Is(err, zerr.ErrCliBadConfig)) + case tableCase.wantErrIs != nil: + require.ErrorIs(t, err, tableCase.wantErrIs) + default: + require.Failf(t, "incomplete table case: %s", tableCase.name) + } + }) + } + + t.Run("success_ResetVar_verify_tls_then_WriteFile", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + cfgPath := writeTestZotFile(t, dir, + `{"configs":[{"_name":"a","url":"https://example.com","verify-tls":false}]}`) + + require.NoError(t, resetConfigValue(cfgPath, "a", verifyTLSConfig)) + + cfg, err := ReadZliConfigFile(cfgPath) + require.NoError(t, err) + require.Len(t, cfg.Configs, 1) + require.Nil(t, cfg.Configs[0].VerifyTLS) + }) +} + +func TestSetConfigValue(t *testing.T) { + t.Parallel() + + validProfile := `{"configs":[{"_name":"a","url":"https://example.com"}]}` + + tests := []struct { + name string + cfgContents string + writeFile bool + configName string + key string + val string + wantErrIs error + wantCliBadWrap bool + }{ + { + name: "fresh_ErrConfigNotFound", + writeFile: false, + configName: "any", + key: URLFlag, + val: "https://example.com", + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "read_invalid_JSON_ErrCliBadConfig", + cfgContents: `not-json`, + writeFile: true, + configName: "a", + key: URLFlag, + val: "https://example.com", + wantCliBadWrap: true, + }, + { + name: "profile_not_found_ErrConfigNotFound", + cfgContents: validProfile, + writeFile: true, + configName: "nobody", + key: URLFlag, + val: "https://other.example", + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "SetVar_invalid_URL_ErrInvalidURL", + cfgContents: validProfile, + writeFile: true, + configName: "a", + key: URLFlag, + val: "not-a-valid-url", + wantErrIs: zerr.ErrInvalidURL, + }, + { + name: "SetVar_illegal_key_ErrIllegalConfigKey", + cfgContents: validProfile, + writeFile: true, + configName: "a", + key: "bogus-key", + val: "x", + wantErrIs: zerr.ErrIllegalConfigKey, + }, + { + name: "illegal_name_key_before_read_ErrIllegalConfigKey", + writeFile: false, + configName: "any", + key: nameKey, + val: "other", + wantErrIs: zerr.ErrIllegalConfigKey, + }, + } + + for _, tableCase := range tests { + t.Run(tableCase.name, func(t *testing.T) { + t.Parallel() + + cfgPath := tempConfigPath(t, tableCase.writeFile, tableCase.cfgContents) + + err := setConfigValue(cfgPath, tableCase.configName, tableCase.key, tableCase.val) + + switch { + case tableCase.wantCliBadWrap: + require.Error(t, err) + require.False(t, isConfigUnavailable(err)) + require.True(t, errors.Is(err, zerr.ErrCliBadConfig)) + case tableCase.wantErrIs != nil: + require.ErrorIs(t, err, tableCase.wantErrIs) + default: + require.Failf(t, "incomplete table case: %s", tableCase.name) + } + }) + } + + t.Run("success_SetVar_then_WriteFile", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + cfgPath := writeTestZotFile(t, dir, validProfile) + + require.NoError(t, setConfigValue(cfgPath, "a", showspinnerConfig, "false")) + + got, err := getConfigValue(cfgPath, "a", showspinnerConfig) + require.NoError(t, err) + require.Equal(t, "false", got) + }) +} + +func TestConfigCmd_listFreshAndFindErrors(t *testing.T) { + t.Parallel() + + validProfile := `{"configs":[{"_name":"a","url":"https://example.com"}]}` + + tests := []struct { + name string + writeFile bool + cfgContents string + runGetAll bool + configName string // only when runGetAll && wantErrIs != nil (Find miss) + wantOut string + wantErrIs error + }{ + { + name: "getAllConfig_fresh_returns_empty", + runGetAll: true, + wantOut: "", + }, + { + name: "getAllConfig_unknown_profile_ErrConfigNotFound", + writeFile: true, + cfgContents: validProfile, + runGetAll: true, + configName: "missing", + wantErrIs: zerr.ErrConfigNotFound, + }, + { + name: "getConfigNames_fresh_returns_empty", + wantOut: "", + }, + } + + for _, tableCase := range tests { + t.Run(tableCase.name, func(t *testing.T) { + t.Parallel() + + cfgPath := tempConfigPath(t, tableCase.writeFile, tableCase.cfgContents) + + switch { + case tableCase.runGetAll && tableCase.wantErrIs != nil: + _, err := getAllConfig(cfgPath, tableCase.configName) + require.ErrorIs(t, err, tableCase.wantErrIs) + case tableCase.runGetAll: + out, err := getAllConfig(cfgPath, "any") + require.NoError(t, err) + require.Equal(t, tableCase.wantOut, out) + default: + out, err := getConfigNames(cfgPath) + require.NoError(t, err) + require.Equal(t, tableCase.wantOut, out) + } + }) + } +} diff --git a/pkg/cli/client/config_cmd_test.go b/pkg/cli/client/config_cmd_test.go index 3d74a413..ae84dd8b 100644 --- a/pkg/cli/client/config_cmd_test.go +++ b/pkg/cli/client/config_cmd_test.go @@ -4,7 +4,7 @@ package client_test import ( "bytes" - "log" + "errors" "os" "regexp" "strings" @@ -93,28 +93,15 @@ func TestConfigCmdMain(t *testing.T) { _ = makeConfigFile(t, "") - err := os.Setenv("HOME", "nonExistentDirectory") - if err != nil { - panic(err) - } + t.Setenv("HOME", "nonExistentDirectory") cmd := client.NewConfigCommand() buff := bytes.NewBufferString("") cmd.SetOut(buff) cmd.SetErr(buff) cmd.SetArgs(args) - err = cmd.Execute() + err := cmd.Execute() So(err, ShouldNotBeNil) - - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - err = os.Setenv("HOME", home) - if err != nil { - log.Fatal(err) - } }) Convey("Test error on home directory at new add config", t, func() { @@ -122,28 +109,15 @@ func TestConfigCmdMain(t *testing.T) { _ = makeConfigFile(t, "") - err := os.Setenv("HOME", "nonExistentDirectory") - if err != nil { - panic(err) - } + t.Setenv("HOME", "nonExistentDirectory") cmd := client.NewConfigAddCommand() buff := bytes.NewBufferString("") cmd.SetOut(buff) cmd.SetErr(buff) cmd.SetArgs(args) - err = cmd.Execute() + err := cmd.Execute() So(err, ShouldNotBeNil) - - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - err = os.Setenv("HOME", home) - if err != nil { - log.Fatal(err) - } }) Convey("Test add config with invalid format", t, func() { @@ -157,7 +131,7 @@ func TestConfigCmdMain(t *testing.T) { cmd.SetErr(buff) cmd.SetArgs(args) err := cmd.Execute() - So(err, ShouldEqual, zerr.ErrCliBadConfig) + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) }) Convey("Test add config with invalid URL", t, func() { @@ -198,7 +172,7 @@ func TestConfigCmdMain(t *testing.T) { Convey("Test remove missing config entry", t, func() { args := []string{"remove", "configtest"} - _ = makeConfigFile(t, `{"configs":[]`) + _ = makeConfigFile(t, `{"configs":[]}`) cmd := client.NewConfigCommand() buff := bytes.NewBufferString("") @@ -222,7 +196,7 @@ func TestConfigCmdMain(t *testing.T) { cmd.SetArgs(args) err := cmd.Execute() So(err, ShouldNotBeNil) - So(buff.String(), ShouldContainSubstring, "config json is empty") + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) }) Convey("Test remove bad config file entry", t, func() { @@ -237,7 +211,7 @@ func TestConfigCmdMain(t *testing.T) { cmd.SetArgs(args) err := cmd.Execute() So(err, ShouldNotBeNil) - So(buff.String(), ShouldContainSubstring, "invalid server config") + So(buff.String(), ShouldContainSubstring, zerr.ErrCliBadConfig.Error()) }) Convey("Test remove config bad permissions", t, func() { diff --git a/pkg/cli/client/config_test.go b/pkg/cli/client/config_test.go new file mode 100644 index 00000000..aad1a545 --- /dev/null +++ b/pkg/cli/client/config_test.go @@ -0,0 +1,221 @@ +//go:build search + +package client_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + zerr "zotregistry.dev/zot/v2/errors" + "zotregistry.dev/zot/v2/pkg/cli/client" +) + +func writeZotFile(t *testing.T, dir, json string) string { + t.Helper() + + p := filepath.Join(dir, ".zot") + assert.NoError(t, os.WriteFile(p, []byte(json), 0o600)) + + return p +} + +func TestReadZliConfigFile_errors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfgContents string + wantSubs string + wantErrIs []error + }{ + { + name: "configs_not_array", + cfgContents: `{"configs":{"x":1}}`, + wantSubs: `field "configs" must be a JSON array`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "configs_element_not_object", + cfgContents: `{"configs":[1]}`, + wantSubs: "must be a JSON object", + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "missing_profile_name", + cfgContents: `{"configs":[{"url":"https://example.com"}]}`, + wantSubs: `"_name" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "empty_profile_name", + cfgContents: `{"configs":[{"_name":"","url":"https://example.com"}]}`, + wantSubs: `"_name" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "profile_name_not_string", + cfgContents: `{"configs":[{"_name":1,"url":"https://example.com"}]}`, + wantSubs: `"_name" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "missing_url", + cfgContents: `{"configs":[{"_name":"main"}]}`, + wantSubs: `"url" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "empty_url", + cfgContents: `{"configs":[{"_name":"main","url":""}]}`, + wantSubs: `"url" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "url_not_string", + cfgContents: `{"configs":[{"_name":"main","url":123}]}`, + wantSubs: `"url" must be a non-empty string`, + wantErrIs: []error{zerr.ErrCliBadConfig}, + }, + { + name: "missing_configs_field", + cfgContents: `{}`, + wantSubs: "", + wantErrIs: []error{zerr.ErrCliBadConfig, zerr.ErrCliMissingConfigsField}, + }, + } + + for _, tableCase := range tests { + t.Run(tableCase.name, func(t *testing.T) { + t.Parallel() + + p := writeZotFile(t, t.TempDir(), tableCase.cfgContents) + _, err := client.ReadZliConfigFile(p) + + require.Error(t, err) + + for _, target := range tableCase.wantErrIs { + require.ErrorIs(t, err, target) + } + + if tableCase.wantSubs != "" { + assert.Contains(t, err.Error(), tableCase.wantSubs) + } + }) + } +} + +func TestZliConfigFile_RemoveEntry_NotFound(t *testing.T) { + t.Parallel() + + f := client.ZliConfigFile{ + Configs: []client.ZliConfig{{Name: "only", URL: "https://example.com"}}, + } + + err := f.RemoveEntry("missing") + assert.ErrorIs(t, err, zerr.ErrConfigNotFound) +} + +func TestZliConfig_GetVar(t *testing.T) { + t.Parallel() + + zliCfg := client.ZliConfig{Name: "main", URL: "https://example.com"} + + okCases := []struct { + key string + want string + }{ + {"_name", "main"}, + {client.URLFlag, "https://example.com"}, + {"showspinner", ""}, + {"verify-tls", ""}, + } + + for _, okCase := range okCases { + t.Run(okCase.key, func(t *testing.T) { + t.Parallel() + + got, err := zliCfg.GetVar(okCase.key) + require.NoError(t, err) + assert.Equal(t, okCase.want, got) + }) + } + + t.Run("illegal_key", func(t *testing.T) { + t.Parallel() + + _, err := zliCfg.GetVar("not-a-real-key") + assert.ErrorIs(t, err, zerr.ErrIllegalConfigKey) + }) +} + +func TestZliConfig_SetVar(t *testing.T) { + t.Parallel() + + errCases := []struct { + name string + key string + val string + wantErr error + }{ + {"cannot_set_name", "_name", "other", zerr.ErrIllegalConfigKey}, + {"illegal_key", "bogus", "x", zerr.ErrIllegalConfigKey}, + {"invalid_url", client.URLFlag, "not-a-valid-url", zerr.ErrInvalidURL}, + } + + for _, errCase := range errCases { + t.Run(errCase.name, func(t *testing.T) { + t.Parallel() + + cfg := client.ZliConfig{Name: "main", URL: "https://example.com"} + assert.ErrorIs(t, cfg.SetVar(errCase.key, errCase.val), errCase.wantErr) + }) + } + + t.Run("parses_verify_tls_bool", func(t *testing.T) { + t.Parallel() + + cfg := client.ZliConfig{Name: "main", URL: "https://example.com"} + require.NoError(t, cfg.SetVar("verify-tls", "false")) + + v, ok := cfg.VerifyTLS.(bool) + assert.True(t, ok) + assert.False(t, v) + }) +} + +func TestZliConfig_ResetVar(t *testing.T) { + t.Parallel() + + base := client.ZliConfig{Name: "main", URL: "https://example.com"} + + errCases := []struct { + name string + key string + wantErr error + }{ + {"url", client.URLFlag, zerr.ErrCannotResetConfigKey}, + {"name", "_name", zerr.ErrCannotResetConfigKey}, + {"illegal_key", "bogus", zerr.ErrIllegalConfigKey}, + } + + for _, errCase := range errCases { + t.Run(errCase.name, func(t *testing.T) { + t.Parallel() + + cfg := base + assert.ErrorIs(t, cfg.ResetVar(errCase.key), errCase.wantErr) + }) + } + + t.Run("clears_verify_tls", func(t *testing.T) { + t.Parallel() + + cfg := client.ZliConfig{Name: "main", URL: "https://example.com", VerifyTLS: false} + require.NoError(t, cfg.ResetVar("verify-tls")) + assert.Nil(t, cfg.VerifyTLS) + }) +} diff --git a/pkg/cli/client/cve_cmd_internal_test.go b/pkg/cli/client/cve_cmd_internal_test.go index 7f562fe0..07e48eee 100644 --- a/pkg/cli/client/cve_cmd_internal_test.go +++ b/pkg/cli/client/cve_cmd_internal_test.go @@ -46,7 +46,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE help", t, func() { args := []string{"--help"} - _ = makeConfigFile(t, "") + t.Setenv("HOME", t.TempDir()) cmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -62,7 +62,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE help - with the shorthand", t, func() { args := []string{"-h"} - _ = makeConfigFile(t, "") + t.Setenv("HOME", t.TempDir()) cmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -75,7 +75,7 @@ func TestSearchCVECmd(t *testing.T) { So(err, ShouldBeNil) }) - Convey("Test CVE no url", t, func() { + Convey("Test CVE config missing mandatory url", t, func() { args := []string{"affected", "CVE-cveIdRandom", "--config", "cvetest"} _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) @@ -87,13 +87,13 @@ func TestSearchCVECmd(t *testing.T) { cmd.SetArgs(args) err := cmd.Execute() So(err, ShouldNotBeNil) - So(errors.Is(err, zerr.ErrNoURLProvided), ShouldBeTrue) + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) }) Convey("Test CVE invalid url", t, func() { args := []string{"list", "dummyImageName:tag", "--url", "invalidUrl"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewCVECommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -109,7 +109,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE invalid url port", t, func() { args := []string{"list", "dummyImageName:tag", "--url", "http://localhost:99999"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewCVECommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -124,7 +124,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE unreachable", t, func() { args := []string{"list", "dummyImageName:tag", "--url", "http://localhost:9999"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewCVECommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -186,7 +186,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by name and CVE ID - long option", t, func() { args := []string{"affected", "CVE-CVEID", "--repo", "dummyImageName", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -205,7 +205,7 @@ func TestSearchCVECmd(t *testing.T) { args := []string{"affected", "CVE-CVEID", "--repo", "dummyImageName", "--url", baseURL} buff := bytes.NewBufferString("") - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) cveCmd.SetOut(buff) @@ -222,7 +222,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by image name - in text format", t, func() { args := []string{"list", "dummyImageName:tag", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -253,7 +253,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by image name - in text format - in verbose mode", t, func() { args := []string{"list", "dummyImageName:tag", "--url", baseURL, "--verbose"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -291,7 +291,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by image name - in json format", t, func() { args := []string{"list", "dummyImageName:tag", "--url", baseURL, "-f", "json"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -312,7 +312,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by image name - in yaml format", t, func() { args := []string{"list", "dummyImageName:tag", "--url", baseURL, "-f", "yaml"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -331,7 +331,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test CVE by image name - invalid format", t, func() { args := []string{"list", "dummyImageName:tag", "--url", baseURL, "-f", "random"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -350,7 +350,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test images by CVE ID - positive", t, func() { args := []string{"affected", "CVE-CVEID", "--repo", "anImage", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -368,7 +368,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test images by CVE ID - positive with retries", t, func() { args := []string{"affected", "CVE-CVEID", "--repo", "anImage", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) mockService := mockServiceForRetry{ mockService: *newMockService(), @@ -393,7 +393,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test images by CVE ID - failed after retries", t, func() { args := []string{"affected", "CVE-CVEID", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) mockService := mockServiceForRetry{ mockService: *newMockService(), @@ -418,7 +418,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test images by CVE ID - invalid CVE ID", t, func() { args := []string{"affected", "CVE-invalidCVEID", "--config", "cvetest"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","url":"https://test-url.com","showspinner":false}]}`) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -432,7 +432,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test images by CVE ID - invalid url", t, func() { args := []string{"affected", "CVE-CVEID", "--url", "invalidURL"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -448,7 +448,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test fixed tags by and image name CVE ID - positive", t, func() { args := []string{"fixed", "fixedImage", "CVE-CVEID", "--url", baseURL} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cveCmd := NewCVECommand(newMockService()) buff := bytes.NewBufferString("") @@ -466,7 +466,7 @@ func TestSearchCVECmd(t *testing.T) { Convey("Test fixed tags by and image name CVE ID - invalid image name", t, func() { args := []string{"affected", "CVE-CVEID", "--image", "invalidImageName", "--config", "cvetest"} - _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","showspinner":false}]}`) + _ = makeConfigFile(t, `{"configs":[{"_name":"cvetest","url":"https://test-url.com","showspinner":false}]}`) cveCmd := NewCVECommand(NewSearchService()) buff := bytes.NewBufferString("") diff --git a/pkg/cli/client/image_cmd_internal_test.go b/pkg/cli/client/image_cmd_internal_test.go index d74a0f12..7939f551 100644 --- a/pkg/cli/client/image_cmd_internal_test.go +++ b/pkg/cli/client/image_cmd_internal_test.go @@ -7,9 +7,7 @@ import ( "context" "errors" "fmt" - "log" "os" - "path" "path/filepath" "regexp" "strings" @@ -35,7 +33,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("Test image help", t, func() { args := []string{"--help"} - _ = makeConfigFile(t, "") + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(newMockService()) buff := bytes.NewBufferString("") @@ -50,7 +48,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("with the shorthand", func() { args[0] = "-h" - _ = makeConfigFile(t, "") + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(newMockService()) buff := bytes.NewBufferString("") @@ -76,7 +74,7 @@ func TestSearchImageCmd(t *testing.T) { cmd.SetArgs(args) err := cmd.Execute() So(err, ShouldNotBeNil) - So(errors.Is(err, zerr.ErrNoURLProvided), ShouldBeTrue) + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) }) Convey("Test image invalid home directory", t, func() { @@ -84,34 +82,21 @@ func TestSearchImageCmd(t *testing.T) { _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","url":"https://test-url.com","showspinner":false}]}`) - err := os.Setenv("HOME", "nonExistentDirectory") - if err != nil { - panic(err) - } + t.Setenv("HOME", "nonExistentDirectory") cmd := NewImageCommand(newMockService()) buff := bytes.NewBufferString("") cmd.SetOut(buff) cmd.SetErr(buff) cmd.SetArgs(args) - err = cmd.Execute() + err := cmd.Execute() So(err, ShouldNotBeNil) - - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - err = os.Setenv("HOME", home) - if err != nil { - log.Fatal(err) - } }) Convey("Test image no params", t, func() { args := []string{"--url", "someUrl"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(newMockService()) buff := bytes.NewBufferString("") @@ -125,7 +110,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("Test image invalid url", t, func() { args := []string{"name", "dummyImageName", "--url", "invalidUrl"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -141,7 +126,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("Test image invalid url port", t, func() { args := []string{"name", "dummyImageName", "--url", "http://localhost:99999"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -155,7 +140,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("without flags", func() { args := []string{"list", "--url", "http://localhost:99999"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -171,7 +156,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("Test image unreachable", t, func() { args := []string{"name", "dummyImageName", "--url", "http://localhost:9999"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) cmd := NewImageCommand(NewSearchService()) buff := bytes.NewBufferString("") @@ -203,7 +188,7 @@ func TestSearchImageCmd(t *testing.T) { Convey("Test image by name", t, func() { args := []string{"name", "dummyImageName", "--url", "http://127.0.0.1:8080"} - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false}]}`) + t.Setenv("HOME", t.TempDir()) imageCmd := NewImageCommand(newMockService()) buff := &bytes.Buffer{} @@ -267,28 +252,15 @@ func TestListRepos(t *testing.T) { _ = makeConfigFile(t, `{"configs":[{"_name":"config-test","url":"https://test-url.com","showspinner":false}]}`) - err := os.Setenv("HOME", "nonExistentDirectory") - if err != nil { - panic(err) - } + t.Setenv("HOME", "nonExistentDirectory") cmd := NewRepoCommand(newMockService()) buff := bytes.NewBufferString("") cmd.SetOut(buff) cmd.SetErr(buff) cmd.SetArgs(args) - err = cmd.Execute() + err := cmd.Execute() So(err, ShouldNotBeNil) - - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - err = os.Setenv("HOME", home) - if err != nil { - log.Fatal(err) - } }) Convey("Test listing repositories error", t, func() { @@ -1522,15 +1494,11 @@ func (service *mockService) getImagesByDigest(ctx context.Context, config Search } func makeConfigFile(t *testing.T, content string) string { + t.Helper() tempDir := t.TempDir() - os.Setenv("HOME", tempDir) + t.Setenv("HOME", tempDir) - home, err := os.UserHomeDir() - if err != nil { - panic(err) - } - - configPath := path.Join(home, "/.zot") + configPath := filepath.Join(tempDir, ".zot") if err := os.WriteFile(configPath, []byte(content), 0o600); err != nil { panic(err) diff --git a/pkg/cli/client/search_functions_internal_test.go b/pkg/cli/client/search_functions_internal_test.go index 544edd68..6b38ae9c 100644 --- a/pkg/cli/client/search_functions_internal_test.go +++ b/pkg/cli/client/search_functions_internal_test.go @@ -939,7 +939,7 @@ func TestUtils(t *testing.T) { So(verifyTLS, ShouldBeFalse) // bad showspinner - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":"bad", "verify-tls": false}]}`) + _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","url":"https://test-url.com","showspinner":"bad", "verify-tls": false}]}`) cmd = &cobra.Command{} cmd.Flags().String(ConfigFlag, "imagetest", "") isSpinner, verifyTLS, err = GetCliConfigOptions(cmd) @@ -948,7 +948,7 @@ func TestUtils(t *testing.T) { So(verifyTLS, ShouldBeFalse) // bad verify-tls - _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","showspinner":false, "verify-tls": "bad"}]}`) + _ = makeConfigFile(t, `{"configs":[{"_name":"imagetest","url":"https://test-url.com","showspinner":false, "verify-tls": "bad"}]}`) cmd = &cobra.Command{} cmd.Flags().String(ConfigFlag, "imagetest", "") isSpinner, verifyTLS, err = GetCliConfigOptions(cmd) diff --git a/pkg/cli/client/utils.go b/pkg/cli/client/utils.go index d1a6f73f..37b63310 100644 --- a/pkg/cli/client/utils.go +++ b/pkg/cli/client/utils.go @@ -7,7 +7,7 @@ import ( "fmt" "io" "os" - "path" + "path/filepath" "strings" "sync" "time" @@ -457,14 +457,14 @@ func GetCliConfigOptions(cmd *cobra.Command) (bool, bool, error) { return false, false, err } - configDir := path.Join(home, "/.zot") + configPath := filepath.Join(home, ".zot") - isSpinner, err := parseBooleanConfig(configDir, configName, showspinnerConfig) + isSpinner, err := parseBooleanConfig(configPath, configName, showspinnerConfig) if err != nil { return false, false, err } - verifyTLS, err := parseBooleanConfig(configDir, configName, verifyTLSConfig) + verifyTLS, err := parseBooleanConfig(configPath, configName, verifyTLSConfig) if err != nil { return false, false, err } @@ -492,10 +492,6 @@ func GetServerURLFromFlags(cmd *cobra.Command) (string, error) { return serverURL, fmt.Errorf("reading url from config failed: %w", err) } - if serverURL == "" { - return "", fmt.Errorf("%w: url field from config is empty", zerr.ErrNoURLProvided) - } - if err := validateURL(serverURL); err != nil { return "", err } @@ -509,9 +505,9 @@ func ReadServerURLFromConfig(configName string) (string, error) { return "", err } - configDir := path.Join(home, "/.zot") + configPath := filepath.Join(home, ".zot") - urlFromConfig, err := getConfigValue(configDir, configName, "url") + urlFromConfig, err := getConfigValue(configPath, configName, URLFlag) if err != nil { return "", err }