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

chore(java): Full java 17 only support #2107

Merged
merged 15 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
uses: docker/setup-buildx-action@v3
- uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: 'zulu'
cache: 'gradle'
- name: Prepare build variables
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
uses: docker/setup-buildx-action@v3
- uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: 'zulu'
cache: 'gradle'
- name: Prepare build variables
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
uses: docker/setup-buildx-action@v3
- uses: actions/setup-java@v4
with:
java-version: 11
java-version: 17
distribution: 'zulu'
cache: 'gradle'
- name: Assemble release info
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.compile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
FROM ubuntu:bionic
FROM ubuntu:jammy
LABEL maintainer="[email protected]"
RUN echo "mysql-server mysql-server/root_password password sa" | debconf-set-selections
RUN echo "mysql-server mysql-server/root_password_again password sa" | debconf-set-selections
RUN apt-get update && apt-get install -y \
openjdk-11-jdk \
openjdk-17-jdk \
mysql-server \
&& rm -rf /var/lib/apt/lists/*
ENV GRADLE_USER_HOME /workspace/.gradle
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.slim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM alpine:3.11
FROM alpine:3.20
LABEL maintainer="[email protected]"
RUN apk --no-cache add --update bash openjdk11-jre
RUN apk --no-cache add --update bash openjdk17-jre
RUN addgroup -S -g 10111 spinnaker
RUN adduser -S -G spinnaker -u 10111 spinnaker
COPY keel-web/build/install/keel /opt/keel
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.ubuntu
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM ubuntu:bionic
FROM ubuntu:jammy
LABEL maintainer="[email protected]"
RUN apt-get update && apt-get -y install openjdk-11-jre-headless wget
RUN apt-get update && apt-get -y install openjdk-17-jre-headless wget
RUN adduser --system --uid 10111 --group spinnaker
COPY keel-web/build/install/keel /opt/keel
RUN mkdir -p /opt/keel/plugins && chown -R spinnaker:nogroup /opt/keel/plugins
Expand Down
4 changes: 0 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ allprojects {
apply(plugin: "com.github.ben-manes.versions")
apply(plugin: "com.diffplug.spotless")

java {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

dependencies {
annotationProcessor(platform("io.spinnaker.kork:kork-bom:${korkVersion}"))
Expand Down
3 changes: 3 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
targetJava17=true
kotlinVersion=1.6.21
fiatVersion=1.50.0
korkVersion=7.243.0
Expand All @@ -9,6 +10,8 @@ okHttpVersion=4.5.0
resilience4jVersion=1.5.0
spinnakerGradleVersion=8.32.1

org.gradle.jvmargs=-Xmx4g -Xms4g

# Used to control whether to spin up docker to run liquibase before jooq
buildingInDocker=false

Expand Down
4 changes: 2 additions & 2 deletions gradle/kotlin.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ apply plugin: "kotlin-allopen"
compileKotlin {
kotlinOptions {
languageVersion = "1.6"
jvmTarget = "11"
jvmTarget = "17"
// see https://kotlinlang.org/docs/java-to-kotlin-interop.html#compatibility-mode-for-default-methods
freeCompilerArgs += "-Xjvm-default=enable"
}
Expand All @@ -14,7 +14,7 @@ compileKotlin {
compileTestKotlin {
kotlinOptions {
languageVersion = "1.6"
jvmTarget = "11"
jvmTarget = "17"
freeCompilerArgs += "-Xjvm-default=enable"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import strikt.assertions.isNull
import strikt.assertions.isTrue
import java.time.Clock
import java.time.Instant
import java.time.temporal.ChronoUnit

abstract class ArtifactRepositoryTests<T : ArtifactRepository> : JUnit5Minutests {
val publisher: ApplicationEventPublisher = mockk(relaxed = true)
Expand Down Expand Up @@ -649,7 +650,7 @@ abstract class ArtifactRepositoryTests<T : ArtifactRepository> : JUnit5Minutests
}

context("artifact creation timestamp exists") {
val createdAt = Instant.now()
val createdAt = Instant.now().truncatedTo(ChronoUnit.MILLIS)
jasonmcintosh marked this conversation as resolved.
Show resolved Hide resolved
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved

before {
subject.register(versionedSnapshotDebian)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.netflix.spinnaker.keel.serialization;

import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.datatype.jsr310.ser.InstantSerializerBase;
import java.time.Instant;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;

public class PrecisionSqlSerializer extends InstantSerializerBase<Instant> {
jasonmcintosh marked this conversation as resolved.
Show resolved Hide resolved
public PrecisionSqlSerializer() {
super(
Instant.class,
Instant::toEpochMilli,
Instant::getEpochSecond,
Instant::getNano,
new DateTimeFormatterBuilder().appendInstant(3).toFormatter());
}

@Override
protected InstantSerializerBase<?> withFormat(
Boolean aBoolean, DateTimeFormatter dateTimeFormatter, JsonFormat.Shape shape) {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule
import com.fasterxml.jackson.module.kotlin.registerKotlinModule
import com.netflix.spinnaker.keel.jackson.registerKeelApiModule
import de.huxhorn.sulky.ulid.ULID
import org.springframework.boot.jackson.JsonComponentModule
import java.text.SimpleDateFormat
import java.time.Instant
import java.util.TimeZone

/**
Expand All @@ -29,18 +29,20 @@ fun configuredObjectMapper(): ObjectMapper = ObjectMapper().configureForKeel()
* Factory method for [YAMLMapper]s configured how we like 'em.
*/
fun configuredYamlMapper(): YAMLMapper = YAMLMapper().configureForKeel().disable(USE_NATIVE_TYPE_ID)

fun <T : ObjectMapper> T.configureForKeel(): T =
apply {
fun <T : ObjectMapper> T.configureForKeel(): T {
val javaTimeModule = JavaTimeModule()
javaTimeModule.addSerializer(Instant::class.java,PrecisionSqlSerializer())
Copy link

Choose a reason for hiding this comment

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

Can we add a round trip test that uses the serializer and then deserializes to ensure precision is in fact kept? Other than that, this PR looks good

Copy link
Member Author

Choose a reason for hiding this comment

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

It's actually tested indirectly via the ArtifactRepositoryTests file but let me see about a direct test. It'll only keep as much precision as it writes in the JSON fields. So this WILL reduce precision on these columns from what the JVM could track - but it's never had that until java 17 it looks like so we're not losing anything - though I may try to do a "6" instead of "3" precision as docs say MySQL CAN handle that level.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a test see what you think :)

return apply {
registerKeelApiModule()
.registerKotlinModule()
.registerULIDModule()
.registerModule(JavaTimeModule())
.registerModule(javaTimeModule)
.configureSaneDateTimeRepresentation()
.disable(FAIL_ON_UNKNOWN_PROPERTIES)
.enable(ACCEPT_CASE_INSENSITIVE_ENUMS)
.setSerializationInclusion(NON_NULL)
}
}

private fun ObjectMapper.registerULIDModule(): ObjectMapper =
registerModule(
Expand Down
2 changes: 1 addition & 1 deletion keel-sql/src/test/resources/testcontainers.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ryuk.container.image = public.ecr.aws/s4w6t4b6/testcontainers/ryuk:0.3.4
ryuk.container.image = testcontainers/ryuk:0.11.0
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI the ECR image wasn't working in local testing - thus the need to switch. PROBABLY should use docker.io/testcontainers/ryuk:0.11.0 but this works