removed RetrofitError for the get applications API of Front50Service

spinnaker · Apr 2, 2024 · ecb13dd · ecb13dd
1 parent e084f18
commit ecb13dd
Show file tree

Hide file tree

Showing 12 changed files with 71 additions and 23 deletions.
diff --git a/...rc/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy b/...rc/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy
@@ -95,8 +95,9 @@ class DeleteApplicationTask extends AbstractFront50Task {
       }
       log.error("Could not delete application", e)
       return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build()
-    } catch (SpinnakerHttpException httpException){
-      if (httpException.responseCode == 404) {
+    }
+    catch (SpinnakerHttpException e){
+      if (e.responseCode == 404) {
         return TaskResult.SUCCEEDED
       }
       log.error("Could not delete application", httpException)

diff --git a/...est/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTaskSpec.groovy b/...est/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTaskSpec.groovy
@@ -18,11 +18,15 @@ package com.netflix.spinnaker.orca.applications.tasks
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.front50.Front50Service
 import com.netflix.spinnaker.orca.front50.model.Application
 import com.netflix.spinnaker.orca.KeelService
+import org.springframework.http.HttpStatus
+import retrofit.RetrofitError
 import retrofit.client.Response
+import retrofit.converter.JacksonConverter
 import spock.lang.Specification
 import spock.lang.Subject
 
@@ -105,4 +109,42 @@ class DeleteApplicationTaskSpec extends Specification {
     then:
     taskResult.status == ExecutionStatus.SUCCEEDED
   }
+
+  void "should handle SpinnakerHttpException and ignore the exception if the response code is 404 while fetching applications"() {
+    given:
+    task.front50Service = Mock(Front50Service) {
+      get(config.application.name) >> {
+        def url = "https://front50service.com/v2/applications/"+config.application.name
+        Response response = new Response(url, HttpStatus.NOT_FOUND.value(), HttpStatus.NOT_FOUND.name(), [], null)
+        RetrofitError httpError = RetrofitError.httpError(url, response, new JacksonConverter(), null)
+        throw new SpinnakerHttpException(httpError)
+      }
+    }
+
+    when:
+    def taskResult = task.execute(pipeline.stages.first())
+
+    then:
+    taskResult.status == ExecutionStatus.SUCCEEDED
+
+  }
+
+  void "should handle SpinnakerHttpException if the response code is not 404 while fetching applications"() {
+    given:
+    task.front50Service = Mock(Front50Service) {
+      get(config.application.name) >> {
+        def url = "https://front50service.com/v2/applications/"+config.application.name
+        Response response = new Response(url, HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.name(), [], null)
+        RetrofitError httpError = RetrofitError.httpError(url, response, new JacksonConverter(), null)
+        throw new SpinnakerHttpException(httpError)
+      }
+    }
+
+    when:
+    def taskResult = task.execute(pipeline.stages.first())
+
+    then:
+    taskResult.status == ExecutionStatus.TERMINAL
+
+  }
 }
diff --git a/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTask.groovy b/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTask.groovy
@@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.kork.artifacts.model.Artifact
 import com.netflix.spinnaker.kork.core.RetrySupport
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
@@ -41,7 +42,6 @@ import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
-import retrofit.RetrofitError
 
 
 import static com.netflix.spinnaker.kork.web.selector.v2.SelectableService.*
@@ -86,7 +86,7 @@ class CreateBakeTask implements RetryableTask {
           stage.context.user = user
         }
       }
-    } catch (RetrofitError e) {
+    } catch (SpinnakerServerException e) {
       // ignore exception, we will just use the owner passed to us
       if (!e.message.contains("404")) {
         log.warn("Error retrieving application {} from front50, ignoring.", stage.execution.application, e)

diff --git a/...c/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/job/WaitOnJobCompletion.groovy b/...c/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/job/WaitOnJobCompletion.groovy
@@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.frigga.Names
 import com.netflix.spinnaker.kork.core.RetrySupport
 import com.netflix.spinnaker.kork.exceptions.ConfigurationException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.OverridableTimeoutRetryableTask
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
@@ -34,7 +35,6 @@ import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
 import org.springframework.stereotype.Component
-import retrofit.RetrofitError
 
 import javax.annotation.Nonnull
 import javax.annotation.Nullable
@@ -200,7 +200,7 @@ public class WaitOnJobCompletion implements CloudProviderAware, OverridableTimeo
     }
     try {
       return front50Service.get(appName) != null
-    } catch (RetrofitError e) {
+    } catch (SpinnakerServerException e) {
       throw e
     }
   }

diff --git a/...main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java b/...main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java
@@ -26,6 +26,7 @@
 import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.kork.exceptions.SystemException;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.clouddriver.config.PollerConfigurationProperties;
@@ -49,7 +50,6 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
-import retrofit.RetrofitError;
 
 @Slf4j
 @Component
