diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index f1e42e1663..ba036b2022 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -8,6 +8,7 @@ So you want to contribute code to this project? Excellent! We're glad you're her - [Using IntelliJ IDEA](#using-intellij-idea) - [Running integration tests](#running-integration-tests) - [Bulk test runs](#bulk-test-runs) + - [Checkstyle Violations](#checkstyle-violations) - [Submitting Changes](#submitting-changes) - [Backports](#backports) @@ -146,6 +147,35 @@ Integration tests are automatically run on all pull requests for all supported v ### Bulk test runs To collect reliability data on test runs there is a manual GitHub action workflow called `Bulk Integration Test`. The workflow is started for a branch on this project or in a fork by going to [GitHub action workflows](https://github.com/opensearch-project/security/actions/workflows/integration-tests.yml) and selecting `Run Workflow`. +### Checkstyle Violations +Checkstyle enforced several rules within this codebase. Sometimes exceptions will be necessary for components that are set for deprecation but the new version is unavailable. There are two formats of suppression that can be used when dealing with violations of this nature, one for disabling a single rule, or another for disabling all rules - its best to be as specific as possible. + +*Execute Checkstyle* +``` +./gradlew checkstyleMain checkstyleTest +``` + +*Example violation* +``` +[ant:checkstyle] [ERROR] /local/home/security/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java:178: Usage should be switched to cluster manager [RegexpSingleline] +``` + +*Single Rule Suppression* +``` + // CS-SUPPRESS-SINGLE: RegexpSingleline See http://github/issues/1234 + ... + Code that violates the rule + ... + // CS-ENFORCE-SINGLE +``` + +*Suppression All Checkstyle Rules* +``` + // CS-SUPRESS-ALL: Legacy code to be deleted in Z.Y.X see http://github/issues/1234 + ... + // CS-ENFORCE-ALL +``` + ## Submitting Changes See [CONTRIBUTING](CONTRIBUTING.md). diff --git a/checkstyle/sun_checks.xml b/checkstyle/sun_checks.xml index 099c8d39a5..5ffbedaf5a 100644 --- a/checkstyle/sun_checks.xml +++ b/checkstyle/sun_checks.xml @@ -201,7 +201,18 @@ - + + + + + + + + + + + + diff --git a/src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java b/src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java index b289eba0ef..c0569e8390 100644 --- a/src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java +++ b/src/main/java/org/opensearch/security/configuration/ClusterInfoHolder.java @@ -77,7 +77,7 @@ public void clusterChanged(ClusterChangedEvent event) { initialized = true; } - isLocalNodeElectedClusterManager = event.localNodeMaster()?Boolean.TRUE:Boolean.FALSE; + isLocalNodeElectedClusterManager = event.localNodeClusterManager()?Boolean.TRUE:Boolean.FALSE; } public Boolean getHas6xNodes() { diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 4a81e517a0..f69e736b8f 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -174,7 +174,9 @@ public boolean invoke(String action, ActionRequest request, final ActionListener //When we encounter a terms or sampler aggregation with masked fields activated we forcibly //need to switch off global ordinals because field masking can break ordering + // CS-SUPPRESS-SINGLE: RegexpSingleline Ignore term inside of url //https://www.elastic.co/guide/en/elasticsearch/reference/master/eager-global-ordinals.html#_avoiding_global_ordinal_loading + // CS-ENFORCE-SINGLE if (evaluatedDlsFlsConfig.hasFieldMasking()) { if (searchRequest.source() != null && searchRequest.source().aggregations() != null) { diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java index 7ea87cba09..3403dc1ee8 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java @@ -11,6 +11,7 @@ package org.opensearch.security.dlic.rest.api; +// CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663 import java.io.IOException; import java.nio.file.Path; import java.util.Collections; @@ -69,6 +70,7 @@ import org.opensearch.threadpool.ThreadPool; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; +// CS-ENFORCE-SINGLE public class MigrateApiAction extends AbstractApiAction { private static final List routes = addRoutesPrefix(Collections.singletonList( diff --git a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java index 0329ebf6fe..2553a13677 100644 --- a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java +++ b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java @@ -26,6 +26,7 @@ package org.opensearch.security.tools; +// CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663 import java.io.ByteArrayInputStream; import java.io.Console; import java.io.File; @@ -139,6 +140,7 @@ import static org.opensearch.common.xcontent.DeprecationHandler.THROW_UNSUPPORTED_OPERATION; import static org.opensearch.security.support.SecurityUtils.replaceEnvVars; +// CS-ENFORCE-SINGLE @SuppressWarnings("deprecation") public class SecurityAdmin { diff --git a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java index 9a356ff92e..8a4129e32b 100644 --- a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java @@ -45,6 +45,7 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.threadpool.ThreadPool; @@ -83,12 +84,9 @@ public void testRolesInject() throws Exception { Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster(). health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java index 588bcbb7fc..57a2d45a28 100644 --- a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java @@ -39,6 +39,7 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.threadpool.ThreadPool; @@ -74,12 +75,9 @@ public Collection createComponents(Client client, ClusterService cluster public void testRolesValidation() throws Exception { setup(Settings.EMPTY, new DynamicSecurityConfig().setSecurityRoles("roles.yml"), Settings.EMPTY); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/SlowIntegrationTests.java b/src/test/java/org/opensearch/security/SlowIntegrationTests.java index 6352c920ea..fd01dc7bdd 100644 --- a/src/test/java/org/opensearch/security/SlowIntegrationTests.java +++ b/src/test/java/org/opensearch/security/SlowIntegrationTests.java @@ -42,6 +42,7 @@ import org.opensearch.node.PluginAwareNode; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.cluster.ClusterConfiguration; import org.opensearch.security.test.helper.file.FileHelper; @@ -70,12 +71,9 @@ public void testNodeClientAllowedWithServerCertificate() throws Exception { Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") @@ -100,12 +98,9 @@ public void testNodeClientDisallowedWithNonServerCertificate() throws Exception Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") @@ -134,12 +129,9 @@ public void testNodeClientDisallowedWithNonServerCertificate2() throws Exception Assert.assertEquals(clusterInfo.numNodes, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getNumberOfNodes()); Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java index 56a7b727b4..4f3105501f 100644 --- a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java @@ -37,6 +37,7 @@ import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.threadpool.ThreadPool; @@ -72,12 +73,9 @@ public void testSecurityUserInjection() throws Exception { .put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, true) .build(); setup(clusterNodeSettings, new DynamicSecurityConfig().setSecurityRolesMapping("roles_transport_inject_user.yml"), Settings.EMPTY); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") @@ -129,12 +127,9 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception { .put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, false) .build(); setup(clusterNodeSettings, new DynamicSecurityConfig().setSecurityRolesMapping("roles_transport_inject_user.yml"), Settings.EMPTY); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java index e6b533c0b9..acd5e37b68 100644 --- a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java +++ b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java @@ -84,7 +84,9 @@ public ClusterTransportClientSettings() { } public ClusterTransportClientSettings(Settings clusterSettings, Settings transportSettings) { - super(clusterSettings, transportSettings); + super(Settings.builder() + .put(clusterSettings) + .putList("node.roles", "remote_cluster_client").build(), transportSettings); } public Settings clusterSettings() { @@ -1001,12 +1003,10 @@ public void testCcsWithRoleInjection() throws Exception { .source("{\"cluster\": \""+cl2Info.clustername+"\"}", XContentType.JSON)).actionGet(); } - final Settings tcSettings = Settings.builder() + final Settings.Builder clusterClientSettings = Settings.builder().putList("node.roles", "remote_cluster_client"); + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(clusterClientSettings, false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", cl1Info.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + cl1Info.clustername + "/cert/data") .put("path.logs", "./target/data/" + cl1Info.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java index 51cab5107f..720f59980d 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.dlic.dlsfls; +// CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663 import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -59,12 +60,14 @@ import org.opensearch.script.ScriptService; import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.Netty4Plugin; import org.opensearch.transport.TransportService; import org.opensearch.watcher.ResourceWatcherService; +// CS-ENFORCE-SINGLE public class CCReplicationTest extends AbstractDlsFlsTest { public static class MockReplicationPlugin extends Plugin implements ActionPlugin { @@ -180,14 +183,11 @@ public void testReplication() throws Exception { Assert.assertEquals(clusterInfo.numNodes, clusterHelper.nodeClient().admin().cluster().health( new ClusterHealthRequest().waitForGreenStatus()).actionGet().getNumberOfNodes()); Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster(). - health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); + health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus()); - final Settings tcSettings = Settings.builder() + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) .put(minimumSecuritySettings(Settings.EMPTY).get(0)) .put("cluster.name", clusterInfo.clustername) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/cert/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsFlsCrossClusterSearchTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsFlsCrossClusterSearchTest.java index d44d5b4f6e..3fd7d0a406 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsFlsCrossClusterSearchTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsFlsCrossClusterSearchTest.java @@ -47,7 +47,7 @@ private void setupCcs(String remoteRoles) throws Exception { System.setProperty("security.display_lic_none","true"); - cl2Info = cl2.startCluster(minimumSecuritySettings(Settings.EMPTY), ClusterConfiguration.DEFAULT); + cl2Info = cl2.startCluster(minimumSecuritySettings(Settings.builder().putList("node.roles", "remote_cluster_client").build()), ClusterConfiguration.DEFAULT); initialize(cl2, cl2Info, new DynamicSecurityConfig().setSecurityRoles(remoteRoles)); System.out.println("### cl2 complete ###"); @@ -66,7 +66,8 @@ public void tearDown() throws Exception { private Settings crossClusterNodeSettings(ClusterInfo remote) { Settings.Builder builder = Settings.builder() - .putList("cluster.remote.cross_cluster_two.seeds", remote.nodeHost+":"+remote.nodePort); + .putList("cluster.remote.cross_cluster_two.seeds", remote.nodeHost+":"+remote.nodePort) + .putList("node.roles", "remote_cluster_client"); return builder.build(); } diff --git a/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java b/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java index 790477ffe3..961eadeab5 100644 --- a/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java +++ b/src/test/java/org/opensearch/security/ssl/OpenSSLTest.java @@ -40,6 +40,7 @@ import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper; import org.opensearch.transport.Netty4Plugin; @@ -206,11 +207,9 @@ public void testNodeClientSSLwithOpenSslTLSv13() throws Exception { RestHelper rh = nonSslRestHelper(); - final Settings tcSettings = Settings.builder().put("cluster.name", clusterInfo.clustername).put("path.home", "/tmp") + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) + .put("cluster.name", clusterInfo.clustername).put("path.home", "/tmp") .put("node.name", "client_node_" + new Random().nextInt()) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/" + clusterInfo.clustername + "/ssl/data") .put("path.logs", "./target/data/" + clusterInfo.clustername + "/ssl/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/ssl/SSLTest.java b/src/test/java/org/opensearch/security/ssl/SSLTest.java index d98ee3f3f2..ab28b4a88f 100644 --- a/src/test/java/org/opensearch/security/ssl/SSLTest.java +++ b/src/test/java/org/opensearch/security/ssl/SSLTest.java @@ -56,6 +56,7 @@ import org.opensearch.security.ssl.util.ExceptionUtils; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper; @@ -495,11 +496,9 @@ public void testNodeClientSSL() throws Exception { RestHelper rh = nonSslRestHelper(); - final Settings tcSettings = Settings.builder().put("cluster.name", clusterInfo.clustername).put("path.home", ".") + final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false) + .put("cluster.name", clusterInfo.clustername).put("path.home", ".") .put("node.name", "client_node_" + new Random().nextInt()) - .put("node.data", false) - .put("node.master", false) - .put("node.ingest", false) .put("path.data", "./target/data/"+clusterInfo.clustername+"/ssl/data") .put("path.logs", "./target/data/"+clusterInfo.clustername+"/ssl/logs") .put("path.home", "./target") diff --git a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java index f9913d9478..b95104dd9f 100644 --- a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java +++ b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java @@ -42,6 +42,8 @@ import com.carrotsearch.randomizedtesting.RandomizedTest; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import io.netty.handler.ssl.OpenSsl; import org.apache.http.Header; import org.apache.http.HttpHost; @@ -69,6 +71,7 @@ import org.opensearch.client.RestClient; import org.opensearch.client.RestClientBuilder; import org.opensearch.client.RestHighLevelClient; +import org.opensearch.cluster.node.DiscoveryNodeRole; import org.opensearch.common.settings.Settings; import org.opensearch.security.action.configupdate.ConfigUpdateAction; import org.opensearch.security.action.configupdate.ConfigUpdateRequest; @@ -93,6 +96,7 @@ @ThreadLeakScope(Scope.NONE) public abstract class AbstractSecurityUnitTest extends RandomizedTest { + private static final String NODE_ROLE_KEY = "node.roles"; protected static final AtomicLong num = new AtomicLong(); protected static boolean withRemoteCluster; @@ -194,6 +198,28 @@ public void waitForInit(Client client) { } } + public static Settings.Builder nodeRolesSettings(final Settings.Builder settingsBuilder, final boolean isClusterManager, final boolean isDataNode) { + final ImmutableList.Builder nodeRolesBuilder = ImmutableList.builder(); + if (isDataNode) { + nodeRolesBuilder.add(DiscoveryNodeRole.DATA_ROLE.roleName()); + } + if (isClusterManager) { + nodeRolesBuilder.add(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName()); + } + + final Settings nodeRoleSettings = Settings.builder().putList(NODE_ROLE_KEY, nodeRolesBuilder.build()).build(); + return mergeNodeRolesAndSettings(settingsBuilder, nodeRoleSettings); + } + + public static Settings.Builder mergeNodeRolesAndSettings(final Settings.Builder settingsBuilder, final Settings otherSettings) { + final ImmutableSet.Builder originalRoles = ImmutableSet.builder() + .addAll(settingsBuilder.build().getAsList(NODE_ROLE_KEY, ImmutableList.of())) + .addAll(otherSettings.getAsList(NODE_ROLE_KEY, ImmutableList.of())); + + return settingsBuilder.put(otherSettings) + .putList(NODE_ROLE_KEY, originalRoles.build().asList()); + } + protected void initialize(ClusterHelper clusterHelper, ClusterInfo clusterInfo, DynamicSecurityConfig securityConfig) throws IOException { try (Client tc = clusterHelper.nodeClient()) { Assert.assertEquals(clusterInfo.numNodes, @@ -244,7 +270,6 @@ protected Settings.Builder minimumSecuritySettingsBuilder(int node, boolean sslO } builder.put("cluster.routing.allocation.disk.threshold_enabled", false); builder.put(other); - return builder; } diff --git a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java index fdbef60d70..5aea8f7dfe 100644 --- a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java +++ b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterHelper.java @@ -26,6 +26,7 @@ package org.opensearch.security.test.helper.cluster; +// CS-SUPPRESS-SINGLE: RegexpSingleline https://github.com/opensearch-project/OpenSearch/issues/3663 import java.io.File; import java.io.IOException; import java.util.Comparator; @@ -60,11 +61,13 @@ import org.opensearch.http.HttpInfo; import org.opensearch.node.Node; import org.opensearch.node.PluginAwareNode; +import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.NodeSettingsSupplier; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.cluster.ClusterConfiguration.NodeSettings; import org.opensearch.security.test.helper.network.SocketUtils; import org.opensearch.transport.TransportInfo; +// CS-ENFORCE-SINGLE public final class ClusterHelper { @@ -171,9 +174,15 @@ public final synchronized ClusterInfo startCluster(final NodeSettingsSupplier no for (int i = 0; i < internalClusterManagerNodeSettings.size(); i++) { NodeSettings setting = internalClusterManagerNodeSettings.get(i); int nodeNum = nodeNumCounter--; - PluginAwareNode node = new PluginAwareNode(setting.clusterManagerNode, - getMinimumNonSecurityNodeSettingsBuilder(nodeNum, setting.clusterManagerNode, setting.dataNode, internalNodeSettings.size(), tcpClusterManagerPortsOnly, tcpPortsAllIt.next(), httpPortsIt.next()) - .put(nodeSettingsSupplier == null ? Settings.Builder.EMPTY_SETTINGS : nodeSettingsSupplier.get(nodeNum)).build(), setting.getPlugins()); + final Settings.Builder nodeSettingsBuilder = getMinimumNonSecurityNodeSettingsBuilder(nodeNum, setting.clusterManagerNode, setting.dataNode, internalNodeSettings.size(), tcpClusterManagerPortsOnly, tcpPortsAllIt.next(), httpPortsIt.next()); + final Settings settingsForNode; + if (nodeSettingsSupplier != null) { + final Settings suppliedSettings = nodeSettingsSupplier.get(nodeNum); + settingsForNode = AbstractSecurityUnitTest.mergeNodeRolesAndSettings(nodeSettingsBuilder, suppliedSettings).build(); + } else { + settingsForNode = nodeSettingsBuilder.build(); + } + PluginAwareNode node = new PluginAwareNode(setting.clusterManagerNode, settingsForNode, setting.getPlugins()); System.out.println(node.settings()); new Thread(new Runnable() { @@ -197,9 +206,15 @@ public void run() { for (int i = 0; i < internalNonClusterManagerNodeSettings.size(); i++) { NodeSettings setting = internalNonClusterManagerNodeSettings.get(i); int nodeNum = nodeNumCounter--; - PluginAwareNode node = new PluginAwareNode(setting.clusterManagerNode, - getMinimumNonSecurityNodeSettingsBuilder(nodeNum, setting.clusterManagerNode, setting.dataNode, internalNodeSettings.size(), tcpClusterManagerPortsOnly, tcpPortsAllIt.next(), httpPortsIt.next()) - .put(nodeSettingsSupplier == null ? Settings.Builder.EMPTY_SETTINGS : nodeSettingsSupplier.get(nodeNum)).build(), setting.getPlugins()); + final Settings.Builder nodeSettingsBuilder = getMinimumNonSecurityNodeSettingsBuilder(nodeNum, setting.clusterManagerNode, setting.dataNode, internalNodeSettings.size(), tcpClusterManagerPortsOnly, tcpPortsAllIt.next(), httpPortsIt.next()); + final Settings settingsForNode; + if (nodeSettingsSupplier != null) { + final Settings suppliedSettings = nodeSettingsSupplier.get(nodeNum); + settingsForNode = AbstractSecurityUnitTest.mergeNodeRolesAndSettings(nodeSettingsBuilder, suppliedSettings).build(); + } else { + settingsForNode = nodeSettingsBuilder.build(); + } + PluginAwareNode node = new PluginAwareNode(setting.clusterManagerNode, settingsForNode, setting.getPlugins()); System.out.println(node.settings()); new Thread(new Runnable() { @@ -293,7 +308,7 @@ public ClusterInfo waitForCluster(final ClusterHealthStatus status, final TimeVa try { log.debug("waiting for cluster state {} and {} nodes", status.name(), expectedNodeCount); final ClusterHealthResponse healthResponse = client.admin().cluster().prepareHealth() - .setWaitForStatus(status).setTimeout(timeout).setMasterNodeTimeout(timeout).setWaitForNodes("" + expectedNodeCount).execute() + .setWaitForStatus(status).setTimeout(timeout).setClusterManagerNodeTimeout(timeout).setWaitForNodes("" + expectedNodeCount).execute() .actionGet(); if (healthResponse.isTimedOut()) { throw new IOException("cluster state is " + healthResponse.getStatus().name() + " with " @@ -366,13 +381,11 @@ public ClusterInfo waitForCluster(final ClusterHealthStatus status, final TimeVa } // @formatter:off - private Settings.Builder getMinimumNonSecurityNodeSettingsBuilder(final int nodenum, final boolean clusterManagerNode, - final boolean dataNode, int nodeCount, SortedSet clusterManagerTcpPorts, int tcpPort, int httpPort) { + private Settings.Builder getMinimumNonSecurityNodeSettingsBuilder(final int nodenum, final boolean isClusterManagerNode, + final boolean isDataNode, int nodeCount, SortedSet clusterManagerTcpPorts, int tcpPort, int httpPort) { - return Settings.builder() + return AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), isClusterManagerNode, isDataNode) .put("node.name", "node_"+clustername+ "_num" + nodenum) - .put("node.data", dataNode) - .put("node.master", clusterManagerNode) .put("cluster.name", clustername) .put("path.data", "./target/data/"+clustername+"/data") .put("path.logs", "./target/data/"+clustername+"/logs")