Fix cluster default initialization

Signed-off-by: Andrey Pleskach <[email protected]>
opensearch-project · Jan 29, 2024 · 627b33a · 627b33a
1 parent dadd03f
commit 627b33a
Show file tree

Hide file tree

Showing 10 changed files with 509 additions and 63 deletions.
diff --git a/...h/security/DefaultConfigurationTests.java → ...ty/AbstractDefaultConfigurationTests.java b/...h/security/DefaultConfigurationTests.java → ...ty/AbstractDefaultConfigurationTests.java
@@ -1,78 +1,78 @@
 /*
-* Copyright OpenSearch Contributors
-* SPDX-License-Identifier: Apache-2.0
-*
-* The OpenSearch Contributors require contributions made to
-* this file be licensed under the Apache-2.0 license or a
-* compatible open source license.
-*
-*/
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ */
 package org.opensearch.security;
 
 import java.io.IOException;
 import java.nio.file.Path;
-import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
 import org.apache.commons.io.FileUtils;
 import org.awaitility.Awaitility;
 import org.junit.AfterClass;
-import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.security.state.SecurityMetadata;
 import org.opensearch.test.framework.cluster.LocalCluster;
 import org.opensearch.test.framework.cluster.TestRestClient;
-import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.aMapWithSize;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
+import static org.junit.Assert.assertTrue;
 
 @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
 @ThreadLeakScope(ThreadLeakScope.Scope.NONE)
