Allow attempt to load security config in case of plugin restart even …

…if security index already exists (opensearch-project#1154) (cherry picked from commit 5a81a42)
niravpi · Jun 8, 2021 · b5c575b · b5c575b
1 parent 17ffd57
commit b5c575b
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 17 deletions.
diff --git a/...com/amazon/opendistroforelasticsearch/security/configuration/ConfigurationRepository.java b/...com/amazon/opendistroforelasticsearch/security/configuration/ConfigurationRepository.java
@@ -131,22 +131,19 @@ public void run() {
                                 try(StoredContext ctx = threadContext.stashContext()) {
                                     threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true");
 
-                                    final boolean isSecurityIndexCreated = createSecurityIndexIfAbsent();
+                                    createSecurityIndexIfAbsent();
                                     waitForSecurityIndexToBeAtLeastYellow();
 
-                                    if (isSecurityIndexCreated) {
-                                        ConfigHelper.uploadFile(client, cd+"config.yml", opendistrosecurityIndex, CType.CONFIG, DEFAULT_CONFIG_VERSION);
-                                        ConfigHelper.uploadFile(client, cd+"roles.yml", opendistrosecurityIndex, CType.ROLES, DEFAULT_CONFIG_VERSION);
-                                        ConfigHelper.uploadFile(client, cd+"roles_mapping.yml", opendistrosecurityIndex, CType.ROLESMAPPING, DEFAULT_CONFIG_VERSION);
-                                        ConfigHelper.uploadFile(client, cd+"internal_users.yml", opendistrosecurityIndex, CType.INTERNALUSERS, DEFAULT_CONFIG_VERSION);
-                                        ConfigHelper.uploadFile(client, cd+"action_groups.yml", opendistrosecurityIndex, CType.ACTIONGROUPS, DEFAULT_CONFIG_VERSION);
-                                        if(DEFAULT_CONFIG_VERSION == 2) {
-                                            ConfigHelper.uploadFile(client, cd+"tenants.yml", opendistrosecurityIndex, CType.TENANTS, DEFAULT_CONFIG_VERSION);
-                                        }
-                                        final boolean populateEmptyIfFileMissing = true;
-                                        ConfigHelper.uploadFile(client, cd+"nodes_dn.yml", opendistrosecurityIndex, CType.NODESDN, DEFAULT_CONFIG_VERSION, populateEmptyIfFileMissing);
-                                        LOGGER.info("Default config applied");
+                                    ConfigHelper.uploadFile(client, cd+"config.yml", opendistrosecurityIndex, CType.CONFIG, DEFAULT_CONFIG_VERSION);
+                                    ConfigHelper.uploadFile(client, cd+"roles.yml", opendistrosecurityIndex, CType.ROLES, DEFAULT_CONFIG_VERSION);
+                                    ConfigHelper.uploadFile(client, cd+"roles_mapping.yml", opendistrosecurityIndex, CType.ROLESMAPPING, DEFAULT_CONFIG_VERSION);
+                                    ConfigHelper.uploadFile(client, cd+"internal_users.yml", opendistrosecurityIndex, CType.INTERNALUSERS, DEFAULT_CONFIG_VERSION);
+                                    ConfigHelper.uploadFile(client, cd+"action_groups.yml", opendistrosecurityIndex, CType.ACTIONGROUPS, DEFAULT_CONFIG_VERSION);
+                                    if(DEFAULT_CONFIG_VERSION == 2) {
+                                        ConfigHelper.uploadFile(client, cd+"tenants.yml", opendistrosecurityIndex, CType.TENANTS, DEFAULT_CONFIG_VERSION);
                                     }
+                                    final boolean populateEmptyIfFileMissing = true;
+                                    ConfigHelper.uploadFile(client, cd+"nodes_dn.yml", opendistrosecurityIndex, CType.NODESDN, DEFAULT_CONFIG_VERSION, populateEmptyIfFileMissing);
 
                                     // audit.yml is not packaged by default
                                     final String auditConfigPath = cd + "audit.yml";

diff --git a/...t/java/com/amazon/opendistroforelasticsearch/security/InitializationIntegrationTests.java b/...t/java/com/amazon/opendistroforelasticsearch/security/InitializationIntegrationTests.java
@@ -198,6 +198,33 @@ public void testDefaultConfig() throws Exception {
         Assert.assertEquals(HttpStatus.SC_OK, rh.executeGetRequest("", encodeBasicHeader("admin", "admin")).getStatusCode());
     }
 
+    @Test
+    public void testInvalidDefaultConfig() throws Exception {
+        String defaultInitDirectory = System.getProperty("security.default_init.dir");
+        try {
+            System.setProperty("security.default_init.dir", new File("./src/test/resources/invalid_config").getAbsolutePath());
+            final Settings settings = Settings.builder()
+                    .put(ConfigConstants.OPENDISTRO_SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true)
+                    .build();
+            setup(Settings.EMPTY, null, settings, false);
+            RestHelper rh = nonSslRestHelper();
+            Thread.sleep(10000);
+            Assert.assertEquals(HttpStatus.SC_SERVICE_UNAVAILABLE, rh.executeGetRequest("", encodeBasicHeader("admin", "admin")).getStatusCode());
+
+            System.setProperty("security.default_init.dir", defaultInitDirectory);
+            restart(Settings.EMPTY, null, settings, false);
+            rh = nonSslRestHelper();
+            Thread.sleep(10000);
+            Assert.assertEquals(HttpStatus.SC_OK, rh.executeGetRequest("", encodeBasicHeader("admin", "admin")).getStatusCode());
+        } finally {
+            if (defaultInitDirectory != null) {
+                System.setProperty("security.default_init.dir", defaultInitDirectory);
+            } else {
+                System.clearProperty("security.default_init.dir");
+            }
+        }
+    }
+
     @Test
     public void testDisabled() throws Exception {
 

diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/security/test/SingleClusterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/security/test/SingleClusterTest.java
@@ -71,6 +71,13 @@ protected void setup(Settings initTransportClientSettings, DynamicSecurityConfig
         setup(initTransportClientSettings, dynamicSecuritySettings, nodeOverride, initOpendistroSecurityIndex, ClusterConfiguration.DEFAULT);
     }
 
+    protected void restart(Settings initTransportClientSettings, DynamicSecurityConfig dynamicSecuritySettings, Settings nodeOverride, boolean initOpendistroSecurityIndex) throws Exception {
+        clusterInfo = clusterHelper.startCluster(minimumSecuritySettings(ccs(nodeOverride)), ClusterConfiguration.DEFAULT);
+        if(initOpendistroSecurityIndex && dynamicSecuritySettings != null) {
+            initialize(clusterInfo, initTransportClientSettings, dynamicSecuritySettings);
+        }
+    }
+
     private Settings ccs(Settings nodeOverride) throws Exception {
         if(remoteClusterHelper != null) {
             Assert.assertNull("No remote clusters", remoteClusterInfo);

diff --git a/...ava/com/amazon/opendistroforelasticsearch/security/test/helper/cluster/ClusterHelper.java b/...ava/com/amazon/opendistroforelasticsearch/security/test/helper/cluster/ClusterHelper.java
@@ -77,10 +77,12 @@ public final class ClusterHelper {
     protected final List<PluginAwareNode> esNodes = new LinkedList<>();
 
     private final String clustername;
+    private ClusterState clusterState;
 
     public ClusterHelper(String clustername) {
         super();
         this.clustername = clustername;
+        this.clusterState = ClusterState.UNINITIALIZED;
     }
 
     public String getClusterName() {
@@ -98,16 +100,22 @@ public final ClusterInfo startCluster(final NodeSettingsSupplier nodeSettingsSup
         return startCluster(nodeSettingsSupplier, clusterConfiguration, 10, null);
     }
 
-
     public final synchronized ClusterInfo startCluster(final NodeSettingsSupplier nodeSettingsSupplier, ClusterConfiguration clusterConfiguration, int timeout, Integer nodes)
             throws Exception {
 
+        switch (clusterState) {
+            case UNINITIALIZED:
+                FileUtils.deleteDirectory(new File("./target/data/" + clustername));
+                break;
+            case STARTED:
+                closeAllNodes();
+                break;
+        }
+
         if (!esNodes.isEmpty()) {
             throw new RuntimeException("There are still " + esNodes.size() + " nodes instantiated, close them first.");
         }
 
-        FileUtils.deleteDirectory(new File("./target/data/"+clustername));
-
         List<NodeSettings> internalNodeSettings = clusterConfiguration.getNodeSettings();
 
         final String forkno = System.getProperty("forkno");
@@ -225,19 +233,24 @@ public void run() {
             throw new RuntimeException("Default template could not be created");
         }
 
+        clusterState = ClusterState.STARTED;
         return cInfo;
     }
 
     public final void stopCluster() throws Exception {
+        closeAllNodes();
+        FileUtils.deleteDirectory(new File("./target/data/"+clustername));
+    }
 
+    private void closeAllNodes() throws  Exception {
         //close non master nodes
         esNodes.stream().filter(n->!n.isMasterEligible()).forEach(node->closeNode(node));
 
         //close master nodes
         esNodes.stream().filter(n->n.isMasterEligible()).forEach(node->closeNode(node));
         esNodes.clear();
 
-        FileUtils.deleteDirectory(new File("./target/data/"+clustername));
+        clusterState = ClusterState.STOPPED;
     }
 
     private static void closeNode(Node node) {
@@ -372,4 +385,10 @@ private Settings.Builder getMinimumNonSecurityNodeSettingsBuilder(final int node
 	    return (masterEligibleNodes/2) + 1;
 
 	}*/
+
+    private enum ClusterState{
+        UNINITIALIZED,
+        STARTED,
+        STOPPED
+    }
 }
diff --git a/src/test/resources/invalid_config/config.yml b/src/test/resources/invalid_config/config.yml
@@ -0,0 +1,13 @@
+---
+_meta:
+  type: "config"
+  config_version: 2
+
+INVALID_BLOB_HERE
+
+config:
+  dynamic:
+    filtered_alias_mode: "disallow"
+    disable_rest_auth: false
+    disable_intertransport_auth: false
+    respect_request_indices_options: false