amadan

carlos / platform Public

canary: refuse an app whose name ends in a dash; assert the re-run

App names may end in a dash (controlname.ValidApp) and a hostname label
may not, so app "jam-" made "probe-jam-" for every canary name and failed
at the create door with an "invalid host" no other --name could fix. Such
an app is now refused up front, before any write, saying why. Refused
rather than trimmed: trimming would map "jam-" onto the label app "jam"
already uses for the same canary name, a silent shared host.

An exhaustive sweep over a small alphabet pins the mapping for every
accepted app name: the host is either valid under the create door's own
predicate or refused. Leading dashes cannot occur (app names start with a
letter, canary names are validated).

The e2e re-run now asserts its output (reuses the host, promotes) instead
of ignoring the command error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 13d68c3321db8823c019f644c0e77a1df138ae9e parent 5e148d0
3 files changed, +120 −3
  • canary.go +30 −2
  • canary_create_e2e_test.go +31 −1
  • canary_test.go +59 −0

CI cancelled — superseded by a newer push — run details

diff --git a/canary.go b/canary.go
index fb3df980b..0ad379f0f 100644
--- a/canary.go
+++ b/canary.go
@@ -237,6 +237,34 @@ 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
// exit on failure: outside a git repository a canary is still perfectly
// deployable with an explicit --name, and canaryNameFrom gives a better
@@ -506,8 +534,8 @@ func runCanaryDeploy(args []string) {
// The name and the app share ONE hostname label (canaryHost), so this is
// a refusal before the first read, let alone a write.
- if !canaryLabelFits(canary, app2) {
- log.Fatalf("canary: %s", canaryLabelTooLong(canary, app2))
+ if why := canaryHostRefusal(canary, app2); why != "" {
+ log.Fatalf("canary: %s", why)
}
// --region is checked with reads only, before the first write (the
diff --git a/canary_create_e2e_test.go b/canary_create_e2e_test.go
index 0b1d222df..f79d5109d 100644
--- a/canary_create_e2e_test.go
+++ b/canary_create_e2e_test.go
@@ -9,6 +9,7 @@ import (
"testing"
"time"
+ "github.com/carlosframework/platform/internal/console"
"github.com/carlosframework/platform/internal/provision"
"github.com/carlosframework/platform/internal/release"
)
@@ -55,10 +56,16 @@ func TestCanaryCreateMakesAOneLabelHost(t *testing.T) {
}
// A second run finds it by that host: nothing to create.
- out, _ = e.run(t, append(args, artifact(t))...)
+ // 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)
@@ -132,3 +139,26 @@ func TestCanaryDoesNotTakeOverAnInstanceAtItsHost(t *testing.T) {
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_test.go b/canary_test.go
index 2fd6f55c2..51f8823e7 100644
--- a/canary_test.go
+++ b/canary_test.go
@@ -6,6 +6,7 @@ import (
"testing"
"time"
+ "github.com/carlosframework/platform/internal/controlname"
"github.com/carlosframework/platform/internal/provision"
"github.com/carlosframework/platform/internal/release"
)
@@ -328,3 +329,61 @@ func TestCanaryHostChannelReadsWhatThePlanDecidedFrom(t *testing.T) {
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)
+ }
+}