[Identity] Caught up with POD Identity fix added on 1.5.1 (#18054)

Azure · Oct 6, 2021 · 50ba79c · 50ba79c
1 parent 906cecf
commit 50ba79c
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 26 deletions.
diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md
@@ -15,6 +15,7 @@
 ### Bugs Fixed
 
 - Fixed a bug that caused `AzureCliCredential` to fail when a custom tenant ID was provided.
+- Caught up with the bug fixes for Azure POD Identity that were implemented on version 1.5.1.
 
 ### Other Changes
 

diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts
@@ -26,7 +26,7 @@ function expiresInParser(requestBody: any): number {
     // Use the expires_on timestamp if it's available
     const expires = +requestBody.expires_on * 1000;
     logger.info(
-      `${msiName}: IMDS using expires_on: ${expires} (original value: ${requestBody.expires_on})`
+      `${msiName}: Using expires_on: ${expires} (original value: ${requestBody.expires_on})`
     );
     return expires;
   } else {
@@ -41,33 +41,51 @@ function expiresInParser(requestBody: any): number {
 
 function prepareRequestOptions(
   scopes: string | string[],
-  clientId?: string
+  clientId?: string,
+  options?: {
+    skipQuery?: boolean;
+    skipMetadataHeader?: boolean;
+  }
 ): PipelineRequestOptions {
   const resource = mapScopesToResource(scopes);
   if (!resource) {
     throw new Error(`${msiName}: Multiple scopes are not supported.`);
   }
 
-  const queryParameters: any = {
-    resource,
-    "api-version": imdsApiVersion
-  };
-
-  if (clientId) {
-    queryParameters.client_id = clientId;
+  const { skipQuery, skipMetadataHeader } = options || {};
+  let query = "";
+
+  // Pod Identity will try to process this request even if the Metadata header is missing.
+  // We can exclude the request query to ensure no IMDS endpoint tries to process the ping request.
+  if (!skipQuery) {
+    const queryParameters: any = {
+      resource,
+      "api-version": imdsApiVersion
+    };
+    if (clientId) {
+      queryParameters.client_id = clientId;
+    }
+    const params = new URLSearchParams(queryParameters);
+    query = `?${params.toString()}`;
   }
 
-  const params = new URLSearchParams(queryParameters);
-  const query = params.toString();
   const url = new URL(imdsEndpointPath, process.env.AZURE_POD_IDENTITY_AUTHORITY_HOST ?? imdsHost);
 
+  const rawHeaders: Record<string, string> = {
+    Accept: "application/json",
+    Metadata: "true"
+  };
+
+  // Remove the Metadata header to invoke a request error from some IMDS endpoints.
+  if (skipMetadataHeader) {
+    delete rawHeaders.Metadata;
+  }
+
   return {
-    url: `${url}?${query}`,
+    // In this case, the `?` should be added in the "query" variable `skipQuery` is not set.
+    url: `${url}${query}`,
     method: "GET",
-    headers: createHttpHeaders({
-      Accept: "application/json",
-      Metadata: "true"
-    })
+    headers: createHttpHeaders(rawHeaders)
   };
 }
 
@@ -100,15 +118,10 @@ export const imdsMsi: MSI = {
       return true;
     }
 
-    const requestOptions = prepareRequestOptions(resource, clientId);
-
-    // This will always be populated, but let's make TypeScript happy
-    if (requestOptions.headers) {
-      // Remove the Metadata header to invoke a request error from
-      // IMDS endpoint
-      requestOptions.headers.delete("Metadata");
-    }
-
+    const requestOptions = prepareRequestOptions(resource, clientId, {
+      skipMetadataHeader: true,
+      skipQuery: true
+    });
     requestOptions.tracingOptions = options.tracingOptions;
 
     try {

diff --git a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts
@@ -9,7 +9,8 @@ import { RestError } from "@azure/core-rest-pipeline";
 import { ManagedIdentityCredential } from "../../../src";
 import {
   imdsHost,
-  imdsApiVersion
+  imdsApiVersion,
+  imdsEndpointPath
 } from "../../../src/credentials/managedIdentityCredential/constants";
 import {
   imdsMsi,
@@ -69,6 +70,11 @@ describe("ManagedIdentityCredential", function() {
     });
 
     // The first request is the IMDS ping.
+    // This ping request has to skip a header and the query parameters for it to work on POD identity.
+    const imdsPingRequest = authDetails.requests[0];
+    assert.ok(!imdsPingRequest.headers!.metadata);
+    assert.equal(imdsPingRequest.url, new URL(imdsEndpointPath, imdsHost).toString());
+
     // The second one tries to authenticate against IMDS once we know the endpoint is available.
     const authRequest = authDetails.requests[1];