amadan
deploy: no-host static redeploy promotes onto the records' own channel
Re-review important #1: staticRecordChannel answered "" for a no-host call, so `carlos deploy -kind static -app X` — the obvious redeploy once a domain is attached — fell to the channels listing: a two-channel app was refused despite the records naming the channel, and a single listed channel DIFFERING from the records' promoted onto the wrong channel while watching hosts that follow another. Now the no-host case resolves the single distinct non-empty channel the records agree on, and only disagreement (or nothing recorded) falls through to the listing and its -channel refusal. The gate test's no-host case now runs with no -channel (the old assertion's explicit flag hid exactly this gap) plus a disagreeing-records case pinning the fallthrough. Minors: staticDeployHosts skips non-static records itself (self-consistent rather than caller-guarded); attachUnclaimedHost's comment states that it runs on redeploys now and why a 401/403 on the GET rightly aborts (same instanceGate the deploy's instanceList already passed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+68
−13
deploy.go+40 −9deploy_test.go+28 −4
diff --git a/deploy.go b/deploy.go| index 03f75ad44..541e76a9a 100644 |
| --- a/deploy.go |
| +++ b/deploy.go |
| @@ -252,21 +252,38 @@ func staticOnlyRecords(recs []apiInstanceRecord) bool { |
| return true |
| } |
| -// staticRecordChannel is the channel of the static record naming host, or "" |
| -// — the record a first-route attach wrote already knows what its domain |
| -// follows, so a redeploy needs no -channel and no channels-listing round |
| -// trip. "" (host absent, or no -host at all) sends the caller to the |
| -// channels listing instead. |
| +// staticRecordChannel resolves a static-path deploy's channel from the |
| +// records themselves — the record a first-route attach wrote already knows |
| +// what its domain follows, so a redeploy needs no -channel and no |
| +// channels-listing round trip. With -host, it is that host's record's |
| +// channel; without, it is the single distinct non-empty channel the records |
| +// agree on — the no-host redeploy is watching THOSE hosts, so their own |
| +// channel is the only one that keeps the promote and the watch pointed at |
| +// the same thing (a channels listing that happened to hold one OTHER |
| +// channel would promote onto it while watching hosts following another). |
| +// "" — host unrecorded, records disagreeing, or nothing recorded — sends |
| +// the caller to the channels listing (and its refusal names -channel). |
| func staticRecordChannel(recs []apiInstanceRecord, host string) string { |
| - if host == "" { |
| + if host != "" { |
| + for _, r := range recs { |
| + if r.Host == host { |
| + return r.Channel |
| + } |
| + } |
| return "" |
| } |
| + channel := "" |
| for _, r := range recs { |
| - if r.Host == host { |
| - return r.Channel |
| + if r.Channel == "" { |
| + continue |
| + } |
| + if channel == "" { |
| + channel = r.Channel |
| + } else if r.Channel != channel { |
| + return "" // disagreeing records: a human picks, via -channel |
| } |
| } |
| - return "" |
| + return channel |
| } |
| // staticDeployHosts resolves the hosts a static-path deploy watches. -host |
| @@ -284,6 +301,12 @@ func staticDeployHosts(hostFlag string, recs []apiInstanceRecord) ([]string, err |
| } |
| var hosts []string |
| for _, r := range recs { |
| + // Static records only, so the helper holds its own invariant |
| + // rather than trusting the caller's gate: an instance record's |
| + // host is adopted and watched via the instance path, never here. |
| + if r.Kind != "static" { |
| + continue |
| + } |
| hosts = append(hosts, r.Host) |
| } |
| if len(hosts) > 0 { |
| @@ -341,6 +364,14 @@ func staticDeployChannel(chans []apiChannel) (string, error) { |
| // but it is still a write request this deploy has no business sending: the |
| // list already proves there is nothing to do, and only a host held by a |
| // DIFFERENT owner answers 409 "hostname unavailable". |
| +// |
| +// Since the kind-aware static gate, this runs on REDEPLOYS too (records |
| +// present), not just the first deploy — so a 401/403 on the GET aborting |
| +// the whole deploy is a question worth answering: the domains door sits |
| +// behind the SAME instanceGate as the instanceList runDeploy has already |
| +// passed, so a token that got this far can list domains too, and a 401/403 |
| +// here means the credential genuinely lost authority mid-deploy — worth |
| +// stopping for, not degrading past. |
| func attachUnclaimedHost(ctx context.Context, sess *apiSession, acct apiAcct, app, host, kind, channel string, recs []apiInstanceRecord) { |
| domains, err := sess.domainList(ctx, acct, app) |
| if err != nil { |
diff --git a/deploy_test.go b/deploy_test.go| index 4172aaabf..500092546 100644 |
| --- a/deploy_test.go |
| +++ b/deploy_test.go |
| @@ -1168,10 +1168,34 @@ func TestDeployTargetsStaticKindAware(t *testing.T) { |
| t.Fatal("second host never consulted the channels listing") |
| } |
| - // No -host at all: the static records themselves are the watch targets. |
| - _, hosts, static, err = deployTargets([]apiInstanceRecord{staticRec}, "static", "canary/site", "", channels) |
| - if err != nil || !static || len(hosts) != 1 || hosts[0] != "site.example.test" { |
| - t.Fatalf("no -host = hosts %v static %v err %v, want the record's host watched", hosts, static, err) |
| + // No -host at all — the obvious redeploy invocation once a domain is |
| + // attached: the records are the watch targets AND their agreed channel |
| + // is the promote target, with no -channel and NO channels round trip. A |
| + // listing that happened to hold one other channel must not win here — |
| + // promoting onto it while watching hosts that follow the record's |
| + // channel would verify the wrong thing (re-review, important #1). |
| + channelsCalled = false |
| + ch, hosts, static, err = deployTargets([]apiInstanceRecord{staticRec}, "static", "", "", channels) |
| + if err != nil || !static || len(hosts) != 1 || hosts[0] != "site.example.test" || ch != "canary/site" { |
| + t.Fatalf("no -host = ch %q hosts %v static %v err %v, want the record's host watched on the record's channel", ch, hosts, static, err) |
| + } |
| + if channelsCalled { |
| + t.Fatal("no -host redeploy consulted the channels listing — the records already agree on a channel") |
| + } |
| + |
| + // Records DISAGREEING on a channel resolve nothing: fall through to the |
| + // channels listing (whose single entry answers, and whose plural case |
| + // refuses naming -channel) rather than guessing between the records. |
| + twoChannels := []apiInstanceRecord{ |
| + {Host: "site.example.test", Channel: "canary/site", Kind: "static"}, |
| + {Host: "blog.example.test", Channel: "canary/blog", Kind: "static"}, |
| + } |
| + ch, hosts, static, err = deployTargets(twoChannels, "static", "", "", channels) |
| + if err != nil || !static || len(hosts) != 2 || ch != "canary/other" { |
| + t.Fatalf("disagreeing records = ch %q hosts %v static %v err %v, want the listing's channel via fallthrough", ch, hosts, static, err) |
| + } |
| + if !channelsCalled { |
| + t.Fatal("disagreeing records never fell through to the channels listing") |
| } |
| // A real instance record still pulls a -kind static deploy onto the |