From ee64ec5330b679e001acd80acd86c31feff7cdb9 Mon Sep 17 00:00:00 2001 From: Pranav-b-7 Date: Tue, 28 Nov 2023 11:17:21 +0530 Subject: [PATCH] refactor(front50): Enhance Exception Handling in Front50Service Retrofit Client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These changes lay the groundwork for a smooth and successful upgrade to retrofit2.x. The refactoring ensures that the retrofit client in 'Front50Service' is well-prepared for the upcoming version, and the accompanying test coverage guarantees the stability of the exception handling logic. Key Changes: 1. Error Handler Addition: - Integrated the ‘SpinnakerRetrofitErrorHandler’ as the error handler in the front50 retrofit configuration. This sets the stage for a seamless transition to retrofit2.x, ensuring a standardised approach to error handling. 2. Catch Block Replacement: - Replaced the catch block that previously caught ‘RetrofitError’ with catching ‘Spinnaker*Exception’. This aligns the exception handling logic with the new retrofit version, enhancing compatibility and maintainability. 3. Test Cases Coverage: - Covered test cases for each of the changes made, ensuring comprehensive testing of the refactored exception handling logic. This step is crucial to validate the correctness and reliability of the updated error handling. --- .../tasks/DeleteApplicationTask.groovy | 25 ++++++++++++++++--- .../orca/bakery/tasks/CreateBakeTask.groovy | 4 +-- .../tasks/job/WaitOnJobCompletion.groovy | 4 +-- .../pollers/EphemeralServerGroupsPoller.java | 9 ++++--- .../orca/clouddriver/tasks/job/JobUtils.java | 4 +-- .../orca/clouddriver/utils/TrafficGuard.java | 7 +++--- .../clouddriver/utils/TrafficGuardSpec.groovy | 3 ++- orca-front50/orca-front50.gradle | 1 + .../config/Front50Configuration.groovy | 2 ++ .../front50/tasks/AbstractFront50Task.groovy | 6 ++--- .../pipeline/EnabledPipelineValidator.java | 4 +-- .../tasks/DeleteDeliveryConfigTask.java | 7 +++--- .../front50/tasks/MonitorFront50Task.java | 11 ++++---- .../tasks/UpsertDeliveryConfigTask.java | 7 +++--- .../EnabledPipelineValidatorSpec.groovy | 7 +++--- orca-web/orca-web.gradle | 1 + .../controllers/OperationsController.groovy | 5 ++-- .../orca/controllers/ProjectController.groovy | 6 ++--- .../ExecutionsImportController.java | 9 ++++--- .../ExecutionsImportControllerSpec.groovy | 4 +-- 20 files changed, 76 insertions(+), 50 deletions(-) diff --git a/orca-applications/src/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy b/orca-applications/src/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy index f88800e15ce..9312f1b1737 100644 --- a/orca-applications/src/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy +++ b/orca-applications/src/main/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTask.groovy @@ -18,6 +18,8 @@ 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.kork.retrofit.exceptions.SpinnakerServerException import com.netflix.spinnaker.orca.api.pipeline.TaskResult import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus import com.netflix.spinnaker.orca.front50.Front50Service @@ -74,12 +76,15 @@ class DeleteApplicationTask extends AbstractFront50Task { front50Service.delete(application.name) try { front50Service.deletePermission(application.name) - } catch (RetrofitError re) { - if (re.response?.status == 404) { + } catch (SpinnakerHttpException re) { + if (re.responseCode == 404) { return TaskResult.SUCCEEDED } log.error("Could not delete application permission", re) return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() + } catch (SpinnakerServerException serverException){ + log.error("Could not delete application permission", serverException) + return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() } // delete Managed Delivery data if (keelService != null) { @@ -87,13 +92,27 @@ class DeleteApplicationTask extends AbstractFront50Task { keelService.deleteDeliveryConfig(application.name) } } - } catch (RetrofitError e) { + }/** + * @see KeelService#deleteDeliveryConfig(java.lang.String) is invoked in the above try block. + * Since it throws RetrofitError, the below catch block with RetrofitError is still relevant. + * */ + catch (RetrofitError e) { if (e.response?.status == 404) { return TaskResult.SUCCEEDED } log.error("Could not delete application", e) return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() } + catch (SpinnakerHttpException e){ + if (e.responseCode == 404) { + return TaskResult.SUCCEEDED + } + log.error("Could not delete application", e) + return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() + } catch (SpinnakerServerException e){ + log.error("Could not delete application", e) + return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() + } return TaskResult.builder(ExecutionStatus.SUCCEEDED).outputs(outputs).build() } 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 index f725aca9e05..14f6e086933 100644 --- 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/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/job/WaitOnJobCompletion.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/job/WaitOnJobCompletion.groovy index 0cb5d184988..9eed43d5836 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/job/WaitOnJobCompletion.groovy +++ b/orca-clouddriver/src/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/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java index 6781dce3a27..10e7e96181f 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java +++ b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java @@ -27,6 +27,8 @@ import com.netflix.spinnaker.kork.annotations.VisibleForTesting; 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.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType; import com.netflix.spinnaker.orca.clouddriver.CloudDriverService; import com.netflix.spinnaker.orca.clouddriver.config.PollerConfigurationProperties; @@ -50,7 +52,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Slf4j @Component @@ -279,11 +280,13 @@ private Map buildDestroyServerGroupOperation( Optional 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); + } catch (SpinnakerServerException e) { + throw new SystemException(format("Failed to retrieve application '%s'", applicationName), e); } } diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/job/JobUtils.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/job/JobUtils.java index 00ab93492ef..e570b311585 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/job/JobUtils.java +++ b/orca-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/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java index 05508b66c92..e8228e6f558 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java +++ b/orca-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/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy b/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy index 1726cc90fd9..29acb626991 100644 --- a/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy +++ b/orca-clouddriver/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-front50/orca-front50.gradle b/orca-front50/orca-front50.gradle index 7fcef96d252..4b476b74a88 100644 --- a/orca-front50/orca-front50.gradle +++ b/orca-front50/orca-front50.gradle @@ -30,6 +30,7 @@ dependencies { implementation("org.springframework.security:spring-security-config") implementation("org.springframework.security:spring-security-core") implementation("org.springframework.security:spring-security-web") + implementation("io.spinnaker.kork:kork-retrofit") compileOnly("org.projectlombok:lombok") annotationProcessor("org.projectlombok:lombok") diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 6378359853c..fb4b3e4a80d 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.jakewharton.retrofit.Ok3Client import com.netflix.spinnaker.config.DefaultServiceEndpoint import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler import com.netflix.spinnaker.orca.events.ExecutionEvent import com.netflix.spinnaker.orca.events.ExecutionListenerAdapter import com.netflix.spinnaker.orca.front50.Front50Service @@ -78,6 +79,7 @@ class Front50Configuration { .setLogLevel(retrofitLogLevel) .setLog(new RetrofitSlf4jLog(Front50Service)) .setConverter(new JacksonConverter(mapper)) + .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) .build() .create(Front50Service) } diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/AbstractFront50Task.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/AbstractFront50Task.groovy index f674c3ddf0e..0407f42c581 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/AbstractFront50Task.groovy +++ b/orca-front50/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/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java index b9f14d61469..5e9bf296d5f 100644 --- a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java +++ b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java @@ -18,6 +18,7 @@ import static java.lang.String.format; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution; import com.netflix.spinnaker.orca.api.pipeline.models.Trigger; import com.netflix.spinnaker.orca.front50.Front50Service; @@ -30,7 +31,6 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component public class EnabledPipelineValidator implements PipelineValidator { @@ -73,7 +73,7 @@ public void checkRunnable(PipelineExecution pipeline) { } return; - } catch (RetrofitError ignored) { + } catch (SpinnakerServerException ignored) { // treat a failure to fetch pipeline config history as non-fatal and fallback to the // previous behavior // (handles the fast property case where the supplied pipeline config id does _not_ diff --git a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/DeleteDeliveryConfigTask.java b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/DeleteDeliveryConfigTask.java index 837068d90a2..b94bbb50699 100644 --- a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/DeleteDeliveryConfigTask.java +++ b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/DeleteDeliveryConfigTask.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.orca.api.pipeline.Task; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution; @@ -14,7 +15,6 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component public class DeleteDeliveryConfigTask implements Task { @@ -62,10 +62,9 @@ public Optional getDeliveryConfig(String id) { try { DeliveryConfig deliveryConfig = front50Service.getDeliveryConfig(id); return Optional.of(deliveryConfig); - } catch (RetrofitError e) { + } catch (SpinnakerHttpException e) { // ignore an unknown (404) or unauthorized (403, 401) - if (e.getResponse() != null - && Arrays.asList(404, 403, 401).contains(e.getResponse().getStatus())) { + if (Arrays.asList(404, 403, 401).contains(e.getResponseCode())) { return Optional.empty(); } else { throw e; diff --git a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/MonitorFront50Task.java b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/MonitorFront50Task.java index ab0362f4e06..5f84df53bf0 100644 --- a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/MonitorFront50Task.java +++ b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/MonitorFront50Task.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.orca.api.pipeline.RetryableTask; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution; @@ -36,7 +37,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component public class MonitorFront50Task implements RetryableTask { @@ -162,8 +162,8 @@ private TaskResult monitor( private Optional> getPipeline(String id) { try { return Optional.of(front50Service.getPipeline(id)); - } catch (RetrofitError e) { - if (e.getResponse() != null && e.getResponse().getStatus() == HTTP_NOT_FOUND) { + } catch (SpinnakerHttpException e) { + if (e.getResponseCode() == HTTP_NOT_FOUND) { return Optional.empty(); } throw e; @@ -175,10 +175,9 @@ private Optional> getDeliveryConfig(String id) { try { DeliveryConfig deliveryConfig = front50Service.getDeliveryConfig(id); return Optional.of(objectMapper.convertValue(deliveryConfig, Map.class)); - } catch (RetrofitError e) { + } catch (SpinnakerHttpException e) { // ignore an unknown (404) or unauthorized (403, 401) - if (e.getResponse() != null - && Arrays.asList(404, 403, 401).contains(e.getResponse().getStatus())) { + if (Arrays.asList(404, 403, 401).contains(e.getResponseCode())) { return Optional.empty(); } else { throw e; diff --git a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/UpsertDeliveryConfigTask.java b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/UpsertDeliveryConfigTask.java index 3247b16c0ef..375d39d8ee4 100644 --- a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/UpsertDeliveryConfigTask.java +++ b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/tasks/UpsertDeliveryConfigTask.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException; import com.netflix.spinnaker.orca.api.pipeline.Task; import com.netflix.spinnaker.orca.api.pipeline.TaskResult; import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus; @@ -16,7 +17,6 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Component public class UpsertDeliveryConfigTask implements Task { @@ -65,9 +65,8 @@ private boolean configExists(String id) { try { front50Service.getDeliveryConfig(id); return true; - } catch (RetrofitError e) { - if (e.getResponse() != null - && Arrays.asList(404, 403, 401).contains(e.getResponse().getStatus())) { + } catch (SpinnakerHttpException e) { + if (Arrays.asList(404, 403, 401).contains(e.getResponseCode())) { return false; } else { throw e; diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy index fe09a6105d1..2659122d731 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy @@ -16,6 +16,7 @@ package com.netflix.spinnaker.orca.front50.pipeline +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException import com.netflix.spinnaker.orca.front50.Front50Service import com.netflix.spinnaker.orca.pipeline.PipelineValidator.PipelineValidationFailed import com.netflix.spinnaker.orca.pipeline.model.DefaultTrigger @@ -179,14 +180,12 @@ class EnabledPipelineValidatorSpec extends Specification { trigger = new DefaultTrigger("manual", null, "fzlem", [strategy: "kthxbye"]) } } - def notFoundError() { - RetrofitError.httpError( + new SpinnakerHttpException(RetrofitError.httpError( "http://localhost", new Response("http://localhost", HTTP_NOT_FOUND, "Not Found", [], null), null, null - ) + )) } - } diff --git a/orca-web/orca-web.gradle b/orca-web/orca-web.gradle index 82ad37be6cd..c9e6a4c1c52 100644 --- a/orca-web/orca-web.gradle +++ b/orca-web/orca-web.gradle @@ -63,6 +63,7 @@ dependencies { implementation("net.logstash.logback:logstash-logback-encoder") implementation("io.spinnaker.fiat:fiat-api:$fiatVersion") implementation("io.spinnaker.fiat:fiat-core:$fiatVersion") + implementation("io.spinnaker.kork:kork-retrofit") runtimeOnly("io.spinnaker.kork:kork-runtime") diff --git a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/OperationsController.groovy b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/OperationsController.groovy index c0a6cf9a646..08b89bf6e9a 100644 --- a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/OperationsController.groovy +++ b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/OperationsController.groovy @@ -23,6 +23,7 @@ import com.netflix.spinnaker.fiat.shared.FiatService import com.netflix.spinnaker.fiat.shared.FiatStatus import com.netflix.spinnaker.kork.exceptions.ConfigurationException import com.netflix.spinnaker.kork.exceptions.SpinnakerException +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution import com.netflix.spinnaker.orca.api.pipeline.models.Trigger import com.netflix.spinnaker.orca.clouddriver.service.JobService @@ -143,8 +144,8 @@ class OperationsController { Map pipelineConfig = AuthenticatedRequest.allowAnonymous({ front50Service.getPipeline(pipelineConfigId) }) pipelineConfig.trigger = trigger return pipelineConfig - } catch (RetrofitError e) { - if (e.response?.status == HTTP_NOT_FOUND) { + } catch (SpinnakerHttpException e) { + if (e.responseCode == HTTP_NOT_FOUND) { throw new NotFoundException("Pipeline config $pipelineConfigId not found") } throw e diff --git a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/ProjectController.groovy b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/ProjectController.groovy index 0bffa9e64f7..c65c8dae493 100644 --- a/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/ProjectController.groovy +++ b/orca-web/src/main/groovy/com/netflix/spinnaker/orca/controllers/ProjectController.groovy @@ -17,13 +17,13 @@ package com.netflix.spinnaker.orca.controllers +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution import com.netflix.spinnaker.orca.front50.Front50Service import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository import org.springframework.beans.factory.annotation.Autowired import org.springframework.web.bind.annotation.* -import retrofit.RetrofitError import rx.schedulers.Schedulers @RestController @@ -49,8 +49,8 @@ class ProjectController { try { def project = front50Service.getProject(projectId) pipelineConfigIds = project.config.pipelineConfigs*.pipelineConfigId - } catch (RetrofitError e) { - if (e.response?.status == 404) { + } catch (SpinnakerHttpException e) { + if (e.responseCode == 404) { return [] } diff --git a/orca-web/src/main/java/com/netflix/spinnaker/orca/controllers/ExecutionsImportController.java b/orca-web/src/main/java/com/netflix/spinnaker/orca/controllers/ExecutionsImportController.java index c3e258e25d7..3af82a1462c 100644 --- a/orca-web/src/main/java/com/netflix/spinnaker/orca/controllers/ExecutionsImportController.java +++ b/orca-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/orca-web/src/test/groovy/com/netflix/spinnaker/orca/controllers/ExecutionsImportControllerSpec.groovy b/orca-web/src/test/groovy/com/netflix/spinnaker/orca/controllers/ExecutionsImportControllerSpec.groovy index 84cffc9c5eb..bd6eee83668 100644 --- a/orca-web/src/test/groovy/com/netflix/spinnaker/orca/controllers/ExecutionsImportControllerSpec.groovy +++ b/orca-web/src/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,12 +98,11 @@ 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 * _ result.executionId == executionId result.executionStatus == ExecutionStatus.SUCCEEDED } - }