From 3ab348b7e60f34423bb61ceffd1987b709fbfc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Thu, 16 Feb 2017 11:31:33 +0100 Subject: [PATCH] PluginConfigFactory: reload also outdated secure store With refresh=true, the getFromGerritConfig method only checked if gerrit.config was outdated and only reloaded the gerrit.config. Reloading of an outdated secure.config didn't work. If plugin stored a part of its configuration in the secure.config, like the replication plugin did, then there was no way to reload a changed username/password, without restarting Gerrit server. Change-Id: I57b6dd4ba367fb23aa20dea3f5ac36d8fcf52e26 --- .../gerrit/pgm/init/UpgradeFrom2_0_xTest.java | 10 ++++++++++ .../gerrit/server/config/PluginConfigFactory.java | 3 +++ .../server/securestore/DefaultSecureStore.java | 15 +++++++++++++++ .../gerrit/server/securestore/SecureStore.java | 6 ++++++ 4 files changed, 34 insertions(+) diff --git a/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java b/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java index 461a2416de..7721fca494 100644 --- a/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java +++ b/gerrit-pgm/src/test/java/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java @@ -139,5 +139,15 @@ public class UpgradeFrom2_0_xTest extends InitTestCase { public Iterable list() { throw new UnsupportedOperationException("not used by tests"); } + + @Override + public boolean isOutdated() { + throw new UnsupportedOperationException("not used by tests"); + } + + @Override + public void reload() { + throw new UnsupportedOperationException("not used by tests"); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java index ce295bd5c9..6310c0881a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/PluginConfigFactory.java @@ -101,6 +101,9 @@ public class PluginConfigFactory implements ReloadPluginListener { * @return the plugin configuration from the 'gerrit.config' file */ public PluginConfig getFromGerritConfig(String pluginName, boolean refresh) { + if (refresh && secureStore.isOutdated()) { + secureStore.reload(); + } File configFile = site.gerrit_config.toFile(); if (refresh && cfgSnapshot.isModified(configFile)) { cfgSnapshot = FileSnapshot.save(configFile); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java index 677e9ff5b8..b729b09ac3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/DefaultSecureStore.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.securestore; import com.google.gerrit.common.FileUtil; import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; +import com.google.inject.ProvisionException; import com.google.inject.Singleton; import java.io.File; import java.io.IOException; @@ -107,6 +108,20 @@ public class DefaultSecureStore extends SecureStore { return result; } + @Override + public boolean isOutdated() { + return sec.isOutdated(); + } + + @Override + public void reload() { + try { + sec.load(); + } catch (IOException | ConfigInvalidException e) { + throw new ProvisionException("Couldn't reload secure.config", e); + } + } + private void save() { try { saveSecure(sec); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java index 5c87391586..b5aebee3ad 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/securestore/SecureStore.java @@ -150,4 +150,10 @@ public abstract class SecureStore { /** @return list of stored entries. */ public abstract Iterable list(); + + /** @return true if currently loaded values are outdated */ + public abstract boolean isOutdated(); + + /** Reload the values */ + public abstract void reload(); }