Merge pull request 'Grant a provider only the consumers bound to it (hq issue 274)' (#87) from fix/a-grant-follows-the-binding into main

cmd/mesh-controller/grant_follows_binding_test.go

@@ -0,0 +1,197 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/inventory"
+)
+
+// novox/hq issue 274: a provider is granted exactly the consumers whose own resolution binds them to
+// it — not every consumer a pair credential from it was ever made for.
+
+// grantOf is the grant of one provision to one consuming module, and whether there is one at all.
+func grantOf(grants []catalogue.Grant, provision, consumer, module string) (catalogue.Grant, bool) {
+	for _, g := range grants {
+		if g.Provision == provision && g.Consumer == consumer && (g.From == module || g.From == "") {
+			return g, true
+		}
+	}
+	return catalogue.Grant{}, false
+}
+
+// The morning after issue 273, through the stores: a consumer was bound to the store on another
+// machine, a credential from there was made, and a person pinned it back to the store beside it. Both
+// credentials are on record. The store it left is no longer granted it — so it retires it and keeps
+// its data (ADR 0230) — and says so; the store it is bound to keeps its grant; and the credential from
+// the store it left stays on record.
+func TestAConsumerPinnedBackIsNoLongerGrantedByTheProviderItLeft(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	inv := open.inventory
+	for _, m := range storeManifests() {
+		register(t, open, m)
+	}
+	if _, err := inv.SeedSeats(ctx, catalogue.DefaultSeats()); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := assign(ctx, open, "anchor", "store"); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.HoldSeat(ctx, "mesh-store", catalogue.ScopeMesh, "anchor", "store"); err != nil {
+		t.Fatal(err)
+	}
+	for _, a := range [][2]string{{"laptop", "store"}, {"laptop", "board"}} {
+		if _, err := assign(ctx, open, a[0], a[1]); err != nil {
+			t.Fatalf("assign %s %s: %v", a[0], a[1], err)
+		}
+	}
+
+	// Bound to the anchor's store, and sent so: the credential from the anchor is made and recorded.
+	if err := inv.PinProvision(ctx, "laptop", "postgres-database", "anchor", "store"); err != nil {
+		t.Fatal(err)
+	}
+	plan, _, err := planFor(ctx, open, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n := need(t, plan, "board", "postgres-database"); n.From != "anchor" {
+		t.Fatalf("pinned to the anchor and bound to %s", n.From)
+	}
+	if err := inv.RecordBindings(ctx, "laptop", boundToData(plan, nil)); err != nil {
+		t.Fatal(err)
+	}
+	grants, _, unbound, err := grantsFor(ctx, open, "anchor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if g, ok := grantOf(grants, "postgres-database", "laptop", "board"); !ok || g.From != "board" || len(unbound) != 0 {
+		t.Fatalf("a consumer bound to the anchor is not granted there: %+v, unbound %+v", grants, unbound)
+	}
+
+	// Pinned back beside its data, as the operator did.
+	if err := inv.PinProvision(ctx, "laptop", "postgres-database", "laptop", "store"); err != nil {
+		t.Fatal(err)
+	}
+	plan, _, err = planFor(ctx, open, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n := need(t, plan, "board", "postgres-database"); n.From != "laptop" {
+		t.Fatalf("pinned back to the laptop and bound to %s", n.From)
+	}
+	if err := inv.RecordBindings(ctx, "laptop", boundToData(plan, nil)); err != nil {
+		t.Fatal(err)
+	}
+	holders, err := inv.HoldersOf(ctx, "postgres-database", "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(holders) != 2 {
+		t.Fatalf("today's state is two credentials on record, one from each store: %+v", holders)
+	}
+
+	// The store it left: no longer granted, and said.
+	grants, _, unbound, err = grantsFor(ctx, open, "anchor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if g, ok := grantOf(grants, "postgres-database", "laptop", "board"); ok && g.From != "" {
+		t.Fatalf("the anchor's store is still granted a consumer bound to the laptop's: %+v", g)
+	}
+	if len(unbound) != 1 || unbound[0].Module != "board" || unbound[0].Provider != "anchor" ||
+		strings.Join(unbound[0].BoundTo, ",") != "laptop" {
+		t.Fatalf("the consumer that moved is not the one said: %+v", unbound)
+	}
+	planned, settings, err := planFor(ctx, open, "anchor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	declared, err := declarationFor(ctx, open, "anchor", planned, settings)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := declared.Received["store"]["postgres-database"]; len(got) != 0 {
+		t.Fatalf("the anchor's store is still told about %+v", got)
+	}
+	said := printed(t, func() error { reportLeftOut("anchor", declared); return nil })
+	if !strings.Contains(said, "board on laptop is bound to laptop for postgres-database, not to anchor") ||
+		!strings.Contains(said, "cleanup delete") {
+		t.Fatalf("the push does not say whom the anchor no longer grants:\n%s", said)
+	}
+
+	// The store it is bound to: granted.
+	grants, _, unbound, err = grantsFor(ctx, open, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if g, ok := grantOf(grants, "postgres-database", "laptop", "board"); !ok || g.From != "board" || len(unbound) != 0 {
+		t.Fatalf("the consumer is not granted by the store it is bound to: %+v, unbound %+v", grants, unbound)
+	}
+
+	// And the credential from the store it left is kept: the key to the login and data held there.
+	if holders, err = inv.HoldersOf(ctx, "postgres-database", "laptop"); err != nil || len(holders) != 2 {
+		t.Fatalf("a credential was forgotten while its provider still holds the login: %+v, %v", holders, err)
+	}
+}
+
+// A consumer whose resolution cannot be read is an error, never a consumer bound nowhere — withdrawing
+// a grant on that reading would take its access away (issue 152).
+func TestAConsumerWhoseResolutionCannotBeReadIsNotWithdrawn(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	inv := open.inventory
+	for _, m := range storeManifests() {
+		register(t, open, m)
+	}
+	for _, a := range [][2]string{{"anchor", "store"}, {"laptop", "board"}} {
+		if _, err := assign(ctx, open, a[0], a[1]); err != nil {
+			t.Fatalf("assign %s %s: %v", a[0], a[1], err)
+		}
+	}
+	if _, _, err := planFor(ctx, open, "laptop"); err != nil {
+		t.Fatal(err)
+	}
+
+	// The laptop's key changes to one nothing can seal to: its resolution cannot be completed, and
+	// that is not its set failing to compose.
+	laptop, err := inv.NodeByName(ctx, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.RecordSealingKey(ctx, laptop.ID, "not a key"); err != nil {
+		t.Fatal(err)
+	}
+	if _, _, err := planFor(ctx, open, "laptop"); err == nil || unresolvable(err) {
+		t.Fatalf("the seam this test relies on moved: %v", err)
+	}
+	grants, _, unbound, err := grantsFor(ctx, open, "anchor")
+	if err == nil {
+		t.Fatalf("an unreadable consumer was answered: grants %+v, unbound %+v", grants, unbound)
+	}
+	if !strings.Contains(err.Error(), "what laptop asked of postgres-database cannot be read") {
+		t.Fatalf("the error does not say whose resolution could not be read: %v", err)
+	}
+}
+
+// The rule itself, on a resolution: bound is the provider a need for exactly that credential is
+// answered by, never one answered by a record, and a different local name is a different credential.
+func TestBindsFromIsTheProviderOfThatCredential(t *testing.T) {
+	r := catalogue.Resolution{Needs: []catalogue.Needed{
+		{Name: "postgres-database", For: "board", From: "laptop"},
+		{Name: "postgres-database", For: "board", From: "laptop", Local: "reports"},
+		{Name: "postgres-database", For: "wiki", From: "anchor"},
+		{Name: "a-licence", For: "board", From: "the-licence", ByRecord: true},
+	}}
+	s := inventory.Secret{Name: "postgres-database", ConsumerModule: "board"}
+	if got := r.BindsFrom(s.Name, s.ConsumerModule, s.Local); strings.Join(got, ",") != "laptop" {
+		t.Fatalf("board's credential is bound to %v", got)
+	}
+	if got := r.BindsFrom("postgres-database", "board", "archive"); len(got) != 0 {
+		t.Fatalf("a local name nothing asks for is bound to %v", got)
+	}
+	if got := r.BindsFrom("a-licence", "board", ""); len(got) != 0 {
+		t.Fatalf("a need answered by a record is bound to %v", got)
+	}
+}

cmd/mesh-controller/identity_bound_test.go

@@ -96,7 +96,7 @@ func TestAnOverflowingConsumerNeverRefusesItsProvidersMachine(t *testing.T) {
 	if len(declared.withheld) != 1 || declared.withheld[0].Identity != "mesh_laptop_photoalbum" {
 		t.Fatalf("the overflowing consumer is not the one withheld: %+v", declared.withheld)
 	}
-	grants, _, err := grantsFor(ctx, open, "anchor")
+	grants, _, _, err := grantsFor(ctx, open, "anchor")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/mesh-controller/plan.go

@@ -405,7 +405,8 @@ func declarationWith(ctx context.Context, open *stores, node string,
 	}
 	out := sendable{Resources: composed.Resources, Adoption: adoption,
 		Received: composed.Received, Mesh: with.Mesh, BusUsers: with.BusUsers,
-		LeftOut: sortedKeysOf(composed.LeftOut), leftOutWhy: composed.LeftOut, withheld: with.Withheld}
+		LeftOut: sortedKeysOf(composed.LeftOut), leftOutWhy: composed.LeftOut, withheld: with.Withheld,
+		unbound: with.Unbound}
 	// And which build of each module it carries, for the send to record (novox/hq issue 259, ADR
 	// 0221). Read only on the send path: a question about what would be sent records nothing.
 	if choosing == Allocating {
@@ -500,6 +501,10 @@ func reportLeftOut(node string, declared sendable) {
 	for _, o := range declared.withheld {
 		fmt.Printf("%s: %s\n", node, o)
 	}
+	// And whom it no longer serves because they are bound elsewhere (novox/hq issue 274).
+	for _, u := range declared.unbound {
+		fmt.Printf("%s: %s\n", node, u)
+	}
 }
 
 // renderingFor is everything a node's declaration is composed with, and the node's record.
@@ -507,7 +512,7 @@ func renderingFor(ctx context.Context, open *stores, node string,
 	plan catalogue.Resolution, settings catalogue.SettingsBy,
 	gens map[string]catalogue.Generator, choosing Choosing) (catalogue.Rendering, inventory.Node, error) {
 	inv := open.inventory
-	grants, withheld, err := grantsFor(ctx, open, node)
+	grants, withheld, unbound, err := grantsFor(ctx, open, node)
 	if err != nil {
 		return catalogue.Rendering{}, inventory.Node{}, err
 	}
@@ -830,7 +835,7 @@ func renderingFor(ctx context.Context, open *stores, node string,
 		Suffix: overlay.Suffix(), MeshRange: meshRange, TunnelInterface: overlay.Interface, Accounts: accounts, Foundation: foundation,
 		Kept: kept, Adopted: record.Adopted, OutwardLinks: outwardLinks,
 		Given: given, Taken: taken, Seats: seats, ArtifactStore: artifactStore, SeatReach: reach, Built: built,
-		BusUsers: busUsers, Withheld: withheld,
+		BusUsers: busUsers, Withheld: withheld, Unbound: unbound,
 	}, record, nil
 }
 
@@ -971,22 +976,32 @@ func certificateFor(ctx context.Context, open *stores, node string) (string, str
 // A consumer whose identity overflows the provision's bound is left out of the grants and returned
 // beside them, for push, plan and `status` to say; every other consumer is granted and the provider's
 // declaration composes. Refusing here once made a whole machine unpushable for one module elsewhere.
-func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Grant, []catalogue.Overflow, error) {
+//
+// **A provider is granted exactly the consumers whose own resolution binds them to it** (novox/hq
+// issue 274). The pair credentials on record say only whom this node was ever asked by: after a
+// consumer of a provision that keeps its data was moved and pinned back (issue 273, ADR 0232), the
+// credential from the provider it left was still on record, so that provider went on being asked for
+// five databases nobody used and never retired them. A credential whose consumer is bound elsewhere
+// is withdrawn here like one nobody asks for — the provider retires it and keeps its data (ADR 0230)
+// — and returned beside the grants, for plan and push to say. It stays on record: it is the key to
+// the login the provider keeps until `cleanup delete`, and to that data should a person pin it back.
+func grantsFor(ctx context.Context, open *stores, node string) (
+	[]catalogue.Grant, []catalogue.Overflow, []catalogue.Unbound, error) {
 	inv := open.inventory
 	issued, err := inv.SecretsFrom(ctx, node)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
 	// Where each consumer is, so a provider that must reach back to one does not have to know how
 	// the mesh names machines.
 	shelf, err := inv.Catalogue(ctx)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	onNetwork, err := whereEveryoneIs(ctx, inv, shelf)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
 	// What each consumer actually asked for, taken from that machine's own resolution rather than
@@ -994,6 +1009,7 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran
 	// can do nothing with it, and the name a consumer wants is the consumer's to say.
 	out := make([]catalogue.Grant, 0, len(issued))
 	var withheld []catalogue.Overflow
+	var unbound []catalogue.Unbound
 	for _, s := range issued {
 		plan, settings, err := planFor(ctx, open, s.Consumer)
 		switch {
@@ -1006,11 +1022,11 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran
 			// The mesh could not be asked what they wanted, which is not the same as their wanting
 			// nothing — and withholding a grant on that reading takes a consumer's access away
 			// (novox/hq 04-ISSUES/152).
-			return nil, nil, fmt.Errorf("what %s asked of %s cannot be read: %w", s.Consumer, s.Name, err)
+			return nil, nil, nil, fmt.Errorf("what %s asked of %s cannot be read: %w", s.Consumer, s.Name, err)
 		}
 		values, asks, err := plan.ContributionsFrom(s.Name, s.ConsumerModule, settings)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		// A port in there is the consumer's software port until this. The consumer is on another
 		// machine, so the assignment that moved it is that machine's — fetched here rather than
@@ -1018,7 +1034,7 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran
 		// this case (novox/hq 04-ISSUES/038, the cross-node half).
 		published, err := portsOn(ctx, inv, s.Consumer, s.ConsumerModule)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		values = catalogue.AtPublishedPort(values, s.ConsumerModule, published)
 		from := s.ConsumerModule
@@ -1028,6 +1044,15 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran
 			// working for ever after its consumer went away.
 			from = ""
 		}
+		if bound := plan.BindsFrom(s.Name, s.ConsumerModule, s.Local); from != "" && !slices.Contains(bound, node) {
+			// It still asks, and not of this node: its resolution — the same one read above, so an
+			// unreadable one is the error above and never an empty answer here (issue 152) — binds
+			// this credential to another provider, or under this local name to none. Withdrawn
+			// exactly as a credential nobody asks for, and said.
+			from = ""
+			unbound = append(unbound, catalogue.Unbound{Provision: s.Name, Provider: node,
+				Consumer: s.Consumer, Module: s.ConsumerModule, Local: s.Local, BoundTo: bound})
+		}
 		// The consumer's identity slug, from its own manifest, carried on the grant so the provider
 		// derives the same login the consumer does (novox/hq ADR 0049). Judged against the bound of
 		// this provision, as the consumer's resolution states it from the provider's offer (ADR
@@ -1054,7 +1079,7 @@ func grantsFor(ctx context.Context, open *stores, node string) ([]catalogue.Gran
 			Provision: s.Name, Consumer: s.Consumer, At: onNetwork[s.Consumer],
 			From: from, Values: values, Slug: slug, Sealed: s.ForProvider, Local: s.Local})
 	}
-	return out, withheld, nil
+	return out, withheld, unbound, nil
 }
 
 // boundOfGrant is the identity bound the consumer's own resolution states for the requirement this

cmd/mesh-controller/sendable.go

@@ -51,6 +51,9 @@ type sendable struct {
 	// withheld is every consumer this machine's grants leave out, because its identity overflows the
 	// provision's bound (novox/hq ADR 0225); for push and plan to say, never on the wire.
 	withheld []catalogue.Overflow
+	// unbound is every consumer whose credential from this machine is on record and that is bound
+	// elsewhere (novox/hq issue 274); for push and plan to say, never on the wire.
+	unbound []catalogue.Unbound
 	// Builds is the build of each module this declaration carries — module to the commit its build
 	// was made from — recorded with the send and never on the wire (novox/hq issue 259, ADR 0221).
 	// Composed only on the send path; nil records that it is not known.

internal/catalogue/bound.go

@@ -1,6 +1,10 @@
 package catalogue
 
-import "fmt"
+import (
+	"fmt"
+	"slices"
+	"strings"
+)
 
 // A consumer of a provision that keeps its data stays bound where its data is (novox/hq ADR 0232).
 //
@@ -125,3 +129,55 @@ func boundNeed(n Needed, bound Chosen, catalogue map[string]Manifest, node Node,
 	held.From, held.At, held.Module, held.Serves, held.Identity = p.Node, p.At, p.Module, p.Serves, identity
 	return held, true, ""
 }
+
+// Unbound is one consumer that still asks for a provision and whose own resolution binds it to
+// another provider than the one a pair credential on record was made with (novox/hq issue 274).
+//
+// **A provider is granted exactly the consumers bound to it.** The credential from the old provider
+// stays on record — it is the key to a login that provider keeps, disabled, with the consumer's data,
+// until a person deletes it with `cleanup delete` (ADR 0230), and a pin back must find it — but it is
+// no longer granted, so the provider stops being asked for it and retires it. Said on every plan and
+// push of the provider, so a credential the mesh keeps and does not use is never kept silently.
+type Unbound struct {
+	// Provision is what was required, Provider the machine the credential on record is from.
+	Provision string `json:"provision"`
+	Provider  string `json:"provider"`
+	// Consumer is the machine, Module the module on it that requires it, Local the credential's
+	// name inside it where it keeps several (ADR 0094).
+	Consumer string `json:"consumer"`
+	Module   string `json:"module"`
+	Local    string `json:"local,omitempty"`
+	// BoundTo is every provider the consumer's resolution binds this credential to now; empty when
+	// it binds it nowhere — the module asks for the provision under other local names.
+	BoundTo []string `json:"bound_to,omitempty"`
+}
+
+func (u Unbound) String() string {
+	who := u.Module
+	if u.Local != "" {
+		who += " (as " + u.Local + ")"
+	}
+	now := "is bound to no provider under that name"
+	if len(u.BoundTo) > 0 {
+		now = "is bound to " + strings.Join(u.BoundTo, ", ")
+	}
+	return fmt.Sprintf("%s on %s %s for %s, not to %s — %s no longer grants it, so it retires that "+
+		"login and keeps its data until `cleanup delete` (ADR 0230); the credential from %s stays on "+
+		"record while that login does", who, u.Consumer, now, u.Provision, u.Provider, u.Provider, u.Provider)
+}
+
+// BindsFrom is the providers this resolution binds a pair credential's need to — the provision, the
+// module that requires it and the credential's local name — answered by a machine rather than by a
+// record. None means this machine states no such binding.
+func (r Resolution) BindsFrom(provision, module, local string) []string {
+	var from []string
+	for _, n := range r.Needs {
+		if n.ByRecord || n.Name != provision || n.For != module || n.Local != local {
+			continue
+		}
+		if !slices.Contains(from, n.From) {
+			from = append(from, n.From)
+		}
+	}
+	return from
+}

internal/catalogue/declaration.go

@@ -174,6 +174,9 @@ type Rendering struct {
 	// bound (novox/hq ADR 0225). Composed into nothing; carried so the machine's declaration can say
 	// whom it does not serve, and why, beside what it does.
 	Withheld []Overflow
+	// Unbound is every consumer whose pair credential from this node is on record and whose own
+	// resolution binds it elsewhere (novox/hq issue 274): never granted, carried to be said.
+	Unbound []Unbound
 
 	// Ports is where this machine puts what each module needs reachable, by module and by the
 	// port the software itself uses (novox/hq ADR 0038).