diff --git a/internal/compose/resolution/declaration.go b/internal/compose/resolution/declaration.go index 6f0f165..3133367 100644 --- a/internal/compose/resolution/declaration.go +++ b/internal/compose/resolution/declaration.go @@ -147,7 +147,7 @@ func resolutionCompose(r mcat.Resolution, with mcat.Rendering, owner map[string] // Before placing, because a placement is a setting too. left := LeftOut(r, with.Settings, with.Adopted) kept := make([]mcat.Manifest, 0, len(r.Modules)) - var stillBackedUp []mcat.Manifest + var stillBackedUp, stillRunning []mcat.Manifest for module, why := range with.NotPlaced { if _, already := left[module]; !already { left[module] = why @@ -162,6 +162,10 @@ func resolutionCompose(r mcat.Resolution, with mcat.Rendering, owner map[string] // the data it already holds on the machine is the reason to keep copying it. Only its data, // as the backup holder's lines are derived from it, and the directories they name. stillBackedUp = append(stillBackedUp, backupView(m)) + // **And what it runs is still let through** (issue 1966): its containers and services are left as they run, so + // the filter keeps opening what its listens say, at the machine ports this node holds for it, rather than + // closing a port that still serves (the artifact store's 5100 on the control-node, 98 minutes). + stillRunning = append(stillRunning, m) continue } kept = append(kept, m) @@ -255,7 +259,9 @@ func resolutionCompose(r mcat.Resolution, with mcat.Rendering, owner map[string] // Once, from every module's listens -- not per module. A module receiving only its own ports // would write a rule set that closed every other module on the machine. Each module's per-node // exposure settings override its listens' source first (novox/hq ADR 0046). - rules, err := Rules(r, with) + filtered := r + filtered.Modules = append(slices.Clone(r.Modules), stillRunning...) + rules, err := Rules(filtered, with) if err != nil { return nil, err } @@ -1242,46 +1248,19 @@ func resolutionFiltersHere(r mcat.Resolution) string { // is adopted or converged — the one loads it as a filter, the other declares it as openings. func Rules(r mcat.Resolution, with mcat.Rendering) ([]mcat.Rule, error) { // A module whose settings cannot compose is left out of the declaration (novox/hq ADR 0163, - // rule 6), and out of the filter with it: nothing of it is declared, so nothing of it is let - // through. + // rule 6), and not out of the filter: what it runs is left running, so a port it serves stays + // open (issue 1966). Its per-node exposure and reaches still narrow it (ADR 0046); where they + // cannot be read for it, its listens open no wider than the mesh, never at the manifest's + // default, which could reopen a port an operator had narrowed. left := LeftOut(r, with.Settings, with.Adopted) exposure := map[string]map[int]string{} for _, m := range r.Modules { - if _, isLeft := left[m.Module]; isLeft { - continue - } - e, err := Exposure(m, with.Settings[m.Module]) + e, err := exposureOf(m, with.Settings[m.Module]) if err != nil { - return nil, err - } - // And how far each endpoint reaches, which says the same thing to the filter and more - // besides (novox/hq ADR 0138). Folded in here rather than beside: the filter has one - // question — from where — and a reach answers it, so giving it two inputs would let them - // disagree. Reaches refuses a port that both name, so this cannot silently prefer one. - reaches, err := Reaches(m, with.Settings[m.Module]) - if err != nil { - return nil, err - } - // **Only for an endpoint the proxy does not serve.** A routed endpoint's port is how the - // proxy reaches it and nothing else (ADR 0045), so `public` there asks for a public name and - // says nothing about the port — opening it to the world as well would undo the arrangement - // the proxy exists for, and would silently reopen a port an operator had narrowed. - // - // Found by trying to express a real module: one whose routed name must be public and whose - // machine-side port must not be. Under one value for both, there was no way to say it. - routed := RoutedPorts(m) - for port, reach := range reaches { - if routed[port] { - continue + if _, isLeft := left[m.Module]; !isLeft { + return nil, err } - source, ok := FilterSource(reach) - if !ok { - return nil, fmt.Errorf("%s: %q is not a reach the filter can read", m.Module, reach) - } - if e == nil { - e = map[int]string{} - } - e[port] = source + e = noWiderThanTheMesh(m) } if e != nil { exposure[m.Module] = e @@ -1290,6 +1269,60 @@ func Rules(r mcat.Resolution, with mcat.Rendering) ([]mcat.Rule, error) { return Filtering(r, with.Generators, with.Ports, exposure) } +// noWiderThanTheMesh is the exposure of a module left out whose own cannot be read: each of its +// listens from wider than the mesh narrowed to the mesh, a narrower one kept. +func noWiderThanTheMesh(m mcat.Manifest) map[int]string { + var e map[int]string + for _, l := range m.Listens { + if l.From == mcat.FromLocalNetwork || l.From == mcat.FromEverywhere { + if e == nil { + e = map[int]string{} + } + e[l.Port] = mcat.FromMesh + } + } + return e +} + +// exposureOf is a module's per-node exposure, declared port to source: its expose setting, and +// the reaches of its endpoints the proxy does not serve. +func exposureOf(m mcat.Manifest, layers []mcat.Layer) (map[int]string, error) { + e, err := Exposure(m, layers) + if err != nil { + return nil, err + } + // And how far each endpoint reaches, which says the same thing to the filter and more + // besides (novox/hq ADR 0138). Folded in here rather than beside: the filter has one + // question — from where — and a reach answers it, so giving it two inputs would let them + // disagree. Reaches refuses a port that both name, so this cannot silently prefer one. + reaches, err := Reaches(m, layers) + if err != nil { + return nil, err + } + // **Only for an endpoint the proxy does not serve.** A routed endpoint's port is how the + // proxy reaches it and nothing else (ADR 0045), so `public` there asks for a public name and + // says nothing about the port — opening it to the world as well would undo the arrangement + // the proxy exists for, and would silently reopen a port an operator had narrowed. + // + // Found by trying to express a real module: one whose routed name must be public and whose + // machine-side port must not be. Under one value for both, there was no way to say it. + routed := RoutedPorts(m) + for port, reach := range reaches { + if routed[port] { + continue + } + source, ok := FilterSource(reach) + if !ok { + return nil, fmt.Errorf("%s: %q is not a reach the filter can read", m.Module, reach) + } + if e == nil { + e = map[int]string{} + } + e[port] = source + } + return e, nil +} + // provisionsAs is the account that reads what the mesh writes for this provider: the one secret // per consumer it must open to set that consumer's password (novox/hq issue 225). // diff --git a/internal/compose/resolution/filtering_test.go b/internal/compose/resolution/filtering_test.go index e8976c1..79e4e53 100644 --- a/internal/compose/resolution/filtering_test.go +++ b/internal/compose/resolution/filtering_test.go @@ -60,6 +60,59 @@ func TestTheModuleAskingForTheRuleSetGetsEveryModulesPorts(t *testing.T) { if !strings.Contains(content, "dport 443 accept") { t.Fatalf("the rule set holds only the asking module's ports:\n%s", content) } + + // Issue 1966, novox 2026-11-09 07:46-09:24 UTC: a module left out of the declaration for a setting its new definition + // no longer composes (ADR 0163 rule 6) keeps running what it ran, so the filter keeps letting it through. The + // artifact store's build 6e1dfaa8 listens on 5100 from a process and publishes no container port 5000, while its + // node layer still gave 5000 the machine port 5100: left out, and the filter closed 5100 for 98 minutes while the old + // registry container still served there; every build failed "dial tcp 10.10.0.1:5100: i/o timeout". + r.Modules = append(r.Modules, mcat.Manifest{Module: "distribution", + Listens: []mcat.Listening{{Name: "registry", Port: 5100, Protocol: "tcp", From: mcat.FromEverywhere}}, + Resources: []map[string]any{{"id": "store", "type": "container", "name": "mesh-registry", + "image": "registry@sha256:" + strings.Repeat("a", 64), "ports": []any{"127.0.0.1:5001:5001"}}}}) + with := mcat.Rendering{Mesh: []string{"198.51.100.2"}, OutwardLinks: []string{"eth0"}, TunnelInterface: "mesh0", + Settings: mcat.SettingsBy{"distribution": {{From: "novox", Placement: map[string]any{mcat.PortsSetting: map[string]any{"5000": 5100}}}}}} + if left := LeftOut(r, with.Settings, false); left["distribution"] == "" { + t.Fatalf("the stale port setting does not leave distribution out: %v", left) + } + if out, err = Declaration(r, with); err != nil { + t.Fatalf("declaration with a module left out: %v", err) + } + content = "" + for _, res := range out { + if res["path"] == "/etc/mesh/filter.nft" { + content, _ = res["content"].(string) + } + } + if !strings.Contains(content, "dport 5100 accept") || !strings.Contains(content, "dport 443 accept") { + t.Fatalf("the filter closes the port of a module left out of the declaration, which still runs:\n%s", content) + } + // Its per-node narrowing still holds (ADR 0046), and an exposure that cannot be read for it opens it no wider than + // the mesh, never at the manifest's default (review round 1). + for _, row := range []struct { + expose map[string]any + from string + }{ + {map[string]any{"5100": mcat.FromMachine}, mcat.FromMachine}, + {map[string]any{"9999": mcat.FromMachine}, mcat.FromMesh}, // a port it does not listen on: refused + } { + layer := with.Settings["distribution"][0] + layer.Placement = map[string]any{mcat.PortsSetting: map[string]any{"5000": 5100}, mcat.ExposeSetting: row.expose} + with.Settings = mcat.SettingsBy{"distribution": {layer}} + rules, err := Rules(r, with) + if err != nil { + t.Fatalf("expose %v: %v", row.expose, err) + } + var from []string + for _, rule := range rules { + if rule.Port == 5100 { + from = append(from, rule.From) + } + } + if len(from) != 1 || from[0] != row.from { + t.Errorf("expose %v: the left-out module's 5100 opens from %v, want %s", row.expose, from, row.from) + } + } } // The host does not sort, so the order the mesh writes is the order a machine applies.