Skip to content

Commit 7326dec

Browse files
j-sandyrahul-chekuri
j-sandy
authored and
rahul-chekuri
committed
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
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.
1 parent 776de76 commit 7326dec

File tree

2 files changed

+42
-57
lines changed

2 files changed

+42
-57
lines changed

‎igor-web/src/test/groovy/com/netflix/spinnaker/igor/build/BuildControllerSpec.groovy

+33-49
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.netflix.spinnaker.igor.build
1818

19+
import com.netflix.spinnaker.config.PluginsAutoConfiguration
1920
import com.netflix.spinnaker.igor.PendingOperationsCache
2021
import com.netflix.spinnaker.igor.build.model.GenericBuild
2122
import com.netflix.spinnaker.igor.build.model.UpdatedBuild
@@ -33,16 +34,24 @@ import com.netflix.spinnaker.igor.travis.service.TravisService
3334
import com.netflix.spinnaker.kork.web.exceptions.ExceptionMessageDecorator
3435
import com.netflix.spinnaker.kork.web.exceptions.GenericExceptionHandlers
3536
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
37+
import org.spockframework.spring.SpringBean
38+
import org.springframework.beans.factory.annotation.Autowired
39+
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
40+
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
3641
import okhttp3.Protocol
3742
import okhttp3.Request
3843
import okhttp3.Response
3944
import okhttp3.mockwebserver.MockWebServer
4045
import org.springframework.http.HttpStatus
4146
import org.springframework.http.MediaType
4247
import org.springframework.mock.web.MockHttpServletResponse
48+
import org.springframework.test.context.ContextConfiguration
49+
import org.springframework.test.context.TestPropertySource
4350
import org.springframework.test.web.servlet.MockMvc
4451
import org.springframework.test.web.servlet.setup.MockMvcBuilders
4552
import spock.lang.Shared
53+
import retrofit.client.Header
54+
import retrofit.client.Response
4655
import spock.lang.Specification
4756

