Skip to content

Commit fd5f1ac

Browse files
ryanslademrnugget
ryanslade
and
mrnugget
authored
Deprecate -endpoint flag (#235)
* Deprecate -endpoint flag This removes the documentation of the endpoint flag but still allows it to be used to override either the config or ennvironment variables. * Update changelog. * Update cmd/src/main.go Co-authored-by: Thorsten Ball <[email protected]> * Clean up CHANGELOG * Use t.Cleanup * Use Go 1.14 * Use Go 1.14 in Travis too * Try specific version of Go for Appveyor * Try example appveyor.yml * Revert "Try example appveyor.yml" This reverts commit 5f86ece. * One more attempt, use different GOPATH Apparently Go 1.14.3 is installed by default: https://www.appveyor.com/docs/windows-images-software/#golang * Revert back to being Go 1.13 compatible Co-authored-by: Thorsten Ball <[email protected]>
1 parent 037d69f commit fd5f1ac

File tree

3 files changed

+213
-14
lines changed

3 files changed

+213
-14
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ All notable changes to `src-cli` are documented in this file.
1414
### Added
1515

1616
- Pull missing docker images automatically. [#191](https://github.com/sourcegraph/src-cli/pull/191)
17-
1817
- 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)
1918

2019
### Changed
2120

2221
- 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)
22+
- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235)
2323

2424
### Fixed
2525

2626
### Removed
27+

‎cmd/src/main.go‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"os/user"
1010
"path/filepath"
1111
"strings"
12+
13+
"github.com/pkg/errors"
1214
)
1315

1416
const usageText = `src is a tool that provides access to Sourcegraph instances.
@@ -24,7 +26,6 @@ Environment variables
2426
2527
The options are:
2628
27-
-endpoint= specifies the endpoint to use e.g. "https://sourcegraph.com" (overrides SRC_ENDPOINT if set)
2829
-v print verbose output
2930
3031
The commands are:
@@ -47,9 +48,11 @@ Use "src [command] -h" for more information about a command.
4748
`
4849

4950
var (
51+
verbose = flag.Bool("v", false, "print verbose output")
52+
53+
// The following arguments are deprecated which is why they are no longer documented
5054
configPath = flag.String("config", "", "")
5155
endpoint = flag.String("endpoint", "", "")
52-
verbose = flag.Bool("v", false, "print verbose output")
5356
)
5457

