fix(compute/serve): provide default override_host to Viceroy via temporary manifest and support IPv6 hostnames

CHANGELOG.md

@@ -6,6 +6,8 @@
 
 ### Bug Fixes:
 
+- fix(compute/serve): provide default `override_host` to Viceroy via temporary manifest and handle IPv6 hostnames ([#1940](https://github.com/fastly/cli/pull/1940))
+
 ### Enhancements:
 
 ### Dependencies:

pkg/commands/compute/serve.go

@@ -174,8 +174,6 @@ func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		text.Break(out)
 	}
 
-	c.setBackendsWithDefaultOverrideHostIfMissing(out)
-
 	spinner, err := text.NewSpinner(out)
 	if err != nil {
 		return err
@@ -204,6 +202,12 @@ func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		}
 	}
 
+	manifestToServe, cleanup, err := c.setBackendsWithDefaultOverrideHostIfMissing(out, manifestPath)
+	if err != nil {
+		return err
+	}
+	defer cleanup()
+
 	bin, err := c.GetViceroy(spinner, out, manifestPath)
 	if err != nil {
 		return err
@@ -264,7 +268,7 @@ func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) {
 			enableWebsocketsPassthrough: enableWebsocketsPassthrough,
 			errLog:                      c.Globals.ErrLog,
 			extraArgs:                   c.ViceroyBinExtraArgs,
-			manifestPath:                manifestPath,
+			manifestPath:                manifestToServe,
 			out:                         out,
 			profileGuest:                c.profileGuest,
 			profileGuestDir:             c.profileGuestDir,
@@ -336,23 +340,51 @@ func (c *ServeCommand) Build(in io.Reader, out io.Writer) error {
 
 // setBackendsWithDefaultOverrideHostIfMissing sets an override_host for any
 // local_server.backends that is missing that property. The value will only be
-// set if the URL defined uses a hostname (e.g. http://127.0.0.1/ won't) so we
-// can set the override_host to match the hostname.
-func (c *ServeCommand) setBackendsWithDefaultOverrideHostIfMissing(out io.Writer) {
+// set if the URL defined uses a hostname (e.g. http://127.0.0.1/ or http://[::1]/ won't)
+// so we can set the override_host to match the hostname.
+//
+// Viceroy reads backend configuration from the manifest file on disk. When any
+// default override_host values are applied, the modified manifest is written
+// to a temporary file passed to Viceroy to avoid mutating the user's project file.
+func (c *ServeCommand) setBackendsWithDefaultOverrideHostIfMissing(out io.Writer, manifestPath string) (string, func(), error) {
+	updated := false
 	for k, backend := range c.Globals.Manifest.File.LocalServer.Backends {
 		if backend.OverrideHost == "" {
 			if u, err := url.Parse(backend.URL); err == nil {
-				segs := strings.Split(u.Host, ":") // avoid parsing IP with port
-				if ip := net.ParseIP(segs[0]); ip == nil {
-					if c.Globals.Verbose() {
-						text.Info(out, "[local_server.backends.%s] (%s) is configured without an `override_host`. We will use %s as a default to help avoid any unexpected errors. See https://www.fastly.com/documentation/reference/compute/fastly-toml#local-server for more details.", k, backend.URL, u.Host)
-					}
+				hostname := u.Hostname()
+				if ip := net.ParseIP(hostname); ip == nil && hostname != "" {
+					text.Info(out, "[local_server.backends.%s] (%s) is configured without an `override_host`. We will use %s as a default to help avoid any unexpected errors. See https://www.fastly.com/documentation/reference/compute/fastly-toml#local-server for more details.", k, backend.URL, u.Host)
 					backend.OverrideHost = u.Host
 					c.Globals.Manifest.File.LocalServer.Backends[k] = backend
+					updated = true
 				}
 			}
 		}
 	}
+	if !updated {
+		return manifestPath, func() {}, nil
+	}
+
+	tmpFile, err := os.CreateTemp("", "fastly-serve-*.toml")
+	if err != nil {
+		return "", nil, fmt.Errorf("failed to create temporary manifest: %w", err)
+	}
+	tmpPath := tmpFile.Name()
+	if err := tmpFile.Close(); err != nil {
+		_ = os.Remove(tmpPath)
+		return "", nil, fmt.Errorf("failed to close temporary manifest: %w", err)
+	}
+
+	if err := c.Globals.Manifest.File.Write(tmpPath); err != nil {
+		_ = os.Remove(tmpPath)
+		return "", nil, fmt.Errorf("failed to write temporary manifest: %w", err)
+	}
+
+	cleanup := func() {
+		_ = os.Remove(tmpPath)
+	}
+
+	return tmpPath, cleanup, nil
 }
 
 // GetViceroy returns the path to the installed binary.

pkg/commands/compute/serve_internal_test.go

@@ -1,9 +1,17 @@
 package compute
 
 import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/fsnotify/fsnotify"
+
+	"github.com/fastly/cli/pkg/argparser"
+	"github.com/fastly/cli/pkg/global"
+	"github.com/fastly/cli/pkg/manifest"
 )
 
 // TestIsContentChange checks that content/existence events trigger a rebuild and
@@ -30,3 +38,205 @@
 		})
 	}
 }
