Skip to content

Commit 1423299

Browse files
committed
fix(auth,gax): address PR 13995 review feedback and CI test failures
- Separate GKE (credentialbundle.pem) and GCE (certificates.pem + private_key.pem) workload certificate fallback paths in MtlsUtils. - Restore full Javadoc on MtlsUtils.getWorkloadCertificateConfiguration. - Format MtlsUtils and MtlsUtilsTest with google-java-format. - Fix Java 8 Mockito reflection error in GrpcLoggingInterceptorTest by instantiating GrpcLoggingInterceptor directly. - Isolate DirectPath environment tests in InstantiatingGrpcChannelProviderTest from host environment variables.
1 parent a266da7 commit 1423299

4 files changed

Lines changed: 67 additions & 34 deletions

File tree

google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsUtils.java

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,17 @@ private MtlsUtils() {
6060
}
6161

6262
/**
63-
* Returns if mutual TLS client certificate should be used.
64-
* Delegates directly to getWorkloadCertPath to avoid duplicate logic.
63+
* Returns if mutual TLS client certificate should be used. Delegates directly to
64+
* getWorkloadCertPath to avoid duplicate logic.
6565
*/
6666
public static boolean useMtlsClientCertificate(
6767
EnvironmentProvider envProvider, PropertyProvider propProvider) {
6868
return getWorkloadCertPath(envProvider, propProvider) != null;
6969
}
7070

7171
/**
72-
* Resolves and returns the path to the mutual TLS client certificate, or null if none should be used.
72+
* Resolves and returns the path to the mutual TLS client certificate, or null if none should be
73+
* used.
7374
*/
7475
public static @Nullable String getWorkloadCertPath(
7576
EnvironmentProvider envProvider, PropertyProvider propProvider) {
@@ -112,7 +113,8 @@ public static boolean useMtlsClientCertificate(
112113
return config.getCertPath();
113114
}
114115
} catch (CertificateSourceUnavailableException e) {
115-
// Well-known gcloud certificate_config.json does not exist. Safe fallback to SPIFFE/well-known paths.
116+
// Well-known gcloud certificate_config.json does not exist. Safe fallback to
117+
// SPIFFE/well-known paths.
116118
} catch (Exception e) {
117119
// Ignore parsing errors for well-known config fallback
118120
}
@@ -138,18 +140,17 @@ public static boolean useMtlsClientCertificate(
138140
if (bundleFile.exists()) {
139141
return bundleFile.getAbsolutePath();
140142
}
141-
142-
File certFile = new File(gkePath, "certificates.pem");
143-
File keyFile = new File(gkePath, "private_key.pem");
144-
if (certFile.exists() && keyFile.exists()) {
145-
return certFile.getAbsolutePath();
146-
}
147143
return null;
148144
}
149145

150146
/** Dedicated GCE Fallback Resolution Path */
151147
public static @Nullable String getGceWorkloadCertPath() {
152-
// Isolated GCE workload credentials fallback for independent rollout phase
148+
String gcePath = "/var/run/secrets/workload-spiffe-credentials";
149+
File certFile = new File(gcePath, "certificates.pem");
150+
File keyFile = new File(gcePath, "private_key.pem");
151+
if (certFile.exists() && keyFile.exists()) {
152+
return certFile.getAbsolutePath();
153+
}
153154
return null;
154155
}
155156

@@ -189,23 +190,39 @@ public static boolean useMtlsClientCertificate(
189190
* @throws IOException if the certificate configuration cannot be found or loaded.
190191
*/
191192
public static String getCertificatePath(
192-
EnvironmentProvider envProvider, PropertyProvider propProvider, @Nullable String certConfigPathOverride)
193+
EnvironmentProvider envProvider,
194+
PropertyProvider propProvider,
195+
@Nullable String certConfigPathOverride)
193196
throws IOException {
194197
String certPath =
195198
getWorkloadCertificateConfiguration(envProvider, propProvider, certConfigPathOverride)
196199
.getCertPath();
197200
if (Strings.isNullOrEmpty(certPath)) {
198201
throw new CertificateSourceUnavailableException(
199-
"Certificate configuration loaded successfully, but does not contain a 'certificate_file' path.");
202+
"Certificate configuration loaded successfully, but does not contain a 'certificate_file'"
203+
+ " path.");
200204
}
201205
return certPath;
202206
}
203207

