Skip to content

Commit

Permalink
fix: Block instead of returning expired certificate from CloudSqlInst…
Browse files Browse the repository at this point in the history
…ance.getInstanceData().
  • Loading branch information
hessjcg committed Oct 10, 2023
1 parent 9968302 commit 901558d
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
17 changes: 17 additions & 0 deletions core/src/main/java/com/google/cloud/sql/core/CloudSqlInstance.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,23 @@ private InstanceData getInstanceData(long timeoutMs) {
ListenableFuture<InstanceData> instanceDataFuture;
synchronized (instanceDataGuard) {
instanceDataFuture = currentInstanceData;
// If the currentInstanceData has expired, then force refresh (which will balk if a refresh
// is already running) and make this and future requests to getInstanceData wait on the
// refresh operation to complete.
if (instanceDataFuture.isDone()) {
Instant expiration;
try {
expiration = instanceDataFuture.get().getExpiration();
if (expiration == null || expiration.isBefore(Instant.now())) {
forceRefresh();
currentInstanceData = nextInstanceData;
instanceDataFuture = currentInstanceData;
}
} catch (ExecutionException | InterruptedException e) {
e.printStackTrace();
forceRefresh();
}
}
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.cloud.sql.core;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.cloud.sql.AuthType;
import com.google.common.collect.ImmutableMap;
Expand All @@ -32,6 +33,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -272,6 +274,12 @@ public void testCloudSqlRefreshesExpiredData() throws Exception {
Thread.sleep(10);
}

// Now that the InstanceData has expired, this getSslData should pause until new data
// has been retrieved. In this case, the new data has not yet been retrieved, so the operation
// should time out after 100ms and throw an exception.
assertThrows(RuntimeException.class, () -> instance.getSslData(100));
assertThat(refreshCount.get()).isEqualTo(1);

// Allow the second refresh operation to complete
refresh1.proceed();
refresh1.waitForPauseToEnd(1000L);
Expand Down Expand Up @@ -455,13 +463,25 @@ public void testRefreshRetriesOnAfterFailedAttempts() throws Exception {
Thread.sleep(10);
}

// Start a thread to getSslData(). This will eventually return after the
// failed attempts.
AtomicReference<SslData> earlyFetchAttempt = new AtomicReference<>();
Thread t = new Thread(() -> earlyFetchAttempt.set(instance.getSslData(TEST_TIMEOUT_MS)));
t.start();

// Orchestrate the failed attempts

// Allow the second refresh operation to complete
badRequest1.proceed();
badRequest1.waitForPauseToEnd(5000);
badRequest1.waitForCondition(() -> refreshCount.get() == 2, 2000);

// Now that the InstanceData has expired, this getSslData should pause until new data
// has been retrieved. In this case, the new data has not yet been retrieved, so the operation
// should time out after 10ms.
assertThrows(RuntimeException.class, () -> instance.getSslData(10));
assertThat(d).isSameInstanceAs(aboutToExpireData.getSslData());

// Allow the second bad request completes
badRequest2.proceed();
badRequest2.waitForCondition(() -> refreshCount.get() == 3, 2000);
Expand All @@ -470,7 +490,13 @@ public void testRefreshRetriesOnAfterFailedAttempts() throws Exception {
goodRequest.proceed();
goodRequest.waitForCondition(() -> refreshCount.get() == 4, 2000);

// Try getSslData() again, and assert the refresh operation eventually completes.
// Wait for the thread to exit, meaning getSslData() finally returned
// after several failed attempts. Assert that the correct InstanceData
// eventually returned.
t.join();
assertThat(earlyFetchAttempt.get()).isSameInstanceAs(data.getSslData());

// Try getSslData() again, and assert the refresh operation completed.
goodRequest.waitForCondition(
() -> instance.getSslData(TEST_TIMEOUT_MS) == data.getSslData(), 2000);
}
Expand Down

0 comments on commit 901558d

Please sign in to comment.