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`
2026-09-22 05:05:20 +08:00 · 2025-09-26 12:25:58 -05:00
parent 403a9e57c9
commit d29a52462b
6 changed files with 359 additions and 99 deletions
@@ -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,
 		}
@@ -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<AppSlug>%[1]s)--(?P<AgentName>%[1]s)--(?P<WorkspaceName>%[1]s)--(?P<Username>%[1]s)$`,
+	// Supports apps with and without agent name
+	// Format: {PORT/APP_SLUG}[--{AGENT_NAME}]--{WORKSPACE_NAME}--{USERNAME}
+	appURL = regexp.MustCompile(fmt.Sprintf(
+		`^(?P<AppSlug>%[1]s)(?:--(?P<AgentName>%[1]s))?--(?P<WorkspaceName>%[1]s)--(?P<Username>%[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
 }

@@ -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)
+		})
+	}
+}