-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
d93650a
c9ba34f
766a6d3
bf72ab7
296f912
6257992
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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; | ||
|
|
||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
||
| // 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security plugin configuration skipped for local project installations. When 🐛 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 |
||
| }); | ||
| }); | ||
| } | ||
|
|
||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security and reliability concerns with certificate download.
Consider:
🔒 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);
}
}
🤖 Prompt for AI Agents |
||
|
|
||
| // 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, find the OpenSearchCluster class and understand getNodes()
find . -name "OpenSearchCluster.java" -type fRepository: 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 -20Repository: 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 -A10Repository: opensearch-project/OpenSearch Length of output: 2140 🏁 Script executed: # Search for numberOfNodes to understand how nodes are added
rg -n "numberOfNodes" --type java -C3Repository: 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.javaRepository: 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 -B3Repository: 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.javaRepository: 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 -20Repository: 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 -A2Repository: 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 -5Repository: 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 -40Repository: 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 -10Repository: 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 -A2Repository: 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.javaRepository: 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 -A3Repository: 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 -A5Repository: 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.gradleRepository: 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 -A2Repository: 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 -30Repository: 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 -A3Repository: 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.javaRepository: 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 -A5Repository: 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 -A3Repository: 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.javaRepository: 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 -10Repository: 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.javaRepository: 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 -A2Repository: 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.javaRepository: 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 nodeRepository: 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 -50Repository: 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 -A3Repository: 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.javaRepository: opensearch-project/OpenSearch Length of output: 928 Use The 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 |
||
|
|
||
| // 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) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 theinstalledPluginssystem 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