Skip to content

Commit 49772f0

Browse files
committed
fix(gax): address mTLS cert config parsing and channel refresh lifecycle edge cases
1 parent 89428d6 commit 49772f0

3 files changed

Lines changed: 22 additions & 8 deletions

File tree

sdk-platform-java/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/RefreshingHttpJsonChannel.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,15 @@
3535
import com.google.api.gax.httpjson.ForwardingHttpJsonClientCallListener.SimpleForwardingHttpJsonClientCallListener;
3636
import com.google.api.gax.rpc.mtls.WorkloadCertificateUtils;
3737
import com.google.common.annotations.VisibleForTesting;
38+
import java.util.concurrent.CancellationException;
3839
import java.util.concurrent.TimeUnit;
3940
import java.util.concurrent.atomic.AtomicBoolean;
4041
import java.util.concurrent.atomic.AtomicInteger;
4142
import java.util.concurrent.atomic.AtomicReference;
4243
import java.util.function.Supplier;
4344
import java.util.logging.Level;
4445
import java.util.logging.Logger;
46+
import org.jspecify.annotations.Nullable;
4547

4648
/**
4749
* An implementation of {@link ManagedHttpJsonChannel} that supports dynamic mTLS certificate
@@ -240,7 +242,6 @@ public void shutdownNow() {
240242
synchronized (refreshLock) {
241243
isShuttingDown = true;
242244
for (ChannelEntry entry : allEntries) {
243-
entry.requestShutdown();
244245
entry.channel.shutdownNow();
245246
}
246247
}
@@ -329,6 +330,7 @@ private void shutdown() {
329330
private static class ReleasingHttpJsonClientCall<ReqT, RespT>
330331
extends SimpleForwardingHttpJsonClientCall<ReqT, RespT> {
331332

333+
private @Nullable CancellationException cancellationException;
332334
private final ChannelEntry entry;
333335
private final AtomicBoolean wasClosed = new AtomicBoolean(false);
334336
private final AtomicBoolean wasReleased = new AtomicBoolean(false);
@@ -340,6 +342,12 @@ private static class ReleasingHttpJsonClientCall<ReqT, RespT>
340342

341343
@Override
342344
public void start(Listener<RespT> responseListener, HttpJsonMetadata requestHeaders) {
345+
if (cancellationException != null) {
346+
if (wasReleased.compareAndSet(false, true)) {
347+
entry.release();
348+
}
349+
throw new IllegalStateException("Call is already cancelled", cancellationException);
350+
}
343351
try {
344352
super.start(
345353
new SimpleForwardingHttpJsonClientCallListener<RespT>(responseListener) {
@@ -365,5 +373,11 @@ public void onClose(int statusCode, HttpJsonMetadata trailers) {
365373
throw e;
366374
}
367375
}
376+
377+
@Override
378+
public void cancel(@Nullable String message, @Nullable Throwable cause) {
379+
this.cancellationException = new CancellationException(message);
380+
super.cancel(message, cause);
381+
}
368382
}
369383
}

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/mtls/CertificateBasedAccess.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ private CertificateConfig parseCertificateConfig(String configPath) throws IOExc
111111
String keyPath = extractJsonValue(content, "key_path");
112112

113113
if (certPath == null || keyPath == null) {
114-
throw new IllegalStateException(
115-
"Malformed certificate config JSON. Must contain 'cert_path' and 'key_path'.");
114+
return null;
116115
}
117116

118117
return new CertificateConfig(certPath, keyPath);
@@ -176,6 +175,9 @@ private boolean validateAndResolveConfig(String configPath) {
176175
}
177176
try {
178177
CertificateConfig config = parseCertificateConfig(configPath);
178+
if (config == null) {
179+
return false;
180+
}
179181
if (!fileExistenceProvider.exists(config.certPath)
180182
|| !fileExistenceProvider.exists(config.keyPath)) {
181183
throw new IllegalStateException(
@@ -216,7 +218,7 @@ public String getWorkloadCertPath() {
216218
if (certConfigPath != null && !certConfigPath.isEmpty()) {
217219
try {
218220
CertificateConfig config = parseCertificateConfig(certConfigPath);
219-
return config.certPath;
221+
return config != null ? config.certPath : null;
220222
} catch (Exception e) {
221223
throw new IllegalStateException("Failed to parse GOOGLE_API_CERTIFICATE_CONFIG", e);
222224
}

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/rpc/mtls/CertificateBasedAccessTest.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ void testUseMtlsClientCertificateEnvTrueOverride() {
208208
}
209209

210210
@Test
211-
void testUseMtlsClientCertificateConfigMalformedJson() {
211+
void testUseMtlsClientCertificateConfigNonWorkloadJson() {
212212
TestEnv env = new TestEnv();
213213
env.set("GOOGLE_API_CERTIFICATE_CONFIG", "/path/to/config.json");
214214

@@ -217,9 +217,7 @@ void testUseMtlsClientCertificateConfigMalformedJson() {
217217

218218
CertificateBasedAccess cba = createCba(env, fs);
219219

220-
IllegalStateException ex =
221-
assertThrows(IllegalStateException.class, cba::useMtlsClientCertificate);
222-
assertTrue(ex.getMessage().contains("Failed to parse or validate certificate config"));
220+
assertFalse(cba.useMtlsClientCertificate());
223221
}
224222

225223
@Test

0 commit comments

Comments
 (0)