fix(orca): Push service account permission message to deck (#3817)

* fix(orca): Push service account permission message to deck * fix(orca): use UserException set the right TERMINAL status Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Sep 2, 2020 · 9f781f9 · 9f781f9
1 parent 60f5dc8
commit 9f781f9
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 7 deletions.
diff --git a/...nt50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/SaveServiceAccountTask.java b/...nt50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/SaveServiceAccountTask.java
@@ -16,12 +16,15 @@
 
 package com.netflix.spinnaker.orca.front50.tasks;
 
+import static java.lang.String.format;
+
 import com.google.common.collect.ImmutableMap;
 import com.netflix.spinnaker.fiat.model.UserPermission;
 import com.netflix.spinnaker.fiat.model.resources.Role;
 import com.netflix.spinnaker.fiat.model.resources.ServiceAccount;
 import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator;
 import com.netflix.spinnaker.fiat.shared.FiatStatus;
+import com.netflix.spinnaker.kork.exceptions.UserException;
 import com.netflix.spinnaker.orca.api.pipeline.RetryableTask;
 import com.netflix.spinnaker.orca.api.pipeline.TaskResult;
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus;
@@ -128,9 +131,9 @@ public TaskResult execute(@Nonnull StageExecution stage) {
     }
 
     if (!isUserAuthorized(user, roles)) {
-      // TODO: Push this to the output result so Deck can show it.
       log.warn("User {} is not authorized with all roles for pipeline", user);
-      return TaskResult.ofStatus(ExecutionStatus.TERMINAL);
+      throw new UserException(
+          format("User '%s' is not authorized with all roles for pipeline", user));
     }
 
     ServiceAccount svcAcct = new ServiceAccount();

diff --git a/...rc/test/groovy/com/netflix/spinnaker/orca/front50/tasks/SaveServiceAccountTaskSpec.groovy b/...rc/test/groovy/com/netflix/spinnaker/orca/front50/tasks/SaveServiceAccountTaskSpec.groovy
@@ -143,19 +143,27 @@ class SaveServiceAccountTaskSpec extends Specification {
       ]
     }
 
+    def user = "abc@somedomain.io"
+    def message = ""
+    def exceptionMessage = "User '"+ user +"' is not authorized with all roles for pipeline"
+
     when:
-    stage.getExecution().setTrigger(new DefaultTrigger('manual', null, 'abc@somedomain.io'))
-    def result = task.execute(stage)
+    stage.getExecution().setTrigger(new DefaultTrigger('manual', null, user))
+
+    try {
+      task.execute(stage)
+    } catch (Exception e) {
+      message = e.message
+    }
 
     then:
-    1 * fiatPermissionEvaluator.getPermission('abc@somedomain.io') >> {
+    1 * fiatPermissionEvaluator.getPermission(user) >> {
       new UserPermission().addResources([new Role('foo')]).view
     }
 
     0 * front50Service.saveServiceAccount(_)
 
-    result.status == ExecutionStatus.TERMINAL
-    result.context == ImmutableMap.of()
+    message == exceptionMessage
   }
 
   def "should allow an admin to save pipelines"() {