From d29a52462bf93316cbd6dca8066780fdd71ab93a Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 26 Sep 2025 12:25:58 -0500 Subject: [PATCH] feat: remove agent name from app URLs (#19750) ## Summary In this pull request we're removing `agent_name` from subdomains in APP urls when an `app` is used in the subdomain. `agent_names` will still be used when a `port` is used in the subdomain. Closes: https://github.com/coder/coder/issues/18485 ### Changes - Updated regex to support an optional agent name - Added logic to support checking the app slug for a matching port (e.g., 8080 or 8080s) ### Testing - Updated all tests to support an optional `agent_name` --- coderd/agentapi/manifest.go | 16 +- coderd/agentapi/manifest_test.go | 179 ++++++++++++++------- coderd/database/db2sdk/db2sdk.go | 9 +- coderd/workspaceapps/apptest/setup.go | 11 +- coderd/workspaceapps/appurl/appurl.go | 66 +++++--- coderd/workspaceapps/appurl/appurl_test.go | 177 +++++++++++++++++++- 6 files changed, 359 insertions(+), 99 deletions(-) diff --git a/coderd/agentapi/manifest.go b/coderd/agentapi/manifest.go index 855ff4b8ac..2221d2bc03 100644 --- a/coderd/agentapi/manifest.go +++ b/coderd/agentapi/manifest.go @@ -110,7 +110,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest } } - apps, err := dbAppsToProto(dbApps, workspaceAgent, workspace.OwnerUsername, workspace) + apps, err := dbAppsToProto(dbApps, workspaceAgent, workspace.OwnerUsername, workspace, a.AppHostname) if err != nil { return nil, xerrors.Errorf("converting workspace apps: %w", err) } @@ -196,11 +196,11 @@ func dbAgentScriptToProto(script database.WorkspaceAgentScript) *agentproto.Work } } -func dbAppsToProto(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace) ([]*agentproto.WorkspaceApp, error) { +func dbAppsToProto(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace, appHostname string) ([]*agentproto.WorkspaceApp, error) { ret := make([]*agentproto.WorkspaceApp, len(dbApps)) for i, dbApp := range dbApps { var err error - ret[i], err = dbAppToProto(dbApp, agent, ownerName, workspace) + ret[i], err = dbAppToProto(dbApp, agent, ownerName, workspace, appHostname) if err != nil { return nil, xerrors.Errorf("parse app %v (%q): %w", i, dbApp.Slug, err) } @@ -208,7 +208,7 @@ func dbAppsToProto(dbApps []database.WorkspaceApp, agent database.WorkspaceAgent return ret, nil } -func dbAppToProto(dbApp database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace) (*agentproto.WorkspaceApp, error) { +func dbAppToProto(dbApp database.WorkspaceApp, agent database.WorkspaceAgent, ownerName string, workspace database.Workspace, appHostname string) (*agentproto.WorkspaceApp, error) { sharingLevelRaw, ok := agentproto.WorkspaceApp_SharingLevel_value[strings.ToUpper(string(dbApp.SharingLevel))] if !ok { return nil, xerrors.Errorf("unknown app sharing level: %q", dbApp.SharingLevel) @@ -219,6 +219,12 @@ func dbAppToProto(dbApp database.WorkspaceApp, agent database.WorkspaceAgent, ow return nil, xerrors.Errorf("unknown app health: %q", dbApp.SharingLevel) } + // SubdomainName should be empty if AppHostname is not configured + subdomainName := "" + if appHostname != "" { + subdomainName = db2sdk.AppSubdomain(dbApp, agent.Name, workspace.Name, ownerName) + } + return &agentproto.WorkspaceApp{ Id: dbApp.ID[:], Url: dbApp.Url.String, @@ -228,7 +234,7 @@ func dbAppToProto(dbApp database.WorkspaceApp, agent database.WorkspaceAgent, ow Command: dbApp.Command.String, Icon: dbApp.Icon, Subdomain: dbApp.Subdomain, - SubdomainName: db2sdk.AppSubdomain(dbApp, agent.Name, workspace.Name, ownerName), + SubdomainName: subdomainName, SharingLevel: agentproto.WorkspaceApp_SharingLevel(sharingLevelRaw), Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ Url: dbApp.HealthcheckUrl, diff --git a/coderd/agentapi/manifest_test.go b/coderd/agentapi/manifest_test.go index fc46f5fe48..4a346638d4 100644 --- a/coderd/agentapi/manifest_test.go +++ b/coderd/agentapi/manifest_test.go @@ -191,65 +191,6 @@ func TestGetManifest(t *testing.T) { // These are done manually to ensure the conversion logic matches what a // human expects. var ( - protoApps = []*agentproto.WorkspaceApp{ - { - Id: apps[0].ID[:], - Url: apps[0].Url.String, - External: apps[0].External, - Slug: apps[0].Slug, - DisplayName: apps[0].DisplayName, - Command: apps[0].Command.String, - Icon: apps[0].Icon, - Subdomain: apps[0].Subdomain, - SubdomainName: fmt.Sprintf("%s--%s--%s--%s", apps[0].Slug, agent.Name, workspace.Name, owner.Username), - SharingLevel: agentproto.WorkspaceApp_AUTHENTICATED, - Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ - Url: apps[0].HealthcheckUrl, - Interval: durationpb.New(time.Duration(apps[0].HealthcheckInterval) * time.Second), - Threshold: apps[0].HealthcheckThreshold, - }, - Health: agentproto.WorkspaceApp_HEALTHY, - Hidden: false, - }, - { - Id: apps[1].ID[:], - Url: apps[1].Url.String, - External: apps[1].External, - Slug: apps[1].Slug, - DisplayName: apps[1].DisplayName, - Command: apps[1].Command.String, - Icon: apps[1].Icon, - Subdomain: false, - SubdomainName: "", - SharingLevel: agentproto.WorkspaceApp_PUBLIC, - Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ - Url: "", - Interval: durationpb.New(0), - Threshold: 0, - }, - Health: agentproto.WorkspaceApp_DISABLED, - Hidden: false, - }, - { - Id: apps[2].ID[:], - Url: apps[2].Url.String, - External: apps[2].External, - Slug: apps[2].Slug, - DisplayName: apps[2].DisplayName, - Command: apps[2].Command.String, - Icon: apps[2].Icon, - Subdomain: false, - SubdomainName: "", - SharingLevel: agentproto.WorkspaceApp_OWNER, - Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ - Url: apps[2].HealthcheckUrl, - Interval: durationpb.New(time.Duration(apps[2].HealthcheckInterval) * time.Second), - Threshold: apps[2].HealthcheckThreshold, - }, - Health: agentproto.WorkspaceApp_UNHEALTHY, - Hidden: true, - }, - } protoScripts = []*agentproto.WorkspaceAgentScript{ { Id: scripts[0].ID[:], @@ -308,6 +249,66 @@ func TestGetManifest(t *testing.T) { t.Run("OK", func(t *testing.T) { t.Parallel() + protoApps := []*agentproto.WorkspaceApp{ + { + Id: apps[0].ID[:], + Url: apps[0].Url.String, + External: apps[0].External, + Slug: apps[0].Slug, + DisplayName: apps[0].DisplayName, + Command: apps[0].Command.String, + Icon: apps[0].Icon, + Subdomain: apps[0].Subdomain, + SubdomainName: fmt.Sprintf("%s--%s--%s", apps[0].Slug, workspace.Name, owner.Username), + SharingLevel: agentproto.WorkspaceApp_AUTHENTICATED, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: apps[0].HealthcheckUrl, + Interval: durationpb.New(time.Duration(apps[0].HealthcheckInterval) * time.Second), + Threshold: apps[0].HealthcheckThreshold, + }, + Health: agentproto.WorkspaceApp_HEALTHY, + Hidden: false, + }, + { + Id: apps[1].ID[:], + Url: apps[1].Url.String, + External: apps[1].External, + Slug: apps[1].Slug, + DisplayName: apps[1].DisplayName, + Command: apps[1].Command.String, + Icon: apps[1].Icon, + Subdomain: false, + SubdomainName: "", + SharingLevel: agentproto.WorkspaceApp_PUBLIC, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: "", + Interval: durationpb.New(0), + Threshold: 0, + }, + Health: agentproto.WorkspaceApp_DISABLED, + Hidden: false, + }, + { + Id: apps[2].ID[:], + Url: apps[2].Url.String, + External: apps[2].External, + Slug: apps[2].Slug, + DisplayName: apps[2].DisplayName, + Command: apps[2].Command.String, + Icon: apps[2].Icon, + Subdomain: false, + SubdomainName: "", + SharingLevel: agentproto.WorkspaceApp_OWNER, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: apps[2].HealthcheckUrl, + Interval: durationpb.New(time.Duration(apps[2].HealthcheckInterval) * time.Second), + Threshold: apps[2].HealthcheckThreshold, + }, + Health: agentproto.WorkspaceApp_UNHEALTHY, + Hidden: true, + }, + } + mDB := dbmock.NewMockStore(gomock.NewController(t)) api := &agentapi.ManifestAPI{ @@ -438,6 +439,66 @@ func TestGetManifest(t *testing.T) { t.Run("NoAppHostname", func(t *testing.T) { t.Parallel() + protoApps := []*agentproto.WorkspaceApp{ + { + Id: apps[0].ID[:], + Url: apps[0].Url.String, + External: apps[0].External, + Slug: apps[0].Slug, + DisplayName: apps[0].DisplayName, + Command: apps[0].Command.String, + Icon: apps[0].Icon, + Subdomain: apps[0].Subdomain, + SubdomainName: "", // Empty because AppHostname is empty + SharingLevel: agentproto.WorkspaceApp_AUTHENTICATED, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: apps[0].HealthcheckUrl, + Interval: durationpb.New(time.Duration(apps[0].HealthcheckInterval) * time.Second), + Threshold: apps[0].HealthcheckThreshold, + }, + Health: agentproto.WorkspaceApp_HEALTHY, + Hidden: false, + }, + { + Id: apps[1].ID[:], + Url: apps[1].Url.String, + External: apps[1].External, + Slug: apps[1].Slug, + DisplayName: apps[1].DisplayName, + Command: apps[1].Command.String, + Icon: apps[1].Icon, + Subdomain: false, + SubdomainName: "", + SharingLevel: agentproto.WorkspaceApp_PUBLIC, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: "", + Interval: durationpb.New(0), + Threshold: 0, + }, + Health: agentproto.WorkspaceApp_DISABLED, + Hidden: false, + }, + { + Id: apps[2].ID[:], + Url: apps[2].Url.String, + External: apps[2].External, + Slug: apps[2].Slug, + DisplayName: apps[2].DisplayName, + Command: apps[2].Command.String, + Icon: apps[2].Icon, + Subdomain: false, + SubdomainName: "", + SharingLevel: agentproto.WorkspaceApp_OWNER, + Healthcheck: &agentproto.WorkspaceApp_Healthcheck{ + Url: apps[2].HealthcheckUrl, + Interval: durationpb.New(time.Duration(apps[2].HealthcheckInterval) * time.Second), + Threshold: apps[2].HealthcheckThreshold, + }, + Health: agentproto.WorkspaceApp_UNHEALTHY, + Hidden: true, + }, + } + mDB := dbmock.NewMockStore(gomock.NewController(t)) api := &agentapi.ManifestAPI{ diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index eaa79c152f..7d9622b316 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -532,13 +532,20 @@ func AppSubdomain(dbApp database.WorkspaceApp, agentName, workspaceName, ownerNa if appSlug == "" { appSlug = dbApp.DisplayName } + + // Agent name is optional when app slug is present + normalizedAgentName := agentName + if !appurl.PortRegex.MatchString(appSlug) { + normalizedAgentName = "" + } + return appurl.ApplicationURL{ // We never generate URLs with a prefix. We only allow prefixes when // parsing URLs from the hostname. Users that want this feature can // write out their own URLs. Prefix: "", AppSlugOrPort: appSlug, - AgentName: agentName, + AgentName: normalizedAgentName, WorkspaceName: workspaceName, Username: ownerName, }.String() diff --git a/coderd/workspaceapps/apptest/setup.go b/coderd/workspaceapps/apptest/setup.go index fdede20cc4..7fef20503b 100644 --- a/coderd/workspaceapps/apptest/setup.go +++ b/coderd/workspaceapps/apptest/setup.go @@ -159,10 +159,16 @@ func (d *Details) PathAppURL(app App) *url.URL { // SubdomainAppURL returns the URL for the given subdomain app. func (d *Details) SubdomainAppURL(app App) *url.URL { + // Agent name is optional when app slug is present + agentName := app.AgentName + if !appurl.PortRegex.MatchString(app.AppSlugOrPort) { + agentName = "" + } + appHost := appurl.ApplicationURL{ Prefix: app.Prefix, AppSlugOrPort: app.AppSlugOrPort, - AgentName: app.AgentName, + AgentName: agentName, WorkspaceName: app.WorkspaceName, Username: app.Username, } @@ -234,7 +240,7 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De details.Apps.Owner = App{ Username: me.Username, WorkspaceName: workspace.Name, - AgentName: agnt.Name, + AgentName: "", // Agent name is optional when app slug is present AppSlugOrPort: proxyTestAppNameOwner, Query: proxyTestAppQuery, } @@ -474,7 +480,6 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U // findProtoApp is needed as the order of apps returned from PG database // is not guaranteed. AppSlugOrPort: findProtoApp(t, protoApps, app.Slug).Slug, - AgentName: proxyTestAgentName, WorkspaceName: workspace.Name, Username: me.Username, } diff --git a/coderd/workspaceapps/appurl/appurl.go b/coderd/workspaceapps/appurl/appurl.go index 2676c07164..65dced6c10 100644 --- a/coderd/workspaceapps/appurl/appurl.go +++ b/coderd/workspaceapps/appurl/appurl.go @@ -14,10 +14,12 @@ import ( var ( // nameRegex is the same as our UsernameRegex without the ^ and $. nameRegex = "[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*" - appURL = regexp.MustCompile(fmt.Sprintf( - // {PORT/APP_SLUG}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} - `^(?P%[1]s)--(?P%[1]s)--(?P%[1]s)--(?P%[1]s)$`, + // Supports apps with and without agent name + // Format: {PORT/APP_SLUG}[--{AGENT_NAME}]--{WORKSPACE_NAME}--{USERNAME} + appURL = regexp.MustCompile(fmt.Sprintf( + `^(?P%[1]s)(?:--(?P%[1]s))?--(?P%[1]s)--(?P%[1]s)$`, nameRegex)) + PortRegex = regexp.MustCompile(`^\d{4}s?$`) validHostnameLabelRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) ) @@ -67,8 +69,10 @@ func (a ApplicationURL) String() string { var appURL strings.Builder _, _ = appURL.WriteString(a.Prefix) _, _ = appURL.WriteString(a.AppSlugOrPort) - _, _ = appURL.WriteString("--") - _, _ = appURL.WriteString(a.AgentName) + if a.AgentName != "" { + _, _ = appURL.WriteString("--") + _, _ = appURL.WriteString(a.AgentName) + } _, _ = appURL.WriteString("--") _, _ = appURL.WriteString(a.WorkspaceName) _, _ = appURL.WriteString("--") @@ -81,7 +85,10 @@ func (a ApplicationURL) String() string { // `{variable}` syntax to extract these parts. For testing purposes and for // completeness of this package, we include it. func (a ApplicationURL) Path() string { - return fmt.Sprintf("/@%s/%s.%s/apps/%s", a.Username, a.WorkspaceName, a.AgentName, a.AppSlugOrPort) + if a.AgentName != "" { + return fmt.Sprintf("/@%s/%s.%s/apps/%s", a.Username, a.WorkspaceName, a.AgentName, a.AppSlugOrPort) + } + return fmt.Sprintf("/@%s/%s/apps/%s", a.Username, a.WorkspaceName, a.AppSlugOrPort) } // PortInfo returns the port, protocol, and whether the AppSlugOrPort is a port or not. @@ -140,13 +147,18 @@ func (a *ApplicationURL) ChangePortProtocol(target string) ApplicationURL { // // Subdomains should be in the form: // -// ({PREFIX}---)?{PORT{s?}/APP_SLUG}--{AGENT_NAME}--{WORKSPACE_NAME}--{USERNAME} -// e.g. -// https://8080--main--dev--dean.hi.c8s.io -// https://8080s--main--dev--dean.hi.c8s.io -// https://app--main--dev--dean.hi.c8s.io -// https://prefix---8080--main--dev--dean.hi.c8s.io -// https://prefix---app--main--dev--dean.hi.c8s.io +// ({PREFIX}---)?{PORT{s?}/APP_SLUG}[--{AGENT_NAME}]--{WORKSPACE_NAME}--{USERNAME} +// +// Where agent name is: +// - REQUIRED for ports: 8080--agent--workspace--user, 8080s--agent--workspace--user +// - OPTIONAL for app slugs: myapp--workspace--user (agent name omitted) +// +// Examples: +// - https://8080--main--dev--dean.hi.c8s.io (port with required agent) +// - https://8080s--main--dev--dean.hi.c8s.io (port with required agent) +// - https://app--dev--dean.hi.c8s.io (app slug, no agent name required) +// - https://prefix---8080--main--dev--dean.hi.c8s.io (port with prefix) +// - https://prefix---app--dev--dean.hi.c8s.io (app slug with prefix) // // The optional prefix is permitted to allow customers to put additional URL at // the beginning of their application URL (i.e. if they want to simulate @@ -154,9 +166,6 @@ func (a *ApplicationURL) ChangePortProtocol(target string) ApplicationURL { // // Prefix requires three hyphens at the end to separate it from the rest of the // URL so we can add/remove segments in the future from the parsing logic. -// -// TODO(dean): make the agent name optional when using the app slug. This will -// reduce the character count for app URLs. func ParseSubdomainAppURL(subdomain string) (ApplicationURL, error) { var ( prefixSegments = strings.Split(subdomain, "---") @@ -167,18 +176,29 @@ func ParseSubdomainAppURL(subdomain string) (ApplicationURL, error) { subdomain = prefixSegments[len(prefixSegments)-1] } - matches := appURL.FindAllStringSubmatch(subdomain, -1) - if len(matches) == 0 { + matches := appURL.FindStringSubmatch(subdomain) + if matches == nil { return ApplicationURL{}, xerrors.Errorf("invalid application url format: %q", subdomain) } - matchGroup := matches[0] + + appSlug := matches[appURL.SubexpIndex("AppSlug")] + agentName := matches[appURL.SubexpIndex("AgentName")] + + // Agent name is optional for app slugs but required for ports + if PortRegex.MatchString(appSlug) { + if agentName == "" { + return ApplicationURL{}, xerrors.Errorf("agent name is required for port-based URLs: %q", subdomain) + } + } else { + agentName = "" + } return ApplicationURL{ Prefix: prefix, - AppSlugOrPort: matchGroup[appURL.SubexpIndex("AppSlug")], - AgentName: matchGroup[appURL.SubexpIndex("AgentName")], - WorkspaceName: matchGroup[appURL.SubexpIndex("WorkspaceName")], - Username: matchGroup[appURL.SubexpIndex("Username")], + AppSlugOrPort: appSlug, + AgentName: agentName, + WorkspaceName: matches[appURL.SubexpIndex("WorkspaceName")], + Username: matches[appURL.SubexpIndex("Username")], }, nil } diff --git a/coderd/workspaceapps/appurl/appurl_test.go b/coderd/workspaceapps/appurl/appurl_test.go index 9dfdb4452c..a02a2a1efb 100644 --- a/coderd/workspaceapps/appurl/appurl_test.go +++ b/coderd/workspaceapps/appurl/appurl_test.go @@ -20,7 +20,7 @@ func TestApplicationURLString(t *testing.T) { { Name: "Empty", URL: appurl.ApplicationURL{}, - Expected: "------", + Expected: "----", }, { Name: "AppName", @@ -53,6 +53,66 @@ func TestApplicationURLString(t *testing.T) { }, Expected: "yolo---app--agent--workspace--user", }, + { + Name: "5DigitAppSlug", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "30000", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "30000--workspace--user", + }, + { + Name: "4DigitPort", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "1234", + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "1234--agent--workspace--user", + }, + { + Name: "3DigitPort", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "123", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "123--workspace--user", + }, + { + Name: "LegacyAppSlug_WithAgent_StillWorks", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "myapp", + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "myapp--agent--workspace--user", + }, + { + Name: "AppSlug_WithNumbers", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "app123", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "app123--workspace--user", + }, + { + Name: "NumbersWithLetters", + URL: appurl.ApplicationURL{ + AppSlugOrPort: "8080abc", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + Expected: "8080abc--workspace--user", + }, } for _, c := range testCases { @@ -91,10 +151,14 @@ func TestParseSubdomainAppURL(t *testing.T) { ExpectedError: "invalid application url format", }, { - Name: "Invalid_App--Workspace--User", - Subdomain: "app--workspace--user", - Expected: appurl.ApplicationURL{}, - ExpectedError: "invalid application url format", + Name: "Valid_App--Workspace--User", + Subdomain: "app--workspace--user", + Expected: appurl.ApplicationURL{ + AppSlugOrPort: "app", + AgentName: "", // Agent name is optional when app slug is present + WorkspaceName: "workspace", + Username: "user", + }, }, { Name: "Invalid_TooManyComponents", @@ -102,13 +166,19 @@ func TestParseSubdomainAppURL(t *testing.T) { Expected: appurl.ApplicationURL{}, ExpectedError: "invalid application url format", }, + { + Name: "Invalid_Port--Workspace--User", + Subdomain: "8080--workspace--user", + Expected: appurl.ApplicationURL{}, + ExpectedError: "agent name is required for port-based URLs", + }, // Correct { Name: "AppName--Agent--Workspace--User", Subdomain: "app--agent--workspace--user", Expected: appurl.ApplicationURL{ AppSlugOrPort: "app", - AgentName: "agent", + AgentName: "", WorkspaceName: "workspace", Username: "user", }, @@ -138,7 +208,7 @@ func TestParseSubdomainAppURL(t *testing.T) { Subdomain: "app-slug--agent-name--workspace-name--user-name", Expected: appurl.ApplicationURL{ AppSlugOrPort: "app-slug", - AgentName: "agent-name", + AgentName: "", WorkspaceName: "workspace-name", Username: "user-name", }, @@ -149,7 +219,49 @@ func TestParseSubdomainAppURL(t *testing.T) { Expected: appurl.ApplicationURL{ Prefix: "dean---was---here---", AppSlugOrPort: "app", - AgentName: "agent", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + }, + { + Name: "5DigitAppSlug--Workspace--User", + Subdomain: "30000--workspace--user", + Expected: appurl.ApplicationURL{ + AppSlugOrPort: "30000", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + }, + { + Name: "Invalid_4DigitPort--Workspace--User", + Subdomain: "1234--workspace--user", + Expected: appurl.ApplicationURL{}, + ExpectedError: "agent name is required for port-based URLs", + }, + { + Name: "3DigitPort_WithoutAgent", + Subdomain: "123--workspace--user", + Expected: appurl.ApplicationURL{ + AppSlugOrPort: "123", + AgentName: "", + WorkspaceName: "workspace", + Username: "user", + }, + }, + { + Name: "Invalid_4DigitPortS_WithoutAgent", + Subdomain: "8080s--workspace--user", + Expected: appurl.ApplicationURL{}, + ExpectedError: "agent name is required for port-based URLs", + }, + { + Name: "ParseLegacyAppSlug_WithAgent", + Subdomain: "myapp--agent--workspace--user", + Expected: appurl.ApplicationURL{ + AppSlugOrPort: "myapp", + AgentName: "", WorkspaceName: "workspace", Username: "user", }, @@ -461,3 +573,52 @@ func TestConvertAppURLForCSP(t *testing.T) { }) } } + +func TestURLGenerationVsParsing(t *testing.T) { + t.Parallel() + + testCases := []struct { + Name string + AppSlugOrPort string + AgentName string + ExpectedParsed string + }{ + { + Name: "AppSlug_AgentOmittedInParsing", + AppSlugOrPort: "myapp", + AgentName: "agent", + ExpectedParsed: "", + }, + { + Name: "4DigitPort_AgentPreserved", + AppSlugOrPort: "8080", + AgentName: "agent", + ExpectedParsed: "agent", + }, + { + Name: "5DigitAppSlug_AgentOmittedInParsing", + AppSlugOrPort: "30000", + AgentName: "agent", + ExpectedParsed: "", + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + original := appurl.ApplicationURL{ + AppSlugOrPort: tc.AppSlugOrPort, + AgentName: tc.AgentName, + WorkspaceName: "workspace", + Username: "user", + } + + urlString := original.String() + parsed, err := appurl.ParseSubdomainAppURL(urlString) + require.NoError(t, err) + + require.Equal(t, tc.ExpectedParsed, parsed.AgentName, + "Agent name should be '%s' after parsing", tc.ExpectedParsed) + }) + } +}