refactor(tests): replace AntPathMatcher as path matching strategy instead of default PathPatternParser and refactor spring security from 5.x to 6.x during upgrade of spring boot 3.0.x

j-sandy · j-sandy · commit fe356624281e · 2025-01-10T12:11:05.000+05:30
As of Spring Boot 2.6, the PathPatternParser is used by default. However, for pattern like /abc/**/xyz, PathPattenParser does not resolve the path. https://spring.io/blog/2022/05/24/preparing-for-spring-boot-3-0#use-spring-mvcs-pathpatternparser spring-projects/spring-framework#24952 Due to this change, encountered below errors in igor-web module: ``` BuildControllerSpec > get the status of a build FAILED java.lang.IllegalStateException: Invalid mapping on handler class [com.netflix.spinnaker.igor.build.BuildController]: public void com.netflix.spinnaker.igor.build.BuildController.update(java.lang.String,java.lang.Integer,com.netflix.spinnaker.igor.build.model.UpdatedBuild,jakarta.servlet.http.HttpServletRequest) at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.lambda$detectHandlerMethods$1(AbstractHandlerMethodMapping.java:287) at org.springframework.core.MethodIntrospector.lambda$selectMethods$0(MethodIntrospector.java:74) at org.springframework.util.ReflectionUtils.doWithMethods(ReflectionUtils.java:366) at org.springframework.core.MethodIntrospector.selectMethods(MethodIntrospector.java:72) at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.detectHandlerMethods(AbstractHandlerMethodMapping.java:280) at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.processCandidateBean(AbstractHandlerMethodMapping.java:265) at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.initHandlerMethods(AbstractHandlerMethodMapping.java:224) at org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.afterPropertiesSet(AbstractHandlerMethodMapping.java:212) at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.afterPropertiesSet(RequestMappingHandlerMapping.java:225) at org.springframework.test.web.servlet.setup.StandaloneMockMvcBuilder.registerMvcSingletons(StandaloneMockMvcBuilder.java:419) at org.springframework.test.web.servlet.setup.StandaloneMockMvcBuilder.initWebAppContext(StandaloneMockMvcBuilder.java:391) at org.springframework.test.web.servlet.setup.AbstractMockMvcBuilder.build(AbstractMockMvcBuilder.java:157) at com.netflix.spinnaker.igor.build.BuildControllerSpec.setup(BuildControllerSpec.groovy:116) Caused by: org.springframework.web.util.pattern.PatternParseException: No more pattern data allowed after {*...} or ** pattern element at app//org.springframework.web.util.pattern.InternalPathPatternParser.peekDoubleWildcard(InternalPathPatternParser.java:250) at app//org.springframework.web.util.pattern.InternalPathPatternParser.parse(InternalPathPatternParser.java:113) at app//org.springframework.web.util.pattern.PathPatternParser.parse(PathPatternParser.java:129) at app//org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition.parse(PathPatternsRequestCondition.java:84) at app//org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition.<init>(PathPatternsRequestCondition.java:74) at app//org.springframework.web.servlet.mvc.method.RequestMappingInfo$DefaultBuilder.build(RequestMappingInfo.java:714) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.createRequestMappingInfo(RequestMappingHandlerMapping.java:400) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.createRequestMappingInfo(RequestMappingHandlerMapping.java:345) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getMappingForMethod(RequestMappingHandlerMapping.java:302) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getMappingForMethod(RequestMappingHandlerMapping.java:76) at app//org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.lambda$detectHandlerMethods$1(AbstractHandlerMethodMapping.java:283) ... 12 more ``` ``` GoogleCloudBuildTest > missingAccountTest() FAILED java.lang.IllegalStateException: Failed to load ApplicationContext at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:143) at org.springframework.test.context.support.DefaultTestContext.getApplicationContext(DefaultTestContext.java:127) at org.springframework.test.context.web.ServletTestExecutionListener.setUpRequestContextIfNecessary(ServletTestExecutionListener.java:191) at org.springframework.test.context.web.ServletTestExecutionListener.prepareTestInstance(ServletTestExecutionListener.java:130) at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:241) at org.springframework.test.context.junit.jupiter.SpringExtension.postProcessTestInstance(SpringExtension.java:138) Caused by: org.springframework.web.util.pattern.PatternParseException: No more pattern data allowed after {*...} or ** pattern element at app//org.springframework.web.util.pattern.InternalPathPatternParser.peekDoubleWildcard(InternalPathPatternParser.java:250) at app//org.springframework.web.util.pattern.InternalPathPatternParser.parse(InternalPathPatternParser.java:113) at app//org.springframework.web.util.pattern.PathPatternParser.parse(PathPatternParser.java:129) at app//org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition.parse(PathPatternsRequestCondition.java:84) at app//org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition.<init>(PathPatternsRequestCondition.java:74) at app//org.springframework.web.servlet.mvc.method.RequestMappingInfo$DefaultBuilder.build(RequestMappingInfo.java:714) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.createRequestMappingInfo(RequestMappingHandlerMapping.java:400) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.createRequestMappingInfo(RequestMappingHandlerMapping.java:345) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getMappingForMethod(RequestMappingHandlerMapping.java:302) at app//org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getMappingForMethod(RequestMappingHandlerMapping.java:76) at app//org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.lambda$detectHandlerMethods$1(AbstractHandlerMethodMapping.java:283) ... 118 more ``` So, refactoring the tests to replace AntPathMatcher instead of PathPatternParser by adding the property `spring.mvc.pathmatch.matching-strategy = ANT_PATH_MATCHER` Ref: spinnaker#1211 ======================================================== With spring boot upgrade, spring security also upgrades from 5.x to 6.x. As per the migration [steps](https://www.baeldung.com/spring-security-migrate-5-to-6), `WebSecurityConfigurerAdapter` has been removed. So, it is not required to be extended, instead bean can be registered.
diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/build/BuildControllerSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/build/BuildControllerSpec.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.build
 
+import com.netflix.spinnaker.config.PluginsAutoConfiguration
 import com.netflix.spinnaker.igor.PendingOperationsCache
 import com.netflix.spinnaker.igor.build.model.GenericBuild
 import com.netflix.spinnaker.igor.build.model.UpdatedBuild
@@ -33,15 +34,18 @@ import com.netflix.spinnaker.igor.travis.service.TravisService
 import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator
 import com.netflix.spinnaker.kork.web.exceptions.GenericExceptionHandlers
 import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
-import okhttp3.mockwebserver.MockWebServer
+import org.spockframework.spring.SpringBean
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
+import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
 import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
 import org.springframework.mock.web.MockHttpServletResponse
+import org.springframework.test.context.ContextConfiguration
+import org.springframework.test.context.TestPropertySource
 import org.springframework.test.web.servlet.MockMvc
-import org.springframework.test.web.servlet.setup.MockMvcBuilders
 import retrofit.client.Header
 import retrofit.client.Response
-import spock.lang.Shared
 import spock.lang.Specification
 
 import static com.netflix.spinnaker.igor.build.BuildController.InvalidJobParameterException
@@ -55,19 +59,24 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
  * Tests for BuildController
  */
 @SuppressWarnings(['DuplicateNumberLiteral', 'PropertyName'])
