From 18baaf3e8ac3e2536f1db1da2e00f799623f460b Mon Sep 17 00:00:00 2001 From: David Byron <82477955+dbyron-sf@users.noreply.github.com> Date: Mon, 19 Feb 2024 20:19:17 -0800 Subject: [PATCH 1/3] feat(retrofit): include the request method in SpinnakerHttpException messages (#1150) when available Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../kork/retrofit/exceptions/SpinnakerHttpException.java | 8 +++++++- .../exceptions/SpinnakerRetrofit2ErrorHandleTest.java | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java index 7a705797f..500e93d30 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerHttpException.java @@ -194,8 +194,14 @@ public String getMessage() { return super.getMessage(); } + if (getHttpMethod() == null) { + return String.format( + "Status: %s, URL: %s, Message: %s", responseCode, this.getUrl(), getRawMessage()); + } + return String.format( - "Status: %s, URL: %s, Message: %s", responseCode, this.getUrl(), getRawMessage()); + "Status: %s, Method: %s, URL: %s, Message: %s", + responseCode, getHttpMethod(), this.getUrl(), getRawMessage()); } @Override diff --git a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java index f82efd3a0..bc294dc48 100644 --- a/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java +++ b/kork-retrofit/src/test/java/com/netflix/spinnaker/kork/retrofit/exceptions/SpinnakerRetrofit2ErrorHandleTest.java @@ -234,7 +234,8 @@ void testNonJsonHttpErrorResponse() { assertThat(spinnakerHttpException.getResponseBody()).isNull(); assertThat(spinnakerHttpException.getResponseCode()).isEqualTo(responseCode); assertThat(spinnakerHttpException) - .hasMessage("Status: " + responseCode + ", URL: " + url + ", Message: " + reason); + .hasMessage( + "Status: " + responseCode + ", Method: GET, URL: " + url + ", Message: " + reason); assertThat(spinnakerHttpException.getUrl()).isEqualTo(url); assertThat(spinnakerHttpException.getReason()).isEqualTo(reason); } From eba9e740284541a844dadc8f1f9b57ca99c89ea7 Mon Sep 17 00:00:00 2001 From: David Byron <82477955+dbyron-sf@users.noreply.github.com> Date: Mon, 19 Feb 2024 22:16:46 -0800 Subject: [PATCH 2/3] chore(deps): use version 2.7.1 of jinjava (#1152) Here are snippets from $ ./gradlew orca-web:dependencies before: | | \--- com.hubspot.jinjava:jinjava:2.5.2 | | +--- org.slf4j:slf4j-api:1.7.21 -> 1.7.36 | | +--- com.google.guava:guava:22.0 -> 30.0-jre | | | +--- com.google.guava:failureaccess:1.0.1 | | | +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava | | | +--- com.google.code.findbugs:jsr305:3.0.2 | | | +--- org.checkerframework:checker-qual:3.5.0 -> 3.19.0 | | | +--- com.google.errorprone:error_prone_annotations:2.3.4 -> 2.10.0 | | | \--- com.google.j2objc:j2objc-annotations:1.3 | | +--- org.javassist:javassist:3.24.1-GA | | +--- org.jsoup:jsoup:1.10.3 | | +--- com.google.re2j:re2j:1.2 | | +--- org.apache.commons:commons-lang3:3.5 -> 3.12.0 | | +--- commons-net:commons-net:3.3 | | +--- com.google.code.findbugs:annotations:3.0.1 | | +--- com.fasterxml.jackson.core:jackson-databind:2.7.9.5 -> 2.12.7.1 (*) | | +--- com.fasterxml.jackson.core:jackson-core:2.7.9 -> 2.12.7 (*) | | \--- ch.obermuhlner:big-math:2.0.0 after: | | \--- com.hubspot.jinjava:jinjava:2.7.1 | | +--- org.slf4j:slf4j-api:1.7.25 -> 1.7.36 | | +--- com.google.guava:guava:31.1-jre -> 30.0-jre | | | +--- com.google.guava:failureaccess:1.0.1 | | | +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava | | | +--- com.google.code.findbugs:jsr305:3.0.2 | | | +--- org.checkerframework:checker-qual:3.5.0 -> 3.19.0 | | | +--- com.google.errorprone:error_prone_annotations:2.3.4 -> 2.10.0 | | | \--- com.google.j2objc:j2objc-annotations:1.3 | | +--- org.javassist:javassist:3.24.1-GA | | +--- com.google.re2j:re2j:1.2 | | +--- org.apache.commons:commons-lang3:3.9 -> 3.12.0 | | +--- commons-net:commons-net:3.9.0 | | +--- com.googlecode.java-ipv6:java-ipv6:0.17 | | +--- com.google.code.findbugs:annotations:3.0.1 | | +--- com.fasterxml.jackson.core:jackson-annotations:2.14.0 -> 2.12.7 (*) | | +--- com.fasterxml.jackson.core:jackson-databind:2.14.0 -> 2.12.7.1 (*) | | +--- com.fasterxml.jackson.core:jackson-core:2.14.0 -> 2.12.7 (*) | | +--- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.14.0 -> 2.12.7 | | | +--- com.fasterxml.jackson.core:jackson-databind:2.12.7 -> 2.12.7.1 (*) | | | +--- org.yaml:snakeyaml:1.27 | | | +--- com.fasterxml.jackson.core:jackson-core:2.12.7 (*) | | | \--- com.fasterxml.jackson:jackson-bom:2.12.7 (*) | | \--- ch.obermuhlner:big-math:2.0.0 Note the following CVE exposure before this PR: - jinjava 2.5.2 - CVE-2020-12668, fixed in 2.5.3 - sonatype-2021-0948, fixed in 2.5.10 - commons-net 3.3 - CVE-2021-37533, fixed in 3.9 - jsoup 1.10.3 - CVE-2021-37714, fixed in 1.14.2 - CVE-2022-36033, fixed in 1.15.3 After this PR, all these are resolved. jinjava 2.7.1 brings in commons-net 3.9 and jsoup 1.15.3, though jsoup is shaded. See https://github.com/HubSpot/jinjava/blob/jinjava-2.7.1/pom.xml#L34 and https://github.com/HubSpot/jinjava/blob/jinjava-2.7.1/pom.xml#L240. Use version 2.7.1 of jinjava since it's the first version that fixes https://github.com/HubSpot/jinjava/issues/429 via https://github.com/HubSpot/jinjava/pull/1008. Co-authored-by: Jason Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- spinnaker-dependencies/spinnaker-dependencies.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spinnaker-dependencies/spinnaker-dependencies.gradle b/spinnaker-dependencies/spinnaker-dependencies.gradle index 35106800b..59bb07d41 100644 --- a/spinnaker-dependencies/spinnaker-dependencies.gradle +++ b/spinnaker-dependencies/spinnaker-dependencies.gradle @@ -80,8 +80,7 @@ dependencies { api("com.google.cloud:google-cloud-secretmanager:2.3.10") api("com.google.code.findbugs:jsr305:3.0.2") api("com.google.guava:guava:30.0-jre") - // JinJava 2.5.3 has a bad bug: https://github.com/HubSpot/jinjava/issues/429 - api("com.hubspot.jinjava:jinjava:2.5.2") + api("com.hubspot.jinjava:jinjava:2.7.1") api("com.jakewharton.retrofit:retrofit1-okhttp3-client:1.1.0") api("com.jcraft:jsch:${versions.jsch}") api("com.jcraft:jsch.agentproxy.connector-factory:${versions.jschAgentProxy}") From 42d2de607a364497ee64cfc3206aff206ef5e451 Mon Sep 17 00:00:00 2001 From: David Byron <82477955+dbyron-sf@users.noreply.github.com> Date: Mon, 19 Feb 2024 22:28:52 -0800 Subject: [PATCH 3/3] chore(dependencies): upgrade pf4j from 3.2.0 to 3.10.0 to resolve CVE-2023-40828 (#1153) pf4j 3.10.0 brings in slf4j-api 2.0.6 which is not compatible with logback 1.2.x. Upgrading logback created incompatibility issues with Springboot's LogbackLoggingSystem. > Task :kork-tomcat:test com.netflix.spinnaker.kork.tomcat.CRLFHeaderTest > clientTest() FAILED java.lang.NoClassDefFoundError at LogbackLoggingSystem.java:293 Caused by: java.lang.ClassNotFoundException at BuiltinClassLoader.java:581 1 test completed, 1 failed Caused by: java.lang.ClassNotFoundException at BuiltinClassLoader.java:581 So, pin slf4j-api to 1.7.36 to retain compatibility with logback to 1.2.x. Removed a test(extensions index is written to META-INF) from TestPluginGeneratorTest.kt as the upgraded pf4j no longer create extensions.idx if no extensions exist.(refer https://github.com/pf4j/pf4j/issues/508) Note: PluginWrapper is deprecated in 3.10.0 and will be removed in the next major release as per this - https://github.com/pf4j/pf4j/pull/512. So next pf4j upgrade would break the functionality of existing plugins. Co-authored-by: Kiran Godishala Co-authored-by: Jason Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../netflix/kork/plugins/TestPluginGeneratorTest.kt | 10 ---------- .../plugins/repository/PluginRefPluginRepository.kt | 2 +- spinnaker-dependencies/spinnaker-dependencies.gradle | 11 ++++++++++- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/kork-plugins-tck/src/test/kotlin/com/spinnaker/netflix/kork/plugins/TestPluginGeneratorTest.kt b/kork-plugins-tck/src/test/kotlin/com/spinnaker/netflix/kork/plugins/TestPluginGeneratorTest.kt index 228ae6847..c17b0bef5 100644 --- a/kork-plugins-tck/src/test/kotlin/com/spinnaker/netflix/kork/plugins/TestPluginGeneratorTest.kt +++ b/kork-plugins-tck/src/test/kotlin/com/spinnaker/netflix/kork/plugins/TestPluginGeneratorTest.kt @@ -38,16 +38,6 @@ class TestPluginGeneratorTest : JUnit5Minutests { expectThat(resolve("classes")).describedAs("classes directory").isDirectory() } - test("extensions index is written to META-INF") { - expectThat(resolve("classes/META-INF")).and { - isDirectory() - get { resolve("extensions.idx") }.and { - isRegularFile() - get { toFile().readText() }.isEqualTo("# Generated by PF4J\n") - } - } - } - test("generated class is written to subdirectories matching package") { expectThat(resolve("classes/com/netflix/spinnaker/kork/plugins/testplugin/generated")).and { isDirectory() diff --git a/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/repository/PluginRefPluginRepository.kt b/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/repository/PluginRefPluginRepository.kt index 5ff62e71a..4534b3dcb 100644 --- a/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/repository/PluginRefPluginRepository.kt +++ b/kork-plugins/src/main/kotlin/com/netflix/spinnaker/kork/plugins/repository/PluginRefPluginRepository.kt @@ -26,6 +26,6 @@ import org.pf4j.util.ExtensionFileFilter /** * A [PluginRepository] supporting [PluginRef] type [Plugin]s by matching files with the extension [PluginRef.EXTENSION]. */ -class PluginRefPluginRepository(pluginPath: Path) : BasePluginRepository(pluginPath, ExtensionFileFilter(PluginRef.EXTENSION)) { +class PluginRefPluginRepository(pluginPath: Path) : BasePluginRepository(listOf(pluginPath), ExtensionFileFilter(PluginRef.EXTENSION)) { override fun deletePluginPath(pluginPath: Path?): Boolean = false } diff --git a/spinnaker-dependencies/spinnaker-dependencies.gradle b/spinnaker-dependencies/spinnaker-dependencies.gradle index 59bb07d41..9a15997a5 100644 --- a/spinnaker-dependencies/spinnaker-dependencies.gradle +++ b/spinnaker-dependencies/spinnaker-dependencies.gradle @@ -159,7 +159,16 @@ dependencies { } } api("org.objenesis:objenesis:2.5.1") - api("org.pf4j:pf4j:3.2.0") + api("org.pf4j:pf4j:3.10.0") + // pf4j:3.10.0 brings in slf4j-api:2.0.6 which is not compatible with logback 1.2.x. + // And the upgraded logback version(1.3.8) is becoming incompatible with SpringBoot's LogbackLoggingSystem: + // java.lang.NoClassDefFoundError at LogbackLoggingSystem.java:293 + // Hence pinning slf4j-api at 1.7.36 which spring boot 2.5.15 brings in. + api("org.slf4j:slf4j-api"){ + version { + strictly("1.7.36") + } + } api("org.pf4j:pf4j-update:2.3.0") // snakeyaml 1.29 fails to parse yaml (including some k8s manifests), so