Skip to content

Commit 50efd9a

Browse files
peterguyampcode-com
andcommitted
refactor: use parsed *url.URL types instead of string endpoints
- config struct: unexport fields, replace Endpoint/Proxy strings with endpointURL/proxyURL (*url.URL) and proxyPath, parse once in readConfig - api.ClientOpts: Endpoint string → EndpointURL *url.URL - login: accept *url.URL, move endpoint conflict check to handler - code_intel_upload: simplify makeCodeIntelUploadURL with stack copy, remove error return - api.go: use JoinPath for URL construction instead of string concat - Update all call sites and tests Amp-Thread-ID: https://ampcode.com/threads/T-019cc5da-fb05-71f8-962b-689cfc5b494d Co-authored-by: Amp <amp@ampcode.com>
1 parent c6de254 commit 50efd9a

23 files changed

Lines changed: 327 additions & 276 deletions

cmd/src/batch_common.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
537537
if err != nil {
538538
return execUI.CreatingBatchSpecError(lr.MaxUnlicensedChangesets, err)
539539
}
540-
previewURL := cfg.Endpoint + url
540+
previewURL := cfg.endpointURL.JoinPath(url).String()
541541
execUI.CreatingBatchSpecSuccess(previewURL)
542542

543543
hasWorkspaceFiles := false
@@ -567,7 +567,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
567567
if err != nil {
568568
return err
569569
}
570-
execUI.ApplyingBatchSpecSuccess(cfg.Endpoint + batch.URL)
570+
execUI.ApplyingBatchSpecSuccess(cfg.endpointURL.JoinPath(batch.URL).String())
571571

572572
return nil
573573
}

cmd/src/batch_remote.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ Examples:
157157

158158
executionURL := fmt.Sprintf(
159159
"%s/%s/batch-changes/%s/executions/%s",
160-
strings.TrimSuffix(cfg.Endpoint, "/"),
160+
cfg.endpointURL,
161161
strings.TrimPrefix(namespace.URL, "/"),
162162
batchChangeName,
163163
batchSpecID,

cmd/src/batch_repositories.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ Examples:
131131
Max: max,
132132
RepoCount: len(repos),
133133
Repos: repos,
134-
SourcegraphEndpoint: cfg.Endpoint,
134+
SourcegraphEndpoint: cfg.endpointURL.String(),
135135
}); err != nil {
136136
return err
137137
}

cmd/src/code_intel_upload.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"flag"
88
"fmt"
99
"io"
10-
"net/url"
1110
"os"
1211
"strings"
1312
"time"
@@ -87,10 +86,7 @@ func handleCodeIntelUpload(args []string) error {
8786
return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err)
8887
}
8988

90-
uploadURL, err := makeCodeIntelUploadURL(uploadID)
91-
if err != nil {
92-
return err
93-
}
89+
uploadURL := makeCodeIntelUploadURL(uploadID)
9490

9591
if codeintelUploadFlags.json {
9692
serialized, err := json.Marshal(map[string]any{
@@ -132,7 +128,7 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
132128
associatedIndexID = &codeintelUploadFlags.associatedIndexID
133129
}
134130

135-
cfg.AdditionalHeaders["Content-Type"] = "application/x-protobuf+scip"
131+
cfg.additionalHeaders["Content-Type"] = "application/x-protobuf+scip"
136132

137133
logger := upload.NewRequestLogger(
138134
os.Stdout,
@@ -153,9 +149,9 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
153149
AssociatedIndexID: associatedIndexID,
154150
},
155151
SourcegraphInstanceOptions: upload.SourcegraphInstanceOptions{
156-
SourcegraphURL: cfg.Endpoint,
157-
AccessToken: cfg.AccessToken,
158-
AdditionalHeaders: cfg.AdditionalHeaders,
152+
SourcegraphURL: cfg.endpointURL.String(),
153+
AccessToken: cfg.accessToken,
154+
AdditionalHeaders: cfg.additionalHeaders,
159155
MaxRetries: 5,
160156
RetryInterval: time.Second,
161157
Path: codeintelUploadFlags.uploadRoute,
@@ -191,16 +187,12 @@ func printInferredArguments(out *output.Output) {
191187

192188
// makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier.
193189
// The base of the URL is constructed from the configured Sourcegraph instance.
194-
func makeCodeIntelUploadURL(uploadID int) (string, error) {
195-
url, err := url.Parse(cfg.Endpoint)
196-
if err != nil {
197-
return "", err
198-
}
199-
190+
func makeCodeIntelUploadURL(uploadID int) string {
191+
u := *cfg.endpointURL
200192
graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID))
201-
url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID
202-
url.User = nil
203-
return url.String(), nil
193+
u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID
194+
u.User = nil
195+
return u.String()
204196
}
205197

206198
type errorWithHint struct {

cmd/src/debug_compose.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Examples:
7575
return errors.Wrap(err, "failed to get containers for subcommand with err")
7676
}
7777
// Safety check user knows what they are targeting with this debug command
78-
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)
78+
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)
7979
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
8080
return nil
8181
}

cmd/src/debug_kube.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Examples:
8484
return errors.Wrapf(err, "failed to get current-context")
8585
}
8686
// Safety check user knows what they've targeted with this command
87-
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)
87+
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)
8888
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
8989
return nil
9090
}

cmd/src/debug_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Examples:
7272
defer zw.Close()
7373

7474
// Safety check user knows what they are targeting with this debug command
75-
log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.Endpoint, base)
75+
log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.endpointURL, base)
7676
if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
7777
return nil
7878
}

