fix(cloudfoundry): add configurable cache expiry for clients (#5508)

* fix(cloudfoundry): add configurable cache expiry for Applications client and make Routes cache expiration configurable

* fix(cloudfoundry): merge conflict

(cherry picked from commit d12b5f0)

# Conflicts:
#	clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java

@@ -35,6 +35,7 @@
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.*;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.Package;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.Process;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.config.CloudFoundryConfigurationProperties;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.*;
 import com.netflix.spinnaker.clouddriver.helpers.AbstractServerGroupNameResolver;
 import com.netflix.spinnaker.clouddriver.model.HealthState;
@@ -43,6 +44,7 @@
 import java.util.*;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -74,7 +76,8 @@ public Applications(
       Processes processes,
       Integer resultsPerPage,
       boolean onlySpinnakerManaged,
-      ForkJoinPool forkJoinPool) {
+      ForkJoinPool forkJoinPool,
+      CloudFoundryConfigurationProperties.LocalCacheConfig localCacheConfig) {
     this.account = account;
     this.appsManagerUri = appsManagerUri;
     this.metricsUri = metricsUri;
@@ -84,7 +87,19 @@ public Applications(
     this.resultsPerPage = resultsPerPage;
     this.onlySpinnakerManaged = onlySpinnakerManaged;
     this.forkJoinPool = forkJoinPool;
+
+    CacheBuilder<Object, Object> builder = CacheBuilder.newBuilder();
+    if (localCacheConfig.getApplicationsAccessExpirySeconds() >= 0) {
+      builder.expireAfterAccess(
+          localCacheConfig.getApplicationsAccessExpirySeconds(), TimeUnit.SECONDS);
+    }
+    if (localCacheConfig.getApplicationsWriteExpirySeconds() >= 0) {
+      builder.expireAfterWrite(
+          localCacheConfig.getApplicationsWriteExpirySeconds(), TimeUnit.SECONDS);
+    }
+
     this.serverGroupCache =
+<<<<<<< HEAD
         CacheBuilder.newBuilder()
             .build(
                 new CacheLoader<String, CloudFoundryServerGroup>() {
@@ -96,6 +111,19 @@ public CloudFoundryServerGroup load(@Nonnull String guid)
                         .orElseThrow(ResourceNotFoundException::new);
                   }
                 });
+=======
+        builder.build(
+            new CacheLoader<>() {
+              @Override
+              public CloudFoundryServerGroup load(@Nonnull String guid)
+                  throws ResourceNotFoundException {
+                return safelyCall(() -> api.findById(guid))
+                    .map(Applications.this::map)
+                    .flatMap(sg -> sg)
+                    .orElseThrow(ResourceNotFoundException::new);
+              }
+            });
+>>>>>>> d12b5f056 (fix(cloudfoundry): add configurable cache expiry for clients (#5508))
   }
 
   @Nullable

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java

@@ -79,7 +79,8 @@ public HttpCloudFoundryClient(
       Integer resultsPerPage,
       ForkJoinPool forkJoinPool,
       OkHttpClient.Builder okHttpClientBuilder,
-      CloudFoundryConfigurationProperties.ClientConfig clientConfig) {
+      CloudFoundryConfigurationProperties.ClientConfig clientConfig,
+      CloudFoundryConfigurationProperties.LocalCacheConfig localCacheConfig) {
 
     this.apiHost = apiHost;
     this.user = user;
@@ -136,7 +137,8 @@ public HttpCloudFoundryClient(
             processes,
             resultsPerPage,
             onlySpinnakerManaged,
-            forkJoinPool);
+            forkJoinPool,
+            localCacheConfig);
     this.domains = new Domains(retrofit.create(DomainService.class), organizations);
     this.serviceInstances =
         new ServiceInstances(
@@ -151,7 +153,8 @@ public HttpCloudFoundryClient(
             domains,
             spaces,
             resultsPerPage,
-            forkJoinPool);
+            forkJoinPool,
+            localCacheConfig);
     this.serviceKeys = new ServiceKeys(retrofit.create(ServiceKeyService.class), spaces);
     this.tasks = new Tasks(retrofit.create(TaskService.class));
 

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Routes.java

@@ -29,6 +29,7 @@
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Resource;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Route;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.RouteMapping;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.config.CloudFoundryConfigurationProperties;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryDomain;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryLoadBalancer;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryServerGroup;
@@ -59,7 +60,7 @@ public class Routes {
   private final Integer resultsPerPage;
 
   private final ForkJoinPool forkJoinPool;
-  private LoadingCache<String, List<RouteMapping>> routeMappings;
+  private final LoadingCache<String, List<RouteMapping>> routeMappings;
 
   public Routes(
       String account,
@@ -68,29 +69,36 @@ public Routes(
       Domains domains,
       Spaces spaces,
       Integer resultsPerPage,
-      ForkJoinPool forkJoinPool) {
+      ForkJoinPool forkJoinPool,
+      CloudFoundryConfigurationProperties.LocalCacheConfig localCacheConfig) {
     this.account = account;
     this.api = api;
     this.applications = applications;
     this.domains = domains;
     this.spaces = spaces;
     this.resultsPerPage = resultsPerPage;
-
     this.forkJoinPool = forkJoinPool;
+
+    CacheBuilder<Object, Object> builder = CacheBuilder.newBuilder();
+    if (localCacheConfig.getRoutesAccessExpirySeconds() >= 0) {
+      builder.expireAfterAccess(localCacheConfig.getRoutesAccessExpirySeconds(), TimeUnit.SECONDS);
+    }
+    if (localCacheConfig.getRoutesWriteExpirySeconds() >= 0) {
+      builder.expireAfterWrite(localCacheConfig.getRoutesWriteExpirySeconds(), TimeUnit.SECONDS);
+    }
+
     this.routeMappings =
-        CacheBuilder.newBuilder()
-            .expireAfterWrite(3, TimeUnit.MINUTES)
-            .build(
-                new CacheLoader<String, List<RouteMapping>>() {
-                  @Override
-                  public List<RouteMapping> load(@Nonnull String guid)
-                      throws CloudFoundryApiException, ResourceNotFoundException {
-                    return collectPageResources("route mappings", pg -> api.routeMappings(guid, pg))
-                        .stream()
-                        .map(Resource::getEntity)
-                        .collect(Collectors.toList());
-                  }
-                });
+        builder.build(
+            new CacheLoader<>() {
+              @Override
+              public List<RouteMapping> load(@Nonnull String guid)
+                  throws CloudFoundryApiException, ResourceNotFoundException {
+                return collectPageResources("route mappings", pg -> api.routeMappings(guid, pg))
+                    .stream()
+                    .map(Resource::getEntity)
+                    .collect(Collectors.toList());
+              }
+            });
   }
 
   private CloudFoundryLoadBalancer map(Resource<Route> res) throws CloudFoundryApiException {
@@ -174,7 +182,8 @@ public List<CloudFoundryLoadBalancer> all(List<CloudFoundrySpace> spaces)
       throws CloudFoundryApiException {
     try {
       if (!spaces.isEmpty()) {
-        List<String> spaceGuids = spaces.stream().map(s -> s.getId()).collect(Collectors.toList());
+        List<String> spaceGuids =
+            spaces.stream().map(CloudFoundrySpace::getId).collect(Collectors.toList());
         String orgFilter =
             "organization_guid IN "
                 + spaces.stream()

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/config/CloudFoundryConfigurationProperties.java

@@ -46,6 +46,8 @@ public class CloudFoundryConfigurationProperties implements DisposableBean {
 
   @NestedConfigurationProperty private ClientConfig client = new ClientConfig();
 
+  @NestedConfigurationProperty private LocalCacheConfig localCacheConfig = new LocalCacheConfig();
+
   @Override
   public void destroy() {
     this.accounts = new ArrayList<>();
@@ -82,4 +84,12 @@ public static class ClientConfig {
     private int readTimeout = 10000;
     private int maxRetries = 3;
   }
+
+  @Data
+  public static class LocalCacheConfig {
+    private long applicationsAccessExpirySeconds = -1;
+    private long applicationsWriteExpirySeconds = 600;
+    private long routesAccessExpirySeconds = -1;
+    private long routesWriteExpirySeconds = 180;
+  }
 }

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/config/CloudFoundryProviderConfig.java

@@ -103,7 +103,8 @@ public AbstractCredentialsLoader<CloudFoundryCredentials> cloudFoundryCredential
                 cloudFoundryThreadPool,
                 a.getSpaceFilter(),
                 okHttpClient,
-                configurationProperties.getClient()),
+                configurationProperties.getClient(),
+                configurationProperties.getLocalCacheConfig()),
         cloudFoundryCredentialsRepository);
   }
 

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java

@@ -76,14 +76,10 @@ public class CloudFoundryCredentials extends AbstractAccountCredentials<CloudFou
   private final Supplier<List<CloudFoundrySpace>> spaceSupplier =
       Memoizer.memoizeWithExpiration(this::spaceSupplier, SPACE_EXPIRY_SECONDS, TimeUnit.SECONDS);
 
-  private CacheRepository cacheRepository;
-
-  private Permissions permissions;
-
+  private final CacheRepository cacheRepository;
+  private final Permissions permissions;
   private final ForkJoinPool forkJoinPool;
-
   private final List<CloudFoundrySpace> filteredSpaces;
-
   private final CloudFoundryClient cloudFoundryClient;
 
   public CloudFoundryCredentials(
@@ -102,7 +98,8 @@ public CloudFoundryCredentials(
       ForkJoinPool forkJoinPool,
       Map<String, Set<String>> spaceFilter,
       OkHttpClient okHttpClient,
-      CloudFoundryConfigurationProperties.ClientConfig clientConfig) {
+      CloudFoundryConfigurationProperties.ClientConfig clientConfig,
+      CloudFoundryConfigurationProperties.LocalCacheConfig localCacheConfig) {
     this.name = name;
     this.appsManagerUri = appsManagerUri;
     this.metricsUri = metricsUri;
@@ -130,7 +127,8 @@ public CloudFoundryCredentials(
             resultsPerPage,
             forkJoinPool,
             okHttpClient.newBuilder(),
-            clientConfig);
+            clientConfig,
+            localCacheConfig);
     this.filteredSpaces = createFilteredSpaces(spaceFilter);
   }
 
@@ -148,7 +146,7 @@ public Collection<Map<String, String>> getRegions() {
             s -> {
               if (!filteredSpaces.isEmpty()) {
                 List<String> filteredRegions =
-                    filteredSpaces.stream().map(fs -> fs.getRegion()).collect(toList());
+                    filteredSpaces.stream().map(CloudFoundrySpace::getRegion).collect(toList());
                 return filteredRegions.contains(s.getRegion());
               }
               return true;
@@ -206,7 +204,7 @@ public int hashCode() {
 
   protected List<CloudFoundrySpace> createFilteredSpaces(Map<String, Set<String>> spaceFilter) {
     List<CloudFoundrySpace> spaces = new ArrayList<>();
-    if (spaceFilter.isEmpty() || spaceFilter == null) {
+    if (spaceFilter.isEmpty()) {
       return emptyList();
     }
 
@@ -230,11 +228,11 @@ protected List<CloudFoundrySpace> createFilteredSpaces(Map<String, Set<String>>
         this.getClient()
             .getSpaces()
             .findAllBySpaceNamesAndOrgNames(
-                spaceFilter.values().stream().flatMap(l -> l.stream()).collect(Collectors.toList()),
+                spaceFilter.values().stream()
+                    .flatMap(Collection::stream)
+                    .collect(Collectors.toList()),
                 List.copyOf(spaceFilter.keySet()));
-    allSpaces.stream()
-        .filter(s -> filteredRegions.contains(s.getRegion()))
-        .forEach(s -> spaces.add(s));
+    allSpaces.stream().filter(s -> filteredRegions.contains(s.getRegion())).forEach(spaces::add);
 
     if (spaces.isEmpty())
       throw new IllegalArgumentException(

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/cache/CacheRepositoryTest.java

@@ -128,7 +128,8 @@ private CloudFoundryCredentials createCredentials(String name) {
         ForkJoinPool.commonPool(),
         emptyMap(),
         new OkHttpClient(),
-        new CloudFoundryConfigurationProperties.ClientConfig());
+        new CloudFoundryConfigurationProperties.ClientConfig(),
+        new CloudFoundryConfigurationProperties.LocalCacheConfig());
   }
 
   @Test

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/ApplicationsTest.java

@@ -59,7 +59,8 @@ class ApplicationsTest {
           processes,
           resultsPerPage,
           true,
-          ForkJoinPool.commonPool());
+          ForkJoinPool.commonPool(),
+          new CloudFoundryConfigurationProperties.LocalCacheConfig());
   private final String spaceId = "space-guid";
   private final CloudFoundrySpace cloudFoundrySpace =
       CloudFoundrySpace.builder()
@@ -84,7 +85,8 @@ void errorHandling() {
             resultsPerPage,
             ForkJoinPool.commonPool(),
             new OkHttpClient().newBuilder(),
-            new CloudFoundryConfigurationProperties.ClientConfig());
+            new CloudFoundryConfigurationProperties.ClientConfig(),
+            new CloudFoundryConfigurationProperties.LocalCacheConfig());
 
     assertThatThrownBy(() -> client.getApplications().all(emptyList()))
         .isInstanceOf(CloudFoundryApiException.class);

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClientTest.java

@@ -247,6 +247,7 @@ private HttpCloudFoundryClient createCloudFoundryClientWithRetryConfig(
         500,
         ForkJoinPool.commonPool(),
         new OkHttpClient.Builder(),
-        clientConfig);
+        clientConfig,
+        new CloudFoundryConfigurationProperties.LocalCacheConfig());
   }
 }

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/RoutesTest.java

@@ -18,8 +18,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.eq;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -29,6 +29,7 @@
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Resource;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Route;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.RouteMapping;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.config.CloudFoundryConfigurationProperties;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryDomain;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryLoadBalancer;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryOrganization;
@@ -66,7 +67,15 @@ void toRouteId() {
         .thenAnswer(invocation -> Calls.response(Response.success(new Page<>())));
 
     Routes routes =
-        new Routes("pws", routeService, null, domains, spaces, 500, ForkJoinPool.commonPool());
+        new Routes(
+            "pws",
+            routeService,
+            null,
+            domains,
+            spaces,
+            500,
+            ForkJoinPool.commonPool(),
+            new CloudFoundryConfigurationProperties.LocalCacheConfig());
     RouteId routeId = routes.toRouteId("demo1-prod.apps.calabasas.cf-app.com/path/v1.0");
     assertThat(routeId).isNotNull();
     assertThat(routeId.getHost()).isEqualTo("demo1-prod");
@@ -76,7 +85,16 @@ void toRouteId() {
 
   @Test
   void toRouteIdReturnsNullForInvalidRoute() {
-    Routes routes = new Routes(null, null, null, null, null, 500, ForkJoinPool.commonPool());
+    Routes routes =
+        new Routes(
+            null,
+            null,
+            null,
+            null,
+            null,
+            500,
+            ForkJoinPool.commonPool(),
+            new CloudFoundryConfigurationProperties.LocalCacheConfig());
     assertNull(routes.toRouteId("demo1-pro cf-app.com/path"));
   }
 
@@ -139,7 +157,15 @@ void findShouldFilterCorrectlyOnMultipleResults() {
         .thenAnswer(invocation -> Calls.response(Response.success(routeMappingPage)));
 
     Routes routes =
-        new Routes("pws", routeService, null, domains, spaces, 500, ForkJoinPool.commonPool());
+        new Routes(
+            "pws",
+            routeService,
+            null,
+            domains,
+            spaces,
+            500,
+            ForkJoinPool.commonPool(),
+            new CloudFoundryConfigurationProperties.LocalCacheConfig());
 
     CloudFoundryLoadBalancer loadBalancer =
         routes.find(new RouteId().setHost("somehost").setDomainGuid("domain-guid"), "space-guid");

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/converters/AbstractLoadBalancersAtomicOperationConverterTest.java

@@ -106,7 +106,8 @@ class AbstractLoadBalancersAtomicOperationConverterTest {
           ForkJoinPool.commonPool(),
           emptyMap(),
           new OkHttpClient(),
-          new CloudFoundryConfigurationProperties.ClientConfig()) {
+          new CloudFoundryConfigurationProperties.ClientConfig(),
+          new CloudFoundryConfigurationProperties.LocalCacheConfig()) {
         public CloudFoundryClient getClient() {
           return cloudFoundryClient;
         }