From ae24d3f8eedfca992280961c5c69e845a838fbe1 Mon Sep 17 00:00:00 2001 From: Pranav-b-7 Date: Tue, 28 Nov 2023 11:17:21 +0530 Subject: [PATCH] refactor(front50): use SpinnakerRetrofitErrorHandler with Front50Service This PR lays the foundational work for upgrading the retrofit version to 2.x, specifically focusing on refactoring the exception handling for Front50Service. There are no behaviour changes detected with these code changes and hence no additional tests added to demonstrate the functionalities. Few existing tests are modified to align it with the new changes. --- .../applications/tasks/DeleteApplicationTask.groovy | 7 +++++-- .../spinnaker/orca/bakery/tasks/CreateBakeTask.groovy | 4 ++-- .../clouddriver/tasks/job/WaitOnJobCompletion.groovy | 4 ++-- .../pollers/EphemeralServerGroupsPoller.java | 9 ++++++--- .../orca/clouddriver/tasks/job/JobUtils.java | 4 ++-- .../orca/clouddriver/utils/TrafficGuard.java | 7 +++---- .../orca/clouddriver/utils/TrafficGuardSpec.groovy | 3 ++- orca-front50/orca-front50.gradle | 1 + .../orca/front50/config/Front50Configuration.groovy | 2 ++ .../orca/front50/tasks/AbstractFront50Task.groovy | 7 +++---- .../front50/pipeline/EnabledPipelineValidator.java | 4 ++-- .../orca/front50/tasks/DeleteDeliveryConfigTask.java | 7 +++---- .../orca/front50/tasks/MonitorFront50Task.java | 11 +++++------ .../orca/front50/tasks/UpsertDeliveryConfigTask.java | 7 +++---- .../pipeline/EnabledPipelineValidatorSpec.groovy | 5 +++-- orca-pipelinetemplate/orca-pipelinetemplate.gradle | 1 + .../loader/Front50SchemeLoaderSpec.groovy | 5 +++-- .../orca/controllers/OperationsController.groovy | 5 ++--- .../orca/controllers/ProjectController.groovy | 6 +++--- .../orca/controllers/ExecutionsImportController.java | 9 ++++++--- .../controllers/ExecutionsImportControllerSpec.groovy | 3 ++- 21 files changed, 61 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 ae0d991f39..9a2b1c7f03 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 @@ -76,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 e){ + log.error("Could not delete application permission", e) + return TaskResult.builder(ExecutionStatus.TERMINAL).outputs(outputs).build() } // delete Managed Delivery data if (keelService != null) { 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 f725aca9e0..14f6e08693 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 0cb5d18498..9eed43d583 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 054e76f27b..cd58ea5763 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 @@ -26,6 +26,8 @@ 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.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; @@ -49,7 +51,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import retrofit.RetrofitError; @Slf4j @Component @@ -277,11 +278,13 @@ private Map buildDestroyServerGroupOperation( private 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 00ab93492e..e570b31158 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 05508b66c9..e8228e6f55 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 1726cc90fd..29acb62699 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 7fcef96d25..4b476b74a8 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 6378359853..fb4b3e4a80 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 f674c3ddf0..6445cc6b2b 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,11 +84,10 @@ 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 } - throw e } } 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 b9f14d6146..5e9bf296d5 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 837068d90a..b94bbb5069 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 ab0362f4e0..5f84df53bf 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 3247b16c0e..375d39d8ee 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 fe09a6105d..f27ea9582d 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 @@ -181,12 +182,12 @@ class EnabledPipelineValidatorSpec extends Specification { } 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-pipelinetemplate/orca-pipelinetemplate.gradle b/orca-pipelinetemplate/orca-pipelinetemplate.gradle index 3f7ba0448c..ab515d0e5c 100644 --- a/orca-pipelinetemplate/orca-pipelinetemplate.gradle +++ b/orca-pipelinetemplate/orca-pipelinetemplate.gradle @@ -30,4 +30,5 @@ dependencies { testImplementation("org.spockframework:spock-unitils") testImplementation("org.codehaus.groovy:groovy-json") testImplementation("io.spinnaker.fiat:fiat-core:$fiatVersion") + testImplementation("io.spinnaker.kork:kork-retrofit") } diff --git a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/loader/Front50SchemeLoaderSpec.groovy b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/loader/Front50SchemeLoaderSpec.groovy index ec805007a8..438fc753b2 100644 --- a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/loader/Front50SchemeLoaderSpec.groovy +++ b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/loader/Front50SchemeLoaderSpec.groovy @@ -16,6 +16,7 @@ package com.netflix.spinnaker.orca.pipelinetemplate.loader import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException import com.netflix.spinnaker.orca.front50.Front50Service import com.netflix.spinnaker.orca.pipelinetemplate.exceptions.TemplateLoaderException import retrofit.RetrofitError @@ -47,10 +48,10 @@ class Front50SchemeLoaderSpec extends Specification { then: front50Service.getPipelineTemplate("myTemplateId") >> { - throw RetrofitError.networkError("http://front50/no-exist", new IOException("resource not found")) + throw new SpinnakerNetworkException(RetrofitError.networkError("http://front50/no-exist", new IOException("resource not found"))) } def e = thrown(TemplateLoaderException) - e.cause instanceof RetrofitError + e.cause instanceof SpinnakerNetworkException } void 'should load simple pipeline template'() { 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 1352c43fae..d4f48d7f95 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 @@ -46,7 +46,6 @@ import groovy.util.logging.Slf4j import javassist.NotFoundException import org.springframework.beans.factory.annotation.Autowired import org.springframework.web.bind.annotation.* -import retrofit.RetrofitError import retrofit.http.Query import javax.servlet.http.HttpServletResponse @@ -145,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 0bffa9e64f..c65c8dae49 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 c3e258e25d..3af82a1462 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 84cffc9c5e..4e59415e59 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,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 * _