fix(web): fix the mismatches in the return types of APIs in gate and …

…orca (spinnaker#1883) * 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 <[email protected]>
KathrynLewis · Mar 1, 2025 · 15465c9 · 15465c9
1 parent 5b500d3
commit 15465c9
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 12 deletions.
diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/services/TaskService.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/services/TaskService.java
@@ -150,8 +150,8 @@ public Map createAndWaitForCompletion(Map body) {
 
   /** @deprecated This pipeline operation does not belong here. */
   @Deprecated
-  public Map cancelPipeline(final String id, final String reason) {
-    return Retrofit2SyncCall.execute(
+  public void cancelPipeline(final String id, final String reason) {
+    Retrofit2SyncCall.execute(
         getOrcaServiceSelector().select().cancelPipeline(id, reason, false, ""));
   }
 

diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/OrcaService.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/services/internal/OrcaService.java
@@ -90,23 +90,23 @@ Call<List> searchForPipelineExecutionsByTrigger(
 
   @Headers("Accept: application/json")
   @PUT("/pipelines/{id}/cancel")
-  Call<Map> cancelPipeline(
+  Call<Void> cancelPipeline(
       @Path("id") String id,
       @Query("reason") String reason,
       @Query("force") boolean force,
       @Body String ignored);
 
   @Headers("Accept: application/json")
   @PUT("/pipelines/{id}/pause")
-  Call<Map> pausePipeline(@Path("id") String id, @Body String ignored);
+  Call<Void> pausePipeline(@Path("id") String id, @Body String ignored);
 
   @Headers("Accept: application/json")
   @PUT("/pipelines/{id}/resume")
-  Call<Map> resumePipeline(@Path("id") String id, @Body String ignored);
+  Call<Void> resumePipeline(@Path("id") String id, @Body String ignored);
 
   @Headers("Accept: application/json")
   @DELETE("/pipelines/{id}")
-  Call<Map> deletePipeline(@Path("id") String id);
+  Call<Void> deletePipeline(@Path("id") String id);
 
   @Headers("Accept: application/json")
   @PUT("/pipelines/{executionId}/stages/{stageId}/restart")

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
@@ -130,7 +130,7 @@ class ApplicationController {
   @Deprecated
   @Operation(summary = "Cancel pipeline")
   @RequestMapping(value = "/{application}/pipelines/{id}/cancel", method = RequestMethod.PUT)
-  Map cancelPipeline(@PathVariable("id") String id,
+  void cancelPipeline(@PathVariable("id") String id,
                      @RequestParam(required = false) String reason) {
     taskService.cancelPipeline(id, reason)
   }

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
@@ -275,7 +275,7 @@ class PipelineController {
 
   @Operation(summary = "Delete a pipeline execution")
   @DeleteMapping("{id}")
-  Map deletePipeline(@PathVariable("id") String id) {
+  void deletePipeline(@PathVariable("id") String id) {
     pipelineService.deletePipeline(id);
   }
 

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/PipelineService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/PipelineService.groovy
@@ -170,22 +170,22 @@ class PipelineService {
     Retrofit2SyncCall.execute(orcaServiceSelector.select().getPipeline(id))
   }
 
-  Map cancelPipeline(String id, String reason, boolean force) {
+  void cancelPipeline(String id, String reason, boolean force) {
     setApplicationForExecution(id)
     Retrofit2SyncCall.execute(orcaServiceSelector.select().cancelPipeline(id, reason, force, ""))
   }
 
-  Map pausePipeline(String id) {
+  void pausePipeline(String id) {
     setApplicationForExecution(id)
     Retrofit2SyncCall.execute(orcaServiceSelector.select().pausePipeline(id, ""))
   }
 
-  Map resumePipeline(String id) {
+  void resumePipeline(String id) {
     setApplicationForExecution(id)
     Retrofit2SyncCall.execute(orcaServiceSelector.select().resumePipeline(id, ""))
   }
 
-  Map deletePipeline(String id) {
+  void deletePipeline(String id) {
     setApplicationForExecution(id)
     Retrofit2SyncCall.execute(orcaServiceSelector.select().deletePipeline(id))
   }

diff --git a/gate-web/src/test/java/com/netflix/spinnaker/gate/service/PipelineServiceTest.java b/gate-web/src/test/java/com/netflix/spinnaker/gate/service/PipelineServiceTest.java
@@ -0,0 +1,146 @@
+/*
+ * Copyright 2025 OpsMx, 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.service;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static com.netflix.spinnaker.kork.common.Header.ACCOUNTS;
+import static com.netflix.spinnaker.kork.common.Header.USER;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.when;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import com.netflix.spinnaker.gate.Main;
+import com.netflix.spinnaker.gate.health.DownstreamServicesHealthIndicator;
+import com.netflix.spinnaker.gate.services.DefaultProviderLookupService;
+import com.netflix.spinnaker.gate.services.internal.ClouddriverService;
+import com.netflix.spinnaker.gate.services.internal.ClouddriverServiceSelector;
+import java.util.Collections;
+import java.util.Map;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.mock.mockito.MockBean;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.test.context.DynamicPropertyRegistry;
+import org.springframework.test.context.DynamicPropertySource;
+import org.springframework.test.context.TestPropertySource;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.web.context.WebApplicationContext;
+import retrofit2.mock.Calls;
+
+@SpringBootTest(classes = Main.class)
+@TestPropertySource(
+    properties = {
+      "spring.config.location=classpath:gate-test.yml",
+      "services.front50.applicationRefreshInitialDelayMs=3600000"
+    })
+public class PipelineServiceTest {
+  private MockMvc webAppMockMvc;
+
+  @RegisterExtension
+  static WireMockExtension wmOrca =
+      WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build();
+
+  @Autowired private WebApplicationContext webApplicationContext;
+
+  ObjectMapper objectMapper = new ObjectMapper();
+
+  /**
+   * This takes X-SPINNAKER-* headers from requests to gate and puts them in the MDC. This is
+   * enabled when gate runs normally (by GateConfig), but needs explicit mention to function in
+   * these tests.
+   */
+  @Autowired
+  @Qualifier("authenticatedRequestFilter")
+  private FilterRegistrationBean filterRegistrationBean;
+
+  @MockBean ClouddriverServiceSelector clouddriverServiceSelector;
+
+  @MockBean ClouddriverService clouddriverService;
+
+  /** To prevent periodic calls to service's /health endpoints */
+  @MockBean DownstreamServicesHealthIndicator downstreamServicesHealthIndicator;
+
+  /** To prevent periodic calls to load accounts from clouddriver */
+  @MockBean DefaultProviderLookupService defaultProviderLookupService;
+
+  private static final String USERNAME = "some user";
+  private static final String ACCOUNT = "my-account";
+  private static final String PIPELINE_EXECUTION_ID = "my-pipeline-execution-id";
+
+  @DynamicPropertySource
+  static void registerUrls(DynamicPropertyRegistry registry) {
+    // Configure wiremock's random ports into gate
+    System.out.println("wiremock orca url: " + wmOrca.baseUrl());
+    registry.add("services.orca.base-url", wmOrca::baseUrl);
+  }
+
+  @BeforeEach
+  void init(TestInfo testInfo) {
+    System.out.println("--------------- Test " + testInfo.getDisplayName());
+
+    webAppMockMvc =
+        webAppContextSetup(webApplicationContext)
+            .addFilters(filterRegistrationBean.getFilter())
+            .build();
+
+    // Keep the background thread that refreshes the applications cache in
+    // ApplicationService happy.
+    when(clouddriverServiceSelector.select()).thenReturn(clouddriverService);
+    when(clouddriverService.getAllApplicationsUnrestricted(anyBoolean()))
+        .thenReturn(Calls.response(Collections.emptyList()));
+  }
+
+  @Test
+  void invokeDeletePipelineExecution() throws Exception {
+    wmOrca.stubFor(
+        WireMock.get(urlEqualTo("/pipelines/" + PIPELINE_EXECUTION_ID))
+            .willReturn(
+                aResponse()
+                    .withStatus(200)
+                    .withBody(objectMapper.writeValueAsString(Map.of("foo", "bar")))));
+
+    // simulate Orca response to the delete request
+    wmOrca.stubFor(
+        WireMock.delete(urlEqualTo("/pipelines/" + PIPELINE_EXECUTION_ID))
+            .willReturn(aResponse().withStatus(200)));
+
+    webAppMockMvc
+        .perform(
+            delete("/pipelines/" + PIPELINE_EXECUTION_ID)
+                .header(
+                    USER.getHeader(),
+                    USERNAME) // to silence warning when X-SPINNAKER-USER is missing
+                .header(
+                    ACCOUNTS.getHeader(),
+                    ACCOUNT)) // to silence warning when X-SPINNAKER-ACCOUNTS is missing
+        .andDo(print())
+        .andExpect(status().is2xxSuccessful());
+  }
+}