chore: Address PR comments

Author: lqiu96

oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java

@@ -218,11 +218,13 @@ private final GoogleCredentials getDefaultCredentialsUnsynchronized(
       LOGGER.log(Level.FINE, "Attempting to load credentials from GCE");
       credentials = tryGetComputeCredentials(transportFactory);
       // tryGetComputeCredentials can return a null value. This check won't set the source
-      // if the ComputeEngineCredentials fails (prevents a NPE)
+      // if the ComputeEngineCredentials is unable to be created
       if (credentials != null) {
         credentials =
             credentials.withSource(
-                String.format("Metadata Server URL set to %s", getEnv(GCE_METADATA_HOST_ENV_VAR)));
+                String.format(
+                    "Metadata Server URL set to %s",
+                    ComputeEngineCredentials.getMetadataServerUrl(this)));
       }
     }
 

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

@@ -88,18 +88,19 @@ String getCredentialName() {
       return credentialName;
     }
 
+    @Nullable
     String getFileType() {
       return fileType;
     }
   }
 
-  /* The following package-private fields to provide additional info for errors message */
+  // The following package-private fields to provide additional info for errors message
   // Source of the credential (e.g. env var value or well know file location)
   String source;
   // User-friendly name of the Credential class
   String name;
   // Identity of the credential
-  // Note: This field is marked transient to prevent it from being serialized
+  // Note: This field may contain data such as serviceAccountEmail which should not be serialized
   transient String principal;
 
   private final String universeDomain;
@@ -245,24 +246,23 @@ public static GoogleCredentials fromStream(
       throw new IOException("Error reading credentials from stream, 'type' field not specified.");
     }
 
