diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index 21fdce2e21..e294347dcf 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -537,7 +537,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error if err != nil { return execUI.CreatingBatchSpecError(lr.MaxUnlicensedChangesets, err) } - previewURL := cfg.Endpoint + url + previewURL := cfg.endpointURL.JoinPath(url).String() execUI.CreatingBatchSpecSuccess(previewURL) hasWorkspaceFiles := false @@ -567,7 +567,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error if err != nil { return err } - execUI.ApplyingBatchSpecSuccess(cfg.Endpoint + batch.URL) + execUI.ApplyingBatchSpecSuccess(cfg.endpointURL.JoinPath(batch.URL).String()) return nil } diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 7dd7628c03..86ad3650d1 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -157,7 +157,7 @@ Examples: executionURL := fmt.Sprintf( "%s/%s/batch-changes/%s/executions/%s", - strings.TrimSuffix(cfg.Endpoint, "/"), + cfg.endpointURL, strings.TrimPrefix(namespace.URL, "/"), batchChangeName, batchSpecID, diff --git a/cmd/src/batch_repositories.go b/cmd/src/batch_repositories.go index b02a4f5e58..97a7ea6cfe 100644 --- a/cmd/src/batch_repositories.go +++ b/cmd/src/batch_repositories.go @@ -131,7 +131,7 @@ Examples: Max: max, RepoCount: len(repos), Repos: repos, - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), }); err != nil { return err } diff --git a/cmd/src/code_intel_upload.go b/cmd/src/code_intel_upload.go index 303e2199e0..031f38d4f0 100644 --- a/cmd/src/code_intel_upload.go +++ b/cmd/src/code_intel_upload.go @@ -7,7 +7,6 @@ import ( "flag" "fmt" "io" - "net/url" "os" "strings" "time" @@ -87,10 +86,7 @@ func handleCodeIntelUpload(args []string) error { return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err) } - uploadURL, err := makeCodeIntelUploadURL(uploadID) - if err != nil { - return err - } + uploadURL := makeCodeIntelUploadURL(uploadID) if codeintelUploadFlags.json { serialized, err := json.Marshal(map[string]any{ @@ -132,7 +128,7 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions { associatedIndexID = &codeintelUploadFlags.associatedIndexID } - cfg.AdditionalHeaders["Content-Type"] = "application/x-protobuf+scip" + cfg.additionalHeaders["Content-Type"] = "application/x-protobuf+scip" logger := upload.NewRequestLogger( os.Stdout, @@ -153,9 +149,9 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions { AssociatedIndexID: associatedIndexID, }, SourcegraphInstanceOptions: upload.SourcegraphInstanceOptions{ - SourcegraphURL: cfg.Endpoint, - AccessToken: cfg.AccessToken, - AdditionalHeaders: cfg.AdditionalHeaders, + SourcegraphURL: cfg.endpointURL.String(), + AccessToken: cfg.accessToken, + AdditionalHeaders: cfg.additionalHeaders, MaxRetries: 5, RetryInterval: time.Second, Path: codeintelUploadFlags.uploadRoute, @@ -191,16 +187,11 @@ func printInferredArguments(out *output.Output) { // makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier. // The base of the URL is constructed from the configured Sourcegraph instance. -func makeCodeIntelUploadURL(uploadID int) (string, error) { - url, err := url.Parse(cfg.Endpoint) - if err != nil { - return "", err - } - +func makeCodeIntelUploadURL(uploadID int) string { + u := *cfg.endpointURL graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID)) - url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID - url.User = nil - return url.String(), nil + u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID + return u.String() } type errorWithHint struct { diff --git a/cmd/src/debug_compose.go b/cmd/src/debug_compose.go index 8e26d95b04..32c746065d 100644 --- a/cmd/src/debug_compose.go +++ b/cmd/src/debug_compose.go @@ -75,7 +75,7 @@ Examples: return errors.Wrap(err, "failed to get containers for subcommand with err") } // Safety check user knows what they are targeting with this debug command - log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.Endpoint, base) + log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.endpointURL, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/debug_kube.go b/cmd/src/debug_kube.go index 69af7571e9..24f0b955c0 100644 --- a/cmd/src/debug_kube.go +++ b/cmd/src/debug_kube.go @@ -84,7 +84,7 @@ Examples: return errors.Wrapf(err, "failed to get current-context") } // Safety check user knows what they've targeted with this command - log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.Endpoint, kubectx, namespace, base) + log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.endpointURL, kubectx, namespace, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/debug_server.go b/cmd/src/debug_server.go index 8ef59fc02a..d219ec7fdb 100644 --- a/cmd/src/debug_server.go +++ b/cmd/src/debug_server.go @@ -72,7 +72,7 @@ Examples: defer zw.Close() // Safety check user knows what they are targeting with this debug command - log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.Endpoint, base) + log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.endpointURL, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/login.go b/cmd/src/login.go index ab5a097c71..d3d540cb50 100644 --- a/cmd/src/login.go +++ b/cmd/src/login.go @@ -44,17 +44,21 @@ Examples: if err := flagSet.Parse(args); err != nil { return err } - endpoint := cfg.Endpoint + if flagSet.NArg() >= 1 { - endpoint = flagSet.Arg(0) - } - if endpoint == "" { - return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set") + arg := flagSet.Arg(0) + parsed, err := parseEndpoint(arg) + if err != nil { + return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg)) + } + if parsed.String() != cfg.endpointURL.String() { + return cmderrors.Usage(fmt.Sprintf("endpoint argument %s conflicts with configured endpoint %s", parsed, cfg.endpointURL)) + } } client := cfg.apiClient(apiFlags, io.Discard) - return loginCmd(context.Background(), cfg, client, endpoint, os.Stdout) + return loginCmd(context.Background(), cfg, client, os.Stdout) } commands = append(commands, &command{ @@ -64,9 +68,7 @@ Examples: }) } -func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg string, out io.Writer) error { - endpointArg = cleanEndpoint(endpointArg) - +func loginCmd(ctx context.Context, cfg *config, client api.Client, out io.Writer) error { printProblem := func(problem string) { fmt.Fprintf(out, "❌ Problem: %s\n", problem) } @@ -77,23 +79,16 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s export SRC_ACCESS_TOKEN=(your access token) To verify that it's working, run the login command again. -`, endpointArg, endpointArg) +`, cfg.endpointURL, cfg.endpointURL) - if cfg.ConfigFilePath != "" { + if cfg.configFilePath != "" { fmt.Fprintln(out) - fmt.Fprintf(out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.ConfigFilePath) + fmt.Fprintf(out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.configFilePath) } - noToken := cfg.AccessToken == "" - endpointConflict := endpointArg != cfg.Endpoint - if noToken || endpointConflict { + if cfg.accessToken == "" { fmt.Fprintln(out) - switch { - case noToken: - printProblem("No access token is configured.") - case endpointConflict: - printProblem(fmt.Sprintf("The configured endpoint is %s, not %s.", cfg.Endpoint, endpointArg)) - } + printProblem("No access token is configured.") fmt.Fprintln(out, createAccessTokenMessage) return cmderrors.ExitCode1 } @@ -107,7 +102,7 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s if strings.HasPrefix(err.Error(), "error: 401 Unauthorized") || strings.HasPrefix(err.Error(), "error: 403 Forbidden") { printProblem("Invalid access token.") } else { - printProblem(fmt.Sprintf("Error communicating with %s: %s", endpointArg, err)) + printProblem(fmt.Sprintf("Error communicating with %s: %s", cfg.endpointURL, err)) } fmt.Fprintln(out, createAccessTokenMessage) fmt.Fprintln(out, " (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)") @@ -117,11 +112,11 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s if result.CurrentUser == nil { // This should never happen; we verified there is an access token, so there should always be // a user. - printProblem(fmt.Sprintf("Unable to determine user on %s.", endpointArg)) + printProblem(fmt.Sprintf("Unable to determine user on %s.", cfg.endpointURL)) return cmderrors.ExitCode1 } fmt.Fprintln(out) - fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, endpointArg) + fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, cfg.endpointURL) fmt.Fprintln(out) return nil } diff --git a/cmd/src/login_test.go b/cmd/src/login_test.go index 37fbf7a703..99d2c3d492 100644 --- a/cmd/src/login_test.go +++ b/cmd/src/login_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "strings" "testing" @@ -14,27 +15,17 @@ import ( ) func TestLogin(t *testing.T) { - check := func(t *testing.T, cfg *config, endpointArg string) (output string, err error) { + check := func(t *testing.T, cfg *config) (output string, err error) { t.Helper() var out bytes.Buffer - err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), endpointArg, &out) + err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), &out) return strings.TrimSpace(out.String()), err } - t.Run("different endpoint in config vs. arg", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com") - if err != cmderrors.ExitCode1 { - t.Fatal(err) - } - wantOut := "❌ Problem: No access token is configured.\n\n🛠 To fix: Create an access token by going to https://sourcegraph.example.com/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=https://sourcegraph.example.com\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again." - if out != wantOut { - t.Errorf("got output %q, want %q", out, wantOut) - } - }) - t.Run("no access token", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com") + endpoint := &url.URL{Scheme: "https", Host: "sourcegraph.example.com"} + out, err := check(t, &config{endpointURL: endpoint}) if err != cmderrors.ExitCode1 { t.Fatal(err) } @@ -45,7 +36,8 @@ func TestLogin(t *testing.T) { }) t.Run("warning when using config file", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com", ConfigFilePath: "f"}, "https://example.com") + endpoint := &url.URL{Scheme: "https", Host: "example.com"} + out, err := check(t, &config{endpointURL: endpoint, configFilePath: "f"}) if err != cmderrors.ExitCode1 { t.Fatal(err) } @@ -62,13 +54,13 @@ func TestLogin(t *testing.T) { })) defer s.Close() - endpoint := s.URL - out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint) + u, _ := url.ParseRequestURI(s.URL) + out, err := check(t, &config{endpointURL: u, accessToken: "x"}) if err != cmderrors.ExitCode1 { t.Fatal(err) } wantOut := "❌ Problem: Invalid access token.\n\n🛠 To fix: Create an access token by going to $ENDPOINT/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=$ENDPOINT\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again.\n\n (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)" - wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint) + wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL) if out != wantOut { t.Errorf("got output %q, want %q", out, wantOut) } @@ -81,13 +73,13 @@ func TestLogin(t *testing.T) { })) defer s.Close() - endpoint := s.URL - out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint) + u, _ := url.ParseRequestURI(s.URL) + out, err := check(t, &config{endpointURL: u, accessToken: "x"}) if err != nil { t.Fatal(err) } wantOut := "✔️ Authenticated as alice on $ENDPOINT" - wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint) + wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL) if out != wantOut { t.Errorf("got output %q, want %q", out, wantOut) } diff --git a/cmd/src/main.go b/cmd/src/main.go index edfb1073d7..547b6f5d58 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -107,29 +107,54 @@ func normalizeDashHelp(args []string) []string { return args } +func parseEndpoint(endpoint string) (*url.URL, error) { + u, err := url.ParseRequestURI(strings.TrimSuffix(endpoint, "/")) + if err != nil { + return nil, err + } + if !(u.Scheme == "http" || u.Scheme == "https") { + return nil, errors.Newf("invalid scheme %s: require http or https", u.Scheme) + } + if u.Host == "" { + return nil, errors.Newf("empty host") + } + // auth in the URL is not used, and could be explosed in log output. + // Explicitly clear it in case it's accidentally set in SRC_ENDPOINT or the config file. + u.User = nil + return u, nil +} + var cfg *config -// config represents the config format. +// config holds the resolved configuration used at runtime. type config struct { + accessToken string + additionalHeaders map[string]string + proxyURL *url.URL + proxyPath string + configFilePath string + endpointURL *url.URL +} + +// configFromFile holds the config as read from the config file, +// which is validated and parsed into the config struct. +type configFromFile struct { Endpoint string `json:"endpoint"` AccessToken string `json:"accessToken"` AdditionalHeaders map[string]string `json:"additionalHeaders"` Proxy string `json:"proxy"` - ProxyURL *url.URL - ProxyPath string - ConfigFilePath string } // apiClient returns an api.Client built from the configuration. func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client { return api.NewClient(api.ClientOpts{ - Endpoint: c.Endpoint, - AccessToken: c.AccessToken, - AdditionalHeaders: c.AdditionalHeaders, + EndpointURL: c.endpointURL, + AccessToken: c.accessToken, + AdditionalHeaders: c.additionalHeaders, Flags: flags, Out: out, - ProxyURL: c.ProxyURL, - ProxyPath: c.ProxyPath, + ProxyURL: c.proxyURL, + ProxyPath: c.proxyPath, }) } @@ -151,12 +176,20 @@ func readConfig() (*config, error) { if err != nil && (!os.IsNotExist(err) || userSpecified) { return nil, err } + + var cfgFromFile configFromFile var cfg config + var endpointStr string + var proxyStr string if err == nil { - cfg.ConfigFilePath = cfgPath - if err := json.Unmarshal(data, &cfg); err != nil { + cfg.configFilePath = cfgPath + if err := json.Unmarshal(data, &cfgFromFile); err != nil { return nil, err } + endpointStr = cfgFromFile.Endpoint + cfg.accessToken = cfgFromFile.AccessToken + cfg.additionalHeaders = cfgFromFile.AdditionalHeaders + proxyStr = cfgFromFile.Proxy } envToken := os.Getenv("SRC_ACCESS_TOKEN") @@ -177,21 +210,32 @@ func readConfig() (*config, error) { // Apply config overrides. if envToken != "" { - cfg.AccessToken = envToken + cfg.accessToken = envToken } if envEndpoint != "" { - cfg.Endpoint = envEndpoint + endpointStr = envEndpoint } - if cfg.Endpoint == "" { - cfg.Endpoint = "https://sourcegraph.com" + if endpointStr == "" { + endpointStr = "https://sourcegraph.com" } if envProxy != "" { - cfg.Proxy = envProxy + proxyStr = envProxy } - if cfg.Proxy != "" { + // Lastly, apply endpoint flag if set + if endpoint != nil && *endpoint != "" { + endpointStr = *endpoint + } + + if endpointURL, err := parseEndpoint(endpointStr); err != nil { + return nil, errors.Newf("invalid endpoint: %s", endpointStr) + } else { + cfg.endpointURL = endpointURL + } - parseEndpoint := func(endpoint string) (scheme string, address string) { + if proxyStr != "" { + + parseProxyEndpoint := func(endpoint string) (scheme string, address string) { parts := strings.SplitN(endpoint, "://", 2) if len(parts) == 2 { return parts[0], parts[1] @@ -205,15 +249,15 @@ func readConfig() (*config, error) { return slices.Contains(urlSchemes, scheme) } - scheme, address := parseEndpoint(cfg.Proxy) + scheme, address := parseProxyEndpoint(proxyStr) if isURLScheme(scheme) { - endpoint := cfg.Proxy + endpoint := proxyStr // assume socks means socks5, because that's all we support if scheme == "socks" { endpoint = "socks5://" + address } - cfg.ProxyURL, err = url.Parse(endpoint) + cfg.proxyURL, err = url.Parse(endpoint) if err != nil { return nil, err } @@ -224,38 +268,27 @@ func readConfig() (*config, error) { } isValidUDS, err := isValidUnixSocket(path) if err != nil { - return nil, errors.Newf("Invalid proxy configuration: %w", err) + return nil, errors.Newf("invalid proxy configuration: %w", err) } if !isValidUDS { return nil, errors.Newf("invalid proxy socket: %s", path) } - cfg.ProxyPath = path + cfg.proxyPath = path } else { - return nil, errors.Newf("invalid proxy endpoint: %s", cfg.Proxy) + return nil, errors.Newf("invalid proxy endpoint: %s", proxyStr) } } - cfg.AdditionalHeaders = parseAdditionalHeaders() + cfg.additionalHeaders = parseAdditionalHeaders() // Ensure that we're not clashing additonal headers - _, hasAuthorizationAdditonalHeader := cfg.AdditionalHeaders["authorization"] - if cfg.AccessToken != "" && hasAuthorizationAdditonalHeader { + _, hasAuthorizationAdditonalHeader := cfg.additionalHeaders["authorization"] + if cfg.accessToken != "" && hasAuthorizationAdditonalHeader { return nil, errConfigAuthorizationConflict } - // Lastly, apply endpoint flag if set - if endpoint != nil && *endpoint != "" { - cfg.Endpoint = *endpoint - } - - cfg.Endpoint = cleanEndpoint(cfg.Endpoint) - return &cfg, nil } -func cleanEndpoint(urlStr string) string { - return strings.TrimSuffix(urlStr, "/") -} - // isValidUnixSocket checks if the given path is a valid Unix socket. // // Parameters: @@ -275,7 +308,7 @@ func isValidUnixSocket(path string) (bool, error) { if os.IsNotExist(err) { return false, nil } - return false, errors.Newf("Not a UNIX Domain Socket: %v: %w", path, err) + return false, errors.Newf("not a UNIX Domain Socket: %v: %w", path, err) } defer conn.Close() diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index c37c36792a..95bccf5cbd 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/sourcegraph/src-cli/internal/api" ) @@ -29,7 +30,7 @@ func TestReadConfig(t *testing.T) { tests := []struct { name string - fileContents *config + fileContents *configFromFile envToken string envFooHeader string envHeaders string @@ -42,24 +43,29 @@ func TestReadConfig(t *testing.T) { { name: "defaults", want: &config{ - Endpoint: "https://sourcegraph.com", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + additionalHeaders: map[string]string{}, }, }, { name: "config file, no overrides, trim slash", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", Proxy: "https://proxy.com:8080", }, want: &config{ - Endpoint: "https://example.com", - AccessToken: "deadbeef", - AdditionalHeaders: map[string]string{}, - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "deadbeef", + additionalHeaders: map[string]string{}, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, @@ -67,7 +73,7 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, token override only", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", }, @@ -77,7 +83,7 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, endpoint override only", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", }, @@ -87,27 +93,29 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, proxy override only (allow)", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", Proxy: "https://proxy.com:8080", }, envProxy: "socks5://other.proxy.com:9999", want: &config{ - Endpoint: "https://example.com", - AccessToken: "deadbeef", - Proxy: "socks5://other.proxy.com:9999", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "deadbeef", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "other.proxy.com:9999", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "config file, all override", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", Proxy: "https://proxy.com:8080", @@ -116,48 +124,58 @@ func TestReadConfig(t *testing.T) { envEndpoint: "https://override.com", envProxy: "socks5://other.proxy.com:9999", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - Proxy: "socks5://other.proxy.com:9999", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "other.proxy.com:9999", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "no config file, token from environment", envToken: "abc", want: &config{ - Endpoint: "https://sourcegraph.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{}, }, }, { name: "no config file, endpoint from environment", envEndpoint: "https://example.com", want: &config{ - Endpoint: "https://example.com", - AccessToken: "", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "", + additionalHeaders: map[string]string{}, }, }, { name: "no config file, proxy from environment", envProxy: "https://proxy.com:8080", want: &config{ - Endpoint: "https://sourcegraph.com", - AccessToken: "", - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + accessToken: "", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { @@ -166,79 +184,92 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envProxy: "https://proxy.com:8080", want: &config{ - Endpoint: "https://example.com", - AccessToken: "abc", - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "abc", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "UNIX Domain Socket proxy using scheme and absolute path", envProxy: "unix://" + socketPath, want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "unix://" + socketPath, - ProxyPath: socketPath, - ProxyURL: nil, - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: socketPath, + proxyURL: nil, + additionalHeaders: map[string]string{}, }, }, { name: "UNIX Domain Socket proxy with absolute path", envProxy: socketPath, want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: socketPath, - ProxyPath: socketPath, - ProxyURL: nil, - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: socketPath, + proxyURL: nil, + additionalHeaders: map[string]string{}, }, }, { name: "socks --> socks5", envProxy: "socks://localhost:1080", want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "socks://localhost:1080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "localhost:1080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "socks5h", envProxy: "socks5h://localhost:1080", want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "socks5h://localhost:1080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5h", Host: "localhost:1080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "endpoint flag should override config", flagEndpoint: "https://override.com/", - fileContents: &config{ + fileContents: &configFromFile{ Endpoint: "https://example.com/", AccessToken: "deadbeef", AdditionalHeaders: map[string]string{}, }, want: &config{ - Endpoint: "https://override.com", - AccessToken: "deadbeef", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "deadbeef", + additionalHeaders: map[string]string{}, }, }, { @@ -247,9 +278,12 @@ func TestReadConfig(t *testing.T) { envEndpoint: "https://example.com", envToken: "abc", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{}, }, }, { @@ -259,9 +293,12 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envFooHeader: "bar", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{"foo": "bar"}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{"foo": "bar"}, }, }, { @@ -271,9 +308,12 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envHeaders: "foo:bar\nfoo-bar:bar-baz", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{"foo-bar": "bar-baz", "foo": "bar"}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{"foo-bar": "bar-baz", "foo": "bar"}, }, }, { @@ -331,8 +371,11 @@ func TestReadConfig(t *testing.T) { t.Fatal(err) } - config, err := readConfig() - if diff := cmp.Diff(test.want, config); diff != "" { + got, err := readConfig() + if diff := cmp.Diff(test.want, got, + cmp.AllowUnexported(config{}), + cmpopts.IgnoreFields(config{}, "configFilePath"), + ); diff != "" { t.Errorf("config: %v", diff) } var errMsg string diff --git a/cmd/src/search.go b/cmd/src/search.go index 385ac7c72d..a2cd1a0bbe 100644 --- a/cmd/src/search.go +++ b/cmd/src/search.go @@ -266,7 +266,7 @@ Other tips: } improved := searchResultsImproved{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), Query: queryString, Site: result.Site, searchResults: result.Search.Results, diff --git a/cmd/src/search_jobs.go b/cmd/src/search_jobs.go index 96c5d9070a..15bf5d8a25 100644 --- a/cmd/src/search_jobs.go +++ b/cmd/src/search_jobs.go @@ -156,8 +156,8 @@ func parseColumns(columnsFlag string) []string { // createSearchJobsClient creates a reusable API client for search jobs commands func createSearchJobsClient(out *flag.FlagSet, apiFlags *api.Flags) api.Client { return api.NewClient(api.ClientOpts{ - Endpoint: cfg.Endpoint, - AccessToken: cfg.AccessToken, + EndpointURL: cfg.endpointURL, + AccessToken: cfg.accessToken, Out: out.Output(), Flags: apiFlags, }) diff --git a/cmd/src/search_jobs_logs.go b/cmd/src/search_jobs_logs.go index 2fe9b6bfee..6327a609ab 100644 --- a/cmd/src/search_jobs_logs.go +++ b/cmd/src/search_jobs_logs.go @@ -22,7 +22,7 @@ func fetchJobLogs(jobID string, logURL string) (io.ReadCloser, error) { return nil, err } - req.Header.Add("Authorization", "token "+cfg.AccessToken) + req.Header.Add("Authorization", "token "+cfg.accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/cmd/src/search_jobs_results.go b/cmd/src/search_jobs_results.go index 2e45f8219f..9d8bc7a9ab 100644 --- a/cmd/src/search_jobs_results.go +++ b/cmd/src/search_jobs_results.go @@ -22,7 +22,7 @@ func fetchJobResults(jobID string, resultsURL string) (io.ReadCloser, error) { return nil, err } - req.Header.Add("Authorization", "token "+cfg.AccessToken) + req.Header.Add("Authorization", "token "+cfg.accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/cmd/src/search_stream.go b/cmd/src/search_stream.go index 512e7f9f75..9b5415851a 100644 --- a/cmd/src/search_stream.go +++ b/cmd/src/search_stream.go @@ -160,7 +160,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventRepoMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventRepoMatch: match, }) if err != nil { @@ -172,7 +172,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventCommitMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventCommitMatch: match, }) if err != nil { @@ -184,7 +184,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventSymbolMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventSymbolMatch: match, }, ) diff --git a/cmd/src/search_stream_test.go b/cmd/src/search_stream_test.go index 1653b273ac..71b099abb9 100644 --- a/cmd/src/search_stream_test.go +++ b/cmd/src/search_stream_test.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "testing" @@ -126,8 +127,9 @@ func TestSearchStream(t *testing.T) { s := testServer(t, http.HandlerFunc(mockStreamHandler)) defer s.Close() + u, _ := url.ParseRequestURI(s.URL) cfg = &config{ - Endpoint: s.URL, + endpointURL: u, } defer func() { cfg = nil }() diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 90de530c68..d67fff6be0 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -225,7 +225,7 @@ type UserToDelete struct { // Verify user wants to remove users with table of users and a command prompt for [y/N] func confirmUserRemoval(usersToDelete []UserToDelete, daysThreshold int, displayUsers bool) (bool, error) { if displayUsers { - fmt.Printf("Users to remove from %s\n", cfg.Endpoint) + fmt.Printf("Users to remove from %s\n", cfg.endpointURL) t := table.NewWriter() t.SetOutputMirror(os.Stdout) t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"}) @@ -243,7 +243,7 @@ func confirmUserRemoval(usersToDelete []UserToDelete, daysThreshold int, display } input := "" for strings.ToLower(input) != "y" && strings.ToLower(input) != "n" { - fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.Endpoint) + fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.endpointURL) if _, err := fmt.Scanln(&input); err != nil { return false, err } diff --git a/internal/api/api.go b/internal/api/api.go index 5f750c1d4a..40ca5c6750 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -12,7 +12,6 @@ import ( "net/url" "os" "runtime" - "strings" ioaux "github.com/jig/teereadcloser" "github.com/kballard/go-shellquote" @@ -71,7 +70,7 @@ type request struct { // ClientOpts encapsulates the options given to NewClient. type ClientOpts struct { - Endpoint string + EndpointURL *url.URL AccessToken string AdditionalHeaders map[string]string @@ -124,7 +123,7 @@ func NewClient(opts ClientOpts) Client { return &client{ opts: ClientOpts{ - Endpoint: opts.Endpoint, + EndpointURL: opts.EndpointURL, AccessToken: opts.AccessToken, AdditionalHeaders: opts.AdditionalHeaders, Flags: flags, @@ -159,7 +158,7 @@ func (c *client) NewHTTPRequest(ctx context.Context, method, p string, body io.R } func (c *client) createHTTPRequest(ctx context.Context, method, p string, body io.Reader) (*http.Request, error) { - req, err := http.NewRequestWithContext(ctx, method, strings.TrimRight(c.opts.Endpoint, "/")+"/"+p, body) + req, err := http.NewRequestWithContext(ctx, method, c.opts.EndpointURL.JoinPath(p).String(), body) if err != nil { return nil, err } @@ -334,6 +333,6 @@ func (r *request) curlCmd() (string, error) { s += fmt.Sprintf(" %s \\\n", shellquote.Join("-H", k+": "+v)) } s += fmt.Sprintf(" %s \\\n", shellquote.Join("-d", string(data))) - s += fmt.Sprintf(" %s", shellquote.Join(r.client.opts.Endpoint+"/.api/graphql")) + s += fmt.Sprintf(" %s", shellquote.Join(r.client.opts.EndpointURL.JoinPath(".api/graphql").String())) return s, nil } diff --git a/internal/batches/executor/executor_test.go b/internal/batches/executor/executor_test.go index 9fc96d927d..03f25e08a8 100644 --- a/internal/batches/executor/executor_test.go +++ b/internal/batches/executor/executor_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "runtime" @@ -406,7 +407,8 @@ func TestExecutor_Integration(t *testing.T) { // Setup an api.Client that points to this test server var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) // Temp dir for log files and downloaded archives testTempDir := t.TempDir() @@ -827,7 +829,8 @@ func testExecuteTasks(t *testing.T, tasks []*Task, archives ...mock.RepoArchive) t.Cleanup(ts.Close) var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) // Prepare images // diff --git a/internal/batches/repozip/fetcher_test.go b/internal/batches/repozip/fetcher_test.go index f871237e45..56d03d85a6 100644 --- a/internal/batches/repozip/fetcher_test.go +++ b/internal/batches/repozip/fetcher_test.go @@ -5,6 +5,7 @@ import ( "context" "net/http" "net/http/httptest" + "net/url" "os" "path" "path/filepath" @@ -44,7 +45,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -89,7 +91,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -153,7 +156,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -193,7 +197,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -262,7 +267,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client,