Converge only on the preview the operator saw, named by its digest, and never on an account older than 15 minutes (hq ADR 0100)

cmd/mesh-controller/adopting_test.go

@@ -115,7 +115,7 @@ func TestTakingAModuleNotOnTheNodeIsRefused(t *testing.T) {
 
 func TestConvergingIsRefusedOnAPreviewThatWouldBeStale(t *testing.T) {
 	open, sent := anAdoptedAnchor(t)
-	_, err := converge(t.Context(), open, "anchor", false, "")
+	_, err := converge(t.Context(), open, "anchor", false, "", "")
 	if err == nil || !strings.Contains(err.Error(), "has not reported") {
 		t.Fatalf("a node that never reported was previewed: %v", err)
 	}
@@ -127,7 +127,7 @@ func TestConvergingIsRefusedOnAPreviewThatWouldBeStale(t *testing.T) {
 func TestTheFlipIsRefusedWhileAFoundContainerIsHeld(t *testing.T) {
 	open, sent := anAdoptedAnchor(t)
 	reportsHolding(t, open, heldContainer, heldFile)
-	_, err := converge(t.Context(), open, "anchor", true, "")
+	_, err := converge(t.Context(), open, "anchor", true, "", "")
 	if err == nil || !strings.Contains(err.Error(), "take anchor hello-web once its data has moved") {
 		t.Fatalf("the flip was not refused while hello-web holds its found container: %v", err)
 	}
@@ -157,7 +157,7 @@ func TestConvergingPreviewsThenChangesAndAdoptingKeepsWhatWasTaken(t *testing.T)
 	}
 	reportsHolding(t, open, heldFile)
 
-	preview, err := converge(ctx, open, "anchor", false, "")
+	preview, err := converge(ctx, open, "anchor", false, "", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -191,7 +191,7 @@ func TestConvergingPreviewsThenChangesAndAdoptingKeepsWhatWasTaken(t *testing.T)
 		t.Fatal("the preview changed something")
 	}
 
-	if _, err := converge(ctx, open, "anchor", true, ""); err != nil {
+	if _, err := converge(ctx, open, "anchor", true, digestIn(t, preview), ""); err != nil {
 		t.Fatal(err)
 	}
 	n, _ := open.inventory.NodeByName(ctx, "anchor")
@@ -245,7 +245,7 @@ func hasID(resources []map[string]any, id string) bool {
 func TestTheApiRefusesTheFlipInTheCommandLinesWords(t *testing.T) {
 	open, _ := anAdoptedAnchor(t)
 	reportsHolding(t, open, heldContainer)
-	_, direct := converge(t.Context(), open, "anchor", true, "")
+	_, direct := converge(t.Context(), open, "anchor", true, "", "")
 	if direct == nil {
 		t.Fatal("the flip was not refused")
 	}
@@ -283,7 +283,7 @@ func TestThePreviewSaysWhatNarrowsToTheMeshCloses(t *testing.T) {
 		{Protocol: "tcp", Address: "0.0.0.0", Port: 8080, By: "hello-web", Published: true,
 			ContainerPort: 80},
 	})
-	preview, err := converge(ctx, open, "anchor", false, "")
+	preview, err := converge(ctx, open, "anchor", false, "", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -307,3 +307,81 @@ func TestThePreviewSaysWhatNarrowsToTheMeshCloses(t *testing.T) {
 		t.Errorf("a port open to everywhere is not said to stay:\n%s", preview)
 	}
 }
+
+// digestIn is the digest a converge preview printed.
+func digestIn(t *testing.T, preview string) string {
+	t.Helper()
+	for _, line := range strings.Split(preview, "\n") {
+		if fields := strings.Fields(line); len(fields) == 2 && fields[0] == "preview" {
+			return fields[1]
+		}
+	}
+	t.Fatalf("the preview printed no digest:\n%s", preview)
+	return ""
+}
+
+// The flip acts on the preview the operator saw: it names that preview's digest, and it is refused
+// when the digest is missing, when anything the preview says has changed since, or when the node's
+// account of itself is too old to be the machine as it is.
+func TestTheFlipActsOnlyOnThePreviewTheOperatorSaw(t *testing.T) {
+	open, sent := anAdoptedAnchor(t)
+	ctx := t.Context()
+	if _, err := take(ctx, open, "anchor", "hello-web"); err != nil {
+		t.Fatal(err)
+	}
+	reportsHolding(t, open, heldFile)
+	preview, err := converge(ctx, open, "anchor", false, "", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	saw := digestIn(t, preview)
+	if !strings.Contains(preview, "converge anchor --yes "+saw) {
+		t.Fatalf("the preview does not say how to act on it:\n%s", preview)
+	}
+	unchanged := func() {
+		t.Helper()
+		if n, _ := open.inventory.NodeByName(ctx, "anchor"); !n.Adopted || len(*sent) != 0 {
+			t.Fatal("a refused flip changed something")
+		}
+	}
+
+	if _, err := converge(ctx, open, "anchor", true, "", ""); err == nil ||
+		!strings.Contains(err.Error(), "--yes "+saw) {
+		t.Fatalf("a flip naming no preview was not refused: %v", err)
+	}
+	unchanged()
+
+	// Something new is reachable: the preview the operator saw is not what would happen.
+	reportsReaching(t, open, []link.Reach{
+		{Protocol: "tcp", Address: "0.0.0.0", Port: 22, By: "sshd"},
+		{Protocol: "tcp", Address: "0.0.0.0", Port: 8080, By: "hello-web", Published: true,
+			ContainerPort: 80},
+		{Protocol: "tcp", Address: "0.0.0.0", Port: 6000, By: "something-new"},
+	}, heldFile)
+	_, err = converge(ctx, open, "anchor", true, saw, "")
+	if err == nil || !strings.Contains(err.Error(), "has changed since preview "+saw) {
+		t.Fatalf("a flip on a changed preview was not refused: %v", err)
+	}
+	unchanged()
+
+	// An account older than the flip trusts is refused, whatever digest is named.
+	again, err := converge(ctx, open, "anchor", false, "", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	saved := reportFreshFor
+	reportFreshFor = time.Nanosecond
+	_, err = converge(ctx, open, "anchor", true, digestIn(t, again), "")
+	reportFreshFor = saved
+	if err == nil || !strings.Contains(err.Error(), "wait for its next report") {
+		t.Fatalf("a flip on an old account was not refused: %v", err)
+	}
+	unchanged()
+
+	if _, err := converge(ctx, open, "anchor", true, digestIn(t, again), ""); err != nil {
+		t.Fatalf("the flip on the preview just seen was refused: %v", err)
+	}
+	if n, _ := open.inventory.NodeByName(ctx, "anchor"); n.Adopted {
+		t.Fatal("the flip did not converge the node")
+	}
+}

cmd/mesh-controller/adoption.go

@@ -2,6 +2,8 @@ package main
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"errors"
 	"flag"
 	"fmt"
@@ -127,6 +129,10 @@ func take(ctx context.Context, open *stores, node, module string) (string, error
 	return said + fmt.Sprintf("\n  run `push %s` to cut it over", node), nil
 }
 
+// reportFreshFor is how old a node's account of itself may be for the flip to act on it. A
+// variable so a test can age a report without waiting.
+var reportFreshFor = 15 * time.Minute
+
 // converge previews, and with yes makes, the flip of an adopted node to converged: every module it
 // runs is taken, the filter module is assigned to load the mesh's derived filter in place of the
 // guard, and the found firewall is retired — disabled, never flushed — by the host.
@@ -134,7 +140,14 @@ func take(ctx context.Context, open *stores, node, module string) (string, error
 // Refused while an assigned module still holds a found container: each service is taken on its
 // own, when its data has moved, never by the flip. And refused on a preview that would be stale:
 // what is reachable is the node's last account, so that account must be of what it was last sent.
-func converge(ctx context.Context, open *stores, node string, yes bool, filter string) (string, error) {
+//
+// **The flip acts on the preview the operator saw** and on nothing else. The preview ends with a
+// short digest of what it said — every reachable thing and its fate, the modules the flip takes and
+// the filter — and yes must name that digest: if anything the preview would say has changed since,
+// the flip is refused rather than done on a preview nobody read. And it is refused on an account
+// older than reportFreshFor: what was reachable then is not evidence of what is reachable now.
+func converge(ctx context.Context, open *stores, node string, yes bool, digest string,
+	filter string) (string, error) {
 	inv := open.inventory
 	if filter == "" {
 		filter = DefaultFilter
@@ -219,9 +232,25 @@ func converge(ctx context.Context, open *stores, node string, yes bool, filter s
 	}
 	derived := derivedFilter{rules: rules, foundation: with.Foundation, mesh: with.Mesh,
 		outward: plan.PublicDomain != ""}
-	preview := previewOf(node, reported, derived, plan, taken, filter, runs[filter])
+	preview, saw := previewOf(node, reported, derived, plan, taken, filter, runs[filter])
+	preview += "\n\n  preview " + saw
 	if !yes {
-		return preview + fmt.Sprintf("\n\nNothing has changed. Run `converge %s --yes` to do it.", node), nil
+		return preview + fmt.Sprintf("\n\nNothing has changed. Run `converge %s --yes %s` to do "+
+			"it.", node, saw), nil
+	}
+	if age := time.Since(reported.At); age > reportFreshFor {
+		return preview, fmt.Errorf("%s last said what is reachable on it %s ago, and the flip acts "+
+			"only on an account newer than %s: wait for its next report, or run `push %s --wait 2m`, "+
+			"then preview again", node, age.Round(time.Second), reportFreshFor, node)
+	}
+	if digest == "" {
+		return preview, fmt.Errorf("converging %s acts on the preview you saw: name its digest, "+
+			"`converge %s --yes %s`, once you have read it", node, node, saw)
+	}
+	if digest != saw {
+		return preview, fmt.Errorf("what converging %s would do has changed since preview %s "+
+			"(it is now %s): read the preview above, and run `converge %s --yes %s` if it is "+
+			"what you want", node, digest, saw, node, saw)
 	}
 
 	// The flip. The filter first, and only kept if the node still resolves with it: a node that
@@ -253,9 +282,12 @@ func converge(ctx context.Context, open *stores, node string, yes bool, filter s
 	return said + "\n  sent: the host loads the mesh's filter and disables the firewall it found", nil
 }
 
-// previewOf is what converging a node will change, before it changes it.
+// previewOf is what converging a node will change, before it changes it, and a short digest of
+// what it said: every reachable thing and its fate, the modules the flip takes and the filter. The
+// digest is what the flip is asked to act on, so it changes whenever any of those would.
 func previewOf(node string, reported inventory.Adoption, derived derivedFilter,
-	plan catalogue.Resolution, taken []string, filter string, filterAssigned bool) string {
+	plan catalogue.Resolution, taken []string, filter string, filterAssigned bool) (string, string) {
+	var said []string
 	var b strings.Builder
 	fmt.Fprintf(&b, "converging %s\n", node)
 	fmt.Fprintf(&b, "\n  reachable on the machine now, as it reported at %s:\n",
@@ -271,7 +303,9 @@ func previewOf(node string, reported inventory.Adoption, derived derivedFilter,
 		if r.Published {
 			what += fmt.Sprintf(" (published, container port %d)", r.ContainerPort)
 		}
-		fmt.Fprintf(&b, "    %-44s %s\n", what, derived.fate(r))
+		fate := derived.fate(r)
+		fmt.Fprintf(&b, "    %-44s %s\n", what, fate)
+		said = append(said, fmt.Sprintf("reach %s %s %s", r.Address, what, fate))
 	}
 	if len(reported.Reachable) == 0 {
 		b.WriteString("    nothing reported\n")
@@ -302,6 +336,7 @@ func previewOf(node string, reported inventory.Adoption, derived derivedFilter,
 	}
 	for _, m := range takes {
 		fmt.Fprintf(&b, "    %s\n", m)
+		said = append(said, "take "+m)
 		for _, h := range reported.Held {
 			if h.Module == m && h.Kind == "file" {
 				fmt.Fprintf(&b, "      replacing the found file %s", h.Target)
@@ -321,7 +356,11 @@ func previewOf(node string, reported inventory.Adoption, derived derivedFilter,
 	} else {
 		fmt.Fprintf(&b, "  the found firewall (%s) is disabled, never flushed: its configuration stays on disk\n", fw)
 	}
-	return strings.TrimRight(b.String(), "\n")
+	said = append(said, fmt.Sprintf("filter %s assigned=%t firewall=%s", filter, filterAssigned, fw))
+	// Sorted: the same account, reported in another order, is the same preview.
+	sort.Strings(said)
+	sum := sha256.Sum256([]byte(strings.Join(said, "\n")))
+	return strings.TrimRight(b.String(), "\n"), hex.EncodeToString(sum[:])[:12]
 }
 
 // derivedFilter is what the filter the flip loads is rendered from, as AsNftables renders it.
@@ -416,17 +455,18 @@ func takeCommand(ctx context.Context, args []string) error {
 
 func convergeCommand(ctx context.Context, args []string) error {
 	set := flag.NewFlagSet("converge", flag.ContinueOnError)
-	yes := set.Bool("yes", false, "do it; without it, only the preview")
+	yes := set.String("yes", "", "do it, naming the digest the preview printed; without it, only "+
+		"the preview")
 	filter := set.String("filter", DefaultFilter, "the module that loads the mesh's filter")
 	positionals, err := parseAround(set, args)
 	if err != nil {
 		return err
 	}
 	if len(positionals) != 1 {
-		return errors.New("converge <node> [--yes] [--filter nftables]")
+		return errors.New("converge <node> [--yes <digest>] [--filter nftables]")
 	}
 	return runAct(ctx, func(open *stores) (string, error) {
-		return converge(ctx, open, positionals[0], *yes, *filter)
+		return converge(ctx, open, positionals[0], *yes != "", *yes, *filter)
 	})
 }
 

cmd/mesh-controller/api.go

@@ -105,7 +105,7 @@ func commands(who Authenticator) http.Handler {
 		return take(ctx, open, in.Node, in.Module)
 	}))
 	mux.HandleFunc("POST /converge", acting(who, false, func(ctx context.Context, open *stores, in request) (string, error) {
-		return converge(ctx, open, in.Node, in.Yes, in.Filter)
+		return converge(ctx, open, in.Node, in.Yes, in.Digest, in.Filter)
 	}))
 	mux.HandleFunc("POST /adopt", acting(who, false, func(ctx context.Context, open *stores, in request) (string, error) {
 		return adopt(ctx, open, in.Node)
@@ -124,9 +124,10 @@ func commands(who Authenticator) http.Handler {
 type request struct {
 	Node   string `json:"node"`
 	Module string `json:"module"`
-	// Yes and Filter are converge's: do it rather than preview it, and which module loads the
+	// Yes, Digest and Filter are converge's: do it rather than preview it, the digest of the
-	// mesh's filter.
+	// preview it acts on, and which module loads the mesh's filter.
 	Yes    bool   `json:"yes,omitempty"`
+	Digest string `json:"digest,omitempty"`
 	Filter string `json:"filter,omitempty"`
 }
 

cmd/mesh-controller/main.go

@@ -162,7 +162,7 @@ func usage() {
   assign <node> <module>         put a module on a node
   unassign <node> <module>       take it off
   take <node> <module>           cut a module over on an adopted node, once its data has moved
-  converge <node> [--yes] [--filter nftables]   preview, then make, an adopted node converged
+  converge <node> [--yes <digest>] [--filter nftables]   preview, then make, an adopted node converged
   adopt <node>                   return a converged node to adopted; what was taken stays taken
   settings set <module> <file>   what a module's config should say, for the whole mesh
   settings set <module> <file> --node <n>   ...or for one machine