Merge pull request #5613 from oasisprotocol/jberci/fix/observers

go/worker/keymanager: Fix node ACL management
oasisprotocol · Mar 30, 2024 · 6c420cf · 6c420cf
2 parents 4f80a62 + 1c14a33
commit 6c420cf
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 30 deletions.
diff --git a/.changelog/5613.trivial.md b/.changelog/5613.trivial.md
@@ -0,0 +1,5 @@
+go/worker/keymanager: Fix node ACL management
+
+Compute nodes from any previous committees should be removed from an ACL
+even if there was an error retrieving metadata for the latest
+committees.
diff --git a/go/worker/keymanager/watcher.go b/go/worker/keymanager/watcher.go
@@ -294,6 +294,7 @@ func (w *rtNodeWatcher) watch(ctx context.Context) {
 	defer nodeSub.Close()
 
 	// And populate the list of observer nodes with the currently known set of them.
+	// epoCh will get the current epoch immediately, so no need to populate compute nodes specially.
 	nodes, err := w.consensus.Registry().GetNodes(ctx, consensus.HeightLatest)
 	if err != nil {
 		w.logger.Error("failed to fetch list of nodes from the registry",
@@ -313,41 +314,41 @@ func (w *rtNodeWatcher) watch(ctx context.Context) {
 		case <-ctx.Done():
 			return
 		case <-epoCh:
-			func() {
-				// Get executor committee members.
-				cms, err := w.consensus.Scheduler().GetCommittees(ctx, &scheduler.GetCommitteesRequest{
-					Height:    consensus.HeightLatest,
-					RuntimeID: w.runtimeID,
-				})
-				if err != nil {
-					w.logger.Error("failed to fetch runtime committee",
-						"err", err,
-					)
-					return
+			// Old members need to be cleared out of the ACL even in case of errors.
+			clear(computeNodes)
+
+			// Get executor committee members.
+			cms, err := w.consensus.Scheduler().GetCommittees(ctx, &scheduler.GetCommitteesRequest{
+				Height:    consensus.HeightLatest,
+				RuntimeID: w.runtimeID,
+			})
+			if err != nil {
+				w.logger.Error("failed to fetch runtime committee",
+					"err", err,
+				)
+				break
+			}
+
+			for _, cm := range cms {
+				if cm.Kind != scheduler.KindComputeExecutor {
+					continue
 				}
-				clear(computeNodes)
 
-				for _, cm := range cms {
-					if cm.Kind != scheduler.KindComputeExecutor {
+				for _, member := range cm.Members {
+					nd, err := w.consensus.Registry().GetNode(ctx, &registry.IDQuery{
+						ID:     member.PublicKey,
+						Height: consensus.HeightLatest,
+					})
+					if err != nil {
+						w.logger.Error("failed to fetch node descriptor for committee member",
+							"err", err,
+							"member", member.PublicKey,
+						)
 						continue
 					}
-
-					for _, member := range cm.Members {
-						nd, err := w.consensus.Registry().GetNode(ctx, &registry.IDQuery{
-							ID:     member.PublicKey,
-							Height: consensus.HeightLatest,
-						})
-						if err != nil {
-							w.logger.Error("failed to fetch node descriptor for committee member",
-								"err", err,
-								"member", member.PublicKey,
-							)
-							continue
-						}
-						computeNodes[nd.ID] = nd
-					}
+					computeNodes[nd.ID] = nd
 				}
-			}()
+			}
 		case ne := <-nodeCh:
 			switch ne.IsRegistration {
 			case true: