diff options
27 files changed, 933 insertions, 71 deletions
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java index 9c658e0..5e2c758 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java @@ -43,10 +43,9 @@ public class AutoReloadConfigDecorator implements ReplicationConfig, LifecycleLi public AutoReloadConfigDecorator( @PluginName String pluginName, WorkQueue workQueue, - ReplicationConfigImpl replicationConfig, AutoReloadRunnable reloadRunner, EventBus eventBus) { - this.currentConfig = replicationConfig; + this.currentConfig = reloadRunner.getCurrentReplicationConfig(); this.autoReloadExecutor = workQueue.createQueue(1, pluginName + "_auto-reload-config"); this.reloadRunner = reloadRunner; eventBus.register(this); @@ -129,4 +128,9 @@ public class AutoReloadConfigDecorator implements ReplicationConfig, LifecycleLi public Config getConfig() { return currentConfig.getConfig(); } + + @Override + public boolean useLegacyCredentials() { + return currentConfig.useLegacyCredentials(); + } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java index e9198b2..a752a35 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java @@ -61,6 +61,10 @@ public class AutoReloadRunnable implements Runnable { reload(); } + public ReplicationConfig getCurrentReplicationConfig() { + return loadedConfig; + } + synchronized void reload() { String pendingConfigVersion = loadedConfig.getVersion(); try { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java index e9409a6..132fa4f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java @@ -14,49 +14,57 @@ package com.googlesource.gerrit.plugins.replication; -import static com.google.gerrit.common.FileUtil.lastModified; - import com.google.common.flogger.FluentLogger; import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.securestore.SecureStore; import com.google.inject.Inject; import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig; import java.io.IOException; -import java.nio.file.Files; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.transport.CredentialsProvider; -public class AutoReloadSecureCredentialsFactoryDecorator implements CredentialsFactory { +class AutoReloadSecureCredentialsFactoryDecorator implements CredentialsFactory { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final AtomicReference<SecureCredentialsFactory> secureCredentialsFactory; - private volatile long secureCredentialsFactoryLoadTs; + private final AtomicReference<CredentialsFactory> secureCredentialsFactory; + private final SecureStore secureStore; private final SitePaths site; - private ReplicationConfig config; + private final ReplicationConfig config; @Inject - public AutoReloadSecureCredentialsFactoryDecorator(SitePaths site, ReplicationConfig config) + public AutoReloadSecureCredentialsFactoryDecorator( + SitePaths site, SecureStore secureStore, ReplicationConfig config) throws ConfigInvalidException, IOException { this.site = site; + this.secureStore = secureStore; this.config = config; - this.secureCredentialsFactory = new AtomicReference<>(new SecureCredentialsFactory(site)); - this.secureCredentialsFactoryLoadTs = getSecureConfigLastEditTs(); + this.secureCredentialsFactory = + new AtomicReference<>(newSecureCredentialsFactory(site, secureStore, config)); + if (config.useLegacyCredentials()) { + logger.atWarning().log( + "Using legacy credentials in clear text in secure.config. Please encrypt your credentials using " + + "'java -jar gerrit.war passwd' for each remote, remove the gerrit.useLegacyCredentials in replication.config " + + "and then reload the replication plugin."); + } } - private long getSecureConfigLastEditTs() { - if (!Files.exists(site.secure_config)) { - return 0L; + private static CredentialsFactory newSecureCredentialsFactory( + SitePaths site, SecureStore secureStore, ReplicationConfig config) + throws ConfigInvalidException, IOException { + if (config.useLegacyCredentials()) { + return new LegacyCredentialsFactory(site); } - return lastModified(site.secure_config); + return new SecureCredentialsFactory(secureStore); } @Override public CredentialsProvider create(String remoteName) { try { if (needsReload()) { + secureStore.reload(); secureCredentialsFactory.compareAndSet( - secureCredentialsFactory.get(), new SecureCredentialsFactory(site)); - secureCredentialsFactoryLoadTs = getSecureConfigLastEditTs(); + secureCredentialsFactory.get(), newSecureCredentialsFactory(site, secureStore, config)); logger.atInfo().log("secure.config reloaded as it was updated on the file system"); } } catch (Exception e) { @@ -69,7 +77,11 @@ public class AutoReloadSecureCredentialsFactoryDecorator implements CredentialsF } private boolean needsReload() { - return config.getConfig().getBoolean("gerrit", "autoReload", false) - && getSecureConfigLastEditTs() != secureCredentialsFactoryLoadTs; + return config.getConfig().getBoolean("gerrit", "autoReload", false) && secureStore.isOutdated(); + } + + @Override + public boolean validate(String remoteConfigName) { + return secureCredentialsFactory.get().validate(remoteConfigName); } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java index 3bb64ab..141fb2d 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/CredentialsFactory.java @@ -18,4 +18,8 @@ import org.eclipse.jgit.transport.CredentialsProvider; public interface CredentialsFactory { CredentialsProvider create(String remoteName); + + default boolean validate(String remoteConfigName) { + return true; + } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java index 6054a4a..08ed7cb 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java @@ -97,6 +97,8 @@ public class Destination { private static final String PROJECT_NOT_AVAILABLE = "source project %s not available"; + private final CredentialsFactory credentialsFactory; + public interface Factory { Destination create(DestinationConfiguration config); } @@ -149,6 +151,7 @@ public class Destination { GroupIncludeCache groupIncludeCache, DynamicItem<EventDispatcher> eventDispatcher, Provider<ReplicationTasksStorage> rts, + CredentialsFactory credentialsFactory, @Assisted DestinationConfiguration cfg) { this.eventDispatcher = eventDispatcher; gitManager = gitRepositoryManager; @@ -157,6 +160,7 @@ public class Destination { this.projectCache = projectCache; this.stateLog = stateLog; this.replicationTasksStorage = rts; + this.credentialsFactory = credentialsFactory; config = cfg; CurrentUser remoteUser; if (!cfg.getAuthGroupNames().isEmpty()) { @@ -216,6 +220,10 @@ public class Destination { threadScoper = child.getInstance(PerThreadRequestScope.Scoper.class); } + public boolean validate() { + return (credentialsFactory == null || credentialsFactory.validate(getRemoteConfigName())); + } + private void addRecursiveParents( AccountGroup.UUID g, ImmutableSet.Builder<AccountGroup.UUID> builder, diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java index 8a41945..dc3b05e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java @@ -180,6 +180,12 @@ public class DestinationsCollection implements ReplicationDestinations { public synchronized void startup(WorkQueue workQueue) { shuttingDown = false; for (Destination cfg : destinations) { + if (!cfg.validate()) { + throw new IllegalStateException( + "Unable to start replication plugin because remote " + + cfg.getRemoteConfigName() + + " is not valid"); + } cfg.start(workQueue); } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java index 4220ddb..0a2afa6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java @@ -23,9 +23,11 @@ import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; +import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -61,6 +63,30 @@ public class FanoutConfigResource extends FileConfigResource { } } + @Override + public void update(Config updates) throws IOException { + Set<String> remotes = updates.getSubsections("remote"); + for (String remote : remotes) { + File remoteFile = remoteConfigsDirPath.resolve(remote + ".config").toFile(); + FileBasedConfig remoteConfig = new FileBasedConfig(remoteFile, FS.DETECTED); + try { + remoteConfig.load(); + } catch (ConfigInvalidException e) { + throw new IOException( + String.format("Cannot parse configuration file: %s", remoteFile.getAbsoluteFile()), e); + } + Set<String> options = updates.getNames("remote", remote); + for (String option : options) { + List<String> values = List.of(updates.getStringList("remote", remote, option)); + remoteConfig.setStringList("remote", remote, option, values); + } + remoteConfig.save(); + } + + removeRemotes(updates); + super.update(updates); + } + private static void removeRemotes(Config config) { Set<String> remoteNames = config.getSubsections("remote"); if (remoteNames.size() > 0) { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java index baccb83..7a14cd1 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java @@ -17,6 +17,7 @@ package com.googlesource.gerrit.plugins.replication; import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; import com.google.gerrit.common.UsedAt; import com.google.gerrit.common.UsedAt.Project; import com.google.gerrit.server.config.SitePaths; @@ -24,6 +25,7 @@ import com.google.inject.Inject; import com.googlesource.gerrit.plugins.replication.api.ConfigResource; import java.io.IOException; import java.nio.file.Path; +import java.util.List; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.storage.file.FileBasedConfig; @@ -54,6 +56,25 @@ public class FileConfigResource implements ConfigResource { } @Override + public void update(Config updates) throws IOException { + for (String section : updates.getSections()) { + for (String subsection : updates.getSubsections(section)) { + for (String name : updates.getNames(section, subsection, true)) { + List<String> values = + Lists.newArrayList(updates.getStringList(section, subsection, name)); + config.setStringList(section, subsection, name, values); + } + } + + for (String name : updates.getNames(section, true)) { + List<String> values = Lists.newArrayList(updates.getStringList(section, null, name)); + config.setStringList(section, null, name, values); + } + } + config.save(); + } + + @Override public String getVersion() { return Long.toString(config.getFile().lastModified()); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java new file mode 100644 index 0000000..a89c936 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/LegacyCredentialsFactory.java @@ -0,0 +1,66 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication; + +import com.google.gerrit.server.config.SitePaths; +import com.google.inject.Inject; +import java.io.IOException; +import java.util.Objects; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.eclipse.jgit.util.FS; + +/** + * Looks up a remote's password in secure.config. + * + * @deprecated This class is not secure and should no longer be used; it was allowing to record and + * read credentials in clear text in secure.config even though the Gerrit administrator + * installed a proper SecureStore implementation such as the secure-config.jar libModule. + */ +@Deprecated(forRemoval = true) +class LegacyCredentialsFactory implements CredentialsFactory { + private final Config config; + + @Inject + public LegacyCredentialsFactory(SitePaths site) throws ConfigInvalidException, IOException { + config = load(site); + } + + private static Config load(SitePaths site) throws ConfigInvalidException, IOException { + FileBasedConfig cfg = new FileBasedConfig(site.secure_config.toFile(), FS.DETECTED); + if (cfg.getFile().exists() && cfg.getFile().length() > 0) { + try { + cfg.load(); + } catch (ConfigInvalidException e) { + throw new ConfigInvalidException( + String.format("Config file %s is invalid: %s", cfg.getFile(), e.getMessage()), e); + } catch (IOException e) { + throw new IOException( + String.format("Cannot read %s: %s", cfg.getFile(), e.getMessage()), e); + } + } + return cfg; + } + + @Override + public CredentialsProvider create(String remoteName) { + String user = Objects.toString(config.getString("remote", remoteName, "username"), ""); + String pass = Objects.toString(config.getString("remote", remoteName, "password"), ""); + return new UsernamePasswordCredentialsProvider(user, pass); + } +} diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java index 5e8100b..58a10f0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java @@ -15,6 +15,7 @@ package com.googlesource.gerrit.plugins.replication; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Suppliers; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.UsedAt; @@ -25,6 +26,8 @@ import com.google.inject.Provider; import com.google.inject.util.Providers; import com.googlesource.gerrit.plugins.replication.api.ConfigResource; import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides; +import java.io.IOException; +import java.util.function.Supplier; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; @@ -33,13 +36,16 @@ public class MergedConfigResource { @UsedAt(Project.PLUGIN_PULL_REPLICATION) public static MergedConfigResource withBaseOnly(ConfigResource base) { MergedConfigResource mergedConfigResource = new MergedConfigResource(); - mergedConfigResource.base = Providers.of(base); + mergedConfigResource.baseConfigProvider = Providers.of(base); return mergedConfigResource; } private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - @Inject private Provider<ConfigResource> base; + @Inject private Provider<ConfigResource> baseConfigProvider; + + private final Supplier<ConfigResource> base = + Suppliers.memoize(() -> this.baseConfigProvider.get()); @Inject(optional = true) @Nullable @@ -72,7 +78,15 @@ public class MergedConfigResource { return baseVersion + overrides.get().getVersion(); } - private boolean noOverrides() { + boolean noOverrides() { return overrides == null || overrides.get() == null; } + + void update(Config remotesConfig) throws IOException { + if (noOverrides()) { + base.get().update(remotesConfig); + } else { + overrides.get().update(remotesConfig); + } + } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java index 854ca6a..8f9b805 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java @@ -30,21 +30,23 @@ public class ReplicationConfigImpl implements ReplicationConfig { private static final int DEFAULT_SSH_CONNECTION_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes private final SitePaths site; + private final MergedConfigResource configResource; + private final boolean useLegacyCredentials; private boolean replicateAllOnPluginStart; private boolean defaultForceUpdate; private int maxRefsToLog; private final int maxRefsToShow; private int sshCommandTimeout; private int sshConnectionTimeout; - private final MergedConfigResource configResource; private final Path pluginDataDir; + private final Config config; @Inject public ReplicationConfigImpl( MergedConfigResource configResource, SitePaths site, @PluginData Path pluginDataDir) { this.site = site; + config = configResource.getConfig(); this.configResource = configResource; - Config config = configResource.getConfig(); this.replicateAllOnPluginStart = config.getBoolean("gerrit", "replicateOnStartup", false); this.defaultForceUpdate = config.getBoolean("gerrit", "defaultForceUpdate", false); this.maxRefsToLog = config.getInt("gerrit", "maxRefsToLog", 0); @@ -61,6 +63,7 @@ public class ReplicationConfigImpl implements ReplicationConfig { DEFAULT_SSH_CONNECTION_TIMEOUT_MS, MILLISECONDS); this.pluginDataDir = pluginDataDir; + this.useLegacyCredentials = config.getBoolean("gerrit", "useLegacyCredentials", false); } @Nullable @@ -120,7 +123,12 @@ public class ReplicationConfigImpl implements ReplicationConfig { @Override public Config getConfig() { - return configResource.getConfig(); + return config; + } + + @Override + public boolean useLegacyCredentials() { + return useLegacyCredentials; } @Override diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java index b0343c9..ec9cf50 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java @@ -23,9 +23,11 @@ import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.ProvisionException; import com.google.inject.Scopes; +import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.internal.UniqueAnnotations; import com.googlesource.gerrit.plugins.replication.api.ConfigResource; import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig; +import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionState; import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -57,6 +59,12 @@ public class ReplicationConfigModule extends AbstractModule { } else { bind(ReplicationConfig.class).to(ReplicationConfigImpl.class).in(Scopes.SINGLETON); } + + bind(ReplicationQueue.class).in(Scopes.SINGLETON); + bind(ReplicationDestinations.class).to(DestinationsCollection.class); + + install(new FactoryModuleBuilder().build(Destination.Factory.class)); + install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class)); } public FileBasedConfig getReplicationConfig() { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java index ac27280..c7e560e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java @@ -23,6 +23,7 @@ import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.extensions.events.HeadUpdatedListener; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; +import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.server.events.EventTypes; import com.google.inject.AbstractModule; @@ -30,11 +31,11 @@ import com.google.inject.Inject; import com.google.inject.Scopes; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.internal.UniqueAnnotations; +import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi; import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationDoneEvent; import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationFailedEvent; import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationScheduledEvent; import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionReplicationSucceededEvent; -import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionState; import com.googlesource.gerrit.plugins.replication.events.RefReplicatedEvent; import com.googlesource.gerrit.plugins.replication.events.RefReplicationDoneEvent; import com.googlesource.gerrit.plugins.replication.events.ReplicationScheduledEvent; @@ -52,9 +53,7 @@ class ReplicationModule extends AbstractModule { @Override protected void configure() { - install(new FactoryModuleBuilder().build(Destination.Factory.class)); install(configModule); - bind(ReplicationQueue.class).in(Scopes.SINGLETON); bind(ObservableQueue.class).to(ReplicationQueue.class); bind(LifecycleListener.class) .annotatedWith(UniqueAnnotations.create()) @@ -63,6 +62,7 @@ class ReplicationModule extends AbstractModule { DynamicSet.bind(binder(), GitBatchRefUpdateListener.class).to(ReplicationQueue.class); DynamicSet.bind(binder(), ProjectDeletedListener.class).to(ReplicationQueue.class); DynamicSet.bind(binder(), HeadUpdatedListener.class).to(ReplicationQueue.class); + DynamicItem.bind(binder(), ReplicationRemotesApi.class).to(ReplicationRemotesApiImpl.class); bind(OnStartStop.class).in(Scopes.SINGLETON); bind(LifecycleListener.class).annotatedWith(UniqueAnnotations.create()).to(OnStartStop.class); @@ -77,10 +77,8 @@ class ReplicationModule extends AbstractModule { .to(StartReplicationCapability.class); install(new FactoryModuleBuilder().build(PushAll.Factory.class)); - install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class)); bind(EventBus.class).in(Scopes.SINGLETON); - bind(ReplicationDestinations.class).to(DestinationsCollection.class); bind(ConfigParser.class).to(DestinationConfigParser.class).in(Scopes.SINGLETON); DynamicSet.setOf(binder(), ReplicationStateListener.class); diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java new file mode 100644 index 0000000..cc6bcad --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiImpl.java @@ -0,0 +1,100 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication; + +import com.google.gerrit.server.securestore.SecureStore; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import org.eclipse.jgit.lib.Config; + +@Singleton +class ReplicationRemotesApiImpl implements ReplicationRemotesApi { + + private final SecureStore secureStore; + private final MergedConfigResource mergedConfigResource; + + @Inject + ReplicationRemotesApiImpl(SecureStore secureStore, MergedConfigResource mergedConfigResource) { + this.secureStore = secureStore; + this.mergedConfigResource = mergedConfigResource; + } + + @Override + public Config get(String... remoteNames) { + Config replicationConfig = mergedConfigResource.getConfig(); + Config remoteConfig = new Config(); + for (String remoteName : remoteNames) { + Set<String> configNames = replicationConfig.getNames("remote", remoteName); + for (String configName : configNames) { + String[] values = replicationConfig.getStringList("remote", remoteName, configName); + if (values.length > 0) { + remoteConfig.setStringList("remote", remoteName, configName, Arrays.asList(values)); + } + } + } + return remoteConfig; + } + + @Override + public void update(Config remoteConfig) throws IOException { + if (remoteConfig.getSubsections("remote").isEmpty()) { + throw new IllegalArgumentException( + "configuration update must have at least one 'remote' section"); + } + + SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedCredentials(remoteConfig); + persistRemotesCredentials(configs); + + mergedConfigResource.update(configs.remotes); + } + + private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedCredentials(Config configUpdates) { + SeparatedRemoteConfigs configs = new SeparatedRemoteConfigs(); + for (String subSection : configUpdates.getSubsections("remote")) { + for (String name : configUpdates.getNames("remote", subSection)) { + List<String> values = List.of(configUpdates.getStringList("remote", subSection, name)); + if ("password".equals(name) || "username".equals(name)) { + configs.credentials.setStringList("remote", subSection, name, values); + } else { + configs.remotes.setStringList("remote", subSection, name, values); + } + } + } + + return configs; + } + + private void persistRemotesCredentials(SeparatedRemoteConfigs configs) { + for (String subSection : configs.credentials.getSubsections("remote")) { + copyRemoteStringList(configs.credentials, subSection, "username"); + copyRemoteStringList(configs.credentials, subSection, "password"); + } + } + + private void copyRemoteStringList(Config config, String subSection, String key) { + secureStore.setList( + "remote", subSection, key, List.of(config.getStringList("remote", subSection, key))); + } + + private static class SeparatedRemoteConfigs { + private final Config remotes = new Config(); + private final Config credentials = new Config(); + } +} diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java index ed15b92..957744e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/SecureCredentialsFactory.java @@ -1,4 +1,4 @@ -// Copyright (C) 2011 The Android Open Source Project +// Copyright (C) 2024 The Android Open Source Project // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -14,46 +14,40 @@ package com.googlesource.gerrit.plugins.replication; -import com.google.gerrit.server.config.SitePaths; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.server.securestore.SecureStore; import com.google.inject.Inject; -import java.io.IOException; import java.util.Objects; -import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; -import org.eclipse.jgit.util.FS; -/** Looks up a remote's password in secure.config. */ -public class SecureCredentialsFactory implements CredentialsFactory { - private final Config config; +/** Looks up a remote's password in SecureStore */ +class SecureCredentialsFactory implements CredentialsFactory { + private static final FluentLogger log = FluentLogger.forEnclosingClass(); + private final SecureStore secureStore; @Inject - public SecureCredentialsFactory(SitePaths site) throws ConfigInvalidException, IOException { - config = load(site); - } - - private static Config load(SitePaths site) throws ConfigInvalidException, IOException { - FileBasedConfig cfg = new FileBasedConfig(site.secure_config.toFile(), FS.DETECTED); - if (cfg.getFile().exists() && cfg.getFile().length() > 0) { - try { - cfg.load(); - } catch (ConfigInvalidException e) { - throw new ConfigInvalidException( - String.format("Config file %s is invalid: %s", cfg.getFile(), e.getMessage()), e); - } catch (IOException e) { - throw new IOException( - String.format("Cannot read %s: %s", cfg.getFile(), e.getMessage()), e); - } - } - return cfg; + public SecureCredentialsFactory(SecureStore secureStore) { + this.secureStore = secureStore; } @Override public CredentialsProvider create(String remoteName) { - String user = Objects.toString(config.getString("remote", remoteName, "username"), ""); - String pass = Objects.toString(config.getString("remote", remoteName, "password"), ""); + String user = Objects.toString(secureStore.get("remote", remoteName, "username"), ""); + String pass = Objects.toString(secureStore.get("remote", remoteName, "password"), ""); return new UsernamePasswordCredentialsProvider(user, pass); } + + @Override + public boolean validate(String remoteName) { + try { + String unused = secureStore.get("remote", remoteName, "username"); + unused = secureStore.get("remote", remoteName, "password"); + return true; + } catch (Throwable t) { + log.atSevere().withCause(t).log( + "Credentials for replication remote %s are invalid", remoteName); + return false; + } + } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java index 3764ede..44ec88c 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java @@ -22,5 +22,6 @@ public class ApiModule extends AbstractModule { protected void configure() { DynamicItem.itemOf(binder(), ReplicationPushFilter.class); DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class); + DynamicItem.itemOf(binder(), ReplicationRemotesApi.class); } } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java index 8babd01..0052fc2 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java @@ -14,6 +14,7 @@ package com.googlesource.gerrit.plugins.replication.api; +import java.io.IOException; import org.eclipse.jgit.lib.Config; /** @@ -37,10 +38,19 @@ public interface ConfigResource { Config getConfig(); /** - * Current logical version string of the current configuration loaded in memory, depending on the - * actual implementation of the configuration on the persistent storage. + * Update the configuration resource. * - * @return current logical version number. + * <p>Allows to persist changes to the configuration resource. + * + * @param config updated configuration + * @throws IOException when configuration cannot be persisted + */ + void update(Config config) throws IOException; + + /** + * Current logical version string of the current configuration on the persistent storage. + * + * @return latest logical version number on the persistent storage */ String getVersion(); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java index 8ef4e2b..d5ccb02 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java @@ -102,4 +102,16 @@ public interface ReplicationConfig { * @return the config. */ Config getConfig(); + + /** + * Use legacy credentials for migrating existing sites. + * + * <p>Existing installations may have used a mix of encrypted and clear text credentials in + * secure.config, leveraging the replication plugin bug that was not accessing it using the + * correct API. The legacy feature flag 'gerrit.useLegacyCredentials' allows Gerrit to still use + * direct access to secure.config without decrypting its values. + * + * @return true if the secure.config should be read always directly and without decryption + */ + boolean useLegacyCredentials(); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java new file mode 100644 index 0000000..76d6d35 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationRemotesApi.java @@ -0,0 +1,55 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication.api; + +import static com.google.gerrit.common.UsedAt.Project.PLUGIN_GITHUB; + +import com.google.gerrit.common.UsedAt; +import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.server.securestore.SecureStore; +import java.io.IOException; +import org.eclipse.jgit.lib.Config; + +/** Public API to update replication plugin remotes configurations programmatically. */ +@UsedAt(PLUGIN_GITHUB) +@DynamicItem.Final(implementedByPlugin = "replication") +public interface ReplicationRemotesApi { + + /** + * Retrieves the configuration for a remote by name. + * + * <p>Builds a JGit {@link Config} with the remote's configuration obtained by parsing the + * replication configuration sources. + * + * <p>NOTE: The remotes secrets are excluded for security reasons. + * + * @param remoteNames the remote names to retrieve. + * @return {@link Config} associated with the remoteName(s) + */ + Config get(String... remoteNames); + + /** + * Adds or updates the remote configuration for the replication plugin. + * + * <p>Provided JGit {@link Config} object should contain at least one named <em>remote</em> + * section. All other configurations will be ignored. + * + * <p>NOTE: The {@code remote.$name.password} will be stored using {@link SecureStore}. + * + * @param remoteConfig remotes to add or update + * @throws IOException when persisting fails + */ + void update(Config remoteConfig) throws IOException; +} diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 6f9a49d..bb4be9a 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md @@ -170,6 +170,14 @@ gerrit.sshConnectionTimeout : Timeout for SSH connections. If 0, there is no timeout and the client waits indefinitely. By default, 2 minutes. +gerrit.useLegacyCredentials +: Use legacy credentials on secure.config which did not allow encryption + through a SecureStore provider. Legacy credentials should not be used in + production and are provided purely for backward compatibility with existing + secure.config files. When enabling the SecureStore in Gerrit, for instance, + the secure-config.jar libModule, make sure that all existing replication + credentials are encrypted. By default, false. + replication.distributionInterval : Interval in seconds for running the replication distributor. When run, the replication distributor will add all persisted waiting tasks diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java index b8bbc08..935fdb4 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java @@ -57,7 +57,8 @@ public abstract class AbstractConfigTest { public final DestinationConfiguration config; protected FakeDestination(DestinationConfiguration config) { - super(injectorMock(), null, null, null, null, null, null, null, null, null, null, config); + super( + injectorMock(), null, null, null, null, null, null, null, null, null, null, null, config); this.config = config; } @@ -78,7 +79,7 @@ public abstract class AbstractConfigTest { } @Before - public void setup() { + public void setup() throws IOException { when(destinationFactoryMock.create(any(DestinationConfiguration.class))) .thenAnswer( new Answer<Destination>() { @@ -103,6 +104,10 @@ public abstract class AbstractConfigTest { return newReplicationConfig("replication.config"); } + protected FileBasedConfig newSecureConfig() { + return newReplicationConfig("secure.config"); + } + protected FileBasedConfig newReplicationConfig(String path) { FileBasedConfig replicationConfig = new FileBasedConfig(sitePaths.etc_dir.resolve(path).toFile(), FS.DETECTED); diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java index 6facf25..021707a 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java @@ -264,10 +264,6 @@ public class AutoReloadConfigDecoratorTest extends AbstractConfigTest { eventBus, Providers.of(replicationQueueMock)); return new AutoReloadConfigDecorator( - "replication", - workQueueMock, - newReplicationFileBasedConfig(), - autoReloadRunnable, - eventBus); + "replication", workQueueMock, autoReloadRunnable, eventBus); } } diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java new file mode 100644 index 0000000..c959cc9 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecoratorTest.java @@ -0,0 +1,97 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.when; + +import com.google.gerrit.server.securestore.SecureStore; +import com.googlesource.gerrit.plugins.replication.api.ReplicationConfig; +import java.io.IOException; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.CredentialItem; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class AutoReloadSecureCredentialsFactoryDecoratorTest extends AbstractConfigTest { + private static final String REMOTE_TEST = "remote-test"; + private static final String USERNAME_CLEARTEXT = "username-clear"; + private static final String PASSWORD_CLEARTEXT = "password-clear"; + private static final String USERNAME_CIPHERTEXT = "username-encrypted"; + private static final String PASSWORD_CIPHERTEXT = "password-encrypted"; + + @Mock private SecureStore secureStoreMock; + @Mock private ReplicationConfig replicationConfigMock; + + public AutoReloadSecureCredentialsFactoryDecoratorTest() throws IOException { + super(); + } + + @Override + @Before + public void setup() throws IOException { + when(replicationConfigMock.getConfig()).thenReturn(new Config()); + + when(secureStoreMock.get("remote", REMOTE_TEST, "username")).thenReturn(USERNAME_CIPHERTEXT); + when(secureStoreMock.get("remote", REMOTE_TEST, "password")).thenReturn(PASSWORD_CIPHERTEXT); + + FileBasedConfig secureConfig = newSecureConfig(); + secureConfig.setString("remote", REMOTE_TEST, "username", USERNAME_CLEARTEXT); + secureConfig.setString("remote", REMOTE_TEST, "password", PASSWORD_CLEARTEXT); + secureConfig.save(); + } + + @Test + public void shouldCreateLegacyCredentials() throws Exception { + when(replicationConfigMock.useLegacyCredentials()).thenReturn(true); + assertUsernamePasswordCredentials( + getCredentialsProvider(), USERNAME_CLEARTEXT, PASSWORD_CLEARTEXT); + } + + @Test + public void shouldCreateEncryptedCredentialsByDefault() throws Exception { + assertUsernamePasswordCredentials( + getCredentialsProvider(), USERNAME_CIPHERTEXT, PASSWORD_CIPHERTEXT); + } + + private UsernamePasswordCredentialsProvider getCredentialsProvider() + throws ConfigInvalidException, IOException { + AutoReloadSecureCredentialsFactoryDecorator credFactory = + new AutoReloadSecureCredentialsFactoryDecorator( + sitePaths, secureStoreMock, replicationConfigMock); + CredentialsProvider legacyCredentials = credFactory.create(REMOTE_TEST); + assertThat(legacyCredentials).isInstanceOf(UsernamePasswordCredentialsProvider.class); + return (UsernamePasswordCredentialsProvider) legacyCredentials; + } + + private static void assertUsernamePasswordCredentials( + UsernamePasswordCredentialsProvider credentials, String username, String password) { + CredentialItem.Username usernameItem = new CredentialItem.Username(); + CredentialItem.Password passwordItem = new CredentialItem.Password(); + assertThat(credentials.get(new URIish(), usernameItem, passwordItem)).isTrue(); + + assertThat(usernameItem.getValue()).isEqualTo(username); + assertThat(new String(passwordItem.getValue())).isEqualTo(password); + } +} diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java index 9147ea1..a622170 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.IOException; import java.util.List; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; import org.junit.Before; @@ -285,9 +286,69 @@ public class FanoutConfigResourceTest extends AbstractConfigTest { assertThat(objectUnderTest.getVersion()).isEqualTo(replicationConfigVersion); } + @Test + public void shouldAddConfigOptionToMainConfig() throws Exception { + FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths); + Config update = new Config(); + update.setString("new", null, "value", "set"); + + objectUnderTest.update(update); + Config updatedConfig = objectUnderTest.getConfig(); + + assertThat(updatedConfig.getString("new", null, "value")).isEqualTo("set"); + } + + @Test + public void shouldUpdateConfigOptionInMainConfig() throws Exception { + FileBasedConfig config = newReplicationConfig(); + config.setString("updatable", null, "value", "orig"); + config.save(); + FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths); + Config update = new Config(); + update.setString("updatable", null, "value", "updated"); + + objectUnderTest.update(update); + Config updatedConfig = objectUnderTest.getConfig(); + + assertThat(updatedConfig.getString("updatable", null, "value")).isEqualTo("updated"); + } + + @Test + public void shouldAddNewRemoteFile() throws Exception { + FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths); + Config update = new Config(); + update.setString("remote", remoteName1, "url", remoteUrl1); + + objectUnderTest.update(update); + + Config actual = loadRemoteConfig(remoteName1); + assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl1); + } + + @Test + public void shouldUpdateExistingRemote() throws Exception { + FileBasedConfig rawRemoteConfig = newRemoteConfig(remoteName1); + rawRemoteConfig.setString("remote", remoteName1, "url", remoteUrl1); + rawRemoteConfig.save(); + FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths); + Config update = new Config(); + update.setString("remote", remoteName1, "url", remoteUrl2); + + objectUnderTest.update(update); + + Config actual = loadRemoteConfig(remoteName1); + assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl2); + } + protected FileBasedConfig newRemoteConfig(String configFileName) { return new FileBasedConfig( sitePaths.etc_dir.resolve("replication/" + configFileName + ".config").toFile(), FS.DETECTED); } + + private Config loadRemoteConfig(String siteName) throws Exception { + FileBasedConfig config = newRemoteConfig(siteName); + config.load(); + return config; + } } diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java new file mode 100644 index 0000000..92bf58d --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java @@ -0,0 +1,121 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication; + +import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.io.MoreFiles; +import com.google.gerrit.server.config.SitePaths; +import com.googlesource.gerrit.plugins.replication.api.ConfigResource; +import java.nio.file.Files; +import java.nio.file.Path; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class FileConfigResourceTest { + private static final String VALUE_KEY = "value"; + private static final String INITIAL_KEY = "initial"; + private static final String UPDATABLE_KEY = "updatable"; + + private Path testDir; + private SitePaths sitePaths; + private ConfigResource configResource; + + @Before + public void setUp() throws Exception { + testDir = Files.createTempDirectory("fileConfigResourceTest"); + sitePaths = new SitePaths(testDir); + configResource = newFileConfigResource(); + } + + @After + public void tearDown() throws Exception { + MoreFiles.deleteRecursively(testDir, ALLOW_INSECURE); + } + + @Test + public void updateEmptyFile() throws Exception { + Config configUpdate = newConfigUpdate(); + + Config beforeUpdate = configResource.getConfig(); + assertThat(beforeUpdate.getSections()).isEmpty(); + + configResource.update(configUpdate); + Config updatedConfig = newFileConfigResource().getConfig(); + + assertConfigUpdate(updatedConfig); + } + + @Test + public void appendOptionToConfig() throws Exception { + FileBasedConfig rawConfig = getRawReplicationConfig(); + rawConfig.setInt(INITIAL_KEY, null, VALUE_KEY, 10); + rawConfig.save(); + configResource = newFileConfigResource(); + Config configUpdate = newConfigUpdate(); + + configResource.update(configUpdate); + Config updatedConfig = configResource.getConfig(); + + assertConfigUpdate(updatedConfig); + assertThat(updatedConfig.getInt(INITIAL_KEY, null, VALUE_KEY, -1)).isEqualTo(10); + } + + @Test + public void updateExistingOption() throws Exception { + int expectedValue = 20; + Config configUpdate = new Config(); + configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, expectedValue); + FileBasedConfig rawConfig = getRawReplicationConfig(newConfigUpdate()); + rawConfig.save(); + + configResource.update(configUpdate); + Config updatedConfig = configResource.getConfig(); + + assertConfigUpdate(updatedConfig, expectedValue); + } + + private FileConfigResource newFileConfigResource() { + return new FileConfigResource(sitePaths); + } + + private Config newConfigUpdate() { + Config configUpdate = new Config(); + configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, 1); + return configUpdate; + } + + private void assertConfigUpdate(Config config) { + assertConfigUpdate(config, 1); + } + + private void assertConfigUpdate(Config config, int expectedValue) { + assertThat(config.getInt(UPDATABLE_KEY, null, VALUE_KEY, -1)).isEqualTo(expectedValue); + } + + private FileBasedConfig getRawReplicationConfig() { + return getRawReplicationConfig(new Config()); + } + + private FileBasedConfig getRawReplicationConfig(Config base) { + Path configPath = sitePaths.etc_dir.resolve(FileConfigResource.CONFIG_NAME); + return new FileBasedConfig(base, configPath.toFile(), FS.DETECTED); + } +} diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java index 18bb603..cae78e3 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java @@ -22,6 +22,7 @@ import com.google.inject.Guice; import com.googlesource.gerrit.plugins.replication.api.ApiModule; import com.googlesource.gerrit.plugins.replication.api.ConfigResource; import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides; +import org.apache.commons.lang3.NotImplementedException; import org.eclipse.jgit.lib.Config; import org.junit.Test; @@ -78,6 +79,11 @@ public class MergedConfigResourceTest { } @Override + public void update(Config config) { + throw new NotImplementedException("not implemented"); + } + + @Override public String getVersion() { return "base"; } @@ -93,6 +99,11 @@ public class MergedConfigResourceTest { } @Override + public void update(Config config) { + throw new NotImplementedException("not implemented"); + } + + @Override public String getVersion() { return "override"; } diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java new file mode 100644 index 0000000..40d9846 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesApiTest.java @@ -0,0 +1,212 @@ +// Copyright (C) 2024 The Android Open Source Project +// +// 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. + +package com.googlesource.gerrit.plugins.replication; + +import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import com.google.common.io.MoreFiles; +import com.google.common.truth.StringSubject; +import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.securestore.SecureStore; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.googlesource.gerrit.plugins.replication.api.ConfigResource; +import com.googlesource.gerrit.plugins.replication.api.ReplicationConfigOverrides; +import com.googlesource.gerrit.plugins.replication.api.ReplicationRemotesApi; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jgit.lib.Config; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class ReplicationRemotesApiTest { + + private Path testSite; + private SecureStore secureStoreMock; + private FileConfigResource baseConfig; + private Injector testInjector; + private AtomicReference<ReplicationConfigOverrides> testOverrides; + private String url1; + private String remoteName1; + private String url2; + private String remoteName2; + + @Before + public void setUp() throws Exception { + testSite = Files.createTempDirectory("replicationRemotesUpdateTest"); + secureStoreMock = mock(SecureStore.class); + baseConfig = new FileConfigResource(new SitePaths(testSite)); + testOverrides = new AtomicReference<>(new TestReplicationConfigOverrides()); + testInjector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(ConfigResource.class).toInstance(baseConfig); + bind(SecureStore.class).toInstance(secureStoreMock); + bind(ReplicationRemotesApi.class).to(ReplicationRemotesApiImpl.class); + DynamicItem.itemOf(binder(), ReplicationConfigOverrides.class); + DynamicItem.bind(binder(), ReplicationConfigOverrides.class) + .toProvider(testOverrides::get); + } + }); + url1 = "fake_url1"; + remoteName1 = "site1"; + url2 = "fake_url2"; + remoteName2 = "site2"; + } + + @After + public void tearDown() throws Exception { + MoreFiles.deleteRecursively(testSite, ALLOW_INSECURE); + } + + @Test + public void shouldThrowWhenNoRemotesInTheUpdate() { + Config update = new Config(); + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update)); + + update.setString("non-remote", null, "value", "one"); + + assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update)); + } + + @Test + public void shouldReturnEmptyConfigWhenNoRemotes() { + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + assertThat(objectUnderTest.get("site").getSections()).isEmpty(); + } + + @Test + public void addRemoteSectionsToBaseConfigWhenNoOverrides() throws Exception { + testOverrides.set(null); + + Config update = new Config(); + setRemoteSite(update, remoteName1, "url", url1); + setRemoteSite(update, remoteName2, "url", url2); + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + objectUnderTest.update(update); + + assertRemoteSite(baseConfig.getConfig(), remoteName1, "url").isEqualTo(url1); + assertRemoteSite(baseConfig.getConfig(), remoteName2, "url").isEqualTo(url2); + } + + @Test + public void shouldReturnRemotesFromBaseConfigWhenNoOverrides() { + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + baseConfig.getConfig().setString("remote", remoteName1, "url", url1); + baseConfig.getConfig().setString("remote", remoteName2, "url", url2); + + Config remotesConfig = objectUnderTest.get(remoteName1, remoteName2); + assertRemoteSite(remotesConfig, remoteName1, "url").isEqualTo(url1); + assertRemoteSite(remotesConfig, remoteName2, "url").isEqualTo(url2); + } + + @Test + public void addRemotesSectionToBaseOverridesConfig() throws Exception { + Config update = new Config(); + setRemoteSite(update, remoteName1, "url", url1); + setRemoteSite(update, remoteName2, "url", url2); + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + objectUnderTest.update(update); + + assertRemoteSite(testOverrides.get().getConfig(), remoteName1, "url").isEqualTo(url1); + assertRemoteSite(testOverrides.get().getConfig(), remoteName2, "url").isEqualTo(url2); + assertRemoteSite(baseConfig.getConfig(), remoteName1, "url").isNull(); + assertRemoteSite(baseConfig.getConfig(), remoteName2, "url").isNull(); + + Config remotesConfig = objectUnderTest.get(remoteName1, remoteName2); + assertRemoteSite(remotesConfig, remoteName1, "url").isEqualTo(url1); + assertRemoteSite(remotesConfig, remoteName2, "url").isEqualTo(url2); + } + + @Test + public void shouldSetPasswordViaSecureStoreButNotStoreInConfig() throws Exception { + Config update = new Config(); + String password = "encrypted-password"; + String remoteName = "site"; + setRemoteSite(update, remoteName, "password", password); + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + objectUnderTest.update(update); + + verify(secureStoreMock).setList("remote", remoteName, "password", List.of(password)); + assertRemoteSite(baseConfig.getConfig(), remoteName, "password").isNull(); + assertRemoteSite(testOverrides.get().getConfig(), remoteName, "password").isNull(); + assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "password").isNull(); + } + + @Test + public void shouldSetUsernameViaSecureStoreButNotStoreInConfig() throws Exception { + Config update = new Config(); + String username = "encrypted-username"; + String remoteName = "site"; + setRemoteSite(update, remoteName, "username", username); + ReplicationRemotesApi objectUnderTest = getReplicationRemotesApi(); + + objectUnderTest.update(update); + + verify(secureStoreMock).setList("remote", remoteName, "username", List.of(username)); + assertRemoteSite(baseConfig.getConfig(), remoteName, "username").isNull(); + assertRemoteSite(testOverrides.get().getConfig(), remoteName, "username").isNull(); + assertRemoteSite(objectUnderTest.get(remoteName), remoteName, "username").isNull(); + } + + private void setRemoteSite(Config config, String remoteName, String name, String value) { + config.setString("remote", remoteName, name, value); + } + + private StringSubject assertRemoteSite(Config config, String remoteName, String name) { + return assertThat(config.getString("remote", remoteName, name)); + } + + private ReplicationRemotesApi getReplicationRemotesApi() { + return testInjector.getInstance(ReplicationRemotesApi.class); + } + + static class TestReplicationConfigOverrides implements ReplicationConfigOverrides { + private Config config = new Config(); + + @Override + public Config getConfig() { + return config; + } + + @Override + public void update(Config update) throws IOException { + config = update; + } + + @Override + public String getVersion() { + return "none"; + } + } +} |