From ecb13dd91a4d81191ca2847a66509fdcb1d0cca0 Mon Sep 17 00:00:00 2001 From: Pranav-b-7 Date: Tue, 5 Dec 2023 12:04:05 +0530 Subject: [PATCH] removed RetrofitError for the get applications API of Front50Service --- .../tasks/DeleteApplicationTask.groovy | 5 ++- .../tasks/DeleteApplicationTaskSpec.groovy | 42 +++++++++++++++++++ .../orca/bakery/tasks/CreateBakeTask.groovy | 4 +- .../tasks/job/WaitOnJobCompletion.groovy | 4 +- .../pollers/EphemeralServerGroupsPoller.java | 6 +-- .../orca/clouddriver/tasks/job/JobUtils.java | 4 +- .../orca/clouddriver/utils/TrafficGuard.java | 7 ++-- .../clouddriver/utils/TrafficGuardSpec.groovy | 3 +- orca-core/orca-core.gradle | 1 + .../front50/tasks/AbstractFront50Task.groovy | 6 +-- .../ExecutionsImportController.java | 9 ++-- .../ExecutionsImportControllerSpec.groovy | 3 +- 12 files changed, 71 insertions(+), 23 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..67f643c063 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 @@ -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/orca-applications/src/test/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTaskSpec.groovy b/orca-applications/src/test/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTaskSpec.groovy index 225365803b..5826f04af2 100644 --- a/orca-applications/src/test/groovy/com/netflix/spinnaker/orca/applications/tasks/DeleteApplicationTaskSpec.groovy +++ b/orca-applications/src/test/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 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..a90630fb82 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,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 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); 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-core/orca-core.gradle b/orca-core/orca-core.gradle index 1608c5e2ac..dd9c9821e8 100644 --- 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/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..0407f42c58 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-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 * _