One forge user, nox-agent, for every machine's agents' SSH pushes, each machine's key titled by machine (issue 991, ADR 0410)

modules/gitea/internal/forgeaccount/account.go

@@ -87,23 +87,9 @@ func (m *Accounts) EnsureAccount(ctx context.Context, account string) error {
 // EnsureInTeam makes the agents team when there is none, puts a team that is there back to its shape, and adds the
 // account to it; an account already in it stays (ADR 0398 §1, issue 948).
 func (m *Accounts) EnsureInTeam(ctx context.Context, account string) error {
-	var id int64
-	for page := 1; page <= 20 && id == 0; page++ {
-		var teams []struct {
-			ID   int64  `json:"id"`
-			Name string `json:"name"`
-		}
-		if err := m.Forge.GetJSON(ctx, fmt.Sprintf("/api/v1/orgs/%s/teams?limit=50&page=%d", TeamOrg, page), &teams); err != nil {
-			return fmt.Errorf("the teams of %s could not be read: %w", TeamOrg, err)
-		}
-		for _, t := range teams {
-			if t.Name == TeamName {
-				id = t.ID
-			}
-		}
-		if len(teams) < 50 {
-			break
-		}
+	id, err := m.teamID(ctx)
+	if err != nil {
+		return err
 	}
 	if id != 0 {
 		if err := m.Forge.SendJSON(ctx, http.MethodPatch, fmt.Sprintf("/api/v1/teams/%d", id), teamShape(), nil); err != nil {
@@ -128,6 +114,29 @@ func (m *Accounts) EnsureInTeam(ctx context.Context, account string) error {
 	return nil
 }
 
+// teamID is the agents team's id, or 0 when there is none.
+func (m *Accounts) teamID(ctx context.Context) (int64, error) {
+	var id int64
+	for page := 1; page <= 20 && id == 0; page++ {
+		var teams []struct {
+			ID   int64  `json:"id"`
+			Name string `json:"name"`
+		}
+		if err := m.Forge.GetJSON(ctx, fmt.Sprintf("/api/v1/orgs/%s/teams?limit=50&page=%d", TeamOrg, page), &teams); err != nil {
+			return 0, fmt.Errorf("the teams of %s could not be read: %w", TeamOrg, err)
+		}
+		for _, t := range teams {
+			if t.Name == TeamName {
+				id = t.ID
+			}
+		}
+		if len(teams) < 50 {
+			break
+		}
+	}
+	return id, nil
+}
+
 func (m *Accounts) password() string {
 	if m.Password != nil {
 		return m.Password()
@@ -148,11 +157,19 @@ func (m *Accounts) say(format string, a ...any) {
 // NodeName is what a machine's name is, for it to name an account.
 var NodeName = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,31}$`)
 
-// ForgeUser is the forge user of a machine's key of the given kind (ADR 0400 §3): `agent-<node>` for the machine's
-// own account (kind machine). A kind not built yet has none, and the answer is empty.
+// AgentUser is the one forge user of every machine's agents (ADR 0410 §1), in the agents write team.
+const AgentUser = "nox-agent"
+
+// ForgeUser is the forge user of a machine's key of the given kind (ADR 0410 §1): `nox-agent` for every machine
+// and account (kind machine). A kind not built yet has none, and the answer is empty. The machine's name must still
+// be one an account could be named for.
 func ForgeUser(node, kind string) string {
 	if kind == "machine" && NodeName.MatchString(node) {
-		return AccountPrefix + node
+		return AgentUser
 	}
 	return ""
 }
+
+// NodeAccount is the forge account the token tool (cmd/agent-token) keeps for a node's agent: `agent-<node>` (ADR
+// 0365 §1). It is not the forge user of the machine's ssh key (ForgeUser), and its behaviour is unchanged.
+func NodeAccount(node string) string { return AccountPrefix + node }

modules/gitea/internal/forgeaccount/keys.go

@@ -75,11 +75,18 @@ func (m *Accounts) DeleteKey(ctx context.Context, user string, id int64) error {
 	return nil
 }
 
+// MachineScope is the title prefix of one machine's keys, `mesh:<node>.`: what a pass for that machine may remove.
+func MachineScope(node string) string { return TitlePrefix + node + "." }
+
 // EnsureKeys registers each wanted key the account does not hold and removes the keys the mesh registered
-// (title starting `mesh:`) that are no longer wanted. A key whose title does not start `mesh:` is the operator's and
+// for one machine (title starting scope, `mesh:<node>.`) that are no longer wanted. One forge user holds every
+// machine's keys (ADR 0410 §2), so a key under another machine's title is never removed here. A key whose title does not start `mesh:` is the operator's and
 // is never touched, and a wanted key the account already holds under another title is left as it is (ADR 0399 §2).
 // A key is added before another title's is removed, so an account is not left without one.
-func (m *Accounts) EnsureKeys(ctx context.Context, user string, wanted []WantedKey) error {
+func (m *Accounts) EnsureKeys(ctx context.Context, user, scope string, wanted []WantedKey) error {
+	if !strings.HasPrefix(scope, TitlePrefix) {
+		return fmt.Errorf("the scope %q is not a mesh key title prefix", scope)
+	}
 	held, err := m.Keys(ctx, user)
 	if err != nil {
 		return err
@@ -110,7 +117,7 @@ func (m *Accounts) EnsureKeys(ctx context.Context, user string, wanted []WantedK
 		}
 		// The forge holds a title once: the mesh's key of the same title, now another key, goes first.
 		for _, k := range held {
-			if k.Title == w.Title && strings.HasPrefix(k.Title, TitlePrefix) && !want[material(k.Key)] {
+			if k.Title == w.Title && strings.HasPrefix(k.Title, scope) && !want[material(k.Key)] {
 				if err := remove(k); err != nil {
 					return err
 				}
@@ -122,7 +129,7 @@ func (m *Accounts) EnsureKeys(ctx context.Context, user string, wanted []WantedK
 		m.say("registered the key %s for %s", w.Title, user)
 	}
 	for _, k := range held {
-		if strings.HasPrefix(k.Title, TitlePrefix) && !want[material(k.Key)] {
+		if strings.HasPrefix(k.Title, scope) && !want[material(k.Key)] {
 			if err := remove(k); err != nil {
 				return err
 			}
@@ -133,23 +140,25 @@ func (m *Accounts) EnsureKeys(ctx context.Context, user string, wanted []WantedK
 
 // RemoveKeysTitled removes the account's keys whose title starts with prefix, and only a prefix the mesh gave
 // (starting `mesh:`, such as `mesh:g14.`): what the mesh registered for one machine, never a key of the operator's
-// (ADR 0399 §2).
-func (m *Accounts) RemoveKeysTitled(ctx context.Context, user, prefix string) error {
+// (ADR 0399 §2). It answers how many keys it removed.
+func (m *Accounts) RemoveKeysTitled(ctx context.Context, user, prefix string) (int, error) {
 	if !strings.HasPrefix(prefix, TitlePrefix) {
-		return nil
+		return 0, nil
 	}
 	held, err := m.Keys(ctx, user)
 	if err != nil {
-		return err
+		return 0, err
 	}
+	removed := 0
 	for _, k := range held {
 		if !strings.HasPrefix(k.Title, prefix) {
 			continue
 		}
 		if err := m.DeleteKey(ctx, user, k.ID); err != nil {
-			return err
+			return removed, err
 		}
+		removed++
 		m.say("removed the key %s of %s: its machine no longer binds the forge", k.Title, user)
 	}
-	return nil
+	return removed, nil
 }

modules/gitea/internal/forgeaccount/keys_test.go

@@ -108,7 +108,7 @@ func keyAccounts(t *testing.T, f *keyForge) *Accounts {
 func TestEnsureKeysAddsTheWantedKeyUnderItsTitle(t *testing.T) {
 	f := &keyForge{keys: map[string][]heldKey{}}
 	a := keyAccounts(t, f)
-	if err := a.EnsureKeys(t.Context(), "agent-g14", []WantedKey{{Title: "mesh:g14.nox", Key: keyA + " nox@g14 (mesh)"}}); err != nil {
+	if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", []WantedKey{{Title: "mesh:g14.nox", Key: keyA + " nox@g14 (mesh)"}}); err != nil {
 		t.Fatal(err)
 	}
 	if len(f.posts) != 1 || f.posts[0]["title"] != "mesh:g14.nox" || f.posts[0]["read_only"] != false ||
@@ -122,7 +122,7 @@ func TestEnsureKeysAgainChangesNothing(t *testing.T) {
 	a := keyAccounts(t, f)
 	want := []WantedKey{{Title: "mesh:g14.nox", Key: keyA + " nox@g14 (mesh)"}}
 	for range 2 {
-		if err := a.EnsureKeys(t.Context(), "agent-g14", want); err != nil {
+		if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", want); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -135,7 +135,7 @@ func TestEnsureKeysReplacesAChangedMeshKeyOfTheSameTitle(t *testing.T) {
 	// The forge refuses a title held twice, so the old key goes first.
 	f := &keyForge{keys: map[string][]heldKey{"agent-g14": {{ID: 7, Title: "mesh:g14.nox", Key: keyA}}}, next: 7}
 	a := keyAccounts(t, f)
-	if err := a.EnsureKeys(t.Context(), "agent-g14", []WantedKey{{Title: "mesh:g14.nox", Key: keyB + " nox@g14 (mesh)"}}); err != nil {
+	if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", []WantedKey{{Title: "mesh:g14.nox", Key: keyB + " nox@g14 (mesh)"}}); err != nil {
 		t.Fatal(err)
 	}
 	held := f.keys["agent-g14"]
@@ -147,7 +147,7 @@ func TestEnsureKeysReplacesAChangedMeshKeyOfTheSameTitle(t *testing.T) {
 func TestEnsureKeysRemovesAMeshKeyOfAnotherTitleAfterTheNewOneIsIn(t *testing.T) {
 	f := &keyForge{keys: map[string][]heldKey{"agent-g14": {{ID: 7, Title: "mesh:g14.old", Key: keyA}}}, next: 7}
 	a := keyAccounts(t, f)
-	if err := a.EnsureKeys(t.Context(), "agent-g14", []WantedKey{{Title: "mesh:g14.nox", Key: keyB}}); err != nil {
+	if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", []WantedKey{{Title: "mesh:g14.nox", Key: keyB}}); err != nil {
 		t.Fatal(err)
 	}
 	var post, del int
@@ -171,7 +171,7 @@ func TestEnsureKeysNeverTouchesAKeyTheOperatorAdded(t *testing.T) {
 	}}, next: 7}
 	a := keyAccounts(t, f)
 	// Nothing wanted any more: the mesh's key goes, the operator's stays.
-	if err := a.EnsureKeys(t.Context(), "agent-g14", nil); err != nil {
+	if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", nil); err != nil {
 		t.Fatal(err)
 	}
 	held := f.keys["agent-g14"]
@@ -183,10 +183,31 @@ func TestEnsureKeysNeverTouchesAKeyTheOperatorAdded(t *testing.T) {
 func TestEnsureKeysLeavesAWantedKeyTheOperatorAlreadyHoldsUnderAnotherTitle(t *testing.T) {
 	f := &keyForge{keys: map[string][]heldKey{"agent-g14": {{ID: 3, Title: "by hand", Key: keyA}}}, next: 3}
 	a := keyAccounts(t, f)
-	if err := a.EnsureKeys(t.Context(), "agent-g14", []WantedKey{{Title: "mesh:g14.nox", Key: keyA + " nox@g14 (mesh)"}}); err != nil {
+	if err := a.EnsureKeys(t.Context(), "agent-g14", "mesh:g14.", []WantedKey{{Title: "mesh:g14.nox", Key: keyA + " nox@g14 (mesh)"}}); err != nil {
 		t.Fatal(err)
 	}
 	if len(f.posts) != 0 || len(f.keys["agent-g14"]) != 1 || f.keys["agent-g14"][0].Title != "by hand" {
 		t.Fatalf("posts = %v, held = %v", f.posts, f.keys)
 	}
 }
+
+// One forge user holds every machine's key (ADR 0410 §2): a pass for one machine never removes another machine's.
+func TestEnsureKeysNeverRemovesAnotherMachinesMeshKey(t *testing.T) {
+	f := &keyForge{keys: map[string][]heldKey{"nox-agent": {
+		{ID: 1, Title: "mesh:ace.nox", Key: keyB},
+		{ID: 2, Title: "mesh:g14.nox", Key: keyA},
+		{ID: 3, Title: "mesh:g14.gone", Key: "ssh-ed25519 CCCC"},
+		{ID: 4, Title: "by hand", Key: "ssh-ed25519 DDDD"},
+	}}, next: 4}
+	a := keyAccounts(t, f)
+	if err := a.EnsureKeys(t.Context(), "nox-agent", "mesh:g14.", []WantedKey{{Title: "mesh:g14.nox", Key: keyA}}); err != nil {
+		t.Fatal(err)
+	}
+	var titles []string
+	for _, k := range f.keys["nox-agent"] {
+		titles = append(titles, k.Title)
+	}
+	if got := strings.Join(titles, ","); got != "mesh:ace.nox,mesh:g14.nox,by hand" {
+		t.Fatalf("held = %s: this machine's stale key goes, another machine's and the operator's stay", got)
+	}
+}

modules/gitea/internal/forgeaccount/user_test.go

@@ -2,13 +2,19 @@ package forgeaccount
 
 import "testing"
 
-func TestForgeUserOfAMachineAccountIsTheNodesAgent(t *testing.T) {
-	if got := ForgeUser("g14", "machine"); got != "agent-g14" {
+func TestForgeUserOfAMachineAccountIsTheOneAgentUser(t *testing.T) {
+	if got := ForgeUser("g14", "machine"); got != "nox-agent" {
 		t.Fatalf("ForgeUser = %q", got)
 	}
 	if got := ForgeUser("g14", "other"); got != "" {
 		t.Fatalf("a kind not built yet has no forge user, got %q", got)
 	}
+	if got := ForgeUser("ace", "machine"); got != "nox-agent" {
+		t.Fatalf("every machine shares one forge user, got %q", got)
+	}
+	if got := NodeAccount("g14"); got != "agent-g14" {
+		t.Fatalf("the token tool's per-node account = %q", got)
+	}
 }
 
 func TestRemoveKeysTitledTakesOnlyTheMeshKeysOfThatTitle(t *testing.T) {
@@ -18,13 +24,13 @@ func TestRemoveKeysTitledTakesOnlyTheMeshKeysOfThatTitle(t *testing.T) {
 		{ID: 3, Title: "by hand", Key: "ssh-ed25519 CCCC"},
 	}}, next: 3}
 	a := keyAccounts(t, f)
-	if err := a.RemoveKeysTitled(t.Context(), "agent-g14", "mesh:g14.nox"); err != nil {
+	if _, err := a.RemoveKeysTitled(t.Context(), "agent-g14", "mesh:g14.nox"); err != nil {
 		t.Fatal(err)
 	}
 	if got := f.keys["agent-g14"]; len(got) != 2 || got[0].ID != 2 || got[1].ID != 3 {
 		t.Fatalf("held = %v", got)
 	}
-	if err := a.RemoveKeysTitled(t.Context(), "agent-g14", "by hand"); err != nil {
+	if _, err := a.RemoveKeysTitled(t.Context(), "agent-g14", "by hand"); err != nil {
 		t.Fatal(err)
 	}
 	if len(f.keys["agent-g14"]) != 2 {