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;