From 514bec19eb9ffc16bd2a161f0856baa5d671b93f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 27 Jan 2022 11:03:08 +0200 Subject: [PATCH] Fix and unmute package upgrade tests (#83043) Fixes PackageUpgradeTests --- .../packaging/test/PackageUpgradeTests.java | 42 +++++++++++++++---- ...ackagesSecurityAutoConfigurationTests.java | 22 +++++----- .../packaging/test/PackagingTestCase.java | 33 ++++----------- 3 files changed, 53 insertions(+), 44 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..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 @@ -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,10 @@ public static void filterVersions() { public void test10InstallBwcVersion() throws Exception { installation = installPackage(sh, bwcDistribution); assertInstalled(bwcDistribution); - // 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); + // 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); + } } public void test11ModifyKeystore() throws Exception { @@ -84,22 +90,24 @@ 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 installation = Packages.forceUpgradePackage(sh, distribution); } else { installation = Packages.upgradePackage(sh, distribution); - verifySecurityNotAutoConfigured(installation); } + // We add this so that we don't trigger the SecurityImplicitBehaviorBootstrapCheck in 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"); + } + 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); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/76283") public void test21CheckUpgradedVersion() throws Exception { assertWhileRunning(() -> { assertDocsExist(); }); } @@ -112,4 +120,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(es); + 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( + "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 (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..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 @@ -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(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 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(AUTOCONFIG_DIRNAME).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)); // } @@ -312,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) + ) ); } } 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..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 @@ -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,21 @@ 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"; 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,13 +652,10 @@ 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), - 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(), @@ -687,7 +681,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 +701,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(); - } - }