diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index c092aeb..4a272f1 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -92,8 +92,21 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { var out []map[string]any for _, m := range r.Modules { resources := m.Resources + + // What the mesh computes for this module goes FIRST, before the module's own resources. + // + // **Order is stated, not derived — the host does not sort** (novox/hq ADR 0005), so + // whatever the mesh writes down is the order a machine applies. A module's service or + // container routinely depends on one of these files; nothing here ever depends on a + // module's resources, because none of it is computed from them. + // + // Appended, this was wrong in a way that only showed on the first apply and then healed: + // the service started before its certificate or its rule set existed, failed, and the next + // reconcile fixed it. A fault that repairs itself on the second attempt is worse than one + // that does not, because what gets remembered is that it works. + var first []map[string]any if f := m.Filtering; f != nil { - resources = append(append([]map[string]any{}, resources...), map[string]any{ + first = append(first, map[string]any{ "id": FilteringID(), "type": "file", "path": f.Into, "content": filtering, "mode": "0600", }) @@ -105,14 +118,14 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { return nil, fmt.Errorf( "%s wants a certificate for this machine and none was issued", m.Module) } - resources = append(append([]map[string]any{}, resources...), map[string]any{ + first = append(first, map[string]any{ "id": CertificateID(), "type": "file", "path": c.Into, // Public. It travels in the open like any other file, because it is a statement // about a key rather than the key. "content": with.Certificate, "mode": "0644", }) if c.Authority != "" { - resources = append(resources, map[string]any{ + first = append(first, map[string]any{ "id": AuthorityID(), "type": "file", "path": c.Authority, "content": with.Authority, "mode": "0644", }) @@ -127,7 +140,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { return nil, fmt.Errorf( "%s needs a secret called %q and none was made for it", m.Module, name) } - resources = append(append([]map[string]any{}, resources...), map[string]any{ + first = append(first, map[string]any{ "id": NeedID(name), "type": "file", "path": m.Needs[name], "sealed": sealed, }) } @@ -144,7 +157,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // worse than none: something would read it and fail authenticating. continue } - resources = append(append([]map[string]any{}, resources...), map[string]any{ + first = append(first, map[string]any{ "id": SecretID(to), "type": "file", "path": m.Secrets[to], "sealed": found.Sealed, }) @@ -164,7 +177,7 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // and nothing would say so. continue } - resources = append(append([]map[string]any{}, resources...), map[string]any{ + first = append(first, map[string]any{ "id": GrantID(to, g.Consumer), "type": "file", "path": grantPath(m.Grants[to], g.Consumer), @@ -189,14 +202,14 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { if err != nil { return nil, err } - resources = append(append([]map[string]any{}, resources...), file) + first = append(first, file) } for _, to := range sortedKeys(m.Receives) { file, err := receivedFile(to, m.Receives[to], given[to]) if err != nil { return nil, err } - resources = append(append([]map[string]any{}, resources...), file) + first = append(first, file) } if m.Computed != "" { generator, known := with.Generators[m.Computed] @@ -217,6 +230,11 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { } resources = generated } + + // Now, and not before: a module whose resources are computed replaces them wholesale, and + // merging earlier would throw away the files it still needs. + resources = append(append([]map[string]any{}, first...), resources...) + for _, unsettled := range resources { resource, err := ApplySettings(unsettled, with.Settings[m.Module]) if err != nil { diff --git a/internal/catalogue/filtering_test.go b/internal/catalogue/filtering_test.go index c7bc2e7..7e17ce2 100644 --- a/internal/catalogue/filtering_test.go +++ b/internal/catalogue/filtering_test.go @@ -1,6 +1,7 @@ package catalogue import ( + "fmt" "strings" "testing" ) @@ -237,3 +238,64 @@ func TestAMeshOnBothAddressFamiliesRendersBoth(t *testing.T) { t.Fatalf("both families are in one set, so the file will not load:\n%s", nft) } } + +// The host does not sort, so the order the mesh writes is the order a machine applies. +// +// A module's service or container routinely depends on a file the mesh computed — a certificate, +// a credential, a rule set. Written after the service, the service is applied first, fails, and +// the next reconcile fixes it. A fault that repairs itself on the second attempt is worse than one +// that does not: what gets remembered is that it works. +func TestWhatTheMeshComputesIsAppliedBeforeWhatTheModuleDeclared(t *testing.T) { + r := Resolution{Modules: []Manifest{{ + Module: "firewall", + Filtering: &Filtering{Into: "/etc/nftables.conf"}, + Resources: []map[string]any{ + {"id": "load", "type": "service", "unit": "nftables.service", "state": "running", + "restart-on": []any{"filtering"}}, + }, + }}} + out, err := r.Declaration(Rendering{Mesh: []string{"198.51.100.2"}}) + if err != nil { + t.Fatalf("declaration: %v", err) + } + var order []string + for _, res := range out { + order = append(order, fmt.Sprint(res["id"])) + } + if len(order) != 2 { + t.Fatalf("expected the rule set and the service, got %v", order) + } + if order[0] != "firewall.filtering" { + t.Fatalf("the service is applied before the file it reflects: %v", order) + } +} + +// The same, for a module whose resources are computed elsewhere — that branch replaces the +// module's resources wholesale, and merging in the wrong place would throw away its credentials. +func TestAComputedModuleStillGetsWhatTheMeshMadeForIt(t *testing.T) { + r := Resolution{Modules: []Manifest{{ + Module: "networking", Computed: "mesh-network", + Needs: map[string]string{"key": "/var/lib/mesh/key"}, + }}} + out, err := r.Declaration(Rendering{ + Needed: map[string]map[string]string{"networking": {"key": "sealed"}}, + Generators: map[string]Generator{"mesh-network": computedOnce{}}, + }) + if err != nil { + t.Fatalf("declaration: %v", err) + } + var ids []string + for _, res := range out { + ids = append(ids, fmt.Sprint(res["id"])) + } + if len(ids) == 0 || ids[0] != "networking."+NeedID("key") { + t.Fatalf("a computed module lost the secret the mesh made for it, or applies it late: %v", ids) + } +} + +type computedOnce struct{} + +func (computedOnce) Resources(string) ([]map[string]any, bool, error) { + return []map[string]any{{"id": "wg", "type": "file", "path": "/etc/wireguard/wg0.conf", + "content": "x", "mode": "0600"}}, true, nil +}