Add option to preserve migration mode during online external-ids migration
Introduce a `--preserve-migration-mode` flag to the SSH command
`migrate-externalids-to-insensitive`. When enabled, this option skips
setting `auth.userNameCaseInsensitiveMigrationMode` to `false` after the
migration completes.
This change allows admins of multi-primary and/or IaC environments to
update the configuration across all primaries and/or with tooling like
Chef/Ansible after migration, rather than having it modified
automatically by the command on one primary.
Release-Notes: Add --preserve-migration-mode option to online external-ids migration
Change-Id: I3d6915e36b878b5c353d78045a99459b669c3814
diff --git a/Documentation/cmd-migrate-externalids-to-insensitive.txt b/Documentation/cmd-migrate-externalids-to-insensitive.txt
index b023089..6458303 100644
--- a/Documentation/cmd-migrate-externalids-to-insensitive.txt
+++ b/Documentation/cmd-migrate-externalids-to-insensitive.txt
@@ -7,6 +7,7 @@
[verse]
--
_ssh_ -p <port> <host> _gerrit migrate-externalids-to-insensitive_
+ [--preserve-migration-mode]
--
== DESCRIPTION
@@ -29,6 +30,13 @@
== SCRIPTING
This command is intended to be used in scripts.
+== OPTIONS
+--preserve-migration-mode::
+ Skip setting `auth.userNameCaseInsensitiveMigrationMode` to `false` after the
+ migration completes successfully. This option is useful in multi-primary and
+ IaC setups where admins may prefer to update the migration mode config on all
+ primaries and/or using IaC tooling after the migration completes.
+
== EXAMPLES
Start the online external ids migration:
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/OnlineExternalIdCaseSensitivityMigrator.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/OnlineExternalIdCaseSensitivityMigrator.java
index f72cfa9..e441681 100644
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/OnlineExternalIdCaseSensitivityMigrator.java
+++ b/java/com/google/gerrit/server/account/externalids/storage/notedb/OnlineExternalIdCaseSensitivityMigrator.java
@@ -70,7 +70,7 @@
globalConfig.getBoolean("auth", "userNameCaseInsensitive", false);
}
- public void migrate() {
+ public void migrate(boolean preserveMigrationMode) {
if (!isUserNameCaseInsensitive || !isUserNameCaseInsensitiveMigrationMode) {
logger.atSevere().log(
"External IDs online migration requires auth.userNameCaseInsensitive and"
@@ -91,7 +91,9 @@
monitor.endTask();
}
try {
- updateGerritConfig();
+ if (!preserveMigrationMode) {
+ updateGerritConfig();
+ }
monitor.beginTask("Reindex accounts", ProgressMonitor.UNKNOWN);
@SuppressWarnings("unused")
diff --git a/java/com/google/gerrit/sshd/commands/ExternalIdCaseSensitivityMigrationCommand.java b/java/com/google/gerrit/sshd/commands/ExternalIdCaseSensitivityMigrationCommand.java
index 5c38dd7..0fded1b 100644
--- a/java/com/google/gerrit/sshd/commands/ExternalIdCaseSensitivityMigrationCommand.java
+++ b/java/com/google/gerrit/sshd/commands/ExternalIdCaseSensitivityMigrationCommand.java
@@ -22,12 +22,19 @@
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.Config;
+import org.kohsuke.args4j.Option;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(
name = "migrate-externalids-to-insensitive",
description = "Migrate external-ids to case insensitive")
public class ExternalIdCaseSensitivityMigrationCommand extends SshCommand {
+ @Option(
+ name = "--preserve-migration-mode",
+ usage =
+ "Skip setting auth.userNameCaseInsensitiveMigrationMode "
+ + "to false after the migration finishes successfully.")
+ private boolean preserveMigrationMode;
@Inject OnlineExternalIdCaseSensitivityMigrator onlineExternalIdCaseSensitivityMigrator;
@Inject @GerritServerConfig private Config globalConfig;
@@ -45,7 +52,7 @@
+ " auth.userNameCaseInsensitiveMigrationMode to be set to true. Cannot start"
+ " migration!");
}
- onlineExternalIdCaseSensitivityMigrator.migrate();
+ onlineExternalIdCaseSensitivityMigrator.migrate(preserveMigrationMode);
stdout.println(
"External ids case insensitivity migration started. To check if it's completed look for"
+ " \"External IDs migration completed!\" message in the Gerrit server logs");
diff --git a/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensitivityMigratorIT.java b/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensitivityMigratorIT.java
index 9879657..3fb9d4e 100644
--- a/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensitivityMigratorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/account/externalids/OnlineExternalIdCaseSensitivityMigratorIT.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdCaseSensitivityMigrator;
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdNotes;
import com.google.gerrit.server.account.externalids.storage.notedb.OnlineExternalIdCaseSensitivityMigrator;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
@@ -42,6 +43,8 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
import org.junit.Test;
public class OnlineExternalIdCaseSensitivityMigratorIT extends AbstractDaemonTest {
@@ -52,6 +55,7 @@
@Inject private ExternalIdKeyFactory externalIdKeyFactory;
@Inject private ExternalIdFactory externalIdFactory;
@Inject private OnlineExternalIdCaseSensitivityMigrator objectUnderTest;
+ @Inject private SitePaths sitePaths;
@Override
public Module createModule() {
@@ -85,7 +89,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isTrue();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isFalse();
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
extIdNotes = externalIdNotesFactory.load(allUsersRepo);
assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "JonDoe").isPresent()).isFalse();
@@ -93,6 +97,47 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isFalse();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isTrue();
+
+ FileBasedConfig cfg = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
+ cfg.load();
+ assertThat(cfg.getBoolean("auth", null, "userNameCaseInsensitiveMigrationMode", true))
+ .isFalse();
+ }
+ }
+
+ @Test
+ @GerritConfig(name = "auth.userNameCaseInsensitive", value = "true")
+ @GerritConfig(name = "auth.userNameCaseInsensitiveMigrationMode", value = "true")
+ public void shouldMigrateExternalIdWithPreserveMigrationMode()
+ throws IOException, ConfigInvalidException {
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+
+ createExternalId(
+ md, extIdNotes, SCHEME_GERRIT, "JonDoe", accountId, isUserNameCaseInsensitive);
+ createExternalId(
+ md, extIdNotes, SCHEME_USERNAME, "JonDoe", accountId, isUserNameCaseInsensitive);
+
+ assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "JonDoe").isPresent()).isTrue();
+ assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "jondoe").isPresent()).isFalse();
+
+ assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isTrue();
+ assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isFalse();
+
+ objectUnderTest.migrate(true);
+
+ extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "JonDoe").isPresent()).isFalse();
+ assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "jondoe").isPresent()).isTrue();
+
+ assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isFalse();
+ assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isTrue();
+
+ FileBasedConfig cfg = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
+ cfg.load();
+ assertThat(cfg.getBoolean("auth", null, "userNameCaseInsensitiveMigrationMode", true))
+ .isTrue();
}
}
@@ -118,7 +163,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isFalse();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isTrue();
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
}
}
@@ -157,7 +202,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").get().email())
.isEqualTo("test@email.com");
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
extIdNotes = externalIdNotesFactory.load(allUsersRepo);
assertThat(getExactExternalId(extIdNotes, SCHEME_GERRIT, "JonDoe").isPresent()).isFalse();
@@ -187,7 +232,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isTrue();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isFalse();
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
extIdNotes = externalIdNotesFactory.load(allUsersRepo);
assertThat(
@@ -215,7 +260,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isTrue();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isTrue();
- assertThrows(DuplicateExternalIdKeyException.class, () -> objectUnderTest.migrate());
+ assertThrows(DuplicateExternalIdKeyException.class, () -> objectUnderTest.migrate(false));
}
}
@@ -235,7 +280,7 @@
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "JonDoe").isPresent()).isTrue();
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isFalse();
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
extIdNotes = externalIdNotesFactory.load(allUsersRepo);
@@ -257,7 +302,7 @@
createExternalId(
md, extIdNotes, SCHEME_USERNAME, "JonDoe", accountId, isUserNameCaseInsensitive);
- objectUnderTest.migrate();
+ objectUnderTest.migrate(false);
extIdNotes = externalIdNotesFactory.load(allUsersRepo);
assertThat(getExactExternalId(extIdNotes, SCHEME_USERNAME, "jondoe").isPresent()).isFalse();
