Fix and unmute package upgrade tests (#83043) (#83206)

jkakavas · web-flow · commit 6f0dede4ba00 · 2022-01-31T07:01:39.000+02:00
Fixes PackageUpgradeTests (cherry picked from commit 514bec1)
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
@@ -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 {
@@ -90,15 +96,18 @@ public void test20InstallUpgradedVersion() throws Exception {
             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(); });
     }
@@ -111,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
@@ -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<String> 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<String> 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
@@ -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<Exception> codeBlock, long maxWait
      * @param es the {@link Installation} to check
      */
     public void verifySecurityAutoConfigured(Installation es) throws Exception {
-        Optional<String> 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<String> 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();
-    }
-
 }
