diff --git a/modules/gitea/cmd/agent-token/token.go b/modules/gitea/cmd/agent-token/token.go index 8b56fc37..95b708d7 100644 --- a/modules/gitea/cmd/agent-token/token.go +++ b/modules/gitea/cmd/agent-token/token.go @@ -195,7 +195,7 @@ func (m *Minter) Answer(ask Ask) (Answer, error) { skip(h, "the forge names no origin it is reached at from that machine") continue } - account := forgeaccount.ForgeUser(h.Node, "machine") + account := forgeaccount.NodeAccount(h.Node) if err := m.accounts().EnsureAccount(ctx, account); err != nil { return Answer{}, err } diff --git a/modules/gitea/cmd/forge-account-provider/bus_test.go b/modules/gitea/cmd/forge-account-provider/bus_test.go index a912e92e..996a1816 100644 --- a/modules/gitea/cmd/forge-account-provider/bus_test.go +++ b/modules/gitea/cmd/forge-account-provider/bus_test.go @@ -314,12 +314,12 @@ func TestTheProviderWholeAnswersAMachineThatReportedItsKey(t *testing.T) { e, ok = rt.boundEntry("g14.ssh-client.forge-account") return ok }) - if e.Values["user"] != "agent-g14" || e.Values["ssh_host"] != "novox.internal" || + if e.Values["user"] != "nox-agent" || e.Values["ssh_host"] != "novox.internal" || e.Values["known_host"] != "[novox.internal]:222 ssh-ed25519 AAAAC3HOSTKEY" || e.Values["host_key"] != "ssh-ed25519 AAAAC3HOSTKEY" || e.Values["ssh_alias"] != "mesh-forge" || e.Values["ssh_base"] != "ssh://git@novox.internal:222/" || e.Values["https_base"] != "https://git.novox.be/" { t.Fatalf("values = %v", e.Values) } - if got := forge.titles("agent-g14"); len(got) != 1 || got[0] != "mesh:g14.nox" { + if got := forge.titles("nox-agent"); len(got) != 1 || got[0] != "mesh:g14.nox" { t.Fatalf("registered keys = %v", got) } @@ -329,7 +329,7 @@ func TestTheProviderWholeAnswersAMachineThatReportedItsKey(t *testing.T) { eventually(t, "the new key registered and the old one gone", func() bool { forge.mu.Lock() defer forge.mu.Unlock() - k := forge.held["agent-g14"] + k := forge.held["nox-agent"] return len(k) == 1 && strings.HasPrefix(k[0]["key"].(string), pubB) }) } diff --git a/modules/gitea/cmd/forge-account-provider/main.go b/modules/gitea/cmd/forge-account-provider/main.go index aa4b7c56..9585ac0a 100644 --- a/modules/gitea/cmd/forge-account-provider/main.go +++ b/modules/gitea/cmd/forge-account-provider/main.go @@ -2,7 +2,7 @@ // the node's runtime launches and speaks MCP to over stdio, beside the module's other bundles. // // For each machine whose ssh-client binds the provision and has reported its public key (ssh-client's state `keys`), -// it makes the machine's agent a forge account of its own (agent-, in the agents team), registers the key for +// it makes the machine's agent the one forge user nox-agent (in the agents team; ADR 0410), registers the key for // it, and serves where and as whom to reach the forge over ssh: the user, the forge machine's mesh name and port, its // host key and the two url roots. A public key is no secret, so the provision has no credential. // diff --git a/modules/gitea/cmd/forge-account-provider/service.go b/modules/gitea/cmd/forge-account-provider/service.go index 49fe84a5..b9f33717 100644 --- a/modules/gitea/cmd/forge-account-provider/service.go +++ b/modules/gitea/cmd/forge-account-provider/service.go @@ -69,6 +69,8 @@ type Service struct { bound provider.Bus // the bus the entries are written to (set by Provider) + letGo map[string]bool // nodes whose per-node user of before has been said to be let go + making sync.Mutex // one Make at a time: they register keys, and two at once would race each other } @@ -81,7 +83,7 @@ func NewService(cfg Config) *Service { if cfg.Log == nil { cfg.Log = func(f string, a ...any) { fmt.Fprintf(os.Stderr, "[gitea forge-account] "+f+"\n", a...) } } - return &Service{cfg: cfg, keys: map[string]machineKey{}, Pause: time.Sleep, + return &Service{cfg: cfg, keys: map[string]machineKey{}, letGo: map[string]bool{}, Pause: time.Sleep, accounts: &forgeaccount.Accounts{Forge: cfg.Forge, Log: cfg.Log}} } @@ -127,9 +129,12 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error { } have := s.keysOf(node) if len(have) == 0 { + if err := s.releasePerNodeUser(ctx, node); err != nil { + return err + } // A key the machine no longer reports is no longer wanted (ADR 0400 §2): the mesh's keys go, the operator's // stay, and the entry that served the machine goes with them. - if err := s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.TitlePrefix+node+"."); err != nil { + if _, err := s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.MachineScope(node)); err != nil { return err } if s.bound != nil { @@ -156,7 +161,13 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error { for _, k := range have { wanted = append(wanted, forgeaccount.WantedKey{Title: forgeaccount.KeyTitle(k.Node, k.Account), Key: k.Public}) } - if err := s.accounts.EnsureKeys(ctx, user, wanted); err != nil { + // The forge holds one public key on one user only, so the key leaves agent- before it joins nox-agent. That + // is done here, after everything that can fail before it, so the window in which the machine's key is on no user + // is only the span between these two calls; if the add fails, the next pass (at most ten minutes) puts it back. + if err := s.releasePerNodeUser(ctx, node); err != nil { + return err + } + if err := s.accounts.EnsureKeys(ctx, user, forgeaccount.MachineScope(node), wanted); err != nil { return err } s.mu.Lock() @@ -175,7 +186,37 @@ func (s *Service) Gone(ctx context.Context, e provider.Entry) error { if user == "" { return nil } - return s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.TitlePrefix+e.Consumer+".") + if err := s.releasePerNodeUser(ctx, e.Consumer); err != nil { + return err + } + _, err := s.accounts.RemoveKeysTitled(ctx, user, forgeaccount.MachineScope(e.Consumer)) + return err +} + +// releasePerNodeUser lets go the per-node user (agent-) a machine had before one user held every machine's keys +// (ADR 0410 §3): the mesh keys registered on it are removed. The user is never +// deleted and stays in the agents team while agent-token mints it a token, and the keys of the operator's own stay. It is said once per machine. +func (s *Service) releasePerNodeUser(ctx context.Context, node string) error { + old := forgeaccount.NodeAccount(node) + if !forgeaccount.NodeName.MatchString(node) || old == forgeaccount.AgentUser { + return nil + } + removed, err := s.accounts.RemoveKeysTitled(ctx, old, forgeaccount.MachineScope(node)) + if err != nil && !errors.Is(err, forgehttp.ErrNotFound) { + return err + } + if removed == 0 { + return nil // never existed, or holds no mesh key of the machine's: nothing was let go, nothing is said + } + s.mu.Lock() + said := s.letGo[node] + s.letGo[node] = true + s.mu.Unlock() + if !said { + s.cfg.Log("let go the forge user %s of before: its mesh keys are removed; the user "+ + "is not deleted and stays in the agents team while its token is minted (the agents of every machine are now %s)", old, forgeaccount.AgentUser) + } + return nil } // KeyChanged takes in one machine key as the bus keeps it, and tells OnKeys its node. diff --git a/modules/gitea/cmd/forge-account-provider/service_test.go b/modules/gitea/cmd/forge-account-provider/service_test.go index 61b8404e..8f3df6ae 100644 --- a/modules/gitea/cmd/forge-account-provider/service_test.go +++ b/modules/gitea/cmd/forge-account-provider/service_test.go @@ -127,6 +127,14 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { case http.MethodGet: reply(200, f.held[m[1]]) case http.MethodPost: + for user, ks := range f.held { + for _, k := range ks { + if user != m[1] && fmt.Sprint(k["key"]) != "" && strings.Fields(fmt.Sprint(k["key"]))[1] == strings.Fields(fmt.Sprint(in["key"]))[1] { + reply(422, map[string]any{"message": "Key content has been used as non-deploy key"}) + return + } + } + } f.next++ f.held[m[1]] = append(f.held[m[1]], map[string]any{"id": f.next, "title": in["title"], "key": in["key"]}) reply(201, map[string]any{"id": f.next}) @@ -157,7 +165,24 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { case p == "/api/v1/teams/5" && r.Method == http.MethodPatch: reply(200, map[string]any{}) case strings.HasPrefix(p, "/api/v1/teams/5/members/") && r.Method == http.MethodPut: - f.members = append(f.members, strings.TrimPrefix(p, "/api/v1/teams/5/members/")) + in := strings.TrimPrefix(p, "/api/v1/teams/5/members/") + for _, m := range f.members { + if m == in { + reply(204, nil) + return + } + } + f.members = append(f.members, in) + reply(204, nil) + case strings.HasPrefix(p, "/api/v1/teams/5/members/") && r.Method == http.MethodDelete: + gone := strings.TrimPrefix(p, "/api/v1/teams/5/members/") + var kept []string + for _, m := range f.members { + if m != gone { + kept = append(kept, m) + } + } + f.members = kept reply(204, nil) default: http.Error(w, "unexpected "+r.Method+" "+p, 400) @@ -239,7 +264,7 @@ func TestAConsumerWithAKeyGetsItsEntryAndItsKeyIsRegistered(t *testing.T) { r.key("g14", "nox", pubA+" nox@g14 (mesh)") r.pass() e := r.bus.entry(t, "g14.ssh-client.forge-account") - want := map[string]any{"user": "agent-g14", "ssh_host": "novox.internal", "ssh_port": float64(222), + want := map[string]any{"user": "nox-agent", "ssh_host": "novox.internal", "ssh_port": float64(222), "known_host": "[novox.internal]:222 ssh-ed25519 AAAAC3HOSTKEY", "ssh_alias": "mesh-forge", "host_key": "ssh-ed25519 AAAAC3HOSTKEY", "ssh_base": "ssh://git@novox.internal:222/", "https_base": "https://git.novox.be/"} @@ -251,10 +276,10 @@ func TestAConsumerWithAKeyGetsItsEntryAndItsKeyIsRegistered(t *testing.T) { if e.VaultName != "" || e.User != "" { t.Errorf("a public key is no secret; the entry names a credential: %+v", e) } - if got := r.forge.titles("agent-g14"); len(got) != 1 || got[0] != "mesh:g14.nox" { + if got := r.forge.titles("nox-agent"); len(got) != 1 || got[0] != "mesh:g14.nox" { t.Errorf("registered keys = %v", got) } - if !r.forge.users["agent-g14"] || len(r.forge.members) != 1 || r.forge.members[0] != "agent-g14" { + if !r.forge.users["nox-agent"] || len(r.forge.members) != 1 || r.forge.members[0] != "nox-agent" { t.Errorf("account %v, team members %v", r.forge.users, r.forge.members) } if strings.Join(r.runner.args, " ") != "ssh-keyscan -t ed25519 -p 222 127.0.0.1" { @@ -295,15 +320,15 @@ func TestAChangedKeyReplacesTheOldMeshKey(t *testing.T) { r.key("g14", "nox", pubA+" nox@g14 (mesh)") r.pass() r.forge.mu.Lock() - r.forge.held["agent-g14"] = append(r.forge.held["agent-g14"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) + r.forge.held["nox-agent"] = append(r.forge.held["nox-agent"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) r.forge.mu.Unlock() r.key("g14", "nox", "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC nox@g14 (mesh)") r.pass() - got := strings.Join(r.forge.titles("agent-g14"), ",") + got := strings.Join(r.forge.titles("nox-agent"), ",") if got != "my own,mesh:g14.nox" { t.Fatalf("keys = %s", got) } - for _, k := range r.forge.held["agent-g14"] { + for _, k := range r.forge.held["nox-agent"] { if k["title"] == "mesh:g14.nox" && !strings.Contains(k["key"].(string), "CCCC") { t.Fatalf("the old key stayed: %v", k) } @@ -316,7 +341,7 @@ func TestAConsumerThatNoLongerBindsLosesItsMeshKeysAndKeepsItsAccount(t *testing r.key("g14", "nox", pubA+" nox@g14 (mesh)") r.pass() r.forge.mu.Lock() - r.forge.held["agent-g14"] = append(r.forge.held["agent-g14"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) + r.forge.held["nox-agent"] = append(r.forge.held["nox-agent"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) r.forge.mu.Unlock() r.bus.mu.Lock() delete(r.bus.buckets[provider.AssignmentsBucket], "g14.ssh-client") @@ -325,10 +350,10 @@ func TestAConsumerThatNoLongerBindsLosesItsMeshKeysAndKeepsItsAccount(t *testing if r.bus.has("g14.ssh-client.forge-account") { t.Fatal("the entry stayed") } - if got := r.forge.titles("agent-g14"); len(got) != 1 || got[0] != "my own" { + if got := r.forge.titles("nox-agent"); len(got) != 1 || got[0] != "my own" { t.Fatalf("keys = %v", got) } - if !r.forge.users["agent-g14"] { + if !r.forge.users["nox-agent"] { t.Fatal("the account was removed") } } @@ -428,11 +453,11 @@ func TestADeletedKeyEntryRemovesTheMeshKeyAndTheBoundEntryButNotTheOperatorsKey( r.key("g14", "nox", pubA+" nox@g14 (mesh)") r.pass() r.forge.mu.Lock() - r.forge.held["agent-g14"] = append(r.forge.held["agent-g14"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) + r.forge.held["nox-agent"] = append(r.forge.held["nox-agent"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) r.forge.mu.Unlock() r.svc.KeyGone("g14.nox") r.pass() - if got := r.forge.titles("agent-g14"); len(got) != 1 || got[0] != "my own" { + if got := r.forge.titles("nox-agent"); len(got) != 1 || got[0] != "my own" { t.Fatalf("keys = %v", got) } if r.bus.has("g14.ssh-client.forge-account") { @@ -446,7 +471,7 @@ func TestAPassBeforeTheKeysAreLoadedRemovesAndDeletesNothing(t *testing.T) { r.key("g14", "nox", pubA+" nox@g14 (mesh)") r.pass() r.forge.mu.Lock() - r.forge.held["agent-g14"] = append(r.forge.held["agent-g14"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) + r.forge.held["nox-agent"] = append(r.forge.held["nox-agent"], map[string]any{"id": int64(99), "title": "my own", "key": pubB}) r.forge.mu.Unlock() // A restart: the keys are not read yet, so a node with none is not a node that reported none. r.svc.mu.Lock() @@ -454,7 +479,7 @@ func TestAPassBeforeTheKeysAreLoadedRemovesAndDeletesNothing(t *testing.T) { r.svc.loaded = false r.svc.mu.Unlock() out := r.pass() - if got := strings.Join(r.forge.titles("agent-g14"), ","); got != "mesh:g14.nox,my own" { + if got := strings.Join(r.forge.titles("nox-agent"), ","); got != "mesh:g14.nox,my own" { t.Fatalf("keys = %s", got) } if !r.bus.has("g14.ssh-client.forge-account") { @@ -468,7 +493,7 @@ func TestAPassBeforeTheKeysAreLoadedRemovesAndDeletesNothing(t *testing.T) { r.svc.loaded = true r.svc.mu.Unlock() r.pass() - if got := strings.Join(r.forge.titles("agent-g14"), ","); got != "my own" || r.bus.has("g14.ssh-client.forge-account") { + if got := strings.Join(r.forge.titles("nox-agent"), ","); got != "my own" || r.bus.has("g14.ssh-client.forge-account") { t.Fatalf("keys = %s", got) } } @@ -541,3 +566,133 @@ func TestKeepRunningBacksOffToTwoMinutes(t *testing.T) { t.Fatalf("the backoff is %v", pauses) } } + +// The per-node users made before (ADR 0410 §3) are let go: their mesh keys and their team seat go, the user and the +// operator's own keys stay, and it is said once. +func TestThePerNodeUserMadeBeforeIsLetGoButNeverDeleted(t *testing.T) { + r := newRig(t) + var said []string + r.svc.cfg.Log = func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) } + r.bus.bindClient("g14") + r.key("g14", "nox", pubA+" nox@g14 (mesh)") + r.forge.mu.Lock() + r.forge.users["agent-g14"] = true + r.forge.members = []string{"agent-g14", "agent-ace"} + r.forge.held["agent-g14"] = []map[string]any{ + {"id": int64(90), "title": "mesh:g14.nox", "key": pubA}, {"id": int64(91), "title": "my own", "key": pubB}} + r.forge.held["agent-ace"] = []map[string]any{{"id": int64(92), "title": "mesh:ace.nox", "key": pubB}} + r.forge.mu.Unlock() + r.pass() + r.pass() + if got := r.forge.titles("agent-g14"); len(got) != 1 || got[0] != "my own" { + t.Fatalf("the old user's keys = %v", got) + } + if got := r.forge.titles("agent-ace"); len(got) != 1 { + t.Fatalf("another machine's old user was touched: %v", got) + } + if got := strings.Join(r.forge.members, ","); got != "agent-g14,agent-ace,nox-agent" { + t.Fatalf("team = %s", got) + } + if !r.forge.users["agent-g14"] { + t.Fatal("the old user was deleted") + } + n := 0 + for _, l := range said { + if strings.Contains(l, "agent-g14") && strings.Contains(l, "let go") { + n++ + } + } + if n != 1 { + t.Fatalf("said %d times: %v", n, said) + } +} + +// A key on nox-agent that is not titled `mesh:` survives every pass: key kept, key changed, key gone (ADR 0410 §2). +func TestAKeyOnTheAgentUserTitledOtherwiseSurvivesEveryPass(t *testing.T) { + r := newRig(t) + r.bus.bindClient("g14") + r.key("g14", "nox", pubA+" nox@g14 (mesh)") + r.pass() + r.forge.mu.Lock() + r.forge.held["nox-agent"] = append(r.forge.held["nox-agent"], map[string]any{"id": int64(99), "title": "by hand", "key": pubB}) + r.forge.mu.Unlock() + r.pass() + r.key("g14", "nox", pubB+" nox@g14 (mesh)") // the machine's key changed to the one held by hand + r.pass() + r.svc.KeyGone("g14.nox") + r.pass() + got := r.forge.titles("nox-agent") + if len(got) != 1 || got[0] != "by hand" { + t.Fatalf("keys = %v", got) + } +} + +// Letting a per-node user go is said once per user across passes (ADR 0410 §3). +func TestLettingEachPerNodeUserGoIsSaidOnceAcrossPasses(t *testing.T) { + r := newRig(t) + var said []string + r.svc.cfg.Log = func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) } + for n, pub := range map[string]string{"g14": pubA, "ace": pubB} { + r.bus.bindClient(n) + r.key(n, "nox", pub+" nox@"+n+" (mesh)") + r.forge.held["agent-"+n] = []map[string]any{{"id": int64(80 + len(n)), "title": "mesh:" + n + ".nox", "key": pub}} + } + r.pass() + r.pass() + r.pass() + count := map[string]int{} + for _, l := range said { + for _, u := range []string{"agent-g14", "agent-ace"} { + if strings.Contains(l, u) && strings.Contains(l, "let go") { + count[u]++ + } + } + } + if count["agent-g14"] != 1 || count["agent-ace"] != 1 { + t.Fatalf("said %v: %v", count, said) + } +} + +// Gitea holds one public key on one user only (ADR 0410 §3): the key leaves agent- before it joins nox-agent. +func TestAMachinesKeyMovesFromItsOldUserToTheAgentUser(t *testing.T) { + r := newRig(t) + r.bus.bindClient("g14") + r.key("g14", "nox", pubA+" nox@g14 (mesh)") + r.forge.held["agent-g14"] = []map[string]any{{"id": int64(90), "title": "mesh:g14.nox", "key": pubA}} + out := r.pass() + if got := r.forge.titles("agent-g14"); len(got) != 0 { + t.Fatalf("old user keys = %v (pass: %v)", got, out) + } + if got := r.forge.titles("nox-agent"); len(got) != 1 || got[0] != "mesh:g14.nox" { + t.Fatalf("nox-agent keys = %v (pass: %v)", got, out) + } +} + +// The old user lets the key go only just before nox-agent takes it: a pass that fails earlier leaves the key where it +// is, so the machine is never on no user (ADR 0410 §3). +func TestAPassThatFailsBeforeTheKeyIsAddedLeavesTheOldUsersKey(t *testing.T) { + r := newRig(t) + r.runner.err = errors.New("sshd down") + r.bus.bindClient("g14") + r.key("g14", "nox", pubA+" nox@g14 (mesh)") + r.forge.held["agent-g14"] = []map[string]any{{"id": int64(90), "title": "mesh:g14.nox", "key": pubA}} + r.pass() + if got := r.forge.titles("agent-g14"); len(got) != 1 { + t.Fatalf("old user keys = %v", got) + } +} + +// Nothing is said for a user that never existed or held no mesh key of the machine's. +func TestNothingIsSaidForAPerNodeUserWithoutMeshKeys(t *testing.T) { + r := newRig(t) + var said []string + r.svc.cfg.Log = func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) } + r.bus.bindClient("g14") + r.key("g14", "nox", pubA+" nox@g14 (mesh)") + r.pass() + for _, l := range said { + if strings.Contains(l, "let go") { + t.Fatalf("said %q", l) + } + } +} diff --git a/modules/gitea/internal/forgeaccount/account.go b/modules/gitea/internal/forgeaccount/account.go index 6c36381b..0b3af3d0 100644 --- a/modules/gitea/internal/forgeaccount/account.go +++ b/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-` 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-` (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 } diff --git a/modules/gitea/internal/forgeaccount/keys.go b/modules/gitea/internal/forgeaccount/keys.go index 2f5a43f6..68387ea5 100644 --- a/modules/gitea/internal/forgeaccount/keys.go +++ b/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:.`: 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:.`) 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 } diff --git a/modules/gitea/internal/forgeaccount/keys_test.go b/modules/gitea/internal/forgeaccount/keys_test.go index 105c871a..42a7860e 100644 --- a/modules/gitea/internal/forgeaccount/keys_test.go +++ b/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) + } +} diff --git a/modules/gitea/internal/forgeaccount/user_test.go b/modules/gitea/internal/forgeaccount/user_test.go index 82875058..d1df3f91 100644 --- a/modules/gitea/internal/forgeaccount/user_test.go +++ b/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 {