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

Set JAVA_HOME in gradle-jdks-setup script #447

Merged
merged 14 commits into from
Oct 25, 2024
Merged

Set JAVA_HOME in gradle-jdks-setup script #447

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d2fab83
set java home
crogoz Oct 24, 2024
bcbcb4b
fix no java_home set
crogoz Oct 24, 2024
b97f469
Add generated changelog entries
svc-changelog Oct 24, 2024
1cf1a67
set
crogoz Oct 24, 2024
4cdd6c7
Merge branch 'cr/java-home' of github.com:palantir/gradle-jdks into c…
crogoz Oct 24, 2024
39acc92
set
crogoz Oct 24, 2024
e269cf6
checl
crogoz Oct 24, 2024
6794c75
test hava_home
crogoz Oct 24, 2024
fa57ae2
add certs
crogoz Oct 24, 2024
8c2a628
add certs
crogoz Oct 24, 2024
0726559
java home
crogoz Oct 24, 2024
b6bb8d8
test output
crogoz Oct 24, 2024
6f1043b
gradlew patch
crogoz Oct 25, 2024
6f9a3e7
fix output format
crogoz Oct 25, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
37 changes: 8 additions & 29 deletions ...onTest/java/com/palantir/gradle/jdks/setup/GradleJdkInstallationSetupIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.palantir.gradle.jdks.setup.common.CurrentArch;
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;
Expand All @@ -46,27 +45,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 @@ -134,28 +132,14 @@ private Path setupGradleDirectoryStructure(Os os) throws IOException {
// 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"));

return gradleDirectory;
}

private static void writeFileContent(Path path, String content) throws IOException {
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 +157,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 \"21.0.4\"")
.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 +180,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
View file
Open in desktop

This file was deleted.

4 changes: 1 addition & 3 deletions gradle-jdks-setup/src/integrationTest/resources/template.Dockerfile
View file
Open in desktop
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
17 changes: 14 additions & 3 deletions gradle-jdks-setup/src/integrationTest/resources/testing-script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,18 @@
set -e

/root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/bin/java -version
if [ -f /palantir.crt ];then
echo "Palantir cert:" $(/root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/bin/keytool -keystore /root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/lib/security/cacerts -list -alias gradlejdks_palantir3rd-generationrootca -storepass changeit)
# Running again the gradle-jdk-setup to check the JAVA_HOME.

# >>> Gradle JDK setup >>>
# !! Contents within this block are managed by 'palantir/gradle-jdks' !!
if [ -f gradle/gradle-jdks-setup.sh ]; then
if ! . 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
echo "Example.com cert:" $(/root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/bin/keytool -keystore /root/.gradle/gradle-jdks/amazon-corretto-11.0.21.9.1/lib/security/cacerts -list -alias gradleJdks_example.com -storepass changeit)
# <<< Gradle JDK setup <<<
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this manually placed here or is it kept up to date with gradlew-patch.sh somehow?


echo "JAVA_HOME is set to: $JAVA_HOME"
3 changes: 2 additions & 1 deletion gradle-jdks-setup/src/main/resources/gradle-jdks-setup.sh
View file
Open in desktop
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
View file
Open in desktop
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 <<<