From ed2db48acc59c2ae4238994f6b82d50a1b699018 Mon Sep 17 00:00:00 2001 From: Nicolas Cohen Date: Wed, 22 May 2019 11:01:34 -0700 Subject: [PATCH] fix(apps): Clear resource provider caches to ensure up-to-date data (#409) --- .../fiat/permissions/DefaultPermissionsResolver.java | 7 +++++++ .../spinnaker/fiat/permissions/PermissionsResolver.java | 3 +++ .../com/netflix/spinnaker/fiat/providers/BaseProvider.java | 4 ++++ .../netflix/spinnaker/fiat/providers/ResourceProvider.java | 2 ++ .../com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java | 3 +++ 5 files changed, 19 insertions(+) diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java index 6e1d737b5..094913ec7 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/DefaultPermissionsResolver.java @@ -91,6 +91,13 @@ public UserPermission resolveAndMerge(@NonNull ExternalUser user) { return getUserPermission(user.getId(), combo); } + @Override + public void clearCache() { + for (ResourceProvider provider : resourceProviders) { + provider.clearCache(); + } + } + private boolean resolveAdminRole(Set roles) { List adminRoles = fiatAdminConfig.getAdmin().getRoles(); return roles.stream().map(Role::getName).anyMatch(adminRoles::contains); diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/PermissionsResolver.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/PermissionsResolver.java index 4b25a3e65..5ab5b2f36 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/PermissionsResolver.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/permissions/PermissionsResolver.java @@ -36,4 +36,7 @@ public interface PermissionsResolver { /** Resolves multiple user's permissions. Returned map is keyed by userId. */ Map resolve(Collection users) throws PermissionResolutionException; + + /** Clears resource cache: apps, service accounts,... */ + void clearCache(); } diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseProvider.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseProvider.java index 1535118f0..adada3e5d 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseProvider.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/BaseProvider.java @@ -86,5 +86,9 @@ private Cache> buildCache(int expireAfterWrite) { .build(); } + public void clearCache() { + cache.invalidate(CACHE_KEY); + } + protected abstract Set loadAll() throws ProviderException; } diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/ResourceProvider.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/ResourceProvider.java index 2ab790603..a86c6bda7 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/ResourceProvider.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/providers/ResourceProvider.java @@ -27,4 +27,6 @@ public interface ResourceProvider { Set getAllRestricted(Set roles, boolean isAdmin) throws ProviderException; Set getAllUnrestricted() throws ProviderException; + + void clearCache(); } diff --git a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java index 1e42a3f7c..b18c0bac8 100644 --- a/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java +++ b/fiat-roles/src/main/java/com/netflix/spinnaker/fiat/roles/UserRolesSyncer.java @@ -157,6 +157,9 @@ public long syncAndReturn(List roles) { + "resolution may not complete until this server becomes healthy again."); } + // Ensure we're going to reload app and service account definitions + permissionsResolver.clearCache(); + while (true) { try { Map combo = new HashMap<>();