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

Set JAVA_HOME in gradle-jdks-setup script #447

Merged
merged 14 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-447.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: fix
fix:
description: gradle-jdks-setup.sh script sets `JAVA_HOME` to gradle JDK
links:
- https://github.com/palantir/gradle-jdks/pull/447
2 changes: 1 addition & 1 deletion gradle-jdks-setup/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ tasks.integrationTest {
dependsOn tasks.fatJar
}

tasks.build.dependsOn tasks.fatJar
tasks.build.dependsOn tasks.fatJar, tasks.integrationTest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the integration tests were not running before :(


// DO NOT UPDATE! This needs to be kept as low as possible, such that it is compatible with all the jdk versions the
// consumers would want to set up with the JDK auto-management workflow (used by ./gradle-jdks-setup.sh and GradleWrapperMain.java classes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@
import com.palantir.gradle.jdks.setup.common.Arch;
import com.palantir.gradle.jdks.setup.common.CommandRunner;
import com.palantir.gradle.jdks.setup.common.CurrentArch;
import com.palantir.gradle.jdks.setup.common.GradleJdksPatchHelper;
import com.palantir.gradle.jdks.setup.common.Os;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.IntStream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

Expand All @@ -46,27 +47,26 @@ public class GradleJdkInstallationSetupIntegrationTest {
private static final AmazonCorrettoJdkDistribution CORRETTO_JDK_DISTRIBUTION = new AmazonCorrettoJdkDistribution();
private static final boolean DO_NOT_INSTALL_CURL = false;
private static final boolean INSTALL_CURL = true;
private static final CaResources caResources = new CaResources(new StdLogger());

@TempDir
private Path workingDir;

@Test
public void can_setup_jdk_with_certs_centos() throws IOException, InterruptedException {
setupGradleDirectoryStructure(Os.LINUX_GLIBC);
assertJdkWithNoCertsWasSetUp(dockerBuildAndRunTestingScript("centos:7", "/bin/bash", DO_NOT_INSTALL_CURL));
dockerBuildAndRunTestingScript("centos:7", "/bin/bash", DO_NOT_INSTALL_CURL);
}

@Test
public void can_setup_jdk_with_certs_ubuntu() throws IOException, InterruptedException {
setupGradleDirectoryStructure(Os.LINUX_GLIBC);
assertJdkWithNoCertsWasSetUp(dockerBuildAndRunTestingScript("ubuntu:20.04", "/bin/bash", INSTALL_CURL));
dockerBuildAndRunTestingScript("ubuntu:20.04", "/bin/bash", INSTALL_CURL);
}

@Test
public void can_setup_jdk_with_certs_alpine() throws IOException, InterruptedException {
setupGradleDirectoryStructure(Os.LINUX_MUSL);
assertJdkWithNoCertsWasSetUp(dockerBuildAndRunTestingScript("alpine:3.16.0", "/bin/sh", DO_NOT_INSTALL_CURL));
dockerBuildAndRunTestingScript("alpine:3.16.0", "/bin/sh", DO_NOT_INSTALL_CURL);
}

private Path setupGradleDirectoryStructure(Os os) throws IOException {
Expand Down Expand Up @@ -132,21 +132,19 @@ private Path setupGradleDirectoryStructure(Os os) throws IOException {
gradleDirectory.resolve("gradle-jdks-functions.sh"));

// copy the testing script to the working directory
Files.copy(Path.of("src/integrationTest/resources/testing-script.sh"), workingDir.resolve("testing-script.sh"));

// copy the certs to the working directory
caResources
.readPalantirRootCaFromSystemTruststore()
.map(AliasContentCert::getContent)
.ifPresent(content -> {
try {
Files.write(workingDir.resolve("palantir.crt"), content.getBytes(StandardCharsets.UTF_8));
} catch (IOException e) {
throw new RuntimeException("Failed to write cert", e);
}
});
Comment on lines -138 to -147
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer need this as we just read certs from the system truststore right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes


Files.copy(Path.of("src/integrationTest/resources/example.com.crt"), workingDir.resolve("example.com.crt"));
List<String> gradlewPatchLines =
Files.readAllLines(Path.of("../gradle-jdks/src/main/resources/gradlew-patch.sh"));
List<String> initialTestLines =
Files.readAllLines(Path.of("src/integrationTest/resources/testing-script.template.sh"));
int placeholderIndex = IntStream.range(0, initialTestLines.size())
.filter(lineNo -> initialTestLines.get(lineNo).equals("PLACEHOLDER_INSERT_GRADLEW_PATCH"))
.findFirst()
.orElseThrow(() ->
new RuntimeException("unable to find PLACEHOLDER_INSERT_GRADLEW_PATCH in testing-script.sh"));
initialTestLines.remove(placeholderIndex);
Files.write(
workingDir.resolve("testing-script.sh"),
GradleJdksPatchHelper.getContentWithPatch(initialTestLines, gradlewPatchLines, placeholderIndex));

return gradleDirectory;
}
Expand All @@ -155,7 +153,7 @@ private static void writeFileContent(Path path, String content) throws IOExcepti
Files.writeString(path, content + "\n");
}

private String dockerBuildAndRunTestingScript(String baseImage, String shell, boolean installCurl)
private void dockerBuildAndRunTestingScript(String baseImage, String shell, boolean installCurl)
throws IOException, InterruptedException {
Path dockerFile = Path.of("src/integrationTest/resources/template.Dockerfile");
String dockerImage = String.format("jdk-test-%s", baseImage);
Expand All @@ -173,7 +171,10 @@ private String dockerBuildAndRunTestingScript(String baseImage, String shell, bo
"-f",
dockerFile.toAbsolutePath().toString(),
workingDir.toAbsolutePath().toString()));
return runCommandWithZeroExitCode(List.of("docker", "run", "--rm", dockerImage, shell, "/testing-script.sh"));
assertThat(runCommandWithZeroExitCode(
List.of("docker", "run", "--rm", dockerImage, shell, "/testing-script.sh")))
.contains("openjdk version \"11.0.21\"")
.contains("JAVA_HOME is set to /root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1");
}

