Skip to content

Commit

Permalink
chore(dependencies): Upgrade Spring Boot to 2.5.14 (#1011)
Browse files Browse the repository at this point in the history
* chore(dependencies): Upgrade Spring Boot to 2.5.14

* fix(dependency): Issue with kork-jedis while upgrading spring-boot to 2.5.14

While upgrading the spring-boot, the compilation of kork-jedis module failed with following error:
```
> Task :kork-jedis:compileJava FAILED
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2652: error: slowlogGetBinary(long) in InstrumentedJedis cannot implement slowlogGetBinary(long) in AdvancedBinaryJedisCommands
  public List<byte[]> slowlogGetBinary(long entries) {
                      ^
  return type List<byte[]> is not compatible with List<Object>
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2646: error: slowlogGetBinary() in InstrumentedJedis cannot implement slowlogGetBinary() in AdvancedBinaryJedisCommands
  public List<byte[]> slowlogGetBinary() {
                      ^
  return type List<byte[]> is not compatible with List<Object>
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2645: error: method does not override or implement a method from a supertype
  @OverRide
  ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2648: error: incompatible types: inference variable T has incompatible bounds
    return instrumented(command, () -> delegated.slowlogGetBinary());
                       ^
    lower bounds: List<byte[]>,Object
    lower bounds: List<Object>
  where T is a type-variable:
    T extends Object declared in method <T>instrumented(String,Callable<T>)
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2651: error: method does not override or implement a method from a supertype
  @OverRide
  ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedis.java:2654: error: incompatible types: inference variable T has incompatible bounds
    return instrumented(command, () -> delegated.slowlogGetBinary(entries));
                       ^
    lower bounds: List<byte[]>,Object
    lower bounds: List<Object>
  where T is a type-variable:
    T extends Object declared in method <T>instrumented(String,Callable<T>)
```

```
> Task :kork-jedis:compileJava FAILED
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedisPool.java:84: error: name clash: initPool(GenericObjectPoolConfig,PooledObjectFactory<Jedis>) in InstrumentedJedisPool and initPool(GenericObjectPoolConfig<Jedis>,PooledObjectFactory<Jedis>) in Pool have the same erasure, yet neither overrides the other
  public void initPool(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
              ^
/kork/kork-jedis/src/main/java/com/netflix/spinnaker/kork/jedis/telemetry/InstrumentedJedisPool.java:83: error: method does not override or implement a method from a supertype
  @OverRide
```

The root cause is the upgrade of redis.clients:jedis from 3.3.0 to 3.6.3 as transitive dependency of spring-boot, that brings the breaking changes in APIs as mentioned below:
redis/jedis#2084
redis/jedis#2361

Fixed the issue with required code changes.

* fix(dependency): Issue with kork-web while upgrading spring-boot to 2.5.14

While upgrading the spring-boot, the compilation of kork-web module failed with following error:
```
> Task :kork-web:compileGroovy FAILED
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:41: error: incompatible types: Boolean cannot be converted to ErrorAttributeOptions
        errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
                                                       ^
```

```
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/controllers/GenericErrorController.java:51: error: method does not override or implement a method from a supertype
  @OverRide
  ^
```

```
> Task :kork-web:compileGroovy
/kork/kork-web/src/main/java/com/netflix/spinnaker/kork/web/selector/v2/SelectableService.java uses unchecked or unsafe operations.
startup failed:
/kork/kork-web/src/main/groovy/com/netflix/spinnaker/config/ErrorConfiguration.groovy: 37: Method 'getErrorAttributes' from class 'com.netflix.spinnaker.config.ErrorConfiguration$1' does not override method from its superclass or interfaces but is annotated with @OverRide.
 @ line 37, column 7.
         @OverRide
         ^
```
The root cause is the deprecation of following methods in Spring boot 2.3.x and now removal of code from spring boot 2.5.x:

ErrorAttributes.getErrorAttributes(ServerRequest, boolean)
spring-projects/spring-boot@158933c    spring-projects/spring-boot#21324

ErrorController.getErrorPath()
spring-projects/spring-boot#19844

Fixed the issue with required code changes.

* chore (dependency): pinning the io.rest-assured to 4.2.0

Spring boot 2.5.14 upgrade brings io.rest-assured 4.3.3 as transitive dependency.
io.rest-assured 4.3.x require groovy 3.0.2. So, pinning the nearest version using groovy 2.x. After upgrading the groovy to 3.x, pin can be removed. [https://github.com/rest-assured/rest-assured/blob/9b683130c93188cabdef850e89d0c9417d847a17/changelog.txt#L200]

* chore(dependency): pinning ch.qos.logback to 1.2.10

Spring boot 2.5.14 upgrade brings ch.qos.logback 1.2.11 as transitive dependency.
A bug is reported in 1.2.11 [https://jira.qos.ch/browse/LOGBACK-1623] and it is fixed in 1.2.12.
However the 1.2.12 package has not been released yet. So, pinning the version to 1.2.10 untill required package is released.

* chore(dependency): Pinning groovy to 2.5.15 with spring boot 2.5.x upgrade

Spring boot 2.5.x brings groovy 3.x as its transitive dependency.
https://docs.spring.io/spring-boot/docs/2.5.14/reference/html/dependency-versions.html#appendix.dependency-versions

Currently spinnaker services use gradle 6.x, that does not support groovy 3.x.
https://docs.gradle.org/6.8.1/userguide/compatibility.html

Restricting groovy to 2.5.x, till upgrade of gradle to 7.x.
https://docs.gradle.org/current/userguide/resolution_rules.html#sec:denying_version
To avoid transitive upgrade of groovy, pinning it with enforcedPlatform() closure.
It forces version for internal submodules of kork as well as for all the consumer spinnaker services.

* chore(dependency): pin org.liquibase to 3.10.3

 While upgrading spring boot 2.5.x, liquibase version transitively upgrades to 4.3.5. The liquibase version starting from 4.0.0 till 4.12.0 has an [issue](liquibase/liquibase#2818) w.r.t parsing the changelog file, if found at multiple places within the classpath and encounter the below error:
 ```
 Caused by: liquibase.exception.ChangeLogParseException: Error parsing classpath:db/healthcheck.yml
	at liquibase.parser.core.yaml.YamlChangeLogParser.parse(YamlChangeLogParser.java:89)
	at liquibase.Liquibase.getDatabaseChangeLog(Liquibase.java:369)
	at liquibase.Liquibase.lambda$update$1(Liquibase.java:224)
	at liquibase.Scope.lambda$child$0(Scope.java:180)
	at liquibase.Scope.child(Scope.java:189)
	at liquibase.Scope.child(Scope.java:179)
	at liquibase.Scope.child(Scope.java:158)
	at liquibase.Liquibase.runInScope(Liquibase.java:2405)
	at liquibase.Liquibase.update(Liquibase.java:211)
	at liquibase.Liquibase.update(Liquibase.java:197)
	at liquibase.integration.spring.SpringLiquibase.performUpdate(SpringLiquibase.java:314)
	at liquibase.integration.spring.SpringLiquibase.afterPropertiesSet(SpringLiquibase.java:269)
	at com.netflix.spinnaker.kork.sql.migration.SpringLiquibaseProxy.afterPropertiesSet(SpringLiquibaseProxy.kt:65)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1858)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1795)
	... 87 more
Caused by: java.io.IOException: Found 2 files that match classpath:db/healthcheck.yml: file:/spinnaker/kork/kork-sql/build/resources/main/db/healthcheck.yml, jar:file:/spinnaker/kork/kork-sql/build/libs/kork-sql.jar!/db/healthcheck.yml
	at liquibase.resource.AbstractResourceAccessor.openStream(AbstractResourceAccessor.java:25)
	at liquibase.parser.core.yaml.YamlChangeLogParser.parse(YamlChangeLogParser.java:25)
	... 101 more
 ```
 This duplicate changelog issue is fixed in 4.13.0, but identified another issue that gets introduced in 4.13.0. This [issue](liquibase/liquibase#3091) hinders the migration of sql scripts available in [orca](https://github.com/spinnaker/orca/tree/master/orca-sql/src/main/resources/db/changelog), containing `afterColumn`, with a validation error for postgresql.
 The efforts to resolve the issue are in progress, so pinning the version of org.liquibase:liquibase-core to 3.10.3 (latest of 3.x series).
  • Loading branch information
j-sandy authored Mar 31, 2023
1 parent 7e6e0d4 commit f841432
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2643,13 +2643,13 @@ public Long slowlogLen() {
}

@Override
public List<byte[]> slowlogGetBinary() {
public List<Object> slowlogGetBinary() {
String command = "slowlogGetBinary";
return instrumented(command, () -> delegated.slowlogGetBinary());
}

@Override
public List<byte[]> slowlogGetBinary(long entries) {
public List<Object> slowlogGetBinary(long entries) {
String command = "slowlogGetBinary";
return instrumented(command, () -> delegated.slowlogGetBinary(entries));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
import java.lang.reflect.Field;
import java.util.Objects;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.commons.pool2.PooledObjectFactory;
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.JedisPool;

Expand Down Expand Up @@ -84,11 +82,6 @@ public boolean isClosed() {
return delegated.isClosed();
}

@Override
public void initPool(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
// Explicitly not initializing the pool here, as the delegated pool will initialize itself
}

@Override
public void destroy() {
delegated.destroy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator
import com.netflix.spinnaker.kork.web.exceptions.ExceptionSummaryService
import com.netflix.spinnaker.kork.web.exceptions.GenericExceptionHandlers
import org.springframework.beans.factory.ObjectProvider
import org.springframework.boot.web.error.ErrorAttributeOptions
import org.springframework.boot.web.servlet.error.DefaultErrorAttributes
import org.springframework.boot.web.servlet.error.ErrorAttributes
import org.springframework.context.annotation.Bean
Expand All @@ -35,10 +36,10 @@ class ErrorConfiguration {
final DefaultErrorAttributes defaultErrorAttributes = new DefaultErrorAttributes()
return new ErrorAttributes() {
@Override
Map<String, Object> getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) {
Map<String, Object> getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions includeStackTrace) {
// By default, Spring echoes back the user's requested path. This opens up a potential XSS vulnerability where a
// user, for example, requests "GET /<script>alert('Hi')</script> HTTP/1.1".
Map<String, Object> errorAttributes = defaultErrorAttributes.getErrorAttributes(webRequest, includeStackTrace)
Map<String, Object> errorAttributes = defaultErrorAttributes.getErrorAttributes(webRequest, includeStackTrace.getIncludes().contains(ErrorAttributeOptions.Include.STACK_TRACE))
errorAttributes.remove("path")
return errorAttributes
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.netflix.spinnaker.kork.exceptions.HasAdditionalAttributes;
import java.util.Map;
import org.springframework.boot.web.error.ErrorAttributeOptions;
import org.springframework.boot.web.servlet.error.ErrorAttributes;
import org.springframework.boot.web.servlet.error.ErrorController;
import org.springframework.web.bind.annotation.RequestMapping;
Expand All @@ -33,12 +34,15 @@ public GenericErrorController(ErrorAttributes errorAttributes) {
this.errorAttributes = errorAttributes;
}

@RequestMapping(value = "/error")
@RequestMapping(value = "${server.error.path:/error}")
public Map error(
@RequestParam(value = "trace", defaultValue = "false") Boolean includeStackTrace,
WebRequest webRequest) {
Map<String, Object> attributes =
errorAttributes.getErrorAttributes(webRequest, includeStackTrace);
ErrorAttributeOptions options =
includeStackTrace
? ErrorAttributeOptions.defaults().including(ErrorAttributeOptions.Include.STACK_TRACE)
: ErrorAttributeOptions.defaults();
Map<String, Object> attributes = errorAttributes.getErrorAttributes(webRequest, options);

Throwable exception = errorAttributes.getError(webRequest);
if (exception != null && exception instanceof HasAdditionalAttributes) {
Expand All @@ -47,9 +51,4 @@ public Map error(

return attributes;
}

@Override
public String getErrorPath() {
return "/error";
}
}
55 changes: 51 additions & 4 deletions spinnaker-dependencies/spinnaker-dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ ext {
bouncycastle : "1.70",
brave : "5.12.3",
gcp : "25.3.0",
groovy : "2.5.15",
jooq : "3.13.6",
jsch : "0.1.54",
jschAgentProxy : "0.0.9",
Expand All @@ -22,12 +23,17 @@ ext {
okhttp : "2.7.5", // CVE-2016-2402
okhttp3 : "3.14.9",
openapi : "1.3.9", // this needs to be kept in sync with spring boot as it pulls in the spring-boot-dependencies BOM
//Spring boot 2.5.14 upgrade brings io.rest-assured 4.3.3 as transitive dependency.
//io.rest-assured 4.3.x require groovy 3.0.2. So, pinning the nearest version
//using groovy 2.x. This can be removed with groovy 3.x upgrade.
//[https://github.com/rest-assured/rest-assured/blob/9b683130c93188cabdef850e89d0c9417d847a17/changelog.txt#L200]
restassured : "4.2.0",
retrofit : "1.9.0",
retrofit2 : "2.8.1",
spectator : "1.0.6",
spek : "1.1.5",
spek2 : "2.0.9",
springBoot : "2.4.13",
springBoot : "2.5.14",
springCloud : "2020.0.6",
springfoxSwagger : "2.9.2",
swagger : "1.5.20", //this should stay in sync with what springfoxSwagger expects
Expand All @@ -53,6 +59,12 @@ dependencies {

api(platform("org.apache.logging.log4j:log4j-bom:2.16.0"))

//Upgrade of spring boot 2.5.x brings groovy 3.x as transitive dependency.
//To avoid transitive upgrade of groovy, pinning it with enforcedPlatform() closure.
//It forces version for internal submodules of kork as well as for all the consumer spinnaker services.
api(enforcedPlatform("org.codehaus.groovy:groovy-bom:${versions.groovy}")){
force = true
}
api(platform("org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.4.3"))
//kotlinVersion comes from gradle.properties since we have kotlin code in
// this project and need to configure gradle plugins etc.
Expand All @@ -74,9 +86,18 @@ dependencies {

constraints {
api("cglib:cglib-nodep:3.3.0")
api("ch.qos.logback:logback-access:${versions.logback}")
api("ch.qos.logback:logback-classic:${versions.logback}")
api("ch.qos.logback:logback-core:${versions.logback}")
//A bug is reported in 1.2.11 and fixed in 1.2.12.
//So pinning the version to 1.2.10 untill required package is released.
//[https://jira.qos.ch/browse/LOGBACK-1623]
api("ch.qos.logback:logback-core:${versions.logback}"){
force = true
}
api("ch.qos.logback:logback-classic:${versions.logback}"){
force = true
}
api("ch.qos.logback:logback-classic:${versions.logback}"){
force = true
}
api("com.amazonaws:aws-java-sdk:${versions.aws}")
api("com.google.api-client:google-api-client:1.30.10") // TODO: Track update for CVE-2020-7692, reanalysis pending.
api("com.google.apis:google-api-services-admin-directory:directory_v1-rev105-1.25.0")
Expand Down Expand Up @@ -140,6 +161,18 @@ dependencies {
api("de.huxhorn.sulky:de.huxhorn.sulky.ulid:8.2.0")
api("dev.minutest:minutest:1.13.0")
api("io.mockk:mockk:1.10.5")
api("io.rest-assured:rest-assured:${versions.restassured}"){
force = true
}
api("io.rest-assured:rest-assured-common:${versions.restassured}"){
force = true
}
api("io.rest-assured:json-path:${versions.restassured}"){
force = true
}
api("io.rest-assured:xml-path:${versions.restassured}"){
force = true
}
api("io.springfox:springfox-swagger-ui:${versions.springfoxSwagger}")
api("io.springfox:springfox-swagger2:${versions.springfoxSwagger}")
api("io.swagger:swagger-annotations:${versions.swagger}")
Expand All @@ -160,6 +193,20 @@ dependencies {
api("org.jetbrains.spek:spek-subject-extension:${versions.spek}")
api("org.jooq:jooq:${versions.jooq}")
api("org.jooq:jooq-kotlin:${versions.jooq}")

// The liquibase version starting from 4.0.0 till 4.12.0
// has an issue w.r.t parsing the changelog file,
// if found at multiple places within the classpath
// https://github.com/liquibase/liquibase/issues/2818
// This duplicate changelog issue is fixed in 4.13.0,
// but identified another issue that gets introduced in 4.13.0.
// This issue(https://github.com/liquibase/liquibase/issues/3091)
// hinders the migration of sql scripts available in orca
// (https://github.com/spinnaker/orca/tree/master/orca-sql/src/main/resources/db/changelog),
// containing `afterColumn` construct, with a validation error for postgresql. So pin with 3.10.3
api("org.liquibase:liquibase-core:3.10.3"){
force = true
}
api("org.objenesis:objenesis:2.5.1")
api("org.pf4j:pf4j:3.2.0")
api("org.pf4j:pf4j-update:2.3.0")
Expand Down

0 comments on commit f841432

Please sign in to comment.