Skip to content

Commit 787b8b2

Browse files
deansheatherdeansheather
authored
fix: fix app hostname returning port number (coder#5441)
1 parent 44c10bb commit 787b8b2

File tree

5 files changed

+57
-17
lines changed

5 files changed

+57
-17
lines changed

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ type Options struct {
6161
AccessURL *url.URL
6262
// AppHostname should be the wildcard hostname to use for workspace
6363
// applications INCLUDING the asterisk, (optional) suffix and leading dot.
64+
// It will use the same scheme and port number as the access URL.
6465
// E.g. "*.apps.coder.com" or "*-apps.coder.com".
6566
AppHostname string
6667
// AppHostnameRegex contains the regex version of options.AppHostname as

‎coderd/coderdtest/coderdtest.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ import (
7878
)
7979

8080
type Options struct {
81+
// AccessURL denotes a custom access URL. By default we use the httptest
82+
// server's URL. Setting this may result in unexpected behavior (especially
83+
// with running agents).
84+
AccessURL *url.URL
8185
AppHostname string
8286
AWSCertificates awsidentity.Certificates
8387
Authorizer rbac.Authorizer
@@ -144,7 +148,7 @@ func newWithCloser(t *testing.T, options *Options) (*codersdk.Client, io.Closer)
144148
return client, closer
145149
}
146150

147-
func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.CancelFunc, *coderd.Options) {
151+
func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.CancelFunc, *url.URL, *coderd.Options) {
148152
if options == nil {
149153
options = &Options{}
150154
}
@@ -214,6 +218,11 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
214218
derpPort, err := strconv.Atoi(serverURL.Port())
215219
require.NoError(t, err)
216220

221+
accessURL := options.AccessURL
222+
if accessURL == nil {
223+
accessURL = serverURL
224+
}
225+
217226
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
218227
t.Cleanup(stunCleanup)
219228

@@ -236,12 +245,12 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
236245
mutex.Lock()
237246
defer mutex.Unlock()
238247
handler = h
239-
}, cancelFunc, &coderd.Options{
248+
}, cancelFunc, serverURL, &coderd.Options{
240249
AgentConnectionUpdateFrequency: 150 * time.Millisecond,
241250
// Force a long disconnection timeout to ensure
242251
// agents are not marked as disconnected during slow tests.
243252
AgentInactiveDisconnectTimeout: testutil.WaitShort,
244-
AccessURL: serverURL,
253+
AccessURL: accessURL,
245254
AppHostname: options.AppHostname,
246255
AppHostnameRegex: appHostnameRegex,
247256
Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug),
@@ -298,15 +307,15 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
298307
if options == nil {
299308
options = &Options{}
300309
}
301-
setHandler, cancelFunc, newOptions := NewOptions(t, options)
310+
setHandler, cancelFunc, serverURL, newOptions := NewOptions(t, options)
302311
// We set the handler after server creation for the access URL.
303312
coderAPI := coderd.New(newOptions)
304313
setHandler(coderAPI.RootHandler)
305314
var provisionerCloser io.Closer = nopcloser{}
306315
if options.IncludeProvisionerDaemon {
307316
provisionerCloser = NewProvisionerDaemon(t, coderAPI)
308317
}
309-
client := codersdk.New(coderAPI.AccessURL)
318+
client := codersdk.New(serverURL)
310319
t.Cleanup(func() {
311320
cancelFunc()
312321
_ = provisionerCloser.Close()

‎coderd/workspaceapps.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ var nonCanonicalHeaders = map[string]string{
5858

5959
func (api *API) appHost(rw http.ResponseWriter, r *http.Request) {
6060
host := api.AppHostname
61-
if api.AccessURL.Port() != "" {
61+
if host != "" && api.AccessURL.Port() != "" {
6262
host += fmt.Sprintf(":%s", api.AccessURL.Port())
6363
}
6464

‎coderd/workspaceapps_test.go

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,48 @@ const (
4747
func TestGetAppHost(t *testing.T) {
4848
t.Parallel()
4949

50-
cases := []string{"", proxyTestSubdomainRaw}
50+
cases := []struct {
51+
name string
52+
accessURL string
53+
appHostname string
54+
expected string
55+
}{
56+
{
57+
name: "OK",
58+
accessURL: "https://test.coder.com",
59+
appHostname: "*.test.coder.com",
60+
expected: "*.test.coder.com",
61+
},
62+
{
63+
name: "None",
64+
accessURL: "https://test.coder.com",
65+
appHostname: "",
66+
expected: "",
67+
},
68+
{
69+
name: "OKWithPort",
70+
accessURL: "https://test.coder.com:8443",
71+
appHostname: "*.test.coder.com",
72+
expected: "*.test.coder.com:8443",
73+
},
74+
{
75+
name: "OKWithSuffix",
76+
accessURL: "https://test.coder.com:8443",
77+
appHostname: "*--suffix.test.coder.com",
78+
expected: "*--suffix.test.coder.com:8443",
79+
},
80+
}
5181
for _, c := range cases {
5282
c := c
53-
name := c
54-
if name == "" {
55-
name = "Empty"
56-
}
57-
t.Run(name, func(t *testing.T) {
83+
t.Run(c.name, func(t *testing.T) {
5884
t.Parallel()
85+
86+
accessURL, err := url.Parse(c.accessURL)
87+
require.NoError(t, err)
88+
5989
client := coderdtest.New(t, &coderdtest.Options{
60-
AppHostname: c,
90+
AccessURL: accessURL,
91+
AppHostname: c.appHostname,
6192
})
6293

6394
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -71,8 +102,7 @@ func TestGetAppHost(t *testing.T) {
71102
_ = coderdtest.CreateFirstUser(t, client)
72103
host, err = client.GetAppHost(ctx)
73104
require.NoError(t, err)
74-
domain := strings.Split(host.Host, ":")[0]
75-
require.Equal(t, c, domain)
105+
require.Equal(t, c.expected, host.Host)
76106
})
77107
}
78108
}

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
6262
if options.Options == nil {
6363
options.Options = &coderdtest.Options{}
6464
}
65-
setHandler, cancelFunc, oop := coderdtest.NewOptions(t, options.Options)
65+
setHandler, cancelFunc, serverURL, oop := coderdtest.NewOptions(t, options.Options)
6666
coderAPI, err := coderd.New(context.Background(), &coderd.Options{
6767
RBAC: true,
6868
AuditLogging: options.AuditLogging,
@@ -86,7 +86,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
8686
_ = provisionerCloser.Close()
8787
_ = coderAPI.Close()
8888
})
89-
client := codersdk.New(coderAPI.AccessURL)
89+
client := codersdk.New(serverURL)
9090
client.HTTPClient = &http.Client{
9191
Transport: &http.Transport{
9292
TLSClientConfig: &tls.Config{

0 commit comments

Comments
 (0)