Skip to content

Update TestClustersPlugin to accept system props installedPlugins and security.enabled for convenience installing plugins for integ tests #20393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cwperks wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Update TestClustersPlugin to accept system props installedPlugins and security.enabled for convenience installing plugins for integ tests #20393

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d93650a
Configure security in TestClustersPlugin
cwperks Jan 7, 2026
c9ba34f
Use non-deprecated method
cwperks Jan 7, 2026
766a6d3
Allow files to be read on the test classpath
cwperks Jan 8, 2026
bf72ab7
Add to CHANGELOG
cwperks Jan 8, 2026
296f912
Merge branch 'main' into test-clusters-security
cwperks Mar 22, 2026
6257992
Remove CHANGELOG entries
cwperks Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.opensearch.gradle.JdkDownloadPlugin;
import org.opensearch.gradle.ReaperPlugin;
import org.opensearch.gradle.ReaperService;
import org.opensearch.gradle.VersionProperties;
import org.opensearch.gradle.info.BuildParams;
import org.opensearch.gradle.info.GlobalBuildInfoPlugin;
import org.opensearch.gradle.internal.InternalDistributionDownloadPlugin;
Expand All @@ -52,10 +53,16 @@
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.TaskState;
import org.gradle.api.tasks.testing.Test;

import javax.inject.Inject;

import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.net.URI;
import java.util.Arrays;
import java.util.List;

import static org.opensearch.gradle.util.GradleUtils.noop;

Expand Down Expand Up @@ -111,6 +118,137 @@ public void apply(Project project) {

// register cluster hooks
project.getRootProject().getPluginManager().apply(TestClustersHookPlugin.class);

// configure installed plugins from -PinstalledPlugins property
configureInstalledPlugins(project, container);
}

