Skip to content

Commit a167443

Browse files
ryansladeryanslade
authored
Remove -endpoint flag (#225)
* Remove -endpoint flag This change removes the -endpoint flag It also ensures the following: * Both env vars override the config file. * If a config file exists, either zero or both env vars must be specified. It is an error if only one is specified. This is to avoid confusion; we never merge config from the file and env vars. * Add CHANGELOG.md * Add another pending change
1 parent 3b9ea70 commit a167443

File tree

3 files changed

+197
-13
lines changed

3 files changed

+197
-13
lines changed

‎CHANGELOG.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!--
2+
###################################### READ ME ###########################################
3+
### This changelog should always be read on `master` branch. Its contents on version ###
4+
### branches do not necessarily reflect the changes that have gone into that branch. ###
5+
##########################################################################################
6+
-->
7+
8+
# Changelog
9+
10+
All notable changes to `src-cli` are documented in this file.
11+
12+
## Unreleased
13+
14+
### Added
15+
16+
- Pull missing docker images automatically. [#191](https://github.com/sourcegraph/src-cli/pull/191)
17+
18+
### Changed
19+
20+
### Fixed
21+
22+
### Removed
23+
24+
- Remove the `-endpoint` flag. [#225](https://github.com/sourcegraph/src-cli/pull/225)

‎cmd/src/main.go‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"encoding/json"
55
"flag"
6+
"github.com/pkg/errors"
67
"io/ioutil"
78
"log"
89
"os"
@@ -24,7 +25,6 @@ Environment variables
2425
2526
The options are:
2627
27-
-endpoint= specifies the endpoint to use e.g. "https://sourcegraph.com" (overrides SRC_ENDPOINT if set)
2828
-v print verbose output
2929
3030
The commands are:
@@ -48,7 +48,6 @@ Use "src [command] -h" for more information about a command.
4848

4949
var (
5050
configPath = flag.String("config", "", "")
51-
endpoint = flag.String("endpoint", "", "")
5251
verbose = flag.Bool("v", false, "print verbose output")
5352
)
5453

@@ -76,14 +75,14 @@ func readConfig() (*config, error) {
7675
cfgPath := *configPath
7776
userSpecified := *configPath != ""
7877

79-
user, err := user.Current()
78+
u, err := user.Current()
8079
if err != nil {
8180
return nil, err
8281
}
8382
if !userSpecified {
84-
cfgPath = filepath.Join(user.HomeDir, "src-config.json")
83+
cfgPath = filepath.Join(u.HomeDir, "src-config.json")
8584
} else if strings.HasPrefix(cfgPath, "~/") {
86-
cfgPath = filepath.Join(user.HomeDir, cfgPath[2:])
85+
cfgPath = filepath.Join(u.HomeDir, cfgPath[2:])
8786
}
8887
data, err := ioutil.ReadFile(os.ExpandEnv(cfgPath))
8988
if err != nil && (!os.IsNotExist(err) || userSpecified) {
@@ -96,17 +95,26 @@ func readConfig() (*config, error) {
9695
}
9796
}
9897

98+
envToken := os.Getenv("SRC_ACCESS_TOKEN")
99+
envEndpoint := os.Getenv("SRC_ENDPOINT")
100+
101+
if userSpecified {
102+
// If a config file is present, either zero or both environment variables must be present.
103+
// We don't want to partially apply environment variables.
104+
if envToken == "" && envEndpoint != "" {
105+
return nil, errConfigMerge
106+
}
107+
if envToken != "" && envEndpoint == "" {
108+
return nil, errConfigMerge
109+
}
110+
}
111+
99112
// Apply config overrides.
100-
if envToken := os.Getenv("SRC_ACCESS_TOKEN"); envToken != "" {
113+
if envToken != "" {
101114
cfg.AccessToken = envToken
102115
}
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-
}
116+
if envEndpoint != "" {
117+
cfg.Endpoint = envEndpoint
110118
}
111119
if cfg.Endpoint == "" {
112120
cfg.Endpoint = "https://sourcegraph.com"
@@ -116,3 +124,5 @@ func readConfig() (*config, error) {
116124

117125
return &cfg, nil
118126
}
127+
128+
var errConfigMerge = errors.New("config merging not supported, zero or both environment variables must be set")

‎cmd/src/main_test.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
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+
want *config
37+
wantErr string
38+
}{
39+
{
40+
name: "defaults",
41+
want: &config{
42+
Endpoint: "https://sourcegraph.com",
43+
},
44+
},
45+
{
46+
name: "config file, no overrides, trim slash",
47+
fileContents: &config{
48+
Endpoint: "https://example.com/",
49+
AccessToken: "deadbeef",
50+
},
51+
want: &config{
52+
Endpoint: "https://example.com",
53+
AccessToken: "deadbeef",
54+
},
55+
},
56+
{
57+
name: "config file, token override only",
58+
fileContents: &config{
59+
Endpoint: "https://example.com/",
60+
AccessToken: "deadbeef",
61+
},
62+
envToken: "abc",
63+
want: nil,
64+
wantErr: errConfigMerge.Error(),
65+
},
66+
{
67+
name: "config file, endpoint override only",
68+
fileContents: &config{
69+
Endpoint: "https://example.com/",
70+
AccessToken: "deadbeef",
71+
},
72+
envEndpoint: "https://exmaple2.com",
73+
want: nil,
74+
wantErr: errConfigMerge.Error(),
75+
},
76+
{
77+
name: "config file, both override",
78+
fileContents: &config{
79+
Endpoint: "https://example.com/",
80+
AccessToken: "deadbeef",
81+
},
82+
envToken: "abc",
83+
envEndpoint: "https://override.com",
84+
want: &config{
85+
Endpoint: "https://override.com",
86+
AccessToken: "abc",
87+
},
88+
},
89+
{
90+
name: "no config file, token from environment",
91+
envToken: "abc",
92+
want: &config{
93+
Endpoint: "https://sourcegraph.com",
94+
AccessToken: "abc",
95+
},
96+
},
97+
{
98+
name: "no config file, endpoint from environment",
99+
envEndpoint: "https://example.com",
100+
want: &config{
101+
Endpoint: "https://example.com",
102+
AccessToken: "",
103+
},
104+
},
105+
{
106+
name: "no config file, both variables",
107+
envEndpoint: "https://example.com",
108+
envToken: "abc",
109+
want: &config{
110+
Endpoint: "https://example.com",
111+
AccessToken: "abc",
112+
},
113+
},
114+
}
115+
116+
for _, test := range tests {
117+
t.Run(test.name, func(t *testing.T) {
118+
oldConfigPath := *configPath
119+
defer func() { *configPath = oldConfigPath }()
120+
121+
if test.fileContents != nil {
122+
p, cleanup := makeTempConfig(t, *test.fileContents)
123+
defer cleanup()
124+
*configPath = p
125+
}
126+
oldToken := os.Getenv("SRC_ACCESS_TOKEN")
127+
defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }()
128+
oldEndpoint := os.Getenv("SRC_ENDPOINT")
129+
defer func() { os.Setenv("SRC_ENDPOINT", oldEndpoint) }()
130+
131+
if err := os.Setenv("SRC_ACCESS_TOKEN", test.envToken); err != nil {
132+
t.Fatal(err)
133+
}
134+
if err := os.Setenv("SRC_ENDPOINT", test.envEndpoint); err != nil {
135+
t.Fatal(err)
136+
}
137+
config, err := readConfig()
138+
if diff := cmp.Diff(test.want, config); diff != "" {
139+
t.Errorf("config: %v", diff)
140+
}
141+
var errMsg string
142+
if err != nil {
143+
errMsg = err.Error()
144+
}
145+
if diff := cmp.Diff(test.wantErr, errMsg); diff != "" {
146+
t.Errorf("err: %v", diff)
147+
}
148+
})
149+
}
150+
}

0 commit comments

Comments
 (0)