cmd/src/login.go

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,23 @@ Examples:
5454
if err := flagSet.Parse(args); err != nil {
5555
return err
5656
}
57-
endpoint := cfg.Endpoint
57+
5858
if flagSet.NArg() >= 1 {
59-
endpoint = flagSet.Arg(0)
60-
}
61-
if endpoint == "" {
62-
return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set")
59+
arg := flagSet.Arg(0)
60+
parsed, err := parseEndpoint(arg)
61+
if err != nil {
62+
return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg))
63+
}
64+
if parsed.String() != cfg.endpointURL.String() {
65+
return cmderrors.Usage(fmt.Sprintf("The configured endpoint is %s, not %s", cfg.endpointURL, parsed))
66+
}
6367
}
6468

6569
client := cfg.apiClient(apiFlags, io.Discard)
6670

6771
return loginCmd(context.Background(), loginParams{
6872
cfg: cfg,
6973
client: client,
70-
endpoint: endpoint,
7174
out: os.Stdout,
7275
useOAuth: *useOAuth,
7376
apiFlags: apiFlags,
@@ -85,7 +88,6 @@ Examples:
8588
type loginParams struct {
8689
cfg *config
8790
client api.Client
88-
endpoint string
8991
out io.Writer
9092
useOAuth bool
9193
apiFlags *api.Flags
@@ -99,16 +101,15 @@ type loginFlowKind int
99101
const (
100102
loginFlowOAuth loginFlowKind = iota
101103
loginFlowMissingAuth
102-
loginFlowEndpointConflict
103104
loginFlowValidate
104105
)
105106

106107
var loadStoredOAuthToken = oauth.LoadToken
107108

108109
func loginCmd(ctx context.Context, p loginParams) error {
109-
if p.cfg.ConfigFilePath != "" {
110+
if p.cfg.configFilePath != "" {
110111
fmt.Fprintln(p.out)
111-
fmt.Fprintf(p.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", p.cfg.ConfigFilePath)
112+
fmt.Fprintf(p.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", p.cfg.configFilePath)
112113
}
113114

114115
_, flow := selectLoginFlow(ctx, p)
@@ -117,28 +118,23 @@ func loginCmd(ctx context.Context, p loginParams) error {
117118

118119
// selectLoginFlow decides what login flow to run based on flags and config.
119120
func selectLoginFlow(ctx context.Context, p loginParams) (loginFlowKind, loginFlow) {
120-
endpointArg := cleanEndpoint(p.endpoint)
121-
122121
if p.useOAuth {
123122
return loginFlowOAuth, runOAuthLogin
124123
}
125-
if !hasEffectiveAuth(ctx, p.cfg, endpointArg) {
124+
if !hasEffectiveAuth(ctx, p.cfg) {
126125
return loginFlowMissingAuth, runMissingAuthLogin
127126
}
128-
if endpointArg != p.cfg.Endpoint {
129-
return loginFlowEndpointConflict, runEndpointConflictLogin
130-
}
131127
return loginFlowValidate, runValidatedLogin
132128
}
133129

134130
// hasEffectiveAuth determines whether we have auth credentials to continue. It first checks for a resolved Access Token in
135131
// config, then it checks for a stored OAuth token.
136-
func hasEffectiveAuth(ctx context.Context, cfg *config, resolvedEndpoint string) bool {
137-
if cfg.AccessToken != "" {
132+
func hasEffectiveAuth(ctx context.Context, cfg *config) bool {
133+
if cfg.accessToken != "" {
138134
return true
139135
}
140136

141-
if _, err := loadStoredOAuthToken(ctx, resolvedEndpoint); err == nil {
137+
if _, err := loadStoredOAuthToken(ctx, cfg.endpointURL.String()); err == nil {
142138
return true
143139
}
144140

cmd/src/login_oauth.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ import (
1414
)
1515

1616
func runOAuthLogin(ctx context.Context, p loginParams) error {
17-
endpointArg := cleanEndpoint(p.endpoint)
18-
client, err := oauthLoginClient(ctx, p, endpointArg)
17+
endpoint := p.cfg.endpointURL.String()
18+
client, err := oauthLoginClient(ctx, p, endpoint)
1919
if err != nil {
2020
printLoginProblem(p.out, fmt.Sprintf("OAuth Device flow authentication failed: %s", err))
21-
fmt.Fprintln(p.out, loginAccessTokenMessage(endpointArg))
21+
fmt.Fprintln(p.out, loginAccessTokenMessage(endpoint))
2222
return cmderrors.ExitCode1
2323
}
2424

25-
if err := validateCurrentUser(ctx, client, p.out, endpointArg); err != nil {
25+
if err := validateCurrentUser(ctx, client, p.out, endpoint); err != nil {
2626
return err
2727
}
2828

@@ -44,12 +44,12 @@ func oauthLoginClient(ctx context.Context, p loginParams, endpoint string) (api.
4444
}
4545

4646
return api.NewClient(api.ClientOpts{
47-
Endpoint: p.cfg.Endpoint,
48-
AdditionalHeaders: p.cfg.AdditionalHeaders,
47+
EndpointURL: p.cfg.endpointURL,
48+
AdditionalHeaders: p.cfg.additionalHeaders,
4949
Flags: p.apiFlags,
5050
Out: p.out,
51-
ProxyURL: p.cfg.ProxyURL,
52-
ProxyPath: p.cfg.ProxyPath,
51+
ProxyURL: p.cfg.proxyURL,
52+
ProxyPath: p.cfg.proxyPath,
5353
OAuthToken: token,
5454
}), nil
5555
}

0 commit comments

Comments
 (0)