@SuppressWarnings("unchecked")
private void configureInstalledPlugins(Project project, NamedDomainObjectContainer<OpenSearchCluster> container) {
String installedPluginsProp = System.getProperty("installedPlugins");
boolean securityEnabled = "true".equals(System.getProperty("security.enabled"));

if (installedPluginsProp == null && !securityEnabled) {
return;
}

project.afterEvaluate(p -> {
container.configureEach(cluster -> {
List<String> plugins = new java.util.ArrayList<>();

// Parse explicit plugin list if provided
if (installedPluginsProp != null) {
try {
plugins.addAll((List<String>) groovy.util.Eval.me(installedPluginsProp));
} catch (Exception e) {
throw new IllegalArgumentException("Invalid installedPlugins format. Expected list like: ['plugin1','plugin2']", e);
}
}
Comment on lines +140 to +146
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Code injection vulnerability via groovy.util.Eval.me().

Using groovy.util.Eval.me() to parse user input is dangerous. An attacker could inject arbitrary Groovy code via the installedPlugins system property, leading to arbitrary code execution during the build.

Consider using a safe JSON parser or simple string parsing instead.

🔒 Proposed fix using simple string parsing 
-                if (installedPluginsProp != null) {
-                    try {
-                        plugins.addAll((List<String>) groovy.util.Eval.me(installedPluginsProp));
-                    } catch (Exception e) {
-                        throw new IllegalArgumentException("Invalid installedPlugins format. Expected list like: ['plugin1','plugin2']", e);
-                    }
-                }
+                if (installedPluginsProp != null) {
+                    // Parse simple list format: ['plugin1','plugin2'] or plugin1,plugin2
+                    String cleaned = installedPluginsProp.trim();
+                    if (cleaned.startsWith("[") && cleaned.endsWith("]")) {
+                        cleaned = cleaned.substring(1, cleaned.length() - 1);
+                    }
+                    for (String part : cleaned.split(",")) {
+                        String pluginName = part.trim().replaceAll("^['\"]|['\"]$", "");
+                        if (!pluginName.isEmpty()) {
+                            plugins.add(pluginName);
+                        }
+                    }
+                }


// Auto-add security plugin if security is enabled and not already in list
if (securityEnabled && plugins.stream().noneMatch(p2 -> p2.contains("opensearch-security"))) {
plugins.add("opensearch-security");
}

for (String pluginName : plugins) {
// Check if it's a local project plugin
if (project.findProject(":plugins:" + pluginName) != null) {
cluster.plugin("plugins:" + pluginName);
} else {
// Resolve from Maven
project.getRepositories().mavenLocal();
project.getRepositories().maven(repo -> {
repo.setName("OpenSearch Snapshots");
repo.setUrl("https://ci.opensearch.org/ci/dbc/snapshots/maven/");
});

String version = VersionProperties.getOpenSearch().replace("-SNAPSHOT", ".0-SNAPSHOT");
String coords = pluginName.contains(":") ? pluginName : (pluginName + ":" + version);
if (!coords.contains("@")) {
coords += "@zip";
}
String fullCoords = coords.split(":").length < 3 ? "org.opensearch.plugin:" + coords : coords;

org.gradle.api.artifacts.Configuration config = project.getConfigurations()
.detachedConfiguration(project.getDependencies().create(fullCoords));
config.getResolutionStrategy().cacheChangingModulesFor(0, "seconds");
cluster.plugin(project.getLayout().file(project.provider(() -> config.getSingleFile())));

// Configure security if opensearch-security plugin is installed
if (pluginName.contains("opensearch-security")) {
configureSecurityPlugin(project, cluster);
}
}
}
Comment on lines +153 to +182
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Security plugin configuration skipped for local project installations.

When opensearch-security is found as a local project (line 155-156), the configureSecurityPlugin() method is never called. This means local builds of the security plugin won't have the necessary TLS/certificate configuration applied, causing inconsistent behavior between local and Maven-resolved plugins.

🐛 Proposed fix 
                 for (String pluginName : plugins) {
                     // Check if it's a local project plugin
                     if (project.findProject(":plugins:" + pluginName) != null) {
                         cluster.plugin("plugins:" + pluginName);
+                        // Configure security if opensearch-security plugin is installed locally
+                        if (pluginName.contains("opensearch-security")) {
+                            configureSecurityPlugin(project, cluster);
+                        }
                     } else {
                         // Resolve from Maven
                         // ... existing code ...
                     }
                 }
🤖 Prompt for AI Agents 
In
@buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java
around lines 153 - 182, The security plugin configuration is only applied for
Maven-resolved plugins; when a local project is detected via
project.findProject(":plugins:" + pluginName) the configureSecurityPlugin(...)
call is skipped. Fix this by adding the same conditional check used later to
call configureSecurityPlugin(project, cluster) immediately after the local
branch where cluster.plugin("plugins:" + pluginName) is invoked (i.e., inside
the if block that checks project.findProject), so that pluginName containing
"opensearch-security" triggers configureSecurityPlugin for local project plugins
as well.

});
});
}

private static final String[] SECURITY_CERT_FILES = {
"esnode.pem",
"esnode-key.pem",
"kirk.pem",
"kirk-key.pem",
"root-ca.pem",
"sample.pem",
"test-kirk.jks" };
private static final String SECURITY_CERTS_URL =
"https://raw.githubusercontent.com/opensearch-project/security/refs/heads/main/bwc-test/src/test/resources/security/";

