diff --git a/internal/app/load.go b/internal/app/load.go index 332f3384..bfaacdd3 100644 --- a/internal/app/load.go +++ b/internal/app/load.go @@ -560,7 +560,24 @@ func crossFieldRules(p *Spec) error { // Materialise the durable volume in the project rather than only in the // generated runtime, so the canonical form, the preflight collision // check and the renderer all name the same volume. - if d.dataPath != "" && len(svc.Volumes) == 0 { + // A service declared disposable owns no durable volume, so a volume the + // author named would be carried through the canonical form and the + // preflight collision check while nothing ever created or mounted it. + // Refuse rather than ignore the declaration. + if serviceIsEphemeral(svc) && len(svc.Volumes) > 0 { + return errf("project_invalid", "services."+name+".volumes", "", + "%q declares volumes and persistence.mode: ephemeral; an ephemeral service owns no durable volume, "+ + "so declare durable persistence or remove the volumes", name) + } + // Protection is a contract about recovering durable data. An ephemeral + // service has none: seeding the active volume fails at apply time on a + // volume that was never created, and the sealed identity would name it. + if serviceIsEphemeral(svc) && svc.Protection != nil { + return errf("project_invalid", "services."+name+".protection", "", + "%q declares protection and persistence.mode: ephemeral; there is no durable data to protect, "+ + "so declare durable persistence or remove the protection policy", name) + } + if d.dataPath != "" && len(svc.Volumes) == 0 && !serviceIsEphemeral(svc) { svc.Volumes = []string{"data"} p.Services[name] = svc p.derivedPaths["services."+name+".volumes"] = OriginDefault diff --git a/internal/app/services.go b/internal/app/services.go index 15dc2d70..a860f12a 100644 --- a/internal/app/services.go +++ b/internal/app/services.go @@ -84,6 +84,14 @@ type driver struct { urlUser string // settings maps declared settings onto the driver's own mechanism. settings settingsForm + // persistenceOptions are the server options this driver derives from + // persistence.mode, keyed by mode. They are defaults: an authored setting of + // the same name replaces the value rather than being appended beside it, so + // the rendered command carries one value per option. + // + // A driver with none leaves its server's own defaults alone; the mode still + // decides whether Onebox gives it a durable volume. + persistenceOptions map[string]map[string]string } type settingsForm int @@ -100,6 +108,20 @@ const ( settingsUnsupported ) +// serviceIsEphemeral reports a service the author declared disposable. +// +// persistence.mode is the declaration Onebox can act on, and until now no +// managed service consulted it: every driver rendered a durable volume whatever +// the mode said, so `mode: ephemeral` changed nothing at all. The mode owns +// this decision and a driver setting cannot override it — a server flag has no +// business redefining what Onebox claims about data lifetime. +// +// Omitted persistence stays durable, which is what defaults.go already fills in, +// so existing projects render exactly as before. +func serviceIsEphemeral(s Service) bool { + return s.Persistence != nil && s.Persistence.Mode == "ephemeral" +} + // drivers is the catalogue. A name that is not here is refused by the loader. var drivers = map[string]driver{ "postgres": { @@ -134,18 +156,31 @@ var drivers = map[string]driver{ // rollout converges onto it. SET proves the write path; EX bounds the // key so the probe cannot accumulate one. health: []string{"CMD-SHELL", "redis-cli -a \"$REDIS_PASSWORD\" set ob:health 1 EX 30 | grep -qx OK"}, - command: []string{"sh", "-c", "exec redis-server --requirepass \"$REDIS_PASSWORD\" --appendonly yes"}, + command: []string{"sh", "-c", "exec redis-server --requirepass \"$REDIS_PASSWORD\""}, secretEnv: []string{"REDIS_PASSWORD"}, urlUser: "default", scheme: "redis", settings: settingsRedisFlag, + // Durable keeps the append-only log this driver has always run with. + // Ephemeral turns off both persistence mechanisms: a service declared + // disposable should not pay an fsync per write, nor grow an AOF in a + // container layer nothing intends to read back. + persistenceOptions: map[string]map[string]string{ + "durable": {"appendonly": "yes"}, + "ephemeral": {"appendonly": "no", "save": ""}, + }, }, "valkey": { majorUpgradeInPlace: true, image: "valkey/valkey", port: 6379, dataPath: "/data", // Same failure mode and the same probe as redis; see the note there. health: []string{"CMD-SHELL", "valkey-cli -a \"$REDIS_PASSWORD\" set ob:health 1 EX 30 | grep -qx OK"}, - command: []string{"sh", "-c", "exec valkey-server --requirepass \"$REDIS_PASSWORD\" --appendonly yes"}, + command: []string{"sh", "-c", "exec valkey-server --requirepass \"$REDIS_PASSWORD\""}, secretEnv: []string{"REDIS_PASSWORD"}, urlUser: "default", scheme: "redis", settings: settingsRedisFlag, + // Same contract as redis; see the note there. + persistenceOptions: map[string]map[string]string{ + "durable": {"appendonly": "yes"}, + "ephemeral": {"appendonly": "no", "save": ""}, + }, }, "mongodb": { database: true, @@ -315,7 +350,7 @@ func (p *Spec) renderService(n Names, name string, s Service, selectedImage stri } command := append([]string(nil), d.command...) - if err := applySettings(name, d, s.Settings, env, &command); err != nil { + if err := applySettings(name, d, s, s.Settings, env, &command); err != nil { return nil, err } if len(env) > 0 { @@ -332,7 +367,7 @@ func (p *Spec) renderService(n Names, name string, s Service, selectedImage stri } volumes := map[string]any{} - if d.dataPath != "" { + if d.dataPath != "" && !serviceIsEphemeral(s) { vol := dataVolume(s) full := n.ServiceVolume(name, vol) svc["volumes"] = []string{full + ":" + d.dataPath} @@ -396,10 +431,38 @@ func identityEnv(key string, d driver, app string) map[string]any { // applySettings puts declared settings where the driver actually reads them. // A driver with no safe mechanism refuses rather than accepting values it // would silently ignore. -func applySettings(name string, d driver, settings map[string]any, env map[string]any, command *[]string) error { - if len(settings) == 0 { +func applySettings(name string, d driver, s Service, settings map[string]any, env map[string]any, command *[]string) error { + // The mode's options first, then the authored ones over them. Merging here + // rather than appending is what keeps one value per option in the rendered + // command: appending produced `--appendonly yes --appendonly no` for an + // author who set the flag the driver had already fixed. + effective := map[string]any{} + // Only ephemeral turns persistence off. Every other mode — durable, + // external, and any mode added later — keeps the durable options, because a + // mode this table does not know about must not silently disable a server's + // persistence. `external` is exactly that case: it means the operator covers + // this data, and reading it as "no options" rendered a durable volume with + // the append-only log switched off. + options := d.persistenceOptions["durable"] + if serviceIsEphemeral(s) { + options = d.persistenceOptions["ephemeral"] + } + for k, v := range options { + effective[k] = v + } + for k, v := range settings { + effective[k] = v + } + if len(effective) == 0 { + return nil + } + // A driver with no mechanism refuses only what the author asked for. Mode + // options are Onebox's own and are never declared for such a driver, so + // this cannot refuse a project that set nothing. + if d.settings == settingsUnsupported && len(settings) == 0 { return nil } + settings = effective keys := make([]string, 0, len(settings)) for k := range settings { keys = append(keys, k) diff --git a/internal/app/services_test.go b/internal/app/services_test.go index e1736521..21995ff2 100644 --- a/internal/app/services_test.go +++ b/internal/app/services_test.go @@ -391,3 +391,152 @@ func renderServices(t *testing.T, src string) map[string][]byte { } return r.Services } + +// persistence.mode was declared, validated, defaulted — and then ignored by +// every managed service. `mode: ephemeral` rendered the same durable volume as +// a service that declared nothing, and redis and valkey additionally fsynced +// every write into it through a hardcoded --appendonly yes. +func TestEphemeralServicesOwnNoDurableVolume(t *testing.T) { + for _, svc := range []string{ + "postgres", "mysql", "mariadb", "mongodb", "clickhouse", + "redis", "valkey", "rabbitmq", "meilisearch", "nats", + } { + version := map[string]string{ + "postgres": "17", "mysql": "8.0", "mariadb": "11.4", "mongodb": "8.0", + "clickhouse": "25.3", "redis": "8-alpine", "valkey": "8-alpine", + "rabbitmq": "4", "meilisearch": "1.10", "nats": "2.10", + }[svc] + rendered := renderServices(t, "api_version: onebox.run/v1\napp: sample\n"+ + "environments: {production: {server: root@h}}\nworkloads:\n web: {role: application, image: x:1}\n"+ + "services:\n "+svc+":\n version: \""+version+"\"\n persistence: {mode: ephemeral}\n") + if strings.Contains(string(rendered[svc]), "_"+svc+"_data") { + t.Fatalf("%s declared ephemeral still owns a durable volume:\n%s", svc, rendered[svc]) + } + } +} + +// Omitted persistence stays durable, so a project that never declared it keeps +// the volume and the append-only log it has always had. +func TestDurableRedisKeepsItsVolumeAndAppendOnlyLog(t *testing.T) { + for _, decl := range []string{ + " redis: {version: 8-alpine}\n", + " redis: {version: 8-alpine, persistence: {mode: durable}}\n", + } { + rendered := renderServices(t, "api_version: onebox.run/v1\napp: sample\n"+ + "environments: {production: {server: root@h}}\nworkloads:\n web: {role: application, image: x:1}\nservices:\n"+decl) + doc := string(rendered["redis"]) + if !strings.Contains(doc, "_redis_data") { + t.Fatalf("durable redis lost its volume:\n%s", doc) + } + if !strings.Contains(doc, `--appendonly "yes"`) { + t.Fatalf("durable redis lost its append-only log:\n%s", doc) + } + } +} + +// A disposable cache should not pay an fsync per write, nor grow a log nothing +// intends to read back. +func TestEphemeralRedisFamilyDisablesBothPersistenceMechanisms(t *testing.T) { + for _, svc := range []string{"redis", "valkey"} { + rendered := renderServices(t, "api_version: onebox.run/v1\napp: sample\n"+ + "environments: {production: {server: root@h}}\nworkloads:\n web: {role: application, image: x:1}\n"+ + "services:\n "+svc+": {version: 8-alpine, persistence: {mode: ephemeral}}\n") + doc := string(rendered[svc]) + if !strings.Contains(doc, `--appendonly "no"`) { + t.Fatalf("ephemeral %s still runs the append-only log:\n%s", svc, doc) + } + if !strings.Contains(doc, `--save ""`) { + t.Fatalf("ephemeral %s still snapshots:\n%s", svc, doc) + } + } +} + +// An authored setting replaces the mode's default rather than being appended +// beside it. Appending produced `--appendonly yes --appendonly no`, which is +// what made an author compensate for the driver in the first place. +func TestAuthoredSettingOverridesTheModeDefaultExactlyOnce(t *testing.T) { + rendered := renderServices(t, `api_version: onebox.run/v1 +app: sample +environments: {production: {server: root@h}} +workloads: + web: {role: application, image: x:1} +services: + redis: + version: 8-alpine + persistence: {mode: ephemeral} + settings: {appendonly: "yes"} +`) + doc := string(rendered["redis"]) + if n := strings.Count(doc, "--appendonly"); n != 1 { + t.Fatalf("--appendonly appears %d times, want exactly 1:\n%s", n, doc) + } + if !strings.Contains(doc, `--appendonly "yes"`) { + t.Fatalf("the authored setting did not win:\n%s", doc) + } + // The mode still owns storage: an explicit server flag does not buy a volume. + if strings.Contains(doc, "_redis_data") { + t.Fatalf("a driver setting overrode the declared data lifetime:\n%s", doc) + } +} + +// Only ephemeral turns persistence off. `external` means the operator covers +// this data, and a mode table that did not know the word rendered a durable +// volume with the append-only log silently switched off — a durability +// downgrade on the one mode that says the data matters. +func TestOnlyEphemeralDisablesServerPersistence(t *testing.T) { + for _, mode := range []string{"durable", "external"} { + rendered := renderServices(t, "api_version: onebox.run/v1\napp: sample\n"+ + "environments: {production: {server: root@h}}\nworkloads:\n web: {role: application, image: x:1}\n"+ + "services:\n redis: {version: 8-alpine, persistence: {mode: "+mode+"}}\n") + doc := string(rendered["redis"]) + if !strings.Contains(doc, `--appendonly "yes"`) { + t.Fatalf("mode %q disabled the append-only log:\n%s", mode, doc) + } + if !strings.Contains(doc, "_redis_data") { + t.Fatalf("mode %q lost its volume:\n%s", mode, doc) + } + } + // Every value the grammar permits is covered above or by the ephemeral + // tests, so a new mode cannot be added without this failing. + if len(ePersistence) != 3 { + t.Fatalf("persistence modes changed to %v; extend these tests before shipping", ePersistence) + } +} + +// An ephemeral service owns no durable volume. A volume the author names would +// still reach the canonical form, Spec.All's preflight collision check, and the +// protected-identity record, while nothing ever created or mounted it — the +// declaration would be silently ignored rather than refused. +func TestEphemeralServiceCannotDeclareVolumes(t *testing.T) { + src := `api_version: onebox.run/v1 +app: sample +environments: {production: {server: root@h}} +workloads: + web: {role: application, image: x:1} +services: + redis: {version: 8-alpine, persistence: {mode: ephemeral}, volumes: [cache]} +` + _, err := LoadBytes([]byte(src), "ob.yml") + if err == nil { + t.Fatal("an ephemeral service declaring volumes was accepted") + } + if !strings.Contains(err.Error(), "owns no durable volume") { + t.Fatalf("refusal does not explain itself: %v", err) + } +} + +// Protection is a contract about recovering durable data. With no volume +// rendered, seeding the active volume fails at apply time against one that was +// never created, and the sealed identity names it anyway. +func TestEphemeralServiceCannotDeclareProtection(t *testing.T) { + src := strings.Replace(validProtectionProject, + " postgres:\n version: 17\n protection:", + " postgres:\n version: 17\n persistence: {mode: ephemeral}\n protection:", 1) + _, err := LoadBytes([]byte(src), "ob.yml") + if err == nil { + t.Fatal("an ephemeral service declaring protection was accepted") + } + if !strings.Contains(err.Error(), "no durable data to protect") { + t.Fatalf("refusal does not explain itself: %v", err) + } +} diff --git a/internal/app/testdata/contract-verdicts.json b/internal/app/testdata/contract-verdicts.json index 4838e49e..190001c8 100644 --- a/internal/app/testdata/contract-verdicts.json +++ b/internal/app/testdata/contract-verdicts.json @@ -382,7 +382,7 @@ { "case": "conformance/settings key that is a real driver flag", "loads": true, - "digest": "37ef191260abddc6f674ea8bed3bb82970358f8395df2d5a7f5b9b5142268b47 redis=69fece9ab514b04f" + "digest": "37ef191260abddc6f674ea8bed3bb82970358f8395df2d5a7f5b9b5142268b47 redis=f5e4171f39cd0dcb" }, { "case": "conformance/settings key with a shell metacharacter", @@ -517,12 +517,12 @@ { "case": "corpus/authentik.yml", "loads": true, - "digest": "6d3a2518ab077033b35018bb81aa9702a641a2ad7433468afacb6070083d60a8 postgres=dc4f8448b8b82b4a redis=191161a0c85c6c0d" + "digest": "6d3a2518ab077033b35018bb81aa9702a641a2ad7433468afacb6070083d60a8 postgres=dc4f8448b8b82b4a redis=d2660eeb4faa49fe" }, { "case": "corpus/ext-authentik-managed.yml", "loads": true, - "digest": "f6b0b547d0bc7d55b8309e57f4f928a57e2ce44dd4fecc773599c9785580083f postgres=dc4f8448b8b82b4a redis=191161a0c85c6c0d" + "digest": "f6b0b547d0bc7d55b8309e57f4f928a57e2ce44dd4fecc773599c9785580083f postgres=dc4f8448b8b82b4a redis=d2660eeb4faa49fe" }, { "case": "corpus/ext-authentik.yml", @@ -542,7 +542,7 @@ { "case": "corpus/ext-immich-sourced.yml", "loads": true, - "digest": "b852204b8ab417c0b5ca7c162108f95189829c69f8b7284d58120fe1e17c47d9 postgres=76af15324857fa90 redis=0281e909253b33c5" + "digest": "b852204b8ab417c0b5ca7c162108f95189829c69f8b7284d58120fe1e17c47d9 postgres=76af15324857fa90 redis=c7c8bf8044d5342a" }, { "case": "corpus/ext-immich.yml", @@ -552,7 +552,7 @@ { "case": "corpus/ext-n8n.yml", "loads": true, - "digest": "d9b22f801a91ee7c4f6e9d24811e9454374067466263cc08cbc18cb9aefc8241 postgres=809549d286e2dbdc redis=3cef59e7d2733f0a" + "digest": "d9b22f801a91ee7c4f6e9d24811e9454374067466263cc08cbc18cb9aefc8241 postgres=809549d286e2dbdc redis=86933b446609e6d8" }, { "case": "corpus/ext-paperless.yml", @@ -612,7 +612,7 @@ { "case": "corpus/penpot.yml", "loads": true, - "digest": "10d0d86d56452027d9d317f6d77d0a696ed95eb2595e9cf039db6d187ff338e6 postgres=fc584b1b50db23a6 redis=be4534525f623f76" + "digest": "10d0d86d56452027d9d317f6d77d0a696ed95eb2595e9cf039db6d187ff338e6 postgres=fc584b1b50db23a6 redis=fcccb6a023ae5734" }, { "case": "corpus/pursue.yml",