fix(retrofit2): retrofit2 bug fixes (#1873)

* test(retrofit2): add tests to demonstrate the issue with missing retrofit2 related calls * fix(retrofit2): fix the missing calls related to retrofit2 and align the tests accordingly
spinnaker · Feb 21, 2025 · b0ee1b1 · b0ee1b1
1 parent 52982e9
commit b0ee1b1
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 5 deletions.
diff --git a/gate-oauth2/gate-oauth2.gradle b/gate-oauth2/gate-oauth2.gradle
@@ -8,4 +8,6 @@ dependencies {
   implementation "org.apache.groovy:groovy-json"
   implementation "org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure"
   implementation "org.springframework.session:spring-session-core"
+
+  testImplementation "com.squareup.retrofit2:retrofit-mock"
 }
diff --git a/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy b/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy
@@ -24,6 +24,7 @@ import com.netflix.spinnaker.gate.services.CredentialsService
 import com.netflix.spinnaker.gate.services.PermissionService
 import com.netflix.spinnaker.gate.services.internal.Front50Service
 import com.netflix.spinnaker.kork.core.RetrySupport
+import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.security.User
 import groovy.json.JsonSlurper
@@ -186,7 +187,7 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
       return false
     }
     try {
-      def serviceAccounts = front50Service.getServiceAccounts()
+      def serviceAccounts = Retrofit2SyncCall.execute(front50Service.getServiceAccounts())
       return serviceAccounts.find { email.equalsIgnoreCase(it.name) }
     } catch (SpinnakerServerException e) {
       log.warn("Could not get list of service accounts.", e)

diff --git a/...oovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy b/...oovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServicesSpec.groovy
@@ -16,6 +16,9 @@
 
 package com.netflix.spinnaker.gate.security.oauth2
 
+import com.netflix.spinnaker.gate.services.PermissionService
+import com.netflix.spinnaker.gate.services.internal.Front50Service
+import retrofit2.mock.Calls
 import spock.lang.Specification
 import spock.lang.Subject
 
@@ -107,4 +110,24 @@ class SpinnakerUserInfoTokenServicesSpec extends Specification {
     1                         || []
     [blergh: "blarg"]         || []
   }
+
+  def "verify SpinnakerUserInfoTokenServices#isServiceAccount"() {
+    given:
+    def permissionService = Mock(PermissionService){
+      1 * isEnabled() >> true
+    }
+    def front50Service = Mock(Front50Service){
+      1 * getServiceAccounts() >> Calls.response([['name':'ex@foo.com']])
+    }
+    def tokenServices = new SpinnakerUserInfoTokenServices(userInfoMapping: new OAuth2SsoConfig.UserInfoMapping(), permissionService: permissionService, front50Service: front50Service)
+    def details = [
+      client_email: 'ex@foo.com'
+    ]
+
+    when:
+    def isServiceAccount = tokenServices.isServiceAccount(details)
+
+    then:
+    isServiceAccount
+  }
 }
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
@@ -75,7 +75,7 @@ class PipelineController {
   @Operation(summary = "Delete a pipeline definition")
   @DeleteMapping("/{application}/{pipelineName:.+}")
   void deletePipeline(@PathVariable String application, @PathVariable String pipelineName) {
-    List<Map> pipelineConfigs = front50Service.getPipelineConfigsForApplication(application, null, true)
+    List<Map> pipelineConfigs = Retrofit2SyncCall.execute(front50Service.getPipelineConfigsForApplication(application, null, true))
     if (pipelineConfigs!=null && !pipelineConfigs.isEmpty()){
       Optional<Map> filterResult = pipelineConfigs.stream().filter({ pipeline -> ((String) pipeline.get("name")) != null && ((String) pipeline.get("name")).trim().equalsIgnoreCase(pipelineName) }).findFirst()
       if (filterResult.isPresent()){

diff --git a/...-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy b/...-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy
@@ -33,13 +33,18 @@ import spock.lang.Specification
 
 import java.nio.charset.StandardCharsets
 
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put
 
 class PipelineControllerSpec extends Specification {
 
-  def taskSerivce = Mock(TaskService)
-  def front50Service = Mock(Front50Service)
+  def taskSerivce = Mock(TaskService){
+    createAndWaitForCompletion(_) >> { [id: 'task-id', application: 'application', status: 'SUCCEEDED'] }
+  }
+  def front50Service = Mock(Front50Service){
+    getPipelineConfigsForApplication('application', null, true) >> Calls.response([['name': 'testpipeline', 'application': 'application']])
+  }
   def pipelineService = Mock(PipelineService)
   def pipelineControllerConfig = new PipelineControllerConfigProperties()
   def mockMvc = MockMvcBuilders
@@ -91,7 +96,28 @@ class PipelineControllerSpec extends Specification {
         ]
       ]
     ]) >> { [id: 'task-id', application: 'application', status: 'SUCCEEDED'] }
-    1 * front50Service.getPipelineConfigsForApplication('application', null, true) >> Calls.response([])
+  }
+
+  def "verify PipelineController#deletePipeline"() {
+    given:
+    def pipeline = [
+      id: "id",
+      name: "testpipeline",
+      stages: [],
+      triggers: [],
+      limitConcurrent: true,
+      parallel: true,
+      index: 4,
+      application: "application"
+    ]
+
+    when:
+    def response = mockMvc.perform(
+      delete("/pipelines/${pipeline.application}/${pipeline.name}").contentType(MediaType.APPLICATION_JSON)
+    ).andReturn().response
+
+    then:
+    notThrown(Exception)
   }
 
   def "should propagate pipeline template errors"() {