From da853ce88d5133195d35c7f1edb2bfaa52ec5be9 Mon Sep 17 00:00:00 2001 From: AMeng Date: Mon, 30 Nov 2015 15:10:06 -0700 Subject: [PATCH] Move job names to the end of URLs so that they can contain slashes --- .../igor/jenkins/BuildController.groovy | 30 ++++++++++++------- .../igor/jenkins/InfoController.groovy | 1 - .../igor/jenkins/BuildControllerSpec.groovy | 11 +++---- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy index a3598e43a..eaed2c882 100644 --- a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy +++ b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/BuildController.groovy @@ -23,6 +23,7 @@ import com.netflix.spinnaker.igor.jenkins.client.model.JobConfig import com.netflix.spinnaker.igor.jenkins.client.model.QueuedJob import groovy.transform.InheritConstructors import groovy.util.logging.Slf4j +import javax.servlet.http.HttpServletRequest import org.springframework.beans.factory.annotation.Autowired import org.springframework.http.HttpStatus import org.springframework.web.bind.annotation.PathVariable @@ -31,6 +32,7 @@ import org.springframework.web.bind.annotation.RequestMethod import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.ResponseStatus import org.springframework.web.bind.annotation.RestController +import org.springframework.web.servlet.HandlerMapping import org.yaml.snakeyaml.Yaml import java.util.concurrent.ExecutorService @@ -47,8 +49,10 @@ class BuildController { @Autowired ObjectMapper objectMapper - @RequestMapping(value = '/jobs/{master}/{job}/{buildNumber}') - Map getJobStatus(@PathVariable String master, @PathVariable String job, @PathVariable Integer buildNumber) { + @RequestMapping(value = '/builds/status/{buildNumber}/{master}/**') + Map getJobStatus(@PathVariable String master, @PathVariable Integer buildNumber, HttpServletRequest request) { + def job = (String) request.getAttribute( + HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(5).join('/') if (!masters.map.containsKey(master)) { throw new MasterNotFoundException() } @@ -69,7 +73,7 @@ class BuildController { result } - @RequestMapping(value = '/jobs/{master}/queue/{item}') + @RequestMapping(value = '/builds/queue/{master}/{item}') QueuedJob getQueueLocation(@PathVariable String master, @PathVariable int item){ if (!masters.map.containsKey(master)) { throw new MasterNotFoundException() @@ -77,18 +81,22 @@ class BuildController { masters.map[master].getQueuedItem(item) } - @RequestMapping(value = '/jobs/{master}/{job}/builds') - List getBuilds(@PathVariable String master, @PathVariable String job) { + @RequestMapping(value = '/builds/all/{master}/**') + List getBuilds(@PathVariable String master, HttpServletRequest request) { + def job = (String) request.getAttribute( + HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(4).join('/') if (!masters.map.containsKey(master)) { throw new MasterNotFoundException() } masters.map[master].getBuilds(job).list } - @RequestMapping(value = '/masters/{master}/jobs/{job:.+}', method = RequestMethod.PUT) + @RequestMapping(value = '/masters/{name}/jobs/**', method = RequestMethod.PUT) String build( - @PathVariable("master") String master, - @PathVariable String job, @RequestParam Map requestParams) { + @PathVariable("name") String master, + @RequestParam Map requestParams, HttpServletRequest request) { + def job = (String) request.getAttribute( + HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(4).join('/') if (!masters.map.containsKey(master)) { throw new MasterNotFoundException() } @@ -124,10 +132,12 @@ class BuildController { queuedLocation.split('/')[-1] } - @RequestMapping(value = '/jobs/{master}/{job}/{buildNumber}/properties/{fileName:.+}') + @RequestMapping(value = '/builds/properties/{buildNumber}/{fileName}/{master}/**') Map getProperties( @PathVariable String master, - @PathVariable String job, @PathVariable Integer buildNumber, @PathVariable String fileName) { + @PathVariable Integer buildNumber, @PathVariable String fileName, HttpServletRequest request) { + def job = (String) request.getAttribute( + HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).split('/').drop(6).join('/') if (!masters.map.containsKey(master)) { throw new MasterNotFoundException() } diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy index 71e6151f9..3e3ab9218 100644 --- a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy +++ b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/InfoController.groovy @@ -31,7 +31,6 @@ import org.springframework.web.bind.annotation.RequestMethod import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.ResponseStatus import org.springframework.web.bind.annotation.RestController -import org.springframework.web.context.request.WebRequest import org.springframework.web.servlet.HandlerMapping import javax.ws.rs.QueryParam diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy index f8262bad3..5c56795f7 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/BuildControllerSpec.groovy @@ -61,7 +61,7 @@ class BuildControllerSpec extends Specification { final HTTP_201 = 201 final BUILD_NUMBER = 123 final QUEUED_JOB_NUMBER = 123456 - final JOB_NAME = "job1" + final JOB_NAME = "job/name/can/have/slashes" final FILE_NAME = "test.yml" void cleanup() { @@ -82,7 +82,7 @@ class BuildControllerSpec extends Specification { 1 * client.getBuild(JOB_NAME, BUILD_NUMBER) >> new Build(number: BUILD_NUMBER) when: - MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/${BUILD_NUMBER}") + MockHttpServletResponse response = mockMvc.perform(get("/builds/status/${BUILD_NUMBER}/${MASTER}/${JOB_NAME}") .accept(MediaType.APPLICATION_JSON)).andReturn().response then: @@ -94,7 +94,7 @@ class BuildControllerSpec extends Specification { 1 * client.getQueuedItem(QUEUED_JOB_NUMBER) >> new QueuedJob(number: QUEUED_JOB_NUMBER) when: - MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/queue/${QUEUED_JOB_NUMBER}") + MockHttpServletResponse response = mockMvc.perform(get("/builds/queue/${MASTER}/${QUEUED_JOB_NUMBER}") .accept(MediaType.APPLICATION_JSON)).andReturn().response then: @@ -106,7 +106,7 @@ class BuildControllerSpec extends Specification { 1 * client.getBuilds(JOB_NAME) >> new BuildsList(list: [new Build(number: 111), new Build(number: 222)]) when: - MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/builds") + MockHttpServletResponse response = mockMvc.perform(get("/builds/all/${MASTER}/${JOB_NAME}") .accept(MediaType.APPLICATION_JSON)).andReturn().response then: @@ -119,7 +119,8 @@ class BuildControllerSpec extends Specification { number: BUILD_NUMBER, artifacts: [new BuildArtifact(fileName: FILE_NAME, relativePath: FILE_NAME)]) when: - MockHttpServletResponse response = mockMvc.perform(get("/jobs/${MASTER}/${JOB_NAME}/${BUILD_NUMBER}/properties/${FILE_NAME}") + MockHttpServletResponse response = mockMvc.perform( + get("/builds/properties/${BUILD_NUMBER}/${FILE_NAME}/${MASTER}/${JOB_NAME}") .accept(MediaType.APPLICATION_JSON)).andReturn().response then: