From 0f50c5708f1097a8ae3b53b461f5c9c06dfaf415 Mon Sep 17 00:00:00 2001 From: Kiran Godishala <53332225+kirangodishala@users.noreply.github.com> Date: Sat, 1 Mar 2025 00:36:33 +0530 Subject: [PATCH] fix(web): add Retrofit2SyncCall.execute to ClouddriverService in CredentialsController (#1881) * test(web): add a test to demonstrate the retrofit error that occurs when /credentials/type/{accountType} is invoked * fix(web): add Retrofit2SyncCall.execute to ClouddriverService in CredentialsController (cherry picked from commit 5b500d3ccf4fb893461ff1ee139aff55059365b2) --- .../controllers/CredentialsController.groovy | 9 +- .../CredentialsControllerSpec.groovy | 5 +- .../CredentialsControllerTest.java | 138 ++++++++++++++++++ 3 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 gate-web/src/test/java/com/netflix/spinnaker/gate/controllers/CredentialsControllerTest.java diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CredentialsController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CredentialsController.groovy index a4771bda6e..0dcde5d73c 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CredentialsController.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/CredentialsController.groovy @@ -25,6 +25,7 @@ import com.netflix.spinnaker.gate.services.internal.ClouddriverService import com.netflix.spinnaker.gate.services.internal.ClouddriverService.Account import com.netflix.spinnaker.gate.services.internal.ClouddriverService.AccountDetails import com.netflix.spinnaker.kork.annotations.Alpha +import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall import com.netflix.spinnaker.security.User import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; @@ -101,7 +102,7 @@ class CredentialsController { @Parameter(description='Account name to start account definition listing from. Used for pagination.') @RequestParam(required = false) String startingAccountName ) { - clouddriverService.getAccountDefinitionsByType(accountType, limit, startingAccountName) + Retrofit2SyncCall.execute(clouddriverService.getAccountDefinitionsByType(accountType, limit, startingAccountName)) } @PostMapping @@ -111,7 +112,7 @@ class CredentialsController { @Parameter(description='Account definition body including a discriminator field named "type" with the account type.') @RequestBody ClouddriverService.AccountDefinition accountDefinition ) { - clouddriverService.createAccountDefinition(accountDefinition) + Retrofit2SyncCall.execute(clouddriverService.createAccountDefinition(accountDefinition)) } @PutMapping @@ -121,7 +122,7 @@ class CredentialsController { @Parameter(description='Account definition body including a discriminator field named "type" with the account type.') @RequestBody ClouddriverService.AccountDefinition accountDefinition ) { - clouddriverService.updateAccountDefinition(accountDefinition) + Retrofit2SyncCall.execute(clouddriverService.updateAccountDefinition(accountDefinition)) } @DeleteMapping('/{accountName}') @@ -131,6 +132,6 @@ class CredentialsController { @Parameter(description='Name of account definition to delete.') @PathVariable String accountName ) { - clouddriverService.deleteAccountDefinition(accountName) + Retrofit2SyncCall.execute(clouddriverService.deleteAccountDefinition(accountName)) } } diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/CredentialsControllerSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/CredentialsControllerSpec.groovy index 2205bde4ff..d58a7940b7 100644 --- a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/CredentialsControllerSpec.groovy +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/CredentialsControllerSpec.groovy @@ -28,6 +28,7 @@ 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.accept.ContentNegotiationManagerFactoryBean +import retrofit2.mock.Calls import spock.lang.Specification import spock.lang.Subject import spock.lang.Unroll @@ -91,7 +92,7 @@ class CredentialsControllerSpec extends Specification { then: response.status == 200 - 1 * clouddriverService.getAccountDefinitionsByType(accountType, _, _) + 1 * clouddriverService.getAccountDefinitionsByType(accountType, _, _) >> Calls.response(null) where: accountType | _ @@ -110,7 +111,7 @@ class CredentialsControllerSpec extends Specification { then: response.status == 200 - 1 * clouddriverService.getAccountDefinitionsByType(accountType, limit, startingAccountName) + 1 * clouddriverService.getAccountDefinitionsByType(accountType, limit, startingAccountName) >> Calls.response(null) where: accountType | limit | startingAccountName diff --git a/gate-web/src/test/java/com/netflix/spinnaker/gate/controllers/CredentialsControllerTest.java b/gate-web/src/test/java/com/netflix/spinnaker/gate/controllers/CredentialsControllerTest.java new file mode 100644 index 0000000000..c406f45bee --- /dev/null +++ b/gate-web/src/test/java/com/netflix/spinnaker/gate/controllers/CredentialsControllerTest.java @@ -0,0 +1,138 @@ +/* + * 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.controllers; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +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.REQUEST_ID; +import static com.netflix.spinnaker.kork.common.Header.USER; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +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.internal.ClouddriverService; +import java.util.List; +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.http.HttpStatus; +import org.springframework.http.MediaType; +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; + +@SpringBootTest(classes = Main.class) +@TestPropertySource( + properties = { + "spring.config.location=classpath:gate-test.yml", + "services.front50.applicationRefreshInitialDelayMs=3600000" + }) +public class CredentialsControllerTest { + + private static final String SUBMITTED_REQUEST_ID = "submitted-request-id"; + private static final String USERNAME = "some user"; + private static final String ACCOUNT = "my-account"; + private MockMvc webAppMockMvc; + + @RegisterExtension + static WireMockExtension wmClouddriver = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + + @Autowired private WebApplicationContext webApplicationContext; + + @Autowired ObjectMapper objectMapper; + + /** To prevent periodic calls to service's /health endpoints */ + @MockBean DownstreamServicesHealthIndicator downstreamServicesHealthIndicator; + + /** + * 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; + + @DynamicPropertySource + static void registerUrls(DynamicPropertyRegistry registry) { + // Configure wiremock's random ports into gate + System.out.println("wiremock clouddriver url: " + wmClouddriver.baseUrl()); + registry.add("services.clouddriver.base-url", wmClouddriver::baseUrl); + } + + @BeforeEach + void init(TestInfo testInfo) { + System.out.println("--------------- Test " + testInfo.getDisplayName()); + + webAppMockMvc = + webAppContextSetup(webApplicationContext) + .addFilters(filterRegistrationBean.getFilter()) + .build(); + + wmClouddriver.stubFor( + WireMock.get(urlEqualTo("/credentials?expand=true")) + .willReturn(aResponse().withStatus(HttpStatus.OK.value()).withBody("[{}]"))); + } + + @Test + void invokeAccountDefinitionsByType() throws Exception { + ClouddriverService.AccountDefinition accountDefinition = + new ClouddriverService.AccountDefinition(); + accountDefinition.setName("test"); + accountDefinition.setType("sometype"); + + String accountDefinitionJson = objectMapper.writeValueAsString(List.of(accountDefinition)); + + // mock clouddriver response + wmClouddriver.stubFor( + WireMock.get(urlPathEqualTo("/credentials/type/sometype")) + .willReturn( + aResponse().withStatus(HttpStatus.OK.value()).withBody(accountDefinitionJson))); + + webAppMockMvc + .perform( + get("/credentials/type/sometype") + .contentType(MediaType.APPLICATION_JSON_VALUE) + .header(USER.getHeader(), USERNAME) + .header(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID) + .header(ACCOUNTS.getHeader(), ACCOUNT)) + .andDo(print()) + .andExpect(status().is2xxSuccessful()) + .andExpect(content().string(accountDefinitionJson)) + .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID)); + } +}