cmd/combine: fix multiple runs (#2963)

Instead of checking if `validator_keys` directory exists at the end of the load manifest loop, do it earlier and exclude potential non-Charon node directories.

Also make so that `--force` deletes the existing keys, if any.

Solves the use case in which the user called `combine` command twice with an input directory that was also used as an output directory.

category: bug
ticket: #2538 

Closes #2538

cmd/combine/combine.go

@@ -146,6 +146,26 @@ func Combine(ctx context.Context, inputDir, outputDir string, force, noverify bo
 		return errors.New("refusing to overwrite existing private key share", z.Str("path", ksPath))
 	}
 
+	if force {
+		matches, err := filepath.Glob(filepath.Join(outputDir, "keystore-*.json"))
+		if err != nil {
+			return errors.Wrap(err, "can't match keystore files")
+		}
+
+		passMatches, err := filepath.Glob(filepath.Join(outputDir, "keystore-*.txt"))
+		if err != nil {
+			return errors.Wrap(err, "can't match keystore password files")
+		}
+
+		matches = append(matches, passMatches...)
+
+		for _, match := range matches {
+			if err := os.RemoveAll(match); err != nil {
+				return errors.Wrap(err, "can't remove existing keystore file")
+			}
+		}
+	}
+
 	if err := o.keyStoreFunc(combinedKeys, outputDir); err != nil {
 		return errors.Wrap(err, "cannot store keystore")
 	}
@@ -229,6 +249,13 @@ func loadManifest(ctx context.Context, dir string, noverify bool) (*manifestpb.C
 			continue
 		}
 
+		// does this directory contains a "validator_keys" directory? if yes continue and add it as a candidate
+		vcdPath := filepath.Join(dir, sd.Name(), "validator_keys")
+		_, err = os.ReadDir(vcdPath)
+		if err != nil {
+			continue
+		}
+
 		// try opening the lock file
 		lockFile := filepath.Join(dir, sd.Name(), "cluster-lock.json")
 		manifestFile := filepath.Join(dir, sd.Name(), "cluster-manifest.pb")
@@ -246,13 +273,6 @@ func loadManifest(ctx context.Context, dir string, noverify bool) (*manifestpb.C
 			}
 		}
 
-		// does this directory contains a "validator_keys" directory? if yes continue and add it as a candidate
-		vcdPath := filepath.Join(dir, sd.Name(), "validator_keys")
-		_, err = os.ReadDir(vcdPath)
-		if err != nil {
-			continue
-		}
-
 		possibleValKeysDir = append(possibleValKeysDir, vcdPath)
 
 		lastCluster = cl

cmd/combine/combine_test.go

@@ -369,6 +369,15 @@ func combineTest(
 }
 
 func TestCombineTwiceWithoutForceFails(t *testing.T) {
+	runTwice(t, false, require.Error)
+}
+
+func TestCombineTwiceWithForceSucceedes(t *testing.T) {
+	runTwice(t, true, require.NoError)
+}
+
+func runTwice(t *testing.T, force bool, processErr require.ErrorAssertionFunc) {
+	t.Helper()
 	seed := 0
 	random := rand.New(rand.NewSource(int64(seed)))
 	lock, _, shares := cluster.NewForT(t, 2, 3, 4, seed, random)
@@ -442,8 +451,8 @@ func TestCombineTwiceWithoutForceFails(t *testing.T) {
 	err := combine.Combine(context.Background(), dir, od, false, false, eth2util.Network{}, combine.WithInsecureKeysForT(t))
 	require.NoError(t, err)
 
-	err = combine.Combine(context.Background(), dir, od, false, false, eth2util.Network{}, combine.WithInsecureKeysForT(t))
-	require.Error(t, err)
+	err = combine.Combine(context.Background(), dir, od, force, false, eth2util.Network{}, combine.WithInsecureKeysForT(t))
+	processErr(t, err)
 
 	keyFiles, err := keystore.LoadFilesUnordered(od)
 	require.NoError(t, err)