4857
import static com.netflix.spinnaker.igor.build.BuildController.InvalidJobParameterException
@@ -56,19 +65,24 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
5665
* Tests for BuildController
5766
*/
5867
@SuppressWarnings(['DuplicateNumberLiteral', 'PropertyName'])
68+
@AutoConfigureMockMvc(addFilters = false)
69+
@WebMvcTest(controllers = [BuildController])
70+
@ContextConfiguration(classes = [PluginsAutoConfiguration, BuildController, GenericExceptionHandlers])
71+
@TestPropertySource(
72+
properties = [
73+
"spring.application.name = igor",
74+
"spring.mvc.pathmatch.matching-strategy = ANT_PATH_MATCHER"
75+
])
5976
class BuildControllerSpec extends Specification {
6077

78+
@Autowired
6179
MockMvc mockMvc
62-
BuildServices buildServices
63-
BuildCache cache
64-
JenkinsService jenkinsService
65-
BuildOperations service
66-
TravisService travisService
67-
PendingOperationsCache pendingOperationService
68-
ExceptionMessageDecorator exceptionMessageDecorator
69-
70-
@Shared
71-
MockWebServer server
80+
@SpringBean BuildServices buildServices = Mock()
81+
@SpringBean JenkinsService jenkinsService = Mock()
82+
@SpringBean BuildOperations service = Mock()
83+
@SpringBean TravisService travisService = Mock()
84+
@SpringBean PendingOperationsCache pendingOperationService = Mock()
85+
@SpringBean ExceptionMessageDecorator exceptionMessageDecorator = Mock()
7286

7387
def SERVICE = 'SERVICE'
7488
def JENKINS_SERVICE = 'JENKINS_SERVICE'
@@ -84,38 +98,16 @@ class BuildControllerSpec extends Specification {
8498

8599
GenericBuild genericBuild
86100

87-
void cleanup() {
88-
server.shutdown()
89-
}
90-
91101
void setup() {
92-
exceptionMessageDecorator = Mock(ExceptionMessageDecorator)
93-
service = Mock(BuildOperations)
94-
jenkinsService = Mock(JenkinsService)
95102
jenkinsService.getBuildServiceProvider() >> BuildServiceProvider.JENKINS
96-
travisService = Mock(TravisService)
97103
travisService.getBuildServiceProvider() >> BuildServiceProvider.TRAVIS
98-
buildServices = new BuildServices()
99-
buildServices.addServices([
100-
(SERVICE) : service,
101-
(JENKINS_SERVICE): jenkinsService,
102-
(TRAVIS_SERVICE) : travisService,
103-
])
104+
buildServices.getService(SERVICE) >> service
105+
buildServices.getService(JENKINS_SERVICE) >> jenkinsService
106+
buildServices.getService(TRAVIS_SERVICE) >> travisService
107+
104108
genericBuild = new GenericBuild()
105109
genericBuild.number = BUILD_NUMBER
106110
genericBuild.id = BUILD_ID
107-
108-
cache = Mock(BuildCache)
109-
server = new MockWebServer()
110-
pendingOperationService = Mock(PendingOperationsCache)
111-
pendingOperationService.getAndSetOperationStatus(_, _, _) >> {
112-
return new PendingOperationsCache.OperationState()
113-
}
114-
115-
mockMvc = MockMvcBuilders
116-
.standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
117-
.setControllerAdvice(new GenericExceptionHandlers(exceptionMessageDecorator))
118-
.build()
119111
}
120112

121113
void 'get the status of a build'() {
@@ -294,6 +286,7 @@ class BuildControllerSpec extends Specification {
294286

295287
void 'trigger a build with parameters to a job with parameters'() {
296288
given:
289+
pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
297290
1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true, parameterDefinitionList: [new ParameterDefinition(defaultParameterValue: [name: "name", value: null], description: "description")])
298291
def request = new Request.Builder().url("http://test.com").build()
299292
def rawResponse = new Response.Builder()
@@ -315,6 +308,7 @@ class BuildControllerSpec extends Specification {
315308

316309
void 'trigger a build without parameters to a job with parameters with default values'() {
317310
given:
311+
pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
318312
1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true, parameterDefinitionList: [new ParameterDefinition(defaultParameterValue: [name: "name", value: "value"], description: "description")])
319313
def request = new Request.Builder().url("http://test.com").build()
320314
def rawResponse = new Response.Builder()
@@ -336,6 +330,7 @@ class BuildControllerSpec extends Specification {
336330

337331
void 'trigger a build with parameters to a job without parameters'() {
338332
given:
333+
pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
339334
1 * jenkinsService.getJobConfig(JOB_NAME) >> new JobConfig(buildable: true)
340335

341336
when:
@@ -348,6 +343,7 @@ class BuildControllerSpec extends Specification {
348343

349344
void 'trigger a build with an invalid choice'() {
350345
given:
346+
pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
351347
JobConfig config = new JobConfig(buildable: true)
352348
config.parameterDefinitionList = [
353349
new ParameterDefinition(type: "ChoiceParameterDefinition", name: "foo", choices: ["bar", "baz"])
@@ -367,6 +363,7 @@ class BuildControllerSpec extends Specification {
367363

368364
void 'trigger a disabled build'() {
369365
given:
366+
pendingOperationService.getAndSetOperationStatus(_, _, _) >> new PendingOperationsCache.OperationState()
370367
JobConfig config = new JobConfig()
371368
1 * jenkinsService.getJobConfig(JOB_NAME) >> config
372369
1 * exceptionMessageDecorator.decorate(_, _) >> "Job '${JOB_NAME}' is not buildable. It may be disabled."
@@ -417,16 +414,10 @@ class BuildControllerSpec extends Specification {
417414

418415
void "doesn't trigger a build when previous request is still in progress"() {
419416
given:
420-
pendingOperationService = Stub(PendingOperationsCache)
421417
pendingOperationService.getAndSetOperationStatus("${JENKINS_SERVICE}:${PENDING_JOB_NAME}:NO_EXECUTION_ID:foo=bat", _, _) >> {
422418
return new PendingOperationsCache.OperationState(PendingOperationsCache.OperationStatus.PENDING)
423419
}
424420

425-
mockMvc = MockMvcBuilders
426-
.standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
427-
.setControllerAdvice(new GenericExceptionHandlers())
428-
.build()
429-
430421
when:
431422
MockHttpServletResponse response = mockMvc.perform(put("/masters/${JENKINS_SERVICE}/jobs/${PENDING_JOB_NAME}")
432423
.contentType(MediaType.APPLICATION_JSON).param("foo", "bat")).andReturn().response
@@ -437,18 +428,12 @@ class BuildControllerSpec extends Specification {
437428

438429
void "resets the cache once the build status has been retrieved"() {
439430
given:
440-
pendingOperationService = Mock(PendingOperationsCache)
441431
pendingOperationService.getAndSetOperationStatus("${JENKINS_SERVICE}:${JOB_NAME}:NO_EXECUTION_ID:foo=bat", _, _) >> {
442432
PendingOperationsCache.OperationState state = new PendingOperationsCache.OperationState()
443433
state.load(PendingOperationsCache.OperationStatus.COMPLETED.toString() + ":" + BUILD_NUMBER)
444434
return state
445435
}
446436

447-
mockMvc = MockMvcBuilders
448-
.standaloneSetup(new BuildController(buildServices, pendingOperationService, Optional.empty(), Optional.empty(), Optional.empty()))
449-
.setControllerAdvice(new GenericExceptionHandlers())
450-
.build()
451-
452437
when:
453438
MockHttpServletResponse response = mockMvc.perform(put("/masters/${JENKINS_SERVICE}/jobs/${JOB_NAME}")
454439
.contentType(MediaType.APPLICATION_JSON).param("foo", "bat")).andReturn().response
@@ -473,7 +458,6 @@ class BuildControllerSpec extends Specification {
473458

474459
then:
475460
1 * jenkinsService.updateBuild(jobName, BUILD_NUMBER, new UpdatedBuild("this is my new description"))
476-
0 * _
477461
response.status == 200
478462

479463
where:

‎igor-web/src/test/groovy/com/netflix/spinnaker/igor/gcb/GoogleCloudBuildTest.java

+9-8
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,19 @@
5454
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
5555
import org.springframework.boot.test.context.SpringBootTest;
5656
import org.springframework.boot.test.context.TestConfiguration;
57+
import org.springframework.context.annotation.Bean;
5758
import org.springframework.context.annotation.ComponentScan;
5859
import org.springframework.core.annotation.Order;
5960
import org.springframework.http.MediaType;
6061
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
6162
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
62-
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
63+
import org.springframework.security.web.SecurityFilterChain;
6364
import org.springframework.test.context.TestPropertySource;
6465
import org.springframework.test.context.junit.jupiter.SpringExtension;
6566
import org.springframework.test.web.servlet.MockMvc;
66-
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
6767

6868
@ExtendWith(SpringExtension.class)
6969
@AutoConfigureMockMvc
70-
@EnableWebMvc
7170
@ComponentScan({"com.netflix.spinnaker.config", "com.netflix.spinnaker.igor"})
7271
@SpringBootTest(
7372
classes = {
@@ -80,7 +79,8 @@
8079
@TestPropertySource(
8180
properties = {
8281
"spring.config.location=classpath:gcb/gcb-test.yml",
83-
"spring.application.name = igor"
82+
"spring.application.name = igor",
83+
"spring.mvc.pathmatch.matching-strategy = ANT_PATH_MATCHER"
8484
})
8585
public class GoogleCloudBuildTest {
8686
@Autowired private MockMvc mockMvc;
@@ -94,10 +94,11 @@ public class GoogleCloudBuildTest {
9494
@TestConfiguration
9595
@EnableWebSecurity
9696
@Order(1)
97-
static class WebSecurityConfig extends WebSecurityConfigurerAdapter {
98-
@Override
99-
protected void configure(HttpSecurity httpSecurity) throws Exception {
100-
httpSecurity.authorizeRequests().anyRequest().permitAll().and().csrf().disable();
97+
static class WebSecurityConfig {
98+
@Bean
99+
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
100+
http.authorizeHttpRequests().anyRequest().permitAll().and().csrf().disable();
101+
return http.build();
101102
}
102103
}
103104

0 commit comments

Comments
 (0)