From 80610ddbfdb6fd3e9492f6859da2fc547f4d61cb Mon Sep 17 00:00:00 2001 From: Richard Timpson Date: Mon, 5 Jun 2023 16:19:29 -0600 Subject: [PATCH] feat(pipelines): add pipelineNameFilter to /{application}/pipelineConfigs endpoint This plumbs through the new pipelineNameFilter query param to the front50 service call in gate. Refer the front50 change: https://github.com/spinnaker/front50/pull/1504 --- .../services/internal/Front50Service.java | 4 +- .../controllers/ApplicationController.groovy | 5 +- .../controllers/PipelineController.groovy | 6 +- .../gate/services/ApplicationService.groovy | 6 +- .../ApplicationControllerSpec.groovy | 93 +++++++++++++++---- .../controllers/PipelineControllerSpec.groovy | 2 +- 6 files changed, 90 insertions(+), 26 deletions(-) diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/Front50Service.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/Front50Service.java index 62a523fe15..d2a6427b56 100644 --- a/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/Front50Service.java +++ b/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/Front50Service.java @@ -42,7 +42,9 @@ List getApplicationHistory( @GET("/pipelines/{app}") List getPipelineConfigsForApplication( - @Path("app") String app, @Query("refresh") boolean refresh); + @Path("app") String app, + @Query("pipelineNameFilter") String pipelineNameFilter, + @Query("refresh") boolean refresh); @GET("/pipelines/{app}/name/{name}") Map getPipelineConfigByApplicationAndName( diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ApplicationController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ApplicationController.groovy index 9dec875f3f..bdc743bb9d 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ApplicationController.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ApplicationController.groovy @@ -135,8 +135,9 @@ class ApplicationController { @ApiOperation(value = "Retrieve a list of an application's pipeline configurations", response = List.class) @RequestMapping(value = "/{application}/pipelineConfigs", method = RequestMethod.GET) - List getPipelineConfigsForApplication(@PathVariable("application") String application) { - applicationService.getPipelineConfigsForApplication(application) + List getPipelineConfigsForApplication(@PathVariable("application") String application, + @RequestParam(required = false, value="pipelineNameFilter") String pipelineNameFilter) { + applicationService.getPipelineConfigsForApplication(application, pipelineNameFilter) } @ApiOperation(value = "Retrieve a pipeline configuration", response = HashMap.class) diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy index 2dc8437b01..3bbc5f7523 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy @@ -75,7 +75,7 @@ class PipelineController { @ApiOperation(value = "Delete a pipeline definition") @DeleteMapping("/{application}/{pipelineName:.+}") void deletePipeline(@PathVariable String application, @PathVariable String pipelineName) { - List pipelineConfigs = front50Service.getPipelineConfigsForApplication(application, true) + List pipelineConfigs = front50Service.getPipelineConfigsForApplication(application, null, true) if (pipelineConfigs!=null && !pipelineConfigs.isEmpty()){ Optional filterResult = pipelineConfigs.stream().filter({ pipeline -> ((String) pipeline.get("name")) != null && ((String) pipeline.get("name")).trim().equalsIgnoreCase(pipelineName) }).findFirst() if (filterResult.isPresent()){ @@ -220,7 +220,7 @@ class PipelineController { ) } - return front50Service.getPipelineConfigsForApplication((String) pipeline.get("application"), true)?.find { + return front50Service.getPipelineConfigsForApplication((String) pipeline.get("application"), null, true)?.find { id == (String) it.get("id") } } @@ -259,7 +259,7 @@ class PipelineController { String pipelineName = pipelineMap.get("name"); String application = pipelineMap.get("application"); - List pipelineConfigs = front50Service.getPipelineConfigsForApplication(application, true) + List pipelineConfigs = front50Service.getPipelineConfigsForApplication(application, null, true) if (pipelineConfigs!=null && !pipelineConfigs.isEmpty()){ Optional filterResult = pipelineConfigs.stream() diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy index 9a89eebec4..9eb19587c2 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy @@ -150,7 +150,11 @@ class ApplicationService { } List getPipelineConfigsForApplication(String app) { - return front50Service.getPipelineConfigsForApplication(app, true) + return getPipelineConfigsForApplication(app, null); + } + + List getPipelineConfigsForApplication(String app, String pipelineNameFilter) { + return front50Service.getPipelineConfigsForApplication(app, pipelineNameFilter, true) } /** diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/ApplicationControllerSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/ApplicationControllerSpec.groovy index da974d9d3f..44de3ceb76 100644 --- a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/ApplicationControllerSpec.groovy +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/ApplicationControllerSpec.groovy @@ -16,6 +16,7 @@ package com.netflix.spinnaker.gate.controllers +import com.fasterxml.jackson.databind.ObjectMapper import com.netflix.spinnaker.gate.config.ApplicationConfigurationProperties import com.netflix.spinnaker.gate.config.ServiceConfiguration import com.netflix.spinnaker.gate.services.ApplicationService @@ -23,9 +24,7 @@ import com.netflix.spinnaker.gate.services.internal.ClouddriverService import com.netflix.spinnaker.gate.services.internal.ClouddriverServiceSelector import com.netflix.spinnaker.gate.services.internal.Front50Service import com.squareup.okhttp.mockwebserver.MockWebServer -import groovy.json.JsonSlurper 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 org.springframework.web.util.NestedServletException @@ -33,6 +32,8 @@ import spock.lang.Specification import spock.lang.Unroll import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status class ApplicationControllerSpec extends Specification { @@ -62,9 +63,66 @@ class ApplicationControllerSpec extends Specification { mockMvc = MockMvcBuilders.standaloneSetup(new ApplicationController(applicationService: applicationService)).build() } + @Unroll + void 'should return configs for an application' (){ + given: "random configs" + def configs = [ + [ + name: 'pipelineA', + some: 'some-random-x', + ], + [ + name: 'pipelineB', + some: 'some-random-F', + ], + ] + + when: "all configs are requested" + def response = mockMvc.perform(get(endpoint) + .accept(MediaType.APPLICATION_JSON)) + + then: "we only call front50 once, and do not pass through the pipelineNameFilter" + 1 * front50Service.getPipelineConfigsForApplication('true-app', null, true) >> configs + 0 * front50Service._ + + and: "we get all configs" + response.andExpect status().isOk() + response.andExpect content().string(new ObjectMapper().writeValueAsString(configs)) + + where: + endpoint << ["/applications/true-app/pipelineConfigs"] + } + + @Unroll + void 'should return configs for an application with pipelineNameFilter' (){ + given: "only one config" + def configs = [ + [ + name: 'pipelineA', + some: 'some-random-x', + ], + ] + + when: "configs are requested with a filter" + def response = mockMvc.perform(get(endpoint) + .accept(MediaType.APPLICATION_JSON)) + + then: "we only call front50 once, and we do pass through the pipelineNameFilter" + 1 * front50Service.getPipelineConfigsForApplication('true-app', 'pipelineA', true) >> configs + 0 * front50Service._ + + and: "only filtered configs are returned" + response.andExpect status().isOk() + response.andExpect content().string(new ObjectMapper().writeValueAsString(configs)) + + where: + endpoint << ["/applications/true-app/pipelineConfigs?pipelineNameFilter=pipelineA"] + } + @Unroll void 'should return 200 with info on pipeline that exists with config' (){ + given: def configs = [ [ name: 'some-true-pipeline', @@ -77,15 +135,14 @@ class ApplicationControllerSpec extends Specification { someY: 'some-random-Z' ], ] - given: - 1 * front50Service.getPipelineConfigsForApplication('true-app', true) >> configs when: - MockHttpServletResponse response = mockMvc.perform(get(endpoint) - .accept(MediaType.APPLICATION_JSON)).andReturn().response + def response = mockMvc.perform(get(endpoint) + .accept(MediaType.APPLICATION_JSON)) then: - new JsonSlurper().parseText(response.contentAsString) == configs[0] - response.status == 200 + 1 * front50Service.getPipelineConfigsForApplication('true-app', null, true) >> configs + response.andExpect status().isOk() + response.andExpect content().string(new ObjectMapper().writeValueAsString(configs[0])) where: endpoint << ["/applications/true-app/pipelineConfigs/some-true-pipeline"] @@ -93,6 +150,7 @@ class ApplicationControllerSpec extends Specification { @Unroll void 'should return 404 on pipeline that does not exists' (){ + given: def configs = [ [ name: 'some-true-pipeline', @@ -100,12 +158,11 @@ class ApplicationControllerSpec extends Specification { someY: 'some-random-y' ] ] - given: - 1 * front50Service.getPipelineConfigsForApplication('true-app', true) >> configs when: mockMvc.perform(get(endpoint)) then: + 1 * front50Service.getPipelineConfigsForApplication('true-app', null, true) >> configs NestedServletException ex = thrown() ex.message.contains('Pipeline config (id: some-fake-pipeline) not found for Application (id: true-app)') @@ -115,6 +172,7 @@ class ApplicationControllerSpec extends Specification { @Unroll void 'should return 200 with strategy configuration for strategy exists' (){ + given: def configs = [ [ name: 'some-true-strategy', @@ -127,15 +185,14 @@ class ApplicationControllerSpec extends Specification { someY: 'some-random-Z' ], ] - given: - 1 * front50Service.getStrategyConfigs('true-app') >> configs when: - MockHttpServletResponse response = mockMvc.perform(get(endpoint) - .accept(MediaType.APPLICATION_JSON)).andReturn().response + def response = mockMvc.perform(get(endpoint) + .accept(MediaType.APPLICATION_JSON)) then: - new JsonSlurper().parseText(response.contentAsString) == configs[0] - response.status == 200 + 1 * front50Service.getStrategyConfigs('true-app') >> configs + response.andExpect status().isOk() + response.andExpect content().string(new ObjectMapper().writeValueAsString(configs[0])) where: endpoint << ["/applications/true-app/strategyConfigs/some-true-strategy"] @@ -143,6 +200,7 @@ class ApplicationControllerSpec extends Specification { @Unroll void 'should return 404 with strategy configuration for strategy not exists' (){ + given: def configs = [ [ name: 'some-true-strategy', @@ -150,12 +208,11 @@ class ApplicationControllerSpec extends Specification { someY: 'some-random-y' ] ] - given: - 1 * front50Service.getStrategyConfigs('true-app') >> configs when: mockMvc.perform(get(endpoint)) then: + 1 * front50Service.getStrategyConfigs('true-app') >> configs NestedServletException ex = thrown() ex.message.contains('Strategy config (id: some-fake-strategy) not found for Application (id: true-app)') diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy index 0358152798..e16b7e32b9 100644 --- a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy @@ -91,7 +91,7 @@ class PipelineControllerSpec extends Specification { ] ] ]) >> { [id: 'task-id', application: 'application', status: 'SUCCEEDED'] } - 1 * front50Service.getPipelineConfigsForApplication('application', true) >> [] + 1 * front50Service.getPipelineConfigsForApplication('application', null, true) >> [] } def "should propagate pipeline template errors"() {