Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6bb1a34

Browse files
authored
fix: allow ports in wildcard url configuration (#11657)
* fix: allow ports in wildcard url configurationThis just forwards the port to the ui that generates urls.Our existing parsing + regex already supported ports forsubdomain app requests.
1 parent1f0e6ba commit6bb1a34

File tree

12 files changed

+203
-31
lines changed

12 files changed

+203
-31
lines changed

‎coderd/agentapi/manifest.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package agentapi
33
import (
44
"context"
55
"database/sql"
6-
"fmt"
76
"net/url"
87
"strings"
98
"sync/atomic"
@@ -108,19 +107,14 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest
108107
returnnil,xerrors.Errorf("fetching workspace agent data: %w",err)
109108
}
110109

111-
appHost:= appurl.ApplicationURL{
110+
appSlug:= appurl.ApplicationURL{
112111
AppSlugOrPort:"{{port}}",
113112
AgentName:workspaceAgent.Name,
114113
WorkspaceName:workspace.Name,
115114
Username:owner.Username,
116115
}
117-
vscodeProxyURI:=a.AccessURL.Scheme+"://"+strings.ReplaceAll(a.AppHostname,"*",appHost.String())
118-
ifa.AppHostname=="" {
119-
vscodeProxyURI+=a.AccessURL.Hostname()
120-
}
121-
ifa.AccessURL.Port()!="" {
122-
vscodeProxyURI+=fmt.Sprintf(":%s",a.AccessURL.Port())
123-
}
116+
117+
vscodeProxyURI:=vscodeProxyURI(appSlug,a.AccessURL,a.AppHostname)
124118

125119
vargitAuthConfigsuint32
126120
for_,cfg:=rangea.ExternalAuthConfigs {
@@ -155,6 +149,17 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest
155149
},nil
156150
}
157151

152+
funcvscodeProxyURI(app appurl.ApplicationURL,accessURL*url.URL,appHoststring)string {
153+
// This will handle the ports from the accessURL or appHost.
154+
appHost=appurl.SubdomainAppHost(appHost,accessURL)
155+
// If there is no appHost, then we want to use the access url as the proxy uri.
156+
ifappHost=="" {
157+
appHost=accessURL.Host
158+
}
159+
// Return the url with a scheme and any wildcards replaced with the app slug.
160+
returnaccessURL.Scheme+"://"+strings.ReplaceAll(appHost,"*",app.String())
161+
}
162+
158163
funcdbAgentMetadataToProtoDescription(metadata []database.WorkspaceAgentMetadatum) []*agentproto.WorkspaceAgentMetadata_Description {
159164
ret:=make([]*agentproto.WorkspaceAgentMetadata_Description,len(metadata))
160165
fori,metadatum:=rangemetadata {
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package agentapi
2+
3+
import (
4+
"fmt"
5+
"net/url"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
11+
)
12+
13+
funcTest_vscodeProxyURI(t*testing.T) {
14+
t.Parallel()
15+
16+
coderAccessURL,err:=url.Parse("https://coder.com")
17+
require.NoError(t,err)
18+
19+
accessURLWithPort,err:=url.Parse("https://coder.com:8080")
20+
require.NoError(t,err)
21+
22+
basicApp:= appurl.ApplicationURL{
23+
Prefix:"prefix",
24+
AppSlugOrPort:"slug",
25+
AgentName:"agent",
26+
WorkspaceName:"workspace",
27+
Username:"user",
28+
}
29+
30+
cases:= []struct {
31+
Namestring
32+
App appurl.ApplicationURL
33+
AccessURL*url.URL
34+
AppHostnamestring
35+
Expectedstring
36+
}{
37+
{
38+
// No hostname proxies through the access url.
39+
Name:"NoHostname",
40+
AccessURL:coderAccessURL,
41+
AppHostname:"",
42+
App:basicApp,
43+
Expected:coderAccessURL.String(),
44+
},
45+
{
46+
Name:"NoHostnameAccessURLPort",
47+
AccessURL:accessURLWithPort,
48+
AppHostname:"",
49+
App:basicApp,
50+
Expected:accessURLWithPort.String(),
51+
},
52+
{
53+
Name:"Hostname",
54+
AccessURL:coderAccessURL,
55+
AppHostname:"*.apps.coder.com",
56+
App:basicApp,
57+
Expected:fmt.Sprintf("https://%s.apps.coder.com",basicApp.String()),
58+
},
59+
{
60+
Name:"HostnameWithAccessURLPort",
61+
AccessURL:accessURLWithPort,
62+
AppHostname:"*.apps.coder.com",
63+
App:basicApp,
64+
Expected:fmt.Sprintf("https://%s.apps.coder.com:%s",basicApp.String(),accessURLWithPort.Port()),
65+
},
66+
{
67+
Name:"HostnameWithPort",
68+
AccessURL:coderAccessURL,
69+
AppHostname:"*.apps.coder.com:4444",
70+
App:basicApp,
71+
Expected:fmt.Sprintf("https://%s.apps.coder.com:%s",basicApp.String(),"4444"),
72+
},
73+
{
74+
// Port from hostname takes precedence over access url port.
75+
Name:"HostnameWithPortAccessURLWithPort",
76+
AccessURL:accessURLWithPort,
77+
AppHostname:"*.apps.coder.com:4444",
78+
App:basicApp,
79+
Expected:fmt.Sprintf("https://%s.apps.coder.com:%s",basicApp.String(),"4444"),
80+
},
81+
}
82+
83+
for_,c:=rangecases {
84+
c:=c
85+
t.Run(c.Name,func(t*testing.T) {
86+
t.Parallel()
87+
88+
require.NotNilf(t,c.AccessURL,"AccessURL is required")
89+
90+
output:=vscodeProxyURI(c.App,c.AccessURL,c.AppHostname)
91+
require.Equal(t,c.Expected,output)
92+
})
93+
}
94+
}

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ type Options struct {
9393
// AppHostname should be the wildcard hostname to use for workspace
9494
// applications INCLUDING the asterisk, (optional) suffix and leading dot.
9595
// It will use the same scheme and port number as the access URL.
96-
// E.g. "*.apps.coder.com" or "*-apps.coder.com".
96+
// E.g. "*.apps.coder.com" or "*-apps.coder.com" or "*.apps.coder.com:8080".
9797
AppHostnamestring
9898
// AppHostnameRegex contains the regex version of options.AppHostname as
9999
// generated by appurl.CompileHostnamePattern(). It MUST be set if

‎coderd/workspaceapps.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package coderd
33
import (
44
"context"
55
"database/sql"
6-
"fmt"
76
"net/http"
87
"net/url"
98
"strings"
@@ -32,13 +31,8 @@ import (
3231
// @Router /applications/host [get]
3332
// @Deprecated use api/v2/regions and see the primary proxy.
3433
func (api*API)appHost(rw http.ResponseWriter,r*http.Request) {
35-
host:=api.AppHostname
36-
ifhost!=""&&api.AccessURL.Port()!="" {
37-
host+=fmt.Sprintf(":%s",api.AccessURL.Port())
38-
}
39-
4034
httpapi.Write(r.Context(),rw,http.StatusOK, codersdk.AppHostResponse{
41-
Host:host,
35+
Host:appurl.SubdomainAppHost(api.AppHostname,api.AccessURL),
4236
})
4337
}
4438

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,38 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
963963
require.Equal(t,http.StatusOK,resp.StatusCode)
964964
})
965965

966+
t.Run("WildcardPortOK",func(t*testing.T) {
967+
t.Parallel()
968+
969+
// Manually specifying a port should override the access url port on
970+
// the app host.
971+
appDetails:=setupProxyTest(t,&DeploymentOptions{
972+
// Just throw both the wsproxy and primary to same url.
973+
AppHost:"*.test.coder.com:4444",
974+
PrimaryAppHost:"*.test.coder.com:4444",
975+
})
976+
977+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
978+
defercancel()
979+
980+
u:=appDetails.SubdomainAppURL(appDetails.Apps.Owner)
981+
t.Logf("url: %s",u)
982+
require.Equal(t,"4444",u.Port(),"port should be 4444")
983+
984+
// Assert the api response the UI uses has the port.
985+
apphost,err:=appDetails.SDKClient.AppHost(ctx)
986+
require.NoError(t,err)
987+
require.Equal(t,"*.test.coder.com:4444",apphost.Host,"apphost has port")
988+
989+
resp,err:=requestWithRetries(ctx,t,appDetails.AppClient(t),http.MethodGet,u.String(),nil)
990+
require.NoError(t,err)
991+
deferresp.Body.Close()
992+
body,err:=io.ReadAll(resp.Body)
993+
require.NoError(t,err)
994+
require.Equal(t,proxyTestAppBody,string(body))
995+
require.Equal(t,http.StatusOK,resp.StatusCode)
996+
})
997+
966998
t.Run("SuffixWildcardNotMatch",func(t*testing.T) {
967999
t.Parallel()
9681000

‎coderd/workspaceapps/apptest/setup.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ const (
4747
// DeploymentOptions are the options for creating a *Deployment with a
4848
// DeploymentFactory.
4949
typeDeploymentOptionsstruct {
50+
PrimaryAppHoststring
5051
AppHoststring
5152
DisablePathAppsbool
5253
DisableSubdomainAppsbool
@@ -407,7 +408,7 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
407408
Username:me.Username,
408409
}
409410
proxyURL:="http://"+appHost.String()+strings.ReplaceAll(primaryAppHost.Host,"*","")
410-
require.Equal(t,proxyURL,manifest.VSCodePortProxyURI)
411+
require.Equal(t,manifest.VSCodePortProxyURI,proxyURL)
411412
}
412413
agentCloser:=agent.New(agent.Options{
413414
Client:agentClient,

‎coderd/workspaceapps/appurl/appurl.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package appurl
33
import (
44
"fmt"
55
"net"
6+
"net/url"
67
"regexp"
78
"strings"
89

@@ -20,6 +21,36 @@ var (
2021
validHostnameLabelRegex=regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`)
2122
)
2223

24+
// SubdomainAppHost returns the URL of the apphost for subdomain based apps.
25+
// It will omit the scheme.
26+
//
27+
// Arguments:
28+
// apphost: Expected to contain a wildcard, example: "*.coder.com"
29+
// accessURL: The access url for the deployment.
30+
//
31+
// Returns:
32+
// 'apphost:port'
33+
//
34+
// For backwards compatibility and for "accessurl=localhost:0" purposes, we need
35+
// to use the port from the accessurl if the apphost doesn't have a port.
36+
// If the user specifies a port in the apphost, we will use that port instead.
37+
funcSubdomainAppHost(apphoststring,accessURL*url.URL)string {
38+
ifapphost=="" {
39+
return""
40+
}
41+
42+
ifapphost!=""&&accessURL.Port()!="" {
43+
// This should always parse if we prepend a scheme. We should add
44+
// the access url port if the apphost doesn't have a port specified.
45+
appHostU,err:=url.Parse(fmt.Sprintf("https://%s",apphost))
46+
iferr!=nil|| (err==nil&&appHostU.Port()=="") {
47+
apphost+=fmt.Sprintf(":%s",accessURL.Port())
48+
}
49+
}
50+
51+
returnapphost
52+
}
53+
2354
// ApplicationURL is a parsed application URL hostname.
2455
typeApplicationURLstruct {
2556
Prefixstring
@@ -140,9 +171,7 @@ func CompileHostnamePattern(pattern string) (*regexp.Regexp, error) {
140171
ifstrings.Contains(pattern,"http:")||strings.Contains(pattern,"https:") {
141172
returnnil,xerrors.Errorf("hostname pattern must not contain a scheme: %q",pattern)
142173
}
143-
ifstrings.Contains(pattern,":") {
144-
returnnil,xerrors.Errorf("hostname pattern must not contain a port: %q",pattern)
145-
}
174+
146175
ifstrings.HasPrefix(pattern,".")||strings.HasSuffix(pattern,".") {
147176
returnnil,xerrors.Errorf("hostname pattern must not start or end with a period: %q",pattern)
148177
}
@@ -155,6 +184,16 @@ func CompileHostnamePattern(pattern string) (*regexp.Regexp, error) {
155184
if!strings.HasPrefix(pattern,"*") {
156185
returnnil,xerrors.Errorf("hostname pattern must only contain an asterisk at the beginning: %q",pattern)
157186
}
187+
188+
// If there is a hostname:port, we only care about the hostname. For hostname
189+
// pattern reasons, we do not actually care what port the client is requesting.
190+
// Any port provided here is used for generating urls for the ui, not for
191+
// validation.
192+
hostname,_,err:=net.SplitHostPort(pattern)
193+
iferr==nil {
194+
pattern=hostname
195+
}
196+
158197
fori,label:=rangestrings.Split(pattern,".") {
159198
ifi==0 {
160199
// We have to allow the asterisk to be a valid hostname label, so

‎coderd/workspaceapps/appurl/appurl_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,6 @@ func TestCompileHostnamePattern(t *testing.T) {
193193
pattern:"https://*.hi.com",
194194
errorContains:"must not contain a scheme",
195195
},
196-
{
197-
name:"Invalid_ContainsPort",
198-
pattern:"*.hi.com:8080",
199-
errorContains:"must not contain a port",
200-
},
201196
{
202197
name:"Invalid_StartPeriod",
203198
pattern:".hi.com",
@@ -249,6 +244,13 @@ func TestCompileHostnamePattern(t *testing.T) {
249244
errorContains:"contains invalid label",
250245
},
251246

247+
{
248+
name:"Valid_ContainsPort",
249+
pattern:"*.hi.com:8080",
250+
// Although a port is provided, the regex already matches any port.
251+
// So it is ignored for validation purposes.
252+
expectedRegex:`([^.]+)\.hi\.com`,
253+
},
252254
{
253255
name:"Valid_Simple",
254256
pattern:"*.hi",

‎coderd/workspaceproxies.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/coder/coder/v2/coderd/database"
1212
"github.com/coder/coder/v2/coderd/database/dbauthz"
1313
"github.com/coder/coder/v2/coderd/httpapi"
14+
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
1415
"github.com/coder/coder/v2/codersdk"
1516
)
1617

@@ -43,7 +44,7 @@ func (api *API) PrimaryRegion(ctx context.Context) (codersdk.Region, error) {
4344
IconURL:proxy.IconUrl,
4445
Healthy:true,
4546
PathAppURL:api.AccessURL.String(),
46-
WildcardHostname:api.AppHostname,
47+
WildcardHostname:appurl.SubdomainAppHost(api.AppHostname,api.AccessURL),
4748
},nil
4849
}
4950

‎coderd/workspaceproxies_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package coderd_test
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/google/uuid"
@@ -44,7 +45,7 @@ func TestRegions(t *testing.T) {
4445
require.NotEmpty(t,regions[0].IconURL)
4546
require.True(t,regions[0].Healthy)
4647
require.Equal(t,client.URL.String(),regions[0].PathAppURL)
47-
require.Equal(t,appHostname,regions[0].WildcardHostname)
48+
require.Equal(t,fmt.Sprintf("%s:%s",appHostname,client.URL.Port()),regions[0].WildcardHostname)
4849

4950
// Ensure the primary region ID is constant.
5051
regions2,err:=client.Regions(ctx)

‎enterprise/coderd/workspaceproxy_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestRegions(t *testing.T) {
6262
require.NotEmpty(t,regions[0].IconURL)
6363
require.True(t,regions[0].Healthy)
6464
require.Equal(t,client.URL.String(),regions[0].PathAppURL)
65-
require.Equal(t,appHostname,regions[0].WildcardHostname)
65+
require.Equal(t,fmt.Sprintf("%s:%s",appHostname,client.URL.Port()),regions[0].WildcardHostname)
6666

6767
// Ensure the primary region ID is constant.
6868
regions2,err:=client.Regions(ctx)
@@ -149,7 +149,7 @@ func TestRegions(t *testing.T) {
149149
require.NotEmpty(t,regions[0].IconURL)
150150
require.True(t,regions[0].Healthy)
151151
require.Equal(t,client.URL.String(),regions[0].PathAppURL)
152-
require.Equal(t,appHostname,regions[0].WildcardHostname)
152+
require.Equal(t,fmt.Sprintf("%s:%s",appHostname,client.URL.Port()),regions[0].WildcardHostname)
153153

154154
// Ensure non-zero fields of the default proxy
155155
require.NotZero(t,primary.Name)

‎enterprise/wsproxy/wsproxy_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,10 +449,13 @@ func TestWorkspaceProxyWorkspaceApps(t *testing.T) {
449449
<-proxyStatsCollectorFlushDone
450450
}
451451

452+
ifopts.PrimaryAppHost=="" {
453+
opts.PrimaryAppHost="*.primary.test.coder.com"
454+
}
452455
client,closer,api,user:=coderdenttest.NewWithAPI(t,&coderdenttest.Options{
453456
Options:&coderdtest.Options{
454457
DeploymentValues:deploymentValues,
455-
AppHostname:"*.primary.test.coder.com",
458+
AppHostname:opts.PrimaryAppHost,
456459
IncludeProvisionerDaemon:true,
457460
RealIPConfig:&httpmw.RealIPConfig{
458461
TrustedOrigins: []*net.IPNet{{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp