From 2096b72c1a5b54c4dff53289ddd91c8e45ef73ac Mon Sep 17 00:00:00 2001 From: Gustavo Bertoi Date: Mon, 29 Jun 2026 11:37:59 -0300 Subject: [PATCH] =?UTF-8?q?feat(dns):=20N3=20=E2=80=94=20marker-fenced=20/?= =?UTF-8?q?etc/hosts=20manager=20+=20`dns`=20CLI=20(spec=2005)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit internal/dns owns the devstack block in /etc/hosts for resolving ..localhost on OS-resolver clients (the only consistently reliable mechanism cross-platform — spec 05 gotchas). All ops are idempotent and operate on an injectable path (fully temp-file-testable): - Block/Apply/Remove/Present/Missing: Apply replaces (or appends, or on absent creates) ONLY the fenced block — content outside the fence is never touched; Apply(nil) removes it; Remove restores the original byte-for-byte; the file's mode is preserved. - CLI `dns setup|status|remove` derives hostnames from the proxy route table (network.proxy.engine: caddy); writing /etc/hosts needs root, so a permission failure maps to a `sudo` remediation (per locked decision #3: build the logic, test with temp files, flag the human/sudo step). Unit tests (temp file): insert preserving original content, idempotent re-apply, replace-not-append, remove-strips-block-only + exact restore, Apply(nil) removes, Present/Missing diff, create-when-absent. CLI tests: registration + status reports the route host. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/dns.go | 129 +++++++++++++++++++++ internal/cli/dns_test.go | 40 +++++++ internal/cli/root.go | 1 + internal/cli/stubs.go | 3 - internal/dns/hosts.go | 223 +++++++++++++++++++++++++++++++++++++ internal/dns/hosts_test.go | 159 ++++++++++++++++++++++++++ 6 files changed, 552 insertions(+), 3 deletions(-) create mode 100644 internal/cli/dns.go create mode 100644 internal/cli/dns_test.go create mode 100644 internal/dns/hosts.go create mode 100644 internal/dns/hosts_test.go diff --git a/internal/cli/dns.go b/internal/cli/dns.go new file mode 100644 index 0000000..d4f5b4f --- /dev/null +++ b/internal/cli/dns.go @@ -0,0 +1,129 @@ +package cli + +import ( + "errors" + "fmt" + "io/fs" + + "github.com/spf13/cobra" + + "github.com/open-source-cloud/devstack/internal/dns" + "github.com/open-source-cloud/devstack/internal/proxy" +) + +// newDnsCmd wires `dns setup|status|remove` — the marker-fenced /etc/hosts block +// for ..localhost (spec 05). Hostnames come from the proxy +// route table (network.proxy.engine: caddy). Writing /etc/hosts needs root; the +// command surfaces a clear sudo remediation. +func newDnsCmd(g *GlobalOpts) *cobra.Command { + cmd := &cobra.Command{ + Use: "dns", + Short: "Manage the /etc/hosts entries for *.localhost service URLs", + } + cmd.AddCommand(newDnsSetupCmd(g), newDnsStatusCmd(g), newDnsRemoveCmd(g)) + return cmd +} + +// dnsHosts returns the local hostnames the workspace's proxy routes resolve to. +func dnsHosts() ([]string, error) { + m, err := loadWorkspace() + if err != nil { + return nil, err + } + routes := proxy.BuildRoutes(m) + hosts := make([]string, 0, len(routes)) + for _, r := range routes { + hosts = append(hosts, r.Host) + } + return hosts, nil +} + +func newDnsSetupCmd(g *GlobalOpts) *cobra.Command { + return &cobra.Command{ + Use: "setup", + Short: "Write the devstack-managed /etc/hosts block (needs sudo)", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + hosts, err := dnsHosts() + if err != nil { + return err + } + if len(hosts) == 0 { + fmt.Fprintln(cmd.OutOrStdout(), "no proxy routes (set network.proxy.engine: caddy and add service ports)") + return nil + } + changed, err := dns.Apply(dns.DefaultHostsPath, hosts) + if err != nil { + return hostsPermError(err) + } + if g.JSON { + return writeJSON(cmd, map[string]any{"hosts": hosts, "changed": changed}) + } + w := cmd.OutOrStdout() + if changed { + fmt.Fprintf(w, "updated %s with %d host(s)\n", dns.DefaultHostsPath, len(hosts)) + } else { + fmt.Fprintln(w, "/etc/hosts already up to date") + } + return nil + }, + } +} + +func newDnsStatusCmd(g *GlobalOpts) *cobra.Command { + return &cobra.Command{ + Use: "status", + Short: "Show which *.localhost entries are present/missing in /etc/hosts", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + hosts, err := dnsHosts() + if err != nil { + return err + } + present, err := dns.Present(dns.DefaultHostsPath) + if err != nil { + return err + } + missing, err := dns.Missing(dns.DefaultHostsPath, hosts) + if err != nil { + return err + } + if g.JSON { + return writeJSON(cmd, map[string]any{"present": present, "missing": missing}) + } + w := cmd.OutOrStdout() + fmt.Fprintf(w, "present: %v\n", present) + if len(missing) > 0 { + fmt.Fprintf(w, "missing: %v (run `devstack dns setup`)\n", missing) + } + return nil + }, + } +} + +func newDnsRemoveCmd(g *GlobalOpts) *cobra.Command { + return &cobra.Command{ + Use: "remove", + Short: "Remove the devstack-managed /etc/hosts block (needs sudo)", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + changed, err := dns.Remove(dns.DefaultHostsPath) + if err != nil { + return hostsPermError(err) + } + if g.JSON { + return writeJSON(cmd, map[string]any{"changed": changed}) + } + fmt.Fprintf(cmd.OutOrStdout(), "removed devstack /etc/hosts block: changed=%v\n", changed) + return nil + }, + } +} + +// hostsPermError maps a permission failure to the sudo remediation. +func hostsPermError(err error) error { + if errors.Is(err, fs.ErrPermission) { + return fmt.Errorf("%w\nhint: editing %s needs root — re-run with `sudo`", err, dns.DefaultHostsPath) + } + return err +} diff --git a/internal/cli/dns_test.go b/internal/cli/dns_test.go new file mode 100644 index 0000000..28c2a65 --- /dev/null +++ b/internal/cli/dns_test.go @@ -0,0 +1,40 @@ +package cli + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestDnsRegistered(t *testing.T) { + root := NewRootCmd(Options{}) + for _, sub := range []string{"setup", "status", "remove"} { + c, _, err := root.Find([]string{"dns", sub}) + if err != nil || c.Name() != sub || c.RunE == nil { + t.Errorf("dns %s not registered as a real command: %v", sub, err) + } + } +} + +func TestDnsStatusReportsRoutes(t *testing.T) { + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "workspace.yaml"), + "apiVersion: devstack/v1\nkind: Workspace\nname: shop\nnetwork: { proxy: { engine: caddy } }\nprojects:\n - { name: api, path: api }\n") + mustWrite(t, filepath.Join(dir, "api", "devstack.yaml"), + "apiVersion: devstack/v1\nkind: Project\nname: api\nservices:\n web: { template: node.vite, ports: { http: 8080 } }\n") + t.Chdir(dir) + + var out strings.Builder + root := NewRootCmd(Options{}) + root.SetArgs([]string{"dns", "status"}) + root.SetOut(&out) + root.SetErr(&out) + if err := root.Execute(); err != nil { + t.Fatalf("dns status: %v\n%s", err, out.String()) + } + // The proxy route host should appear (present or missing — /etc/hosts is read + // only here and won't contain it, so it lands in missing). + if !strings.Contains(out.String(), "web.api.localhost") { + t.Errorf("dns status should mention the route host:\n%s", out.String()) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index b11d6c4..56140d5 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -69,6 +69,7 @@ func NewRootCmd(opts Options) *cobra.Command { newUpCmd(g), newDownCmd(g), newStatusCmd(g), + newDnsCmd(g), newDoctorCmd(g), newConfigCmd(g), newGenerateCmd(g), diff --git a/internal/cli/stubs.go b/internal/cli/stubs.go index 6f424af..e3bbf62 100644 --- a/internal/cli/stubs.go +++ b/internal/cli/stubs.go @@ -41,9 +41,6 @@ func addStubCommands(root *cobra.Command, _ *GlobalOpts) { stub("uninstall", "Remove the local root CA from trust stores", "M5"), stub("status", "Show local CA trust status", "M5"), ), - stub("dns", "Local DNS helpers (*.localhost)", "M5", - stub("setup", "Configure local DNS resolution", "M5"), - ), stub("tunnel", "Optional public tunnel via cloudflared", "M5", stub("login", "Authenticate cloudflared", "M5"), stub("create", "Create a named tunnel", "M5"), diff --git a/internal/dns/hosts.go b/internal/dns/hosts.go new file mode 100644 index 0000000..1b1566a --- /dev/null +++ b/internal/dns/hosts.go @@ -0,0 +1,223 @@ +// Package dns manages the marker-fenced /etc/hosts block devstack owns for +// resolving ..localhost on OS-resolver clients (spec 05). A +// fenced block is the only consistently reliable mechanism cross-platform +// (systemd-resolved isn't guaranteed on WSL2/minimal Ubuntu; macOS ≤15 resolves +// *.localhost only in browsers; Firefox ignores /etc/hosts for *.localhost). +// +// All operations are idempotent and operate on an injectable path so they are +// fully testable against a temp file; writing the real /etc/hosts needs root +// (the CLI surfaces a clear sudo remediation). Entries outside the fence are +// never touched, and the block is removed cleanly on uninstall. +package dns + +import ( + "bufio" + "fmt" + "os" + "sort" + "strings" +) + +// DefaultHostsPath is the OS hosts file (override in tests). +const DefaultHostsPath = "/etc/hosts" + +// Loopback is the address every managed host resolves to. +const Loopback = "127.0.0.1" + +// Fence markers delimiting the devstack-owned block. Kept stable forever — they +// are how we find and replace our block without disturbing the rest of the file. +const ( + markerBegin = "# >>> devstack >>> (managed — do not edit)" + markerEnd = "# <<< devstack <<<" +) + +// Block renders the fenced block for the given hosts (deduped, sorted), each +// mapped to 127.0.0.1. Returns "" when there are no hosts. +func Block(hosts []string) string { + clean := normalize(hosts) + if len(clean) == 0 { + return "" + } + var b strings.Builder + b.WriteString(markerBegin) + b.WriteByte('\n') + for _, h := range clean { + fmt.Fprintf(&b, "%s\t%s\n", Loopback, h) + } + b.WriteString(markerEnd) + b.WriteByte('\n') + return b.String() +} + +// Apply idempotently writes the devstack block into the file at path: it replaces +// an existing fenced block (or appends one) and leaves everything else intact. +// With no hosts it removes the block. Returns whether the file changed. +func Apply(path string, hosts []string) (bool, error) { + if len(normalize(hosts)) == 0 { + return Remove(path) + } + existing, err := readFile(path) + if err != nil { + if !os.IsNotExist(err) { + return false, err + } + existing = "" // absent hosts file → create it with just our block + } + before, after, had := splitFence(existing) + block := Block(hosts) + + var next string + if had { + next = before + block + after + } else { + next = ensureTrailingNewline(existing) + block + } + if next == existing { + return false, nil + } + return true, writeFile(path, next) +} + +// Remove strips the devstack block (no-op if absent). Returns whether it changed. +func Remove(path string) (bool, error) { + existing, err := readFile(path) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + before, after, had := splitFence(existing) + if !had { + return false, nil + } + next := strings.TrimRight(before, "\n") + if next != "" { + next += "\n" + } + if rest := strings.TrimLeft(after, "\n"); rest != "" { + next += rest + } + return true, writeFile(path, next) +} + +// Present returns the hostnames currently inside the devstack block. +func Present(path string) ([]string, error) { + existing, err := readFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + _, _, had := splitFence(existing) + if !had { + return nil, nil + } + var out []string + in := false + sc := bufio.NewScanner(strings.NewReader(existing)) + for sc.Scan() { + line := sc.Text() + switch { + case strings.TrimSpace(line) == markerBegin: + in = true + case strings.TrimSpace(line) == markerEnd: + in = false + case in: + if fields := strings.Fields(line); len(fields) >= 2 { + out = append(out, fields[1]) + } + } + } + return out, sc.Err() +} + +// Missing returns the desired hosts not currently present in the block (for a +// `dns status` diff without mutating anything). +func Missing(path string, want []string) ([]string, error) { + present, err := Present(path) + if err != nil { + return nil, err + } + have := map[string]bool{} + for _, h := range present { + have[h] = true + } + var missing []string + for _, h := range normalize(want) { + if !have[h] { + missing = append(missing, h) + } + } + return missing, nil +} + +// --- helpers --------------------------------------------------------------- + +// splitFence returns the content before the begin marker, after the end marker, +// and whether a complete fence was found. `before` keeps its trailing newline; +// `after` keeps its leading newline, so before+block+after round-trips. +func splitFence(s string) (before, after string, had bool) { + bi := strings.Index(s, markerBegin) + if bi < 0 { + return s, "", false + } + ei := strings.Index(s, markerEnd) + if ei < 0 || ei < bi { + return s, "", false + } + end := ei + len(markerEnd) + // Consume the newline immediately after the end marker so the block owns it. + if end < len(s) && s[end] == '\n' { + end++ + } + return s[:bi], s[end:], true +} + +func normalize(hosts []string) []string { + seen := map[string]bool{} + var out []string + for _, h := range hosts { + h = strings.TrimSpace(h) + if h == "" || seen[h] { + continue + } + seen[h] = true + out = append(out, h) + } + sort.Strings(out) + return out +} + +func ensureTrailingNewline(s string) string { + if s == "" || strings.HasSuffix(s, "\n") { + return s + } + return s + "\n" +} + +func readFile(path string) (string, error) { + b, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return "", err + } + return "", fmt.Errorf("read %s: %w", path, err) + } + return string(b), nil +} + +// writeFile preserves the file's existing mode (default 0644) and writes in place. +// /etc/hosts must be edited in place (atomic rename across the / mount can fail +// and some platforms require the inode preserved); callers hold the needed privs. +func writeFile(path, content string) error { + mode := os.FileMode(0o644) + if fi, err := os.Stat(path); err == nil { + mode = fi.Mode().Perm() + } + if err := os.WriteFile(path, []byte(content), mode); err != nil { + return fmt.Errorf("write %s: %w", path, err) + } + return nil +} diff --git a/internal/dns/hosts_test.go b/internal/dns/hosts_test.go new file mode 100644 index 0000000..99af5e0 --- /dev/null +++ b/internal/dns/hosts_test.go @@ -0,0 +1,159 @@ +package dns + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func tmpHosts(t *testing.T, content string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "hosts") + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return p +} + +func read(t *testing.T, p string) string { + t.Helper() + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +const baseHosts = "127.0.0.1 localhost\n::1 localhost\n" + +func TestApplyInsertsFencedBlock(t *testing.T) { + p := tmpHosts(t, baseHosts) + changed, err := Apply(p, []string{"api.shop.localhost", "web.shop.localhost"}) + if err != nil || !changed { + t.Fatalf("Apply = %v, %v; want changed", changed, err) + } + got := read(t, p) + // Original content preserved. + if !strings.HasPrefix(got, baseHosts) { + t.Errorf("original hosts content was disturbed:\n%s", got) + } + // Block present with both entries → 127.0.0.1. + for _, h := range []string{"api.shop.localhost", "web.shop.localhost"} { + if !strings.Contains(got, "127.0.0.1\t"+h) { + t.Errorf("missing entry for %s:\n%s", h, got) + } + } + if !strings.Contains(got, markerBegin) || !strings.Contains(got, markerEnd) { + t.Errorf("fence markers missing:\n%s", got) + } +} + +func TestApplyIdempotent(t *testing.T) { + p := tmpHosts(t, baseHosts) + hosts := []string{"api.shop.localhost"} + if _, err := Apply(p, hosts); err != nil { + t.Fatal(err) + } + first := read(t, p) + changed, err := Apply(p, hosts) + if err != nil { + t.Fatal(err) + } + if changed { + t.Error("re-applying the same hosts should report no change") + } + if read(t, p) != first { + t.Error("idempotent Apply must not modify the file") + } +} + +func TestApplyReplacesBlockNotAppend(t *testing.T) { + p := tmpHosts(t, baseHosts) + if _, err := Apply(p, []string{"old.localhost"}); err != nil { + t.Fatal(err) + } + if _, err := Apply(p, []string{"new.localhost"}); err != nil { + t.Fatal(err) + } + got := read(t, p) + if strings.Contains(got, "old.localhost") { + t.Errorf("stale entry not replaced:\n%s", got) + } + if !strings.Contains(got, "new.localhost") { + t.Errorf("new entry missing:\n%s", got) + } + if strings.Count(got, markerBegin) != 1 { + t.Errorf("must keep exactly one fenced block:\n%s", got) + } +} + +func TestRemoveStripsBlockOnly(t *testing.T) { + p := tmpHosts(t, baseHosts) + if _, err := Apply(p, []string{"api.shop.localhost"}); err != nil { + t.Fatal(err) + } + changed, err := Remove(p) + if err != nil || !changed { + t.Fatalf("Remove = %v, %v; want changed", changed, err) + } + got := read(t, p) + if strings.Contains(got, markerBegin) || strings.Contains(got, "api.shop.localhost") { + t.Errorf("block not removed:\n%s", got) + } + if got != baseHosts { + t.Errorf("Remove should restore the original content exactly, got:\n%q", got) + } + // Removing again is a no-op. + if changed, _ := Remove(p); changed { + t.Error("second Remove should be a no-op") + } +} + +func TestApplyEmptyRemoves(t *testing.T) { + p := tmpHosts(t, baseHosts) + if _, err := Apply(p, []string{"api.localhost"}); err != nil { + t.Fatal(err) + } + if _, err := Apply(p, nil); err != nil { + t.Fatal(err) + } + if read(t, p) != baseHosts { + t.Errorf("Apply(nil) should remove the block, got:\n%q", read(t, p)) + } +} + +func TestPresentAndMissing(t *testing.T) { + p := tmpHosts(t, baseHosts) + if _, err := Apply(p, []string{"a.localhost", "b.localhost"}); err != nil { + t.Fatal(err) + } + present, err := Present(p) + if err != nil { + t.Fatal(err) + } + if len(present) != 2 { + t.Errorf("Present = %v, want 2", present) + } + missing, err := Missing(p, []string{"a.localhost", "c.localhost"}) + if err != nil { + t.Fatal(err) + } + if len(missing) != 1 || missing[0] != "c.localhost" { + t.Errorf("Missing = %v, want [c.localhost]", missing) + } +} + +func TestApplyCreatesFileWhenAbsent(t *testing.T) { + p := filepath.Join(t.TempDir(), "newhosts") + // Apply on a non-existent file currently errors on read; ensure a graceful + // path: Remove on absent is a no-op, Apply on absent should create it. + changed, err := Apply(p, []string{"x.localhost"}) + if err != nil { + // Acceptable only if it's a not-exist that we then surface; assert create. + t.Fatalf("Apply on absent file: %v", err) + } + if !changed || !strings.Contains(read(t, p), "x.localhost") { + t.Errorf("Apply should create the file with the block") + } +}