amadan

carlos / platform Public

cli: carlos add says updated when the route already existed (#69)

Paul Campbell 3b2d53ae1fedb7e96b2042cb62c1809371e001b3 parent e11fb91
2 files changed, +72 −1
  • cli.go +14 −1
  • e2e_test.go +58 −0
diff --git a/cli.go b/cli.go
index e49017e48..3d9ca38a7 100644
--- a/cli.go
+++ b/cli.go
@@ -170,6 +170,15 @@ func runAdd(args []string) {
// The account is stamped on the row: it is what adopt and the edge build
// this app's bucket keys from. Reads still fall back to the legacy
// unqualified keys, so an unmigrated app added here keeps working.
+ //
+ // An upsert over an existing host is a rewrite, not a creation, and the
+ // verb the operator reads back must say which one happened (issue #69):
+ // "added" printed over a rewrite is how vitogo's operator was left
+ // believing a fresh row existed while a running edge kept the old one.
+ existing, err := r.Get(host)
+ if err != nil {
+ log.Fatal(err)
+ }
if err := r.Upsert(registry.Route{
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
@@ -178,7 +187,11 @@ func runAdd(args []string) {
}); err != nil {
log.Fatal(err)
}
- fmt.Printf("%s %s\n", colorFor(os.Stdout).ok("added"), host)
+ if existing != nil {
+ fmt.Printf("%s %s (existing route)\n", colorFor(os.Stdout).ok("updated"), host)
+ } else {
+ fmt.Printf("%s %s\n", colorFor(os.Stdout).ok("added"), host)
+ }
}
func runRemove(args []string) {
diff --git a/e2e_test.go b/e2e_test.go
index bbde67b4d..5071e7b82 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -1437,6 +1437,64 @@ func TestAddAccountIDRefusesAName(t *testing.T) {
}
}
+// carlos add on an existing host is an update and must say so — the vitogo
+// incident (issue #69) started with `added vito.platform.tito.io` printed
+// over what was actually a rewrite of an existing row.
+func TestAddOnExistingRouteSaysUpdated(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ cmd := exec.Command(carlosBin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = tmp
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/first.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("first add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "added vito.example.com") {
+ t.Errorf("first add output = %q, want %q", out, "added vito.example.com")
+ }
+
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/second.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("re-add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "updated vito.example.com") {
+ t.Errorf("re-add output = %q, want it to say updated, not added", out)
+ }
+
+ // And the rewrite must actually land — the row carries the new path.
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get("vito.example.com")
+ if err != nil || rt == nil {
+ t.Fatalf("get route: %v (rt=%v)", err, rt)
+ }
+ if rt.DBPath != "/data/second.db" {
+ t.Errorf("db_path = %q, want /data/second.db", rt.DBPath)
+ }
+}
+
// An app still on the pre-accounts keys can be moved INTO ops. Until `-from
// legacy` existed there was no way to say it: a bare move into ops is refused
// (ops is also the default source, and the verb will not guess a direction),