Block disabling mandatory plugin

Change-Id: I0358cfc5629de859d4f0d9ac6b5ac34d16bcdcc4

Documentation/cmd-plugin-remove.txt

@@ -20,6 +20,7 @@ jars in the site path's `plugins` directory to `<plugin-jar-name>.disabled`.
 * Caller must be a member of the privileged 'Administrators' group.
 * link:config-gerrit.html#plugins.allowRemoteAdmin[plugins.allowRemoteAdmin]
 must be enabled in `$site_path/etc/gerrit.config`.
+* Mandatory plugin cannot be disabled
 
 == SCRIPTING
 This command is intended to be used in scripts.

Documentation/rest-api-plugins.txt

@@ -387,6 +387,24 @@ describes the plugin.
   }
 ----
 
+Disabling of a link:config-gerrit.html#plugins.mandatory[mandatory plugin]
+is rejected:
+
+.Request
+----
+  DELETE /plugins/replication HTTP/1.0
+----
+
+.Response
+----
+  HTTP/1.1 405 Method Not Allowed
+  Content-Disposition: attachment
+  Content-Type: application/json; charset=UTF-8
+
+  )]}'
+  Plugin replication is mandatory
+----
+
 [[reload-plugin]]
 === Reload Plugin
 --

java/com/google/gerrit/server/plugins/DisablePlugin.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.plugins;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.extensions.common.Input;
 import com.google.gerrit.extensions.common.PluginInfo;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.server.permissions.GlobalPermission;
@@ -30,11 +31,16 @@ public class DisablePlugin implements RestModifyView<PluginResource, Input> {
 
   private final PluginLoader loader;
   private final PermissionBackend permissionBackend;
+  private final MandatoryPluginsCollection mandatoryPluginsCollection;
 
   @Inject
-  DisablePlugin(PluginLoader loader, PermissionBackend permissionBackend) {
+  DisablePlugin(
+      PluginLoader loader,
+      PermissionBackend permissionBackend,
+      MandatoryPluginsCollection mandatoryPluginsCollection) {
     this.loader = loader;
     this.permissionBackend = permissionBackend;
+    this.mandatoryPluginsCollection = mandatoryPluginsCollection;
   }
 
   @Override
@@ -46,6 +52,9 @@ public class DisablePlugin implements RestModifyView<PluginResource, Input> {
     }
     loader.checkRemoteAdminEnabled();
     String name = resource.getName();
+    if (mandatoryPluginsCollection.contains(name)) {
+      throw new MethodNotAllowedException("Plugin " + name + " is mandatory");
+    }
     loader.disablePlugins(ImmutableSet.of(name));
     return ListPlugins.toPluginInfo(loader.get(name));
   }

java/com/google/gerrit/server/plugins/MandatoryPluginsCollection.java

@@ -0,0 +1,50 @@
+// Copyright (C) 2019 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.google.gerrit.server.plugins;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArraySet;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+public class MandatoryPluginsCollection {
+  private final CopyOnWriteArraySet<String> members;
+
+  @Inject
+  MandatoryPluginsCollection(@GerritServerConfig Config cfg) {
+    members = Sets.newCopyOnWriteArraySet();
+    members.addAll(Arrays.asList(cfg.getStringList("plugins", null, "mandatory")));
+  }
+
+  public boolean contains(String name) {
+    return members.contains(name);
+  }
+
+  public Set<String> asSet() {
+    return ImmutableSet.copyOf(members);
+  }
+
+  @VisibleForTesting
+  public void add(String name) {
+    members.add(name);
+  }
+}

java/com/google/gerrit/server/plugins/PluginLoader.java

@@ -20,7 +20,6 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Lists;
@@ -51,7 +50,6 @@ import java.nio.file.Paths;
 import java.util.AbstractMap;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashSet;
