Remove -endpoint flag

CHANGELOG.md

@@ -0,0 +1,24 @@
+<!--
+###################################### READ ME ###########################################
+### This changelog should always be read on `master` branch. Its contents on version   ###
+### branches do not necessarily reflect the changes that have gone into that branch.   ###
+##########################################################################################
+-->
+
+# Changelog
+
+All notable changes to `src-cli` are documented in this file.
+
+## Unreleased
+
+### Added
+
+- Pull missing docker images automatically. [#191](https://github.com/sourcegraph/src-cli/pull/191)
+
+### Changed
+
+### Fixed
+
+### Removed
+
+- Remove the `-endpoint` flag. [#225](https://github.com/sourcegraph/src-cli/pull/225)

cmd/src/main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"flag"
+	"github.com/pkg/errors"
 	"io/ioutil"
 	"log"
 	"os"
@@ -24,7 +25,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:
@@ -48,7 +48,6 @@ Use "src [command] -h" for more information about a command.
 
 var (
 	configPath = flag.String("config", "", "")
-	endpoint   = flag.String("endpoint", "", "")
 	verbose    = flag.Bool("v", false, "print verbose output")
 )
 
@@ -76,14 +75,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,17 +95,26 @@ 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"
@@ -116,3 +124,5 @@ func readConfig() (*config, error) {
 
 	return &cfg, nil
 }
+
+var errConfigMerge = errors.New("config merging not supported, zero or both environment variables must be set")

cmd/src/main_test.go

@@ -0,0 +1,150 @@
+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
+		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",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			oldConfigPath := *configPath
+			defer func() { *configPath = oldConfigPath }()
+
+			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)
+			}
+		})
+	}
+}