Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions packages/base/src/helpers/__tests__/apikey.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import {EnvHttpProxyAgent} from 'undici'

import {newApiKeyValidator} from '../apikey'
import * as request from '../request'

describe('newApiKeyValidator', () => {
describe('validateApiKey', () => {
test('uses the environment proxy dispatcher', async () => {
const httpRequestSpy = 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(httpRequestSpy).toHaveBeenCalledWith(
expect.objectContaining({
baseURL: 'https://api.datadoghq.com',
dispatcher: expect.any(EnvHttpProxyAgent),
})
)

httpRequestSpy.mockRestore()
})

test('returns false for a 403 response', async () => {
jest.spyOn(request, 'httpRequest').mockRejectedValue(
Object.assign(
new request.RequestError('Forbidden', {}, {data: undefined, status: 403, statusText: 'Forbidden'}),
{
isRequestError: true,
}
)
)

const validator = newApiKeyValidator({apiKey: 'api-key', datadogSite: 'datadoghq.com'})
await expect(validator.validateApiKey()).resolves.toBe(false)

jest.restoreAllMocks()
})

test('rethrows non-403 errors (e.g. network/proxy failure)', async () => {
const networkError = new request.RequestError('read ECONNRESET', {})
jest.spyOn(request, 'httpRequest').mockRejectedValue(networkError)

const validator = newApiKeyValidator({apiKey: 'api-key', datadogSite: 'datadoghq.com'})
await expect(validator.validateApiKey()).rejects.toThrow('read ECONNRESET')

jest.restoreAllMocks()
})

test('returns false for an empty API key', async () => {
const validator = newApiKeyValidator({apiKey: '', datadogSite: 'datadoghq.com'})
await expect(validator.validateApiKey()).resolves.toBe(false)
})
})
})
3 changes: 2 additions & 1 deletion packages/base/src/helpers/apikey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type {BufferedMetricsLogger} from 'datadog-metrics'
import chalk from 'chalk'

import {InvalidConfigurationError} from './errors'
import {httpRequest, isRequestError} from './request'
import {getProxyDispatcher, httpRequest, isRequestError} from './request'
import {datadogRoute} from './request/datadog-route'

/** ApiKeyValidator is an helper interface to interpret Datadog error responses and possibly check the
Expand Down Expand Up @@ -77,6 +77,7 @@ class ApiKeyValidatorImplem {
method: 'GET',
baseURL: `https://api.${this.datadogSite}`,
url: datadogRoute('/api/v1/validate'),
dispatcher: getProxyDispatcher(),
})

return response.data.valid
Expand Down
2 changes: 1 addition & 1 deletion packages/base/src/helpers/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const isRequestError = (error: unknown): error is RequestError =>

const dispatcherCache = new Map<string, ProxyAgent>()

export const getProxyDispatcher = (proxyUrl: string): EnvHttpProxyAgent | ProxyAgent => {
export const getProxyDispatcher = (proxyUrl = ''): EnvHttpProxyAgent | ProxyAgent => {
// EnvHttpProxyAgent reads env vars at construction, so never cache it
if (!proxyUrl) {
return new EnvHttpProxyAgent()
Expand Down
14 changes: 14 additions & 0 deletions packages/plugin-aas/src/__tests__/instrument.test.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),
       })
     )
   })

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,20 @@ describe('aas instrument', () => {
expect(webAppsOperations.restart).not.toHaveBeenCalled()
})

test('Fails with network error message when API key validation throws (e.g. proxy/ECONNRESET)', async () => {
const networkError = Object.assign(new Error('read ECONNRESET'), {code: 'ECONNRESET'})
validateApiKey.mockClear().mockRejectedValue(networkError)

const {code, context} = await runCLI(DEFAULT_INSTRUMENT_ARGS)
expect(code).toEqual(1)
const output = context.stdout.toString()
expect(output).toContain('network error')
expect(output).toContain('HTTP_PROXY')
expect(output).not.toContain('Ensure you copied the value and not the Key ID')
expect(getToken).not.toHaveBeenCalled()
expect(webAppsOperations.get).not.toHaveBeenCalled()
})

test('Warns and exits if Web App is Windows but runtime cannot be detected', async () => {
webAppsOperations.get.mockClear().mockResolvedValue({...CONTAINER_WEB_APP, kind: 'app,windows'})
const {code, context} = await runCLI(DEFAULT_INSTRUMENT_ARGS)
Expand Down
17 changes: 13 additions & 4 deletions packages/plugin-aas/src/commands/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,24 @@ export class PluginCommand extends AasInstrumentCommand {
return 1
}

let isApiKeyValid: boolean
try {
const isApiKeyValid = await newApiKeyValidator({
isApiKeyValid = await newApiKeyValidator({
apiKey: process.env.DD_API_KEY,
datadogSite: getDatadogSite(),
}).validateApiKey()
if (!isApiKeyValid) {
throw Error()
}
} catch (e) {
this.context.stdout.write(
renderSoftWarning(
`Could not validate the API Key stored in the environment variable ${chalk.bold('DD_API_KEY')}: ${maskString(
process.env.DD_API_KEY ?? ''
)}\nA network error occurred while contacting the Datadog API. If you are behind a corporate proxy, ensure ${chalk.bold('HTTP_PROXY')} and ${chalk.bold('HTTPS_PROXY')} are set correctly.\nError: ${e}`
Comment thread
nina9753 marked this conversation as resolved.
Outdated
)
)

return 1
}
if (!isApiKeyValid) {
this.context.stdout.write(
renderSoftWarning(
`Invalid API Key stored in the environment variable ${chalk.bold('DD_API_KEY')}: ${maskString(
Expand Down
Loading