private void configureSecurityPlugin(Project project, OpenSearchCluster cluster) {
File buildDir = project.getBuildDir();
File securityResourcesDir = new File(buildDir, "security-resources");

// Download certificates
for (String certFile : SECURITY_CERT_FILES) {
File localFile = new File(securityResourcesDir, certFile);
if (!localFile.exists()) {
try {
localFile.getParentFile().mkdirs();
try (
InputStream in = URI.create(SECURITY_CERTS_URL + certFile).toURL().openStream();
FileOutputStream out = new FileOutputStream(localFile)
) {
byte[] buffer = new byte[8192];
int bytesRead;
while ((bytesRead = in.read(buffer)) != -1) {
out.write(buffer, 0, bytesRead);
}
}
} catch (Exception e) {
throw new RuntimeException("Failed to download security certificate: " + certFile, e);
}
}
}
Comment on lines +203 to +222
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Security and reliability concerns with certificate download.

  1. Supply chain risk: Certificates are downloaded from a remote URL without integrity verification (no checksums). A compromised GitHub or MITM attack could inject malicious certificates.

  2. Race condition: The !localFile.exists() check followed by download is not atomic. Parallel builds could attempt to download simultaneously, potentially corrupting the file.

  3. Network reliability: Build failures will occur if the network is unavailable or the URL changes.

Consider:

  • Bundling certificates in the repository or a published artifact
  • Adding checksum verification for downloaded files
  • Using file locking for concurrent access
🔒 Suggested improvement with file locking 
             File localFile = new File(securityResourcesDir, certFile);
             if (!localFile.exists()) {
+                File lockFile = new File(securityResourcesDir, certFile + ".lock");
                 try {
                     localFile.getParentFile().mkdirs();
+                    // Use file-based locking for concurrent builds
+                    try (java.nio.channels.FileChannel channel = new java.io.RandomAccessFile(lockFile, "rw").getChannel();
+                         java.nio.channels.FileLock lock = channel.lock()) {
+                        if (localFile.exists()) {
+                            continue; // Another process downloaded it
+                        }
                     try (
                         InputStream in = URI.create(SECURITY_CERTS_URL + certFile).toURL().openStream();
                         FileOutputStream out = new FileOutputStream(localFile)
                     ) {
                         // ... download logic ...
                     }
+                    }
                 } catch (Exception e) {
                     throw new RuntimeException("Failed to download security certificate: " + certFile, e);
                 }
             }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
@buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java
around lines 203 - 222, The certificate download loop in TestClustersPlugin (the
for loop iterating SECURITY_CERT_FILES) is insecure and racy: replace the
on-the-fly download logic with a safe strategy — either bundle certificates as
resources/artifacts or, if remote retrieval is required, implement checksum
verification and an atomic, locked download flow: compute/obtain expected
checksums (per cert name), download to a temp file, verify the checksum, then
atomically move/rename into place, and guard concurrent access with a file lock
(or create-exclusive temp file + Files.move ATOMIC_MOVE) to prevent simultaneous
writers; update the RuntimeException message in the download block to include
the cert name and the underlying exception details to aid debugging.


// Add security-resources directory to test classpath
project.getTasks().withType(Test.class).configureEach(testTask -> {
testTask.setClasspath(testTask.getClasspath().plus(project.files(securityResourcesDir)));
});

// Configure each node with certificates and security settings
cluster.getNodes().forEach(node -> {
// Add certificate files
Arrays.stream(SECURITY_CERT_FILES).forEach(cert -> node.extraConfigFile(cert, new File(securityResourcesDir, cert)));
Comment on lines +229 to +232
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find the OpenSearchCluster class and understand getNodes()
find . -name "OpenSearchCluster.java" -type f

Repository: opensearch-project/OpenSearch

Length of output: 155

🏁 Script executed:

# Also search for any class that might define getNodes() for clusters
rg -l "class.*Cluster" --type java | head -20

Repository: opensearch-project/OpenSearch

Length of output: 1791

🏁 Script executed:

# Check the TestClustersPlugin file to understand the full context
rg -n "afterEvaluate" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ --type java -B5 -A10

Repository: opensearch-project/OpenSearch

Length of output: 2140

🏁 Script executed:

# Search for numberOfNodes to understand how nodes are added
rg -n "numberOfNodes" --type java -C3

Repository: opensearch-project/OpenSearch

Length of output: 50385

🏁 Script executed:

# Read the OpenSearchCluster.java file
head -100 buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java

Repository: opensearch-project/OpenSearch

Length of output: 3839

🏁 Script executed:

# Look for the getNodes() method and node initialization
rg -n "getNodes|addNode|numberOfNodes" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java -A3 -B3

Repository: opensearch-project/OpenSearch

Length of output: 2235

🏁 Script executed:

# Check how nodes are created in TestClustersPlugin - look at the full configuration context
sed -n '220,250p' buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java

Repository: opensearch-project/OpenSearch

Length of output: 2143

🏁 Script executed:

# Search for node creation/addition patterns in the plugin
rg -n "node|Node" buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java | grep -i "add\|create\|new" | head -20

Repository: opensearch-project/OpenSearch

Length of output: 206

🏁 Script executed:

# Search for setNumberOfNodes usage to understand when it's typically called
rg -n "setNumberOfNodes" --type java -B5 -A2

Repository: opensearch-project/OpenSearch

Length of output: 1003

🏁 Script executed:

# Check if there's documentation or examples of how clusters are configured
find buildSrc -name "*.gradle" -o -name "*.java" | xargs grep -l "setNumberOfNodes" | head -5

Repository: opensearch-project/OpenSearch

Length of output: 153

🏁 Script executed:

# Look for how test clusters are typically defined/configured
rg -n "testClusters" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ -A3 -B3 | head -40

Repository: opensearch-project/OpenSearch

Length of output: 1462

🏁 Script executed:

# Search for actual usage of setNumberOfNodes in build.gradle files
find . -name "*.gradle" -type f -exec grep -l "setNumberOfNodes\|numberOfNodes" {} \; | head -10

Repository: opensearch-project/OpenSearch

Length of output: 462

🏁 Script executed:

# Check one of these build.gradle files to see how testClusters are configured
rg -n "testClusters\|numberOfNodes" -A5 -B5 $(find . -name "*.gradle" -type f | head -3 | tr '\n' ' ')

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Check when the configuration freeze happens in OpenSearchCluster
rg -n "checkFrozen\|configurationFrozen\|freeze" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java -B2 -A2

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Look at the full afterEvaluate block to understand the complete configuration flow
sed -n '135,270p' buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java

Repository: opensearch-project/OpenSearch

Length of output: 7166

🏁 Script executed:

# Search for when freeze() is called on OpenSearchCluster
rg -n "freeze\|freeze()" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ -B3 -A3

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Look for the checkFrozen method to understand the lifecycle
rg -n "checkFrozen" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java -B2 -A5

Repository: opensearch-project/OpenSearch

Length of output: 598

🏁 Script executed:

# Check how test cluster DSL is typically used in actual build.gradle files
head -60 ./qa/smoke-test-multinode/build.gradle

Repository: opensearch-project/OpenSearch

Length of output: 1584

🏁 Script executed:

# Search for where freeze() is called to understand when configuration is locked
rg -n "\.freeze()" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ -B5 -A2

Repository: opensearch-project/OpenSearch

Length of output: 1230

🏁 Script executed:

# Check the task execution flow to understand when configuration is frozen
rg -n "beforeTasks\|afterTasks\|Task\|freeze" buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java | head -30

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Check when claimCluster is called relative to task execution
rg -n "claimCluster" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ -B3 -A3

Repository: opensearch-project/OpenSearch

Length of output: 1899

🏁 Script executed:

# Look for when nodes can be added after configuration
rg -n "setNumberOfNodes\|addNode" buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Check the full lifecycle - when does configuration actually freeze
rg -n "configurationFrozen\|freeze" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java -B5 -A5

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Check if configureEach is applied to newly created nodes
rg -n "configureEach\|allObjects\|whenObjectAdded" buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java -B2 -A3

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Look at line 136 to understand how configureEach is used
sed -n '135,145p' buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java

Repository: opensearch-project/OpenSearch

Length of output: 668

🏁 Script executed:

# Check if there's a way to listen for node additions
rg -n "node\|Node" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java | grep -i "all\|when\|listen" | head -10

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Look at the nodes container more closely to see if there's a better pattern
sed -n '100,160p' buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java

Repository: opensearch-project/OpenSearch

Length of output: 2266

🏁 Script executed:

# Check how nodes.all() is used elsewhere - this appears to be the pattern for deferred configuration
rg -n "\.all(" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java -B2 -A2

Repository: opensearch-project/OpenSearch

Length of output: 135

🏁 Script executed:

# Verify the nodes container supports all() method (NamedDomainObjectContainer)
rg -n "NamedDomainObjectContainer" buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchCluster.java

Repository: opensearch-project/OpenSearch

Length of output: 267

🏁 Script executed:

# Check if there are other forEach usages that might have the same issue
rg -n "\.forEach\(" buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java | grep -i node

Repository: opensearch-project/OpenSearch

Length of output: 396

🏁 Script executed:

# Check if setNumberOfNodes is always called before afterEvaluate in actual usage
rg -n "setNumberOfNodes\|numberOfNodes" $(find . -name "build.gradle" -type f) -B2 -A2 | head -50

Repository: opensearch-project/OpenSearch

Length of output: 55

🏁 Script executed:

# Check the Gradle container documentation behavior - does all() apply to future items?
# Let's look at how nodes.all() is used elsewhere in the codebase
rg -n "all\(" buildSrc/src/main/java/org/opensearch/gradle/testclusters/ -B2 -A3

Repository: opensearch-project/OpenSearch

Length of output: 22957

🏁 Script executed:

# Let's verify the exact flow - when is configureSecurityPlugin called vs when can setNumberOfNodes be called
sed -n '310,325p' buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java

Repository: opensearch-project/OpenSearch

Length of output: 928

Use nodes.all() instead of forEach() to ensure dynamically-added nodes receive security configuration.

The forEach() call only applies settings to nodes that exist when the method is executed. If nodes are added later via setNumberOfNodes(), they won't receive the security certificate and settings. Replace the forEach() with all(), which is the established pattern throughout OpenSearchCluster for applying configurations to all nodes (current and future).

Instead of:

cluster.getNodes().forEach(node -> {
    Arrays.stream(SECURITY_CERT_FILES).forEach(cert -> node.extraConfigFile(cert, new File(securityResourcesDir, cert)));
    // ... settings
});

Use:

cluster.getNodes().all(node -> {
    Arrays.stream(SECURITY_CERT_FILES).forEach(cert -> node.extraConfigFile(cert, new File(securityResourcesDir, cert)));
    // ... settings
});
🤖 Prompt for AI Agents 
In
@buildSrc/src/main/java/org/opensearch/gradle/testclusters/TestClustersPlugin.java
around lines 229 - 232, Replace the cluster.getNodes().forEach(...) call with
cluster.getNodes().all(...) so security certificate files and settings are
applied to nodes added later (e.g., via setNumberOfNodes()); inside the all(...)
lambda, keep the logic that iterates SECURITY_CERT_FILES and calls
node.extraConfigFile(...) and any other per-node security configuration so every
current and future node receives the certificate and security settings.


// Security settings
node.setting("plugins.security.ssl.transport.pemcert_filepath", "esnode.pem");
node.setting("plugins.security.ssl.transport.pemkey_filepath", "esnode-key.pem");
node.setting("plugins.security.ssl.transport.pemtrustedcas_filepath", "root-ca.pem");
node.setting("plugins.security.ssl.transport.enforce_hostname_verification", "false");
node.setting("plugins.security.ssl.http.enabled", "true");
node.setting("plugins.security.ssl.http.pemcert_filepath", "esnode.pem");
node.setting("plugins.security.ssl.http.pemkey_filepath", "esnode-key.pem");
node.setting("plugins.security.ssl.http.pemtrustedcas_filepath", "root-ca.pem");
node.setting("plugins.security.allow_unsafe_democertificates", "true");
node.setting("plugins.security.allow_default_init_securityindex", "true");
node.setting("plugins.security.authcz.admin_dn", "\n - CN=kirk,OU=client,O=client,L=test,C=de");
node.setting("plugins.security.audit.type", "internal_opensearch");
node.setting("plugins.security.enable_snapshot_restore_privilege", "true");
node.setting("plugins.security.check_snapshot_restore_write_privileges", "true");
node.setting("plugins.security.restapi.roles_enabled", "[\"all_access\", \"security_rest_api_access\"]");
node.setting("plugins.security.system_indices.enabled", "true");
});
}

private NamedDomainObjectContainer<OpenSearchCluster> createTestClustersContainerExtension(Project project, ReaperService reaper) {
Expand Down
Loading