private static String runCommandWithZeroExitCode(List<String> commandArguments)
Expand All @@ -193,12 +194,4 @@ private static String runCommandWithZeroExitCode(List<String> commandArguments,
.isEqualTo(0);
return output;
}

private static void assertJdkWithNoCertsWasSetUp(String output) {
assertThat(output).contains("Corretto-11.0.21.9.1").contains("Example.com cert: gradleJdks_example.com");

if (caResources.readPalantirRootCaFromSystemTruststore().isPresent()) {
assertThat(output).contains("Palantir cert: gradlejdks_palantir3rd-generationrootca");
}
}
}
29 changes: 0 additions & 29 deletions gradle-jdks-setup/src/integrationTest/resources/example.com.crt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ ARG INSTALL_CURL=false
# Update package lists and conditionally install curl
RUN if [ "$INSTALL_CURL" = "true" ] ; then \
apt-get update && \
apt-get install -y curl ; \
apt-get install -y curl; \
fi
COPY . /
RUN mkdir -p /etc/ssl/certs && cat /example.com.crt >> /etc/ssl/certs/ca-bundle.crt
RUN if [ -f /palantir.crt ]; then cat /palantir.crt >> /etc/ssl/certs/ca-bundle.crt; else echo "File does not exist."; fi
RUN $SCRIPT_SHELL /gradle/gradle-jdks-setup.sh

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/sh

set -e

/root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/bin/java -version
# Running again the gradle-jdk-setup to check the JAVA_HOME.

# inserting the lines from gradle-jdks/src/main/resources/gradlew-patch.sh
PLACEHOLDER_INSERT_GRADLEW_PATCH

echo "JAVA_HOME is set to: $JAVA_HOME"
3 changes: 2 additions & 1 deletion gradle-jdks-setup/src/main/resources/gradle-jdks-setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ gradle_daemon_jdk_distribution_local_path=$(read_value "$APP_GRADLE_DIR"/jdks/"$
"$GRADLE_JDKS_HOME"/"$gradle_daemon_jdk_distribution_local_path"/bin/java -cp "$APP_GRADLE_DIR"/gradle-jdks-setup.jar com.palantir.gradle.jdks.setup.GradleJdkInstallationSetup daemonSetup "$APP_HOME" "$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path"

# [Used by ./gradlew only] Setting the Gradle Daemon Java Home to the JDK distribution
set -- "-Dorg.gradle.java.home=$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path" "$@"
export GRADLE_DAEMON_JDK="$GRADLE_JDKS_HOME/$gradle_daemon_jdk_distribution_local_path"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not setting the JAVA_HOME directly here as this script is called by the palantir-gradle-jdks intellij plugin as well and I don't think we should modify the JAVA_HOME there (when gradle jdk automanagement is set, then we also set the Project SDK)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am I right in thinking setting JAVA_HOME wouldn't really do anything bad though? We'd set the JAVA_HOME then the script would end to no real effect (when called from intellij in the before gradle step right?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Not that I think it really matters, the export GRADLE_DAEMON_JDK approach here is also fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am I right in thinking setting JAVA_HOME wouldn't really do anything bad though? We'd set the JAVA_HOME then the script would end to no real effect (when called from intellij in the before gradle step right?

I think you are right (I tested this initially inside Intellij) and it seemed fine. But I wouldn't want to add to any confusion if in the end something inside Intellij changes and the java home is somehow used.

set -- "-Dorg.gradle.java.home=$GRADLE_DAEMON_JDK" "$@"

cleanup
2 changes: 2 additions & 0 deletions gradle-jdks/src/main/resources/gradlew-patch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@ if [ -f gradle/gradle-jdks-setup.sh ]; then
echo "Failed to set up JDK, running gradle/gradle-jdks-setup.sh failed with non-zero exit code"
exit 1
fi
# Setting JAVA_HOME to the gradle daemon to make sure gradlew uses this jdk for `JAVACMD`
JAVA_HOME="$GRADLE_DAEMON_JDK"
fi
# <<< Gradle JDK setup <<<