diff --git a/CHANGELOG.md b/CHANGELOG.md index fb1a6370f..6c77faa4a 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/pkg/commands/compute/serve.go b/pkg/commands/compute/serve.go index 26b4dc49a..067598f7d 100644 --- a/pkg/commands/compute/serve.go +++ b/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. diff --git a/pkg/commands/compute/serve_internal_test.go b/pkg/commands/compute/serve_internal_test.go index e2007d64f..454532d03 100644 --- a/pkg/commands/compute/serve_internal_test.go +++ b/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 TestIsContentChange(t *testing.T) { }) } } + +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()) + } + }) + } +}