Skip to content
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

Security checks fail for ESSingleNodeTestCase on external plugin with IntelliJ #33045

Closed
mattweber opened this issue Aug 22, 2018 · 3 comments · Fixed by #33066
Closed

Security checks fail for ESSingleNodeTestCase on external plugin with IntelliJ #33045

mattweber opened this issue Aug 22, 2018 · 3 comments · Fixed by #33066
Assignees
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team

Comments

@mattweber
Copy link
Contributor

I get errors when running ESSingleNodeTestCase tests via IntelliJ while developing an external plugin. Tracked this down to the forced insertion of elasticsearch-secure-sm.

The problem is that when we are using an external plugin it is already on the classpath with version and jar in the filename, ie. elasticsearch-secure-sm-6.3.2.jar so when we do the alias and previous value check in Security we get the duplicate and thus an IllegalStateException. We completely miss the duplication check that already exists when force adding elasticsearch-secure-sm due to this.

Not sure what the best way to fix this is:

  1. Do we check if the previous value is not null AND not equal to the value we want to set here.

  2. Maybe make getCodebaseJarMap calculate the alias and use it as the map key?

  3. Something else?

I can work on the PR once I know how you would like me to proceed.

@colings86 colings86 added the :Delivery/Build Build or test infrastructure label Aug 22, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@mattweber
Copy link
Contributor Author

mattweber commented Aug 22, 2018

I think this might be the way to go:

--- a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java
+++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java
@@ -36,7 +36,9 @@ import org.junit.Assert;

 import java.io.InputStream;
 import java.net.SocketPermission;
+import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.Permission;
 import java.security.Permissions;
@@ -177,8 +179,23 @@ public class BootstrapForTesting {
     private static void addClassCodebase(Map<String, URL> codebases, String name, String classname) {
         try {
             Class<?> clazz = BootstrapForTesting.class.getClassLoader().loadClass(classname);
-            if (codebases.put(name, clazz.getProtectionDomain().getCodeSource().getLocation()) != null) {
-                throw new IllegalStateException("Already added " + name + " codebase for testing");
+            URL location = clazz.getProtectionDomain().getCodeSource().getLocation();
+            try {
+                // if we find the class in a file (jar), make sure that jar is not already in the codebases map
+                // this will be the case when using test framework externally
+                Path filePath = PathUtils.get(location.toURI());
+                if (Files.isRegularFile(filePath)) {
+                    String fileName = filePath.getFileName().toString();
+                    if (!codebases.containsKey(name) && !codebases.containsKey(fileName)) {
+                        codebases.put(name, location);
+                    }
+                } else {
+                    if (codebases.put(name, location) != null) {
+                        throw new IllegalStateException("Already added " + name + " codebase for testing");
+                    }
+                }
+            } catch (URISyntaxException e) {
+                throw new RuntimeException(e);
             }
         } catch (ClassNotFoundException e) {
             // no class, fall through to not add. this can happen for any tests that do not include

@rjernst
Copy link
Member

rjernst commented Aug 22, 2018

Thanks for the report @mattweber. I will take a look at this in the next few days.

@rjernst rjernst self-assigned this Aug 22, 2018
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants