feat(api): FiatPermissionEvaluator reliablity (#254)

Adds retry support for calls to fiat in FiatPermissionEvaluator
Invalidates cached users if they contain legacyFallback permissions

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatClientConfigurationProperties.java

@@ -33,10 +33,20 @@ public class FiatClientConfigurationProperties {
   @NestedConfigurationProperty
   private PermissionsCache cache = new PermissionsCache();
 
+  @NestedConfigurationProperty
+  private RetryConfiguration retry = new RetryConfiguration();
+
   @Data
   class PermissionsCache {
     private Integer maxEntries = 1000;
 
     private Integer expiresAfterWriteSeconds = 20;
   }
+
+  @Data
+  class RetryConfiguration {
+    private long maxBackoffMillis = 10000;
+    private long initialBackoffMillis = 500;
+    private double retryMultiplier = 1.5;
+  }
 }

fiat-api/src/main/java/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluator.java

@@ -37,10 +37,13 @@
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
+import org.springframework.util.backoff.BackOffExecution;
+import org.springframework.util.backoff.ExponentialBackOff;
 
 import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -59,14 +62,73 @@ public class FiatPermissionEvaluator implements PermissionEvaluator {
 
   private final Id getPermissionCounterId;
 
+  private final RetryHandler retryHandler;
+
+
+  interface RetryHandler {
+    default <T> T retry(String description, Callable<T> callable) throws Exception {
+      return callable.call();
+    }
+
+    RetryHandler NOOP = new RetryHandler() {};
+  }
+
+  /**
+   * @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;
+    }
+
+    public <T> T retry(String description, Callable<T> callable) throws Exception {
+      ExponentialBackOff backOff = new ExponentialBackOff(initialBackoff, backoffMultiplier);
+      backOff.setMaxElapsedTime(maxBackoff);
+      BackOffExecution backOffExec = backOff.start();
+      while (true) {
+        try {
+          return callable.call();
+        } catch (Throwable e) {
+          long waitTime = backOffExec.nextBackOff();
+          if (waitTime == BackOffExecution.STOP) {
+            throw e;
+          }
+          log.warn(description + " failed. Retrying in " + waitTime + "ms", e);
+          TimeUnit.MILLISECONDS.sleep(waitTime);
+        }
+      }
+    }
+
+  }
+
   @Autowired
   public FiatPermissionEvaluator(Registry registry,
                                  FiatService fiatService,
                                  FiatClientConfigurationProperties configProps,
                                  FiatStatus fiatStatus) {
+    this(registry, fiatService, configProps, fiatStatus, buildRetryHandler(configProps));
+  }
+
+  private static RetryHandler buildRetryHandler(FiatClientConfigurationProperties fiatClientConfigurationProperties) {
+    FiatClientConfigurationProperties.RetryConfiguration retry = fiatClientConfigurationProperties.getRetry();
+    return new ExponentialBackoffRetryHandler(retry.getMaxBackoffMillis(), retry.getInitialBackoffMillis(), retry.getRetryMultiplier());
+  }
+
+  FiatPermissionEvaluator(Registry registry,
+                          FiatService fiatService,
+                          FiatClientConfigurationProperties configProps,
+                          FiatStatus fiatStatus,
+                          RetryHandler retryHandler) {
     this.registry = registry;
     this.fiatService = fiatService;
     this.fiatStatus = fiatStatus;
+    this.retryHandler = retryHandler;
 
     this.permissionsCache = CacheBuilder
         .newBuilder()
@@ -147,7 +209,7 @@ public UserPermission.View getPermission(String username) {
         cacheHit.set(false);
         return AuthenticatedRequest.propagate(() -> {
           try {
-            return fiatService.getUserPermission(username);
+            return retryHandler.retry("getUserPermission for " + username, () -> fiatService.getUserPermission(username));
           } catch (Exception e) {
             if (!fiatStatus.isLegacyFallbackEnabled()) {
               throw e;
@@ -176,22 +238,24 @@ public UserPermission.View getPermission(String username) {
       exception.set(e.getCause() != null ? e.getCause() : e);
     }
 
-    if (!successfulLookup.get()) {
-      log.error(
-          "Cannot get whole user permission for user {}, reason: {}",
-          username,
-          exception.get().getMessage()
-      );
-    }
-
     Id id = getPermissionCounterId
         .withTag("cached", cacheHit.get())
         .withTag("success", successfulLookup.get());
 
     if (!successfulLookup.get()) {
+      log.error(
+              "Cannot get whole user permission for user {}, reason: {}",
+              username,
+              exception.get().getMessage(),
+              exception
+      );
       id = id.withTag("legacyFallback", legacyFallback.get());
     }
 
+    if (legacyFallback.get()) {
+      permissionsCache.invalidate(username);
+    }
+
     registry.counter(id).increment();
 
     return view;

fiat-api/src/test/groovy/com/netflix/spinnaker/fiat/shared/FiatPermissionEvaluatorSpec.groovy

@@ -45,7 +45,8 @@ class FiatPermissionEvaluatorSpec extends Specification {
       registry,
       fiatService,
       buildConfigurationProperties(),
-      fiatStatus
+      fiatStatus,
+      FiatPermissionEvaluator.RetryHandler.NOOP
   )
 
   def cleanup() {
@@ -149,6 +150,26 @@ class FiatPermissionEvaluatorSpec extends Specification {
     hasPermission
   }
 
+  @Unroll
+  def "should retry fiat requests"() {
+    given:
+    FiatPermissionEvaluator evaluator = new FiatPermissionEvaluator(
+            registry,
+            fiatService,
+            buildConfigurationProperties(),
+            fiatStatus,
+            new FiatPermissionEvaluator.ExponentialBackoffRetryHandler(10, 15, 1)
+    )
+
+    when:
+    def view = evaluator.getPermission("testUser")
+
+    then:
+    2 * fiatService.getUserPermission("testUser") >> { throw new IllegalStateException("something something something")}
+
+    view == null
+  }
+
   @Unroll
   def "should support legacy fallback when fiat is unavailable"() {
     given: