Skip to content

Commit 358e1bb

Browse files
smilkurismilkuri
committed
address PR comments
1 parent 0c3d68d commit 358e1bb

File tree

2 files changed

+37
-40
lines changed

2 files changed

+37
-40
lines changed

‎packages/credential-provider-imds/src/fromInstanceMetadata.spec.ts

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import { CredentialsProviderError } from "@smithy/property-provider";
33
import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest";
44

55
import { InstanceMetadataV1FallbackError } from "./error/InstanceMetadataV1FallbackError";
6-
import { checkIfImdsDisabled, fromInstanceMetadata } from "./fromInstanceMetadata";
7-
import * as fromInstanceMetadataModule from "./fromInstanceMetadata";
6+
import { fromInstanceMetadata,getConfiguredProfileName, getImdsProfile, throwIfImdsTurnedOff } from "./fromInstanceMetadata";
87
import { httpRequest } from "./remoteProvider/httpRequest";
98
import { fromImdsCredentials, isImdsCredentials } from "./remoteProvider/ImdsCredentials";
109
import { providerConfigFromInit } from "./remoteProvider/RemoteProviderInit";
@@ -67,7 +66,7 @@ describe("fromInstanceMetadata", () => {
6766
vi.mocked(staticStabilityProvider).mockImplementation((input) => input);
6867
vi.mocked(getInstanceMetadataEndpoint).mockResolvedValue({ hostname } as any);
6968
vi.mocked(loadConfig).mockReturnValue(() => Promise.resolve(false));
70-
vi.spyOn(fromInstanceMetadataModule, "checkIfImdsDisabled").mockResolvedValue(undefined);
69+
vi.spyOn({throwIfImdsTurnedOff}, "throwIfImdsTurnedOff").mockResolvedValue(undefined);
7170
(isImdsCredentials as unknown as any).mockReturnValue(true);
7271
vi.mocked(providerConfigFromInit).mockReturnValue({
7372
timeout: mockTimeout,
@@ -80,8 +79,16 @@ describe("fromInstanceMetadata", () => {
8079
});
8180

8281
it("returns no credentials when AWS_EC2_METADATA_DISABLED=true", async () => {
83-
vi.mocked(loadConfig).mockReturnValueOnce(() => Promise.resolve(true));
84-
vi.mocked(fromInstanceMetadataModule.checkIfImdsDisabled).mockRejectedValueOnce(
82+
vi.mocked(loadConfig).mockImplementation(({ environmentVariableSelector, configFileSelector }) => {
83+
if (environmentVariableSelector && environmentVariableSelector({ AWS_EC2_METADATA_DISABLED: "true" })) {
84+
return () => Promise.resolve(true);
85+
}
86+
if (configFileSelector && configFileSelector({ imds_disabled: "true" })) {
87+
return () => Promise.resolve(true);
88+
}
89+
return () => Promise.resolve(false);
90+
});
91+
vi.spyOn({throwIfImdsTurnedOff},"throwIfImdsTurnedOff").mockRejectedValueOnce(
8592
new CredentialsProviderError("IMDS credential fetching is disabled")
8693
);
8794
const provider = fromInstanceMetadata({});
@@ -100,10 +107,10 @@ describe("fromInstanceMetadata", () => {
100107
vi.mocked(retry).mockImplementation((fn: any) => fn());
101108
vi.mocked(fromImdsCredentials).mockReturnValue(mockCreds);
102109

103-
const result = await fromInstanceMetadata({ ec2InstanceProfileName: profileName })();
110+
const credentials = await fromInstanceMetadata({ ec2InstanceProfileName: profileName })();
104111

105-
expect(result).toEqual(mockCreds);
106-
expect(result.accountId).toBe(mockCreds.accountId);
112+
expect(credentials).toEqual(mockCreds);
113+
expect(credentials.accountId).toBe(mockCreds.accountId);
107114

108115
expect(httpRequest).toHaveBeenCalledTimes(2);
109116
expect(httpRequest).toHaveBeenNthCalledWith(1, mockTokenRequestOptions);
@@ -124,10 +131,10 @@ describe("fromInstanceMetadata", () => {
124131

125132
const provider = fromInstanceMetadata({});
126133

127-
const result = await provider();
134+
const credentials = await provider();
128135

129-
expect(result).toEqual(mockCreds);
130-
expect(result.accountId).toBe(mockCreds.accountId);
136+
expect(credentials).toEqual(mockCreds);
137+
expect(credentials.accountId).toBe(mockCreds.accountId);
131138

132139
expect(httpRequest).toHaveBeenCalledTimes(3);
133140
expect(httpRequest).toHaveBeenNthCalledWith(1, mockTokenRequestOptions);
@@ -165,7 +172,7 @@ describe("fromInstanceMetadata", () => {
165172

166173
vi.mocked(retry).mockImplementation((fn: any) => fn());
167174
vi.mocked(fromImdsCredentials).mockReturnValue(mockCreds);
168-
vi.mocked(checkIfImdsDisabled).mockReturnValueOnce(Promise.resolve());
175+
vi.spyOn({throwIfImdsTurnedOff}, "throwIfImdsTurnedOff").mockResolvedValue();
169176

170177
await expect(fromInstanceMetadata()()).resolves.toEqual(mockCreds);
171178
expect(httpRequest).toHaveBeenNthCalledWith(3, {
@@ -282,7 +289,7 @@ describe("fromInstanceMetadata", () => {
282289
expect(vi.mocked(staticStabilityProvider)).toBeCalledTimes(1);
283290
});
284291

285-
describe("getImdsProfileHelper", () => {
292+
describe("getImdsProfile", () => {
286293
beforeEach(() => {
287294
vi.mocked(httpRequest).mockClear();
288295
vi.mocked(loadConfig).mockClear();
@@ -291,29 +298,27 @@ describe("fromInstanceMetadata", () => {
291298

292299
it("uses ec2InstanceProfileName from init if provided", async () => {
293300
const profileName = "profile-from-init";
294-
const options = { hostname } as any;
301+
const options = { hostname } ;
295302

296-
// Only use vi.spyOn for imported functions
297-
vi.spyOn(fromInstanceMetadataModule, "getConfiguredProfileName").mockResolvedValueOnce(profileName);
303+
vi.spyOn({getConfiguredProfileName}, "getConfiguredProfileName").mockResolvedValueOnce(profileName);
298304

299-
const result = await fromInstanceMetadataModule.getImdsProfileHelper(options, mockMaxRetries, {
305+
const credentials = await getImdsProfile(options, mockMaxRetries, {
300306
ec2InstanceProfileName: profileName,
301307
});
302308

303-
expect(result).toBe(profileName);
309+
expect(credentials).toBe(profileName);
304310
expect(httpRequest).not.toHaveBeenCalled();
305311
});
306312

307313
it("uses environment variable if ec2InstanceProfileName not provided", async () => {
308314
const envProfileName = "profile-from-env";
309315
const options = { hostname } as any;
310316

311-
// Mock loadConfig to simulate env variable present
312317
vi.mocked(loadConfig).mockReturnValue(() => Promise.resolve(envProfileName));
313318

314-
const result = await fromInstanceMetadataModule.getImdsProfileHelper(options, mockMaxRetries, {});
319+
const credentials = await getImdsProfile(options, mockMaxRetries, {});
315320

316-
expect(result).toBe(envProfileName);
321+
expect(credentials).toBe(envProfileName);
317322
expect(httpRequest).not.toHaveBeenCalled();
318323
});
319324

@@ -322,21 +327,19 @@ describe("fromInstanceMetadata", () => {
322327
const legacyProfileName = "profile-from-legacy";
323328
const options = { hostname } as any;
324329

325-
// 1. Simulate config file present: should return configProfileName, no IMDS call
326330
vi.mocked(loadConfig).mockReturnValue(() => Promise.resolve(configProfileName));
327331

328-
let result = await fromInstanceMetadataModule.getImdsProfileHelper(options, mockMaxRetries, {});
329-
expect(result).toBe(configProfileName);
332+
let credentials = await getImdsProfile(options, mockMaxRetries, {});
333+
expect(credentials).toBe(configProfileName);
330334
expect(httpRequest).not.toHaveBeenCalled();
331335

332-
// 2. Simulate config file missing: should call IMDS (extended fails, legacy succeeds)
333336
vi.mocked(loadConfig).mockReturnValue(() => Promise.resolve(null));
334337
vi.mocked(httpRequest)
335338
.mockRejectedValueOnce(Object.assign(new Error(), { statusCode: 404 }))
336339
.mockResolvedValueOnce(legacyProfileName as any);
337340

338-
result = await fromInstanceMetadataModule.getImdsProfileHelper(options, mockMaxRetries, {});
339-
expect(result).toBe(legacyProfileName);
341+
credentials = await getImdsProfile(options, mockMaxRetries, {});
342+
expect(credentials).toBe(legacyProfileName);
340343
expect(httpRequest).toHaveBeenCalledTimes(2);
341344
expect(httpRequest).toHaveBeenNthCalledWith(1, {
342345
...options,

‎packages/credential-provider-imds/src/fromInstanceMetadata.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ const getInstanceMetadataProvider = (init: RemoteProviderInit = {}) => {
4848
const isImdsV1Fallback = disableFetchToken || options.headers?.[X_AWS_EC2_METADATA_TOKEN] == null;
4949

5050
if (isImdsV1Fallback) {
51-
await checkIfImdsDisabled(profile, logger);
51+
await throwIfImdsTurnedOff(profile, logger);
5252
let fallbackBlockedFromProfile = false;
5353
let fallbackBlockedFromProcessEnv = false;
5454

@@ -93,7 +93,7 @@ const getInstanceMetadataProvider = (init: RemoteProviderInit = {}) => {
9393
}
9494
}
9595

96-
const imdsProfile = await getImdsProfileHelper(options, maxRetries, init, profile);
96+
const imdsProfile = await getImdsProfile(options, maxRetries, init, profile);
9797

9898
return retry(async () => {
9999
let creds: AwsCredentialIdentity;
@@ -111,7 +111,7 @@ const getInstanceMetadataProvider = (init: RemoteProviderInit = {}) => {
111111

112112
return async () => {
113113
const endpoint = await getInstanceMetadataEndpoint();
114-
await checkIfImdsDisabled(profile, logger);
114+
await throwIfImdsTurnedOff(profile, logger);
115115
if (disableFetchToken) {
116116
logger?.debug("AWS SDK Instance Metadata", "using v1 fallback (no token fetch)");
117117
return getCredentials(maxRetries, { ...endpoint, timeout });
@@ -145,7 +145,7 @@ const getInstanceMetadataProvider = (init: RemoteProviderInit = {}) => {
145145
* @internal
146146
* Gets IMDS profile with proper error handling and retries
147147
*/
148-
export const getImdsProfileHelper = async (
148+
export const getImdsProfile = async (
149149
options: RequestOptions,
150150
maxRetries: number,
151151
init: RemoteProviderInit = {},
@@ -169,8 +169,6 @@ export const getImdsProfileHelper = async (
169169
if (resolvedProfile) {
170170
return resolvedProfile;
171171
}
172-
// If no configured name, fetch profile name from IMDS
173-
try {
174172
// Try extended API first
175173
try {
176174
const response = await httpRequest({ ...options, path: IMDS_EXTENDED_PATH });
@@ -189,9 +187,6 @@ export const getImdsProfileHelper = async (
189187
throw error;
190188
}
191189
}
192-
} catch (err) {
193-
throw err;
194-
}
195190
}, maxRetries);
196191
};
197192

@@ -209,7 +204,7 @@ const getMetadataToken = async (options: RequestOptions) =>
209204
* @internal
210205
* Checks if IMDS credential fetching is disabled through configuration
211206
*/
212-
export const checkIfImdsDisabled = async (profile?: string, logger?: any): Promise<void> => {
207+
export const throwIfImdsTurnedOff = async (profile?: string, logger?: any): Promise<void> => {
213208
// Load configuration in priority order
214209
const disableImds = await loadConfig(
215210
{
@@ -252,7 +247,7 @@ export const getConfiguredProfileName = async (init: RemoteProviderInit, profile
252247
)();
253248

254249
// Check runtime config (highest priority)
255-
const name = init.ec2InstanceProfileName || profileName;
250+
const name = init.ec2InstanceProfileName ?? profileName;
256251

257252
// Validate if name is provided but empty
258253
if (typeof name === "string" && name.trim() === "") {
@@ -279,8 +274,7 @@ const getCredentialsFromProfile = async (profile: string, options: RequestOption
279274
if (legacyError.statusCode === 404 && init.ec2InstanceProfileName === undefined) {
280275
// If legacy API also returns 404 and we're using a cached profile name,
281276
// the profile might have changed - clear cache and retry
282-
const resolvedProfile = null;
283-
const newProfileName = await getImdsProfileHelper(options, init.maxRetries ?? 3, init, profile, true);
277+
const newProfileName = await getImdsProfile(options, init.maxRetries ?? 3, init, profile, true);
284278
return getCredentialsFromProfile(newProfileName, options, init);
285279
}
286280
throw legacyError;

0 commit comments

Comments
 (0)