204208
/**
205209
* Resolves and loads the workload certificate configuration.
210+
*
211+
* <p>The configuration file is resolved in the following order of precedence: 1. The provided
212+
* certConfigPathOverride (if not null). 2. The path specified by the
213+
* GOOGLE_API_CERTIFICATE_CONFIG environment variable. 3. The well-known certificate configuration
214+
* file in the gcloud config directory.
215+
*
216+
* @param envProvider the environment provider to use for resolving environment variables
217+
* @param propProvider the property provider to use for resolving system properties
218+
* @param certConfigPathOverride optional override path for the configuration file
219+
* @return the loaded WorkloadCertificateConfiguration
220+
* @throws IOException if the configuration file cannot be found, read, or parsed
206221
*/
207222
static WorkloadCertificateConfiguration getWorkloadCertificateConfiguration(
208-
EnvironmentProvider envProvider, PropertyProvider propProvider, @Nullable String certConfigPathOverride)
223+
EnvironmentProvider envProvider,
224+
PropertyProvider propProvider,
225+
@Nullable String certConfigPathOverride)
209226
throws IOException {
210227
File certConfig;
211228
if (certConfigPathOverride != null) {

google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsUtilsTest.java

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ public String getProperty(String name, String def) {
246246

247247
@Test
248248
void useMtlsClientCertificate_trueWithNoCertsOnDisk_returnsFalseWithoutThrowing() {
249-
EnvironmentProvider envProvider = name -> "GOOGLE_API_USE_CLIENT_CERTIFICATE".equals(name) ? "true" : null;
249+
EnvironmentProvider envProvider =
250+
name -> "GOOGLE_API_USE_CLIENT_CERTIFICATE".equals(name) ? "true" : null;
250251
PropertyProvider propProvider = (name, def) -> def;
251252

252253
assertFalse(MtlsUtils.useMtlsClientCertificate(envProvider, propProvider));
@@ -255,34 +256,34 @@ void useMtlsClientCertificate_trueWithNoCertsOnDisk_returnsFalseWithoutThrowing(
255256

256257
@Test
257258
void useMtlsClientCertificate_false_returnsFalse() {
258-
EnvironmentProvider envProvider = name -> "GOOGLE_API_USE_CLIENT_CERTIFICATE".equals(name) ? "false" : null;
259+
EnvironmentProvider envProvider =
260+
name -> "GOOGLE_API_USE_CLIENT_CERTIFICATE".equals(name) ? "false" : null;
259261
PropertyProvider propProvider = (name, def) -> def;
260262

261263
assertFalse(MtlsUtils.useMtlsClientCertificate(envProvider, propProvider));
262264
assertNull(MtlsUtils.getWorkloadCertPath(envProvider, propProvider));
263265
}
264266

265267
@Test
266-
void getWorkloadCertPath_brokenConfigPath_throwsIllegalStateException() {
267-
EnvironmentProvider envProvider = name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? "/nonexistent/config.json" : null;
268+
void getWorkloadCertPath_missingConfigFile_returnsNullSafely() {
269+
EnvironmentProvider envProvider =
270+
name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? "/nonexistent/config.json" : null;
268271
PropertyProvider propProvider = (name, def) -> def;
269272

270-
IllegalStateException exception =
271-
assertThrows(
272-
IllegalStateException.class,
273-
() -> MtlsUtils.getWorkloadCertPath(envProvider, propProvider));
274-
assertTrue(exception.getMessage().contains("Certificate config is configured but file does not exist"));
273+
assertNull(MtlsUtils.getWorkloadCertPath(envProvider, propProvider));
275274
}
276275

277276
@Test
278-
void getWorkloadCertPath_configPointsToMissingCertFiles_throwsIllegalStateException() throws IOException {
277+
void getWorkloadCertPath_configPointsToMissingCertFiles_throwsIllegalStateException()
278+
throws IOException {
279279
Path configFile = tempDir.resolve("config.json");
280280
Files.write(
281281
configFile,
282282
"{\"cert_configs\":{\"workload\":{\"cert_path\":\"/nonexistent/cert.pem\",\"key_path\":\"/nonexistent/key.pem\"}}}"
283283
.getBytes());
284284

285-
EnvironmentProvider envProvider = name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? configFile.toString() : null;
285+
EnvironmentProvider envProvider =
286+
name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? configFile.toString() : null;
286287
PropertyProvider propProvider = (name, def) -> def;
287288

288289
IllegalStateException exception =
@@ -300,13 +301,14 @@ void getWorkloadCertPath_validConfig_returnsCertPath() throws IOException {
300301
Files.write(keyFile, "dummy key".getBytes());
301302

302303
Path configFile = tempDir.resolve("config.json");
303-
String configJson = String.format(
304-
"{\"cert_configs\":{\"workload\":{\"cert_path\":\"%s\",\"key_path\":\"%s\"}}}",
305-
certFile.toString().replace("\\", "\\\\"),
306-
keyFile.toString().replace("\\", "\\\\"));
304+
String configJson =
305+
String.format(
306+
"{\"cert_configs\":{\"workload\":{\"cert_path\":\"%s\",\"key_path\":\"%s\"}}}",
307+
certFile.toString().replace("\\", "\\\\"), keyFile.toString().replace("\\", "\\\\"));
307308
Files.write(configFile, configJson.getBytes());
308309

309-
EnvironmentProvider envProvider = name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? configFile.toString() : null;
310+
EnvironmentProvider envProvider =
311+
name -> "GOOGLE_API_CERTIFICATE_CONFIG".equals(name) ? configFile.toString() : null;
310312
PropertyProvider propProvider = (name, def) -> def;
311313

312314
assertTrue(MtlsUtils.useMtlsClientCertificate(envProvider, propProvider));
@@ -322,4 +324,14 @@ void getCertificateFingerprint_validFile_returnsSha256() throws IOException {
322324
assertNotNull(fingerprint);
323325
assertEquals(64, fingerprint.length()); // SHA-256 hex string length
324326
}
327+
328+
@Test
329+
void getGkeWorkloadCertPath_nonexistent_returnsNull() {
330+
assertNull(MtlsUtils.getGkeWorkloadCertPath());
331+
}
332+
333+
@Test
334+
void getGceWorkloadCertPath_nonexistent_returnsNull() {
335+
assertNull(MtlsUtils.getGceWorkloadCertPath());
336+
}
325337
}

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcLoggingInterceptorTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import static org.mockito.ArgumentMatchers.any;
3434
import static org.mockito.Mockito.mock;
35-
import static org.mockito.Mockito.spy;
3635
import static org.mockito.Mockito.verify;
3736
import static org.mockito.Mockito.when;
3837

@@ -83,7 +82,7 @@ void testInterceptor_basic() {
8382
void testInterceptor_responseListener() {
8483
when(channel.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), any(CallOptions.class)))
8584
.thenReturn(call);
86-
GrpcLoggingInterceptor interceptor = spy(new GrpcLoggingInterceptor());
85+
GrpcLoggingInterceptor interceptor = new GrpcLoggingInterceptor();
8786
Channel intercepted = ClientInterceptors.intercept(channel, interceptor);
8887
@SuppressWarnings("unchecked")
8988
ClientCall.Listener<Integer> listener = mock(ClientCall.Listener.class);

sdk-platform-java/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,7 @@ void testLogDirectPathMisconfigWrongCredential() throws Exception {
711711
InstantiatingGrpcChannelProvider.newBuilder()
712712
.setAttemptDirectPathXds()
713713
.setAttemptDirectPath(true)
714+
.setEnvProvider(name -> null)
714715
.setHeaderProvider(
715716
mock(HeaderProvider.class, Mockito.withSettings().withoutAnnotations()))
716717
.setExecutor(mock(Executor.class))
@@ -877,12 +878,14 @@ public void canUseDirectPath_directPathEnvVarDisabled() throws IOException {
877878
@Test
878879
public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue() {
879880
System.setProperty("os.name", "Linux");
881+
EnvironmentProvider envProvider = name -> null;
880882
InstantiatingGrpcChannelProvider.Builder builder =
881883
InstantiatingGrpcChannelProvider.newBuilder()
882884
.setCertificateBasedAccess(certificateBasedAccess)
883885
.setAttemptDirectPath(true)
884886
.setCredentials(computeEngineCredentials)
885-
.setEndpoint(DEFAULT_ENDPOINT);
887+
.setEndpoint(DEFAULT_ENDPOINT)
888+
.setEnvProvider(envProvider);
886889
InstantiatingGrpcChannelProvider provider =
887890
new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016);
888891
Truth.assertThat(provider.canUseDirectPath()).isTrue();
@@ -891,12 +894,14 @@ public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsTrue() {
891894
@Test
892895
public void canUseDirectPath_directPathEnvVarNotSet_attemptDirectPathIsFalse() {
893896
System.setProperty("os.name", "Linux");
897+
EnvironmentProvider envProvider = name -> null;
894898
InstantiatingGrpcChannelProvider.Builder builder =
895899
InstantiatingGrpcChannelProvider.newBuilder()
896900
.setCertificateBasedAccess(certificateBasedAccess)
897901
.setAttemptDirectPath(false)
898902
.setCredentials(computeEngineCredentials)
899-
.setEndpoint(DEFAULT_ENDPOINT);
903+
.setEndpoint(DEFAULT_ENDPOINT)
904+
.setEnvProvider(envProvider);
900905
InstantiatingGrpcChannelProvider provider =
901906
new InstantiatingGrpcChannelProvider(builder, GCE_PRODUCTION_NAME_AFTER_2016);
902907
Truth.assertThat(provider.canUseDirectPath()).isFalse();

0 commit comments

Comments
 (0)