From d4b917fc6ddbdab51047ed4f1c3b1ab955163246 Mon Sep 17 00:00:00 2001 From: AMeng Date: Thu, 17 Dec 2015 14:30:37 -0700 Subject: [PATCH 1/2] Update endpoints to have job names at the end. Jobs named with the folders plugin can contain forward slashes. --- gate-web/gate-web.gradle | 2 + .../gate/controllers/BuildController.groovy | 23 ++-- .../gate/services/BuildService.groovy | 11 +- .../gate/services/internal/IgorService.groovy | 17 ++- .../controllers/BuildControllerSpec.groovy | 114 ++++++++++++++++++ 5 files changed, 151 insertions(+), 16 deletions(-) create mode 100644 gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy diff --git a/gate-web/gate-web.gradle b/gate-web/gate-web.gradle index 1ec41780df..1d96beb2b1 100644 --- a/gate-web/gate-web.gradle +++ b/gate-web/gate-web.gradle @@ -23,6 +23,8 @@ dependencies { compile('org.springframework.session:spring-session-data-redis:1.0.1.RELEASE') compile('org.opensaml:opensaml:2.6.4') + testCompile 'com.squareup.okhttp:mockwebserver:2.1.0' + //this brings in the jetty GzipFilter which boot will autoconfigure runtime 'org.eclipse.jetty:jetty-servlets:9.2.11.v20150529' } diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy index 1ceb86cb0b..9a32a83cff 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy @@ -19,16 +19,22 @@ package com.netflix.spinnaker.gate.controllers import com.netflix.spinnaker.gate.services.BuildService import groovy.transform.CompileStatic +import javax.servlet.http.HttpServletRequest import org.springframework.beans.factory.annotation.Autowired import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestMethod import org.springframework.web.bind.annotation.RestController +import org.springframework.web.servlet.HandlerMapping @CompileStatic -@RequestMapping("/builds") +@RequestMapping("/v2/builds") @RestController class BuildController { + /* + * Job names can have '/' in them if using the Jenkins Folder plugin. + * Because of this, always put the job name at the end of the URL. + */ @Autowired BuildService buildService @@ -42,18 +48,21 @@ class BuildController { buildService.getJobsForBuildMaster(buildMaster) } - @RequestMapping(value = "/{buildMaster}/jobs/{job:.+}", method = RequestMethod.GET) - Map getJobConfig(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job) { + @RequestMapping(value = "/{buildMaster}/jobs/**", method = RequestMethod.GET) + Map getJobConfig(@PathVariable("buildMaster") String buildMaster, HttpServletRequest request) { + def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).toString().split('/').drop(5).join('/') buildService.getJobConfig(buildMaster, job) } - @RequestMapping(value = "/{buildMaster}/jobs/{job}/builds", method = RequestMethod.GET) - List getBuilds(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job) { + @RequestMapping(value = "/{buildMaster}/builds/**", method = RequestMethod.GET) + List getBuilds(@PathVariable("buildMaster") String buildMaster, HttpServletRequest request) { + def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)toString().split('/').drop(5).join('/') buildService.getBuilds(buildMaster, job) } - @RequestMapping(value = "/{buildMaster}/jobs/{job}/builds/{number}", method = RequestMethod.GET) - Map getBuilds(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job, @PathVariable("number") String number) { + @RequestMapping(value = "/{buildMaster}/build/{number}/**", method = RequestMethod.GET) + Map getBuild(@PathVariable("buildMaster") String buildMaster, @PathVariable("number") String number, HttpServletRequest request) { + def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)toString().split('/').drop(6).join('/') buildService.getBuild(buildMaster, job, number) } } diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/BuildService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/BuildService.groovy index 76bfc95f60..86858b1bd0 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/BuildService.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/BuildService.groovy @@ -26,6 +26,7 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.http.HttpStatus import org.springframework.stereotype.Component import org.springframework.web.bind.annotation.ResponseStatus +import org.springframework.web.util.UriUtils import retrofit.RetrofitError @CompileStatic @@ -36,6 +37,10 @@ class BuildService { @Autowired(required = false) IgorService igorService + private String encode(uri) { + return UriUtils.encodeFragment(uri.toString(), "UTF-8") + } + List getBuildMasters() { if (!igorService) { return [] @@ -69,7 +74,7 @@ class BuildService { } HystrixFactory.newMapCommand(GROUP, "jobConfig") { try { - igorService.getJobConfig(buildMaster, job) + igorService.getJobConfig(buildMaster, encode(job)) } catch (RetrofitError e) { if (e.response?.status == 404) { throw new BuildMasterNotFound("Build master '${buildMaster}' not found") @@ -86,7 +91,7 @@ class BuildService { } HystrixFactory.newListCommand(GROUP, "buildsForJob") { try { - igorService.getBuilds(buildMaster, job) + igorService.getBuilds(buildMaster, encode(job)) } catch (RetrofitError e) { if (e.response?.status == 404) { throw new BuildMasterNotFound("Build master '${buildMaster}' not found") @@ -103,7 +108,7 @@ class BuildService { } HystrixFactory.newMapCommand(GROUP, "buildDetails") { try { - igorService.getBuild(buildMaster, job, number) + igorService.getBuild(buildMaster, encode(job), number) } catch (RetrofitError e) { if (e.response?.status == 404) { throw new BuildMasterNotFound("Build master '${buildMaster}' not found") diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/internal/IgorService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/internal/IgorService.groovy index c0f25d56c8..434bf050b3 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/internal/IgorService.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/internal/IgorService.groovy @@ -17,10 +17,16 @@ package com.netflix.spinnaker.gate.services.internal +import retrofit.http.EncodedPath import retrofit.http.GET import retrofit.http.Path interface IgorService { + /* + * Job names can have '/' in them if using the Jenkins Folder plugin. + * Because of this, always put the job name at the end of the URL. + */ + @GET('/masters') List getBuildMasters() @@ -28,12 +34,11 @@ interface IgorService { List getJobsForBuildMaster(@Path("buildMaster") String buildMaster) @GET('/jobs/{buildMaster}/{job}') - Map getJobConfig(@Path("buildMaster") String buildMaster, @Path("job") String job) - - @GET('/jobs/{buildMaster}/{job}/builds') - List getBuilds(@Path("buildMaster") String buildMaster, @Path("job") String job) + Map getJobConfig(@Path("buildMaster") String buildMaster, @EncodedPath("job") String job) - @GET('/jobs/{buildMaster}/{job}/{number}') - Map getBuild(@Path("buildMaster") String buildMaster, @Path("job") String job, @Path("number") String number) + @GET('/builds/all/{buildMaster}/{job}') + List getBuilds(@Path("buildMaster") String buildMaster, @EncodedPath("job") String job) + @GET('/builds/status/{number}/{buildMaster}/{job}') + Map getBuild(@Path("buildMaster") String buildMaster, @EncodedPath("job") String job, @Path("number") String number) } diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy new file mode 100644 index 0000000000..e8ef71d73d --- /dev/null +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy @@ -0,0 +1,114 @@ +/* + * Copyright 2015 Netflix, 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.gate.controllers + +import com.netflix.spinnaker.gate.services.BuildService +import com.netflix.spinnaker.gate.services.internal.IgorService +import com.squareup.okhttp.mockwebserver.MockWebServer +import org.springframework.http.MediaType +import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.setup.MockMvcBuilders +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import spock.lang.Shared +import spock.lang.Specification + +class BuildControllerSpec extends Specification { + + MockMvc mockMvc + BuildService buildService + IgorService igorService + + @Shared + MockWebServer server + + final MASTER = 'MASTER' + final BUILD_NUMBER = 123 + final JOB_NAME = "name/with/slashes and spaces" + final JOB_NAME_ENCODED = "name/with/slashes%20and%20spaces" + + void cleanup() { + server.shutdown() + } + + void setup() { + igorService = Mock(IgorService) + buildService = new BuildService(igorService: igorService) + server = new MockWebServer() + mockMvc = MockMvcBuilders.standaloneSetup(new BuildController(buildService: buildService)).build() + } + + void 'should get a list of masters'() { + given: + 1 * igorService.getBuildMasters() >> [MASTER, "master2"] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/v2/builds") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[\"${MASTER}\",\"master2\"]" + } + + void 'should get a list of jobs for a master'() { + given: + 1 * igorService.getJobsForBuildMaster(MASTER) >> [JOB_NAME, "another_job"] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/v2/builds/${MASTER}/jobs") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[\"${JOB_NAME}\",\"another_job\"]" + } + + void 'should get a list of builds for a job'() { + given: + 1 * igorService.getBuilds(MASTER, JOB_NAME_ENCODED) >> [["building":false, "number":111], ["building":false, "number":222]] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/v2/builds/${MASTER}/builds/${JOB_NAME}") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[{\"building\":false,\"number\":111},{\"building\":false,\"number\":222}]" + } + + void 'should get a job config'() { + given: + 1 * igorService.getJobConfig(MASTER, JOB_NAME_ENCODED) >> ['name': JOB_NAME, 'url': "http://test.com/job/${JOB_NAME}".toString()] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/v2/builds/${MASTER}/jobs/${JOB_NAME}") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "{\"name\":\"${JOB_NAME}\",\"url\":\"http://test.com/job/${JOB_NAME}\"}" + } + + void 'should get a build'() { + given: + 1 * igorService.getBuild(MASTER, JOB_NAME_ENCODED, BUILD_NUMBER.toString()) >> ["building":false, "number":BUILD_NUMBER] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/v2/builds/${MASTER}/build/${BUILD_NUMBER}/${JOB_NAME}") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "{\"building\":false,\"number\":${BUILD_NUMBER}}" + } +} From a85fdf762610796e4098c186ec5919a7da7188d0 Mon Sep 17 00:00:00 2001 From: AMeng Date: Mon, 21 Dec 2015 12:14:20 -0700 Subject: [PATCH 2/2] Restore legacy endpoints to allow for a graceful migration --- .../gate/controllers/BuildController.groovy | 38 +++++++++-- .../controllers/BuildControllerSpec.groovy | 63 +++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy index 9a32a83cff..9fe3be7249 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BuildController.groovy @@ -28,7 +28,6 @@ import org.springframework.web.bind.annotation.RestController import org.springframework.web.servlet.HandlerMapping @CompileStatic -@RequestMapping("/v2/builds") @RestController class BuildController { /* @@ -38,31 +37,58 @@ class BuildController { @Autowired BuildService buildService - @RequestMapping(method = RequestMethod.GET) + @RequestMapping(value = "v2/builds", method = RequestMethod.GET) List getBuildMasters() { buildService.getBuildMasters() } - @RequestMapping(value = "/{buildMaster}/jobs", method = RequestMethod.GET) + @RequestMapping(value = "/v2/builds/{buildMaster}/jobs", method = RequestMethod.GET) List getJobsForBuildMaster(@PathVariable("buildMaster") String buildMaster) { buildService.getJobsForBuildMaster(buildMaster) } - @RequestMapping(value = "/{buildMaster}/jobs/**", method = RequestMethod.GET) + @RequestMapping(value = "/v2/builds/{buildMaster}/jobs/**", method = RequestMethod.GET) Map getJobConfig(@PathVariable("buildMaster") String buildMaster, HttpServletRequest request) { def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).toString().split('/').drop(5).join('/') buildService.getJobConfig(buildMaster, job) } - @RequestMapping(value = "/{buildMaster}/builds/**", method = RequestMethod.GET) + @RequestMapping(value = "/v2/builds/{buildMaster}/builds/**", method = RequestMethod.GET) List getBuilds(@PathVariable("buildMaster") String buildMaster, HttpServletRequest request) { def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)toString().split('/').drop(5).join('/') buildService.getBuilds(buildMaster, job) } - @RequestMapping(value = "/{buildMaster}/build/{number}/**", method = RequestMethod.GET) + @RequestMapping(value = "/v2/builds/{buildMaster}/build/{number}/**", method = RequestMethod.GET) Map getBuild(@PathVariable("buildMaster") String buildMaster, @PathVariable("number") String number, HttpServletRequest request) { def job = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)toString().split('/').drop(6).join('/') buildService.getBuild(buildMaster, job, number) } + + // LEGACY ENDPOINTS: + + @RequestMapping(value = "/builds", method = RequestMethod.GET) + List getBuildMastersLegacy() { + buildService.getBuildMasters() + } + + @RequestMapping(value = "/builds/{buildMaster}/jobs", method = RequestMethod.GET) + List getJobsForBuildMasterLegacy(@PathVariable("buildMaster") String buildMaster) { + buildService.getJobsForBuildMaster(buildMaster) + } + + @RequestMapping(value = "/builds/{buildMaster}/jobs/{job:.+}", method = RequestMethod.GET) + Map getJobConfigLegacy(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job) { + buildService.getJobConfig(buildMaster, job) + } + + @RequestMapping(value = "/builds/{buildMaster}/jobs/{job}/builds", method = RequestMethod.GET) + List getBuildsLegacy(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job) { + buildService.getBuilds(buildMaster, job) + } + + @RequestMapping(value = "/builds/{buildMaster}/jobs/{job}/builds/{number}", method = RequestMethod.GET) + Map getBuildsLegacy(@PathVariable("buildMaster") String buildMaster, @PathVariable("job") String job, @PathVariable("number") String number) { + buildService.getBuild(buildMaster, job, number) + } } diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy index e8ef71d73d..d483668b90 100644 --- a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/BuildControllerSpec.groovy @@ -39,6 +39,7 @@ class BuildControllerSpec extends Specification { final MASTER = 'MASTER' final BUILD_NUMBER = 123 final JOB_NAME = "name/with/slashes and spaces" + final JOB_NAME_LEGACY = "job" final JOB_NAME_ENCODED = "name/with/slashes%20and%20spaces" void cleanup() { @@ -111,4 +112,66 @@ class BuildControllerSpec extends Specification { then: response.contentAsString == "{\"building\":false,\"number\":${BUILD_NUMBER}}" } + + // LEGACY ENDPOINT TESTS: + + void 'should get a list of masters LEGACY'() { + given: + 1 * igorService.getBuildMasters() >> [MASTER, "master2"] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/builds") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[\"${MASTER}\",\"master2\"]" + } + + void 'should get a list of jobs for a master LEGACY'() { + given: + 1 * igorService.getJobsForBuildMaster(MASTER) >> [JOB_NAME_LEGACY, "another_job"] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/builds/${MASTER}/jobs") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[\"${JOB_NAME_LEGACY}\",\"another_job\"]" + } + + void 'should get a list of builds for a job LEGACY'() { + given: + 1 * igorService.getBuilds(MASTER, JOB_NAME_LEGACY) >> [["building":false, "number":111], ["building":false, "number":222]] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/builds/${MASTER}/jobs/${JOB_NAME_LEGACY}/builds") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "[{\"building\":false,\"number\":111},{\"building\":false,\"number\":222}]" + } + + void 'should get a job config LEGACY'() { + given: + 1 * igorService.getJobConfig(MASTER, JOB_NAME_LEGACY) >> ['name': JOB_NAME_LEGACY, 'url': "http://test.com/job/${JOB_NAME_LEGACY}".toString()] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/builds/${MASTER}/jobs/${JOB_NAME_LEGACY}") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "{\"name\":\"${JOB_NAME_LEGACY}\",\"url\":\"http://test.com/job/${JOB_NAME_LEGACY}\"}" + } + + void 'should get a build LEGACY'() { + given: + 1 * igorService.getBuild(MASTER, JOB_NAME_LEGACY, BUILD_NUMBER.toString()) >> ["building":false, "number":BUILD_NUMBER] + + when: + MockHttpServletResponse response = mockMvc.perform(get("/builds/${MASTER}/jobs/${JOB_NAME_LEGACY}/builds/${BUILD_NUMBER}/") + .accept(MediaType.APPLICATION_JSON)).andReturn().response + + then: + response.contentAsString == "{\"building\":false,\"number\":${BUILD_NUMBER}}" + } }