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

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

Merged
merged 5 commits into from
Nov 13, 2018
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
82e8dff
Handle OS pretty name on old OS without OS release
jasontedor Nov 12, 2018
cac0f9f
Fix Javadocs
jasontedor Nov 12, 2018
c09423d
Javadoc round two
jasontedor Nov 12, 2018
f802b9a
Merge branch 'master' into redhat-release
jasontedor Nov 12, 2018
5c61807
Remove import
jasontedor Nov 12, 2018
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
11 changes: 10 additions & 1 deletion qa/evil-tests/src/test/java/org/elasticsearch/monitor/os/EvilOsProbeTests.java
View file
Open in desktop
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);
Expand Down
29 changes: 21 additions & 8 deletions server/src/main/java/org/elasticsearch/monitor/os/OsProbe.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -567,22 +569,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 file represents identification of the underlying operating system. The structure of the file is
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/These file represents/These files represent/

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed cac0f9f.

* 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
1 change: 1 addition & 0 deletions server/src/main/resources/org/elasticsearch/bootstrap/security.policy
View file
Open in desktop
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