fix(aas): honor HTTP_PROXY/HTTPS_PROXY during API key validation#2405
Conversation
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 1caeef5 | Docs | Datadog PR Page | Give us feedback! |
ava-silver
left a comment
There was a problem hiding this comment.
two quick things before i approve!
There was a problem hiding this comment.
Could we add a base helper test that calls the real validateApiKey() and asserts that its httpRequest receives an EnvHttpProxyAgent dispatcher? The existing AAS test should remain because it covers the user-facing network error, but it mocks validateApiKey() and therefore does not verify the proxy wiring itself.
for example:
test('API key validation uses the environment proxy dispatcher', async () => {
const httpRequest = jest.spyOn(request, 'httpRequest').mockResolvedValue({
config: {},
data: {valid: true},
headers: {},
status: 200,
statusText: 'OK',
})
const validator = newApiKeyValidator({
apiKey: 'api-key',
datadogSite: 'datadoghq.com',
})
await expect(validator.validateApiKey()).resolves.toBe(true)
expect(httpRequest).toHaveBeenCalledWith(
expect.objectContaining({
baseURL: 'https://api.datadoghq.com',
dispatcher: expect.any(EnvHttpProxyAgent),
})
)
})There was a problem hiding this comment.
Good call added packages/base/src/helpers/__tests__/apikey.test.ts which spies on httpRequest directly and asserts an EnvHttpProxyAgent is passed, plus covers the 403, network error, and empty key cases.
87778ed to
7b8c796
Compare
- Pass getProxyDispatcher('') (EnvHttpProxyAgent) into validateApiKey's
httpRequest call so undici routes through proxy env vars
- Split instrument.ts catch block to distinguish network/proxy failures
from a genuinely invalid key, with a clearer error message for each
Rationale: Behind a corporate proxy (e.g. Saint-Gobain), Node's fetch
ignores HTTP_PROXY/HTTPS_PROXY, causing ECONNRESET which was mislabeled
as "Invalid API Key". The fix wires in the existing getProxyDispatcher
helper (already used elsewhere in base) and improves the error message.
This commit made by [/dd:git:commit:quick](https://github.com/DataDog/claude-marketplace/tree/main/dd/commands/git/commit/quick.md)
7b8c796 to
9430d0a
Compare
ava-silver
left a comment
There was a problem hiding this comment.
one nit on wording but overall lgtm
Co-authored-by: Ava Silver <ava.silver@datadoghq.com>
Summary
validateApiKey()inpackages/base/src/helpers/apikey.tscalledhttpRequestwithout adispatcher, so undici'sfetchignoredHTTP_PROXY/HTTPS_PROXY. Behind a corporate proxy this caused anECONNRESET, which was then caught by a blanket catch ininstrument.tsand mislabeled as "Invalid API Key".getProxyDispatcher('')(which returns anEnvHttpProxyAgentreading proxy env vars) into thevalidateApiKeyhttpRequestcall — the same pattern already used bygetRequestBuilderelsewhere in the codebase.instrument.tscatch block so a network/proxy failure reports a clear "network error" message with a hint to checkHTTP_PROXY/HTTPS_PROXY, instead of the misleading "Invalid API Key" message.Fixes: SLES-2940
Test plan
yarn build— cleanyarn test packages/plugin-aas/src/__tests__/instrument.test.ts— passes, including new test for the network-error pathHTTP_PROXY=http://localhost:9 DD_API_KEY=<valid> DD_SITE=datadoghq.com yarn launch aas instrument --resource-id ...→ shows "network error" (proxy used, localhost:9 not listening)DD_API_KEY=<valid> DD_SITE=datadoghq.com yarn launch aas instrument --resource-id ...→ passes API key validation, proceeds to Azure auth (no regression)🤖 Generated with Claude Code