Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions cmd/ob/backup_evidence.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,48 @@ func addBackupEvidenceCommand(root *cobra.Command, g *globalFlags) {
create.Flags().StringVarP(&outPath, "out", "o", "ob-backup-evidence.json", "receipt output path")
_ = create.MarkFlagRequired("plan")
_ = create.MarkFlagRequired("manifest")
var templatePlan string
template := &cobra.Command{
Use: "template",
Short: "print a facts manifest skeleton for a plan's resources",
Long: "Print a manifest skeleton with the plan's resources already filled in.\n\n" +
"The manifest is the only artifact here a person has to author — the plan,\n" +
"the grant and the receipt are all produced by ob — so authoring it blind\n" +
"meant discovering its shape one refusal at a time. Fill in the digests and\n" +
"timestamps your backup tooling produced, then pass it to `create`.",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, _ []string) error {
plan, err := onebox.LoadDeployPlan(templatePlan)
if err != nil {
return err
}
if plan.MigrationBackup == nil {
return errors.New("this plan carries no migration backup requirement, so no manifest is needed")
}
manifest := backupFactsManifest{SchemaVersion: backupFactsSchemaVersion}
for _, resource := range plan.MigrationBackup.Resources {
manifest.Resources = append(manifest.Resources, onebox.MigrationBackupResourceEvidence{
Resource: resource, BackupID: "REPLACE-with-your-backup-id", CreatedAt: "REPLACE-with-RFC3339-time",
Integrity: onebox.BackupIntegrityEvidence{ArtifactDigest: "sha256:REPLACE", Method: "sha256", ValidatedAt: "REPLACE-with-RFC3339-time"},
RestoreTest: restoreTestSkeleton(plan.MigrationBackup.RequireRestoreTest),
})
}
for _, name := range plan.MigrationBackup.RequiredKeyMaterial {
manifest.KeyMaterial = append(manifest.KeyMaterial, onebox.MigrationBackupKeyMaterialEvidence{
Name: name, BackupID: "REPLACE-with-your-backup-id", CreatedAt: "REPLACE-with-RFC3339-time",
Integrity: onebox.BackupIntegrityEvidence{ArtifactDigest: "sha256:REPLACE", Method: "sha256", ValidatedAt: "REPLACE-with-RFC3339-time"},
Usability: onebox.BackupKeyMaterialUsabilityEvidence{Method: "REPLACE-how-you-proved-the-key-opens-it", ValidatedAt: "REPLACE-with-RFC3339-time", ValidationDigest: "sha256:REPLACE"},
})
}
encoder := json.NewEncoder(cmd.OutOrStdout())
encoder.SetIndent("", " ")
return encoder.Encode(manifest)
},
}
template.Flags().StringVar(&templatePlan, "plan", "", "executable plan containing the backup requirement")
_ = template.MarkFlagRequired("plan")
group.AddCommand(create)
group.AddCommand(template)
root.AddCommand(group)
}

Expand Down Expand Up @@ -77,6 +118,20 @@ func runBackupEvidenceCreate(cmd *cobra.Command, g *globalFlags, planPath, manif
return nil
}

// restoreTestSkeleton matches what the plan will accept. A policy requiring a
// passed restore test refuses a "not_tested" receipt, and `passed` needs method,
// tested_at and validation_digest — so emitting the wrong one reinstates the
// discover-by-refusal loop this command exists to end.
func restoreTestSkeleton(required bool) onebox.BackupRestoreTestEvidence {
if !required {
return onebox.BackupRestoreTestEvidence{State: "not_tested"}
}
return onebox.BackupRestoreTestEvidence{
State: "passed", Method: "REPLACE-how-you-restored-and-checked-it",
TestedAt: "REPLACE-with-RFC3339-time", ValidationDigest: "sha256:REPLACE",
}
}