5558
// commands contains all registered subcommands.
@@ -76,14 +79,14 @@ func readConfig() (*config, error) {
7679
cfgPath := *configPath
7780
userSpecified := *configPath != ""
7881

79-
user, err := user.Current()
82+
u, err := user.Current()
8083
if err != nil {
8184
return nil, err
8285
}
8386
if !userSpecified {
84-
cfgPath = filepath.Join(user.HomeDir, "src-config.json")
87+
cfgPath = filepath.Join(u.HomeDir, "src-config.json")
8588
} else if strings.HasPrefix(cfgPath, "~/") {
86-
cfgPath = filepath.Join(user.HomeDir, cfgPath[2:])
89+
cfgPath = filepath.Join(u.HomeDir, cfgPath[2:])
8790
}
8891
data, err := ioutil.ReadFile(os.ExpandEnv(cfgPath))
8992
if err != nil && (!os.IsNotExist(err) || userSpecified) {
@@ -96,23 +99,39 @@ func readConfig() (*config, error) {
9699
}
97100
}
98101

102+
envToken := os.Getenv("SRC_ACCESS_TOKEN")
103+
envEndpoint := os.Getenv("SRC_ENDPOINT")
104+
105+
if userSpecified {
106+
// If a config file is present, either zero or both environment variables must be present.
107+
// We don't want to partially apply environment variables.
108+
if envToken == "" && envEndpoint != "" {
109+
return nil, errConfigMerge
110+
}
111+
if envToken != "" && envEndpoint == "" {
112+
return nil, errConfigMerge
113+
}
114+
}
115+
99116
// Apply config overrides.
100-
if envToken := os.Getenv("SRC_ACCESS_TOKEN"); envToken != "" {
117+
if envToken != "" {
101118
cfg.AccessToken = envToken
102119
}
103-
if *endpoint != "" {
104-
cfg.Endpoint = *endpoint
105-
}
106-
if cfg.Endpoint == "" {
107-
if endpoint := os.Getenv("SRC_ENDPOINT"); endpoint != "" {
108-
cfg.Endpoint = endpoint
109-
}
120+
if envEndpoint != "" {
121+
cfg.Endpoint = envEndpoint
110122
}
111123
if cfg.Endpoint == "" {
112124
cfg.Endpoint = "https://sourcegraph.com"
113125
}
114126

127+
// Lastly, apply endpoint flag if set
128+
if endpoint != nil && *endpoint != "" {
129+
cfg.Endpoint = *endpoint
130+
}
131+
115132
cfg.Endpoint = strings.TrimSuffix(cfg.Endpoint, "/")
116133

117134
return &cfg, nil
118135
}
136+
137+
var errConfigMerge = errors.New("when using a configuration file, zero or all environment variables must be set")

‎cmd/src/main_test.go‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
package main
2+
3+
import (
4+
"encoding/json"
5+
"io/ioutil"
6+
"os"
7+
"path/filepath"
8+
"testing"
9+
10+
"github.com/google/go-cmp/cmp"
11+
)
12+
13+
func TestReadConfig(t *testing.T) {
14+
makeTempConfig := func(t *testing.T, c config) (string, func()) {
15+
data, err := json.Marshal(c)
16+
if err != nil {
17+
t.Fatal(err)
18+
}
19+
tmpDir, err := ioutil.TempDir("", "")
20+
if err != nil {
21+
t.Fatal(err)
22+
}
23+
filePath := filepath.Join(tmpDir, "config.json")
24+
err = ioutil.WriteFile(filePath, data, 0600)
25+
if err != nil {
26+
t.Fatal(err)
27+
}
28+
return filePath, func() { os.RemoveAll(tmpDir) }
29+
}
30+
31+
tests := []struct {
32+
name string
33+
fileContents *config
34+
envToken string
35+
envEndpoint string
36+
flagEndpoint string
37+
want *config
38+
wantErr string
39+
}{
40+
{
41+
name: "defaults",
42+
want: &config{
43+
Endpoint: "https://sourcegraph.com",
44+
},
45+
},
46+
{
47+
name: "config file, no overrides, trim slash",
48+
fileContents: &config{
49+
Endpoint: "https://example.com/",
50+
AccessToken: "deadbeef",
51+
},
52+
want: &config{
53+
Endpoint: "https://example.com",
54+
AccessToken: "deadbeef",
55+
},
56+
},
57+
{
58+
name: "config file, token override only",
59+
fileContents: &config{
60+
Endpoint: "https://example.com/",
61+
AccessToken: "deadbeef",
62+
},
63+
envToken: "abc",
64+
want: nil,
65+
wantErr: errConfigMerge.Error(),
66+
},
67+
{
68+
name: "config file, endpoint override only",
69+
fileContents: &config{
70+
Endpoint: "https://example.com/",
71+
AccessToken: "deadbeef",
72+
},
73+
envEndpoint: "https://exmaple2.com",
74+
want: nil,
75+
wantErr: errConfigMerge.Error(),
76+
},
77+
{
78+
name: "config file, both override",
79+
fileContents: &config{
80+
Endpoint: "https://example.com/",
81+
AccessToken: "deadbeef",
82+
},
83+
envToken: "abc",
84+
envEndpoint: "https://override.com",
85+
want: &config{
86+
Endpoint: "https://override.com",
87+
AccessToken: "abc",
88+
},
89+
},
90+
{
91+
name: "no config file, token from environment",
92+
envToken: "abc",
93+
want: &config{
94+
Endpoint: "https://sourcegraph.com",
95+
AccessToken: "abc",
96+
},
97+
},
98+
{
99+
name: "no config file, endpoint from environment",
100+
envEndpoint: "https://example.com",
101+
want: &config{
102+
Endpoint: "https://example.com",
103+
AccessToken: "",
104+
},
105+
},
106+
{
107+
name: "no config file, both variables",
108+
envEndpoint: "https://example.com",
109+
envToken: "abc",
110+
want: &config{
111+
Endpoint: "https://example.com",
112+
AccessToken: "abc",
113+
},
114+
},
115+
{
116+
name: "endpoint flag should override config",
117+
flagEndpoint: "https://override.com/",
118+
fileContents: &config{
119+
Endpoint: "https://example.com/",
120+
AccessToken: "deadbeef",
121+
},
122+
want: &config{
123+
Endpoint: "https://override.com",
124+
AccessToken: "deadbeef",
125+
},
126+
},
127+
{
128+
name: "endpoint flag should override environment",
129+
flagEndpoint: "https://override.com/",
130+
envEndpoint: "https://example.com",
131+
envToken: "abc",
132+
want: &config{
133+
Endpoint: "https://override.com",
134+
AccessToken: "abc",
135+
},
136+
},
137+
}
138+
139+
for _, test := range tests {
140+
t.Run(test.name, func(t *testing.T) {
141+
oldConfigPath := *configPath
142+
defer func() { *configPath = oldConfigPath }()
143+
144+
if test.flagEndpoint != "" {
145+
val := test.flagEndpoint
146+
endpoint = &val
147+
defer func() { endpoint = nil }()
148+
}
149+
150+
if test.fileContents != nil {
151+
p, cleanup := makeTempConfig(t, *test.fileContents)
152+
defer cleanup()
153+
*configPath = p
154+
}
155+
oldToken := os.Getenv("SRC_ACCESS_TOKEN")
156+
defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }()
157+
oldEndpoint := os.Getenv("SRC_ENDPOINT")
158+
defer func() { os.Setenv("SRC_ENDPOINT", oldEndpoint) }()
159+
160+
if err := os.Setenv("SRC_ACCESS_TOKEN", test.envToken); err != nil {
161+
t.Fatal(err)
162+
}
163+
if err := os.Setenv("SRC_ENDPOINT", test.envEndpoint); err != nil {
164+
t.Fatal(err)
165+
}
166+
config, err := readConfig()
167+
if diff := cmp.Diff(test.want, config); diff != "" {
168+
t.Errorf("config: %v", diff)
169+
}
170+
var errMsg string
171+
if err != nil {
172+
errMsg = err.Error()
173+
}
174+
if diff := cmp.Diff(test.wantErr, errMsg); diff != "" {
175+
t.Errorf("err: %v", diff)
176+
}
177+
})
178+
}
179+
}

0 commit comments

Comments
 (0)