| index 3073ba395..ebe0a0be9 100644 |
| --- a/deploy_test.go |
| +++ b/deploy_test.go |
| @@ -596,10 +596,13 @@ func TestDomainNeedsAttach(t *testing.T) { |
| type deployFakeConsole struct { |
| ts *httptest.Server |
| |
| - mu sync.Mutex |
| - calls []string // "METHOD /path", in hit order |
| - domains []map[string]string |
| - shipVersion string |
| + mu sync.Mutex |
| + calls []string // "METHOD /path", in hit order |
| + domains []map[string]string |
| + shipVersion string |
| + domainsListStatus int // 0 = normal 200 JSON list; else this status with a plain (non-JSON) body — simulates an old console (404) or an unwell one (5xx) |
| + attachStatus int // 0 = normal 201; else this status with an {"error": attachErrMsg} body |
| + attachErrMsg string |
| } |
| |
| func newDeployFakeConsole(t *testing.T, shipVersion string) *deployFakeConsole { |
| @@ -613,12 +616,29 @@ func newDeployFakeConsole(t *testing.T, shipVersion string) *deployFakeConsole { |
| mux.HandleFunc("GET /api/cli/apps/{account}/{app}/domains", func(w http.ResponseWriter, r *http.Request) { |
| fc.record(r) |
| fc.mu.Lock() |
| - domains := fc.domains |
| + domains, status := fc.domains, fc.domainsListStatus |
| fc.mu.Unlock() |
| + if status != 0 { |
| + // A plain body with no JSON "error" field, on purpose: this is |
| + // what an unregistered route (old console) OR a bare 5xx (an |
| + // unwell one) both look like on the wire — neither carries a |
| + // server diagnosis, which is exactly the shape apiErrorFromBody |
| + // treats as "not a real diagnosis, just a status". |
| + w.WriteHeader(status) |
| + return |
| + } |
| json.NewEncoder(w).Encode(map[string]any{"domains": domains}) |
| }) |
| mux.HandleFunc("POST /api/cli/apps/{account}/{app}/domains/{host}", func(w http.ResponseWriter, r *http.Request) { |
| fc.record(r) |
| + fc.mu.Lock() |
| + status, errMsg := fc.attachStatus, fc.attachErrMsg |
| + fc.mu.Unlock() |
| + if status != 0 { |
| + w.WriteHeader(status) |
| + json.NewEncoder(w).Encode(map[string]string{"error": errMsg}) |
| + return |
| + } |
| w.WriteHeader(http.StatusCreated) |
| json.NewEncoder(w).Encode(map[string]string{"host": r.PathValue("host"), "route": "", "dns": ""}) |
| }) |
| @@ -639,6 +659,28 @@ func newDeployFakeConsole(t *testing.T, shipVersion string) *deployFakeConsole { |
| return fc |
| } |
| |
| +// setDomains is the mutex-guarded way to change what the fake's GET |
| +// .../domains answers once the server is already live (t.Setenv-style |
| +// direct field writes are only safe before the httptest.Server starts |
| +// serving requests). |
| +func (fc *deployFakeConsole) setDomains(domains []map[string]string) { |
| + fc.mu.Lock() |
| + defer fc.mu.Unlock() |
| + fc.domains = domains |
| +} |
| + |
| +func (fc *deployFakeConsole) setDomainsListStatus(status int) { |
| + fc.mu.Lock() |
| + defer fc.mu.Unlock() |
| + fc.domainsListStatus = status |
| +} |
| + |
| +func (fc *deployFakeConsole) setAttachRefusal(status int, msg string) { |
| + fc.mu.Lock() |
| + defer fc.mu.Unlock() |
| + fc.attachStatus, fc.attachErrMsg = status, msg |
| +} |
| + |
| func (fc *deployFakeConsole) record(r *http.Request) { |
| fc.mu.Lock() |
| defer fc.mu.Unlock() |
| @@ -775,10 +817,9 @@ func TestDeployExitsHonestlyWhenDNSIsNotPointingYet(t *testing.T) { |
| guide = "create a CNAME 127.0.0.1 -> sitely.oncarlos.test" |
| ) |
| fc := newDeployFakeConsole(t, version) |
| - fc.domains = []map[string]string{{"host": host, "route": "", "dns": "elsewhere", "guidance": guide}} |
| + fc.setDomains([]map[string]string{{"host": host, "route": "", "dns": "elsewhere", "guidance": guide}}) |
| |
| - scratch := t.TempDir() |
| - credPath := filepath.Join(scratch, "credentials") |
| + credPath := filepath.Join(t.TempDir(), "credentials") |
| t.Setenv("CARLOS_CREDENTIALS", credPath) |
| credFile, err := loadCredentials() |
| if err != nil { |
| @@ -790,14 +831,6 @@ func TestDeployExitsHonestlyWhenDNSIsNotPointingYet(t *testing.T) { |
| t.Fatal(err) |
| } |
| |
| - env := append(os.Environ(), |
| - "CARLOS_CREDENTIALS="+credPath, |
| - "HOME="+scratch, |
| - "CARLOS_CONSOLE=", |
| - "CARLOS_DEPLOYMENT_BUCKET=", |
| - "CARLOS_DEPLOYMENT_DIR=", |
| - ) |
| - |
| siteDir := t.TempDir() |
| if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("v1"), 0o644); err != nil { |
| t.Fatal(err) |
| @@ -806,14 +839,21 @@ func TestDeployExitsHonestlyWhenDNSIsNotPointingYet(t *testing.T) { |
| cmd := exec.Command(bin, "deploy", "-app", app, "-account", "ops", "-kind", "static", |
| "-version", version, "-channel", channel, "-host", host, "-console", fc.ts.URL, |
| "-timeout", "100ms", siteDir) |
| - cmd.Env = env |
| + // carlosTestEnv is the repo rule for a subprocess that shells out to the |
| + // carlos binary — it masks CARLOS_CREDENTIALS/HOME/CARLOS_CONSOLE to its |
| + // own scratch space; the explicit CARLOS_CREDENTIALS below wins over |
| + // that default because os/exec keeps only the LAST value for a |
| + // duplicate key, landing the subprocess on the exact credential file |
| + // this test just wrote to (the same path this process's own |
| + // CARLOS_CREDENTIALS was just pointed at, via t.Setenv above). |
| + cmd.Env = carlosTestEnv(t, "CARLOS_CREDENTIALS="+credPath, "CARLOS_DEPLOYMENT_BUCKET=", "CARLOS_DEPLOYMENT_DIR=") |
| cmd.Dir = t.TempDir() // no .carlos/config ancestor — the repo rule (carlosTestEnv's own reasoning) |
| out, err := cmd.CombinedOutput() |
| if err == nil { |
| t.Fatalf("DNS-not-pointing deploy exited 0:\n%s", out) |
| } |
| want := "DNS for " + host + " is not pointing at this deployment yet. Create: " + guide + |
| - ". Watch with 'carlos domains -app " + app + "'; re-run this deploy any time — the shipped release is already promoted." |
| + ". Watch with 'carlos domains -app " + app + " -account ops'; re-run this deploy any time — the shipped release is already promoted." |
| if !strings.Contains(string(out), want) { |
| t.Fatalf("exit text = %q, want it to contain %q", out, want) |
| } |
| @@ -821,3 +861,247 @@ func TestDeployExitsHonestlyWhenDNSIsNotPointingYet(t *testing.T) { |
| t.Fatalf("host already in domainList must not have triggered an attach POST: %v", fc.calls) |
| } |
| } |
| + |
| +// TestDeployDNSGuidanceMessageBothArms pins review fix #1: a host-match alone is |
| +// not enough to say "DNS is not pointing" — DNS == "pointing" with a |
| +// connection-class failure means something else (the certificate, almost |
| +// always) is what's still missing, and printing an empty "Create: ." would |
| +// be a lie by omission. Both arms are driven through a real domainList call |
| +// against a fake console (not hand-built apiDomain values), so the test |
| +// also proves the DNS/Guidance fields survive the wire round trip into the |
| +// message. A third case pins the fall-through: no DNS verdict and no |
| +// guidance yet must defer to the generic timeout report rather than guess. |
| +func TestDeployDNSGuidanceMessageBothArms(t *testing.T) { |
| + fakeDomainsServer := func(t *testing.T, domain map[string]string) []apiDomain { |
| + t.Helper() |
| + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + json.NewEncoder(w).Encode(map[string]any{"domains": []map[string]string{domain}}) |
| + })) |
| + defer ts.Close() |
| + domains, err := newTestSession(ts.URL).domainList(context.Background(), apiAcct{Seg: "ops"}, "sitely") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return domains |
| + } |
| + |
| + // Arm 1: DNS is pointing, so a connection-class failure means the |
| + // certificate isn't issued yet — never "DNS is not pointing" (that |
| + // would be false), and never an empty "Create: ." (pointing carries no |
| + // guidance at all). |
| + domains := fakeDomainsServer(t, map[string]string{"host": "x.test", "dns": "pointing"}) |
| + msg, ok := dnsGuidanceMessage(domains, "x.test", "sitely", apiAcct{Seg: "ops"}) |
| + if !ok { |
| + t.Fatal("pointing arm: want ok") |
| + } |
| + for _, want := range []string{ |
| + "DNS for x.test is pointing here, but the certificate is not issued yet", |
| + "re-run this deploy shortly", |
| + "carlos domains -app sitely -account ops", |
| + } { |
| + if !strings.Contains(msg, want) { |
| + t.Fatalf("pointing-arm message = %q, missing %q", msg, want) |
| + } |
| + } |
| + if strings.Contains(msg, "Create:") || strings.Contains(msg, "not pointing") { |
| + t.Fatalf("pointing arm must not claim DNS is not pointing or print an empty guidance line: %q", msg) |
| + } |
| + |
| + // Arm 2: DNS is elsewhere, with real guidance — the original report. |
| + domains = fakeDomainsServer(t, map[string]string{"host": "x.test", "dns": "elsewhere", "guidance": "create a CNAME"}) |
| + msg, ok = dnsGuidanceMessage(domains, "x.test", "sitely", apiAcct{Seg: "ops"}) |
| + if !ok { |
| + t.Fatal("not-pointing arm: want ok") |
| + } |
| + for _, want := range []string{ |
| + "DNS for x.test is not pointing at this deployment yet. Create: create a CNAME", |
| + "re-run this deploy any time", |
| + "carlos domains -app sitely -account ops", |
| + } { |
| + if !strings.Contains(msg, want) { |
| + t.Fatalf("not-pointing-arm message = %q, missing %q", msg, want) |
| + } |
| + } |
| + |
| + // Fall-through: DNS never checked at all (no dns field, no guidance) — |
| + // neither arm applies, so the caller must fall back to the generic |
| + // watchVersion timeout report rather than print a guessed-at message. |
| + domains = fakeDomainsServer(t, map[string]string{"host": "x.test"}) |
| + if _, ok := dnsGuidanceMessage(domains, "x.test", "sitely", apiAcct{Seg: "ops"}); ok { |
| + t.Fatal("no DNS verdict and no guidance yet must fall through, not print a message") |
| + } |
| + |
| + // A host absent from domains entirely must also fall through. |
| + if _, ok := dnsGuidanceMessage(domains, "not-listed.test", "sitely", apiAcct{Seg: "ops"}); ok { |
| + t.Fatal("a host absent from domainList must fall through, not print a message") |
| + } |
| +} |
| + |
| +// TestDeployAutoAttachStandsDownWhenOldConsoleHasNoDomainsDoor pins review |
| +// fix #2: silence is only right when nothing needed attaching anyway. A |
| +// console that predates the /domains door 404s the list; when the target |
| +// host is genuinely unclaimed (absent from the instance list — always true |
| +// on the no-instance static path), the member is told the door is missing |
| +// and the host needs a console-side attach, and the deploy still PROCEEDS |
| +// (ships and promotes) rather than fataling over a feature this console |
| +// simply cannot offer yet. |
| +func TestDeployAutoAttachStandsDownWhenOldConsoleHasNoDomainsDoor(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + |
| + const ( |
| + app = "sitely" |
| + host = "sitely.example.test" |
| + channel = "canary/site" |
| + ) |
| + fc := newDeployFakeConsole(t, "sha0001") |
| + fc.setDomainsListStatus(http.StatusNotFound) // old console: the route itself is unregistered |
| + credFile, err := loadCredentials() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + credFile.put(credential{Console: fc.ts.URL, Address: "paul@keymail.dev", Token: "cli-token", |
| + ExpiresAt: time.Now().Add(time.Hour)}) |
| + if err := credFile.save(); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + edgeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("X-Carlos-Version", "sha0001") |
| + })) |
| + defer edgeSrv.Close() |
| + origOrigin := watchOrigin |
| + watchOrigin = func(h string) string { return edgeSrv.URL } |
| + defer func() { watchOrigin = origOrigin }() |
| + |
| + siteDir := t.TempDir() |
| + if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("v1"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + buf := captureStdout(t) |
| + runDeploy([]string{"-app", app, "-account", "ops", "-kind", "static", "-version", "sha0001", |
| + "-channel", channel, "-host", host, "-timeout", "5s", siteDir}) |
| + out := buf.String() |
| + |
| + if !strings.Contains(out, "no domains door") || !strings.Contains(out, host) { |
| + t.Fatalf("stdout missing the no-domains-door note naming %s:\n%s", host, out) |
| + } |
| + if idx := fc.callIndex("POST /api/cli/apps/ops/sitely/domains/" + host); idx >= 0 { |
| + t.Fatalf("no domains door — attach must never have been attempted: %v", fc.calls) |
| + } |
| + if idx := fc.callIndex("POST /api/cli/apps/ops/sitely/releases"); idx < 0 { |
| + t.Fatalf("deploy must still proceed to ship: %v", fc.calls) |
| + } |
| + if !strings.Contains(out, "live https://"+host) { |
| + t.Fatalf("deploy must still complete and confirm the URL:\n%s", out) |
| + } |
| +} |
| + |
| +// TestDeployAutoAttachDegradesOnDomainsListServerError pins review fix #3: |
| +// a transport-level or 5xx domainList failure must warn and continue, not |
| +// abort a deploy that would otherwise succeed — the door exists but is |
| +// unwell, which is not the same problem as an unauthorized credential. |
| +func TestDeployAutoAttachDegradesOnDomainsListServerError(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + |
| + const ( |
| + app = "sitely" |
| + host = "sitely.example.test" |
| + channel = "canary/site" |
| + ) |
| + fc := newDeployFakeConsole(t, "sha0001") |
| + fc.setDomainsListStatus(http.StatusInternalServerError) |
| + credFile, err := loadCredentials() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + credFile.put(credential{Console: fc.ts.URL, Address: "paul@keymail.dev", Token: "cli-token", |
| + ExpiresAt: time.Now().Add(time.Hour)}) |
| + if err := credFile.save(); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + edgeSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("X-Carlos-Version", "sha0001") |
| + })) |
| + defer edgeSrv.Close() |
| + origOrigin := watchOrigin |
| + watchOrigin = func(h string) string { return edgeSrv.URL } |
| + defer func() { watchOrigin = origOrigin }() |
| + |
| + siteDir := t.TempDir() |
| + if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("v1"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + buf := captureStdout(t) |
| + runDeploy([]string{"-app", app, "-account", "ops", "-kind", "static", "-version", "sha0001", |
| + "-channel", channel, "-host", host, "-timeout", "5s", siteDir}) |
| + out := buf.String() |
| + |
| + if !strings.Contains(out, "could not check custom domains") || !strings.Contains(out, host) { |
| + t.Fatalf("stdout missing the degrade warning naming %s:\n%s", host, out) |
| + } |
| + if idx := fc.callIndex("POST /api/cli/apps/ops/sitely/domains/" + host); idx >= 0 { |
| + t.Fatalf("a domainList 500 must not be treated as an unclaimed host — attach must never be attempted: %v", fc.calls) |
| + } |
| + if idx := fc.callIndex("POST /api/cli/apps/ops/sitely/releases"); idx < 0 { |
| + t.Fatalf("deploy must still proceed to ship despite the domainList 500: %v", fc.calls) |
| + } |
| + if !strings.Contains(out, "live https://"+host) { |
| + t.Fatalf("deploy must still complete and confirm the URL:\n%s", out) |
| + } |
| +} |
| + |
| +// TestDeployAttachRefusalStopsBeforeShip pins the no-partial-ship |
| +// invariant: when the console refuses the attach POST (422 — a policy |
| +// refusal, e.g. an unshaped host), the deploy must exit nonzero WITHOUT |
| +// ever reaching ship — a half-done deploy (bytes shipped, but the route |
| +// never attached) would be worse than stopping before anything is written. |
| +// Run as a subprocess: log.Fatal exits the process. |
| +func TestDeployAttachRefusalStopsBeforeShip(t *testing.T) { |
| + bin := buildCarlos(t) |
| + const ( |
| + app = "sitely" |
| + host = "sitely.example.test" |
| + channel = "canary/site" |
| + version = "sha0001" |
| + ) |
| + fc := newDeployFakeConsole(t, version) |
| + fc.setAttachRefusal(http.StatusUnprocessableEntity, "not a usable hostname") |
| + |
| + credPath := filepath.Join(t.TempDir(), "credentials") |
| + t.Setenv("CARLOS_CREDENTIALS", credPath) |
| + credFile, err := loadCredentials() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + credFile.put(credential{Console: fc.ts.URL, Address: "paul@keymail.dev", Token: "cli-token", |
| + ExpiresAt: time.Now().Add(time.Hour)}) |
| + if err := credFile.save(); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + siteDir := t.TempDir() |
| + if err := os.WriteFile(filepath.Join(siteDir, "index.html"), []byte("v1"), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + cmd := exec.Command(bin, "deploy", "-app", app, "-account", "ops", "-kind", "static", |
| + "-version", version, "-channel", channel, "-host", host, "-console", fc.ts.URL, |
| + "-timeout", "5s", siteDir) |
| + cmd.Env = carlosTestEnv(t, "CARLOS_CREDENTIALS="+credPath, "CARLOS_DEPLOYMENT_BUCKET=", "CARLOS_DEPLOYMENT_DIR=") |
| + cmd.Dir = t.TempDir() |
| + out, err := cmd.CombinedOutput() |
| + if err == nil { |
| + t.Fatalf("deploy with a refused attach exited 0:\n%s", out) |
| + } |
| + if !strings.Contains(string(out), "not a usable hostname") { |
| + t.Fatalf("exit text missing the console's refusal verbatim:\n%s", out) |
| + } |
| + if idx := fc.callIndex("POST /api/cli/apps/ops/sitely/releases"); idx >= 0 { |
| + t.Fatalf("ship must never happen after a refused attach: %v", fc.calls) |
| + } |
| +} |
amadan