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.

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

Handle OS pretty name on old OS without OS release #35453

Merged
merged 5 commits into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.io.IOException;
import java.nio.file.Files;
import java.util.Collections;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -36,7 +37,15 @@ public class EvilOsProbeTests extends ESTestCase {
public void testOsPrettyName() throws IOException {
final OsInfo osInfo = OsProbe.getInstance().osInfo(randomLongBetween(1, 100), randomIntBetween(1, 8));
if (Constants.LINUX) {
final List<String> lines = Files.readAllLines(PathUtils.get("/etc/os-release"));
final List<String> lines;
if (Files.exists(PathUtils.get("/etc/os-release"))) {
lines = Files.readAllLines(PathUtils.get("/etc/os-release"));
} else if (Files.exists(PathUtils.get("/usr/lib/os-release"))) {
lines = Files.readAllLines(PathUtils.get("/usr/lib/os-release"));
} else {
lines = Collections.singletonList(
"PRETTY_NAME=\"" + Files.readAllLines(PathUtils.get("/etc/system-release")).get(0) + "\"");
}
for (final String line : lines) {
if (line != null && line.startsWith("PRETTY_NAME=")) {
final Matcher matcher = Pattern.compile("PRETTY_NAME=(\"?|'?)?([^\"']+)\\1").matcher(line.trim());
Expand Down
28 changes: 20 additions & 8 deletions server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -566,22 +567,33 @@ private String getPrettyName() throws IOException {
}

/**
* The lines from {@code /etc/os-release} or {@code /usr/lib/os-release} as a fallback. These file represents identification of the
* underlying operating system. The structure of the file is newlines of key-value pairs of shell-compatible variable assignments.
* The lines from {@code /etc/os-release} or {@code /usr/lib/os-release} as a fallback, with an additional fallback to
* {@code /etc/system-release}. These files represent identification of the underlying operating system. The structure of the file is
* newlines of key-value pairs of shell-compatible variable assignments.
*
* @return the lines from {@code /etc/os-release} or {@code /usr/lib/os-release}
* @throws IOException if an I/O exception occurs reading {@code /etc/os-release} or {@code /usr/lib/os-release}
* @return the lines from {@code /etc/os-release} or {@code /usr/lib/os-release} or {@code /etc/system-release}
* @throws IOException if an I/O exception occurs reading {@code /etc/os-release} or {@code /usr/lib/os-release} or
* {@code /etc/system-release}
*/
@SuppressForbidden(reason = "access /etc/os-release or /usr/lib/os-release")
@SuppressForbidden(reason = "access /etc/os-release or /usr/lib/os-release or /etc/system-release")
List<String> readOsRelease() throws IOException {
final List<String> lines;
if (Files.exists(PathUtils.get("/etc/os-release"))) {
lines = Files.readAllLines(PathUtils.get("/etc/os-release"));
} else {
assert lines != null && lines.isEmpty() == false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to have this as an assertion or as an IllegalStateException or similar? If a user runs this on some weird Linux distribution that has an empty file here they might get odd errors later on, although I suppose you could argue that if they're not running on a supported OS then they're on their own anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly, we code for the platforms that we support and use these assertions to ensure that everything is as expected on those platforms.

Copy link
Contributor

Choose a reason for hiding this comment

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

++

return lines;
} else if (Files.exists(PathUtils.get("/usr/lib/os-release"))) {
lines = Files.readAllLines(PathUtils.get("/usr/lib/os-release"));
assert lines != null && lines.isEmpty() == false;
return lines;
} else if (Files.exists(PathUtils.get("/etc/system-release"))) {
// fallback for older Red Hat-like OS
lines = Files.readAllLines(PathUtils.get("/etc/system-release"));
assert lines != null && lines.size() == 1;
return Collections.singletonList("PRETTY_NAME=\"" + lines.get(0) + "\"");
} else {
return Collections.emptyList();
}
assert lines != null && lines.isEmpty() == false;
return lines;
}

public OsStats osStats() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ grant {
// OS release on Linux
permission java.io.FilePermission "/etc/os-release", "read";
permission java.io.FilePermission "/usr/lib/os-release", "read";
permission java.io.FilePermission "/etc/system-release", "read";

// io stats on Linux
permission java.io.FilePermission "/proc/self/mountinfo", "read";
Expand Down