From c5d44f2e782e1d25c6c2da8c8e18417dee7b6de7 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Tue, 6 May 2025 15:43:05 +0000 Subject: [PATCH 1/3] chore(node-config-provider): pass options with signing name to environment variable selector --- .../node-config-provider/src/configLoader.ts | 6 +++--- packages/node-config-provider/src/fromEnv.ts | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/node-config-provider/src/configLoader.ts b/packages/node-config-provider/src/configLoader.ts index f3ccab6eb4c..6766fb0a279 100644 --- a/packages/node-config-provider/src/configLoader.ts +++ b/packages/node-config-provider/src/configLoader.ts @@ -1,14 +1,14 @@ import { chain, memoize } from "@smithy/property-provider"; import { Provider } from "@smithy/types"; -import { fromEnv, GetterFromEnv } from "./fromEnv"; +import { ClientOptions, fromEnv, GetterFromEnv } from "./fromEnv"; import { fromSharedConfigFiles, GetterFromConfig, SharedConfigInit } from "./fromSharedConfigFiles"; import { fromStatic, FromStaticConfig } from "./fromStatic"; /** * @internal */ -export type LocalConfigOptions = SharedConfigInit; +export type LocalConfigOptions = SharedConfigInit & ClientOptions; /** * @internal @@ -39,7 +39,7 @@ export const loadConfig = ( ): Provider => memoize( chain( - fromEnv(environmentVariableSelector), + fromEnv(environmentVariableSelector, { signingName: configuration.signingName }), fromSharedConfigFiles(configFileSelector, configuration), fromStatic(defaultValue) ) diff --git a/packages/node-config-provider/src/fromEnv.ts b/packages/node-config-provider/src/fromEnv.ts index 33941b27fcc..e7553fded3c 100644 --- a/packages/node-config-provider/src/fromEnv.ts +++ b/packages/node-config-provider/src/fromEnv.ts @@ -1,28 +1,34 @@ import { CredentialsProviderError } from "@smithy/property-provider"; -import { Logger, Provider } from "@smithy/types"; +import { Provider } from "@smithy/types"; import { getSelectorName } from "./getSelectorName"; +export interface ClientOptions { + /** + * The SigV4 service signing name. + */ + signingName?: string; +} + // Using Record instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments -export type GetterFromEnv = (env: Record) => T | undefined; +export type GetterFromEnv = (env: Record, options?: ClientOptions) => T | undefined; /** * Get config value given the environment variable name or getter from * environment variable. */ export const fromEnv = - (envVarSelector: GetterFromEnv, logger?: Logger): Provider => + (envVarSelector: GetterFromEnv, options?: ClientOptions): Provider => async () => { try { - const config = envVarSelector(process.env); + const config = envVarSelector(process.env, options); if (config === undefined) { throw new Error(); } return config as T; } catch (e) { throw new CredentialsProviderError( - e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}`, - { logger } + e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}` ); } }; From f30a357bc0409822cce2357fe94b92532308793e Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Tue, 6 May 2025 15:46:02 +0000 Subject: [PATCH 2/3] chore: yarn changeset --- .changeset/rotten-chairs-float.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rotten-chairs-float.md diff --git a/.changeset/rotten-chairs-float.md b/.changeset/rotten-chairs-float.md new file mode 100644 index 00000000000..f9f07eeea08 --- /dev/null +++ b/.changeset/rotten-chairs-float.md @@ -0,0 +1,5 @@ +--- +"@smithy/node-config-provider": minor +--- + +Pass options with signing name to environment variable selector From cb43e9a144a3ce372990226432245edf8afd2723 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Tue, 6 May 2025 16:14:30 +0000 Subject: [PATCH 3/3] test(node-config-provider): pass options with signing name to environment variable selector --- .../src/configLoader.spec.ts | 24 ++++++++++++++++- .../node-config-provider/src/configLoader.ts | 12 +++++---- .../node-config-provider/src/fromEnv.spec.ts | 27 ++++++++++++++----- packages/node-config-provider/src/fromEnv.ts | 9 ++++--- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/packages/node-config-provider/src/configLoader.spec.ts b/packages/node-config-provider/src/configLoader.spec.ts index 995916c4de5..b6b8c061633 100644 --- a/packages/node-config-provider/src/configLoader.spec.ts +++ b/packages/node-config-provider/src/configLoader.spec.ts @@ -39,7 +39,7 @@ describe("loadConfig", () => { configuration ); expect(fromEnv).toHaveBeenCalledTimes(1); - expect(fromEnv).toHaveBeenCalledWith(envVarSelector); + expect(fromEnv).toHaveBeenCalledWith(envVarSelector, undefined); expect(fromSharedConfigFiles).toHaveBeenCalledTimes(1); expect(fromSharedConfigFiles).toHaveBeenCalledWith(configKey, configuration); expect(fromStatic).toHaveBeenCalledTimes(1); @@ -62,4 +62,26 @@ describe("loadConfig", () => { vi.mocked(memoize).mockReturnValueOnce(mockMemoizeReturn); expect(loadConfig({} as any)).toEqual(mockMemoizeReturn); }); + + it("passes signingName in options object of fromEnv()", () => { + const configWithSigningName = { + ...configuration, + signingName: "signingName", + }; + const envVarSelector = (env: Record) => env["AWS_CONFIG_FOO"]; + const configKey = (profile: Profile) => profile["aws_config_foo"]; + const defaultValue = "foo-value"; + + loadConfig( + { + environmentVariableSelector: envVarSelector, + configFileSelector: configKey, + default: defaultValue, + }, + configWithSigningName + ); + + expect(fromEnv).toHaveBeenCalledTimes(1); + expect(fromEnv).toHaveBeenCalledWith(envVarSelector, { signingName: configWithSigningName.signingName }); + }); }); diff --git a/packages/node-config-provider/src/configLoader.ts b/packages/node-config-provider/src/configLoader.ts index 6766fb0a279..08b4de3f84d 100644 --- a/packages/node-config-provider/src/configLoader.ts +++ b/packages/node-config-provider/src/configLoader.ts @@ -1,14 +1,14 @@ import { chain, memoize } from "@smithy/property-provider"; import { Provider } from "@smithy/types"; -import { ClientOptions, fromEnv, GetterFromEnv } from "./fromEnv"; +import { EnvOptions, fromEnv, GetterFromEnv } from "./fromEnv"; import { fromSharedConfigFiles, GetterFromConfig, SharedConfigInit } from "./fromSharedConfigFiles"; import { fromStatic, FromStaticConfig } from "./fromStatic"; /** * @internal */ -export type LocalConfigOptions = SharedConfigInit & ClientOptions; +export type LocalConfigOptions = SharedConfigInit & EnvOptions; /** * @internal @@ -36,11 +36,13 @@ export interface LoadedConfigSelectors { export const loadConfig = ( { environmentVariableSelector, configFileSelector, default: defaultValue }: LoadedConfigSelectors, configuration: LocalConfigOptions = {} -): Provider => - memoize( +): Provider => { + const envOptions = configuration.signingName ? { signingName: configuration.signingName } : undefined; + return memoize( chain( - fromEnv(environmentVariableSelector, { signingName: configuration.signingName }), + fromEnv(environmentVariableSelector, envOptions), fromSharedConfigFiles(configFileSelector, configuration), fromStatic(defaultValue) ) ); +}; diff --git a/packages/node-config-provider/src/fromEnv.spec.ts b/packages/node-config-provider/src/fromEnv.spec.ts index 084468a0ee1..06efe555bc6 100644 --- a/packages/node-config-provider/src/fromEnv.spec.ts +++ b/packages/node-config-provider/src/fromEnv.spec.ts @@ -1,21 +1,28 @@ import { CredentialsProviderError } from "@smithy/property-provider"; -import { afterAll, beforeEach, describe, expect, test as it, vi } from "vitest"; +import { afterAll, afterEach, beforeEach, describe, expect, test as it, vi } from "vitest"; -import { fromEnv, GetterFromEnv } from "./fromEnv"; +import { fromEnv } from "./fromEnv"; describe("fromEnv", () => { describe("with env var getter", () => { const ENV_VAR_NAME = "ENV_VAR_NAME"; - // Using Record instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments - const envVarGetter: GetterFromEnv = (env: Record) => env[ENV_VAR_NAME]!; + const envVarGetter = vi.fn(); const envVarValue = process.env[ENV_VAR_NAME]; const mockEnvVarValue = "mockEnvVarValue"; beforeEach(() => { + envVarGetter.mockImplementation((env: Record) => { + if (env[ENV_VAR_NAME]) return env[ENV_VAR_NAME]; + throw new CredentialsProviderError(`Not found in ENV: ${ENV_VAR_NAME}`); + }); delete process.env[ENV_VAR_NAME]; }); + afterEach(() => { + vi.clearAllMocks(); + }); + afterAll(() => { process.env[ENV_VAR_NAME] = envVarValue; }); @@ -27,9 +34,17 @@ describe("fromEnv", () => { }); }); - it(`returns string value in '${ENV_VAR_NAME}' env var when set`, () => { + it(`returns string value in '${ENV_VAR_NAME}' env var when set`, async () => { + process.env[ENV_VAR_NAME] = mockEnvVarValue; + await expect(fromEnv(envVarGetter)()).resolves.toBe(mockEnvVarValue); + expect(envVarGetter).toHaveBeenCalledWith(process.env, undefined); + }); + + it(`passes options to envVarSelector if it's set`, async () => { process.env[ENV_VAR_NAME] = mockEnvVarValue; - return expect(fromEnv(envVarGetter)()).resolves.toBe(mockEnvVarValue); + const options = { signingName: "signingName" }; + await expect(fromEnv(envVarGetter, options)()).resolves.toBe(mockEnvVarValue); + expect(envVarGetter).toHaveBeenCalledWith(process.env, options); }); it("return complex value from the getter", () => { diff --git a/packages/node-config-provider/src/fromEnv.ts b/packages/node-config-provider/src/fromEnv.ts index e7553fded3c..78b8139e609 100644 --- a/packages/node-config-provider/src/fromEnv.ts +++ b/packages/node-config-provider/src/fromEnv.ts @@ -3,7 +3,10 @@ import { Provider } from "@smithy/types"; import { getSelectorName } from "./getSelectorName"; -export interface ClientOptions { +/** + * @internal + */ +export interface EnvOptions { /** * The SigV4 service signing name. */ @@ -11,14 +14,14 @@ export interface ClientOptions { } // Using Record instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments -export type GetterFromEnv = (env: Record, options?: ClientOptions) => T | undefined; +export type GetterFromEnv = (env: Record, options?: EnvOptions) => T | undefined; /** * Get config value given the environment variable name or getter from * environment variable. */ export const fromEnv = - (envVarSelector: GetterFromEnv, options?: ClientOptions): Provider => + (envVarSelector: GetterFromEnv, options?: EnvOptions): Provider => async () => { try { const config = envVarSelector(process.env, options);