Skip to content
This repository was archived by the owner on Dec 20, 2025. It is now read-only.

Commit de72fa6

Browse files
fix(web): fix the mismatches in the return types of APIs in gate and orca (#1883) (#1884)
* test(web): add a test to demonstrate the mismatch in api response types of gate and orca when delete pipeline execution api is invoked * fix(web): fix the mismatches in the return types of apis in gate and orca for delete, resume, cancel and pause operations of a pipeline. * refactor(web/test): mock DefaultProviderLookupService in PipelineServiceTest to remove errors from test output * refactor(web/test): remove request id header from PipelineServiceTest since it's not needed * refactor(web/test): add X-SPINNAKER-USER and X-SPINNAKER-ACCOUNTS headers to requests in PipelineServiceTest to silence warnings like: 2025-03-01 11:28:27.487 WARN 98848 --- [ Test worker] c.n.s.okhttp.OkHttp3MetricsInterceptor : [] Request GET:http://localhost:52447/pipelines/my-pipeline-execution-id is missing [X-SPINNAKER-USER, X-SPINNAKER-ACCOUNTS] authentication headers and will be treated as anonymous. Request from: com.netflix.spinnaker.okhttp.MetricsInterceptor.doIntercept(MetricsInterceptor.java:82) at com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor.intercept(OkHttp3MetricsInterceptor.java:36) at com.netflix.spinnaker.okhttp.SpinnakerRequestHeaderInterceptor.intercept(SpinnakerRequestHeaderInterceptor.java:64) at com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory$ExecutorCallbackCall.execute(ErrorHandlingExecutorCallAdapterFactory.java:150) at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.executeCall(Retrofit2SyncCall.java:47) at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.execute(Retrofit2SyncCall.java:34) at com.netflix.spinnaker.gate.services.PipelineService.getPipeline(PipelineService.groovy:170) at com.netflix.spinnaker.gate.services.PipelineService$_setApplicationForExecution_closure2.doCall(PipelineService.groovy:222) at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:34) at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:27) at com.netflix.spinnaker.gate.services.PipelineService.setApplicationForExecution(PipelineService.groovy:222) at com.netflix.spinnaker.gate.services.PipelineService.deletePipeline(PipelineService.groovy:189) at com.netflix.spinnaker.gate.controllers.PipelineController.deletePipeline(PipelineController.groovy:279) at com.netflix.spinnaker.gate.service.PipelineServiceTest.invokeDeletePipelineExecution(PipelineServiceTest.java:135) --------- Co-authored-by: David Byron <dbyron@salesforce.com> (cherry picked from commit 15465c9) Co-authored-by: Kiran Godishala <53332225+kirangodishala@users.noreply.github.com>
1 parent 2db175e commit de72fa6

File tree

6 files changed

+158
-12
lines changed

6 files changed

+158
-12
lines changed

gate-core/src/main/java/com/netflix/spinnaker/gate/services/TaskService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ public Map createAndWaitForCompletion(Map body) {
150150

151151
/** @deprecated This pipeline operation does not belong here. */
152152
@Deprecated
153-
public Map cancelPipeline(final String id, final String reason) {
154-
return Retrofit2SyncCall.execute(
153+
public void cancelPipeline(final String id, final String reason) {
154+
Retrofit2SyncCall.execute(
155155
getOrcaServiceSelector().select().cancelPipeline(id, reason, false, ""));
156156
}
157157

gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/OrcaService.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,23 @@ Call<List> searchForPipelineExecutionsByTrigger(
9090

9191
@Headers("Accept: application/json")
9292
@PUT("/pipelines/{id}/cancel")
93-
Call<Map> cancelPipeline(
93+
Call<Void> cancelPipeline(
9494
@Path("id") String id,
9595
@Query("reason") String reason,
9696
@Query("force") boolean force,
9797
@Body String ignored);
9898

9999
@Headers("Accept: application/json")
100100
@PUT("/pipelines/{id}/pause")
101-
Call<Map> pausePipeline(@Path("id") String id, @Body String ignored);
101+
Call<Void> pausePipeline(@Path("id") String id, @Body String ignored);
102102

103103
@Headers("Accept: application/json")
104104
@PUT("/pipelines/{id}/resume")
105-
Call<Map> resumePipeline(@Path("id") String id, @Body String ignored);
105+
Call<Void> resumePipeline(@Path("id") String id, @Body String ignored);
106106

107107
@Headers("Accept: application/json")
108108
@DELETE("/pipelines/{id}")
109-
Call<Map> deletePipeline(@Path("id") String id);
109+
Call<Void> deletePipeline(@Path("id") String id);
110110

111111
@Headers("Accept: application/json")
112112
@PUT("/pipelines/{executionId}/stages/{stageId}/restart")

gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ApplicationController.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class ApplicationController {
130130
@Deprecated
131131
@Operation(summary = "Cancel pipeline")
132132
@RequestMapping(value = "/{application}/pipelines/{id}/cancel", method = RequestMethod.PUT)
133-
Map cancelPipeline(@PathVariable("id") String id,
133+
void cancelPipeline(@PathVariable("id") String id,
134134
@RequestParam(required = false) String reason) {
135135
taskService.cancelPipeline(id, reason)
136136
}

gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class PipelineController {
275275

276276
@Operation(summary = "Delete a pipeline execution")
277277
@DeleteMapping("{id}")
278-
Map deletePipeline(@PathVariable("id") String id) {
278+
void deletePipeline(@PathVariable("id") String id) {
279279
pipelineService.deletePipeline(id);
280280
}
281281

gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/PipelineService.groovy

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,22 @@ class PipelineService {
170170
Retrofit2SyncCall.execute(orcaServiceSelector.select().getPipeline(id))
171171
}
172172

173-
Map cancelPipeline(String id, String reason, boolean force) {
173+
void cancelPipeline(String id, String reason, boolean force) {
174174
setApplicationForExecution(id)
175175
Retrofit2SyncCall.execute(orcaServiceSelector.select().cancelPipeline(id, reason, force, ""))
176176
}
177177

178-
Map pausePipeline(String id) {
178+
void pausePipeline(String id) {
179179
setApplicationForExecution(id)
180180
Retrofit2SyncCall.execute(orcaServiceSelector.select().pausePipeline(id, ""))
181181
}
182182

183-
Map resumePipeline(String id) {
183+
void resumePipeline(String id) {
184184
setApplicationForExecution(id)
185185
Retrofit2SyncCall.execute(orcaServiceSelector.select().resumePipeline(id, ""))
186186
}
187187

188-
Map deletePipeline(String id) {
188+
void deletePipeline(String id) {
189189
setApplicationForExecution(id)
190190
Retrofit2SyncCall.execute(orcaServiceSelector.select().deletePipeline(id))
191191
}
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright 2025 OpsMx, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.netflix.spinnaker.gate.service;
18+
19+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
21+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
22+
import static com.netflix.spinnaker.kork.common.Header.ACCOUNTS;
23+
import static com.netflix.spinnaker.kork.common.Header.USER;
24+
import static org.mockito.ArgumentMatchers.anyBoolean;
25+
import static org.mockito.Mockito.when;
26+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
27+
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
28+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
29+
import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup;
30+
31+
import com.fasterxml.jackson.databind.ObjectMapper;
32+
import com.github.tomakehurst.wiremock.client.WireMock;
33+
import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
34+
import com.netflix.spinnaker.gate.Main;
35+
import com.netflix.spinnaker.gate.health.DownstreamServicesHealthIndicator;
36+
import com.netflix.spinnaker.gate.services.DefaultProviderLookupService;
37+
import com.netflix.spinnaker.gate.services.internal.ClouddriverService;
38+
import com.netflix.spinnaker.gate.services.internal.ClouddriverServiceSelector;
39+
import java.util.Collections;
40+
import java.util.Map;
41+
import org.junit.jupiter.api.BeforeEach;
42+
import org.junit.jupiter.api.Test;
43+
import org.junit.jupiter.api.TestInfo;
44+
import org.junit.jupiter.api.extension.RegisterExtension;
45+
import org.springframework.beans.factory.annotation.Autowired;
46+
import org.springframework.beans.factory.annotation.Qualifier;
47+
import org.springframework.boot.test.context.SpringBootTest;
48+
import org.springframework.boot.test.mock.mockito.MockBean;
49+
import org.springframework.boot.web.servlet.FilterRegistrationBean;
50+
import org.springframework.test.context.DynamicPropertyRegistry;
51+
import org.springframework.test.context.DynamicPropertySource;
52+
import org.springframework.test.context.TestPropertySource;
53+
import org.springframework.test.web.servlet.MockMvc;
54+
import org.springframework.web.context.WebApplicationContext;
55+
import retrofit2.mock.Calls;
56+
57+
@SpringBootTest(classes = Main.class)
58+
@TestPropertySource(
59+
properties = {
60+
"spring.config.location=classpath:gate-test.yml",
61+
"services.front50.applicationRefreshInitialDelayMs=3600000"
62+
})
63+
public class PipelineServiceTest {
64+
private MockMvc webAppMockMvc;
65+
66+
@RegisterExtension
67+
static WireMockExtension wmOrca =
68+
WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build();
69+
70+
@Autowired private WebApplicationContext webApplicationContext;
71+
72+
ObjectMapper objectMapper = new ObjectMapper();
73+
74+
/**
75+
* This takes X-SPINNAKER-* headers from requests to gate and puts them in the MDC. This is
76+
* enabled when gate runs normally (by GateConfig), but needs explicit mention to function in
77+
* these tests.
78+
*/
79+
@Autowired
80+
@Qualifier("authenticatedRequestFilter")
81+
private FilterRegistrationBean filterRegistrationBean;
82+
83+
@MockBean ClouddriverServiceSelector clouddriverServiceSelector;
84+
85+
@MockBean ClouddriverService clouddriverService;
86+
87+
/** To prevent periodic calls to service's /health endpoints */
88+
@MockBean DownstreamServicesHealthIndicator downstreamServicesHealthIndicator;
89+
90+
/** To prevent periodic calls to load accounts from clouddriver */
91+
@MockBean DefaultProviderLookupService defaultProviderLookupService;
92+
93+
private static final String USERNAME = "some user";
94+
private static final String ACCOUNT = "my-account";
95+
private static final String PIPELINE_EXECUTION_ID = "my-pipeline-execution-id";
96+
97+
@DynamicPropertySource
98+
static void registerUrls(DynamicPropertyRegistry registry) {
99+
// Configure wiremock's random ports into gate
100+
System.out.println("wiremock orca url: " + wmOrca.baseUrl());
101+
registry.add("services.orca.base-url", wmOrca::baseUrl);
102+
}
103+
104+
@BeforeEach
105+
void init(TestInfo testInfo) {
106+
System.out.println("--------------- Test " + testInfo.getDisplayName());
107+
108+
webAppMockMvc =
109+
webAppContextSetup(webApplicationContext)
110+
.addFilters(filterRegistrationBean.getFilter())
111+
.build();
112+
113+
// Keep the background thread that refreshes the applications cache in
114+
// ApplicationService happy.
115+
when(clouddriverServiceSelector.select()).thenReturn(clouddriverService);
116+
when(clouddriverService.getAllApplicationsUnrestricted(anyBoolean()))
117+
.thenReturn(Calls.response(Collections.emptyList()));
118+
}
119+
120+
@Test
121+
void invokeDeletePipelineExecution() throws Exception {
122+
wmOrca.stubFor(
123+
WireMock.get(urlEqualTo("/pipelines/" + PIPELINE_EXECUTION_ID))
124+
.willReturn(
125+
aResponse()
126+
.withStatus(200)
127+
.withBody(objectMapper.writeValueAsString(Map.of("foo", "bar")))));
128+
129+
// simulate Orca response to the delete request
130+
wmOrca.stubFor(
131+
WireMock.delete(urlEqualTo("/pipelines/" + PIPELINE_EXECUTION_ID))
132+
.willReturn(aResponse().withStatus(200)));
133+
134+
webAppMockMvc
135+
.perform(
136+
delete("/pipelines/" + PIPELINE_EXECUTION_ID)
137+
.header(
138+
USER.getHeader(),
139+
USERNAME) // to silence warning when X-SPINNAKER-USER is missing
140+
.header(
141+
ACCOUNTS.getHeader(),
142+
ACCOUNT)) // to silence warning when X-SPINNAKER-ACCOUNTS is missing
143+
.andDo(print())
144+
.andExpect(status().is2xxSuccessful());
145+
}
146+
}

0 commit comments

Comments
 (0)