fix(config): Allow dynamic config in pipeline initiator (#887)

Allow `PipelineInitiator` to read the `orca.enabled` property from dynamic config so it can be changed at runtime Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · May 7, 2020 · dbc0c3b · dbc0c3b
1 parent c622f0d
commit dbc0c3b
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 11 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
@@ -27,6 +27,7 @@
 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.kork.dynamicconfig.DynamicConfigService;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import com.netflix.spinnaker.security.User;
 import java.util.Collections;
@@ -55,13 +56,13 @@
 public class PipelineInitiator {
 
   private final Registry registry;
+  private final DynamicConfigService dynamicConfigService;
   private final OrcaService orca;
   private final FiatPermissionEvaluator fiatPermissionEvaluator;
   private final FiatStatus fiatStatus;
 
   private final ObjectMapper objectMapper;
   private final QuietPeriodIndicator quietPeriodIndicator;
-  private final boolean enabled;
   private final int retryCount;
   private final long retryDelayMillis;
   private final ExecutorService executorService;
@@ -75,7 +76,7 @@ public PipelineInitiator(
       @NonNull ExecutorService executorService,
       ObjectMapper objectMapper,
       @NonNull QuietPeriodIndicator quietPeriodIndicator,
-      @Value("${orca.enabled:true}") boolean enabled,
+      @NonNull DynamicConfigService dynamicConfigService,
       @Value("${orca.pipeline-initiator-retry-count:5}") int retryCount,
       @Value("${orca.pipeline-initiator-retry-delay-millis:5000}") long retryDelayMillis) {
     this.registry = registry;
@@ -84,15 +85,15 @@ public PipelineInitiator(
     this.fiatStatus = fiatStatus;
     this.objectMapper = objectMapper;
     this.quietPeriodIndicator = quietPeriodIndicator;
-    this.enabled = enabled;
+    this.dynamicConfigService = dynamicConfigService;
     this.retryCount = retryCount;
     this.retryDelayMillis = retryDelayMillis;
     this.executorService = executorService;
   }
 
   @PostConstruct
   public void initialize() {
-    if (!enabled) {
+    if (!isEnabled()) {
       log.warn("Orca triggering is disabled");
     }
   }
@@ -108,7 +109,7 @@ public void recordPipelineFailure(Pipeline pipeline) {
   }
 
   public void startPipeline(Pipeline pipeline, TriggerSource triggerSource) {
-    if (enabled) {
+    if (isEnabled()) {
       try {
         long now = System.currentTimeMillis();
         boolean inQuietPeriod = quietPeriodIndicator.inQuietPeriod(now);
@@ -378,6 +379,10 @@ private String getTriggerType(Pipeline pipeline) {
     return "N/A";
   }
 
+  private boolean isEnabled() {
+    return dynamicConfigService.isEnabled("orca", true);
+  }
+
   private static boolean isRetryableError(Throwable error) {
     if (!(error instanceof RetrofitError)) {
       return false;

diff --git a/...test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiatorSpec.groovy b/...test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/orca/PipelineInitiatorSpec.groovy
@@ -11,11 +11,9 @@ import com.netflix.spinnaker.fiat.model.UserPermission
 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.kork.dynamicconfig.DynamicConfigService
 import com.netflix.spinnaker.kork.web.context.AuthenticatedRequestContextProvider
 import com.netflix.spinnaker.kork.web.context.RequestContext
-import com.netflix.spinnaker.kork.web.context.RequestContextProvider
-import com.netflix.spinnaker.security.AuthenticatedRequest
-import org.slf4j.MDC
 import spock.lang.Specification
 import spock.lang.Unroll
 
@@ -24,6 +22,7 @@ import java.util.concurrent.TimeUnit
 
 class PipelineInitiatorSpec extends Specification {
   def registry = new NoopRegistry()
+  def noopDynamicConfigService = new DynamicConfigService.NoopDynamicConfig()
   def orca = Mock(OrcaService)
   def fiatPermissionEvaluator = Mock(FiatPermissionEvaluator)
   def fiatStatus = Mock(FiatStatus)
@@ -59,8 +58,9 @@ class PipelineInitiatorSpec extends Specification {
   @Unroll
   def "calls orca #expectedTriggerCalls times when enabled=#enabled flag"() {
     given:
+    def dynamicConfigService = Mock(DynamicConfigService)
     def pipelineInitiator = new PipelineInitiator(
-      registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, MoreExecutors.newDirectExecutorService(), objectMapper, quietPeriodIndicator, enabled, 5, 5000
+      registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, MoreExecutors.newDirectExecutorService(), objectMapper, quietPeriodIndicator, dynamicConfigService, 5, 5000
     )
 
     def pipeline = Pipeline
@@ -78,6 +78,7 @@ class PipelineInitiatorSpec extends Specification {
     pipelineInitiator.startPipeline(pipeline, PipelineInitiator.TriggerSource.SCHEDULER)
 
     then:
+    _ * dynamicConfigService.isEnabled("orca", true) >> { return enabled }
     _ * fiatStatus.isEnabled() >> { return enabled }
     _ * fiatStatus.isLegacyFallbackEnabled() >> { return legacyFallbackEnabled }
 
@@ -107,7 +108,7 @@ class PipelineInitiatorSpec extends Specification {
     RequestContext context = contextProvider.get()
     def executor = Executors.newFixedThreadPool(2)
     def pipelineInitiator = new PipelineInitiator(
-      registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, executor, objectMapper, quietPeriodIndicator, true, 5, 5000
+      registry, orca, Optional.of(fiatPermissionEvaluator), fiatStatus, executor, objectMapper, quietPeriodIndicator, noopDynamicConfigService, 5, 5000
     )
 
     Trigger trigger = (new Trigger.TriggerBuilder().type("cron").build()).atPropagateAuth(true)
@@ -151,7 +152,7 @@ class PipelineInitiatorSpec extends Specification {
   def "calls orca #expectedPlanCalls to plan pipeline if templated"() {
     given:
     def pipelineInitiator = new PipelineInitiator(
-      registry, orca, Optional.empty(), fiatStatus, MoreExecutors.newDirectExecutorService(), objectMapper, quietPeriodIndicator, true, 5, 5000
+      registry, orca, Optional.empty(), fiatStatus, MoreExecutors.newDirectExecutorService(), objectMapper, quietPeriodIndicator, noopDynamicConfigService, 5, 5000
     )
 
     def pipeline = Pipeline.builder()