fix(auth): propagate MDC across the thread boundary for pipeline exec…

…ution (#560) previous change (#555) moved orca invocation to an executor thread without propagating MDC context.
spinnaker · May 24, 2019 · d2a4076 · d2a4076
1 parent bf2f281
commit d2a4076
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 2 deletions.
diff --git a/...ers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiator.java b/...ers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiator.java
@@ -33,6 +33,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.stream.Collectors;
@@ -166,10 +167,13 @@ public void startPipeline(Pipeline pipeline, TriggerSource triggerSource) {
 
   private void triggerPipeline(Pipeline pipeline, TriggerSource triggerSource)
       throws RejectedExecutionException {
-    executorService.submit(() -> triggerPipelineImpl(pipeline, triggerSource));
+    Callable<Void> triggerWithCapturedContext =
+        AuthenticatedRequest.propagate(() -> triggerPipelineImpl(pipeline, triggerSource));
+
+    executorService.submit(triggerWithCapturedContext);
   }
 
-  private void triggerPipelineImpl(Pipeline pipeline, TriggerSource triggerSource) {
+  private Void triggerPipelineImpl(Pipeline pipeline, TriggerSource triggerSource) {
     try {
       TriggerResponse response;
 
@@ -231,6 +235,8 @@ private void triggerPipelineImpl(Pipeline pipeline, TriggerSource triggerSource)
 
       logOrcaErrorMetric(e.getClass().getName(), triggerSource.name(), getTriggerType(pipeline));
     }
+
+    return null;
   }
 
   private TriggerResponse triggerWithRetries(Pipeline pipeline) {

diff --git a/...test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiatorSpec.groovy b/...test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiatorSpec.groovy
@@ -12,9 +12,13 @@ import com.netflix.spinnaker.fiat.model.resources.Account
 import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator
 import com.netflix.spinnaker.fiat.shared.FiatStatus
 import com.netflix.spinnaker.security.AuthenticatedRequest
+import org.slf4j.MDC
 import spock.lang.Specification
 import spock.lang.Unroll
 
+import java.util.concurrent.Executors
+import java.util.concurrent.TimeUnit
+
 class PipelineInitiatorSpec extends Specification {
   def registry = new NoopRegistry()
   def orca = Mock(OrcaService)
@@ -94,6 +98,51 @@ class PipelineInitiatorSpec extends Specification {
     null            | true    | true                  || 1                    || "anonymous"           || "account2,account3"           // null trigger user should default to 'anonymous'
   }
 
+  def "propages auth headers to orca calls without runAs"() {
+    given:
+    def executor = Executors.newFixedThreadPool(2)
+    def pipelineInitiator = new PipelineInitiator(
+      registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, executor, objectMapper, quietPeriodIndicator, true, 5, 5000
+    )
+
+    Trigger trigger = (new Trigger.TriggerBuilder().type("cron").build()).atPropagateAuth(true)
+
+    Pipeline pipeline = Pipeline
+      .builder()
+      .application("application")
+      .name("name")
+      .id("id")
+      .type("pipeline")
+      .trigger(trigger)
+      .build()
+
+    def user = "super-duper-user"
+    def account = "super-duper-account"
+
+    when:
+    MDC.put(AuthenticatedRequest.Header.USER.header, user)
+    MDC.put(AuthenticatedRequest.Header.ACCOUNTS.header, account)
+    pipelineInitiator.startPipeline(pipeline, PipelineInitiator.TriggerSource.SCHEDULER)
+    MDC.remove(AuthenticatedRequest.Header.ACCOUNTS.header)
+    MDC.remove(AuthenticatedRequest.Header.USER.header)
+
+    // Wait for the trigger to actually be invoked (happens on separate thread)
+    executor.shutdown()
+    executor.awaitTermination(2, TimeUnit.SECONDS)
+
+    then:
+    _ * fiatStatus.isEnabled() >> { return enabled }
+    _ * fiatStatus.isLegacyFallbackEnabled() >> { return false }
+
+    1 * orca.trigger(pipeline) >> {
+      captureAuthorizationContext()
+      return new OrcaService.TriggerResponse()
+    }
+
+    capturedSpinnakerUser.orElse(null) == user
+    capturedSpinnakerAccounts.orElse(null) == account
+  }
+
   @Unroll
   def "calls orca #expectedPlanCalls to plan pipeline if templated"() {
     given: