chore(dependencies): Autobump korkVersion (#1843)

* chore(dependencies): Autobump korkVersion * feat(retrofit): replace RetrofitError references with SpinnakerServerException and its subclasses. Update the tests accordingly. * feat(retrofit): more classes to have RetrofitError replaced with SpinnakerServerException and its subclasses * fix(retrofit): minor changes --------- Co-authored-by: root <root@c407cd7c4eac> Co-authored-by: kirangodishala <kiran@opsmx.io>
spinnaker · Nov 5, 2024 · e5c4c74 · e5c4c74
1 parent d497e5e
commit e5c4c74
Show file tree

Hide file tree

Showing 32 changed files with 175 additions and 273 deletions.
diff --git a/gate-core/src/main/java/com/netflix/spinnaker/gate/retrofit/UpstreamBadRequest.java b/gate-core/src/main/java/com/netflix/spinnaker/gate/retrofit/UpstreamBadRequest.java
@@ -17,27 +17,17 @@
 package com.netflix.spinnaker.gate.retrofit;
 
 import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
-import static retrofit.RetrofitError.Kind.HTTP;
 
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
-import java.util.Collection;
-import retrofit.RetrofitError;
 
 public class UpstreamBadRequest extends SpinnakerException {
 
   private final int status;
   private final String url;
   private final Object error;
 
-  private UpstreamBadRequest(RetrofitError cause) {
-    super(cause.getMessage(), cause);
-    status = cause.getResponse().getStatus();
-    url = cause.getUrl();
-    error = cause.getBody();
-  }
-
   private UpstreamBadRequest(SpinnakerHttpException cause) {
     super(cause.getMessage(), cause);
     this.setRetryable(cause.getRetryable());
@@ -58,25 +48,6 @@ public Object getError() {
     return error;
   }
 
-  public static RuntimeException classifyError(RetrofitError error) {
-    if (error.getKind() == HTTP
-        && error.getResponse().getStatus() < INTERNAL_SERVER_ERROR.value()) {
-      return new UpstreamBadRequest(error);
-    } else {
-      return error;
-    }
-  }
-
-  public static RuntimeException classifyError(
-      RetrofitError error, Collection<Integer> supportedHttpStatuses) {
-    if (error.getKind() == HTTP
-        && supportedHttpStatuses.contains(error.getResponse().getStatus())) {
-      return new UpstreamBadRequest(error);
-    } else {
-      return error;
-    }
-  }
-
   public static RuntimeException classifyError(SpinnakerServerException error) {
     if (error instanceof SpinnakerHttpException
         && ((SpinnakerHttpException) error).getResponseCode() < INTERNAL_SERVER_ERROR.value()) {

diff --git a/gate-iap/gate-iap.gradle b/gate-iap/gate-iap.gradle
@@ -2,6 +2,8 @@ dependencies {
   implementation project(":gate-core")
   implementation 'com.nimbusds:nimbus-jose-jwt'
   implementation "com.github.ben-manes.caffeine:guava"
+  implementation "io.spinnaker.kork:kork-exceptions"
+  implementation "io.spinnaker.kork:kork-retrofit"
   implementation "io.spinnaker.kork:kork-security"
   implementation "io.spinnaker.fiat:fiat-api:$fiatVersion"
   implementation "io.spinnaker.fiat:fiat-core:$fiatVersion"

diff --git a/gate-iap/src/main/java/com/netflix/spinnaker/gate/security/iap/IapAuthenticationFilter.java b/gate-iap/src/main/java/com/netflix/spinnaker/gate/security/iap/IapAuthenticationFilter.java
@@ -21,6 +21,7 @@
 import com.netflix.spinnaker.gate.security.iap.IapSsoConfig.IapSecurityConfigProperties;
 import com.netflix.spinnaker.gate.services.PermissionService;
 import com.netflix.spinnaker.gate.services.internal.Front50Service;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import com.netflix.spinnaker.security.User;
 import com.nimbusds.jose.JWSHeader;
 import com.nimbusds.jose.JWSVerifier;
@@ -50,7 +51,6 @@
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
 import org.springframework.web.filter.OncePerRequestFilter;
-import retrofit.RetrofitError;
 
 /**
  * * This filter verifies the request header from Cloud IAP containing a JWT token, after the user
@@ -140,8 +140,8 @@ private boolean isServiceAccount(String email) {
         }
       }
       return false;
-    } catch (RetrofitError re) {
-      log.warn("Could not get list of service accounts.", re);
+    } catch (SpinnakerServerException e) {
+      log.warn("Could not get list of service accounts.", e);
     }
     return false;
   }

diff --git a/gate-oauth2/gate-oauth2.gradle b/gate-oauth2/gate-oauth2.gradle
@@ -3,6 +3,7 @@ dependencies {
   implementation "com.netflix.spectator:spectator-api"
   implementation "io.spinnaker.fiat:fiat-api:$fiatVersion"
   implementation "io.spinnaker.kork:kork-exceptions"
+  implementation "io.spinnaker.kork:kork-retrofit"
   implementation "io.spinnaker.kork:kork-security"
   implementation "org.apache.groovy:groovy-json"
   implementation "org.springframework.security.oauth.boot:spring-security-oauth2-autoconfigure"

diff --git a/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy b/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy
@@ -24,6 +24,7 @@ import com.netflix.spinnaker.gate.services.CredentialsService
 import com.netflix.spinnaker.gate.services.PermissionService
 import com.netflix.spinnaker.gate.services.internal.Front50Service
 import com.netflix.spinnaker.kork.core.RetrySupport
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.security.User
 import groovy.json.JsonSlurper
 import groovy.util.logging.Slf4j
@@ -39,7 +40,6 @@ import org.springframework.security.oauth2.provider.OAuth2Authentication
 import org.springframework.security.oauth2.provider.OAuth2Request
 import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken
-import retrofit.RetrofitError
 
 import java.util.function.BiFunction
 import java.util.regex.Pattern
@@ -188,8 +188,8 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
     try {
       def serviceAccounts = front50Service.getServiceAccounts()
       return serviceAccounts.find { email.equalsIgnoreCase(it.name) }
-    } catch (RetrofitError re) {
-      log.warn("Could not get list of service accounts.", re)
+    } catch (SpinnakerServerException e) {
+      log.warn("Could not get list of service accounts.", e)
     }
     return false
   }

diff --git a/gate-plugins/gate-plugins.gradle b/gate-plugins/gate-plugins.gradle
@@ -25,6 +25,7 @@ dependencies {
   implementation "io.spinnaker.fiat:fiat-core:$fiatVersion"
   implementation "io.spinnaker.fiat:fiat-api:$fiatVersion"
   implementation "io.spinnaker.kork:kork-plugins"
+  implementation "io.spinnaker.kork:kork-retrofit"
   implementation "io.spinnaker.kork:kork-web"
 
   implementation "io.swagger:swagger-annotations"

diff --git a/...ain/kotlin/com/netflix/spinnaker/gate/plugins/web/installed/PluginsInstalledController.kt b/...ain/kotlin/com/netflix/spinnaker/gate/plugins/web/installed/PluginsInstalledController.kt
@@ -13,6 +13,8 @@ import com.netflix.spinnaker.gate.services.internal.SwabbieService
 import com.netflix.spinnaker.kork.plugins.SpinnakerPluginDescriptor
 import com.netflix.spinnaker.kork.plugins.SpinnakerPluginManager
 import com.netflix.spinnaker.kork.plugins.update.SpinnakerUpdateManager
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import io.swagger.annotations.ApiOperation
 import java.util.stream.Collectors
 import org.pf4j.PluginWrapper
@@ -22,7 +24,6 @@ import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.RestController
-import retrofit.RetrofitError
 
 @RestController
 @RequestMapping("/plugins/installed")
@@ -122,8 +123,12 @@ class PluginsInstalledController(
   fun callService(call: () -> List<SpinnakerPluginDescriptor>): List<SpinnakerPluginDescriptor> {
     return try {
       call()
-    } catch (e: RetrofitError) {
-      log.warn("Unable to retrieve installed plugins from '${e.response?.url}' due to '${e.response?.status}'")
+    } catch (e: SpinnakerServerException) {
+      var status: Int? = null
+      if (e is SpinnakerHttpException){
+        status = e.responseCode
+      }
+      log.warn("Unable to retrieve installed plugins from '${e.url}' due to '${status}'")
       return emptyList()
     }
   }

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/GateWebConfig.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/GateWebConfig.groovy
@@ -43,7 +43,6 @@ import org.springframework.web.servlet.config.annotation.ContentNegotiationConfi
 import org.springframework.web.servlet.config.annotation.InterceptorRegistry
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurer
 import org.springframework.web.servlet.handler.HandlerMappingIntrospector
-import retrofit.RetrofitError
 
 import javax.servlet.Filter
 import javax.servlet.http.HttpServletResponse
@@ -111,16 +110,6 @@ public class GateWebConfig implements WebMvcConfigurer {
 
       def message = exception.message
       def failureCause = exception.cause
-      if (failureCause instanceof RetrofitError) {
-        try {
-          def retrofitBody = failureCause.getBodyAs(Map) as Map
-          message = retrofitBody.error ?: message
-        } catch (Exception ignored) {
-          // unable to extract error from response
-        }
-
-        failureCause = failureCause.cause ?: failureCause
-      }
 
       return [
         failureCause: failureCause.toString(),

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BakeController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/BakeController.groovy
@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.gate.controllers
 
 import com.netflix.spinnaker.gate.services.BakeService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import io.swagger.annotations.ApiOperation
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.http.HttpStatus
@@ -26,7 +27,6 @@ import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestController
-import retrofit.RetrofitError
 
 @RequestMapping("/bakery")
 @RestController
@@ -57,7 +57,7 @@ class BakeController {
   @ExceptionHandler
   @ResponseStatus(HttpStatus.NOT_FOUND)
   Map handleBakeOptionsException(Exception e) {
-    def errorMsg = e instanceof RetrofitError && e.getUrl().contains("/logs/") ? "logs.not.found" : "bake.options.not.found"
+    def errorMsg = e instanceof SpinnakerServerException && e.getUrl().contains("/logs/") ? "logs.not.found" : "bake.options.not.found"
 
     return [error: errorMsg, status: HttpStatus.NOT_FOUND, message: e.message]
   }

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ManagedController.java b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ManagedController.java
@@ -14,6 +14,8 @@
 import com.netflix.spinnaker.gate.model.manageddelivery.RetryVerificationRequest;
 import com.netflix.spinnaker.gate.services.NotificationService;
 import com.netflix.spinnaker.gate.services.internal.KeelService;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import com.netflix.spinnaker.kork.web.interceptors.Criticality;
 import groovy.util.logging.Slf4j;
 import io.github.resilience4j.retry.RetryConfig;
@@ -45,7 +47,6 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
-import retrofit.RetrofitError;
 import retrofit.client.Header;
 import retrofit.client.Response;
 
@@ -91,7 +92,7 @@ private void configureRetry() {
         RetryConfig.custom()
             .maxAttempts(5)
             .waitDuration(Duration.ofSeconds(30))
-            .retryExceptions(RetrofitError.class)
+            .retryExceptions(SpinnakerServerException.class)
             .build());
   }
 
@@ -220,18 +221,13 @@ DeliveryConfig deleteManifest(@PathVariable("name") String name) {
   ResponseEntity<Map> validateManifest(@RequestBody DeliveryConfig manifest) {
     try {
       return ResponseEntity.ok(keelService.validateManifest(manifest));
-    } catch (RetrofitError e) {
-      if (e.getResponse().getStatus() == 400) {
-        try {
-          return ResponseEntity.badRequest()
-              .body(objectMapper.readValue(e.getResponse().getBody().in(), Map.class));
-        } catch (Exception ex) {
-          log.error("Error parsing error response from keel", ex);
-          return ResponseEntity.badRequest().body(Collections.emptyMap());
-        }
-      } else {
-        throw e;
+    } catch (SpinnakerHttpException e) {
+      if (e.getResponseCode() == 400) {
+        Map<String, Object> responseBody = e.getResponseBody();
+        responseBody = responseBody == null ? Collections.emptyMap() : responseBody;
+        return ResponseEntity.badRequest().body(responseBody);
       }
+      throw e;
     }
   }
 

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PagerDutyController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PagerDutyController.groovy
@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.gate.controllers
 
 import com.netflix.spinnaker.gate.services.PagerDutyService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
@@ -26,7 +27,6 @@ import org.springframework.scheduling.annotation.Scheduled
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RestController
-import retrofit.RetrofitError
 
 import java.util.concurrent.atomic.AtomicReference
 
@@ -80,7 +80,7 @@ class PagerDutyController {
       log.info("Fetched {} PagerDuty services", services?.size())
       pagerDutyServicesCache.set(services)
     } catch (e) {
-      if (e instanceof RetrofitError && e.response?.status == 429) {
+      if (e instanceof SpinnakerHttpException && e.responseCode == 429) {
         log.warn("Unable to refresh PagerDuty service list (throttled!)")
       } else {
         log.error("Unable to refresh PagerDuty service list", e)
@@ -92,7 +92,7 @@ class PagerDutyController {
       log.info("Fetched {} PagerDuty onCall", onCalls?.size())
       pagerDutyOnCallCache.set(onCalls)
     } catch (e) {
-      if (e instanceof RetrofitError && e.response?.status == 429) {
+      if (e instanceof SpinnakerHttpException && e.responseCode == 429) {
         log.warn("Unable to refresh PagerDuty onCall list (throttled!)")
       } else {
         log.error("Unable to refresh PagerDuty onCall list", e)