fix(credential-provider-node): read config and credentials files only once (#2045)

* chore(credential-provider-node): refactor provider chain

* fix(credential-provider-node): read config files only once

* docs(credential-provider-*): mark laodedConfigs param internal

* chore(credential-provider-node): make functions arrow

Author: AllanZhengYP

packages/credential-provider-ini/src/index.ts

@@ -53,6 +53,8 @@ export interface FromIniInit extends SharedConfigInit {
   /**
    * A promise that will be resolved with loaded and parsed credentials files.
    * Used to avoid loading shared config files multiple times.
+   *
+   * @internal
    */
   loadedConfig?: Promise<SharedConfigFiles>;
 

packages/credential-provider-node/package.json

@@ -31,6 +31,7 @@
     "@aws-sdk/credential-provider-ini": "3.4.1",
     "@aws-sdk/credential-provider-process": "3.4.1",
     "@aws-sdk/property-provider": "3.4.1",
+    "@aws-sdk/shared-ini-file-loader": "3.4.1",
     "@aws-sdk/types": "3.4.1",
     "tslib": "^1.8.0"
   },

packages/credential-provider-node/src/index.spec.ts

@@ -10,6 +10,17 @@ jest.mock("@aws-sdk/credential-provider-env", () => {
 });
 import { fromEnv } from "@aws-sdk/credential-provider-env";
 
+const loadedConfig = {
+  credentialsFile: {
+    foo: { aws_access_key_id: "key", aws_secret_access_key: "secret" },
+  },
+  configFile: { bar: { aws_access_key_id: "key", aws_secret_access_key: "secret" } },
+};
+jest.mock("@aws-sdk/shared-ini-file-loader", () => ({
+  loadSharedConfigFiles: jest.fn().mockReturnValue(loadedConfig),
+}));
+import { loadSharedConfigFiles } from "@aws-sdk/shared-ini-file-loader";
+
 jest.mock("@aws-sdk/credential-provider-ini", () => {
   const iniProvider = jest.fn();
   return {
@@ -39,6 +50,7 @@ jest.mock("@aws-sdk/credential-provider-imds", () => {
     ENV_CMDS_RELATIVE_URI: "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI",
   };
 });
+
 import {
   ENV_CMDS_FULL_URI,
   ENV_CMDS_RELATIVE_URI,
@@ -78,6 +90,7 @@ beforeEach(() => {
   (fromProcess as any).mockClear();
   (fromContainerMetadata as any).mockClear();
   (fromInstanceMetadata as any).mockClear();
+  (loadSharedConfigFiles as any).mockClear();
 });
 
 afterAll(() => {
@@ -200,6 +213,23 @@ describe("defaultProvider", () => {
     expect((fromInstanceMetadata() as any).mock.calls.length).toBe(0);
   });
 
+  it("should read config files only once for all providers", async () => {
+    const creds = {
+      accessKeyId: "foo",
+      secretAccessKey: "bar",
+    };
+
+    (fromEnv() as any).mockImplementation(() => Promise.reject(new ProviderError("Keep moving!")));
+    (fromIni() as any).mockImplementation(() => Promise.reject(new ProviderError("Nothing here!")));
+    (fromProcess() as any).mockImplementation(() => Promise.reject(new ProviderError("Nor here!")));
+    (fromInstanceMetadata() as any).mockImplementation(() => Promise.resolve(creds));
+
+    await expect(defaultProvider()()).resolves;
+    expect((loadSharedConfigFiles as any).mock.calls.length).toBe(1);
+    expect((fromIni as any).mock.calls[1][0]).toMatchObject({ loadedConfig: loadSharedConfigFiles() });
+    expect((fromProcess as any).mock.calls[1][0]).toMatchObject({ loadedConfig: loadSharedConfigFiles() });
+  });
+
   it("should pass configuration on to the ini provider", async () => {
     const iniConfig: FromIniInit = {
       profile: "foo",
@@ -226,7 +256,7 @@ describe("defaultProvider", () => {
     await expect(defaultProvider(iniConfig)()).resolves;
 
     expect((fromIni as any).mock.calls.length).toBe(1);
-    expect((fromIni as any).mock.calls[0][0]).toBe(iniConfig);
+    expect((fromIni as any).mock.calls[0][0]).toEqual({ ...iniConfig, loadedConfig });
   });
 
   it("should pass configuration on to the process provider", async () => {
@@ -249,7 +279,7 @@ describe("defaultProvider", () => {
     await expect(defaultProvider(processConfig)()).resolves;
     expect((fromProcess as any).mock.calls.length).toBe(1);
     expect((fromProcess as any).mock.calls.length).toBe(1);
-    expect((fromProcess as any).mock.calls[0][0]).toBe(processConfig);
+    expect((fromProcess as any).mock.calls[0][0]).toEqual({ ...processConfig, loadedConfig });
   });
 
   it("should pass configuration on to the IMDS provider", async () => {
@@ -273,7 +303,7 @@ describe("defaultProvider", () => {
     await expect(defaultProvider(imdsConfig)()).resolves;
 
     expect((fromInstanceMetadata as any).mock.calls.length).toBe(1);
-    expect((fromInstanceMetadata as any).mock.calls[0][0]).toBe(imdsConfig);
+    expect((fromInstanceMetadata as any).mock.calls[0][0]).toEqual({ ...imdsConfig, loadedConfig });
   });
 
   it("should pass configuration on to the ECS IMDS provider", async () => {
@@ -298,7 +328,7 @@ describe("defaultProvider", () => {
     await expect(defaultProvider(ecsImdsConfig)()).resolves;
 
     expect((fromContainerMetadata as any).mock.calls.length).toBe(1);
-    expect((fromContainerMetadata as any).mock.calls[0][0]).toBe(ecsImdsConfig);
+    expect((fromContainerMetadata as any).mock.calls[0][0]).toEqual({ ...ecsImdsConfig, loadedConfig });
   });
 
   it("should return the same promise across invocations", async () => {

packages/credential-provider-node/src/index.ts

@@ -9,6 +9,7 @@ import {
 import { ENV_PROFILE, fromIni, FromIniInit } from "@aws-sdk/credential-provider-ini";
 import { fromProcess, FromProcessInit } from "@aws-sdk/credential-provider-process";
 import { chain, memoize, ProviderError } from "@aws-sdk/property-provider";
+import { loadSharedConfigFiles } from "@aws-sdk/shared-ini-file-loader";
 import { CredentialProvider } from "@aws-sdk/types";
 
 export const ENV_IMDS_DISABLED = "AWS_EC2_METADATA_DISABLED";
@@ -41,20 +42,21 @@ export const ENV_IMDS_DISABLED = "AWS_EC2_METADATA_DISABLED";
  * @see fromContainerMetadata   The function used to source credentials from the
  *                              ECS Container Metadata Service
  */
-export function defaultProvider(init: FromIniInit & RemoteProviderInit & FromProcessInit = {}): CredentialProvider {
-  const { profile = process.env[ENV_PROFILE] } = init;
-  const providerChain = profile
-    ? chain(fromIni(init), fromProcess(init))
-    : chain(fromEnv(), fromIni(init), fromProcess(init), remoteProvider(init));
+export const defaultProvider = (init: FromIniInit & RemoteProviderInit & FromProcessInit = {}): CredentialProvider => {
+  const options = { profile: process.env[ENV_PROFILE], ...init };
+  if (!options.loadedConfig) options.loadedConfig = loadSharedConfigFiles(init);
+  const providers = [fromIni(options), fromProcess(options), remoteProvider(options)];
+  if (!options.profile) providers.unshift(fromEnv());
+  const providerChain = chain(...providers);
 
   return memoize(
     providerChain,
     (credentials) => credentials.expiration !== undefined && credentials.expiration.getTime() - Date.now() < 300000,
     (credentials) => credentials.expiration !== undefined
   );
-}
+};
 
-function remoteProvider(init: RemoteProviderInit): CredentialProvider {
+const remoteProvider = (init: RemoteProviderInit): CredentialProvider => {
   if (process.env[ENV_CMDS_RELATIVE_URI] || process.env[ENV_CMDS_FULL_URI]) {
     return fromContainerMetadata(init);
   }
@@ -64,4 +66,4 @@ function remoteProvider(init: RemoteProviderInit): CredentialProvider {
   }
 
   return fromInstanceMetadata(init);
-}
+};

packages/credential-provider-process/src/index.ts

@@ -4,6 +4,9 @@ import { ParsedIniData, SharedConfigFiles, SharedConfigInit } from "@aws-sdk/sha
 import { CredentialProvider, Credentials } from "@aws-sdk/types";
 import { exec } from "child_process";
 
+/**
+ * @internal
+ */
 export const ENV_PROFILE = "AWS_PROFILE";
 
 export interface FromProcessInit extends SharedConfigInit {
@@ -15,6 +18,8 @@ export interface FromProcessInit extends SharedConfigInit {
   /**
    * A promise that will be resolved with loaded and parsed credentials files.
    * Used to avoid loading shared config files multiple times.
+   *
+   * @internal
    */
   loadedConfig?: Promise<SharedConfigFiles>;
 }

packages/node-config-provider/src/fromSharedConfigFiles.ts

@@ -26,6 +26,8 @@ export interface SharedConfigInit extends BaseSharedConfigInit {
   /**
    * A promise that will be resolved with loaded and parsed credentials files.
    * Used to avoid loading shared config files multiple times.
+   *
+   * @internal
    */
   loadedConfig?: Promise<SharedConfigFiles>;
 }