Skip to content

Commit

Permalink
Switch UserInjectorPlugin to use reliable per-instance settings (open…
Browse files Browse the repository at this point in the history
…search-project#3760)

### Description

Fix flaky tests in TransportUserInjectorIntegTest by removing reliance
on a static variable that makes the test unpredictable. This instead
adds a setting in the UserInjectorPlugin and relies on each node that
has the plugin installed to supply this setting when instantiating the
node.

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)

Test Fix

### Issues Resolved

- Fixes opensearch-project#3729

### Check List
- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks authored Nov 22, 2023
1 parent c30c084 commit e9d3f26
Showing 1 changed file with 23 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

import com.google.common.collect.Lists;
Expand All @@ -26,7 +27,9 @@
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.env.Environment;
Expand All @@ -47,9 +50,10 @@

public class TransportUserInjectorIntegTest extends SingleClusterTest {

public static final String TEST_INJECTED_USER = "test_injected_user";

public static class UserInjectorPlugin extends Plugin implements ActionPlugin {
Settings settings;
public static String injectedUser = null;

public UserInjectorPlugin(final Settings settings, final Path configPath) {
this.settings = settings;
Expand All @@ -69,17 +73,24 @@ public Collection<Object> createComponents(
IndexNameExpressionResolver indexNameExpressionResolver,
Supplier<RepositoriesService> repositoriesServiceSupplier
) {
if (injectedUser != null) threadPool.getThreadContext()
.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUser);
if (!Strings.isNullOrEmpty(settings.get(TEST_INJECTED_USER))) threadPool.getThreadContext()
.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, settings.get(TEST_INJECTED_USER));
return new ArrayList<>();
}

@Override
public List<Setting<?>> getSettings() {
List<Setting<?>> settings = new ArrayList<Setting<?>>();
settings.add(Setting.simpleString(TEST_INJECTED_USER, Setting.Property.NodeScope, Setting.Property.Filtered));
return settings;
}
}

@Test
public void testSecurityUserInjection() throws Exception {
final Settings clusterNodeSettings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, true).build();
setup(clusterNodeSettings, new DynamicSecurityConfig().setSecurityRolesMapping("roles_transport_inject_user.yml"), Settings.EMPTY);
final Settings tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
final Settings.Builder tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
.put(minimumSecuritySettings(Settings.EMPTY).get(0))
.put("cluster.name", clusterInfo.clustername)
.put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data")
Expand All @@ -89,14 +100,13 @@ public void testSecurityUserInjection() throws Exception {
.put("discovery.initial_state_timeout", "8s")
.put("plugins.security.allow_default_init_securityindex", "true")
.put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, true)
.putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort)
.build();
.putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort);

// 1. without user injection
try (
Node node = new PluginAwareNode(
false,
tcSettings,
tcSettings.build(),
Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
).start()
) {
Expand All @@ -106,12 +116,11 @@ public void testSecurityUserInjection() throws Exception {
}

// 2. with invalid backend roles
UserInjectorPlugin.injectedUser = "ttt|kkk";
Exception exception = null;
try (
Node node = new PluginAwareNode(
false,
tcSettings,
tcSettings.put(TEST_INJECTED_USER, "ttt|kkk").build(),
Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
).start()
) {
Expand All @@ -126,11 +135,10 @@ public void testSecurityUserInjection() throws Exception {
}

// 3. with valid backend roles for injected user
UserInjectorPlugin.injectedUser = "injectedadmin|injecttest";
try (
Node node = new PluginAwareNode(
false,
tcSettings,
tcSettings.put(TEST_INJECTED_USER, "injectedadmin|injecttest").build(),
Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
).start()
) {
Expand All @@ -146,7 +154,7 @@ 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 = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
final Settings.Builder tcSettings = AbstractSecurityUnitTest.nodeRolesSettings(Settings.builder(), false, false)
.put(minimumSecuritySettings(Settings.EMPTY).get(0))
.put("cluster.name", clusterInfo.clustername)
.put("path.data", "./target/data/" + clusterInfo.clustername + "/cert/data")
Expand All @@ -156,14 +164,13 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception {
.put("discovery.initial_state_timeout", "8s")
.put("plugins.security.allow_default_init_securityindex", "true")
.put(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, false)
.putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort)
.build();
.putList("discovery.zen.ping.unicast.hosts", clusterInfo.nodeHost + ":" + clusterInfo.nodePort);

// 1. without user injection
try (
Node node = new PluginAwareNode(
false,
tcSettings,
tcSettings.build(),
Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
).start()
) {
Expand All @@ -173,11 +180,10 @@ public void testSecurityUserInjectionWithConfigDisabled() throws Exception {
}

// with invalid backend roles
UserInjectorPlugin.injectedUser = "ttt|kkk";
try (
Node node = new PluginAwareNode(
false,
tcSettings,
tcSettings.put(TEST_INJECTED_USER, "ttt|kkk").build(),
Lists.newArrayList(Netty4ModulePlugin.class, OpenSearchSecurityPlugin.class, UserInjectorPlugin.class)
).start()
) {
Expand Down

0 comments on commit e9d3f26

Please sign in to comment.