Do not remove keys or entries before the machines' keys are read (issue 955 F)

modules/gitea/cmd/forge-account-provider/service.go

@@ -62,9 +62,10 @@ type Service struct {
 	// Pause waits between asks of a watch the runtime does not answer yet.
 	Pause func(time.Duration)
 
-	mu    sync.Mutex
-	keys  map[string]machineKey
-	known string // the forge's host key line, as the machines write it into known_hosts
+	mu     sync.Mutex
+	keys   map[string]machineKey
+	loaded bool   // the machines' keys have been read: until then a node with none has not reported none
+	known  string // the forge's host key line, as the machines write it into known_hosts
 
 	bound provider.Bus // the bus the entries are written to (set by Provider)
 
@@ -117,6 +118,13 @@ func (s *Service) Make(ctx context.Context, c provider.Consumer) error {
 	if user == "" {
 		return fmt.Errorf("%q is not a machine's name an account can be named for", node)
 	}
+	s.mu.Lock()
+	loaded := s.loaded
+	s.mu.Unlock()
+	if !loaded {
+		// After a restart the first pass can come before the keys arrive: nothing is removed or deleted on that.
+		return errors.New("the machines' keys are not read yet, so nothing is made, removed or deleted")
+	}
 	have := s.keysOf(node)
 	if len(have) == 0 {
 		// A key the machine no longer reports is no longer wanted (ADR 0400 §2): the mesh's keys go, the operator's
@@ -253,6 +261,10 @@ func (s *Service) WatchKeys(ctx context.Context, src KeySource) error {
 			}
 		})
 		if err == nil {
+			// The watch answers once the current values are all handled (stdio KeptState.Watch).
+			s.mu.Lock()
+			s.loaded = true
+			s.mu.Unlock()
 			return nil
 		}
 		pause := time.Minute

modules/gitea/cmd/forge-account-provider/service_test.go

@@ -213,6 +213,7 @@ func newRig(t *testing.T) *rig {
 	svc := NewService(Config{Node: "novox", HostFile: host, OriginsFile: origins, Port: 222,
 		Forge: forgehttp.Client{URL: srv.URL, User: "mesh-admin", PasswordFile: pw}, Runner: runner,
 		Log: func(string, ...any) {}})
+	svc.loaded = true // the machines' keys are read; a test that means otherwise says so
 	return &rig{t: t, bus: newBus(), forge: forge, svc: svc, runner: runner}
 }
 
@@ -377,11 +378,15 @@ func TestKeysWatchedRetriesUntilItAnswersAndPassesOnChanges(t *testing.T) {
 	r := newRig(t)
 	src := &fakeKeys{failFirst: 2}
 	r.svc.Pause = func(time.Duration) {}
+	r.svc.loaded = false
 	passes := 0
 	r.svc.OnKeys = func(string) { passes++ }
 	if err := r.svc.WatchKeys(context.Background(), src); err != nil {
 		t.Fatal(err)
 	}
+	if !r.svc.loaded {
+		t.Fatal("the keys are not loaded once the watch has answered")
+	}
 	if src.calls != 3 {
 		t.Fatalf("asked %d times", src.calls)
 	}
@@ -433,3 +438,36 @@ func TestADeletedKeyEntryRemovesTheMeshKeyAndTheBoundEntryButNotTheOperatorsKey(
 		t.Fatal("the entry still serves a machine whose key is gone")
 	}
 }
+
+func TestAPassBeforeTheKeysAreLoadedRemovesAndDeletesNothing(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["agent-g14"] = append(r.forge.held["agent-g14"], 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()
+	r.svc.keys = map[string]machineKey{}
+	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" {
+		t.Fatalf("keys = %s", got)
+	}
+	if !r.bus.has("g14.ssh-client.forge-account") {
+		t.Fatal("the entry was deleted before the keys were read")
+	}
+	if len(out) != 1 || !strings.Contains(out[0].Why, "not read yet") {
+		t.Fatalf("the wait should be said: %+v", out)
+	}
+	// Loaded, and the node reports none: now its keys go.
+	r.svc.mu.Lock()
+	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") {
+		t.Fatalf("keys = %s", got)
+	}
+}