fix(#8077): the embedded HAServerPlugin.connectCluster API reports the cluster security seed

ha-raft/src/main/java/com/arcadedb/server/ha/raft/RaftHAPlugin.java

@@ -1028,6 +1028,10 @@ public void disconnectCluster() {
    * Not leader-routed, matching the add-peer route and {@code PostServerCommandHandler}, which forwards
    * neither half of the cluster pair: the Ratis client underneath {@code addPeer} sends the
    * configuration change to the leader itself.
+   * <p>
+   * The membership change alone: a residual security-seed failure is reported by
+   * {@link #connectClusterAndReportSeed}, which is what {@code ServerControlPlane.connectCluster} and an embedder
+   * that needs the outcome call (issue #8077).
    */
   @Override
   public void connectCluster(final String serverAddress) {
@@ -1048,6 +1052,20 @@ public void connectCluster(final String serverAddress) {
     raft.addPeer(target.peer(), target.name(), target.httpAddress());
   }
 
+  /**
+   * {@inheritDoc}
+   * <p>
+   * The join first, the seed report second, exactly as {@link #addPeerAndReportSeed} does it (issue #8077): the
+   * seed is asked of the leader, which already seeds every membership change of its own accord (issues #7531 and
+   * #7834), and nothing raised while asking can turn the committed join back into a failed one. Never an empty
+   * {@code Optional}, so {@code ServerControlPlane.connectCluster} reports this result and does not ask again.
+   */
+  @Override
+  public Optional<List<String>> connectClusterAndReportSeed(final String serverAddress) {
+    connectCluster(serverAddress);
+    return Optional.of(seedReportForAdmission(serverAddress));
+  }
+
   @Override
   public void addPeer(final String peerId, final String address) {
     addPeer(peerId, address, null);

ha-raft/src/test/java/com/arcadedb/server/ha/raft/Issue8077EmbeddedConnectClusterSeedReportTest.java

@@ -0,0 +1,153 @@
+/*
+ * Copyright 2021-present Arcade Data Ltd (info@arcadedata.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.arcadedb.server.ha.raft;
+
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Regression test for issue #8077: the embedded {@code HAServerPlugin.connectCluster} API joined a server and told
+ * its caller nothing about the cluster security seed, while {@code ServerControlPlane.connectCluster} (issue #7532)
+ * and the embedded {@code addPeer} API (issue #7820) both report it.
+ * <p>
+ * {@code RaftHAPlugin.connectClusterAndReportSeed} is the reporting form. This pins that it reports, that the join
+ * runs first and is never undone or failed by the seed, and that a join that failed is never followed by a seed.
+ *
+ * @author Roberto Franchini (r.franchini@arcadedata.com)
+ */
+class Issue8077EmbeddedConnectClusterSeedReportTest {
+
+  private static final String ADDRESS = "node3@localhost:2447";
+
+  /**
+   * A plugin whose join and leader-seed request are both stand-ins. Both are the real methods the production code
+   * calls: {@code connectCluster} is the whole of the membership change, and {@code seedSecurityStateForAdmission}
+   * reaches the leader's single seeder (issue #7834). Nothing between them is replaced.
+   */
+  private static class RecordingPlugin extends RaftHAPlugin {
+    final List<String> steps       = new ArrayList<>();
+    List<String>       failedSeeds = List.of();
+    Exception          seedFailure;
+    RuntimeException   joinFailure;
+
+    @Override
+    public void connectCluster(final String serverAddress) {
+      steps.add("join " + serverAddress);
+      if (joinFailure != null)
+        throw joinFailure;
+    }
+
+    @Override
+    public Optional<List<String>> seedSecurityStateForAdmission(final String admittedPeer) throws IOException {
+      steps.add("seed " + admittedPeer);
+      if (seedFailure instanceof IOException io)
+        throw io;
+      if (seedFailure instanceof RuntimeException runtime)
+        throw runtime;
+      return Optional.of(failedSeeds);
+    }
+  }
+
+  /** The reported case: the embedder learns which documents the leader's seed could not commit. */
+  @Test
+  void theEmbeddedJoinReportsTheDocumentsThatDidNotCommit() {
+    final RecordingPlugin plugin = new RecordingPlugin();
+    plugin.failedSeeds = List.of("groups", "API tokens");
+
+    assertThat(plugin.connectClusterAndReportSeed(ADDRESS)).as("an embedding application cannot act on a log line")
+        .contains(List.of("groups", "API tokens"));
+  }
+
+  /** A clean join reports an empty list - never an empty Optional, which would make the control plane seed again. */
+  @Test
+  void aCleanJoinReportsAPresentEmptyList() {
+    final Optional<List<String>> report = new RecordingPlugin().connectClusterAndReportSeed(ADDRESS);
+
+    assertThat(report).isPresent();
+    assertThat(report.get()).isEmpty();
+  }
+
+  /** The join first, then exactly one seed request, for the address that was joined. */
+  @Test
+  void theServerIsJoinedBeforeTheSeedIsAskedForAndTheSeedIsAskedOnce() {
+    final RecordingPlugin plugin = new RecordingPlugin();
+
+    plugin.connectClusterAndReportSeed(ADDRESS);
+
+    assertThat(plugin.steps).containsExactly("join " + ADDRESS, "seed " + ADDRESS);
+  }
+
+  /** A join that failed is not a member, so nothing is seeded and the failure reaches the caller. */
+  @Test
+  void aJoinThatFailedIsNotFollowedByASeed() {
+    final RecordingPlugin plugin = new RecordingPlugin();
+    plugin.joinFailure = new IllegalArgumentException("cannot join this node to itself");
+
+    assertThatThrownBy(() -> plugin.connectClusterAndReportSeed(ADDRESS))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("itself");
+
+    assertThat(plugin.steps).containsExactly("join " + ADDRESS);
+  }
+
+  /**
+   * The same rule against the production {@code connectCluster} rather than the stub: with no Raft server running the
+   * real join refuses, and no seed is asked for.
+   */
+  @Test
+  void aRealJoinThatFailedIsNotFollowedByASeed() {
+    final List<String> seeds = new ArrayList<>();
+    final RaftHAPlugin plugin = new RaftHAPlugin() {
+      @Override
+      public Optional<List<String>> seedSecurityStateForAdmission(final String admittedPeer) {
+        seeds.add(admittedPeer);
+        return Optional.of(List.of());
+      }
+    };
+
+    assertThatThrownBy(() -> plugin.connectClusterAndReportSeed(ADDRESS)).hasMessageContaining("not started");
+
+    assertThat(seeds).as("nothing joined, so nothing is seeded").isEmpty();
+  }
+
+  /** The leader cannot be reached: the join stands and the outcome is reported as unknown, i.e. all three. */
+  @Test
+  void aSeedThatCouldNotBeRunNeverFailsTheJoin() {
+    final RecordingPlugin plugin = new RecordingPlugin();
+    plugin.seedFailure = new IOException("no route to the leader");
+
+    assertThat(plugin.connectClusterAndReportSeed(ADDRESS)).contains(RaftHAPlugin.ALL_SEEDED_SECURITY_DOCUMENTS);
+  }
+
+  /** Any unchecked failure while seeding is the same case, including an UnsupportedOperationException. */
+  @Test
+  void anUncheckedFailureWhileSeedingStillDoesNotFailTheJoin() {
+    final RecordingPlugin plugin = new RecordingPlugin();
+    plugin.seedFailure = new UnsupportedOperationException("something nobody predicted");
+
+    assertThat(plugin.connectClusterAndReportSeed(ADDRESS)).contains(RaftHAPlugin.ALL_SEEDED_SECURITY_DOCUMENTS);
+  }
+}

server/src/main/java/com/arcadedb/server/HAServerPlugin.java

@@ -548,6 +548,46 @@ default void connectCluster(final String serverAddress) {
     throw new UnsupportedOperationException("Dynamic membership not supported by this HA implementation");
   }
 
+  /**
+   * {@link #connectCluster(String)} for a caller that needs the outcome of the cluster security seed, and not only
+   * the membership change (issue #8077, the {@code connect cluster} counterpart of {@link #addPeerAndReportSeed}).
+   * <p>
+   * {@code ServerControlPlane.connectCluster} - the front door both wire transports reach - has reported a residual
+   * seed failure since issue #7532, and it does so by consuming this method, so there is exactly one seed request
+   * per {@code connect cluster} (issue #7834). An embedding application that calls the plugin directly, through
+   * {@code server.getHA()}, calls this one to get the same report; {@link #connectCluster(String)} stays the
+   * membership change alone.
+   * <p>
+   * <b>The peer is a member whenever this returns</b>, failing documents or not. A non-empty list is not a failed
+   * join and must not be retried as one; re-issuing the same join is idempotent on the membership change and
+   * reissues the seed, which is the remediation. A membership change that did <i>not</i> happen leaves by an
+   * exception instead, exactly as {@link #connectCluster(String)} always has. An override must therefore never
+   * throw once the membership change has happened: a seed it could not run is reported, as every document failing.
+   * A non-empty report is returned, not logged: the caller owns it, unlike the void {@code addPeer}, which has no
+   * caller to hand it to.
+   * <p>
+   * <b>It waits for the seed report</b>, which {@link #connectCluster(String)} does not: on Raft that is the bounded
+   * wait {@code addPeer} documents ({@code arcadedb.ha.securitySeedRetryTimeout} plus a fixed margin), seconds in
+   * the worst case. An embedder on a latency-sensitive thread that does not need the report keeps calling
+   * {@link #connectCluster(String)}.
+   * <p>
+   * <b>An empty {@link Optional} is not an empty failure list</b>, with the meaning
+   * {@link #seedSecurityStateForAdmission} gives it: this implementation reports no seed of its own, and the caller
+   * that wants one runs it - which is what the default does, so an implementation predating this method keeps the
+   * seed {@code ServerControlPlane.connectCluster} always ran for it, through {@link #seedSecurityStateForAdmission}
+   * or locally.
+   *
+   * @param serverAddress one entry of {@code arcadedb.ha.serverList}, as for {@link #connectCluster(String)}
+   *
+   * @return the names of the security documents that could not be seeded to the joined server, in the order
+   * {@code ServerSecurity.seedSecurityStateClusterWide} reports them - empty for a clean join - or an empty
+   * {@code Optional} when this implementation leaves the seed to its caller
+   */
+  default Optional<List<String>> connectClusterAndReportSeed(final String serverAddress) {
+    connectCluster(serverAddress);
+    return Optional.empty();
+  }
+
   /**
    * Adds a new peer to the cluster at runtime.
    */

server/src/main/java/com/arcadedb/server/ServerControlPlane.java

@@ -68,6 +68,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
@@ -249,8 +250,12 @@ public ConnectClusterResult connectCluster(final String serverAddress) {
       throw new OperationNotAvailableException("Cannot connect '" + serverAddress
           + "' to the cluster: ArcadeDB is not running with High Availability module enabled. Please add this setting at startup: -Darcadedb.ha.enabled=true");
 
+    // The plugin's own seed report, when it has one (issue #8077): the Raft implementation runs the join and asks the
+    // leader for the seed itself, so its embedded API reports what this verb reports, and this verb consumes that
+    // report instead of asking a second time - one seed request per connect cluster, as issue #7834 requires.
+    final Optional<List<String>> pluginReport;
     try {
-      ha.connectCluster(serverAddress);
+      pluginReport = ha.connectClusterAndReportSeed(serverAddress);
     } catch (final UnsupportedOperationException e) {
       // What this is for is HAServerPlugin.connectCluster's default - an HA implementation with no
       // runtime membership - which is a precondition of this server and must not reach gRPC as
@@ -268,6 +273,10 @@ public ConnectClusterResult connectCluster(final String serverAddress) {
     // and the new peer would run with a stale user set, a stale group document and a stale token store until
     // the next cluster-wide change of each kind.
     //
+    // On Raft the request below is never issued: RaftHAPlugin.connectClusterAndReportSeed has already asked the
+    // leader, and pluginReport is that answer (issue #8077). What follows covers an implementation that leaves
+    // the seed to this verb.
+    //
     // ASKED OF THE LEADER rather than run here (issue #7834). This verb does not require the local node to be
     // the leader - only the membership change underneath it is routed there - while the leader already seeds
     // every membership change of its own accord (issue #7531). Two seeders meant two JVMs, each holding only
@@ -285,11 +294,16 @@ public ConnectClusterResult connectCluster(final String serverAddress) {
     // failed join, because by this point the peer is a committed member. That now includes the IOException the
     // request itself can raise when the leader cannot be reached.
     try {
-      // An empty Optional means this HA implementation has no leader-side seeder; the local seed is then what it
-      // has always been. On Raft it is never empty.
-      final List<String> failedSeeds = ha.seedSecurityStateForAdmission(serverAddress)
-          .orElseGet(() -> server.getSecurity().seedSecurityStateClusterWide(
-              server.getConfiguration().getValueAsLong(GlobalConfiguration.HA_SECURITY_SEED_RETRY_TIMEOUT)));
+      // An implementation that reported no seed of its own (the HAServerPlugin default, so never Raft) gets the seed
+      // this verb always ran for it. Within that, an empty Optional means it has no leader-side seeder either, and
+      // the local seed is then what it has always been.
+      final List<String> failedSeeds;
+      if (pluginReport.isPresent())
+        failedSeeds = pluginReport.get();
+      else
+        failedSeeds = ha.seedSecurityStateForAdmission(serverAddress)
+            .orElseGet(() -> server.getSecurity().seedSecurityStateClusterWide(
+                server.getConfiguration().getValueAsLong(GlobalConfiguration.HA_SECURITY_SEED_RETRY_TIMEOUT)));
       if (!failedSeeds.isEmpty())
         LogManager.instance().log(this, Level.SEVERE,
             "Connect cluster joined '%s' but these security documents could not be seeded to it: %s. That peer is a "