fix/canary-one-label
Squashed from fix/canary-one-label into main.
9 files changed,
+736
−32
canary.go+114 −7canary_create_e2e_test.go+164 −0canary_region_e2e_test.go+2 −2canary_region_test.go+3 −3canary_test.go+139 −16docs/site/cli.md+4 −4internal/console/apex.go+85 −0internal/console/apiinstances.go+38 −0internal/console/apiinstancescanary_test.go+187 −0
CI passed — run details
diff --git a/canary.go b/canary.go| index 503f25563..0ad379f0f 100644 |
| --- a/canary.go |
| +++ b/canary.go |
| @@ -46,7 +46,7 @@ channel, without touching what the app's other instances serve. The bare form |
| is deploy. |
| --name defaults to the current git branch, slugified. The canary's host is |
| -<name>.<app>.<your account's platform domain>, and its channel is |
| +<name>-<app>.<your account's platform domain>, and its channel is |
| canary/<name>. |
| --environment binds the canary to one of the app's named config bundles. |
| @@ -202,13 +202,67 @@ func canaryNameOf(channel string) string { |
| return strings.TrimPrefix(channel, prefix) |
| } |
| -// canaryHost builds README's documented scheme, |
| -// <canary>.<app>.<account-sqid>.<apps-domain>. domain is the instance |
| -// manifest's own Domain — already "<sqid>.<apps-domain>", the same value |
| -// deploy's mintPlatformHost builds "<app>.<domain>" from — so this never |
| -// composes an apps domain by hand. |
| +// canaryHost is <canary>-<app>.<account-sqid>.<apps-domain>: ONE label before |
| +// the domain. domain is the instance manifest's own Domain — already |
| +// "<sqid>.<apps-domain>", the same value deploy's mintPlatformHost builds |
| +// "<app>.<domain>" from — so this never composes an apps domain by hand. |
| +// |
| +// One label because the instance create door says so |
| +// (provision.ValidHost): the host becomes a bucket key segment and, on the |
| +// box, the <host>.sock and <host>.db path segments, and that rail is a |
| +// tenancy boundary this verb does not get to widen. The first version of |
| +// this joined name, app and domain with dots, which the door refused for |
| +// every create from #384 until discussion 207. The domains pass still mints |
| +// its own <canary>.<app>.<sqid>.<apps-domain> alias for a STATIC app's |
| +// canary pointer (internal/domains); that is a route it writes itself, not an |
| +// instance record through the door, and is a different host. |
| +// |
| +// The dash is not a separator anything parses back. ls and rm find a canary |
| +// by its channel (canaryNameOf), never by taking its host apart, so |
| +// "foo-bar" meaning canary foo of app bar or canary "foo-bar" of an app |
| +// of its own is not a question this code ever asks. What the join can do is |
| +// land two different (canary, app) pairs, or a canary and another app's |
| +// apex, on one label. The create door refuses those (apex.go: a label naming |
| +// another claimed app, and hostHolder for a host another app already |
| +// holds), and canaryRecordPlan refuses a host that already carries one of |
| +// this app's own non-canary instances. |
| +// |
| +// The label has to fit 63 bytes with the dash: see canaryLabelTooLong. |
| func canaryHost(name, app, domain string) string { |
| - return name + "." + app + "." + domain |
| + return name + "-" + app + "." + domain |
| +} |
| + |
| +// canaryLabelFits reports whether name and app share one hostname label. |
| +func canaryLabelFits(name, app string) bool { |
| + return len(name)+1+len(app) <= 63 |
| +} |
| + |
| +// canaryHostRefusal is "" when canaryHost(name, app, ...) is a valid single |
| +// label, and otherwise the sentence saying why not. name is already a valid |
| +// canary name (validCanaryName); app is any name the platform accepts |
| +// (controlname.ValidApp), and only the length and a trailing dash can still |
| +// make the joined label invalid. Every accepted app name therefore maps one |
| +// of two ways, never to a host the create door will refuse for a reason |
| +// choosing another --name cannot fix. |
| +func canaryHostRefusal(name, app string) string { |
| + // Refused, not repaired. Trimming the dash would map app "jam-" onto the |
| + // label app "jam" already uses for the same canary name, and a mapping |
| + // that quietly shares a host between two apps is the failure this whole |
| + // fix exists to keep out. A leading dash cannot occur: an app name starts |
| + // with a letter and the canary name is validated. |
| + if strings.HasSuffix(app, "-") { |
| + return canaryAppEndsInDash(app) |
| + } |
| + if !canaryLabelFits(name, app) { |
| + return canaryLabelTooLong(name, app) |
| + } |
| + return "" |
| +} |
| + |
| +// canaryAppEndsInDash refuses an app whose name ends in a dash: app names |
| +// may, a hostname label may not, so no canary name can make a host for it. |
| +func canaryAppEndsInDash(app string) string { |
| + return fmt.Sprintf("the app name %s ends in a dash, and a hostname label cannot, so no canary host can be built for it. Rename the app to use canaries.", app) |
| } |
| // gitBranch is the branch --name defaults to. Unlike gitShortHead it does not |
| @@ -233,6 +287,7 @@ const ( |
| canaryPlanCreate canaryPlan = iota // no record on the canary host yet |
| canaryPlanRechannel // the host exists, following something else |
| canaryPlanReady // already correct — write nothing |
| + canaryPlanTaken // the host holds something that is not a canary |
| ) |
| func (p canaryPlan) String() string { |
| @@ -241,6 +296,8 @@ func (p canaryPlan) String() string { |
| return "create" |
| case canaryPlanRechannel: |
| return "rechannel" |
| + case canaryPlanTaken: |
| + return "taken" |
| default: |
| return "ready" |
| } |
| @@ -330,11 +387,36 @@ func canaryRecordPlan(recs []apiInstanceRecord, host, channel string) canaryPlan |
| if r.Channel == channel { |
| return canaryPlanReady |
| } |
| + // A canary host is one label now, so it can land on an ordinary |
| + // instance of this very app ("probe-titogo" for canary probe, and a |
| + // customer instance of that name). Rechannelling is for moving a |
| + // canary between canary/* channels; it must never carry somebody's |
| + // instance onto one. |
| + if canaryNameOf(r.Channel) == "" { |
| + return canaryPlanTaken |
| + } |
| return canaryPlanRechannel |
| } |
| return canaryPlanCreate |
| } |
| +// canaryHostChannel is the channel of the record sitting on host, from the |
| +// same source canaryPlanFor decided from, or "" when there is none. |
| +func canaryHostChannel(pre canaryPreflight, recs []apiInstanceRecord, host string) string { |
| + if pre.Host != "" { |
| + if pre.Found { |
| + return pre.Rec.Channel |
| + } |
| + return "" |
| + } |
| + for _, r := range recs { |
| + if r.Host == host { |
| + return r.Channel |
| + } |
| + } |
| + return "" |
| +} |
| + |
| // canaryRecords filters an app's instance records down to its canaries. The |
| // prefix test — not a bare "canary" — is the same boundary |
| // internal/host/agent.go's isCanaryRoute draws, and for the same reason: a |
| @@ -450,6 +532,12 @@ func runCanaryDeploy(args []string) { |
| channel := canaryChannel(canary) |
| + // The name and the app share ONE hostname label (canaryHost), so this is |
| + // a refusal before the first read, let alone a write. |
| + if why := canaryHostRefusal(canary, app2); why != "" { |
| + log.Fatalf("canary: %s", why) |
| + } |
| + |
| // --region is checked with reads only, before the first write (the |
| // instancesEnable PUT below): nothing changes when it is refused. |
| var pre canaryPreflight |
| @@ -497,6 +585,8 @@ func runCanaryDeploy(args []string) { |
| // direct read of the canary's own record (codex plan-R5 1). |
| plan := canaryPlanFor(pre, recs, host, channel) |
| switch plan { |
| + case canaryPlanTaken: |
| + log.Fatalf("canary: %s", canaryHostTaken(host, canaryHostChannel(pre, recs, host))) |
| case canaryPlanCreate: |
| rec, cerr := sess.instanceCreate(ctx, acct, app2, host, strings.TrimSpace(*regionFlag), channel, recKind, env, nil, nil, nil) |
| if cerr != nil { |
| @@ -680,3 +770,20 @@ func runCanaryRemove(args []string) { |
| fmt.Fprintf(stdout, "%s %s\n", colorFor(stdout).ok("removed"), rec.Host) |
| mutedf(stdout, "%s itself remains and lapses at the end of its lease — nothing follows it now\n", channel) |
| } |
| + |
| +// ---- refusals ------------------------------------------------------------- |
| + |
| +// canaryLabelTooLong refuses a canary whose name and app do not fit one |
| +// hostname label together. The host is one label, <canary>-<app>, because the |
| +// create door takes exactly one label before the account's domain; a DNS |
| +// label holds 63 bytes, and the dash is one of them. |
| +func canaryLabelTooLong(name, app string) string { |
| + return fmt.Sprintf("the host label %s-%s would be %d characters, and a hostname label holds at most 63. Use a shorter --name.", |
| + name, app, len(name)+1+len(app)) |
| +} |
| + |
| +// canaryHostTaken refuses a canary whose host already carries something |
| +// that is not a canary. channel is what that record follows. |
| +func canaryHostTaken(host, channel string) string { |
| + return fmt.Sprintf("%s already exists and follows %s, which is not a canary channel. Choose another --name.", host, channel) |
| +} |
diff --git a/canary_create_e2e_test.go b/canary_create_e2e_test.go| new file mode 100644 |
| index 000000000..f79d5109d |
| --- /dev/null |
| +++ b/canary_create_e2e_test.go |
| @@ -0,0 +1,164 @@ |
| +// canary_create_e2e_test.go |
| +package main |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "os" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/console" |
| + "github.com/carlosframework/platform/internal/provision" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// Discussion 207: `carlos canary` has not been able to CREATE a canary since |
| +// #384, because canaryHost built <canary>.<app>.<domain> and the create door |
| +// takes one label before the domain. These drive the real CLI against a real |
| +// console, which is what the canary tests until now never did: every one of |
| +// them either refused before the create or found the record already there. |
| +// |
| +// The run itself ends in a watch timeout, since nothing serves the host in |
| +// a test; what is asserted is the record the create wrote. |
| +func TestCanaryCreateMakesAOneLabelHost(t *testing.T) { |
| + for _, region := range []string{"", "ie"} { |
| + name := "no region flag" |
| + if region != "" { |
| + name = "region flag" |
| + } |
| + t.Run(name, func(t *testing.T) { |
| + e := regionE2EConsole(t, "") |
| + ctx := context.Background() |
| + const host = "probe-titogo.bab.oncarlos.test" |
| + args := []string{"canary", "deploy", "--app", "titogo", "--name", "probe", |
| + "--console", e.url, "--version", "abc1234", "--timeout", "1s"} |
| + if region != "" { |
| + args = append(args, "--region", region) |
| + } |
| + out, _ := e.run(t, append(args, artifact(t))...) |
| + if strings.Contains(out, "invalid host") { |
| + t.Fatalf("the create door refused the canary's host:\n%s", out) |
| + } |
| + if n := e.creates.Load(); n != 1 { |
| + t.Fatalf("%d create request(s), want 1:\n%s", n, out) |
| + } |
| + rec, err := provision.GetRecord(ctx, e.store, release.OpsAccount, "titogo", host) |
| + if err != nil || rec.Channel != "canary/probe" { |
| + t.Fatalf("record at %s = %+v %v, want it on canary/probe\n%s", host, rec, err, out) |
| + } |
| + if _, err := provision.GetRecord(ctx, e.store, release.OpsAccount, "titogo", "probe.titogo.bab.oncarlos.test"); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("a two-label record exists: %v", err) |
| + } |
| + if !strings.Contains(out, "canary live at") && !strings.Contains(out, host) { |
| + t.Fatalf("the output never names the canary host %s:\n%s", host, out) |
| + } |
| + |
| + // A second run finds it by that host: nothing to create. |
| + // The re-run fails only on the watch, like the first: it reaches |
| + // the end, finds the record, and says so, instead of dying early |
| + // before a create it was never going to make. |
| + out, err = e.run(t, append(args, artifact(t))...) |
| + if n := e.creates.Load(); n != 1 { |
| + t.Fatalf("the re-run created again (%d creates):\n%s", n, out) |
| + } |
| + if !strings.Contains(out, "canary host "+host+" already follows canary/probe") || !strings.Contains(out, "promoted abc1234 to canary/probe") { |
| + t.Fatalf("the re-run did not reuse the host and promote (err=%v):\n%s", err, out) |
| + } |
| + |
| + // ls reads it back, and rm removes it by name. |
| + out, err = e.run(t, "canary", "ls", "--app", "titogo", "--console", e.url) |
| + if err != nil || !strings.Contains(out, host) || !strings.Contains(out, "canary/probe") { |
| + t.Fatalf("canary ls = %v\n%s, want the one-label host", err, out) |
| + } |
| + out, err = e.run(t, "canary", "rm", "--app", "titogo", "--name", "probe", "--console", e.url) |
| + if err != nil || !strings.Contains(out, "removed "+host) { |
| + t.Fatalf("canary rm = %v\n%s, want it to remove %s", err, out, host) |
| + } |
| + if _, err := provision.GetRecord(ctx, e.store, release.OpsAccount, "titogo", host); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("the record survived rm: %v", err) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The canary name and the app share one 63-byte label now. A name that fits |
| +// on its own but not beside the app is refused before any write, with the |
| +// numbers in it. |
| +func TestCanaryTooLongForOneLabelRefusesBeforeAnyWrite(t *testing.T) { |
| + e := regionE2EConsole(t, "") |
| + long := strings.Repeat("a", 58) // 58 + "-" + "titogo" = 65 |
| + want := "canary: " + canaryLabelTooLong(long, "titogo") |
| + for run := 0; run < 2; run++ { |
| + out, err := e.run(t, "canary", "deploy", "--app", "titogo", "--name", long, |
| + "--console", e.url, "--version", "abc1234", "--timeout", "1s", artifact(t)) |
| + if err == nil || !hasRefusalLine(out, want) { |
| + t.Fatalf("run %d: err=%v out=%s, want the whole line %q", run, err, out, want) |
| + } |
| + } |
| + if e.writes.Load() != 0 || e.releases(t) != 0 { |
| + t.Fatalf("writes %d, releases %d; want nothing written", e.writes.Load(), e.releases(t)) |
| + } |
| + |
| + // The limit itself fits: 56 + 1 + 6 = 63. |
| + fits := strings.Repeat("a", 56) |
| + out, _ := e.run(t, "canary", "deploy", "--app", "titogo", "--name", fits, |
| + "--console", e.url, "--version", "abc1234", "--timeout", "1s", artifact(t)) |
| + if _, err := provision.GetRecord(context.Background(), e.store, release.OpsAccount, "titogo", fits+"-titogo.bab.oncarlos.test"); err != nil { |
| + t.Fatalf("a 63-byte canary label was not created: %v\n%s", err, out) |
| + } |
| +} |
| + |
| +// Flattening puts a canary where an ordinary instance of the same app can |
| +// already be: app "titogo" with a customer instance "probe-titogo". Before |
| +// the flattening a canary host was always one label deeper than any |
| +// instance, so reusing "the record on this host" could only ever reuse a |
| +// canary. Now it could hand a customer's instance to the canary channel, so |
| +// a record that is not on a canary/* channel is refused and left as it is. |
| +func TestCanaryDoesNotTakeOverAnInstanceAtItsHost(t *testing.T) { |
| + e := regionE2EConsole(t, "") |
| + ctx := context.Background() |
| + const host = "probe-titogo.bab.oncarlos.test" |
| + rec := provision.Record{Schema: 1, Host: host, App: "titogo", Account: release.OpsAccount, |
| + Channel: "stable", CreatedAt: time.Now().UTC()} |
| + if err := provision.CreateRecord(ctx, e.store, rec); err != nil { |
| + t.Fatal(err) |
| + } |
| + want := "canary: " + canaryHostTaken(host, "stable") |
| + out, err := e.run(t, "canary", "deploy", "--app", "titogo", "--name", "probe", |
| + "--console", e.url, "--version", "abc1234", "--timeout", "1s", artifact(t)) |
| + if err == nil || !hasRefusalLine(out, want) { |
| + t.Fatalf("err=%v out=%s, want the whole line %q", err, out, want) |
| + } |
| + got, err := provision.GetRecord(ctx, e.store, release.OpsAccount, "titogo", host) |
| + if err != nil || got.Channel != "stable" { |
| + t.Fatalf("the instance = %+v %v, want it still on stable", got, err) |
| + } |
| + if e.releases(t) != 0 { |
| + t.Fatal("a refused canary shipped a release") |
| + } |
| +} |
| + |
| +// App names may end in a dash and hostname labels may not, so an app called |
| +// "jam-" has no canary host for ANY canary name. It is refused before any |
| +// write, saying why, rather than failing at the create door with an |
| +// "invalid host" no other --name can fix. |
| +func TestCanaryRefusesAnAppWhoseNameEndsInADash(t *testing.T) { |
| + e := regionE2EConsole(t, "") |
| + if _, err := e.cst.CreateAppIn(console.OpsAccountID, "jam-", "paul@keymail.dev", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + e.writes.Store(0) |
| + want := "canary: " + canaryAppEndsInDash("jam-") |
| + for run := 0; run < 2; run++ { |
| + out, err := e.run(t, "canary", "deploy", "--app", "jam-", "--name", "probe", |
| + "--console", e.url, "--version", "abc1234", "--timeout", "1s", artifact(t)) |
| + if err == nil || !hasRefusalLine(out, want) { |
| + t.Fatalf("run %d: err=%v out=%s, want the whole line %q", run, err, out, want) |
| + } |
| + } |
| + if e.writes.Load() != 0 || e.creates.Load() != 0 { |
| + t.Fatalf("writes %d, creates %d; want nothing written", e.writes.Load(), e.creates.Load()) |
| + } |
| +} |
diff --git a/canary_region_e2e_test.go b/canary_region_e2e_test.go| index 45fa37045..0244181b5 100644 |
| --- a/canary_region_e2e_test.go |
| +++ b/canary_region_e2e_test.go |
| @@ -35,7 +35,7 @@ func hasRefusalLine(out, want string) bool { |
| // Two identical runs per case: the same refusal both times, and no write, |
| // no new record, no manifest and no upload, before or after. |
| func TestCanaryRegionRefusesBeforeAnyWrite(t *testing.T) { |
| - const existing = "probe.titogo.bab.oncarlos.test" |
| + const existing = "probe-titogo.bab.oncarlos.test" |
| cases := []struct { |
| name, mode, region string |
| fleet, canary bool |
| @@ -119,7 +119,7 @@ func TestCanaryRegionRefusesBeforeAnyWrite(t *testing.T) { |
| // On its own channel it is left as it is; on another it is rechannelled. |
| // Either way there is no create request and the run goes on to ship. |
| func TestCanaryRegionReusesACanaryPastTheListCap(t *testing.T) { |
| - const existing = "probe.titogo.bab.oncarlos.test" |
| + const existing = "probe-titogo.bab.oncarlos.test" |
| for _, c := range []struct{ name, channel, env string }{ |
| {"same channel", "canary/probe", ""}, |
| {"different channel", "canary/old", ""}, |
diff --git a/canary_region_test.go b/canary_region_test.go| index c286fdc76..cd9a05e93 100644 |
| --- a/canary_region_test.go |
| +++ b/canary_region_test.go |
| @@ -38,10 +38,10 @@ func TestCanaryRegionCheck(t *testing.T) { |
| regionUnknownFleet := apiRegionsResult{Regions: names, Default: "ie", AppDefault: &apiAppDefault{PlacementKnown: true, Reason: "fleet", InstanceDomain: "go.tito.io"}} |
| regionUnknown := apiRegionsResult{Regions: names, Default: "ie", AppDefault: &apiAppDefault{PlacementKnown: true, InstanceDomain: "go.tito.io"}} |
| boom := errors.New("timeout") |
| - const host = "c.blog.go.tito.io" |
| + const host = "c-blog.go.tito.io" |
| existingUK := apiInstanceRecord{Host: host, Region: "uk"} |
| existingHome := apiInstanceRecord{Host: host, Region: ""} |
| - other := apiInstanceRecord{Host: "d.blog.go.tito.io", Region: "uk"} |
| + other := apiInstanceRecord{Host: "d-blog.go.tito.io", Region: "uk"} |
| cases := []struct { |
| name string |
| res apiRegionsResult |
| @@ -98,7 +98,7 @@ func TestCanaryRegionCheck(t *testing.T) { |
| // codex plan-R5 1: with --region, the direct read decides the plan, even |
| // when the capped list does not show the record at all. |
| func TestCanaryPlanFor(t *testing.T) { |
| - const host, ch = "c.blog.go.tito.io", "canary/c" |
| + const host, ch = "c-blog.go.tito.io", "canary/c" |
| listed := []apiInstanceRecord{{Host: host, Channel: ch}} |
| cases := []struct { |
| name string |
diff --git a/canary_test.go b/canary_test.go| index d4f3fe574..51f8823e7 100644 |
| --- a/canary_test.go |
| +++ b/canary_test.go |
| @@ -6,6 +6,8 @@ import ( |
| "testing" |
| "time" |
| + "github.com/carlosframework/platform/internal/controlname" |
| + "github.com/carlosframework/platform/internal/provision" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| @@ -113,15 +115,52 @@ func TestCanaryNameWithNothingToGoOnIsRefused(t *testing.T) { |
| } |
| } |
| -// The host is README's documented scheme: <canary>.<app>.<account-sqid>.<apps-domain>. |
| -// The manifest's domain is already "<sqid>.<apps-domain>", the same value |
| -// deploy's mintPlatformHost builds "<app>.<domain>" from. |
| -func TestCanaryHostFollowsTheDocumentedScheme(t *testing.T) { |
| - got := canaryHost("new-queue", "jam", "bab.oncarlos.com") |
| - want := "new-queue.jam.bab.oncarlos.com" |
| +// The host is <canary>-<app>.<account-sqid>.<apps-domain>: one label before |
| +// the domain, because the instance create door takes exactly one (discussion |
| +// 207). The manifest's domain is already "<sqid>.<apps-domain>", the same |
| +// value deploy's mintPlatformHost builds "<app>.<domain>" from. |
| +func TestCanaryHostIsOneLabelBeforeTheDomain(t *testing.T) { |
| + const domain = "bab.oncarlos.com" |
| + got := canaryHost("new-queue", "jam", domain) |
| + want := "new-queue-jam.bab.oncarlos.com" |
| if got != want { |
| t.Errorf("canaryHost = %q, want %q", got, want) |
| } |
| + // The create door's own predicate, not a restatement of it. |
| + if !provision.ValidHost(got, domain) { |
| + t.Errorf("provision.ValidHost(%q, %q) = false: the create door would refuse this canary", got, domain) |
| + } |
| + // The widest label that fits: name + "-" + app = 63. |
| + name := strings.Repeat("n", 63-1-len("jam")) |
| + if h := canaryHost(name, "jam", domain); !canaryLabelFits(name, "jam") || !provision.ValidHost(h, domain) { |
| + t.Errorf("a 63-byte label was refused: fits=%v host=%q", canaryLabelFits(name, "jam"), h) |
| + } |
| +} |
| + |
| +// name + "-" + app is one hostname label: 63 bytes, the dash counted. A name |
| +// validCanaryName accepts on its own can still be too long beside the app. |
| +func TestCanaryLabelFitsOneLabel(t *testing.T) { |
| + for _, tc := range []struct { |
| + name, app string |
| + want bool |
| + }{ |
| + {"new-queue", "jam", true}, |
| + {strings.Repeat("n", 59), "jam", true}, // 59 + 1 + 3 = 63 |
| + {strings.Repeat("n", 60), "jam", false}, // 64 |
| + {strings.Repeat("n", 63), "a", false}, // a valid name, no room for the app |
| + {"a", strings.Repeat("p", 61), true}, // 1 + 1 + 61 = 63 |
| + {"a", strings.Repeat("p", 62), false}, |
| + } { |
| + if got := canaryLabelFits(tc.name, tc.app); got != tc.want { |
| + t.Errorf("canaryLabelFits(%d bytes, %d bytes) = %v, want %v", len(tc.name), len(tc.app), got, tc.want) |
| + } |
| + } |
| + msg := canaryLabelTooLong(strings.Repeat("n", 60), "jam") |
| + for _, want := range []string{"64 characters", "63", "--name"} { |
| + if !strings.Contains(msg, want) { |
| + t.Errorf("refusal %q does not say %q", msg, want) |
| + } |
| + } |
| } |
| func TestCanaryChannelIsNamespaced(t *testing.T) { |
| @@ -134,7 +173,7 @@ func TestCanaryChannelIsNamespaced(t *testing.T) { |
| // no-write case matters — a redeploy of the same canary must not PATCH a |
| // record that is already correct. |
| func TestCanaryRecordPlan(t *testing.T) { |
| - const host = "new-queue.jam.bab.oncarlos.com" |
| + const host = "new-queue-jam.bab.oncarlos.com" |
| const ch = "canary/new-queue" |
| other := apiInstanceRecord{Host: "jam.bab.oncarlos.com", Channel: "stable"} |
| @@ -149,8 +188,13 @@ func TestCanaryRecordPlan(t *testing.T) { |
| []apiInstanceRecord{other, {Host: host, Channel: ch}}, canaryPlanReady}, |
| {"canary host parked on another channel", |
| []apiInstanceRecord{other, {Host: host, Channel: "canary/old"}}, canaryPlanRechannel}, |
| - {"canary host somehow on stable", |
| - []apiInstanceRecord{{Host: host, Channel: "stable"}}, canaryPlanRechannel}, |
| + // One label now: the canary's host can be an ordinary instance of |
| + // the same app. Rechannelling is for canaries; an instance that is |
| + // not on a canary/* channel is refused, whatever it follows. |
| + {"an instance of the app on the canary's host", |
| + []apiInstanceRecord{{Host: host, Channel: "stable"}}, canaryPlanTaken}, |
| + {"a bare canary channel is not a canary/* channel", |
| + []apiInstanceRecord{{Host: host, Channel: "canary"}}, canaryPlanTaken}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| if got := canaryRecordPlan(tc.recs, host, ch); got != tc.want { |
| @@ -188,14 +232,14 @@ func TestCanaryLeaseLabel(t *testing.T) { |
| func TestCanaryRecordForNamePicksExactlyOne(t *testing.T) { |
| recs := []apiInstanceRecord{ |
| {Host: "jam.bab.oncarlos.com", Channel: "stable"}, |
| - {Host: "old.jam.bab.oncarlos.com", Channel: "canary/old"}, |
| - {Host: "new-queue.jam.bab.oncarlos.com", Channel: "canary/new-queue"}, |
| + {Host: "old-jam.bab.oncarlos.com", Channel: "canary/old"}, |
| + {Host: "new-queue-jam.bab.oncarlos.com", Channel: "canary/new-queue"}, |
| } |
| got, ok := canaryRecordForChannel(recs, "canary/new-queue") |
| if !ok { |
| t.Fatal("no record found for canary/new-queue") |
| } |
| - if got.Host != "new-queue.jam.bab.oncarlos.com" { |
| + if got.Host != "new-queue-jam.bab.oncarlos.com" { |
| t.Errorf("host = %q, want the new-queue record", got.Host) |
| } |
| if _, ok := canaryRecordForChannel(recs, "canary/nope"); ok { |
| @@ -207,12 +251,12 @@ func TestCanaryRecordForNamePicksExactlyOne(t *testing.T) { |
| func TestCanaryRecordsFiltersToCanaryChannels(t *testing.T) { |
| recs := []apiInstanceRecord{ |
| {Host: "jam.bab.oncarlos.com", Channel: "stable"}, |
| - {Host: "old.jam.bab.oncarlos.com", Channel: "canary/old"}, |
| - {Host: "eu.jam.bab.oncarlos.com", Channel: "edge"}, |
| - {Host: "new-queue.jam.bab.oncarlos.com", Channel: "canary/new-queue"}, |
| + {Host: "old-jam.bab.oncarlos.com", Channel: "canary/old"}, |
| + {Host: "eu-jam.bab.oncarlos.com", Channel: "edge"}, |
| + {Host: "new-queue-jam.bab.oncarlos.com", Channel: "canary/new-queue"}, |
| // A bare "canary" is NOT a canary/* channel — the same boundary |
| // internal/host/agent.go's isCanaryRoute draws. |
| - {Host: "bare.jam.bab.oncarlos.com", Channel: "canary"}, |
| + {Host: "bare-jam.bab.oncarlos.com", Channel: "canary"}, |
| } |
| got := canaryRecords(recs) |
| if len(got) != 2 { |
| @@ -264,3 +308,82 @@ func TestCanaryTakesTheUpdateHint(t *testing.T) { |
| t.Error("canary is still in noHintVerbs — it was listed there while it was a stub") |
| } |
| } |
| + |
| +// The refusal names the channel the record on the host follows, from the |
| +// same source the plan was decided from: the direct read under --region, the |
| +// list otherwise. |
| +func TestCanaryHostChannelReadsWhatThePlanDecidedFrom(t *testing.T) { |
| + const host = "probe-jam.bab.oncarlos.com" |
| + recs := []apiInstanceRecord{{Host: host, Channel: "stable"}, {Host: "other-jam.bab.oncarlos.com", Channel: "edge"}} |
| + if got := canaryHostChannel(canaryPreflight{}, recs, host); got != "stable" { |
| + t.Errorf("from the list = %q, want stable", got) |
| + } |
| + pre := canaryPreflight{Host: host, Rec: apiInstanceRecord{Host: host, Channel: "beta"}, Found: true} |
| + if got := canaryHostChannel(pre, recs, host); got != "beta" { |
| + t.Errorf("from the direct read = %q, want beta", got) |
| + } |
| + if got := canaryHostChannel(canaryPreflight{Host: host}, recs, host); got != "" { |
| + t.Errorf("direct read found nothing, got %q", got) |
| + } |
| + if msg := canaryHostTaken(host, "stable"); !strings.Contains(msg, host) || !strings.Contains(msg, "stable") { |
| + t.Errorf("refusal %q names neither the host nor its channel", msg) |
| + } |
| +} |
| + |
| +// Every app name the platform accepts has to map to a canary host that is |
| +// either valid or refused up front. App names allow a trailing dash |
| +// (controlname.ValidApp: ^[a-z][a-z0-9-]{0,31}$) and DNS labels do not, so |
| +// app "jam-" would make "probe-jam-" for EVERY canary name, and choosing |
| +// another --name cannot help. Exhaustive over a small alphabet that holds |
| +// every character class and the dash positions: when the refusal is empty |
| +// the host must pass the create door's own predicate. |
| +func TestCanaryHostIsValidOrRefusedForEveryAppName(t *testing.T) { |
| + const domain = "bab.oncarlos.com" |
| + var words func(n int) []string |
| + words = func(n int) []string { |
| + if n == 0 { |
| + return []string{""} |
| + } |
| + var out []string |
| + for _, w := range words(n - 1) { |
| + for _, c := range "a1-" { |
| + out = append(out, w+string(c)) |
| + } |
| + } |
| + return out |
| + } |
| + var names, apps []string |
| + for n := 1; n <= 4; n++ { |
| + for _, w := range words(n) { |
| + if validCanaryName(w) { |
| + names = append(names, w) |
| + } |
| + if controlname.ValidApp(w) { |
| + apps = append(apps, w) |
| + } |
| + } |
| + } |
| + // A real-length app ending in a dash, and the one at the 32-byte limit. |
| + apps = append(apps, "jam-", strings.Repeat("a", 31)+"-", strings.Repeat("a", 32)) |
| + refused, accepted := 0, 0 |
| + for _, app := range apps { |
| + for _, name := range names { |
| + why := canaryHostRefusal(name, app) |
| + ok := provision.ValidHost(canaryHost(name, app, domain), domain) |
| + switch { |
| + case why == "" && !ok: |
| + t.Fatalf("app %q, canary %q: host %q is not valid and nothing refused it", app, name, canaryHost(name, app, domain)) |
| + case why != "" && ok && canaryLabelFits(name, app): |
| + t.Fatalf("app %q, canary %q: refused (%s) though %q is a valid host", app, name, why, canaryHost(name, app, domain)) |
| + } |
| + if why == "" { |
| + accepted++ |
| + } else { |
| + refused++ |
| + } |
| + } |
| + } |
| + if accepted == 0 || refused == 0 { |
| + t.Fatalf("the sweep saw %d accepted and %d refused; it proves nothing", accepted, refused) |
| + } |
| +} |
diff --git a/docs/site/cli.md b/docs/site/cli.md| index c08054481..e458de9ab 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -306,10 +306,10 @@ carlos canary --app jam ./jam |
| The name defaults to the current git branch, slugified; `--name` overrides |
| it. A branch called `new-queue` gives you the channel `canary/new-queue` and |
| -the host `new-queue.jam.<your account>.<apps domain>`. Since the same string |
| +the host `new-queue-jam.<your account>.<apps domain>`. Since the same string |
| has to be both a channel segment and a hostname label, an explicit `--name` |
| is checked rather than quietly repaired — lowercase letters, digits and |
| -dashes. |
| +dashes. The name and the app share one hostname label, so together they can be at most 63 characters. |
| `--region <code>` places a new canary in that region (default: the app's |
| region). It is refused for an existing canary in another region, and for an |
| @@ -330,7 +330,7 @@ requests on the canary host, so the canary serves GETs perfectly and fails |
| every form submission. The verb says so when you leave the flag off. |
| ```sh |
| -carlos env --app jam --environment jam-canary JAM_ORIGIN=https://new-queue.jam.bab.oncarlos.com |
| +carlos env --app jam --environment jam-canary JAM_ORIGIN=https://new-queue-jam.bab.oncarlos.com |
| carlos canary --app jam --environment jam-canary ./jam |
| ``` |
| @@ -624,7 +624,7 @@ and refuses every POST. Write the bundle with `carlos env --environment`, |
| bind it here, then restart the instance to pick the values up. |
| ```sh |
| -carlos instances set-environment --app jam --host new-queue.jam.bab.oncarlos.com --environment jam-canary |
| +carlos instances set-environment --app jam --host new-queue-jam.bab.oncarlos.com --environment jam-canary |
| ``` |
| Without `--region`, a new instance starts in the app's region. An instance in a fleet starts where the fleet runs, and a pooled, static or replica instance starts in the home region. |
diff --git a/internal/console/apex.go b/internal/console/apex.go| index a8a9d3e0a..eb002fdc4 100644 |
| --- a/internal/console/apex.go |
| +++ b/internal/console/apex.go |
| @@ -19,6 +19,7 @@ import ( |
| "os" |
| "strings" |
| + "github.com/carlosframework/platform/internal/domains" |
| "github.com/carlosframework/platform/internal/provision" |
| ) |
| @@ -80,6 +81,29 @@ func (s *Server) apexHolder(ctx context.Context, acct *Account, name string) (*a |
| if host == "" { |
| return nil, nil |
| } |
| + return s.hostHolder(ctx, acct, host, name) |
| +} |
| + |
| +// hostHolder answers whether host is already held by an app of acct other |
| +// than own — nil when it is free, or when the only holder is own itself. |
| +// apexHolder is this question asked of an app's platform apex; the instance |
| +// create door asks it of whatever host a record is about to claim, because |
| +// instance records are keyed per app (provision.RecordKey) and nothing in |
| +// the key stops two apps of one account from both writing the same host. The |
| +// provisioning pass would then keep whichever record it listed first and |
| +// skip the other with a log line (provision/reconcile.go, "duplicate host"), |
| +// so the loser looks created and serves nothing, or serves the other app's |
| +// process under its own name. |
| +// |
| +// A flat canary host (<canary>-<app>.<domain>, discussion 207) is how that |
| +// became easy to reach: canary "a" of app "b-c" and canary "a-b" of app "c" |
| +// are the same label. |
| +// |
| +// Where a holder could be, and the fail-closed rule, are apexHolder's: a |
| +// route at the host, whatever put it there, and an instance record at it |
| +// under any other app of the account, live or trashed. |
| +func (s *Server) hostHolder(ctx context.Context, acct *Account, host, own string) (*apexHold, error) { |
| + name := own |
| if s.cfg.Registry != "" { |
| reg, err := openRegistry(s.cfg.Registry) |
| if err != nil { |
| @@ -137,6 +161,47 @@ func (s *Server) apexHolder(ctx context.Context, acct *Account, name string) (*a |
| return nil, nil |
| } |
| +// hostClaimHolder is the other half of hostHolder: the global hostname claim |
| +// (domains.ClaimKey, one object per host whoever asks) that an attached alias |
| +// is derived from. An alias has no instance record and, until the domains |
| +// pass materializes it, no route either, so hostHolder cannot see it; the |
| +// claim is the authoritative record and one GET. A route minted at a host |
| +// somebody else has claimed would shadow their alias at the edge. |
| +// |
| +// It returns who the claim belongs to when that is not own in acct, nil when |
| +// there is no claim or it is own's. A claim of another account comes back |
| +// with only OtherAccount set and no Holder: domains.ErrHostUnavailable is |
| +// deliberately neutral about who owns a hostname, and this door must not be a |
| +// way round that. Anything but "no such claim" is an error: it fails closed. |
| +func (s *Server) hostClaimHolder(ctx context.Context, acct *Account, host, own string) (*apexHold, error) { |
| + if s.cfg.Releases == nil { |
| + return nil, nil |
| + } |
| + c, err := domains.GetClaim(ctx, s.cfg.Releases, host) |
| + if errors.Is(err, os.ErrNotExist) { |
| + return nil, nil |
| + } |
| + if err != nil { |
| + return nil, &apexErr{bucket: true, err: err} |
| + } |
| + switch { |
| + case c.Account != acct.Sqid: |
| + return &apexHold{OtherAccount: c.Account}, nil |
| + case c.App != own: |
| + return &apexHold{Holder: c.App}, nil |
| + } |
| + return nil, nil |
| +} |
| + |
| +// claimHeldSentence is hostHeldSentence for a hostname claim. A claim of |
| +// another account is refused without saying whose or that it is a claim. |
| +func claimHeldSentence(host string, hold *apexHold) string { |
| + if hold.OtherAccount != "" { |
| + return hostHeldSentence(host, nil) |
| + } |
| + return hostHeldSentence(host, hold) |
| +} |
| + |
| // apexHeldSentence is the refusal, in one voice for every door that gives |
| // it, with the advice that fits who holds the host. |
| func apexHeldSentence(host string, hold *apexHold) string { |
| @@ -157,6 +222,26 @@ func apexHeldSentence(host string, hold *apexHold) string { |
| "; remove that instance first, or choose another name" |
| } |
| +// hostHeldSentence is the instance create door's refusal for a host another |
| +// app of the account already holds (hostHolder), with the advice that fits |
| +// who holds it. It does not claim the host is anyone's apex: that is |
| +// apexHeldSentence's case, and the door's own apex check answers it first. |
| +func hostHeldSentence(host string, hold *apexHold) string { |
| + switch { |
| + case hold == nil: |
| + return host + " is already held" |
| + case hold.OtherAccount != "" && hold.Holder != "": |
| + return host + " is already routed to " + hold.Holder + " in account " + hold.OtherAccount + "; that is an operator repair" |
| + case hold.OtherAccount != "": |
| + return host + " is already routed for account " + hold.OtherAccount + "; that is an operator repair" |
| + case hold.Holder == "": |
| + return host + " already has a route with no app; an operator has to remove it first" |
| + case hold.Trashed: |
| + return host + " is already held by " + hold.Holder + ", which is in the trash; choose another address" |
| + } |
| + return host + " is already held by " + hold.Holder + "; choose another address" |
| +} |
| + |
| // ReportApexSquats logs, once, every route that sits at an app's platform apex |
| // but belongs to a different app — the state the checks above now prevent, |
| // left over from before they existed. It reports and repairs nothing: which |
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index 6b011d2f4..f118f8b4c 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -751,6 +751,44 @@ func (s *Server) handleAPIInstanceCreate(w http.ResponseWriter, r *http.Request) |
| } |
| } |
| } |
| + // The sibling check above names apps; this one names HOSTS. Records are |
| + // keyed per app, so another app of this account may already hold the very |
| + // host this one is asking for without being called by its label: canary |
| + // "a" of app "b-c" and canary "a-b" of app "c" are both a-b-c (the flat |
| + // canary host, discussion 207). The provisioning pass would keep one |
| + // record and skip the other, so the loser would look created and serve |
| + // nothing. Refused here, for every host, on the same fail-closed terms as |
| + // the apex checks; a host this app already holds falls through to the |
| + // create's own "already exists". |
| + // |
| + // KNOWN LIMIT, filed as amadan discussion 232: this is a check, then a |
| + // write, and CreateRecord's conditional write protects only this app's |
| + // own account/app/host key. Two creates for one host from two different |
| + // apps, racing, can both pass and both be written; the provisioning pass |
| + // then keeps one and skips the other ("duplicate host"). Closing that |
| + // needs host-scoped atomic ownership, which is a gap in the instance door |
| + // that predates the flat canary host and is not built here. The |
| + // sequential case, the common one, is closed. |
| + switch hold, err := s.hostHolder(ctx, acct, host, app); { |
| + case err != nil: |
| + log.Printf("console: api instances create %s/%s: host holder check %s: %v", acct.Sqid, app, host, err) |
| + jsonError(w, http.StatusInternalServerError, "could not check the host — try again") |
| + return |
| + case hold != nil: |
| + jsonError(w, http.StatusConflict, hostHeldSentence(host, hold)) |
| + return |
| + } |
| + // An attached alias has no record and may have no route yet, so the |
| + // holder check cannot see it; its authoritative claim is one read. |
| + switch hold, err := s.hostClaimHolder(ctx, acct, host, app); { |
| + case err != nil: |
| + log.Printf("console: api instances create %s/%s: host claim check %s: %v", acct.Sqid, app, host, err) |
| + jsonError(w, http.StatusInternalServerError, "could not check the host — try again") |
| + return |
| + case hold != nil: |
| + jsonError(w, http.StatusConflict, claimHeldSentence(host, hold)) |
| + return |
| + } |
| // A fleet-placed app (customer-fleets plan, `carlos apps place`) governs |
| // every one of its instances' Region uniformly — spec's "one flag on the |
| // app, all its instances follow" (appPlacement, fleets.go) — so when a |
diff --git a/internal/console/apiinstancescanary_test.go b/internal/console/apiinstancescanary_test.go| new file mode 100644 |
| index 000000000..e217562ef |
| --- /dev/null |
| +++ b/internal/console/apiinstancescanary_test.go |
| @@ -0,0 +1,187 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "net/http" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/domains" |
| + "github.com/carlosframework/platform/internal/provision" |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// canaryDoorFixture is a console with the platform apps domain set, and two |
| +// apps in the ops account ("bar" and "foo-bar") with instances enabled on |
| +// the account's platform subdomain, the way `carlos instances enable` pins |
| +// them. It returns the fixture, a member token and the manifest domain. |
| +func canaryDoorFixture(t *testing.T, code string, apps ...string) (*consoleFixture, string, string) { |
| + t.Helper() |
| + fx, member := instanceFixture(t, code) |
| + fx.srv.cfg.AppsDomain = "oncarlos.test" |
| + now := time.Now() |
| + for _, a := range apps { |
| + if _, err := fx.st.CreateAppIn(OpsAccountID, a, "paul@keymail.dev", now); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + for _, a := range append([]string{"titogo"}, apps...) { |
| + if status, _, out := rawReq(t, http.MethodPut, enableURL(fx, "bab", a), member, `{}`); status != http.StatusOK { |
| + t.Fatalf("enable %s = %d %s", a, status, out) |
| + } |
| + } |
| + return fx, member, fx.sqid + ".oncarlos.test" |
| +} |
| + |
| +// The flat canary host is <canary>-<app>.<domain>: one label, so the create |
| +// door that insists on one label accepts it. This is the door half of |
| +// discussion 207: the host `carlos canary` builds must pass ValidHost. |
| +func TestAPIInstanceCreateAcceptsAFlatCanaryHost(t *testing.T) { |
| + fx, member, domain := canaryDoorFixture(t, "code-canary-flat") |
| + status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member, |
| + `{"host":"new-queue-titogo.`+domain+`","channel":"canary/new-queue"}`) |
| + if status != http.StatusCreated { |
| + t.Fatalf("flat canary host create = %d %s, want 201", status, out) |
| + } |
| + // The two-label form stays refused: ValidHost is a tenancy boundary and |
| + // is not loosened for canaries. |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member, |
| + `{"host":"other.titogo.`+domain+`","channel":"canary/other"}`) |
| + if status != http.StatusBadRequest || !strings.Contains(out, "invalid host") { |
| + t.Fatalf("two-label host create = %d %s, want 400 invalid host", status, out) |
| + } |
| +} |
| + |
| +// A canary named "foo" on app "bar" lands on foo-bar.<domain>, which is also |
| +// the platform apex of an app called "foo-bar". Flattening makes that |
| +// reachable, so both orders are pinned: the canary cannot take a live or |
| +// trashed app's apex, and an app cannot be created over a canary that |
| +// already sits on its apex. |
| +func TestFlatCanaryHostCannotTakeAnotherAppsApex(t *testing.T) { |
| + fx, member, domain := canaryDoorFixture(t, "code-canary-apex", "bar", "foo-bar") |
| + |
| + // canary "foo" of app "bar": host foo-bar, app foo-bar's apex. |
| + status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member, |
| + `{"host":"foo-bar.`+domain+`","channel":"canary/foo"}`) |
| + if status != http.StatusConflict || !strings.Contains(out, "foo-bar") || !strings.Contains(out, "platform apex") { |
| + t.Fatalf("canary onto a live app's apex = %d %s, want 409 naming the apex", status, out) |
| + } |
| + if err := fx.st.TrashAppRow(OpsAccountID, "foo-bar", time.Now()); err != nil { |
| + t.Fatal(err) |
| + } |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member, |
| + `{"host":"foo-bar.`+domain+`","channel":"canary/foo"}`) |
| + if status != http.StatusConflict || !strings.Contains(out, "trash") { |
| + t.Fatalf("canary onto a trashed app's apex = %d %s, want 409 naming the trash", status, out) |
| + } |
| + if err := fx.st.RestoreAppRow(OpsAccountID, "foo-bar"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := provision.GetRecord(context.Background(), fx.rel, release.OpsAccount, "bar", "foo-bar."+domain); err == nil { |
| + t.Fatal("a refused canary left a record behind") |
| + } |
| + |
| + // The other order: the canary exists while no app owns the name, and an |
| + // app arrives under it afterwards. |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "bar"), member, |
| + `{"host":"qux-bar.`+domain+`","channel":"canary/qux"}`) |
| + if status != http.StatusCreated { |
| + t.Fatalf("canary on a free label = %d %s, want 201", status, out) |
| + } |
| + if status, out := postJSON(t, fx.ts.URL+"/api/cli/apps/"+fx.sqid+"/new", map[string]string{"name": "qux-bar"}, member); status != http.StatusConflict { |
| + t.Fatalf("creating an app over a canary's host = %d %v, want 409", status, out) |
| + } |
| +} |
| + |
| +// Two canaries of two apps can flatten to the same label: canary "a" of app |
| +// "b-c" and canary "a-b" of app "c" are both a-b-c. Instance records are |
| +// keyed per app, so nothing about the key stops the second one, and the |
| +// provisioning pass would quietly keep whichever record it lists first. The |
| +// door refuses a host another app of the account already holds, whatever |
| +// its channel, and in both directions. |
| +func TestFlatCanaryHostsOfTwoAppsCannotCollide(t *testing.T) { |
| + fx, member, domain := canaryDoorFixture(t, "code-canary-collide", "b-c", "c") |
| + const host = "a-b-c." |
| + |
| + status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "b-c"), member, |
| + `{"host":"`+host+domain+`","channel":"canary/a"}`) |
| + if status != http.StatusCreated { |
| + t.Fatalf("first canary = %d %s, want 201", status, out) |
| + } |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "c"), member, |
| + `{"host":"`+host+domain+`","channel":"canary/a-b"}`) |
| + if status != http.StatusConflict || !strings.Contains(out, "b-c") { |
| + t.Fatalf("colliding canary of another app = %d %s, want 409 naming app b-c", status, out) |
| + } |
| + if _, err := provision.GetRecord(context.Background(), fx.rel, release.OpsAccount, "c", host+domain); err == nil { |
| + t.Fatal("the refused canary left a record under app c") |
| + } |
| + |
| + // An ordinary instance of another app at a host a canary holds is the |
| + // same collision, as is the reverse. |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member, |
| + `{"host":"`+host+domain+`"}`) |
| + if status != http.StatusConflict { |
| + t.Fatalf("ordinary instance on a canary's host = %d %s, want 409", status, out) |
| + } |
| + |
| + // The holder itself is told "already exists", as ever: this is not a |
| + // sibling's host. |
| + status, _, out = rawReq(t, http.MethodPost, instancesURL(fx, "bab", "b-c"), member, |
| + `{"host":"`+host+domain+`","channel":"canary/a"}`) |
| + if status != http.StatusConflict || !strings.Contains(out, "already exists") { |
| + t.Fatalf("re-create by the holder = %d %s, want 409 already exists", status, out) |
| + } |
| +} |
| + |
| +// An attached alias is a hostname claim (domains.ClaimKey) with no instance |
| +// record, and no route until the domains pass derives one, so the holder |
| +// check cannot see it; a route minted at that host would shadow the alias at |
| +// the edge. The create door reads the claim itself: another app's claim, in |
| +// this account or another, refuses the create (the other account's without |
| +// saying whose), this app's own claim does not, and a claim that cannot be |
| +// read fails closed. |
| +func TestInstanceCreateRefusesAHostAnotherAppHasClaimed(t *testing.T) { |
| + fx, member, domain := canaryDoorFixture(t, "code-canary-claim", "other") |
| + ctx := context.Background() |
| + claim := func(host, acct, app string) { |
| + t.Helper() |
| + if _, _, err := domains.AttachClaim(ctx, fx.rel, domains.Claim{Schema: 1, Host: host, Account: acct, App: app, |
| + CreatedAt: time.Now().UTC().Format(time.RFC3339)}); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + create := func(label string) (int, string) { |
| + t.Helper() |
| + status, _, out := rawReq(t, http.MethodPost, instancesURL(fx, "bab", "titogo"), member, |
| + `{"host":"`+label+`.`+domain+`","channel":"canary/`+label+`"}`) |
| + return status, out |
| + } |
| + |
| + claim("probe-titogo."+domain, "bab", "other") |
| + if status, out := create("probe-titogo"); status != http.StatusConflict || !strings.Contains(out, "other") { |
| + t.Fatalf("create over a sibling's claim = %d %s, want 409 naming app other", status, out) |
| + } |
| + |
| + claim("away-titogo."+domain, "zzzz", "other") |
| + status, out := create("away-titogo") |
| + if status != http.StatusConflict || strings.Contains(out, "zzzz") || strings.Contains(out, "other") { |
| + t.Fatalf("create over another account's claim = %d %s, want 409 that names neither", status, out) |
| + } |
| + |
| + claim("mine-titogo."+domain, "bab", "titogo") |
| + if status, out := create("mine-titogo"); status != http.StatusCreated { |
| + t.Fatalf("create over this app's own claim = %d %s, want 201", status, out) |
| + } |
| + |
| + if _, err := fx.rel.Put(ctx, domains.ClaimKey("junk-titogo."+domain), []byte("not json"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if status, out := create("junk-titogo"); status != http.StatusInternalServerError { |
| + t.Fatalf("create with an unreadable claim = %d %s, want 500 (fail closed)", status, out) |
| + } |
| + if _, err := provision.GetRecord(ctx, fx.rel, release.OpsAccount, "titogo", "probe-titogo."+domain); err == nil { |
| + t.Fatal("a refused create left a record behind") |
| + } |
| +} |