-    if (GoogleCredentialsInfo.USER_CREDENTIALS.getFileType().equals(fileType)) {
+    if (fileType.equals(GoogleCredentialsInfo.USER_CREDENTIALS.getFileType())) {
       return UserCredentials.fromJson(fileContents, transportFactory);
     }
-    if (GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS.getFileType().equals(fileType)) {
+    if (fileType.equals(GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS.getFileType())) {
       return ServiceAccountCredentials.fromJson(fileContents, transportFactory);
     }
-    if (GoogleCredentialsInfo.GDCH_CREDENTIALS.getFileType().equals(fileType)) {
+    if (fileType.equals(GoogleCredentialsInfo.GDCH_CREDENTIALS.getFileType())) {
       return GdchCredentials.fromJson(fileContents);
     }
-    if (GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType().equals(fileType)) {
+    if (fileType.equals(GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType())) {
       return ExternalAccountCredentials.fromJson(fileContents, transportFactory);
     }
-    if (GoogleCredentialsInfo.EXTERNAL_ACCOUNT_AUTHORIZED_USER_CREDENTIALS
-        .getFileType()
-        .equals(fileType)) {
+    if (fileType.equals(
+        GoogleCredentialsInfo.EXTERNAL_ACCOUNT_AUTHORIZED_USER_CREDENTIALS.getFileType())) {
       return ExternalAccountAuthorizedUserCredentials.fromJson(fileContents, transportFactory);
     }
-    if (GoogleCredentialsInfo.IMPERSONATED_CREDENTIALS.getFileType().equals(fileType)) {
+    if (fileType.equals(GoogleCredentialsInfo.IMPERSONATED_CREDENTIALS.getFileType())) {
       return ImpersonatedCredentials.fromJson(fileContents, transportFactory);
     }
     throw new IOException(
@@ -556,9 +556,14 @@ GoogleCredentials withSource(String source) {
    *   <li>principal - Identity used for the credential
    * </ul>
    *
-   * Unknown field values are not included in the mapping (e.g. ComputeCredentials may not know the
-   * principal value until after a call to MDS is made and will be excluded if `getCredentialInfo`
-   * is called prior to retrieving that value)
+   * Unknown field values (i.e. null) are not included in the mapping (e.g. ComputeCredentials may
+   * not know the principal value until after a call to MDS is made and the field will be excluded
+   * if `getCredentialInfo` is called prior to retrieving that value). A new map of the fields is
+   * created on every time this method is called as fields may be updated throughout the Credential
+   * lifecycle. This mapping is intended to provide information about the Credential that is created
+   * via ADC. Some fields may not be known if a Credential is directly created (e.g.
+   * `ServiceAccountCredential.fromStream(InputStream)` may not know the source of the file stream).
+   * These fields are populated on a best effort basis.
    *
    * @return ImmutableMap of information regarding how the Credential was initialized
    */

oauth2_http/javatests/com/google/auth/oauth2/DefaultCredentialsProviderTest.java

@@ -792,6 +792,7 @@ public void getDefaultCredentials_envVarSet_userCredential_correctCredentialInfo
             "Env Var %s set to %s", DefaultCredentialsProvider.CREDENTIAL_ENV_VAR, userPath),
         credentialInfo.get("Credential Source"));
     assertEquals("User Credentials", credentialInfo.get("Credential Name"));
+    // No `account` field
     assertNull(credentialInfo.get("Principal"));
   }
 
@@ -818,15 +819,36 @@ public void getDefaultCredentials_wellKnownFile_userCredential_correctCredential
         String.format("Well Known File at %s", wellKnownFile.getCanonicalPath()),
         credentialInfo.get("Credential Source"));
     assertEquals("User Credentials", credentialInfo.get("Credential Name"));
+    // No `account` field
     assertNull(credentialInfo.get("Principal"));
   }
 
   @Test
-  public void getDefaultCredentials_computeEngineCredentials_correctCredentialInfo()
+  public void getDefaultCredentials_computeEngineCredentials_defaultMDSUrl_correctCredentialInfo()
       throws IOException {
     MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
     TestDefaultCredentialsProvider testProvider = new TestDefaultCredentialsProvider();
-    String gceMetadataHost = "192.0.2.0";
+
+    GoogleCredentials credentials = testProvider.getDefaultCredentials(transportFactory);
+
+    assertNotNull(credentials);
+    assertTrue(credentials instanceof ComputeEngineCredentials);
+    Map<String, String> credentialInfo = credentials.getCredentialInfo();
+    assertEquals(
+        String.format(
+            "Metadata Server URL set to %s", ComputeEngineCredentials.DEFAULT_METADATA_SERVER_URL),
+        credentials.getCredentialInfo().get("Credential Source"));
+    assertEquals("Compute Engine Credentials", credentialInfo.get("Credential Name"));
+    // Principal is null until the first MDS call
+    assertNull(credentialInfo.get("Principal"));
+  }
+
+  @Test
+  public void getDefaultCredentials_computeEngineCredentials_customMDSUrl_correctCredentialInfo()
+      throws IOException {
+    MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
+    TestDefaultCredentialsProvider testProvider = new TestDefaultCredentialsProvider();
+    String gceMetadataHost = "8.8.8.8";
     testProvider.setEnv(DefaultCredentialsProvider.GCE_METADATA_HOST_ENV_VAR, gceMetadataHost);
 
     GoogleCredentials credentials = testProvider.getDefaultCredentials(transportFactory);
@@ -835,9 +857,10 @@ public void getDefaultCredentials_computeEngineCredentials_correctCredentialInfo
     assertTrue(credentials instanceof ComputeEngineCredentials);
     Map<String, String> credentialInfo = credentials.getCredentialInfo();
     assertEquals(
-        String.format("Metadata Server URL set to %s", gceMetadataHost),
+        String.format("Metadata Server URL set to http://%s", gceMetadataHost),
         credentials.getCredentialInfo().get("Credential Source"));
     assertEquals("Compute Engine Credentials", credentialInfo.get("Credential Name"));
+    // Principal is null until the first MDS call
     assertNull(credentialInfo.get("Principal"));
   }