feat(api): Support for dynamic configuration of the Fiat retry polici…

…es (#379)
spinnaker · Apr 18, 2019 · 81cb1f0 · 81cb1f0
1 parent 38a59e2
commit 81cb1f0
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 14 deletions.
diff --git a/...pi/src/main/java/com/netflix/spinnaker/fiat/shared/FiatClientConfigurationProperties.java b/...pi/src/main/java/com/netflix/spinnaker/fiat/shared/FiatClientConfigurationProperties.java
@@ -16,13 +16,17 @@
 
 package com.netflix.spinnaker.fiat.shared;
 
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService;
 import lombok.Data;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.NestedConfigurationProperty;
 
 @Data
 @ConfigurationProperties("services.fiat")
 public class FiatClientConfigurationProperties {
+  @Autowired
+  private DynamicConfigService dynamicConfigService;
 
   private boolean enabled;
 
@@ -38,6 +42,11 @@ public class FiatClientConfigurationProperties {
   @NestedConfigurationProperty
   private RetryConfiguration retry = new RetryConfiguration();
 
+  public RetryConfiguration getRetry() {
+    retry.setDynamicConfigService(dynamicConfigService);
+    return retry;
+  }
+
   @Data
   class PermissionsCache {
     private Integer maxEntries = 1000;
@@ -47,8 +56,38 @@ class PermissionsCache {
 
   @Data
   class RetryConfiguration {
+    private DynamicConfigService dynamicConfigService;
+
     private long maxBackoffMillis = 10000;
     private long initialBackoffMillis = 500;
     private double retryMultiplier = 1.5;
+
+    public void setDynamicConfigService(DynamicConfigService dynamicConfigService) {
+      this.dynamicConfigService = dynamicConfigService;
+    }
+
+    public long getMaxBackoffMillis() {
+      if (dynamicConfigService == null) {
+        return maxBackoffMillis;
+      }
+
+      return dynamicConfigService.getConfig(Long.class, "fiat.retry.maxBackoffMills", maxBackoffMillis);
+    }
+
+    public long getInitialBackoffMillis() {
+      if (dynamicConfigService == null) {
+        return initialBackoffMillis;
+      }
+
+      return dynamicConfigService.getConfig(Long.class, "fiat.retry.initialBackoffMillis", initialBackoffMillis);
+    }
+
+    public double getRetryMultiplier() {
+      if (dynamicConfigService == null) {
+        return retryMultiplier;
+      }
+
+      return dynamicConfigService.getConfig(Double.class, "fiat.retry.retryMultiplier", retryMultiplier);
+    }
   }
 }
diff --git a/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java b/fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java
@@ -80,19 +80,18 @@ default <T> T retry(String description, Callable<T> callable) throws Exception {
    * @see ExponentialBackOff
    */
   static class ExponentialBackoffRetryHandler implements RetryHandler {
-    private final long maxBackoff;
-    private final long initialBackoff;
-    private final double backoffMultiplier;
-
-    public ExponentialBackoffRetryHandler(long maxBackoff, long initialBackoff, double backoffMultiplier) {
-      this.maxBackoff = maxBackoff;
-      this.initialBackoff = initialBackoff;
-      this.backoffMultiplier = backoffMultiplier;
+    private final FiatClientConfigurationProperties.RetryConfiguration retryConfiguration;
+
+    public ExponentialBackoffRetryHandler(FiatClientConfigurationProperties.RetryConfiguration retryConfiguration) {
+      this.retryConfiguration = retryConfiguration;
     }
 
     public <T> T retry(String description, Callable<T> callable) throws Exception {
-      ExponentialBackOff backOff = new ExponentialBackOff(initialBackoff, backoffMultiplier);
-      backOff.setMaxElapsedTime(maxBackoff);
+      ExponentialBackOff backOff = new ExponentialBackOff(
+          retryConfiguration.getInitialBackoffMillis(),
+          retryConfiguration.getRetryMultiplier()
+      );
+      backOff.setMaxElapsedTime(retryConfiguration.getMaxBackoffMillis());
       BackOffExecution backOffExec = backOff.start();
       while (true) {
         try {
@@ -107,7 +106,6 @@ public <T> T retry(String description, Callable<T> callable) throws Exception {
         }
       }
     }
-
   }
 
   @Autowired
@@ -119,8 +117,7 @@ public FiatPermissionEvaluator(Registry registry,
   }
 
   private static RetryHandler buildRetryHandler(FiatClientConfigurationProperties fiatClientConfigurationProperties) {
-    FiatClientConfigurationProperties.RetryConfiguration retry = fiatClientConfigurationProperties.getRetry();
-    return new ExponentialBackoffRetryHandler(retry.getMaxBackoffMillis(), retry.getInitialBackoffMillis(), retry.getRetryMultiplier());
+    return new ExponentialBackoffRetryHandler(fiatClientConfigurationProperties.getRetry());
   }
 
   FiatPermissionEvaluator(Registry registry,

diff --git a/...-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy b/...-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy
@@ -148,12 +148,18 @@ class FiatPermissionEvaluatorSpec extends Specification {
   @Unroll
   def "should retry fiat requests"() {
     given:
+    def retryConfiguration = new FiatClientConfigurationProperties.RetryConfiguration()
+    retryConfiguration.setMaxBackoffMillis(10)
+    retryConfiguration.setInitialBackoffMillis(15)
+    retryConfiguration.setRetryMultiplier(1.5)
+
+    and:
     FiatPermissionEvaluator evaluator = new FiatPermissionEvaluator(
             registry,
             fiatService,
             buildConfigurationProperties(),
             fiatStatus,
-            new FiatPermissionEvaluator.ExponentialBackoffRetryHandler(10, 15, 1)
+            new FiatPermissionEvaluator.ExponentialBackoffRetryHandler(retryConfiguration)
     )
 
     when: