fix(web): invoke pipeline config exception handling

gate-web/gate-web.gradle

@@ -32,6 +32,7 @@ dependencies {
   implementation "io.spinnaker.kork:kork-core"
   implementation "io.spinnaker.kork:kork-config"
   implementation "io.spinnaker.kork:kork-plugins"
+  implementation "io.spinnaker.kork:kork-retrofit"
   implementation "io.spinnaker.kork:kork-web"
   implementation "com.netflix.frigga:frigga"
   implementation "redis.clients:jedis"
@@ -78,7 +79,6 @@ dependencies {
   testImplementation "io.spinnaker.kork:kork-jedis-test"
   testImplementation "io.spinnaker.kork:kork-test"
   testImplementation "org.codehaus.groovy:groovy-json"
-  testRuntimeOnly "io.spinnaker.kork:kork-retrofit"
 
   // Add each included authz provider as a runtime dependency
   gradle.includedProviderProjects.each {

gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy

@@ -23,7 +23,9 @@ import com.netflix.spinnaker.gate.services.PipelineService
 import com.netflix.spinnaker.gate.services.TaskService
 import com.netflix.spinnaker.gate.services.internal.Front50Service
 import com.netflix.spinnaker.kork.exceptions.HasAdditionalAttributes
-import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException
+import com.netflix.spinnaker.kork.exceptions.SpinnakerException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import groovy.transform.CompileDynamic
@@ -51,20 +53,44 @@ import static net.logstash.logback.argument.StructuredArguments.value
 @RestController
 @RequestMapping("/pipelines")
 class PipelineController {
-  @Autowired
-  PipelineService pipelineService
-
-  @Autowired
-  TaskService taskService
+  final PipelineService pipelineService
+  final TaskService taskService
+  final Front50Service front50Service
+  final ObjectMapper objectMapper
+  final PipelineControllerConfigProperties pipelineControllerConfigProperties
+
+  /**
+   * Adjusting the front50Service and other retrofit objects for communicating
+   * with downstream services means changing RetrofitServiceFactory in kork and
+   * so it affects more than gate.  Front50 uses that code to communicate with
+   * echo.  Front50 doesn't currently do any special exception handling when it
+   * calls echo.  Gate does a ton though, and so it would be a big change to
+   * adjust all the catching of RetrofitError into catching
+   * SpinnakerHttpException, etc. as appropriate.
+   *
+   * Even if RetrofitServiceFactory were configurable by service type, so only
+   * gate's Front50Service and OrcaService used SpinnakerRetrofitErrorHandler,
+   * it would still be a big change, affecting gate-iap and gate-oauth2 where
+   * there's code that uses front50Service but checks for RetrofitError.
+   *
+   * To limit the scope of the change to invokePipelineConfig, construct a
+   * spinnakerRetrofitErrorHandler and use it directly.
+   */
+  final SpinnakerRetrofitErrorHandler spinnakerRetrofitErrorHandler
 
   @Autowired
-  Front50Service front50Service
-
-  @Autowired
-  ObjectMapper objectMapper
-
-  @Autowired
-  PipelineControllerConfigProperties pipelineControllerConfigProperties
+  PipelineController(PipelineService pipelineService,
+                     TaskService taskService,
+                     Front50Service front50Service,
+                     ObjectMapper objectMapper,
+                     PipelineControllerConfigProperties pipelineControllerConfigProperties) {
+    this.pipelineService = pipelineService
+    this.taskService = taskService
+    this.front50Service = front50Service
+    this.objectMapper = objectMapper
+    this.pipelineControllerConfigProperties = pipelineControllerConfigProperties
+    this.spinnakerRetrofitErrorHandler = SpinnakerRetrofitErrorHandler.newInstance()
+  }
 
   @CompileDynamic
   @ApiOperation(value = "Delete a pipeline definition")
@@ -300,15 +326,30 @@ class PipelineController {
     AuthenticatedRequest.setApplication(application)
     try {
       pipelineService.trigger(application, pipelineNameOrId, trigger)
-    } catch (NotFoundException e) {
-      throw e
-    } catch (e) {
-      log.error("Unable to trigger pipeline (application: {}, pipelineId: {})",
-        value("application", application), value("pipelineId", pipelineNameOrId), e)
-      throw new PipelineExecutionException(e.message)
+    } catch (RetrofitError e) {
+      // If spinnakerRetrofitErrorHandler were registered as a "real" error handler, the code here would look something like
+      //
+      // } catch (SpinnakerHttpException e) {
+      //   throw new SpinnakerHttpException(triggerFailureMessage(application, pipelineNameOrId, e), e)
+      // } catch (SpinnakerException e) {
+      //   throw new SpinnakerException(triggerFailureMessage(application, pipelineNameOrId, e), e)
+      // }
+      Throwable throwable = spinnakerRetrofitErrorHandler.handleError(e)
+      if (throwable instanceof SpinnakerHttpException) {
+        throw new SpinnakerHttpException(triggerFailureMessage(application, pipelineNameOrId, throwable), throwable)
+      }
+      if (throwable instanceof SpinnakerException) {
+        throw new SpinnakerException(triggerFailureMessage(application, pipelineNameOrId, throwable), throwable)
+      }
+      throw throwable
     }
   }
 
+  private String triggerFailureMessage(String application, String pipelineNameOrId, Throwable e) {
+    String.format("Unable to trigger pipeline (application: %s, pipelineId: %s). Error: %s",
+        value("application", application), value("pipelineId", pipelineNameOrId), e.getMessage())
+  }
+
   @ApiOperation(value = "Trigger a pipeline execution", response = Map.class)
   @PreAuthorize("hasPermission(#application, 'APPLICATION', 'EXECUTE')")
   @PostMapping("/v2/{application}/{pipelineNameOrId:.+}")
@@ -429,12 +470,4 @@ class PipelineController {
       this.additionalAttributes = additionalAttributes
     }
   }
-
-  @ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY)
-  @InheritConstructors
-  class PipelineExecutionException extends InvalidRequestException {
-    PipelineExecutionException(String message) {
-      super(message)
-    }
-  }
 }

gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy

@@ -73,7 +73,8 @@ class ApplicationService {
     this.allApplicationsCache
   }
 
-  @Scheduled(fixedDelayString = '${services.front50.applicationRefreshIntervalMs:5000}')
+  @Scheduled(fixedDelayString = '${services.front50.applicationRefreshIntervalMs:5000}',
+             initialDelayString = '${services.front50.applicationRefreshInitialDelayMs:}')
   void refreshApplicationsCache() {
     try {
       allApplicationsCache.set(tick(true))

gate-web/src/test/groovy/com/netflix/spinnaker/gate/FunctionalSpec.groovy

@@ -259,8 +259,12 @@ class FunctionalSpec extends Specification {
    }
 
     @Bean
-    PipelineController pipelineController() {
-      new PipelineController()
+    PipelineController pipelineController(PipelineService pipelineService,
+                                          TaskService taskService,
+                                          Front50Service front50Service,
+                                          ObjectMapper objectMapper,
+                                          PipelineControllerConfigProperties pipelineControllerConfigProperties) {
+      new PipelineController(pipelineService, taskService, front50Service, objectMapper, pipelineControllerConfigProperties)
     }
 
     @Bean

gate-web/src/test/groovy/com/netflix/spinnaker/gate/controllers/PipelineControllerSpec.groovy

@@ -18,6 +18,7 @@ package com.netflix.spinnaker.gate.controllers
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.gate.config.controllers.PipelineControllerConfigProperties
+import com.netflix.spinnaker.gate.services.PipelineService
 import com.netflix.spinnaker.gate.services.TaskService
 import com.netflix.spinnaker.gate.services.internal.Front50Service
 import groovy.json.JsonSlurper
@@ -39,12 +40,14 @@ class PipelineControllerSpec extends Specification {
 
   def taskSerivce = Mock(TaskService)
   def front50Service = Mock(Front50Service)
+  def pipelineService = Mock(PipelineService)
   def pipelineControllerConfig = new PipelineControllerConfigProperties()
   def mockMvc = MockMvcBuilders
-    .standaloneSetup(new PipelineController(objectMapper: new ObjectMapper(),
-      taskService: taskSerivce,
-      front50Service: front50Service,
-      pipelineControllerConfigProperties: pipelineControllerConfig))
+    .standaloneSetup(new PipelineController(pipelineService,
+                                            taskSerivce,
+                                            front50Service,
+                                            new ObjectMapper(),
+                                            pipelineControllerConfig))
     .build()
 
   def "should update a pipeline"() {