func loadBackupFactsManifest(path string) (backupFactsManifest, error) {
file, err := os.Open(path)
if err != nil {
Expand Down
25 changes: 25 additions & 0 deletions cmd/ob/backup_template_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package main

import "testing"

// The skeleton must match what the plan will accept.
//
// It hardcoded "not_tested". A policy with require_migration_restore_test then
// refused the templated manifest, and the skeleton carried none of the fields a
// passed test needs — reinstating the discover-by-refusal loop the command
// exists to end.
func TestTheSkeletonMatchesTheRestoreTestPolicy(t *testing.T) {
if got := restoreTestSkeleton(false); got.State != "not_tested" {
t.Errorf("without a restore-test policy: state = %q, want not_tested", got.State)
}
if got := restoreTestSkeleton(false); got.Method != "" || got.TestedAt != "" || got.ValidationDigest != "" {
t.Errorf("not_tested refuses method/tested_at/validation_digest, so the skeleton must omit them: %+v", got)
}
got := restoreTestSkeleton(true)
if got.State != "passed" {
t.Errorf("with a restore-test policy: state = %q, want passed", got.State)
}
if got.Method == "" || got.TestedAt == "" || got.ValidationDigest == "" {
t.Errorf("passed requires method, tested_at and validation_digest; skeleton must offer all three: %+v", got)
}
}
32 changes: 25 additions & 7 deletions cmd/ob/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os/exec"
"path/filepath"
"sort"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -484,14 +485,31 @@ func inspectDoctorProtections(cfg *app.Spec, configPath string, deps doctorDepen
}
sort.Strings(componentNames)
for _, name := range componentNames {
p := cfg.Workloads[name].Persistence
if p == nil || p.Mode != "durable" {
continue
w := cfg.Workloads[name]
if w.HoldsDurableData() {
message := "holds durable data and Onebox takes no backups; copy it off this host yourself"
switch {
case w.Replicas > 1:
// Do not suggest declaring durable here: the loader refuses a
// declared-durable workload with replicas, so following that
// advice would turn a project that loads into one that does not.
message += ". It also asks for " + strconv.Itoa(w.Replicas) +
" replicas, which would all mount the same volume — run one instance, " +
"then declare persistence: {mode: durable} to state what it holds"
case w.Persistence == nil:
message += ". Declare persistence: {mode: durable} to state this, or mode: ephemeral if the volume is not state"
}
report.Checks = append(report.Checks, doctorProtectionCheck{
Status: doctorWarning, Workload: name, Mechanism: "backup", Available: false,
Message: message,
})
}
if w.HasBindMounts() {
report.Checks = append(report.Checks, doctorProtectionCheck{
Status: doctorPass, Workload: name, Mechanism: "bind_mount", Available: true,
Message: "mounts a host path Onebox does not own; its contents are yours to back up",
})
}
report.Checks = append(report.Checks, doctorProtectionCheck{
Status: doctorWarning, Workload: name, Mechanism: "backup", Available: false,
Message: "holds durable data and Onebox takes no backups; copy it off this host yourself",
})
}
for _, name := range cfg.ServiceNames() {
report.Checks = append(report.Checks, doctorProtectionCheck{
Expand Down
42 changes: 42 additions & 0 deletions cmd/ob/remedy_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package main

import (
"os"
"path/filepath"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -45,3 +48,42 @@ func TestEveryLifecycleRemedyNamesARealCommand(t *testing.T) {
}
}
}

// A remedy that names a flag must name one the command has.
//
// The earlier check resolved only the command path, so renaming `ob abort
// --force` to `--break-migration-gate` left four error strings telling an
// operator to run a flag that no longer existed — two of them in HALT-AND-PAGE
// guidance, read at the moment a migration has already run and the release is
// stuck. Found by running the product, not by any test.
func TestEveryFlagNamedInAnErrorStringExistsOnThatCommand(t *testing.T) {
root := newRootCmd()
pattern := regexp.MustCompile("`ob ([a-z][a-z -]*?) (--[a-z-]+)`")
for _, dir := range []string{"../../internal/engine", "../../internal/app", "../../internal/onebox", "."} {
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
for _, e := range entries {
name := e.Name()
if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
}
body, err := os.ReadFile(filepath.Clean(filepath.Join(dir, name)))
if err != nil {
t.Fatal(err)
}
for _, m := range pattern.FindAllStringSubmatch(string(body), -1) {
path, flag := strings.Fields(m[1]), strings.TrimPrefix(m[2], "--")
cmd, _, findErr := root.Find(path)
if findErr != nil || cmd == nil {
t.Errorf("%s/%s: `ob %s` is not a command", dir, name, m[1])
continue
}
if cmd.Flags().Lookup(flag) == nil && cmd.InheritedFlags().Lookup(flag) == nil {
t.Errorf("%s/%s: `ob %s` has no --%s flag, but an error string tells the operator to use it", dir, name, m[1], flag)
}
}
}
}
}
38 changes: 38 additions & 0 deletions docs/onebox.run-v1.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@
"type": "array"
},
"migration_backup_maximum_age": {
"default": "24h",
"description": "Maximum age of backup evidence accepted for a migration. Expects a duration such as 30s, 5m, 1h30m or 14d.",
"examples": [
"24h"
Expand Down Expand Up @@ -1675,6 +1676,43 @@
]
}
},
{
"if": {
"properties": {
"persistence": {
"anyOf": [
{
"properties": {
"mode": {
"const": "durable"
}
},
"required": [
"mode"
]
},
{
"not": {
"required": [
"mode"
]
}
}
]
}
},
"required": [
"persistence"
]
},
"then": {
"properties": {
"replicas": {
"maximum": 1
}
}
}
},
{
"else": {
"not": {
Expand Down
8 changes: 8 additions & 0 deletions internal/app/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ func applyDefaults(p *Spec, raw map[string]any, derived map[string]Origin) {
e.Policy.AllowAgentProposals = true
mark(path + ".policy.allow_agent_proposals")
}
// Enabling the backup requirement made this field mandatory, and its
// absence produced an untyped complaint about an empty duration for a
// field the author had never heard of. A default is the answer the
// evolution rules already allow.
if e.Policy.RequireMigrationBackup && e.Policy.MigrationBackupMaximumAge == "" {
e.Policy.MigrationBackupMaximumAge = "24h"
mark(path + ".policy.migration_backup_maximum_age")
}
p.Environments[name] = e
}

Expand Down
18 changes: 18 additions & 0 deletions internal/app/jsonschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,24 @@ func applyRoleRules(doc map[string]any) {
"if": map[string]any{"required": []any{"port"}},
"then": map[string]any{"required": []any{"domain"}},
},
// A workload that declares durable persistence cannot be replicated:
// every replica would mount the same volume. The loader refuses this,
// and an editor should underline it too rather than leaving the author
// to discover it at plan time.
map[string]any{
"if": map[string]any{
// `persistence: {}` is durable too — mode defaults to it — so the
// rule must fire on an absent mode as well as an explicit one.
"properties": map[string]any{"persistence": map[string]any{
"anyOf": []any{
map[string]any{"properties": map[string]any{"mode": map[string]any{"const": "durable"}}, "required": []any{"mode"}},
map[string]any{"not": map[string]any{"required": []any{"mode"}}},
},
}},
"required": []any{"persistence"},
},
"then": map[string]any{"properties": map[string]any{"replicas": map[string]any{"maximum": 1}}},
},
// A job declares its data effect: the one field whose absence would
// let an unknown migration through the rollback gate.
map[string]any{
Expand Down
3 changes: 3 additions & 0 deletions internal/app/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,9 @@ func crossFieldRules(p *Spec) error {
"workload %q asks for a rolling release but declares no health check, so nothing says when the "+
"newcomer is ready to take traffic. Declare health:, or use strategy: recreate", name)
}
// Keyed on the authored block, not on HoldsDurableData: inferring
// durability must not tighten a refusal against a project that already
// loads. `ob doctor` reports the hazard instead.
if w.Replicas > 1 && w.Persistence != nil && w.Persistence.Mode == "durable" {
return errf("stateful_replicas", path+".replicas", "",
"workload %q keeps durable state and asks for %d replicas; they would all mount the same volume. "+
Expand Down
10 changes: 10 additions & 0 deletions internal/app/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@ func conformanceCases() []conformanceCase {
{"log option with a shell metacharacter", wl("w: {image: nginx, logging: {options: {\"x; touch /tmp/q\": \"1\"}}}"), false},
{"log driver with a space", wl("w: {image: nginx, logging: {driver: \"not a driver\"}}"), false},
{"a plugin log driver", wl("w: {image: nginx, logging: {driver: \"myorg/fluent:1.2\", options: {max-size: 10m}}}"), true},
// The contract publishes persistence.mode defaulting to durable. That
// default was unreachable while the block was absent, so a workload with
// a managed volume read as holding nothing — and doctor, the backup gate
// and the protection gate each guessed the same wrong way.
{"volumes without persistence still load", wl("w: {image: nginx, volumes: [{name: data, path: /data}]}"), true},
{"a bind mount is not durable", wl("w: {image: nginx, volumes: [{source: ./cfg, path: /etc/app}], replicas: 3}"), true},
// Inference must not tighten a refusal against a project that loads.
{"inferred durability does not refuse replicas", wl("w: {image: nginx, volumes: [{name: data, path: /data}], replicas: 3}"), true},
{"declared durability still refuses replicas", wl("w: {image: nginx, volumes: [{name: data, path: /data}], persistence: {mode: durable}, replicas: 3}"), false},
{"persistence block with no mode still refuses replicas", wl("w: {image: nginx, volumes: [{name: data, path: /data}], persistence: {}, replicas: 3}"), false},
{"protection is no longer a field", wl("w: {image: nginx, protection: {backup: {schedule: {cron: \"0 3 * * *\"}}}}"), false},
{"a near-miss field name", wl("w: {image: nginx, replicaz: 3}"), false},
// A closed value set is only closed if a value outside it is refused,
Expand Down
28 changes: 28 additions & 0 deletions internal/app/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,31 @@ services: {postgres: 17}
t.Fatalf("the refusal must name the field: %v", err)
}
}

// A project that loads must resolve.
//
// An earlier attempt at inferring durability materialised `persistence` into the
// document and carried the "this was inferred" exemption as in-memory state.
// deepCopy round-trips through JSON, so Resolve re-ran the cross-field rules
// against a clone that had the block but not the exemption: any project with a
// named volume, replicas and ANY environment override was refused at resolve
// time — blamed on a `replicas` override nobody wrote. The inference is a
// derived read now, and the document is never edited.
func TestAProjectThatLoadsAlsoResolves(t *testing.T) {
yaml := `api_version: onebox.run/v1
app: a
environments:
production:
server: root@1.2.3.4
overrides: {workloads: {w: {resources: {memory: 512MB}}}}
workloads:
w: {image: nginx, volumes: [{name: data, path: /data}], replicas: 3}
`
p, err := LoadBytes([]byte(yaml), "ob.yml")
if err != nil {
t.Fatalf("load: %v", err)
}
if _, err := p.Resolve("production"); err != nil {
t.Fatalf("resolve refused a project that loaded: %v", err)
}
}
35 changes: 35 additions & 0 deletions internal/app/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,41 @@ func (w Workload) DrainSignal() string {
// the rest and are never health-gated, because a job that stays up has failed.
func (w Workload) IsJob() bool { return w.Role == RoleJob }

// HoldsDurableData answers the question three separate places used to guess at.
//
// The contract publishes `persistence.mode` defaulting to durable, but the
// block is optional, so the default was unreachable unless it was written —
// and doctor, the migration-backup requirement and the protection gate each
// read an absent block as "not durable". A workload with a managed named
// volume holds data that outlives the release whether or not it says so.
//
// A bind mount is deliberately not durable here: onebox neither created the
// host path nor can tell configuration from data by looking at it, so the
// bytes are the operator's. Counting them would demand backup evidence for
// every `./config` mount, and a warning that fires on everything is one
// nobody reads.
func (w Workload) HoldsDurableData() bool {
if w.Persistence != nil {
return w.Persistence.Mode == "durable"
}
for _, v := range w.Volumes {
if !v.IsBind() {
return true
}
}
return false
}

// HasBindMounts reports whether any volume is a host path onebox does not own.
func (w Workload) HasBindMounts() bool {
for _, v := range w.Volumes {
if v.IsBind() {
return true
}
}
return false
}

// Role names. They are the schema's discriminator, so they are constants rather
// than string literals scattered across the execution path.
const (
Expand Down
Loading