+@AutoConfigureMockMvc(addFilters = false)
+@WebMvcTest(controllers = [BuildController])
+@ContextConfiguration(classes = [PluginsAutoConfiguration, BuildController, GenericExceptionHandlers])
+@TestPropertySource(
+  properties = [
+    "spring.application.name = igor",
+    "spring.mvc.pathmatch.matching-strategy = ANT_PATH_MATCHER"
+  ])
 class BuildControllerSpec extends Specification {
 
+  @Autowired
   MockMvc mockMvc
-  BuildServices buildServices
-  BuildCache cache
-  JenkinsService jenkinsService
-  BuildOperations service
-  TravisService travisService
-  PendingOperationsCache pendingOperationService
-  ExceptionMessageDecorator exceptionMessageDecorator
-
-  @Shared
-  MockWebServer server
+  @SpringBean BuildServices buildServices = Mock()
+  @SpringBean JenkinsService jenkinsService = Mock()
+  @SpringBean BuildOperations service = Mock()
+  @SpringBean TravisService travisService = Mock()
+  @SpringBean PendingOperationsCache pendingOperationService = Mock()
+  @SpringBean ExceptionMessageDecorator exceptionMessageDecorator = Mock()
 
   def SERVICE = 'SERVICE'
   def JENKINS_SERVICE = 'JENKINS_SERVICE'
@@ -83,38 +92,16 @@ class BuildControllerSpec extends Specification {
 
   GenericBuild genericBuild
 
-  void cleanup() {
-    server.shutdown()
-  }
-
   void setup() {
-    exceptionMessageDecorator = Mock(ExceptionMessageDecorator)
-    service = Mock(BuildOperations)
-    jenkinsService = Mock(JenkinsService)
     jenkinsService.getBuildServiceProvider() >> BuildServiceProvider.JENKINS
-    travisService = Mock(TravisService)
     travisService.getBuildServiceProvider() >> BuildServiceProvider.TRAVIS
-    buildServices = new BuildServices()
-    buildServices.addServices([
-      (SERVICE)        : service,
-      (JENKINS_SERVICE): jenkinsService,
-      (TRAVIS_SERVICE) : travisService,
-    ])
+    buildServices.getService(SERVICE) >> service
+    buildServices.getService(JENKINS_SERVICE) >> jenkinsService
+    buildServices.getService(TRAVIS_SERVICE) >> travisService
+
     genericBuild = new GenericBuild()
     genericBuild.number = BUILD_NUMBER
     genericBuild.id = BUILD_ID
-
-    cache = Mock(BuildCache)
-    server = new MockWebServer()
-    pendingOperationService = Mock(PendingOperationsCache)
-    pendingOperationService.getAndSetOperationStatus(_, _, _) >> {
-      return new PendingOperationsCache.OperationState()
-    }
-
-    mockMvc = MockMvcBuilders
-      .standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
-      .setControllerAdvice(new GenericExceptionHandlers(exceptionMessageDecorator))
-      .build()
   }
 
   void 'get the status of a build'() {
@@ -285,6 +272,7 @@ class BuildControllerSpec extends Specification {
 
   void 'trigger a build with parameters to a job with parameters'() {
     given:
+    pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
     1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true, parameterDefinitionList: [new ParameterDefinition(defaultParameterValue: [name: "name", value: null], description: "description")])
     1 * jenkinsService.buildWithParameters(JOB_NAME, [name: "myName"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location", "foo/${BUILD_NUMBER}")], null)
 
@@ -298,6 +286,7 @@ class BuildControllerSpec extends Specification {
 
   void 'trigger a build without parameters to a job with parameters with default values'() {
     given:
+    pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
     1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true, parameterDefinitionList: [new ParameterDefinition(defaultParameterValue: [name: "name", value: "value"], description: "description")])
     1 * jenkinsService.buildWithParameters(JOB_NAME, ['startedBy': "igor"]) >> new Response("http://test.com", HTTP_201, "", [new Header("Location", "foo/${BUILD_NUMBER}")], null)
 
@@ -311,6 +300,7 @@ class BuildControllerSpec extends Specification {
 
   void 'trigger a build with parameters to a job without parameters'() {
     given:
+    pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
     1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true)
 
     when:
@@ -323,6 +313,7 @@ class BuildControllerSpec extends Specification {
 
   void 'trigger a build with an invalid choice'() {
     given:
+    pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
     JobConfig config = new JobConfig(buildable: true)
     config.parameterDefinitionList = [
       new ParameterDefinition(type: "ChoiceParameterDefinition", name: "foo", choices: ["bar", "baz"])
@@ -342,6 +333,7 @@ class BuildControllerSpec extends Specification {
 
   void 'trigger a disabled build'() {
     given:
+    pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
     JobConfig config = new JobConfig()
     1 * jenkinsService.getJobConfig(JOB_NAME) >> config
     1 * exceptionMessageDecorator.decorate(_, _) >> "Job '${JOB_NAME}' is not buildable. It may be disabled."
@@ -392,16 +384,10 @@ class BuildControllerSpec extends Specification {
 
   void "doesn't trigger a build when previous request is still in progress"() {
     given:
-    pendingOperationService = Stub(PendingOperationsCache)
     pendingOperationService.getAndSetOperationStatus("${JENKINS_SERVICE}:${PENDING_JOB_NAME}:NO_EXECUTION_ID:foo=bat", _, _) >> {
       return new PendingOperationsCache.OperationState(PendingOperationsCache.OperationStatus.PENDING)
     }
 
-    mockMvc = MockMvcBuilders
-      .standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
-      .setControllerAdvice(new GenericExceptionHandlers())
-      .build()
-
     when:
     MockHttpServletResponse response = mockMvc.perform(put("/masters/${JENKINS_SERVICE}/jobs/${PENDING_JOB_NAME}")
       .contentType(MediaType.APPLICATION_JSON).param("foo", "bat")).andReturn().response
@@ -412,18 +398,12 @@ class BuildControllerSpec extends Specification {
 
   void "resets the cache once the build status has been retrieved"() {
     given:
-    pendingOperationService = Mock(PendingOperationsCache)
     pendingOperationService.getAndSetOperationStatus("${JENKINS_SERVICE}:${JOB_NAME}:NO_EXECUTION_ID:foo=bat", _, _) >> {
       PendingOperationsCache.OperationState state = new PendingOperationsCache.OperationState()
       state.load(PendingOperationsCache.OperationStatus.COMPLETED.toString() + ":" + BUILD_NUMBER)
       return state
     }
 
-    mockMvc = MockMvcBuilders
-      .standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
-      .setControllerAdvice(new GenericExceptionHandlers())
-      .build()
-
     when:
     MockHttpServletResponse response = mockMvc.perform(put("/masters/${JENKINS_SERVICE}/jobs/${JOB_NAME}")
       .contentType(MediaType.APPLICATION_JSON).param("foo", "bat")).andReturn().response
@@ -448,7 +428,6 @@ class BuildControllerSpec extends Specification {
 
     then:
     1 * jenkinsService.updateBuild(jobName, BUILD_NUMBER, new UpdatedBuild("this is my new description"))
-    0 * _
     response.status == 200
 
     where:
diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildTest.java b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildTest.java
@@ -53,20 +53,19 @@
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.core.annotation.Order;
 import org.springframework.http.MediaType;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
-import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.test.web.servlet.MockMvc;
-import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 
 @ExtendWith(SpringExtension.class)
 @AutoConfigureMockMvc
-@EnableWebMvc
 @ComponentScan({"com.netflix.spinnaker.config", "com.netflix.spinnaker.igor"})
 @SpringBootTest(
     classes = {
@@ -79,7 +78,8 @@
 @TestPropertySource(
     properties = {
       "spring.config.location=classpath:gcb/gcb-test.yml",
-      "spring.application.name = igor"
+      "spring.application.name = igor",
+      "spring.mvc.pathmatch.matching-strategy = ANT_PATH_MATCHER"
     })
 public class GoogleCloudBuildTest {
   @Autowired private MockMvc mockMvc;
@@ -93,10 +93,11 @@ public class GoogleCloudBuildTest {
   @TestConfiguration
   @EnableWebSecurity
   @Order(1)
-  static class WebSecurityConfig extends WebSecurityConfigurerAdapter {
-    @Override
-    protected void configure(HttpSecurity httpSecurity) throws Exception {
-      httpSecurity.authorizeRequests().anyRequest().permitAll().and().csrf().disable();
+  static class WebSecurityConfig {
+    @Bean
+    public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
+      http.authorizeHttpRequests().anyRequest().permitAll().and().csrf().disable();
+      return http.build();
     }
   }
 