+
+func TestSetBackendsWithDefaultOverrideHostIfMissing_TemporaryManifest(t *testing.T) {
+	dir := t.TempDir()
+	manifestPath := filepath.Join(dir, "fastly.toml")
+	const initial = `manifest_version = "3.0.0"
+name = "test"
+
+[local_server.backends.http-me]
+url = "https://http-me.glitch.me"
+`
+	if err := os.WriteFile(manifestPath, []byte(initial), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	var file manifest.File
+	if err := file.Read(manifestPath); err != nil {
+		t.Fatal(err)
+	}
+
+	c := &ServeCommand{
+		Base: argparser.Base{
+			Globals: &global.Data{
+				Manifest: &manifest.Data{File: file},
+			},
+		},
+	}
+
+	var out bytes.Buffer
+	tmpManifest, cleanup, err := c.setBackendsWithDefaultOverrideHostIfMissing(&out, manifestPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
+	if tmpManifest == manifestPath {
+		t.Fatalf("expected temporary manifest path, got original %s", manifestPath)
+	}
+
+	// Verify temporary manifest exists and contains override_host.
+	tmpData, err := os.ReadFile(tmpManifest)
+	if err != nil {
+		t.Fatalf("failed to read temporary manifest: %v", err)
+	}
+	tmpBody := string(tmpData)
+	if !strings.Contains(tmpBody, "override_host") || !strings.Contains(tmpBody, "http-me.glitch.me") {
+		t.Fatalf("expected override_host in temporary manifest, got:\n%s", tmpBody)
+	}
+	if !strings.Contains(out.String(), "http-me.glitch.me") {
+		t.Fatalf("expected override_host info message, got: %q", out.String())
+	}
+
+	// Verify original manifest was not mutated on disk.
+	origWritten, err := os.ReadFile(manifestPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(origWritten) != initial {
+		t.Fatalf("original manifest was mutated on disk:\n%s", string(origWritten))
+	}
+
+	// Verify cleanup removes the temporary file.
+	cleanup()
+	if _, err := os.Stat(tmpManifest); !os.IsNotExist(err) {
+		t.Fatalf("expected temporary manifest to be removed after cleanup, stat err: %v", err)
+	}
+}
+
+func TestSetBackendsWithDefaultOverrideHostIfMissing_BackendMatrix(t *testing.T) {
+	tests := []struct {
+		name                string
+		backendURL          string
+		initialOverrideHost string
+		wantOverrideHost    string
+		wantTemporaryFile   bool
+		wantInfoMessage     bool
+	}{
+		{
+			name:                "hostname without port",
+			backendURL:          "https://example.com",
+			initialOverrideHost: "",
+			wantOverrideHost:    "example.com",
+			wantTemporaryFile:   true,
+			wantInfoMessage:     true,
+		},
+		{
+			name:                "hostname with port",
+			backendURL:          "http://example.com:8080",
+			initialOverrideHost: "",
+			wantOverrideHost:    "example.com:8080",
+			wantTemporaryFile:   true,
+			wantInfoMessage:     true,
+		},
+		{
+			name:                "ipv4 without port",
+			backendURL:          "http://127.0.0.1",
+			initialOverrideHost: "",
+			wantOverrideHost:    "",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+		{
+			name:                "ipv4 with port",
+			backendURL:          "http://127.0.0.1:8080",
+			initialOverrideHost: "",
+			wantOverrideHost:    "",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+		{
+			name:                "ipv6 without port",
+			backendURL:          "http://[::1]",
+			initialOverrideHost: "",
+			wantOverrideHost:    "",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+		{
+			name:                "ipv6 with port",
+			backendURL:          "http://[::1]:8080",
+			initialOverrideHost: "",
+			wantOverrideHost:    "",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+		{
+			name:                "ipv6 full address with port",
+			backendURL:          "http://[2001:db8::1]:8080",
+			initialOverrideHost: "",
+			wantOverrideHost:    "",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+		{
+			name:                "hostname with pre-existing override_host",
+			backendURL:          "https://example.com",
+			initialOverrideHost: "custom.domain.test",
+			wantOverrideHost:    "custom.domain.test",
+			wantTemporaryFile:   false,
+			wantInfoMessage:     false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			manifestPath := filepath.Join(dir, "fastly.toml")
+			if err := os.WriteFile(manifestPath, []byte("manifest_version = \"3.0.0\"\nname = \"test\"\n"), 0o600); err != nil {
+				t.Fatal(err)
+			}
+
+			file := manifest.File{
+				LocalServer: manifest.LocalServer{
+					Backends: map[string]manifest.LocalBackend{
+						"test-backend": {
+							URL:          tc.backendURL,
+							OverrideHost: tc.initialOverrideHost,
+						},
+					},
+				},
+			}
+
+			c := &ServeCommand{
+				Base: argparser.Base{
+					Globals: &global.Data{
+						Manifest: &manifest.Data{File: file},
+					},
+				},
+			}
+
+			var out bytes.Buffer
+			manifestToUse, cleanup, err := c.setBackendsWithDefaultOverrideHostIfMissing(&out, manifestPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer cleanup()
+
+			gotBackend := c.Globals.Manifest.File.LocalServer.Backends["test-backend"]
+			if gotBackend.OverrideHost != tc.wantOverrideHost {
+				t.Errorf("override_host = %q, want %q", gotBackend.OverrideHost, tc.wantOverrideHost)
+			}
+
+			if tc.wantTemporaryFile {
+				if manifestToUse == manifestPath {
+					t.Errorf("expected temporary manifest file, got original path %s", manifestPath)
+				}
+				if _, err := os.Stat(manifestToUse); err != nil {
+					t.Errorf("expected temporary manifest to exist on disk: %v", err)
+				}
+			} else {
+				if manifestToUse != manifestPath {
+					t.Errorf("expected original manifest path %s, got %s", manifestPath, manifestToUse)
+				}
+			}
+
+			if tc.wantInfoMessage && !strings.Contains(out.String(), "is configured without an `override_host`") {
+				t.Errorf("expected info message in output, got: %q", out.String())
+			} else if !tc.wantInfoMessage && strings.Contains(out.String(), "is configured without an `override_host`") {
+				t.Errorf("unexpected info message in output: %q", out.String())
+			}
+		})
+	}
+}