Skip to content

Commit

Permalink
fix(credential-provider-node): read config and credentials files only…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
AllanZhengYP authored Feb 18, 2021
1 parent 76a9bd3 commit 7db14b1
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 12 deletions.
2 changes: 2 additions & 0 deletions packages/credential-provider-ini/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down
1 change: 1 addition & 0 deletions packages/credential-provider-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
38 changes: 34 additions & 4 deletions packages/credential-provider-node/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -78,6 +90,7 @@ beforeEach(() => {
(fromProcess as any).mockClear();
(fromContainerMetadata as any).mockClear();
(fromInstanceMetadata as any).mockClear();
(loadSharedConfigFiles as any).mockClear();
});

afterAll(() => {
Expand Down Expand Up @@ -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",
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
18 changes: 10 additions & 8 deletions packages/credential-provider-node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
}
Expand All @@ -64,4 +66,4 @@ function remoteProvider(init: RemoteProviderInit): CredentialProvider {
}

return fromInstanceMetadata(init);
}
};
5 changes: 5 additions & 0 deletions packages/credential-provider-process/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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>;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/node-config-provider/src/fromSharedConfigFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}
Expand Down

0 comments on commit 7db14b1

Please sign in to comment.