fix(metrics-logs): Record permissionsCache caffeine metrics with Caff…

…eineStatsCounter and add debug log when access denied to accounts (#528) Debugging an intermittent access denied error to accounts. CaffeineStatsCounter pushes cache metrics to the registry and the debug log might give us a bit more insight
spinnaker · Dec 11, 2019 · a40265e · a40265e
1 parent fb124cd
commit a40265e
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 2 deletions.
diff --git a/fiat-api/fiat-api.gradle b/fiat-api/fiat-api.gradle
@@ -24,6 +24,7 @@ dependencies {
   implementation "com.netflix.spectator:spectator-api"
   implementation "com.netflix.spinnaker.kork:kork-core"
   implementation "com.netflix.spinnaker.kork:kork-security"
+  implementation "com.netflix.spinnaker.kork:kork-telemetry"
   implementation "com.netflix.spinnaker.kork:kork-web"
   implementation "com.squareup.retrofit:retrofit"
   implementation "com.squareup.retrofit:converter-jackson"

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
@@ -26,11 +26,13 @@
 import com.netflix.spinnaker.fiat.model.resources.Authorizable;
 import com.netflix.spinnaker.fiat.model.resources.ResourceType;
 import com.netflix.spinnaker.kork.exceptions.IntegrationException;
+import com.netflix.spinnaker.kork.telemetry.caffeine.CaffeineStatsCounter;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import com.netflix.spinnaker.security.User;
 import java.io.Serializable;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -137,7 +139,7 @@ private static RetryHandler buildRetryHandler(
             .maximumSize(configProps.getCache().getMaxEntries())
             .expireAfterWrite(
                 configProps.getCache().getExpiresAfterWriteSeconds(), TimeUnit.SECONDS)
-            .recordStats()
+            .recordStats(() -> new CaffeineStatsCounter(registry, "fiat.permissionsCache"))
             .build();
 
     this.getPermissionCounterId = registry.createId("fiat.getPermission");
@@ -394,7 +396,24 @@ private boolean permissionContains(
 
     switch (resourceType) {
       case ACCOUNT:
-        return containsAuth.apply(permission.getAccounts());
+        boolean authorized = containsAuth.apply(permission.getAccounts());
+
+        // Todo(jonsie): Debug transitory access denied issue, remove when not necessary
+        if (!authorized) {
+          Map<String, Set<Authorization>> accounts =
+              permission.getAccounts().stream()
+                  .collect(
+                      Collectors.toMap(Account.View::getName, Account.View::getAuthorizations));
+
+          log.debug(
+              "Authorization={} denied to account={} for user permission={}, found={}",
+              authorization.toString(),
+              resourceName,
+              permission.getName(),
+              accounts.toString());
+        }
+
+        return authorized;
       case APPLICATION:
         boolean applicationHasPermissions =
             permission.getApplications().stream()