fix(front50): Handle failures in pipeline config history lookup (#3004)

spinnaker · Jun 21, 2019 · 5b5464e · 5b5464e
1 parent 9823ade
commit 5b5464e
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/...src/main/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java b/...src/main/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java
@@ -51,13 +51,13 @@ public void checkRunnable(Execution pipeline) {
     }
 
     if (!isStrategy(pipeline)) {
-      // attempt an optimized lookup via pipeline history vs fetching all pipelines for the
-      // application and filtering
-      List<Map<String, Object>> pipelineConfigHistory =
-          front50Service.getPipelineHistory(pipeline.getPipelineConfigId(), 1);
+      try {
+        // attempt an optimized lookup via pipeline history vs fetching all pipelines for the
+        // application and filtering
+        List<Map<String, Object>> pipelineConfigHistory =
+            front50Service.getPipelineHistory(pipeline.getPipelineConfigId(), 1);
 
-      if (!pipelineConfigHistory.isEmpty()) {
-        try {
+        if (!pipelineConfigHistory.isEmpty()) {
           Map<String, Object> pipelineConfig = pipelineConfigHistory.get(0);
           if ((boolean) pipelineConfig.getOrDefault("disabled", false)) {
             throw new PipelineIsDisabled(
@@ -67,12 +67,12 @@ public void checkRunnable(Execution pipeline) {
           }
 
           return;
-        } catch (RetrofitError 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_
-          // actually exist)
         }
+      } catch (RetrofitError 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_
+        // actually exist)
       }
     }
 

diff --git a/...st/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy b/...st/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy
@@ -20,9 +20,12 @@ import com.netflix.spinnaker.orca.front50.Front50Service
 import com.netflix.spinnaker.orca.pipeline.PipelineValidator.PipelineValidationFailed
 import com.netflix.spinnaker.orca.pipeline.model.DefaultTrigger
 import com.netflix.spinnaker.orca.pipeline.model.PipelineTrigger
+import retrofit.RetrofitError
+import retrofit.client.Response
 import spock.lang.Specification
 import spock.lang.Subject
 import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND
 
 class EnabledPipelineValidatorSpec extends Specification {
 
@@ -65,6 +68,25 @@ class EnabledPipelineValidatorSpec extends Specification {
     when:
     validator.checkRunnable(execution)
 
+    then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> {
+      throw RetrofitError.httpError(
+          "http://localhost",
+          new Response("http://localhost", HTTP_NOT_FOUND, "Not Found", [], null),
+          null,
+          null
+      )
+    }
+    1 * front50Service.getPipelines(execution.application, false) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
+    ]
+    0 * _
+
+    notThrown(PipelineValidationFailed)
+
+    when:
+    validator.checkRunnable(execution)
+
     then:
     1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
         [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]