From 736d9368201a228972ffa4dc8126ba0282009398 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 25 Jan 2022 15:40:32 +0200 Subject: [PATCH 1/7] Unmute tests to investigate failures in CI --- .../org/elasticsearch/packaging/test/PackageUpgradeTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java index 7275e9d6ca283..4c66a903bab98 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java @@ -84,7 +84,6 @@ public void test12SetupBwcVersion() throws Exception { stopElasticsearch(); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/79950") public void test20InstallUpgradedVersion() throws Exception { if (bwcDistribution.path.equals(distribution.path)) { // the old and new distributions are the same, so we are testing force upgrading @@ -99,7 +98,6 @@ public void test20InstallUpgradedVersion() throws Exception { ServerUtils.disableSecurityFeatures(installation); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/76283") public void test21CheckUpgradedVersion() throws Exception { assertWhileRunning(() -> { assertDocsExist(); }); } From fe5afc123ef2c6b4278b9d983fe6a453283d8562 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 25 Jan 2022 16:14:48 +0200 Subject: [PATCH 2/7] Fix tests --- .../packaging/test/PackageUpgradeTests.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java index 4c66a903bab98..6ed722c1cd227 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java @@ -12,11 +12,16 @@ import org.apache.http.entity.ContentType; import org.elasticsearch.Version; import org.elasticsearch.packaging.util.Distribution; +import org.elasticsearch.packaging.util.FileUtils; +import org.elasticsearch.packaging.util.Installation; import org.elasticsearch.packaging.util.Packages; import org.elasticsearch.packaging.util.ServerUtils; import org.junit.BeforeClass; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Paths; +import java.util.List; import static org.elasticsearch.packaging.util.Packages.assertInstalled; import static org.elasticsearch.packaging.util.Packages.installPackage; @@ -41,9 +46,8 @@ public static void filterVersions() { public void test10InstallBwcVersion() throws Exception { installation = installPackage(sh, bwcDistribution); assertInstalled(bwcDistribution); + possiblyRemoveSecurityConfiguration(installation); // TODO: Add more tests here to assert behavior when updating from < v8 to > v8 with implicit/explicit behavior, - // maybe as part of https://github.com/elastic/elasticsearch/pull/76879 - ServerUtils.disableSecurityFeatures(installation); } public void test11ModifyKeystore() throws Exception { @@ -90,12 +94,10 @@ public void test20InstallUpgradedVersion() throws Exception { installation = Packages.forceUpgradePackage(sh, distribution); } else { installation = Packages.upgradePackage(sh, distribution); - verifySecurityNotAutoConfigured(installation); } assertInstalled(distribution); verifyPackageInstallation(installation, distribution, sh); - // Upgrade overwrites the configuration file because we run with --force-confnew so we need to disable security again - ServerUtils.disableSecurityFeatures(installation); + verifySecurityNotAutoConfigured(installation); } public void test21CheckUpgradedVersion() throws Exception { @@ -110,4 +112,22 @@ private void assertDocsExist() throws Exception { String response3 = ServerUtils.makeRequest(Request.Get("http://localhost:9200/library2/_doc/1?pretty")); assertThat(response3, containsString("Darkness")); } + + private void possiblyRemoveSecurityConfiguration(Installation es) throws IOException { + ServerUtils.disableSecurityFeatures(installation); + if (Files.exists(installation.config("certs"))) { + FileUtils.rm(installation.config("certs")); + } + // remove security auto-configuration entries, in case bwc was > 8, since we disable security + for (String entry : List.of( + "xpack.security.transport.ssl.keystore.secure_password", + "xpack.security.transport.ssl.truststore.secure_password", + "xpack.security.http.ssl.keystore.secure_password", + "autoconfiguration.password_hash" + )) { + if (installation.executables().keystoreTool.run("list").stdout().contains(entry)) { + installation.executables().keystoreTool.run("remove " + entry); + } + } + } } From 16f0e088444317226988952ca89aa1a4668841e5 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 25 Jan 2022 19:42:15 +0200 Subject: [PATCH 3/7] more fixes --- .../packaging/test/PackageUpgradeTests.java | 17 +++++++--- ...ackagesSecurityAutoConfigurationTests.java | 15 ++++----- .../packaging/test/PackagingTestCase.java | 31 ++++++------------- 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java index 6ed722c1cd227..0ac495c545fda 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java @@ -46,7 +46,9 @@ public static void filterVersions() { public void test10InstallBwcVersion() throws Exception { installation = installPackage(sh, bwcDistribution); assertInstalled(bwcDistribution); - possiblyRemoveSecurityConfiguration(installation); + if (installation.distribution.baseVersion.startsWith("8.")) { + possiblyRemoveSecurityConfiguration(installation); + } // TODO: Add more tests here to assert behavior when updating from < v8 to > v8 with implicit/explicit behavior, } @@ -95,6 +97,11 @@ public void test20InstallUpgradedVersion() throws Exception { } else { installation = Packages.upgradePackage(sh, distribution); } + // We add this so that we don't trigger the SecurityImplicitBehaviorBootstrapCheck in 8 + if (bwcDistribution.baseVersion.startsWith("7.") && distribution.baseVersion.startsWith("8.")) { + ServerUtils.addSettingToExistingConfiguration(installation, "xpack.security.enabled", "false"); + } + assertInstalled(distribution); verifyPackageInstallation(installation, distribution, sh); verifySecurityNotAutoConfigured(installation); @@ -115,8 +122,8 @@ private void assertDocsExist() throws Exception { private void possiblyRemoveSecurityConfiguration(Installation es) throws IOException { ServerUtils.disableSecurityFeatures(installation); - if (Files.exists(installation.config("certs"))) { - FileUtils.rm(installation.config("certs")); + if (Files.exists(es.config("certs"))) { + FileUtils.rm(es.config("certs")); } // remove security auto-configuration entries, in case bwc was > 8, since we disable security for (String entry : List.of( @@ -125,8 +132,8 @@ private void possiblyRemoveSecurityConfiguration(Installation es) throws IOExcep "xpack.security.http.ssl.keystore.secure_password", "autoconfiguration.password_hash" )) { - if (installation.executables().keystoreTool.run("list").stdout().contains(entry)) { - installation.executables().keystoreTool.run("remove " + entry); + if (es.executables().keystoreTool.run("list").stdout().contains(entry)) { + es.executables().keystoreTool.run("remove " + entry); } } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java index b84dd871157c3..9eb55208794ec 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java @@ -29,7 +29,6 @@ import java.nio.file.StandardCopyOption; import java.security.SecureRandom; import java.util.List; -import java.util.Optional; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -52,12 +51,13 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.Assume.assumeTrue; public class PackagesSecurityAutoConfigurationTests extends PackagingTestCase { + private static final String AUTOCONFIG_DIRNAME = "certs"; + @BeforeClass public static void filterDistros() { assumeTrue("rpm or deb", distribution.isPackage()); @@ -75,15 +75,13 @@ public void test10SecurityAutoConfiguredOnPackageInstall() throws Exception { public void test20SecurityNotAutoConfiguredOnReInstallation() throws Exception { // we are testing force upgrading in the current version // In such a case, security remains configured from the initial installation, we don't run it again. - Optional autoConfigDirName = getAutoConfigDirName(installation); + byte[] transportKeystore = Files.readAllBytes(installation.config("certs").resolve("transport.p12")); installation = Packages.forceUpgradePackage(sh, distribution); assertInstalled(distribution); verifyPackageInstallation(installation, distribution, sh); verifySecurityAutoConfigured(installation); - // Since we did not auto-configure the second time, the directory name should be the same - assertThat(autoConfigDirName.isPresent(), is(true)); - assertThat(getAutoConfigDirName(installation).isPresent(), is(true)); - assertThat(getAutoConfigDirName(installation).get(), equalTo(autoConfigDirName.get())); + // Since we did not auto-configure the second time, the keystore should be the one we generated the first time, above + assertThat(transportKeystore, equalTo(Files.readAllBytes(installation.config("certs").resolve("transport.p12")))); } public void test30SecurityNotAutoConfiguredWhenExistingDataDir() throws Exception { @@ -161,9 +159,8 @@ public void test70ReconfigureFailsWhenTlsAutoConfDirMissing() throws Exception { verifySecurityAutoConfigured(installation); assertNotNull(installation.getElasticPassword()); - Optional autoConfigDirName = getAutoConfigDirName(installation); // Move instead of delete because Files.deleteIfExists bails on non empty dirs - Files.move(installation.config(autoConfigDirName.get()), installation.config("temp-autoconf-dir")); + Files.move(installation.config(AUTOCONFIG_DIRNAME), installation.config("temp-autoconf-dir")); Shell.Result result = installation.executables().nodeReconfigureTool.run("--enrollment-token a-token", "y", true); assertThat(result.exitCode(), equalTo(ExitCodes.USAGE)); // } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 644219572e4e3..35963fba4a6c5 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -63,11 +63,9 @@ import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -629,22 +627,22 @@ public static void assertBusy(CheckedRunnable codeBlock, long maxWait * @param es the {@link Installation} to check */ public void verifySecurityAutoConfigured(Installation es) throws Exception { - Optional autoConfigDirName = getAutoConfigDirName(es); - assertThat(autoConfigDirName.isPresent(), Matchers.is(true)); + final String autoConfigDirName = "certs"; + assertThat(Files.exists(es.config(autoConfigDirName)), Matchers.is(true)); final Settings settings; if (es.distribution.isArchive()) { // We chown the installation on Windows to Administrators so that we can auto-configure it. String owner = Platforms.WINDOWS ? "BUILTIN\\Administrators" : "elasticsearch"; - assertThat(es.config(autoConfigDirName.get()), FileMatcher.file(Directory, owner, owner, p750)); + assertThat(es.config(autoConfigDirName), FileMatcher.file(Directory, owner, owner, p750)); Stream.of("http.p12", "http_ca.crt", "transport.p12") - .forEach(file -> assertThat(es.config(autoConfigDirName.get()).resolve(file), FileMatcher.file(File, owner, owner, p660))); + .forEach(file -> assertThat(es.config(autoConfigDirName).resolve(file), FileMatcher.file(File, owner, owner, p660))); settings = Settings.builder().loadFromPath(es.config("elasticsearch.yml")).build(); } else if (es.distribution.isDocker()) { - assertThat(es.config(autoConfigDirName.get()), DockerFileMatcher.file(Directory, "elasticsearch", "root", p750)); + assertThat(es.config(autoConfigDirName), DockerFileMatcher.file(Directory, "elasticsearch", "root", p750)); Stream.of("http.p12", "http_ca.crt", "transport.p12") .forEach( file -> assertThat( - es.config(autoConfigDirName.get()).resolve(file), + es.config(autoConfigDirName).resolve(file), DockerFileMatcher.file(File, "elasticsearch", "root", p660) ) ); @@ -655,11 +653,11 @@ public void verifySecurityAutoConfigured(Installation es) throws Exception { rm(localTempDir); } else { assert es.distribution.isPackage(); - assertThat(es.config(autoConfigDirName.get()), FileMatcher.file(Directory, "root", "elasticsearch", p750)); + assertThat(es.config(autoConfigDirName), FileMatcher.file(Directory, "root", "elasticsearch", p750)); Stream.of("http.p12", "http_ca.crt", "transport.p12") .forEach( file -> assertThat( - es.config(autoConfigDirName.get()).resolve(file), + es.config(autoConfigDirName).resolve(file), FileMatcher.file(File, "root", "elasticsearch", p660) ) ); @@ -687,7 +685,7 @@ public void verifySecurityAutoConfigured(Installation es) throws Exception { * @param es the {@link Installation} to check */ public static void verifySecurityNotAutoConfigured(Installation es) throws Exception { - assertThat(getAutoConfigDirName(es).isPresent(), Matchers.is(false)); + assertThat(Files.exists(es.config("certs")), Matchers.is(false)); if (es.distribution.isPackage()) { if (Files.exists(es.config("elasticsearch.keystore"))) { assertThat( @@ -707,15 +705,4 @@ public static void verifySecurityNotAutoConfigured(Installation es) throws Excep } } - public static Optional getAutoConfigDirName(Installation es) { - final Shell.Result lsResult; - if (es.distribution.platform.equals(Distribution.Platform.WINDOWS)) { - lsResult = sh.run("Get-ChildItem -Path " + es.config + " -Name"); - } else { - lsResult = sh.run("find \"" + es.config + "\" -type d -maxdepth 1"); - } - assertNotNull(lsResult.stdout()); - return Arrays.stream(lsResult.stdout().split("\n")).filter(f -> f.contains("certs")).findFirst(); - } - } From 2cc0d9a0931e21153fa0c3fce7af2c4479e7c52f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 25 Jan 2022 20:04:37 +0200 Subject: [PATCH 4/7] spotless --- .../org/elasticsearch/packaging/test/PackagingTestCase.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 35963fba4a6c5..c1d776018f52e 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -656,10 +656,7 @@ public void verifySecurityAutoConfigured(Installation es) throws Exception { assertThat(es.config(autoConfigDirName), FileMatcher.file(Directory, "root", "elasticsearch", p750)); Stream.of("http.p12", "http_ca.crt", "transport.p12") .forEach( - file -> assertThat( - es.config(autoConfigDirName).resolve(file), - FileMatcher.file(File, "root", "elasticsearch", p660) - ) + file -> assertThat(es.config(autoConfigDirName).resolve(file), FileMatcher.file(File, "root", "elasticsearch", p660)) ); assertThat( sh.run(es.executables().keystoreTool + " list").stdout(), From af411ffec3011142952aa97adb0b89d90ca1ad50 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 25 Jan 2022 22:39:16 +0200 Subject: [PATCH 5/7] Remove redundant assertion --- .../java/org/elasticsearch/packaging/test/PackagingTestCase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index c1d776018f52e..fa8053ba5d5bf 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -628,7 +628,6 @@ public static void assertBusy(CheckedRunnable codeBlock, long maxWait */ public void verifySecurityAutoConfigured(Installation es) throws Exception { final String autoConfigDirName = "certs"; - assertThat(Files.exists(es.config(autoConfigDirName)), Matchers.is(true)); final Settings settings; if (es.distribution.isArchive()) { // We chown the installation on Windows to Administrators so that we can auto-configure it. From 1d542c8f178067f0c6308e222f7668aadeb72b62 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 26 Jan 2022 10:12:57 +0200 Subject: [PATCH 6/7] minor changes --- .../packaging/test/PackageUpgradeTests.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java index 0ac495c545fda..1aff7fc6ab791 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java @@ -46,10 +46,10 @@ public static void filterVersions() { public void test10InstallBwcVersion() throws Exception { installation = installPackage(sh, bwcDistribution); assertInstalled(bwcDistribution); - if (installation.distribution.baseVersion.startsWith("8.")) { + // TODO Modify tests below to work with security when BWC version is after 8.0.0 + if (Version.fromString(bwcDistribution.baseVersion).onOrAfter(Version.V_8_0_0)) { possiblyRemoveSecurityConfiguration(installation); } - // TODO: Add more tests here to assert behavior when updating from < v8 to > v8 with implicit/explicit behavior, } public void test11ModifyKeystore() throws Exception { @@ -98,7 +98,8 @@ public void test20InstallUpgradedVersion() throws Exception { installation = Packages.upgradePackage(sh, distribution); } // We add this so that we don't trigger the SecurityImplicitBehaviorBootstrapCheck in 8 - if (bwcDistribution.baseVersion.startsWith("7.") && distribution.baseVersion.startsWith("8.")) { + if (Version.fromString(bwcDistribution.baseVersion).before(Version.V_8_0_0) + && Version.fromString(distribution.baseVersion).onOrAfter(Version.V_8_0_0)) { ServerUtils.addSettingToExistingConfiguration(installation, "xpack.security.enabled", "false"); } @@ -121,7 +122,7 @@ private void assertDocsExist() throws Exception { } private void possiblyRemoveSecurityConfiguration(Installation es) throws IOException { - ServerUtils.disableSecurityFeatures(installation); + ServerUtils.disableSecurityFeatures(es); if (Files.exists(es.config("certs"))) { FileUtils.rm(es.config("certs")); } From 240e375a4a1c7f40308d66aa0661f17b90aaf8cf Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 26 Jan 2022 18:00:31 +0200 Subject: [PATCH 7/7] use static var instead of hardcoding name everywhere --- .../test/PackagesSecurityAutoConfigurationTests.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java index 9eb55208794ec..fa68da1725edc 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java @@ -75,13 +75,13 @@ public void test10SecurityAutoConfiguredOnPackageInstall() throws Exception { public void test20SecurityNotAutoConfiguredOnReInstallation() throws Exception { // we are testing force upgrading in the current version // In such a case, security remains configured from the initial installation, we don't run it again. - byte[] transportKeystore = Files.readAllBytes(installation.config("certs").resolve("transport.p12")); + byte[] transportKeystore = Files.readAllBytes(installation.config(AUTOCONFIG_DIRNAME).resolve("transport.p12")); installation = Packages.forceUpgradePackage(sh, distribution); assertInstalled(distribution); verifyPackageInstallation(installation, distribution, sh); verifySecurityAutoConfigured(installation); // Since we did not auto-configure the second time, the keystore should be the one we generated the first time, above - assertThat(transportKeystore, equalTo(Files.readAllBytes(installation.config("certs").resolve("transport.p12")))); + assertThat(transportKeystore, equalTo(Files.readAllBytes(installation.config(AUTOCONFIG_DIRNAME).resolve("transport.p12")))); } public void test30SecurityNotAutoConfiguredWhenExistingDataDir() throws Exception { @@ -309,10 +309,13 @@ public void test73ReconfigureCreatesFilesWithCorrectPermissions() throws Excepti true ); assertThat(result.exitCode(), CoreMatchers.equalTo(0)); - assertThat(installation.config("certs"), FileMatcher.file(Directory, "root", "elasticsearch", p750)); + assertThat(installation.config(AUTOCONFIG_DIRNAME), FileMatcher.file(Directory, "root", "elasticsearch", p750)); Stream.of("http.p12", "http_ca.crt", "transport.p12") .forEach( - file -> assertThat(installation.config("certs").resolve(file), FileMatcher.file(File, "root", "elasticsearch", p660)) + file -> assertThat( + installation.config(AUTOCONFIG_DIRNAME).resolve(file), + FileMatcher.file(File, "root", "elasticsearch", p660) + ) ); } }