From 95f45b1a74564852fd6ed79d322023695ad6028d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 10:36:55 +0100 Subject: [PATCH 1/2] Keep a left-out module's openings in the filter: it still runs (issue 1966) 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, but resolutionCompose dropped it from the modules the filter is derived from, so the filter closed the ports it still serves. On novox, 2026-11-09 07:46-09:24 UTC, the artifact store's want at build 6e1dfaa8 (listens on 5100, no container publishes 5000) met the node layer's ports {"5000": 5100}: GivenPorts refused it, distribution was left out, and 5100 closed while the old registry container served there; every build failed "dial tcp 10.10.0.1:5100: i/o timeout". The filter is now derived from the composed modules and the left-out ones; a left-out module's settings (exposure) are still not read. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- internal/compose/resolution/declaration.go | 15 ++++++++--- internal/compose/resolution/filtering_test.go | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/internal/compose/resolution/declaration.go b/internal/compose/resolution/declaration.go index 6f0f165..951883d 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,8 +1248,9 @@ 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 its settings with it: its per-node exposure is not read, and its listens open + // as its manifest says them. It is not left out of the filter: what it runs is left running, + // so a port it serves stays open (issue 1966). left := LeftOut(r, with.Settings, with.Adopted) exposure := map[string]map[int]string{} for _, m := range r.Modules { diff --git a/internal/compose/resolution/filtering_test.go b/internal/compose/resolution/filtering_test.go index e8976c1..be43cc9 100644 --- a/internal/compose/resolution/filtering_test.go +++ b/internal/compose/resolution/filtering_test.go @@ -60,6 +60,33 @@ 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) + } } // The host does not sort, so the order the mesh writes is the order a machine applies. From 17a4ac620c07a70162a09f5e4e524b851b0c2eef Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 10:43:48 +0100 Subject: [PATCH 2/2] A left-out module's openings keep its per-node narrowing (issue 1966, review round 1) Rules skipped a left-out module's expose and reaches, so its listens opened at the manifest's default source and an operator's narrowing for the node was dropped (ADR 0046). Its exposure is now read like any module's (exposureOf); where it cannot be read for that module, its listens open no wider than the mesh (noWiderThanTheMesh), never at the manifest's default. Left for the thorough fix, composing a left-out module as what runs of it (Carried, as a held module is): the filter also opens for a left-out module that never ran on the node, and it opens the new manifest's listens rather than what the old composition runs. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- internal/compose/resolution/declaration.go | 98 ++++++++++++------- internal/compose/resolution/filtering_test.go | 26 +++++ 2 files changed, 88 insertions(+), 36 deletions(-) diff --git a/internal/compose/resolution/declaration.go b/internal/compose/resolution/declaration.go index 951883d..3133367 100644 --- a/internal/compose/resolution/declaration.go +++ b/internal/compose/resolution/declaration.go @@ -1248,47 +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 its settings with it: its per-node exposure is not read, and its listens open - // as its manifest says them. It is not left out of the filter: what it runs is left running, - // so a port it serves stays open (issue 1966). + // 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 @@ -1297,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 be43cc9..79e4e53 100644 --- a/internal/compose/resolution/filtering_test.go +++ b/internal/compose/resolution/filtering_test.go @@ -87,6 +87,32 @@ func TestTheModuleAskingForTheRuleSetGetsEveryModulesPorts(t *testing.T) { 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.