-public class DefaultConfigurationTests {
-
-    private final static Path configurationFolder = ConfigurationFiles.createConfigurationDirectory();
+public abstract class AbstractDefaultConfigurationTests {
+    public final static Path configurationFolder = ConfigurationFiles.createConfigurationDirectory();
     public static final String ADMIN_USER_NAME = "admin";
     public static final String DEFAULT_PASSWORD = "secret";
     public static final String NEW_USER = "new-user";
     public static final String LIMITED_USER = "limited-user";
 
-    @ClassRule
-    public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
-        .nodeSettings(
-            Map.of(
-                "plugins.security.allow_default_init_securityindex",
-                true,
-                "plugins.security.restapi.roles_enabled",
-                List.of("user_admin__all_access")
-            )
-        )
-        .defaultConfigurationInitDirectory(configurationFolder.toString())
-        .loadConfigurationIntoIndex(false)
-        .build();
+    private final LocalCluster cluster;
+
+    protected AbstractDefaultConfigurationTests(LocalCluster cluster) {
+        this.cluster = cluster;
+    }
 
     @AfterClass
     public static void cleanConfigurationDirectory() throws IOException {
         FileUtils.deleteDirectory(configurationFolder.toFile());
     }
 
     @Test
-    public void shouldLoadDefaultConfiguration() {
+    public void shouldLoadDefaultConfiguration() throws IOException {
         try (TestRestClient client = cluster.getRestClient(NEW_USER, DEFAULT_PASSWORD)) {
-            Awaitility.await().alias("Load default configuration").until(() -> client.getAuthInfo().getStatusCode(), equalTo(200));
+            Awaitility.waitAtMost(20, TimeUnit.SECONDS)
+                .await("Load default configuration")
+                .until(() -> client.getAuthInfo().getStatusCode(), equalTo(200));
         }
         try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
             client.confirmCorrectCredentials(ADMIN_USER_NAME);
-            HttpResponse response = client.get("_plugins/_security/api/internalusers");
+            TestRestClient.HttpResponse response = client.get("_plugins/_security/api/internalusers");
             response.assertStatusCode(200);
             Map<String, Object> users = response.getBodyAs(Map.class);
             assertThat(users, allOf(aMapWithSize(3), hasKey(ADMIN_USER_NAME), hasKey(NEW_USER), hasKey(LIMITED_USER)));
+
+            response = client.get("_cluster/state");
+            response.assertStatusCode(200);
+            final var clusterState = response.getBodyAs(Map.class);
+            assertTrue(response.getBody(), clusterState.containsKey(SecurityMetadata.TYPE));
+            @SuppressWarnings("unchecked")
+            final var securityClusterState = (Map<String, Object>) clusterState.get(SecurityMetadata.TYPE);
+            assertTrue(response.getBody(), (Boolean) securityClusterState.get(SecurityMetadata.SECURITY_CONFIGURATION_APPLIED_FIELD_NAME));
         }
     }
 }
diff --git a/...tegrationTest/java/org/opensearch/security/DefaultConfigurationMultiNodeClusterTests.java b/...tegrationTest/java/org/opensearch/security/DefaultConfigurationMultiNodeClusterTests.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ */
+package org.opensearch.security;
+
+import java.util.List;
+import java.util.Map;
+
+import org.junit.ClassRule;
+
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+
+public class DefaultConfigurationMultiNodeClusterTests extends AbstractDefaultConfigurationTests {
+
+    @ClassRule
+    public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
+        .nodeSettings(
+            Map.of(
+                "plugins.security.allow_default_init_securityindex",
+                true,
+                "plugins.security.restapi.roles_enabled",
+                List.of("user_admin__all_access")
+            )
+        )
+        .defaultConfigurationInitDirectory(configurationFolder.toString())
+        .loadConfigurationIntoIndex(false)
+        .build();
+
+    public DefaultConfigurationMultiNodeClusterTests() {
+        super(cluster);
+    }
+}
diff --git a/...egrationTest/java/org/opensearch/security/DefaultConfigurationSingleNodeClusterTests.java b/...egrationTest/java/org/opensearch/security/DefaultConfigurationSingleNodeClusterTests.java
@@ -0,0 +1,44 @@
+/*
+* Copyright OpenSearch Contributors
+* SPDX-License-Identifier: Apache-2.0
+*
+* The OpenSearch Contributors require contributions made to
+* this file be licensed under the Apache-2.0 license or a
+* compatible open source license.
+*
+*/
+package org.opensearch.security;
+
+import java.util.List;
+import java.util.Map;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import org.junit.ClassRule;
+import org.junit.runner.RunWith;
+
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+
+@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class DefaultConfigurationSingleNodeClusterTests extends AbstractDefaultConfigurationTests {
+
+    @ClassRule
+    public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
+        .nodeSettings(
+            Map.of(
+                "plugins.security.allow_default_init_securityindex",
+                true,
+                "plugins.security.restapi.roles_enabled",
+                List.of("user_admin__all_access")
+            )
+        )
+        .defaultConfigurationInitDirectory(configurationFolder.toString())
+        .loadConfigurationIntoIndex(false)
+        .build();
+
+    public DefaultConfigurationSingleNodeClusterTests() {
+        super(cluster);
+    }
+
+}
diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -72,6 +72,8 @@
 import org.opensearch.action.search.SearchScrollAction;
 import org.opensearch.action.support.ActionFilter;
 import org.opensearch.client.Client;
+import org.opensearch.cluster.ClusterState;
+import org.opensearch.cluster.NamedDiff;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
@@ -173,6 +175,9 @@
 import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor;
 import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport;
 import org.opensearch.security.ssl.util.SSLConfigConstants;
+import org.opensearch.security.state.SecurityClusterManagerListener;
+import org.opensearch.security.state.SecurityClusterNonManagerListener;
+import org.opensearch.security.state.SecurityMetadata;
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.security.support.GuardedSearchOperationWrapper;
 import org.opensearch.security.support.HeaderHelper;
@@ -1131,11 +1136,22 @@ public Collection<Object> createComponents(
         components.add(si);
         components.add(dcf);
         components.add(userService);
-
+        if (!SSLConfig.isSslOnlyMode() && !isDisabled(settings)) {
+            clusterService.addListener(new SecurityClusterManagerListener(clusterService, threadPool, cr, settings));
+            clusterService.addListener(new SecurityClusterNonManagerListener(settings, cr));
+        }
         return components;
 
     }
 
+    @Override
+    public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
+        return List.of(
+            new NamedWriteableRegistry.Entry(ClusterState.Custom.class, SecurityMetadata.TYPE, SecurityMetadata::new),
+            new NamedWriteableRegistry.Entry(NamedDiff.class, SecurityMetadata.TYPE, SecurityMetadata::readDiffFrom)
+        );
+    }
+
     @Override
     public Settings additionalSettings() {
 
@@ -1874,10 +1890,6 @@ public List<String> getSettingsFilter() {
 
     @Override
     public void onNodeStarted(DiscoveryNode localNode) {
-        log.info("Node started");
-        if (!SSLConfig.isSslOnlyMode() && !client && !disabled) {
-            cr.initOnNodeStart();
-        }
         this.localNode.set(localNode);
         final Set<ModuleInfo> securityModules = ReflectionHelper.getModulesLoaded();
         log.info("{} OpenSearch Security modules loaded so far: {}", securityModules.size(), securityModules);