Stop creating HealthCheckConfig with a dynamically injected plugin name
Currently, in order to create a `HealthCheckConfig` instance one needs
to pass the plugin name. The plugin name is injected dynamically when
the object is provisioned. The constructor will then create a config
object using a plugin config factory with the provided plugin name.
This in turn means the config object will hold the contents of the
`plugin.config` file. For any "core" healthcheck, ie a healthcheck
defined in the healthcheck plugin, this works fine, ie configuration is
retrieved from the `healthcheck.config` file.
The problem arises for any external check, ie a check defined in an
external plugin. In that case, the `pluginName` will be the one of the
external plugin. So for a plugin foo, the `HealthCheckConfig` object will
use config located in a file foo.config. Here the external plugin's
entire config is leaked into the healthcheck config object.
This is an even bigger problem because it is assumed every plugin
provides its config through a `pluginName.config` file, which is not
true. A prime example of this is the pull-replication plugin, its config
is defined in a `replication.config` file. Therefore, for such cases,
any configuration defined in the plugin's config will be silently
ignored.
A major impact of the above problems is that for external checks, we
can't have the "base" healthcheck config in the `healthcheck.config`
file. By "base" here we refer mainly to the `enabled` flag and the
timeout, both core features of any healthcheck specification.
Such configuration must be defined in the external plugin's config file.
Stop injecting dynamically the plugin name, instead hardcode it to the
healthcheck plugin's name. An additional benefit of this approach is
that the HealthCheckConfig will truly be a singleton object, as both
core and external healthchecks will use the same instance.
Bug: Issue 312895374
Change-Id: I445ceafb69c74bc60530f25b44aa80e09262c2a7
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
index 1dc143c..93cf563 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
@@ -22,7 +22,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritIsReplica;
@@ -39,6 +38,7 @@
@Singleton
public class HealthCheckConfig {
+ public static final String HEALTHCHECK_PLUGIN_NAME = "healthcheck";
public static final String HEALTHCHECK = "healthcheck";
public static final HealthCheckConfig DEFAULT_CONFIG = new HealthCheckConfig(null);
private static final long HEALTHCHECK_TIMEOUT_DEFAULT = 500L;
@@ -61,11 +61,10 @@
@Inject
public HealthCheckConfig(
PluginConfigFactory configFactory,
- @PluginName String pluginName,
AllProjectsName allProjectsName,
AllUsersName allUsersName,
@GerritIsReplica boolean isReplica) {
- config = configFactory.getGlobalPluginConfig(pluginName);
+ config = configFactory.getGlobalPluginConfig(HEALTHCHECK_PLUGIN_NAME);
this.allProjectsName = allProjectsName;
this.allUsersName = allUsersName;
this.isReplica = isReplica;