@@ -90,7 +88,7 @@ public class PluginLoader implements LifecycleListener {
   private final Provider<String> urlProvider;
   private final PersistentCacheFactory persistentCacheFactory;
   private final boolean remoteAdmin;
-  private final Set<String> mandatoryPlugins;
+  private final MandatoryPluginsCollection mandatoryPlugins;
   private final UniversalServerPluginProvider serverPluginFactory;
   private final GerritRuntime gerritRuntime;
 
@@ -105,6 +103,7 @@ public class PluginLoader implements LifecycleListener {
       @CanonicalWebUrl Provider<String> provider,
       PersistentCacheFactory cacheFactory,
       UniversalServerPluginProvider pluginFactory,
+      MandatoryPluginsCollection mpc,
       GerritRuntime gerritRuntime) {
     pluginsDir = sitePaths.plugins_dir;
     dataDir = sitePaths.data_dir;
@@ -118,8 +117,7 @@ public class PluginLoader implements LifecycleListener {
     serverPluginFactory = pluginFactory;
 
     remoteAdmin = cfg.getBoolean("plugins", null, "allowRemoteAdmin", false);
-    mandatoryPlugins =
+    mandatoryPlugins = mpc;
-        ImmutableSet.copyOf(Arrays.asList(cfg.getStringList("plugins", null, "mandatory")));
     this.gerritRuntime = gerritRuntime;
 
     long checkFrequency =
@@ -232,6 +230,11 @@ public class PluginLoader implements LifecycleListener {
           continue;
         }
 
+        if (mandatoryPlugins.contains(name)) {
+          logger.atWarning().log("Mandatory plugin %s cannot be disabled", name);
+          continue;
+        }
+
         logger.atInfo().log("Disabling plugin %s", active.getName());
         Path off =
             active.getSrcFile().resolveSibling(active.getSrcFile().getFileName() + ".disabled");
@@ -435,7 +438,7 @@ public class PluginLoader implements LifecycleListener {
       }
     }
 
-    Set<String> missingMandatory = Sets.difference(mandatoryPlugins, loadedPlugins);
+    Set<String> missingMandatory = Sets.difference(mandatoryPlugins.asSet(), loadedPlugins);
     if (!missingMandatory.isEmpty()) {
       throw new MissingMandatoryPluginsException(missingMandatory);
     }

java/com/google/gerrit/server/plugins/PluginModule.java

@@ -34,6 +34,7 @@ public class PluginModule extends LifecycleModule {
     bind(PluginLoader.class);
     bind(CopyConfigModule.class);
     listener().to(PluginLoader.class);
+    bind(MandatoryPluginsCollection.class);
 
     DynamicSet.setOf(binder(), ServerPluginProvider.class);
     DynamicSet.bind(binder(), ServerPluginProvider.class).to(JarPluginProvider.class);

javatests/com/google/gerrit/acceptance/api/plugin/PluginIT.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.acceptance.api.plugin;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.stream.Collectors.toList;
@@ -35,6 +36,7 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RawInput;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.plugins.MandatoryPluginsCollection;
 import com.google.inject.Inject;
 import java.util.List;
 import org.junit.Test;
@@ -53,6 +55,7 @@ public class PluginIT extends AbstractDaemonTest {
           "plugin-a.js", "plugin-b.html", "plugin-c.js", "plugin-d.html", "plugin_e.js");
 
   @Inject private RequestScopeOperations requestScopeOperations;
+  @Inject private MandatoryPluginsCollection mandatoryPluginsCollection;
 
   @Test
   @GerritConfig(name = "plugins.allowRemoteAdmin", value = "true")
@@ -99,7 +102,19 @@ public class PluginIT extends AbstractDaemonTest {
     assertBadRequest(list().regex(".*in-b").prefix("a"));
     assertBadRequest(list().substring(".*in-b").prefix("a"));
 
-    // Disable
+    // Disable mandatory
+    mandatoryPluginsCollection.add("plugin_e");
+    api = gApi.plugins().name("plugin_e");
+    try {
+      api.disable();
+      assert_().fail("Disabling mandatory plugin should have failed");
+    } catch (MethodNotAllowedException e) {
+      // expected
+    }
+    api = gApi.plugins().name("plugin_e");
+    assertThat(api.get().disabled).isNull();
+
+    // Disable non-mandatory
     api = gApi.plugins().name("plugin-a");
     api.disable();
     api = gApi.plugins().name("plugin-a");