Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Commit 64be2fb

Browse files
jawnsyjawnsy
authored
feat: reuse http.Client (#248)
* Create Client interface, so that it can be mocked/stubbed * Validate client options at creation time * Reuse internal HTTP Client, so that we have connection pooling * Send the Session-Token in a header rather than cookie
1 parent 26b2794 commit 64be2fb

30 files changed

+419
-151
lines changed

‎ci/integration/envs_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,27 @@ import (
1414
"cdr.dev/slog/sloggers/slogtest"
1515
"cdr.dev/slog/sloggers/slogtest/assert"
1616
"github.com/google/go-cmp/cmp"
17+
"github.com/stretchr/testify/require"
1718

1819
"cdr.dev/coder-cli/coder-sdk"
1920
"cdr.dev/coder-cli/pkg/tcli"
2021
)
2122

22-
func cleanupClient(ctx context.Context, t *testing.T) *coder.Client {
23+
func cleanupClient(ctx context.Context, t *testing.T) coder.Client {
2324
creds := login(ctx, t)
2425

2526
u, err := url.Parse(creds.url)
2627
assert.Success(t, "parse base url", err)
2728

28-
return &coder.Client{BaseURL: u, Token: creds.token}
29+
client, err := coder.NewClient(coder.ClientOptions{
30+
BaseURL: u,
31+
Token: creds.token,
32+
})
33+
require.NoError(t, err, "failed to create coder.Client")
34+
return client
2935
}
3036

31-
func cleanupEnv(t *testing.T, client *coder.Client, envID string) func() {
37+
func cleanupEnv(t *testing.T, client coder.Client, envID string) func() {
3238
return func() {
3339
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
3440
defer cancel()
@@ -39,9 +45,9 @@ func cleanupEnv(t *testing.T, client *coder.Client, envID string) func() {
3945
}
4046

4147
// this is a stopgap until we have support for a `coder images` subcommand
42-
// until then, we want can use the *coder.Client to ensure our integration tests
48+
// until then, we want can use the coder.Client to ensure our integration tests
4349
// work on fresh deployments.
44-
func ensureImageImported(ctx context.Context, t *testing.T, client *coder.Client, img string) {
50+
func ensureImageImported(ctx context.Context, t *testing.T, client coder.Client, img string) {
4551
orgs, err := client.Organizations(ctx)
4652
assert.Success(t, "get orgs", err)
4753

‎coder-sdk/activity.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ type activityRequest struct {
1111
}
1212

1313
// PushActivity pushes CLI activity to Coder.
14-
func (c Client) PushActivity(ctx context.Context, source, envID string) error {
14+
func (c *DefaultClient) PushActivity(ctx context.Context, source, envID string) error {
1515
resp, err := c.request(ctx, http.MethodPost, "/api/private/metrics/usage/push", activityRequest{
1616
Source: source,
1717
EnvironmentID: envID,

‎coder-sdk/client.go

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,67 @@
11
package coder
22

33
import (
4+
"errors"
45
"net/http"
5-
"net/http/cookiejar"
66
"net/url"
77
)
88

9+
// ensure that DefaultClient implements Client.
10+
var _ = Client(&DefaultClient{})
11+
912
// Me is the user ID of the authenticated user.
1013
const Me = "me"
1114

12-
// Client wraps the Coder HTTP API.
13-
type Client struct {
15+
// ClientOptions contains options for the Coder SDK Client.
16+
type ClientOptions struct {
17+
// BaseURL is the root URL of the Coder installation.
1418
BaseURL *url.URL
15-
Token string
19+
20+
// Client is the http.Client to use for requests (optional).
21+
// If omitted, the http.DefaultClient will be used.
22+
HTTPClient *http.Client
23+
24+
// Token is the API Token used to authenticate
25+
Token string
1626
}
1727

18-
// newHTTPClient creates a default underlying http client and sets the auth cookie.
19-
//
20-
// NOTE:
21-
// As we do not specify a custom transport, the default one from the stdlib will be used,
22-
// resulting in a persistent connection pool.
23-
// We do not set a timeout here as it could cause issue with the websocket.
24-
// The caller is expected to set it when needed.
25-
//
26-
// WARNING:
27-
// If the caller sets a custom transport to set TLS settings or a custom CA the default
28-
// pool will not be used and it might result in a new dns lookup/tls session/socket begin
29-
// established each time.
30-
func (c Client) newHTTPClient() (*http.Client, error) {
31-
jar, err := cookiejar.New(nil)
32-
if err != nil {
33-
return nil, err
28+
// NewClient creates a new default Coder SDK client.
29+
func NewClient(opts ClientOptions) (*DefaultClient, error) {
30+
httpClient := opts.HTTPClient
31+
if httpClient == nil {
32+
httpClient = http.DefaultClient
33+
}
34+
35+
if opts.BaseURL == nil {
36+
return nil, errors.New("the BaseURL parameter is required")
3437
}
3538

36-
jar.SetCookies(c.BaseURL, []*http.Cookie{{
37-
Name: "session_token",
38-
Value: c.Token,
39-
MaxAge: 86400,
40-
Path: "/",
41-
HttpOnly: true,
42-
Secure: c.BaseURL.Scheme == "https",
43-
}})
39+
if opts.Token == "" {
40+
return nil, errors.New("an API token is required")
41+
}
42+
43+
client := &DefaultClient{
44+
baseURL: opts.BaseURL,
45+
httpClient: httpClient,
46+
token: opts.Token,
47+
}
48+
49+
return client, nil
50+
}
51+
52+
// DefaultClient is the default implementation of the coder.Client
53+
// interface.
54+
//
55+
// The empty value is meaningless and the fields are unexported;
56+
// use NewClient to create an instance.
57+
type DefaultClient struct {
58+
// baseURL is the URL (scheme, hostname/IP address, port,
59+
// path prefix of the Coder installation)
60+
baseURL *url.URL
61+
62+
// httpClient is the http.Client used to issue requests.
63+
httpClient *http.Client
4464

45-
return &http.Client{Jar: jar}, nil
65+
// token is the API Token credential.
66+
token string
4667
}

‎coder-sdk/client_test.go

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,12 @@ func TestPushActivity(t *testing.T) {
4141
u, err := url.Parse(server.URL)
4242
require.NoError(t, err, "failed to parse test server URL")
4343

44-
client := coder.Client{
44+
client, err := coder.NewClient(coder.ClientOptions{
4545
BaseURL: u,
46-
}
46+
Token: "SwdcSoq5Jc-0C1r8wfwm7h6h9i0RDk7JT",
47+
})
48+
require.NoError(t, err, "failed to create coder.Client")
49+
4750
err = client.PushActivity(context.Background(), source, envID)
4851
require.NoError(t, err)
4952
}
@@ -81,9 +84,12 @@ func TestUsers(t *testing.T) {
8184
u, err := url.Parse(server.URL)
8285
require.NoError(t, err, "failed to parse test server URL")
8386

84-
client := coder.Client{
87+
client, err := coder.NewClient(coder.ClientOptions{
8588
BaseURL: u,
86-
}
89+
Token: "JcmErkJjju-KSrztst0IJX7xGJhKQPtfv",
90+
})
91+
require.NoError(t, err, "failed to create coder.Client")
92+
8793
users, err := client.Users(context.Background())
8894
require.NoError(t, err, "error getting Users")
8995
require.Len(t, users, 1, "users should return a single user")
@@ -96,9 +102,8 @@ func TestAuthentication(t *testing.T) {
96102

97103
const token = "g4mtIPUaKt-pPl9Q0xmgKs7acSypHt4Jf"
98104
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
99-
cookie, err := r.Cookie("session_token")
100-
require.NoError(t, err, "error extracting session token")
101-
require.Equal(t, token, cookie.Value, "token does not match")
105+
gotToken := r.Header.Get("Session-Token")
106+
require.Equal(t, token, gotToken, "token does not match")
102107

103108
w.WriteHeader(http.StatusServiceUnavailable)
104109
}))
@@ -109,11 +114,14 @@ func TestAuthentication(t *testing.T) {
109114
u, err := url.Parse(server.URL)
110115
require.NoError(t, err, "failed to parse test server URL")
111116

112-
client := coder.Client{
117+
client, err := coder.NewClient(coder.ClientOptions{
113118
BaseURL: u,
114119
Token: token,
115-
}
116-
_, _ = client.APIVersion(context.Background())
120+
})
121+
require.NoError(t, err, "failed to create coder.Client")
122+
123+
_, err = client.APIVersion(context.Background())
124+
require.NoError(t, err, "failed to get API version information")
117125
}
118126

119127
func TestContextRoot(t *testing.T) {
@@ -140,9 +148,13 @@ func TestContextRoot(t *testing.T) {
140148
for _, prefix := range contextRoots {
141149
u.Path = prefix
142150

143-
client := coder.Client{
151+
client, err := coder.NewClient(coder.ClientOptions{
144152
BaseURL: u,
145-
}
146-
_, _ = client.Users(context.Background())
153+
Token: "FrOgA6xhpM-p5nTfsupmvzYJA6DJSOUoE",
154+
})
155+
require.NoError(t, err, "failed to create coder.Client")
156+
157+
_, err = client.Users(context.Background())
158+
require.Error(t, err, "expected 503 error")
147159
}
148160
}

‎coder-sdk/config.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ type ConfigOAuth struct {
6565
}
6666

6767
// SiteConfigAuth fetches the sitewide authentication configuration.
68-
func (c Client) SiteConfigAuth(ctx context.Context) (*ConfigAuth, error) {
68+
func (c *DefaultClient) SiteConfigAuth(ctx context.Context) (*ConfigAuth, error) {
6969
var conf ConfigAuth
7070
if err := c.requestBody(ctx, http.MethodGet, "/api/private/auth/config", nil, &conf); err != nil {
7171
return nil, err
@@ -74,12 +74,12 @@ func (c Client) SiteConfigAuth(ctx context.Context) (*ConfigAuth, error) {
7474
}
7575

7676
// PutSiteConfigAuth sets the sitewide authentication configuration.
77-
func (c Client) PutSiteConfigAuth(ctx context.Context, req ConfigAuth) error {
77+
func (c *DefaultClient) PutSiteConfigAuth(ctx context.Context, req ConfigAuth) error {
7878
return c.requestBody(ctx, http.MethodPut, "/api/private/auth/config", req, nil)
7979
}
8080

8181
// SiteConfigOAuth fetches the sitewide git provider OAuth configuration.
82-
func (c Client) SiteConfigOAuth(ctx context.Context) (*ConfigOAuth, error) {
82+
func (c *DefaultClient) SiteConfigOAuth(ctx context.Context) (*ConfigOAuth, error) {
8383
var conf ConfigOAuth
8484
if err := c.requestBody(ctx, http.MethodGet, "/api/private/oauth/config", nil, &conf); err != nil {
8585
return nil, err
@@ -88,7 +88,7 @@ func (c Client) SiteConfigOAuth(ctx context.Context) (*ConfigOAuth, error) {
8888
}
8989

9090
// PutSiteConfigOAuth sets the sitewide git provider OAuth configuration.
91-
func (c Client) PutSiteConfigOAuth(ctx context.Context, req ConfigOAuth) error {
91+
func (c *DefaultClient) PutSiteConfigOAuth(ctx context.Context, req ConfigOAuth) error {
9292
return c.requestBody(ctx, http.MethodPut, "/api/private/oauth/config", req, nil)
9393
}
9494

@@ -97,7 +97,7 @@ type configSetupMode struct {
9797
}
9898

9999
// SiteSetupModeEnabled fetches the current setup_mode state of a Coder Enterprise deployment.
100-
func (c Client) SiteSetupModeEnabled(ctx context.Context) (bool, error) {
100+
func (c *DefaultClient) SiteSetupModeEnabled(ctx context.Context) (bool, error) {
101101
var conf configSetupMode
102102
if err := c.requestBody(ctx, http.MethodGet, "/api/private/config/setup-mode", nil, &conf); err != nil {
103103
return false, err
@@ -125,7 +125,7 @@ type ConfigExtensionMarketplace struct {
125125
}
126126

127127
// SiteConfigExtensionMarketplace fetches the extension marketplace configuration.
128-
func (c Client) SiteConfigExtensionMarketplace(ctx context.Context) (*ConfigExtensionMarketplace, error) {
128+
func (c *DefaultClient) SiteConfigExtensionMarketplace(ctx context.Context) (*ConfigExtensionMarketplace, error) {
129129
var conf ConfigExtensionMarketplace
130130
if err := c.requestBody(ctx, http.MethodGet, "/api/private/extensions/config", nil, &conf); err != nil {
131131
return nil, err
@@ -134,6 +134,6 @@ func (c Client) SiteConfigExtensionMarketplace(ctx context.Context) (*ConfigExte
134134
}
135135

136136
// PutSiteConfigExtensionMarketplace sets the extension marketplace configuration.
137-
func (c Client) PutSiteConfigExtensionMarketplace(ctx context.Context, req ConfigExtensionMarketplace) error {
137+
func (c *DefaultClient) PutSiteConfigExtensionMarketplace(ctx context.Context, req ConfigExtensionMarketplace) error {
138138
return c.requestBody(ctx, http.MethodPut, "/api/private/extensions/config", req, nil)
139139
}

‎coder-sdk/devurl.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type delDevURLRequest struct {
2222
}
2323

2424
// DeleteDevURL deletes the specified devurl.
25-
func (c Client) DeleteDevURL(ctx context.Context, envID, urlID string) error {
25+
func (c *DefaultClient) DeleteDevURL(ctx context.Context, envID, urlID string) error {
2626
reqURL := fmt.Sprintf("/api/v0/environments/%s/devurls/%s", envID, urlID)
2727

2828
return c.requestBody(ctx, http.MethodDelete, reqURL, delDevURLRequest{
@@ -41,12 +41,12 @@ type CreateDevURLReq struct {
4141
}
4242

4343
// CreateDevURL inserts a new devurl for the authenticated user.
44-
func (c Client) CreateDevURL(ctx context.Context, envID string, req CreateDevURLReq) error {
44+
func (c *DefaultClient) CreateDevURL(ctx context.Context, envID string, req CreateDevURLReq) error {
4545
return c.requestBody(ctx, http.MethodPost, "/api/v0/environments/"+envID+"/devurls", req, nil)
4646
}
4747

4848
// DevURLs fetches the Dev URLs for a given environment.
49-
func (c Client) DevURLs(ctx context.Context, envID string) ([]DevURL, error) {
49+
func (c *DefaultClient) DevURLs(ctx context.Context, envID string) ([]DevURL, error) {
5050
var devurls []DevURL
5151
if err := c.requestBody(ctx, http.MethodGet, "/api/v0/environments/"+envID+"/devurls", nil, &devurls); err != nil {
5252
return nil, err
@@ -58,6 +58,6 @@ func (c Client) DevURLs(ctx context.Context, envID string) ([]DevURL, error) {
5858
type PutDevURLReq CreateDevURLReq
5959

6060
// PutDevURL updates an existing devurl for the authenticated user.
61-
func (c Client) PutDevURL(ctx context.Context, envID, urlID string, req PutDevURLReq) error {
61+
func (c *DefaultClient) PutDevURL(ctx context.Context, envID, urlID string, req PutDevURLReq) error {
6262
return c.requestBody(ctx, http.MethodPut, "/api/v0/environments/"+envID+"/devurls/"+urlID, req, nil)
6363
}

0 commit comments

Comments
 (0)