[identity] [keyvault] Fix long timeout in KeyVault with MSI (Azure#23035)

* Fix long timeout in KeyVault with MSI

Author: xirzec

sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts

@@ -1,11 +1,10 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { delay } from "@azure/core-util";
+import { delay, isError } from "@azure/core-util";
 import { AccessToken, GetTokenOptions } from "@azure/core-auth";
 import {
   PipelineRequestOptions,
-  RestError,
   createHttpHeaders,
   createPipelineRequest,
 } from "@azure/core-rest-pipeline";
@@ -141,46 +140,35 @@ export const imdsMsi: MSI = {
       getTokenOptions,
       async (options) => {
         requestOptions.tracingOptions = options.tracingOptions;
+
+        // Create a request with a timeout since we expect that
+        // not having a "Metadata" header should cause an error to be
+        // returned quickly from the endpoint, proving its availability.
+        const request = createPipelineRequest(requestOptions);
+
+        // Default to 300 if the default of 0 is used.
+        // Negative values can still be used to disable the timeout.
+        request.timeout = options.requestOptions?.timeout || 300;
+
+        // This MSI uses the imdsEndpoint to get the token, which only uses http://
+        request.allowInsecureConnection = true;
+
         try {
-          // Create a request with a timeout since we expect that
-          // not having a "Metadata" header should cause an error to be
-          // returned quickly from the endpoint, proving its availability.
-          const request = createPipelineRequest(requestOptions);
-
-          request.timeout = options.requestOptions?.timeout ?? 300;
-
-          // This MSI uses the imdsEndpoint to get the token, which only uses http://
-          request.allowInsecureConnection = true;
-
-          try {
-            logger.info(`${msiName}: Pinging the Azure IMDS endpoint`);
-            await identityClient.sendRequest(request);
-          } catch (err: any) {
-            if (
-              (err.name === "RestError" && err.code === RestError.REQUEST_SEND_ERROR) ||
-              err.name === "AbortError" ||
-              err.code === "ENETUNREACH" || // Network unreachable
-              err.code === "ECONNREFUSED" || // connection refused
-              err.code === "EHOSTDOWN" // host is down
-            ) {
-              // If the request failed, or Node.js was unable to establish a connection,
-              // or the host was down, we'll assume the IMDS endpoint isn't available.
-              logger.info(`${msiName}: The Azure IMDS endpoint is unavailable`);
-              return false;
-            }
+          logger.info(`${msiName}: Pinging the Azure IMDS endpoint`);
+          await identityClient.sendRequest(request);
+        } catch (err: unknown) {
+          // If the request failed, or Node.js was unable to establish a connection,
+          // or the host was down, we'll assume the IMDS endpoint isn't available.
+          if (isError(err)) {
+            logger.verbose(`${msiName}: Caught error ${err.name}: ${err.message}`);
           }
-
-          // If we received any response, the endpoint is available
-          logger.info(`${msiName}: The Azure IMDS endpoint is available`);
-          return true;
-        } catch (err: any) {
-          // createWebResource failed.
-          // This error should bubble up to the user.
-          logger.info(
-            `${msiName}: Error when creating the WebResource for the Azure IMDS endpoint: ${err.message}`
-          );
-          throw err;
+          logger.info(`${msiName}: The Azure IMDS endpoint is unavailable`);
+          return false;
         }
+
+        // If we received any response, the endpoint is available
+        logger.info(`${msiName}: The Azure IMDS endpoint is available`);
+        return true;
       }
     );
   },
@@ -190,9 +178,13 @@ export const imdsMsi: MSI = {
   ): Promise<AccessToken | null> {
     const { identityClient, scopes, clientId, resourceId } = configuration;
 
-    logger.info(
-      `${msiName}: Using the Azure IMDS endpoint coming from the environment variable MSI_ENDPOINT=${process.env.MSI_ENDPOINT}, and using the cloud shell to proceed with the authentication.`
-    );
+    if (process.env.AZURE_POD_IDENTITY_AUTHORITY_HOST) {
+      logger.info(
+        `${msiName}: Using the Azure IMDS endpoint coming from the environment variable AZURE_POD_IDENTITY_AUTHORITY_HOST=${process.env.AZURE_POD_IDENTITY_AUTHORITY_HOST}.`
+      );
+    } else {
+      logger.info(`${msiName}: Using the default Azure IMDS endpoint ${imdsHost}.`);
+    }
 
     let nextDelayInMs = imdsMsiRetryConfig.startDelayInMs;
     for (let retries = 0; retries < imdsMsiRetryConfig.maxRetries; retries++) {

sdk/identity/identity/src/util/logging.ts

@@ -70,6 +70,7 @@ export interface CredentialLoggerInstance {
   fullTitle: string;
   info(message: string): void;
   warning(message: string): void;
+  verbose(message: string): void;
   /**
    * The logging functions for warning and error are intentionally left out, since we want the identity logging to be at the info level.
    * Otherwise, they would look like:
@@ -101,11 +102,16 @@ export function credentialLoggerInstance(
   function warning(message: string): void {
     log.warning(`${fullTitle} =>`, message);
   }
+
+  function verbose(message: string): void {
+    log.verbose(`${fullTitle} =>`, message);
+  }
   return {
     title,
     fullTitle,
     info,
     warning,
+    verbose,
   };
 }
 

sdk/keyvault/keyvault-common/src/challengeBasedAuthenticationPolicy.ts

@@ -58,7 +58,7 @@ export function createChallengeCallbacks(): ChallengeCallbacks {
     return {
       abortSignal: request.abortSignal,
       requestOptions: {
-        timeout: request.timeout,
+        timeout: request.timeout > 0 ? request.timeout : undefined,
       },
       tracingOptions: request.tracingOptions,
     };