From 26f431710748f7fab8b4bcd3aab7cfcef0337b52 Mon Sep 17 00:00:00 2001 From: Steven Chang Date: Thu, 24 Sep 2026 16:14:50 -0700 Subject: [PATCH] FIX: interpolate env vars on initial dv new --- internal/cli/new.go | 9 +++ internal/cli/shared.go | 6 ++ internal/cli/template_vars.go | 34 +++++++++ internal/cli/template_vars_test.go | 118 +++++++++++++++++++++++++++++ 4 files changed, 167 insertions(+) diff --git a/internal/cli/new.go b/internal/cli/new.go index 8724162..4ddefd8 100644 --- a/internal/cli/new.go +++ b/internal/cli/new.go @@ -452,6 +452,15 @@ func sealProvisionedContainer(cmd *cobra.Command, cfg config.Config, name, workd extraHosts = append(extraHosts, proxyExtraHosts(cfg, name, proxyHost)...) } + // Interpolate ${DISCOURSE_URL} etc. in template env values now that + // applyLocalProxyMetadata has resolved the final hostname/port/scheme. + tvars := buildTemplateVarsFromEnvs(envs) + for k := range templateEnvs { + if v, ok := envs[k]; ok { + envs[k] = interpolateVars(v, tvars) + } + } + if err := docker.RunDetached(name, workdir, snapshotImage, lifecycle.HostPort, lifecycle.ContainerPort, labels, envs, extraHosts, "", templateMounts); err != nil { return withRollback(fmt.Errorf("recreate container without SSH forwarding: %w", err)) } diff --git a/internal/cli/shared.go b/internal/cli/shared.go index aeabeec..e870980 100644 --- a/internal/cli/shared.go +++ b/internal/cli/shared.go @@ -492,6 +492,12 @@ func ensureContainerRunningWithWorkdirResult(cmd *cobra.Command, cfg config.Conf if proxyHost != "" { extraHosts = append(extraHosts, proxyExtraHosts(cfg, name, proxyHost)...) } + tvars := buildTemplateVarsFromEnvs(envs) + for k := range templateEnvs { + if v, ok := envs[k]; ok { + envs[k] = interpolateVars(v, tvars) + } + } if err := docker.RunDetached(name, workdir, imageTag, chosenPort, cfg.ContainerPort, labels, envs, extraHosts, sshAuthSock, templateMounts); err != nil { return result, err } diff --git a/internal/cli/template_vars.go b/internal/cli/template_vars.go index ded0273..5d1f514 100644 --- a/internal/cli/template_vars.go +++ b/internal/cli/template_vars.go @@ -44,6 +44,40 @@ func buildTemplateVars(cfg config.Config, name string) map[string]string { } } +// buildTemplateVarsFromEnvs derives the same four reserved variables from an +// already-populated envs map. Use this in sealProvisionedContainer where the +// proxy metadata has just been written into envs but no container is running +// yet to query. +func buildTemplateVarsFromEnvs(envs map[string]string) map[string]string { + scheme := envs["DV_LOCAL_PROXY_SCHEME"] + if scheme == "" { + scheme = "http" + } + hostname := envs["DISCOURSE_HOSTNAME"] + if hostname == "" { + hostname = "localhost" + } + portStr := envs["DISCOURSE_PORT"] + port, err := strconv.Atoi(portStr) + if err != nil || port <= 0 { + port = 3000 + } + + var discourseURL string + if (scheme == "https" && port == 443) || (scheme == "http" && port == 80) { + discourseURL = fmt.Sprintf("%s://%s", scheme, hostname) + } else { + discourseURL = fmt.Sprintf("%s://%s:%d", scheme, hostname, port) + } + + return map[string]string{ + "DISCOURSE_HOSTNAME": hostname, + "DISCOURSE_PORT": portStr, + "DISCOURSE_SCHEME": scheme, + "DISCOURSE_URL": discourseURL, + } +} + func resolveDiscourseAccess(cfg config.Config, name string) (scheme, hostname string, port int) { lp := cfg.LocalProxy if lp.Enabled { diff --git a/internal/cli/template_vars_test.go b/internal/cli/template_vars_test.go index 62f1df7..4dc50eb 100644 --- a/internal/cli/template_vars_test.go +++ b/internal/cli/template_vars_test.go @@ -101,6 +101,124 @@ func TestResolveDiscourseAccess(t *testing.T) { }) } +func TestBuildTemplateVarsFromEnvs(t *testing.T) { + cases := []struct { + name string + envs map[string]string + wantURL string + wantScheme string + wantHost string + wantPort string + }{ + { + name: "proxy http port 80 omits port from URL", + envs: map[string]string{ + "DISCOURSE_HOSTNAME": "blizzard.dv.localhost", + "DISCOURSE_PORT": "80", + "DV_LOCAL_PROXY_SCHEME": "http", + }, + wantURL: "http://blizzard.dv.localhost", + wantScheme: "http", + wantHost: "blizzard.dv.localhost", + wantPort: "80", + }, + { + name: "proxy https port 443 omits port from URL", + envs: map[string]string{ + "DISCOURSE_HOSTNAME": "blizzard.dv.localhost", + "DISCOURSE_PORT": "443", + "DV_LOCAL_PROXY_SCHEME": "https", + }, + wantURL: "https://blizzard.dv.localhost", + wantScheme: "https", + wantHost: "blizzard.dv.localhost", + wantPort: "443", + }, + { + name: "non-standard port included in URL", + envs: map[string]string{ + "DISCOURSE_HOSTNAME": "blizzard.dv.localhost", + "DISCOURSE_PORT": "9292", + "DV_LOCAL_PROXY_SCHEME": "http", + }, + wantURL: "http://blizzard.dv.localhost:9292", + wantScheme: "http", + wantHost: "blizzard.dv.localhost", + wantPort: "9292", + }, + { + name: "no proxy falls back to localhost:3000", + envs: map[string]string{}, + wantURL: "http://localhost:3000", + wantScheme: "http", + wantHost: "localhost", + wantPort: "", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + vars := buildTemplateVarsFromEnvs(c.envs) + if got := vars["DISCOURSE_URL"]; got != c.wantURL { + t.Errorf("DISCOURSE_URL = %q, want %q", got, c.wantURL) + } + if got := vars["DISCOURSE_SCHEME"]; got != c.wantScheme { + t.Errorf("DISCOURSE_SCHEME = %q, want %q", got, c.wantScheme) + } + if got := vars["DISCOURSE_HOSTNAME"]; got != c.wantHost { + t.Errorf("DISCOURSE_HOSTNAME = %q, want %q", got, c.wantHost) + } + if c.wantPort != "" { + if got := vars["DISCOURSE_PORT"]; got != c.wantPort { + t.Errorf("DISCOURSE_PORT = %q, want %q", got, c.wantPort) + } + } + }) + } +} + +// TestSealEnvInterpolation covers the regression where template env values +// containing ${DISCOURSE_URL} were passed raw to docker run because +// interpolation only happened in executeTemplate (docker exec), not in +// sealProvisionedContainer or ensureContainerRunningWithWorkdirResult (docker run). +func TestSealEnvInterpolation(t *testing.T) { + // Simulate envs after applyLocalProxyMetadata has run with a proxy active. + envs := map[string]string{ + "DISCOURSE_HOSTNAME": "blizzard-wow.dv.localhost", + "DISCOURSE_PORT": "443", + "DV_LOCAL_PROXY_SCHEME": "https", + } + + templateEnvs := map[string]string{ + "DISCOURSE_BNET_AUTHORIZE_URL": "${DISCOURSE_URL}/oauth/authorize", + "DISCOURSE_BLIZZARD_ORCHESTRATION_URL": "${DISCOURSE_URL}/oauth", + "DISCOURSE_BNET_TOKEN_URL": "http://127.0.0.1:3000/oauth/token", + "DISCOURSE_BNET_API_URL": "http://127.0.0.1:3000", + } + for k, v := range templateEnvs { + envs[k] = v + } + + tvars := buildTemplateVarsFromEnvs(envs) + for k := range templateEnvs { + if v, ok := envs[k]; ok { + envs[k] = interpolateVars(v, tvars) + } + } + + want := map[string]string{ + "DISCOURSE_BNET_AUTHORIZE_URL": "https://blizzard-wow.dv.localhost/oauth/authorize", + "DISCOURSE_BLIZZARD_ORCHESTRATION_URL": "https://blizzard-wow.dv.localhost/oauth", + "DISCOURSE_BNET_TOKEN_URL": "http://127.0.0.1:3000/oauth/token", + "DISCOURSE_BNET_API_URL": "http://127.0.0.1:3000", + } + for k, wantVal := range want { + if got := envs[k]; got != wantVal { + t.Errorf("%s = %q, want %q", k, got, wantVal) + } + } +} + func TestBuildTemplateVars_URL(t *testing.T) { cases := []struct { name string