From e823562f7dd449b980708c1562b718c1e9da70e1 Mon Sep 17 00:00:00 2001 From: Sandesh <30489233+j-sandy@users.noreply.github.com> Date: Tue, 12 Mar 2024 22:27:28 +0530 Subject: [PATCH] chore(dependency): upgrade spring boot from 2.5.15 to 2.6.15 and spring cloud from 2020.0.x to 2021.0.x (#1134) * chore(dependency): upgrade spring boot from 2.5.x to 2.6.x and spring cloud from 2020.0.x to 2021.0.x Spring cloud release 2021.0.x is compatible with spring boot 2.6.x. https://github.com/spring-cloud/spring-cloud-release/wiki/Supported-Versions#supported-releases While upgrading spring boot 2.6.15 and spring cloud 2021.0.8, encounter below errors in kork-plugins and kork-tomcat modules: ``` > Task :kork-plugins:compileTestKotlin FAILED e: /kork/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt: (50, 11): Type mismatch: inferred type is Array but SpringApplication! was expected e: /kork/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt: (51, 11): Type mismatch: inferred type is ConfigurableEnvironment but Array<(out) String!>! was expected e: /kork/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt: (52, 9): No value passed for parameter 'environment' ``` The root cause is the deprecation and removal of `org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent` constructor without `org.springframework.boot.SpringApplication` in spring boot 2.6.x. https://docs.spring.io/spring-boot/docs/2.5.15/api/org/springframework/boot/context/event/ApplicationEnvironmentPreparedEvent.html To fix this error add mock for `SpringApplication` in `ApplicationEnvironmentPreparedEvent` constructor. * fix(springfox): add bean postprocessor to fix springfox to work with spring boot 2.6.x Springfox may cause application failure with spring boot 2.6.x, if used along with actuators. It is mentioned in [release notes](https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.6-Release-Notes#pathpattern-based-path-matching-strategy-for-spring-mvc). The effects are visible in the form of gate test failure with below given error: ``` Caused by: org.springframework.context.ApplicationContextException: Failed to start bean 'documentationPluginsBootstrapper'; nested exception is java.lang.NullPointerException: Cannot invoke "org.springframework.web.servlet.mvc.condition.PatternsRequestCondition.toString()" because the return value of "springfox.documentation.spi.service.contexts.Orderings.patternsCondition(springfox.documentation.RequestHandler)" is null at app//org.springframework.context.support.DefaultLifecycleProcessor.doStart(DefaultLifecycleProcessor.java:181) at app//org.springframework.context.support.DefaultLifecycleProcessor.access$200(DefaultLifecycleProcessor.java:54) at app//org.springframework.context.support.DefaultLifecycleProcessor$LifecycleGroup.start(DefaultLifecycleProcessor.java:356) at java.base@17.0.8.1/java.lang.Iterable.forEach(Iterable.java:75) at app//org.springframework.context.support.DefaultLifecycleProcessor.startBeans(DefaultLifecycleProcessor.java:155) at app//org.springframework.context.support.DefaultLifecycleProcessor.onRefresh(DefaultLifecycleProcessor.java:123) at app//org.springframework.context.support.AbstractApplicationContext.finishRefresh(AbstractApplicationContext.java:937) at app//org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:586) at app//org.springframework.boot.SpringApplication.refresh(SpringApplication.java:745) at app//org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:423) at app//org.springframework.boot.SpringApplication.run(SpringApplication.java:307) at app//org.springframework.boot.test.context.SpringBootContextLoader.loadContext(SpringBootContextLoader.java:148) at app//org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:141) at app//org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:90) ... 61 more Caused by: java.lang.NullPointerException: Cannot invoke "org.springframework.web.servlet.mvc.condition.PatternsRequestCondition.toString()" because the return value of "springfox.documentation.spi.service.contexts.Orderings.patternsCondition(springfox.documentation.RequestHandler)" is null at springfox.documentation.spi.service.contexts.Orderings$8.compare(Orderings.java:112) at springfox.documentation.spi.service.contexts.Orderings$8.compare(Orderings.java:109) at com.google.common.collect.ComparatorOrdering.compare(ComparatorOrdering.java:37) at java.base/java.util.TimSort.countRunAndMakeAscending(TimSort.java:355) at java.base/java.util.TimSort.sort(TimSort.java:234) at java.base/java.util.Arrays.sort(Arrays.java:1233) at com.google.common.collect.Ordering.sortedCopy(Ordering.java:842) at springfox.documentation.spring.web.plugins.WebMvcRequestHandlerProvider.requestHandlers(WebMvcRequestHandlerProvider.java:57) at springfox.documentation.spring.web.plugins.DocumentationPluginsBootstrapper$2.apply(DocumentationPluginsBootstrapper.java:138) at springfox.documentation.spring.web.plugins.DocumentationPluginsBootstrapper$2.apply(DocumentationPluginsBootstrapper.java:135) at com.google.common.collect.Iterators$6.transform(Iterators.java:783) at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:47) at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:47) at com.google.common.collect.Iterators$ConcatenatedIterator.hasNext(Iterators.java:1333) at com.google.common.collect.ImmutableList.copyOf(ImmutableList.java:268) at com.google.common.collect.ImmutableList.copyOf(ImmutableList.java:232) at com.google.common.collect.FluentIterable.toList(FluentIterable.java:617) at springfox.documentation.spring.web.plugins.DocumentationPluginsBootstrapper.defaultContextBuilder(DocumentationPluginsBootstrapper.java:111) at springfox.documentation.spring.web.plugins.DocumentationPluginsBootstrapper.buildContext(DocumentationPluginsBootstrapper.java:96) at springfox.documentation.spring.web.plugins.DocumentationPluginsBootstrapper.start(DocumentationPluginsBootstrapper.java:167) at org.springframework.context.support.DefaultLifecycleProcessor.doStart(DefaultLifecycleProcessor.java:178) ... 74 more ``` In order to fix the issue, adding springfox bean post processor to handle the request of springfox `WebMvcRequestHandlerProvider` class. * fix(tomcat): refactor to map spinnaker property default.rejectIllegalHeader with spring property server.tomcat.reject-illegal-header While upgrading spring boot 2.6.15 and spring cloud 2021.0.8, encounter below error during execution of WebEnvironmentTest.testTomcatWithIllegalHttpHeaders test under kork-tomcat module: ``` DEBUG 164202 --- [o-auto-2-exec-1] o.a.coyote.http11.Http11InputBuffer : Before fill(): parsingHeader: [true], parsingRequestLine: [true], parsingRequestLinePhase: [0], parsingRequestLineStart: [0], byteBuffer.position(): [0], byteBuffer.limit(): [0], end: [0] DEBUG 164202 --- [o-auto-2-exec-1] o.a.coyote.http11.Http11InputBuffer : Received [GET /test-controller HTTP/1.1 Accept: text/plain, application/json, application/*+json, */* X-Dum@my: foo User-Agent: Java/11.0.2 Host: localhost:35189 Connection: keep-alive ] INFO 164202 --- [o-auto-2-exec-1] o.apache.coyote.http11.Http11Processor : Error parsing HTTP request header Note: further occurrences of HTTP request parsing errors will be logged at DEBUG level. java.lang.IllegalArgumentException: The HTTP header line [x-dum@my: foo] does not conform to RFC 7230. The request has been rejected. at org.apache.coyote.http11.Http11InputBuffer.skipLine(Http11InputBuffer.java:1074) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.coyote.http11.Http11InputBuffer.parseHeader(Http11InputBuffer.java:905) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.coyote.http11.Http11InputBuffer.parseHeaders(Http11InputBuffer.java:591) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:286) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:926) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1791) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) ~[tomcat-embed-core-9.0.75.jar:9.0.75] at java.base/java.lang.Thread.run(Thread.java:834) ~[na:na] expected: <200 OK> but was: <400 BAD_REQUEST> Expected :200 OK Actual :400 BAD_REQUEST org.opentest4j.AssertionFailedError: expected: <200 OK> but was: <400 BAD_REQUEST> at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177) at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141) at app//com.netflix.spinnaker.kork.tomcat.WebEnvironmentTest.testTomcatWithIllegalHttpHeaders(WebEnvironmentTest.java:59) ``` The root cause of the issue is introduction of new property "server.tomcat.reject-illegal-header" in spring boot [2.6.x](https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.6-Release-Notes#miscellaneous), which overrides the spinnaker property "default.rejectIllegalHeader". Since "server.tomcat.reject-illegal-header" property is true by default, the test get failed. In order to honor the spinnaker property "default.rejectIllegalHeader", refactored the code to map both the properties. --- .../plugins/SpringPluginStatusProviderTest.kt | 1 + kork-swagger/kork-swagger.gradle | 1 + ...ngfoxHandlerProviderBeanPostProcessor.java | 77 +++++++++++++++++++ .../main/resources/META-INF/spring.factories | 2 + .../DefaultTomcatConnectorCustomizer.java | 4 - .../kork/tomcat/TomcatConfiguration.java | 5 ++ .../tomcat/TomcatConfigurationProperties.java | 2 + .../spinnaker-dependencies.gradle | 10 +-- 8 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 kork-swagger/src/main/java/com/netflix/spinnaker/config/SpringfoxHandlerProviderBeanPostProcessor.java create mode 100644 kork-swagger/src/main/resources/META-INF/spring.factories diff --git a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt index d792f5789..062c978fc 100644 --- a/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt +++ b/kork-plugins/src/test/kotlin/com/netflix/spinnaker/kork/plugins/SpringPluginStatusProviderTest.kt @@ -46,6 +46,7 @@ class SpringPluginStatusProviderTest : JUnit5Minutests { subject.onApplicationEvent( ApplicationEnvironmentPreparedEvent( + mockk(relaxed = true), mockk(relaxed = true), arrayOf(), environment diff --git a/kork-swagger/kork-swagger.gradle b/kork-swagger/kork-swagger.gradle index 5d57e7d5c..8d5118bed 100644 --- a/kork-swagger/kork-swagger.gradle +++ b/kork-swagger/kork-swagger.gradle @@ -8,5 +8,6 @@ dependencies { implementation "com.google.guava:guava" implementation "org.springframework.boot:spring-boot-autoconfigure" implementation "io.springfox:springfox-boot-starter" + implementation "org.springframework:spring-webmvc" } diff --git a/kork-swagger/src/main/java/com/netflix/spinnaker/config/SpringfoxHandlerProviderBeanPostProcessor.java b/kork-swagger/src/main/java/com/netflix/spinnaker/config/SpringfoxHandlerProviderBeanPostProcessor.java new file mode 100644 index 000000000..de624674e --- /dev/null +++ b/kork-swagger/src/main/java/com/netflix/spinnaker/config/SpringfoxHandlerProviderBeanPostProcessor.java @@ -0,0 +1,77 @@ +/* + * Copyright 2024 OpsMx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.config; + +import java.lang.reflect.Field; +import java.util.List; +import java.util.stream.Collectors; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.stereotype.Component; +import org.springframework.util.ReflectionUtils; +import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; +import springfox.documentation.spring.web.plugins.WebMvcRequestHandlerProvider; + +/** + * With spring boot 2.6.x, default strategy for matching request paths against registered Spring MVC + * handler mappings has changed from AntPathMatcher to PathPatternParser. The actuator endpoints + * also use PathPattern based URL matching and path matching strategy cannot be configured for + * actuator endpoints via a configuration property. Using Actuator and Springfox may result in + * application failing to start. {@see https://github.com/springfox/springfox/issues/3462} + */ +@Component +public class SpringfoxHandlerProviderBeanPostProcessor implements BeanPostProcessor { + + /** + * Overrides BeanPostProcessor method postProcessAfterInitialization() to filter the + * springfox.documentation.spring.web.plugins.WebMvcRequestHandlerProvider beans for request + * mapping. + * + * @param bean – the new bean instance + * @param beanName – the name of the bean + * @return the bean instance to use, either the original or a wrapped one; if null, no subsequent + * BeanPostProcessors will be invoked + */ + @Override + public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { + if (bean instanceof WebMvcRequestHandlerProvider) { + customizeSpringfoxHandlerMappings(getHandlerMappings(bean)); + } + return bean; + } + + private void customizeSpringfoxHandlerMappings( + List mappings) { + List copy = + mappings.stream() + .filter(mapping -> mapping.getPatternParser() == null) + .collect(Collectors.toList()); + mappings.clear(); + mappings.addAll(copy); + } + + @SuppressWarnings("unchecked") + private List getHandlerMappings(Object bean) { + try { + Field field = ReflectionUtils.findField(bean.getClass(), "handlerMappings"); + field.setAccessible(true); + return (List) field.get(bean); + } catch (IllegalArgumentException | IllegalAccessException e) { + throw new IllegalStateException(e); + } + } +} diff --git a/kork-swagger/src/main/resources/META-INF/spring.factories b/kork-swagger/src/main/resources/META-INF/spring.factories new file mode 100644 index 000000000..29d91a604 --- /dev/null +++ b/kork-swagger/src/main/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ + com.netflix.spinnaker.config.SpringfoxHandlerProviderBeanPostProcessor diff --git a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/DefaultTomcatConnectorCustomizer.java b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/DefaultTomcatConnectorCustomizer.java index 1d19b8bf6..4158a5d11 100644 --- a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/DefaultTomcatConnectorCustomizer.java +++ b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/DefaultTomcatConnectorCustomizer.java @@ -52,10 +52,6 @@ class DefaultTomcatConnectorCustomizer implements TomcatConnectorCustomizer { public void customize(Connector connector) { this.applySSLSettings(connector); this.applyRelaxedURIProperties(connector); - if (tomcatConfigurationProperties.getRejectIllegalHeader() != null) { - ((AbstractHttp11Protocol) connector.getProtocolHandler()) - .setRejectIllegalHeader(tomcatConfigurationProperties.getRejectIllegalHeader()); - } } Ssl copySslConfigurationWithClientAuth(TomcatServletWebServerFactory tomcat) { diff --git a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfiguration.java b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfiguration.java index e17b560f7..3ced63259 100644 --- a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfiguration.java +++ b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfiguration.java @@ -46,6 +46,11 @@ class TomcatConfiguration { TomcatConnectorCustomizer defaultTomcatConnectorCustomizer( TomcatConfigurationProperties tomcatConfigurationProperties, SslExtensionConfigurationProperties sslExtensionConfigurationProperties) { + if (tomcatConfigurationProperties.getRejectIllegalHeader() != null) { + System.setProperty( + "server.tomcat.reject-illegal-header", + tomcatConfigurationProperties.getRejectIllegalHeader().toString()); + } return new DefaultTomcatConnectorCustomizer( tomcatConfigurationProperties, sslExtensionConfigurationProperties); } diff --git a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java index 1f890711a..2cf4ef499 100644 --- a/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java +++ b/kork-tomcat/src/main/java/com/netflix/spinnaker/kork/tomcat/TomcatConfigurationProperties.java @@ -36,6 +36,8 @@ public class TomcatConfigurationProperties { private List cipherSuites = CipherSuites.getRecommendedCiphers(); + // This property maps to spring boot property server.tomcat.reject-illegal-header, + // which is true by default. private Boolean rejectIllegalHeader; public int getLegacyServerPort() { diff --git a/spinnaker-dependencies/spinnaker-dependencies.gradle b/spinnaker-dependencies/spinnaker-dependencies.gradle index 6576440ae..8955d2d8e 100644 --- a/spinnaker-dependencies/spinnaker-dependencies.gradle +++ b/spinnaker-dependencies/spinnaker-dependencies.gradle @@ -14,7 +14,7 @@ ext { gcp : "25.3.0", jsch : "0.1.54", jschAgentProxy : "0.0.9", - // spring boot 2.5.15 specifies logback 1.2.12. Pin to 1.2.13 to resolve + // spring boot 2.6.15 specifies logback 1.2.12. Pin to 1.2.13 to resolve // CVE-2023-6378 and CVE-2023-6481 until spring boot 3.1.7 which brings in // 1.4.14. See https://logback.qos.ch/news.html#1.3.12. logback : "1.2.13", @@ -28,12 +28,12 @@ ext { spectator : "1.0.6", spek : "1.1.5", spek2 : "2.0.9", - springBoot : "2.5.15", - springCloud : "2020.0.6", + springBoot : "2.6.15", + springCloud : "2021.0.8", springfoxSwagger : "3.0.0", swagger : "1.5.20", //this should stay in sync with what springfoxSwagger expects. - // Spring boot 2.5.15 and 2.6.15 bring in 9.0.75. 2.7.18 + // Spring boot 2.6.15 bring in 9.0.75. 2.7.18 // brings in 9.0.83, which fixes all CVEs to date (20-feb-24). // // See https://tomcat.apache.org/security-9.html for latest security fixes. @@ -181,7 +181,7 @@ dependencies { // 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. + // Hence pinning slf4j-api at 1.7.36 which spring boot 2.6.15 brings in. api("org.slf4j:slf4j-api"){ version { strictly("1.7.36")