From 3ecf401f2f52f784fa59031e9c659a3525ab3c9e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 22 Aug 2019 15:43:01 -0700 Subject: [PATCH 1/8] Make reuse of sql test code explicit The sql project uses a common set of security tests, which are run in subprojects. Currently these are shared through a shared directory, but this is not setup correctly to ensure it is built before tests run. This commit changes the test classes to be an artifact of the sql/qa/security project and makes the test runner use the built artifact (a directory of classes) for tests. closes #45866 --- x-pack/plugin/sql/qa/build.gradle | 3 +++ x-pack/plugin/sql/qa/security/build.gradle | 29 +++++++++++----------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/sql/qa/build.gradle b/x-pack/plugin/sql/qa/build.gradle index f33fd4a430312..bcc95a8317214 100644 --- a/x-pack/plugin/sql/qa/build.gradle +++ b/x-pack/plugin/sql/qa/build.gradle @@ -47,6 +47,9 @@ forbiddenApisMain { thirdPartyAudit.enabled = false subprojects { + if (subprojects.isEmpty() == false) { + return // not a leaf project + } apply plugin: 'elasticsearch.standalone-rest-test' dependencies { diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index 2774c4b85f4a5..de682e6ce6139 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -1,3 +1,6 @@ + +apply plugin: 'elasticsearch.build' + dependencies { testCompile project(':x-pack:plugin:core') } @@ -6,27 +9,23 @@ Project mainProject = project group = "${group}.x-pack.qa.sql.security" +configurations.create('testClasses') + +String classesDir = project.file(project.sourceSets.main.output.classesDirs.singleFile).toString() +artifacts.add('testClasses', project.layout.projectDirectory.dir(classesDir)) { + builtBy tasks.named('testClasses') +} + // Tests are pushed down to subprojects and will be checked there. testingConventions.enabled = false subprojects { - // Use resources from the parent project in subprojects - sourceSets { - test { - mainProject.sourceSets.test.output.classesDirs.each { dir -> - output.addClassesDir { dir } - output.builtBy(mainProject.tasks.testClasses) - } - runtimeClasspath += mainProject.sourceSets.test.output - } - } - - processTestResources { - from mainProject.file('src/test/resources') - } + // Use tests from the root security qa project in subprojects + configurations.create('testClasses') dependencies { testCompile project(":x-pack:plugin:core") + testClasses project(path: mainProject.path, configuration: 'testClasses') } testClusters.integTest { @@ -43,6 +42,8 @@ subprojects { } integTest.runner { + dependsOn configurations.testClasses + testClassesDirs += project.files(configurations.testClasses.singleFile) nonInputProperties.systemProperty 'tests.audit.logfile', "${ -> testClusters.integTest.singleNode().getAuditLog()}" nonInputProperties.systemProperty 'tests.audit.yesterday.logfile', From 48353e979854412ee78c706f10468012f631a640 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 23 Aug 2019 10:15:33 -0700 Subject: [PATCH 2/8] more tries --- x-pack/plugin/sql/qa/build.gradle | 6 ++++-- x-pack/plugin/sql/qa/security/build.gradle | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/sql/qa/build.gradle b/x-pack/plugin/sql/qa/build.gradle index bcc95a8317214..c9bc299606189 100644 --- a/x-pack/plugin/sql/qa/build.gradle +++ b/x-pack/plugin/sql/qa/build.gradle @@ -48,9 +48,11 @@ thirdPartyAudit.enabled = false subprojects { if (subprojects.isEmpty() == false) { - return // not a leaf project + // leaf project + apply plugin: 'elasticsearch.standalone-rest-test' + } else { + apply plugin: 'elasticsearch.build' } - apply plugin: 'elasticsearch.standalone-rest-test' dependencies { /* Since we're a standalone rest test we actually get transitive diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index de682e6ce6139..e240fe251fdf7 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -25,6 +25,7 @@ subprojects { dependencies { testCompile project(":x-pack:plugin:core") + testCompile project(":test:framework") testClasses project(path: mainProject.path, configuration: 'testClasses') } From 190bab425e636589c1d1390d09a15604f901fb9e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 24 Aug 2019 14:23:52 -0700 Subject: [PATCH 3/8] WIP tests running but failing with CNFE --- x-pack/plugin/sql/qa/build.gradle | 2 +- x-pack/plugin/sql/qa/security/build.gradle | 5 +---- .../xpack/sql/qa/security/SqlSecurityTestCase.java | 4 +++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/sql/qa/build.gradle b/x-pack/plugin/sql/qa/build.gradle index c9bc299606189..d3ebd8bfc92e8 100644 --- a/x-pack/plugin/sql/qa/build.gradle +++ b/x-pack/plugin/sql/qa/build.gradle @@ -47,7 +47,7 @@ forbiddenApisMain { thirdPartyAudit.enabled = false subprojects { - if (subprojects.isEmpty() == false) { + if (subprojects.isEmpty()) { // leaf project apply plugin: 'elasticsearch.standalone-rest-test' } else { diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index e240fe251fdf7..b0f1899b7f5c9 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -1,6 +1,4 @@ -apply plugin: 'elasticsearch.build' - dependencies { testCompile project(':x-pack:plugin:core') } @@ -11,7 +9,7 @@ group = "${group}.x-pack.qa.sql.security" configurations.create('testClasses') -String classesDir = project.file(project.sourceSets.main.output.classesDirs.singleFile).toString() +String classesDir = project.file(project.sourceSets.test.output.classesDirs.singleFile).toString() artifacts.add('testClasses', project.layout.projectDirectory.dir(classesDir)) { builtBy tasks.named('testClasses') } @@ -25,7 +23,6 @@ subprojects { dependencies { testCompile project(":x-pack:plugin:core") - testCompile project(":test:framework") testClasses project(path: mainProject.path, configuration: 'testClasses') } diff --git a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java index aaf028181a156..7340a1ab9332e 100644 --- a/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java +++ b/x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/SqlSecurityTestCase.java @@ -641,7 +641,9 @@ public void assertLogs() throws Exception { assertThat(log.containsKey("user.name"), is(true)); List indices = new ArrayList<>(); if (log.containsKey("indices")) { - indices = (ArrayList) log.get("indices"); + @SuppressWarnings("unchecked") + List castIndices = (ArrayList) log.get("indices"); + indices = castIndices; if ("test_admin".equals(log.get("user.name"))) { /* * Sometimes we accidentally sneak access to the security tables. This is fine, From c064e73f72d43618625dcc99a7a4c56d71955f60 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 28 Aug 2019 14:00:36 -0700 Subject: [PATCH 4/8] WIP, working on a fix --- x-pack/plugin/sql/qa/security/build.gradle | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index b0f1899b7f5c9..835de30995cd1 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -9,9 +9,9 @@ group = "${group}.x-pack.qa.sql.security" configurations.create('testClasses') -String classesDir = project.file(project.sourceSets.test.output.classesDirs.singleFile).toString() +String classesDir = sourceSets.test.output.classesDirs.singleFile.toString() artifacts.add('testClasses', project.layout.projectDirectory.dir(classesDir)) { - builtBy tasks.named('testClasses') + builtBy sourceSets.test.output } // Tests are pushed down to subprojects and will be checked there. @@ -40,8 +40,7 @@ subprojects { } integTest.runner { - dependsOn configurations.testClasses - testClassesDirs += project.files(configurations.testClasses.singleFile) + testClassesDirs += configurations.testClasses nonInputProperties.systemProperty 'tests.audit.logfile', "${ -> testClusters.integTest.singleNode().getAuditLog()}" nonInputProperties.systemProperty 'tests.audit.yesterday.logfile', From d285fa3713ccaf889cc8b21d9ba227a0d3a91e44 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 29 Aug 2019 10:18:00 -0700 Subject: [PATCH 5/8] WIP... --- x-pack/plugin/sql/qa/build.gradle | 7 ++++- x-pack/plugin/sql/qa/security/build.gradle | 30 ++++++++++++++++------ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/sql/qa/build.gradle b/x-pack/plugin/sql/qa/build.gradle index d3ebd8bfc92e8..05e57a51f374f 100644 --- a/x-pack/plugin/sql/qa/build.gradle +++ b/x-pack/plugin/sql/qa/build.gradle @@ -53,6 +53,10 @@ subprojects { } else { apply plugin: 'elasticsearch.build' } + + configurations.testRuntimeClasspath { + resolutionStrategy.force "org.slf4j:slf4j-api:1.7.25" + } dependencies { /* Since we're a standalone rest test we actually get transitive @@ -70,7 +74,8 @@ subprojects { // H2GIS testing dependencies testRuntime ("org.orbisgis:h2gis:${h2gisVersion}") { - exclude group: "org.locationtech.jts" + exclude group: "org.locationtech.jts" + exclude group: "com.fasterxml.jackson.core" } testRuntime project(path: xpackModule('sql:jdbc'), configuration: 'nodeps') diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index 835de30995cd1..eeadd52223d11 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -7,23 +7,27 @@ Project mainProject = project group = "${group}.x-pack.qa.sql.security" -configurations.create('testClasses') +configurations.create('testArtifacts') -String classesDir = sourceSets.test.output.classesDirs.singleFile.toString() -artifacts.add('testClasses', project.layout.projectDirectory.dir(classesDir)) { - builtBy sourceSets.test.output -} +TaskProvider testJar = tasks.register("testJar", Jar) { + appendix 'test' + from sourceSets.test.output +} + +artifacts { + testArtifacts testJar +} // Tests are pushed down to subprojects and will be checked there. testingConventions.enabled = false subprojects { // Use tests from the root security qa project in subprojects - configurations.create('testClasses') + configurations.create('testArtifacts') dependencies { testCompile project(":x-pack:plugin:core") - testClasses project(path: mainProject.path, configuration: 'testClasses') + testArtifacts project(path: mainProject.path, configuration: 'testArtifacts') } testClusters.integTest { @@ -39,11 +43,21 @@ subprojects { user username: "test_admin", password: "x-pack-test-password" } + File testArtifactsDir = project.file('build/testArtifacts') + TaskProvider copyTestClasses = tasks.register("copyTestClasses", Copy) { + from { zipTree(configurations.testArtifacts.singleFile) } + into testArtifactsDir + } + integTest.runner { - testClassesDirs += configurations.testClasses + dependsOn copyTestClasses + testClassesDirs += project.files(testArtifactsDir) + classpath += configurations.testArtifacts nonInputProperties.systemProperty 'tests.audit.logfile', "${ -> testClusters.integTest.singleNode().getAuditLog()}" nonInputProperties.systemProperty 'tests.audit.yesterday.logfile', "${ -> testClusters.integTest.singleNode().getAuditLog().getParentFile()}/integTest_audit-${new Date().format('yyyy-MM-dd')}.json" } + + testingConventions.enabled = false } From bd9d86072ed9abc2c0455770eb5d56760bb41626 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 29 Aug 2019 21:25:25 -0700 Subject: [PATCH 6/8] allow for non existent test dir --- .../java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java index 123f22073ae57..8bfbcd370ec54 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java @@ -194,7 +194,7 @@ static List classpathResources(String pattern) throws Exception { } } // normal file access - else { + else if (Files.isDirectory(path)) { Files.walkFileTree(path, EnumSet.allOf(FileVisitOption.class), 1, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { From 36750ddee810923acea33eb5ddc1c0749fab4d26 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 29 Aug 2019 21:39:20 -0700 Subject: [PATCH 7/8] add dependency --- x-pack/plugin/sql/qa/security/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index eeadd52223d11..588d7ee92d6f9 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -45,6 +45,7 @@ subprojects { File testArtifactsDir = project.file('build/testArtifacts') TaskProvider copyTestClasses = tasks.register("copyTestClasses", Copy) { + dependsOn configurations.testArtifacts from { zipTree(configurations.testArtifacts.singleFile) } into testArtifactsDir } From 17162fe27d65dbfe179c809c0694147765d21089 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 11 Sep 2019 09:50:08 -0700 Subject: [PATCH 8/8] use buildDir --- x-pack/plugin/sql/qa/security/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/qa/security/build.gradle b/x-pack/plugin/sql/qa/security/build.gradle index 588d7ee92d6f9..827559e3026b1 100644 --- a/x-pack/plugin/sql/qa/security/build.gradle +++ b/x-pack/plugin/sql/qa/security/build.gradle @@ -43,7 +43,7 @@ subprojects { user username: "test_admin", password: "x-pack-test-password" } - File testArtifactsDir = project.file('build/testArtifacts') + File testArtifactsDir = project.file("$buildDir/testArtifacts") TaskProvider copyTestClasses = tasks.register("copyTestClasses", Copy) { dependsOn configurations.testArtifacts from { zipTree(configurations.testArtifacts.singleFile) }