diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java index 2d10005ab56..fe89ee661c1 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/Applications.java @@ -169,6 +169,13 @@ public List all() { for (CloudFoundryServerGroup serverGroup : serverGroupCache.asMap().values()) { Names names = Names.parseName(serverGroup.getName()); + if (names.getCluster() == null) { + log.debug( + "Skipping app '{}' from foundation '{}' because the name isn't following the frigga naming schema.", + serverGroup.getName(), + this.account); + continue; + } serverGroupsByClusters .computeIfAbsent(names.getCluster(), clusterName -> new HashSet<>()) .add(serverGroup); @@ -417,7 +424,7 @@ private CloudFoundryServerGroup checkHealthStatus( }) .collect(toSet()); - log.debug( + log.trace( "Successfully retrieved " + instances.size() + " instances for application '" diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryApiException.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryApiException.java index 3b51e76c31c..0196aeef7a7 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryApiException.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryApiException.java @@ -36,14 +36,15 @@ public CloudFoundryApiException(ErrorDescription errorCause, RetrofitError retro super( Optional.ofNullable(errorCause) .map(e -> getMessage(e.getErrors().toArray(new String[0]))) - .orElse(getRetrofitErrorMessage(retrofitError))); + .orElse(getRetrofitErrorMessage(retrofitError)), + retrofitError); if (errorCause != null) { this.errorCode = errorCause.getCode(); } } public CloudFoundryApiException(RetrofitError retrofitError) { - super(getRetrofitErrorMessage(retrofitError)); + super(getRetrofitErrorMessage(retrofitError), retrofitError); } public CloudFoundryApiException(Throwable t, String... errors) { diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java index 649c37f6d39..2a71a9206aa 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java @@ -41,7 +41,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -77,8 +77,9 @@ public class HttpCloudFoundryClient implements CloudFoundryClient { private Logger logger = LoggerFactory.getLogger(HttpCloudFoundryClient.class); private AuthenticationService uaaService; - private AtomicLong tokenExpirationNs = new AtomicLong(System.nanoTime()); - private volatile Token token; + private long tokenExpiration = System.currentTimeMillis(); + private Token token; + private final Object tokenLock = new Object(); private JacksonConverter jacksonConverter; @@ -93,13 +94,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient { private Logs logs; private final RequestInterceptor oauthInterceptor = - new RequestInterceptor() { - @Override - public void intercept(RequestFacade request) { - refreshTokenIfNecessary(); - request.addHeader("Authorization", "bearer " + token.getAccessToken()); - } - }; + request -> request.addHeader("Authorization", "bearer " + getToken(false).getAccessToken()); private static class RetryableApiException extends RuntimeException { RetryableApiException(String message) { @@ -120,7 +115,7 @@ Response createRetryInterceptor(Interceptor.Chain chain) { .intervalFunction(IntervalFunction.ofExponentialBackoff(Duration.ofSeconds(10), 3)) .retryExceptions(RetryableApiException.class) .build()); - logger.debug("cf request: " + chain.request().urlString()); + logger.trace("cf request: " + chain.request().urlString()); AtomicReference lastResponse = new AtomicReference<>(); try { return retry.executeCallable( @@ -135,13 +130,12 @@ Response createRetryInterceptor(Interceptor.Chain chain) { Buffer buffer = source.buffer(); String body = buffer.clone().readString(Charset.forName("UTF-8")); if (!body.contains("Bad credentials")) { - refreshToken(); response = chain.proceed( chain .request() .newBuilder() - .header("Authorization", "bearer " + token.getAccessToken()) + .header("Authorization", "bearer " + getToken(true).getAccessToken()) .build()); lastResponse.set(response); } @@ -188,6 +182,7 @@ public HttpCloudFoundryClient( this.password = password; this.okHttpClient = createHttpClient(skipSslValidation); + this.okHttpClient.setReadTimeout(20, TimeUnit.SECONDS); okHttpClient.interceptors().add(this::createRetryInterceptor); @@ -284,23 +279,19 @@ public X509Certificate[] getAcceptedIssuers() { return client; } - private void refreshTokenIfNecessary() { - long currentExpiration = tokenExpirationNs.get(); - long now = System.nanoTime(); - long comp = Math.min(currentExpiration, now); - if (tokenExpirationNs.compareAndSet(comp, now)) { - this.refreshToken(); - } - } - - private void refreshToken() { - try { - token = uaaService.passwordToken("password", user, password, "cf", ""); - } catch (Exception e) { - log.warn("Failed to obtain a token", e); - throw e; + private Token getToken(boolean forceRefresh) { + synchronized (tokenLock) { + if (forceRefresh || (token == null || System.currentTimeMillis() >= tokenExpiration)) { + try { + token = uaaService.passwordToken("password", user, password, "cf", ""); + } catch (Exception e) { + log.warn("Failed to obtain a token", e); + throw e; + } + tokenExpiration = System.currentTimeMillis() + ((token.getExpiresIn() - 120) * 1000); + } + return token; } - tokenExpirationNs.addAndGet(Duration.ofSeconds(token.getExpiresIn()).toNanos()); } private S createService(Class serviceClass) { diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryLoadBalancerCachingAgent.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryLoadBalancerCachingAgent.java index 164a3d343c9..ef61a2bf508 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryLoadBalancerCachingAgent.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryLoadBalancerCachingAgent.java @@ -77,7 +77,7 @@ public CacheResult loadData(ProviderCache providerCache) { cacheData -> { long cacheTime = (long) cacheData.getAttributes().get("cacheTime"); if (cacheTime < loadDataStart - && (int) cacheData.getAttributes().get("processedCount") > 0) { + && (int) cacheData.getAttributes().computeIfAbsent("processedCount", s -> 0) > 0) { toEvict.add(cacheData.getId()); } else { toKeep.put(cacheData.getId(), cacheData); diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgent.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgent.java index 0fa5426463a..5738c9cb807 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgent.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgent.java @@ -92,7 +92,7 @@ public CacheResult loadData(ProviderCache providerCache) { cacheData -> { long cacheTime = (long) cacheData.getAttributes().get("cacheTime"); if (cacheTime < loadDataStart - && (int) cacheData.getAttributes().get("processedCount") > 0) { + && (int) cacheData.getAttributes().computeIfAbsent("processedCount", s -> 0) > 0) { toEvict.add(cacheData.getId()); } else { toKeep.put(cacheData.getId(), cacheData); @@ -150,13 +150,6 @@ public OnDemandResult handle(ProviderCache providerCache, Map data) { if (serverGroupName == null) { return null; } - CloudFoundryServerGroup serverGroup = - this.getClient() - .getApplications() - .findServerGroupByNameAndSpaceId(serverGroupName, space.getId()); - if (serverGroup == null) { - return null; - } log.info("On Demand cache refresh triggered, waiting for Server group loadData to be called"); CloudFoundryServerGroup cloudFoundryServerGroup = diff --git a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java index ec73e1557ce..e2984ca4db5 100644 --- a/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java +++ b/clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentials.java @@ -76,7 +76,7 @@ public CloudFoundryCredentials( this.password = password; this.environment = Optional.ofNullable(environment).orElse("dev"); this.skipSslValidation = skipSslValidation; - this.resultsPerPage = Optional.ofNullable(resultsPerPage).orElse(500); + this.resultsPerPage = Optional.ofNullable(resultsPerPage).orElse(100); this.maxCapiConnectionsForCache = Optional.ofNullable(maxCapiConnectionsForCache).orElse(16); } diff --git a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgentTest.java b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgentTest.java index c949fc86781..82e62378c9d 100644 --- a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgentTest.java +++ b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/provider/agent/CloudFoundryServerGroupCachingAgentTest.java @@ -115,7 +115,7 @@ void handleShouldReturnNullWhenServerGroupNameIsUnspecified() { } @Test - void handleShouldReturnNullWhenServerGroupDoesNotExist() { + void handleShouldReturnAnEvictResultWhenServerGroupDoesNotExists() { String region = "org > space"; String serverGroupName = "server-group"; Map data = @@ -133,12 +133,15 @@ void handleShouldReturnNullWhenServerGroupDoesNotExist() { when(cloudFoundryClient.getApplications()).thenReturn(mockApplications); when(mockApplications.findServerGroupByNameAndSpaceId(any(), any())).thenReturn(null); + when(mockProviderCache.filterIdentifiers(any(), any())) + .thenReturn(Collections.singletonList("key")); + OnDemandAgent.OnDemandResult result = cloudFoundryServerGroupCachingAgent.handle(mockProviderCache, data); - assertThat(result).isNull(); - verify(organizations).findSpaceByRegion(eq(region)); - verify(mockApplications).findServerGroupByNameAndSpaceId(eq(serverGroupName), eq(spaceId)); + assertThat(result).isNotNull(); + assertThat(result.getEvictions()).hasSize(1); + assertThat(result.getEvictions().get(SERVER_GROUPS.getNs())).containsExactly("key"); } @Test diff --git a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsSynchronizerTest.java b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsSynchronizerTest.java index 4ae79a6117d..37ac2f21dac 100644 --- a/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsSynchronizerTest.java +++ b/clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/security/CloudFoundryCredentialsSynchronizerTest.java @@ -149,7 +149,7 @@ private CloudFoundryConfigurationProperties.ManagedAccount createAccount(String private CloudFoundryCredentials createCredentials(String name) { return new CloudFoundryCredentials( - name, null, null, "api." + name, "user-" + name, "pwd-" + name, null, false, 500, 16); + name, null, null, "api." + name, "user-" + name, "pwd-" + name, null, false, null, 16); } private void loadProviderFromRepository() {