diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e541527..207529ecec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,13 +14,14 @@ All notable changes to `src-cli` are documented in this file. ### Added - Pull missing docker images automatically. [#191](https://github.com/sourcegraph/src-cli/pull/191) - - Searches that result in errors will now display any alerts returned by Sourcegraph, including suggestions for how the search could be corrected. [#221](https://github.com/sourcegraph/src-cli/pull/221) ### Changed - The terminal UI has been replaced by the logger-based UI that was previously only visible in verbose-mode (`-v`). [#228](https://github.com/sourcegraph/src-cli/pull/228) +- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) ### Fixed ### Removed + diff --git a/cmd/src/main.go b/cmd/src/main.go index e3bcc3b642..c91e9d0095 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -9,6 +9,8 @@ import ( "os/user" "path/filepath" "strings" + + "github.com/pkg/errors" ) const usageText = `src is a tool that provides access to Sourcegraph instances. @@ -24,7 +26,6 @@ Environment variables The options are: - -endpoint= specifies the endpoint to use e.g. "https://sourcegraph.com" (overrides SRC_ENDPOINT if set) -v print verbose output The commands are: @@ -47,9 +48,11 @@ Use "src [command] -h" for more information about a command. ` var ( + verbose = flag.Bool("v", false, "print verbose output") + + // The following arguments are deprecated which is why they are no longer documented configPath = flag.String("config", "", "") endpoint = flag.String("endpoint", "", "") - verbose = flag.Bool("v", false, "print verbose output") ) // commands contains all registered subcommands. @@ -76,14 +79,14 @@ func readConfig() (*config, error) { cfgPath := *configPath userSpecified := *configPath != "" - user, err := user.Current() + u, err := user.Current() if err != nil { return nil, err } if !userSpecified { - cfgPath = filepath.Join(user.HomeDir, "src-config.json") + cfgPath = filepath.Join(u.HomeDir, "src-config.json") } else if strings.HasPrefix(cfgPath, "~/") { - cfgPath = filepath.Join(user.HomeDir, cfgPath[2:]) + cfgPath = filepath.Join(u.HomeDir, cfgPath[2:]) } data, err := ioutil.ReadFile(os.ExpandEnv(cfgPath)) if err != nil && (!os.IsNotExist(err) || userSpecified) { @@ -96,23 +99,39 @@ func readConfig() (*config, error) { } } + envToken := os.Getenv("SRC_ACCESS_TOKEN") + envEndpoint := os.Getenv("SRC_ENDPOINT") + + if userSpecified { + // If a config file is present, either zero or both environment variables must be present. + // We don't want to partially apply environment variables. + if envToken == "" && envEndpoint != "" { + return nil, errConfigMerge + } + if envToken != "" && envEndpoint == "" { + return nil, errConfigMerge + } + } + // Apply config overrides. - if envToken := os.Getenv("SRC_ACCESS_TOKEN"); envToken != "" { + if envToken != "" { cfg.AccessToken = envToken } - if *endpoint != "" { - cfg.Endpoint = *endpoint - } - if cfg.Endpoint == "" { - if endpoint := os.Getenv("SRC_ENDPOINT"); endpoint != "" { - cfg.Endpoint = endpoint - } + if envEndpoint != "" { + cfg.Endpoint = envEndpoint } if cfg.Endpoint == "" { cfg.Endpoint = "https://sourcegraph.com" } + // Lastly, apply endpoint flag if set + if endpoint != nil && *endpoint != "" { + cfg.Endpoint = *endpoint + } + cfg.Endpoint = strings.TrimSuffix(cfg.Endpoint, "/") return &cfg, nil } + +var errConfigMerge = errors.New("when using a configuration file, zero or all environment variables must be set") diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go new file mode 100644 index 0000000000..91bbff7885 --- /dev/null +++ b/cmd/src/main_test.go @@ -0,0 +1,179 @@ +package main + +import ( + "encoding/json" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestReadConfig(t *testing.T) { + makeTempConfig := func(t *testing.T, c config) (string, func()) { + data, err := json.Marshal(c) + if err != nil { + t.Fatal(err) + } + tmpDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + filePath := filepath.Join(tmpDir, "config.json") + err = ioutil.WriteFile(filePath, data, 0600) + if err != nil { + t.Fatal(err) + } + return filePath, func() { os.RemoveAll(tmpDir) } + } + + tests := []struct { + name string + fileContents *config + envToken string + envEndpoint string + flagEndpoint string + want *config + wantErr string + }{ + { + name: "defaults", + want: &config{ + Endpoint: "https://sourcegraph.com", + }, + }, + { + name: "config file, no overrides, trim slash", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + want: &config{ + Endpoint: "https://example.com", + AccessToken: "deadbeef", + }, + }, + { + name: "config file, token override only", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envToken: "abc", + want: nil, + wantErr: errConfigMerge.Error(), + }, + { + name: "config file, endpoint override only", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envEndpoint: "https://exmaple2.com", + want: nil, + wantErr: errConfigMerge.Error(), + }, + { + name: "config file, both override", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envToken: "abc", + envEndpoint: "https://override.com", + want: &config{ + Endpoint: "https://override.com", + AccessToken: "abc", + }, + }, + { + name: "no config file, token from environment", + envToken: "abc", + want: &config{ + Endpoint: "https://sourcegraph.com", + AccessToken: "abc", + }, + }, + { + name: "no config file, endpoint from environment", + envEndpoint: "https://example.com", + want: &config{ + Endpoint: "https://example.com", + AccessToken: "", + }, + }, + { + name: "no config file, both variables", + envEndpoint: "https://example.com", + envToken: "abc", + want: &config{ + Endpoint: "https://example.com", + AccessToken: "abc", + }, + }, + { + name: "endpoint flag should override config", + flagEndpoint: "https://override.com/", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + want: &config{ + Endpoint: "https://override.com", + AccessToken: "deadbeef", + }, + }, + { + name: "endpoint flag should override environment", + flagEndpoint: "https://override.com/", + envEndpoint: "https://example.com", + envToken: "abc", + want: &config{ + Endpoint: "https://override.com", + AccessToken: "abc", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + oldConfigPath := *configPath + defer func() { *configPath = oldConfigPath }() + + if test.flagEndpoint != "" { + val := test.flagEndpoint + endpoint = &val + defer func() { endpoint = nil }() + } + + if test.fileContents != nil { + p, cleanup := makeTempConfig(t, *test.fileContents) + defer cleanup() + *configPath = p + } + oldToken := os.Getenv("SRC_ACCESS_TOKEN") + defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }() + oldEndpoint := os.Getenv("SRC_ENDPOINT") + defer func() { os.Setenv("SRC_ENDPOINT", oldEndpoint) }() + + if err := os.Setenv("SRC_ACCESS_TOKEN", test.envToken); err != nil { + t.Fatal(err) + } + if err := os.Setenv("SRC_ENDPOINT", test.envEndpoint); err != nil { + t.Fatal(err) + } + config, err := readConfig() + if diff := cmp.Diff(test.want, config); diff != "" { + t.Errorf("config: %v", diff) + } + var errMsg string + if err != nil { + errMsg = err.Error() + } + if diff := cmp.Diff(test.wantErr, errMsg); diff != "" { + t.Errorf("err: %v", diff) + } + }) + } +}