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

Resolve CodeQL warnings #217

Merged
merged 6 commits into from
Oct 1, 2020
Merged

Resolve CodeQL warnings #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
833758e
Annotate test method with RequirePOST
MarkEWaite Oct 1, 2020
f7a8715
Rename readFreeBsdVersion to getFreeBsdVersion for clarify
MarkEWaite Oct 1, 2020
7acb90b
Better name for method to get canonical Linux arch
MarkEWaite Oct 1, 2020
9453679
Better name for method to get release identifier
MarkEWaite Oct 1, 2020
e8fe118
Better name for method to get Red Hat release identifier
MarkEWaite Oct 1, 2020
ff571c2
Better name for method to get SUSE release identifier
MarkEWaite Oct 1, 2020
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
34 changes: 17 additions & 17 deletions src/main/java/org/jvnet/hudson/plugins/platformlabeler/PlatformDetailsTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,22 @@ protected String checkWindows32Bit(
* @return standardized architecture of current Linux operating system
*/
@NonNull
private String checkLinux32Bit(@NonNull final String arch) {
private String getCanonicalLinuxArch(@NonNull final String arch) {
if (!"x86".equalsIgnoreCase(arch)) {
return arch;
}
try {
Process p = Runtime.getRuntime().exec("/bin/uname -m");
p.waitFor();
return checkLinux32BitStream(p.getInputStream(), arch);
return getCanonicalLinuxArchStream(p.getInputStream(), arch);
} catch (IOException | InterruptedException e) {
/* Return arch instead of throwing an exception */
}
return arch;
}

/* Package protected for testing */
String checkLinux32BitStream(@NonNull InputStream stream, @NonNull String arch)
String getCanonicalLinuxArchStream(@NonNull InputStream stream, @NonNull String arch)
throws IOException {
try (BufferedReader b = new BufferedReader(new InputStreamReader(stream, "UTF-8"))) {
String line = b.readLine();
Expand Down Expand Up @@ -205,26 +205,26 @@ protected PlatformDetails computeLabels(
release = new LsbRelease();
}
computedName = release.distributorId();
computedArch = checkLinux32Bit(computedArch);
computedArch = getCanonicalLinuxArch(computedArch);
computedVersion = release.release();
/* Fallback to /etc/os-release file */
if (computedName.equals(UNKNOWN_VALUE_STRING)) {
computedName = readReleaseIdentifier("ID");
computedName = getReleaseIdentifier("ID");
}
if (computedVersion.equals(UNKNOWN_VALUE_STRING)) {
computedVersion = readReleaseIdentifier("VERSION_ID");
computedVersion = getReleaseIdentifier("VERSION_ID");
}
if (computedVersion.equals(UNKNOWN_VALUE_STRING)) {
computedVersion = readReleaseIdentifier("BUILD_ID");
computedVersion = getReleaseIdentifier("BUILD_ID");
}
if (computedName.equals(UNKNOWN_VALUE_STRING)) {
computedName = readRedhatReleaseIdentifier("ID");
computedName = getRedhatReleaseIdentifier("ID");
}
if (computedVersion.equals(UNKNOWN_VALUE_STRING)) {
computedVersion = readRedhatReleaseIdentifier("VERSION_ID");
computedVersion = getRedhatReleaseIdentifier("VERSION_ID");
}
if (computedName.equals(UNKNOWN_VALUE_STRING)) {
computedName = readSuseReleaseIdentifier("ID");
computedName = getSuseReleaseIdentifier("ID");
}
/* This is kind of a hack. lsb_release -a returns only the major
* version on SLES 11 and older, so trying to fall back to
Expand All @@ -240,7 +240,7 @@ protected PlatformDetails computeLabels(
try {
int intVersion = Integer.parseInt(computedVersion);
if (intVersion <= 11) {
String newVersion = readSuseReleaseIdentifier("VERSION_ID");
String newVersion = getSuseReleaseIdentifier("VERSION_ID");
if (!newVersion.equals(UNKNOWN_VALUE_STRING)) {
computedVersion = newVersion;
}
Expand All @@ -250,10 +250,10 @@ protected PlatformDetails computeLabels(
}
}
if (computedVersion.equals(UNKNOWN_VALUE_STRING)) {
computedVersion = readSuseReleaseIdentifier("VERSION_ID");
computedVersion = getSuseReleaseIdentifier("VERSION_ID");
}
} else if (computedName.startsWith("freebsd")) {
computedVersion = readFreeBsdVersion(computedVersion);
computedVersion = getFreeBsdVersion(computedVersion);
} else if (computedName.startsWith("mac")) {
computedName = "mac";
}
Expand Down Expand Up @@ -307,7 +307,7 @@ void setSuseRelease(File suseRelease) {

/* Package protected for use in tests */
@NonNull
String readReleaseIdentifier(@NonNull String field) {
String getReleaseIdentifier(@NonNull String field) {
String value = UNKNOWN_VALUE_STRING;
if (osRelease == null) {
return value;
Expand All @@ -329,7 +329,7 @@ String readReleaseIdentifier(@NonNull String field) {

/* Package protected for use in tests */
@NonNull
String readRedhatReleaseIdentifier(@NonNull String field) {
String getRedhatReleaseIdentifier(@NonNull String field) {
String value = UNKNOWN_VALUE_STRING;
if (redhatRelease == null) {
return value;
Expand All @@ -356,7 +356,7 @@ String readRedhatReleaseIdentifier(@NonNull String field) {
}

@NonNull
String readSuseReleaseIdentifier(@NonNull String field) {
String getSuseReleaseIdentifier(@NonNull String field) {
String value = UNKNOWN_VALUE_STRING;
String version = null;
String patchLevel = null;
Expand Down Expand Up @@ -400,7 +400,7 @@ String readSuseReleaseIdentifier(@NonNull String field) {
}

@NonNull
String readFreeBsdVersion(String version) {
String getFreeBsdVersion(String version) {
try {
Process p = Runtime.getRuntime().exec("/bin/freebsd-version -u");
p.waitFor();
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/org/jvnet/hudson/plugins/platformlabeler/NodeLabelCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.jvnet.hudson.test.JenkinsRule;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.interceptor.RequirePOST;

/**
* Test NodeLabelCache in a few potential error cases.
Expand Down Expand Up @@ -151,6 +152,7 @@ public List<LogRecord> getLogRecords() throws IOException, InterruptedException
}

@Override
@RequirePOST
public void doLaunchSlaveAgent(StaplerRequest sr, StaplerResponse sr1)
throws IOException, ServletException {
throw new UnsupportedOperationException("Unsupported");
Expand Down
4 changes: 2 additions & 2 deletions ...st/java/org/jvnet/hudson/plugins/platformlabeler/PlatformDetailsTaskStaticStringTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ private String computeVersion(String name, String version) {
break;
}
} else if (name.startsWith("FreeBSD")) {
return readFreeBsdVersion(version);
return getFreeBsdVersion(version);
}
return version;
}

private String readFreeBsdVersion(String version) {
private String getFreeBsdVersion(String version) {
/* If no freebsd-version command, return the provided default value */
File freebsdVersionCommand = new File("/bin/freebsd-version");
if (!freebsdVersionCommand.exists()) {
Expand Down
57 changes: 30 additions & 27 deletions src/test/java/org/jvnet/hudson/plugins/platformlabeler/PlatformDetailsTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,36 +79,39 @@ private void assertPlatformDetails(PlatformDetails details) {

@Test
@DisplayName("test 32 bit Linux label computation")
public void testComputeLabelsLinux32Bit() throws Exception {
void testComputeLabelsCanonicalLinuxArch() throws Exception {
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
assertPlatformDetails(details);
}

@Test
@DisplayName("test Linux32Bit stream")
public void testLinux32BitStream() throws IOException {
@DisplayName("test CanonicalLinuxArch stream")
void testCanonicalLinuxArchStream() throws IOException {
String unameOutput = "x86_64";
InputStream stream = new ByteArrayInputStream(unameOutput.getBytes(StandardCharsets.UTF_8));
assertThat(platformDetailsTask.checkLinux32BitStream(stream, SPECIAL_CASE_ARCH), is("amd64"));
assertThat(
platformDetailsTask.getCanonicalLinuxArchStream(stream, SPECIAL_CASE_ARCH), is("amd64"));
}

@Test
@DisplayName("test Linux32Bit stream ARM")
public void testLinux32BitStreamARM() throws IOException {
@DisplayName("test CanonicalLinuxArch stream ARM")
void testCanonicalLinuxArchStreamARM() throws IOException {
String unameOutput = "aarch64";
InputStream stream = new ByteArrayInputStream(unameOutput.getBytes(StandardCharsets.UTF_8));
assertThat(
platformDetailsTask.checkLinux32BitStream(stream, SPECIAL_CASE_ARCH), is(unameOutput));
platformDetailsTask.getCanonicalLinuxArchStream(stream, SPECIAL_CASE_ARCH),
is(unameOutput));
}

@Test
@DisplayName("test Linux32Bit stream empty")
public void testLinux32BitStreamEmpty() throws IOException {
@DisplayName("test CanonicalLinuxArch stream empty")
void testCanonicalLinuxArchStreamEmpty() throws IOException {
String unameOutput = "";
String expectedArch = "Expected-Arch";
InputStream stream = new ByteArrayInputStream(unameOutput.getBytes(StandardCharsets.UTF_8));
assertThat(platformDetailsTask.checkLinux32BitStream(stream, expectedArch), is(expectedArch));
assertThat(
platformDetailsTask.getCanonicalLinuxArchStream(stream, expectedArch), is(expectedArch));
}

@Test
Expand Down Expand Up @@ -172,74 +175,74 @@ public void compareOSName() throws Exception {
}
String computedName =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy").getName();
String readName = platformDetailsTask.readReleaseIdentifier("ID");
String readName = platformDetailsTask.getReleaseIdentifier("ID");
assertThat(computedName, is(readName));
}

@Test
@DisplayName("test release identifier on missing file")
public void readReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
public void getReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
platformDetailsTask.setOsReleaseFile(new File("/this/file/does/not/exist"));
String name = platformDetailsTask.readReleaseIdentifier("ID");
String name = platformDetailsTask.getReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read Red Hat release identifier")
public void readRedhatReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
public void getRedhatReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
platformDetailsTask.setRedhatRelease(new File("/this/file/does/not/exist"));
String name = platformDetailsTask.readRedhatReleaseIdentifier("ID");
String name = platformDetailsTask.getRedhatReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read Red Hat release identifier null file")
public void readRedhatReleaseIdentifierNullFileReturnsUnknownValue() throws Exception {
public void getRedhatReleaseIdentifierNullFileReturnsUnknownValue() throws Exception {
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
platformDetailsTask.setRedhatRelease(null);
String name = platformDetailsTask.readRedhatReleaseIdentifier("ID");
String name = platformDetailsTask.getRedhatReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read Red Hat release identifier wrong file")
public void readRedhatReleaseIdentifierWrongFileReturnsUnknownValue() throws Exception {
public void getRedhatReleaseIdentifierWrongFileReturnsUnknownValue() throws Exception {
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
platformDetailsTask.setRedhatRelease(new File("/etc/hosts")); // Not redhat-release file
String name = platformDetailsTask.readRedhatReleaseIdentifier("ID");
String name = platformDetailsTask.getRedhatReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read SUSE release identifier missing file")
public void readSuseReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
public void getSuseReleaseIdentifierMissingFileReturnsUnknownValue() throws Exception {
platformDetailsTask.setSuseRelease(new File("/this/file/does/not/exist"));
String name = platformDetailsTask.readSuseReleaseIdentifier("ID");
String name = platformDetailsTask.getSuseReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read SUSE release identifier null file")
public void readSuseReleaseIdentifierNullFileReturnsUnknownValue() throws Exception {
public void getSuseReleaseIdentifierNullFileReturnsUnknownValue() throws Exception {
platformDetailsTask.setSuseRelease(null);
String name = platformDetailsTask.readSuseReleaseIdentifier("ID");
String name = platformDetailsTask.getSuseReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

@Test
@DisplayName("Read SUSE release identifier wrong file")
public void readSuseReleaseIdentifierWrongFileReturnsUnknownValue() throws Exception {
public void getSuseReleaseIdentifierWrongFileReturnsUnknownValue() throws Exception {
if (isWindows() || !Files.exists(Paths.get("/etc/os-release"))) {
return;
}
platformDetailsTask.setSuseRelease(new File("/etc/hosts")); // Not SuSE-release file
String name = platformDetailsTask.readSuseReleaseIdentifier("ID");
String name = platformDetailsTask.getSuseReleaseIdentifier("ID");
assertThat(name, is(PlatformDetailsTask.UNKNOWN_VALUE_STRING));
}

Expand All @@ -251,8 +254,8 @@ public void compareOSVersion() throws Exception {
}
PlatformDetails details =
platformDetailsTask.computeLabels(SPECIAL_CASE_ARCH, "linux", "xyzzy");
String version = platformDetailsTask.readReleaseIdentifier("VERSION_ID");
/* Check that the version string returned by readReleaseIdentifier
String version = platformDetailsTask.getReleaseIdentifier("VERSION_ID");
/* Check that the version string returned by getReleaseIdentifier
* is at least at the beginning of one of the detail values. Allow
* Debian 8, 9, and 10 and CentOS 7 to report their base version
* in the /etc/os-release file without reporting their incremental
Expand Down