@@ -277,8 +277,8 @@ private Map<String, Object> buildDestroyServerGroupOperation(
   private Optional<Application> getApplication(String applicationName) {
     try {
       return Optional.of(front50Service.get(applicationName));
-    } catch (RetrofitError e) {
-      if (e.getResponse().getStatus() == HttpStatus.NOT_FOUND.value()) {
+    } catch (SpinnakerHttpException e) {
+      if (e.getResponseCode() == HttpStatus.NOT_FOUND.value()) {
         return Optional.empty();
       }
       throw new SystemException(format("Failed to retrieve application '%s'", applicationName), e);

diff --git a/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/job/JobUtils.java b/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/job/JobUtils.java
@@ -19,6 +19,7 @@
 
 import com.netflix.frigga.Names;
 import com.netflix.spinnaker.kork.core.RetrySupport;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import com.netflix.spinnaker.moniker.Moniker;
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.clouddriver.KatoRestService;
@@ -30,7 +31,6 @@
 import java.util.Optional;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import retrofit.RetrofitError;
 
 @Component
 public class JobUtils implements CloudProviderAware {
@@ -95,7 +95,7 @@ private Boolean applicationExists(String appName) {
     }
     try {
       return front50Service.get(appName) != null;
-    } catch (RetrofitError e) {
+    } catch (SpinnakerServerException e) {
       throw e;
     }
   }

diff --git a/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java b/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java
@@ -23,6 +23,7 @@
 import com.netflix.spectator.api.Registry;
 import com.netflix.spectator.impl.Preconditions;
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
 import com.netflix.spinnaker.moniker.Moniker;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.clouddriver.model.Cluster;
@@ -39,7 +40,6 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import retrofit.RetrofitError;
 
 @Component
 public class TrafficGuard {
@@ -466,10 +466,9 @@ public boolean hasDisableLock(Moniker clusterMoniker, String account, Location l
     Application application;
     try {
       application = front50Service.get(clusterMoniker.getApp());
-    } catch (RetrofitError e) {
+    } catch (SpinnakerHttpException e) {
       // ignore an unknown (404) or unauthorized (403) application
-      if (e.getResponse() != null
-          && Arrays.asList(404, 403).contains(e.getResponse().getStatus())) {
+      if (Arrays.asList(404, 403).contains(e.getResponseCode())) {
         application = null;
       } else {
         throw e;

diff --git a/...iver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy b/...iver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy
@@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spectator.api.NoopRegistry
 import com.netflix.spectator.api.Registry
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.moniker.Moniker
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.clouddriver.ModelUtils
@@ -462,7 +463,7 @@ class TrafficGuardSpec extends Specification {
     !applicationDetails.containsKey("trafficGuards")
     result == false
     1 * front50Service.get("app") >> {
-      throw new RetrofitError(null, null, new Response("http://stash.com", 404, "test reason", [], null), null, null, null, null)
+      throw new SpinnakerHttpException(new RetrofitError(null, null, new Response("http://stash.com", 404, "test reason", [], null), null, null, null, null))
     }
   }
 

diff --git a/orca-core/orca-core.gradle b/orca-core/orca-core.gradle
@@ -56,6 +56,7 @@ dependencies {
   implementation("org.codehaus.groovy:groovy")
   implementation("net.javacrumbs.shedlock:shedlock-spring:4.44.0")
   implementation("net.javacrumbs.shedlock:shedlock-provider-jdbc-template:4.44.0")
+  implementation("io.spinnaker.kork:kork-retrofit")
 
   compileOnly("org.projectlombok:lombok")
   annotationProcessor("org.projectlombok:lombok")

diff --git a/...ont50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/AbstractFront50Task.groovy b/...ont50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/AbstractFront50Task.groovy
@@ -19,13 +19,13 @@ package com.netflix.spinnaker.orca.front50.tasks
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult
 import com.netflix.spinnaker.orca.front50.Front50Service
 import com.netflix.spinnaker.orca.front50.model.Application
 import org.springframework.lang.Nullable
-import retrofit.RetrofitError
 
 import javax.annotation.Nonnull
 import java.util.concurrent.TimeUnit
@@ -84,8 +84,8 @@ abstract class AbstractFront50Task implements RetryableTask {
   Application fetchApplication(String applicationName) {
     try {
       return front50Service.get(applicationName)
-    } catch (RetrofitError e) {
-      if (e.response?.status == 404) {
+    } catch (SpinnakerHttpException e) {
+      if (e.responseCode == 404) {
         return null
       }
 

diff --git a/...-web/src/main/java/com/netflix/spinnaker/orca/controllers/ExecutionsImportController.java b/...-web/src/main/java/com/netflix/spinnaker/orca/controllers/ExecutionsImportController.java
@@ -16,6 +16,8 @@
 
 package com.netflix.spinnaker.orca.controllers;
 
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException;
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus;
 import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution;
@@ -37,7 +39,6 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
-import retrofit.RetrofitError;
 
 @RestController
 @RequestMapping("/admin/executions")
@@ -70,10 +71,12 @@ ExecutionImportResponse createExecution(@RequestBody PipelineExecution execution
     try {
       application = front50Service.get(execution.getApplication());
       log.info("Importing application with name: {}", application.name);
-    } catch (RetrofitError e) {
-      if (e.getKind() == RetrofitError.Kind.HTTP && e.getResponse().getStatus() != 404) {
+    } catch (SpinnakerHttpException e) {
+      if (e.getResponseCode() != 404) {
         log.warn("Exception received while retrieving application from front50", e);
       }
+    } catch (SpinnakerServerException e) {
+      // ignore the exception
     }
 
     if (application == null) {

diff --git a/.../test/groovy/com/netflix/spinnaker/orca/controllers/ExecutionsImportControllerSpec.groovy b/.../test/groovy/com/netflix/spinnaker/orca/controllers/ExecutionsImportControllerSpec.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.orca.controllers
 
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType
@@ -97,7 +98,7 @@ class ExecutionsImportControllerSpec extends Specification {
 
     then:
     noExceptionThrown()
-    1 * front50Service.get('testapp') >> { throw RetrofitError.unexpectedError('http://test.front50.com', new RuntimeException())}
+    1 * front50Service.get('testapp') >> { throw new SpinnakerServerException(RetrofitError.unexpectedError('http://test.front50.com', new RuntimeException()))}
     1 * executionRepository.retrieve(ExecutionType.PIPELINE, executionId) >> { throw new ExecutionNotFoundException('No execution')}
     1 * executionRepository.store(execution)
     0 * _