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.

Sign up for GitHub

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

Fix and unmute package upgrade tests #83043

Merged
merged 8 commits into from
Jan 27, 2022
Merged

Fix and unmute package upgrade tests #83043

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
736d936
Unmute tests to investigate failures in CI
jkakavas Jan 25, 2022
fe5afc1
Fix tests
jkakavas Jan 25, 2022
16f0e08
more fixes
jkakavas Jan 25, 2022
2cc0d9a
spotless
jkakavas Jan 25, 2022
c5f6a98
Merge remote-tracking branch 'origin/master' into fix-upgrade-test
jkakavas Jan 25, 2022
af411ff
Remove redundant assertion
jkakavas Jan 25, 2022
1d542c8
minor changes
jkakavas Jan 26, 2022
240e375
use static var instead of hardcoding name everywhere
jkakavas Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PackageUpgradeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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(); });
}
Expand All @@ -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);
}
}
}
}
15 changes: 6 additions & 9 deletions ...rc/test/java/org/elasticsearch/packaging/test/PackagesSecurityAutoConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand All @@ -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("certs").resolve("transport.p12"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using AUTOCONFIG_DIRNAME instead of "certs" here?

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"))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, should we be using AUTOCONFIG_DIRNAME instead of "certs" here?

}

public void test30SecurityNotAutoConfiguredWhenExistingDataDir() throws Exception {
Expand Down Expand Up @@ -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)); //
}
Expand Down
33 changes: 8 additions & 25 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
)
);
Expand All @@ -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(),
Expand All @@ -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(
Expand All @@ -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();
}

}