diff --git a/integration-tests/openfeature/openfeature-configuration-sources.spec.js b/integration-tests/openfeature/openfeature-configuration-sources.spec.js index 566c36639b..084ea88561 100644 --- a/integration-tests/openfeature/openfeature-configuration-sources.spec.js +++ b/integration-tests/openfeature/openfeature-configuration-sources.spec.js @@ -135,9 +135,9 @@ describe('OpenFeature configuration source contract', () => { assert.strictEqual(configurationCases.length, 63) assert.deepStrictEqual(countDeliveries(configurationCases), { - agentless: 16, - remote_config: 16, - disabled: 31, + agentless: 12, + remote_config: 12, + disabled: 39, }) await runCases(configurationCases) @@ -341,7 +341,7 @@ function assertDeliveryTraffic (testCase) { for (const request of cdnRequests) { assert.strictEqual(request.url, `${AGENTLESS_PATH}?case=${testCase.identifier}`, testCase.label) assert.strictEqual(request.headers['accept-encoding'], 'gzip', testCase.label) - assert.strictEqual(request.headers['dd-api-key'], 'integration-api-key', testCase.label) + assert.strictEqual(request.headers['dd-api-key'], undefined, testCase.label) assert.strictEqual(request.headers['dd-client-library-language'], 'nodejs', testCase.label) assert.strictEqual(request.headers['dd-client-library-version'], VERSION, testCase.label) } @@ -677,6 +677,7 @@ function expectedDelivery (stable, source, legacy) { if (stable === 'false') return 'disabled' if (source.name === 'agentless') return 'agentless' if (source.name === 'remote_config') return 'remote_config' + if (source.name === 'offline' || source.name === 'invalid') return 'disabled' if (legacy === 'true') return 'remote_config' if (legacy === 'false') return 'disabled' return 'agentless' diff --git a/packages/dd-trace/src/config/defaults.js b/packages/dd-trace/src/config/defaults.js index d5b1f2e882..1a2d44f852 100644 --- a/packages/dd-trace/src/config/defaults.js +++ b/packages/dd-trace/src/config/defaults.js @@ -32,13 +32,14 @@ const parseErrors = new Map() * @param {string} source - The source of the value. * @param {string} baseMessage - The base message to use for the warning. * @param {Error} [error] - An error that was thrown while parsing the value. + * @param {boolean} [pickedDefault] - Whether the invalid value was discarded in favor of a fallback. */ -function warnInvalidValue (value, optionName, source, baseMessage, error) { +function warnInvalidValue (value, optionName, source, baseMessage, error, pickedDefault = true) { const canonicalName = (optionsTable[optionName]?.canonicalName ?? optionName) + source // Lazy load log module to avoid circular dependency if (!parseErrors.has(canonicalName)) { - // TODO: Rephrase: It will fallback to former source (or default if not set) - let message = `${baseMessage}: ${util.inspect(value)} for ${optionName} (source: ${source}), picked default` + let message = `${baseMessage}: ${util.inspect(value)} for ${optionName} (source: ${source})` + if (pickedDefault) message += ', picked default' if (error) { error.stack = error.toString() message += `\n\n${util.inspect(error)}` diff --git a/packages/dd-trace/src/config/generated-config-types.d.ts b/packages/dd-trace/src/config/generated-config-types.d.ts index 9dec6c4d28..70b25c2bf9 100644 --- a/packages/dd-trace/src/config/generated-config-types.d.ts +++ b/packages/dd-trace/src/config/generated-config-types.d.ts @@ -432,7 +432,7 @@ export interface GeneratedConfig { }; }; featureFlags: { - DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config"; + DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: string; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL: string | undefined; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS: number; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS: number; @@ -697,7 +697,7 @@ export interface GeneratedEnvVarConfig { DD_EXPERIMENTAL_TEST_OPT_VITEST_NO_WORKER_INIT: boolean | undefined; DD_EXPERIMENTAL_TEST_REQUESTS_FS_CACHE: boolean; DD_EXTERNAL_ENV: string | undefined; - DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: "agentless" | "remote_config"; + DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: string; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL: string | undefined; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS: number; DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS: number; diff --git a/packages/dd-trace/src/config/index.js b/packages/dd-trace/src/config/index.js index 5e952a6653..d9518503c7 100644 --- a/packages/dd-trace/src/config/index.js +++ b/packages/dd-trace/src/config/index.js @@ -343,6 +343,13 @@ class Config extends ConfigBase { } } + const configurationSource = this.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE + if (this.featureFlags.DD_FEATURE_FLAGS_ENABLED && + configurationSource !== 'agentless' && + configurationSource !== 'remote_config') { + setAndTrack(this, 'featureFlags.DD_FEATURE_FLAGS_ENABLED', false) + } + if (this.url || os.type() !== 'Windows_NT' && !trackedConfigOrigins.has('hostname') && diff --git a/packages/dd-trace/src/config/parsers.js b/packages/dd-trace/src/config/parsers.js index beb5a59a17..8caff79b66 100644 --- a/packages/dd-trace/src/config/parsers.js +++ b/packages/dd-trace/src/config/parsers.js @@ -58,6 +58,32 @@ const transformers = { toLowerCase (value) { return toCase(value, 'toLowerCase') }, + /** + * Normalizes a Feature Flagging configuration source while preserving unsupported values for + * diagnostics. A blank value is treated as unset so legacy configuration can still apply. + * + * @param {string} value + * @param {string} optionName + * @param {string} source + * @returns {string | undefined} + */ + configurationSource (value, optionName, source) { + const normalized = value.trim().toLowerCase() + if (normalized === '') return + + if (normalized !== 'agentless' && normalized !== 'remote_config') { + warnInvalidValue( + value, + optionName, + source, + 'Unsupported Feature Flagging configuration source; provider disabled', + undefined, + false + ) + } + + return normalized + }, toUpperCase (value) { return toCase(value, 'toUpperCase') }, diff --git a/packages/dd-trace/src/config/supported-configurations.json b/packages/dd-trace/src/config/supported-configurations.json index bb8fbb2d4c..747dd64a5e 100644 --- a/packages/dd-trace/src/config/supported-configurations.json +++ b/packages/dd-trace/src/config/supported-configurations.json @@ -840,10 +840,9 @@ "implementation": "A", "type": "string", "default": "agentless", - "allowed": "agentless|remote_config", - "description": "Experimental: Select where Feature Flagging loads Universal Flag Configuration. Supported values are agentless and remote_config.", + "description": "Select where Feature Flagging loads Universal Flag Configuration. Supported values are agentless and remote_config; offline is reserved and currently unsupported.", "namespace": "featureFlags", - "transform": "toLowerCase" + "transform": "configurationSource" } ], "DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL": [ @@ -851,7 +850,7 @@ "implementation": "A", "type": "string", "default": null, - "description": "Experimental: Override the agentless Feature Flagging UFC endpoint or base URL.", + "description": "Override the agentless Feature Flagging UFC endpoint or base URL.", "namespace": "featureFlags", "sensitive": true } @@ -861,7 +860,7 @@ "implementation": "A", "type": "int", "default": "30", - "description": "Experimental: Set the agentless Feature Flagging UFC polling interval in seconds, capped at one hour.", + "description": "Set the agentless Feature Flagging UFC polling interval in seconds, capped at one hour.", "allowed": "[1-9]\\d*", "namespace": "featureFlags" } @@ -871,7 +870,7 @@ "implementation": "A", "type": "int", "default": "5", - "description": "Experimental: Set the agentless Feature Flagging UFC request timeout in seconds.", + "description": "Set the agentless Feature Flagging UFC request timeout in seconds.", "allowed": "[1-9]\\d*", "namespace": "featureFlags" } diff --git a/packages/dd-trace/src/openfeature/agentless_configuration_source.js b/packages/dd-trace/src/openfeature/agentless_configuration_source.js index fc50bf1711..fd86a16acc 100644 --- a/packages/dd-trace/src/openfeature/agentless_configuration_source.js +++ b/packages/dd-trace/src/openfeature/agentless_configuration_source.js @@ -20,7 +20,7 @@ const RETRY_JITTER = 0.2 * @property {URL} endpoint * @property {number} pollIntervalMs * @property {number} requestTimeoutMs - * @property {string} apiKey + * @property {string | undefined} apiKey */ /** @@ -138,7 +138,7 @@ class AgentlessConfigurationSource { #request (signal) { const headers = getClientLibraryHeaders() headers['Accept-Encoding'] = 'gzip' - headers['DD-API-KEY'] = this.#config.apiKey + if (this.#config.apiKey) headers['DD-API-KEY'] = this.#config.apiKey if (this.#etag) headers['If-None-Match'] = this.#etag /** @@ -228,10 +228,7 @@ class AgentlessConfigurationSource { this.#failureWarnings.add(category) if (statusCode === 401 || statusCode === 403) { - log.warn( - 'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid', - statusCode - ) + log.warn('Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication', statusCode) } else if (statusCode) { log.warn('Feature Flagging agentless endpoint returned HTTP %d after %d attempts', statusCode, attempts) } else if (attempts > 1) { diff --git a/packages/dd-trace/src/openfeature/configuration_source.js b/packages/dd-trace/src/openfeature/configuration_source.js index 0bd7327953..ea59d14be8 100644 --- a/packages/dd-trace/src/openfeature/configuration_source.js +++ b/packages/dd-trace/src/openfeature/configuration_source.js @@ -1,6 +1,5 @@ 'use strict' -const { isLoopbackHost } = require('../exporters/common/url') const log = require('../log') const DEFAULT_AGENTLESS_PATH = '/api/v2/feature-flagging/config/rules-based/server' @@ -23,13 +22,13 @@ function create (config, applyConfiguration) { DD_FEATURE_FLAGS_ENABLED: enabled, } = config.featureFlags - if (!enabled || source !== 'agentless') { - return - } + if (!enabled || source !== 'agentless') return + + const hasCustomEndpoint = Boolean(baseUrl?.trim()) try { - if (!config.DD_API_KEY) { - throw new Error('DD_API_KEY is required for Feature Flagging agentless delivery') + if (!hasCustomEndpoint && !config.DD_API_KEY) { + throw new Error('DD_API_KEY is required for the default Datadog Feature Flagging endpoint') } const AgentlessConfigurationSource = require('./agentless_configuration_source') @@ -37,7 +36,7 @@ function create (config, applyConfiguration) { endpoint: endpoint(config, baseUrl), pollIntervalMs: Math.min(pollIntervalSeconds, MAX_POLL_INTERVAL_SECONDS) * 1000, requestTimeoutMs: requestTimeoutSeconds * 1000, - apiKey: config.DD_API_KEY, + apiKey: hasCustomEndpoint ? undefined : config.DD_API_KEY, }, applyConfiguration) } catch (error) { log.error('Unable to configure Feature Flagging configuration source', error) @@ -74,10 +73,6 @@ function endpoint (config, configuredBaseUrl) { if (url.protocol !== 'https:' && url.protocol !== 'http:') { throw new Error('Feature Flagging agentless URL must use HTTP or HTTPS') } - if (url.protocol === 'http:' && !isLoopbackHost(url.hostname)) { - throw new Error('Feature Flagging agentless URL must use HTTPS unless it targets loopback') - } - if (url.pathname === '' || url.pathname === '/') { url.pathname = DEFAULT_AGENTLESS_PATH } diff --git a/packages/dd-trace/test/config/index.spec.js b/packages/dd-trace/test/config/index.spec.js index 1c3bdcb307..e4383ba053 100644 --- a/packages/dd-trace/test/config/index.spec.js +++ b/packages/dd-trace/test/config/index.spec.js @@ -5034,16 +5034,26 @@ rules: expected: { enabled: true, source: 'remote_config' }, }, { - name: 'falls back from an invalid source to legacy enablement', + name: 'disables the provider for an invalid source', + source: 'other', + expected: { enabled: false }, + }, + { + name: 'disables the provider for the reserved offline source', + source: 'offline', + expected: { enabled: false }, + }, + { + name: 'disables the provider for an invalid source despite legacy enablement', source: 'other', legacyEnabled: 'true', - expected: { enabled: true, source: 'remote_config' }, + expected: { enabled: false }, }, { - name: 'falls back from the reserved offline source to legacy enablement', + name: 'disables the provider for the reserved offline source despite legacy enablement', source: 'offline', legacyEnabled: 'true', - expected: { enabled: true, source: 'remote_config' }, + expected: { enabled: false }, }, ]) { it(name, () => { @@ -5069,27 +5079,36 @@ rules: }) } - it('falls back from an invalid source through calculated legacy precedence', () => { + it('disables an explicit unsupported source while preserving diagnostic context', () => { process.env.DD_FEATURE_FLAGS_ENABLED = 'true' process.env.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = 'offline' process.env.DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED = 'true' const config = getConfig() + const warning = 'Unsupported Feature Flagging configuration source; provider disabled: ' + + "'offline' for DD_FEATURE_FLAGS_CONFIGURATION_SOURCE (source: env_var)" - assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_ENABLED, true) - assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE, 'remote_config') + assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_ENABLED, false) + assert.strictEqual(config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE, 'offline') assert.strictEqual(config.experimental.flaggingProvider.enabled, true) - assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_ENABLED'), 'env_var') - assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'calculated') + assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_ENABLED'), 'calculated') + assert.strictEqual(config.getOrigin('featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE'), 'env_var') assert.strictEqual(config.getOrigin('experimental.flaggingProvider.enabled'), 'env_var') assertConfigUpdateContains(updateConfig.getCall(0).args[0], [ - { name: 'DD_FEATURE_FLAGS_ENABLED', value: true, origin: 'env_var' }, - { name: 'DD_FEATURE_FLAGS_CONFIGURATION_SOURCE', value: 'remote_config', origin: 'calculated' }, + { name: 'DD_FEATURE_FLAGS_ENABLED', value: false, origin: 'calculated' }, + { + name: 'DD_FEATURE_FLAGS_CONFIGURATION_SOURCE', + value: 'offline', + origin: 'env_var', + error: { message: warning }, + }, { name: 'DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED', value: true, origin: 'env_var' }, ]) + sinon.assert.calledOnceWithExactly(log.warn, warning) config.setRemoteConfig({}) + sinon.assert.calledOnce(log.warn) sinon.assert.notCalled(log.error) }) diff --git a/packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js b/packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js index d8816abb8f..11d8032d82 100644 --- a/packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js +++ b/packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js @@ -181,13 +181,15 @@ describe('AgentlessConfigurationSource', () => { clock.restore() clock = undefined const body = zlib.gzipSync(responseBody()) - nock('http://127.0.0.1:8080', { + config.endpoint = new URL('http://flags.dev.internal:8080/custom/ufc') + delete config.apiKey + nock('http://flags.dev.internal:8080', { + badheaders: ['dd-api-key'], reqheaders: { 'accept-encoding': 'gzip', - 'dd-api-key': 'test-api-key', }, }) - .get('/api/v2/feature-flagging/config/rules-based/server') + .get('/custom/ufc') .reply(200, body, { 'content-encoding': 'gzip', etag: '"real-path"', @@ -414,7 +416,7 @@ describe('AgentlessConfigurationSource', () => { 'third', ]) assert.deepStrictEqual(log.warn.thirdCall.args, [ - 'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid', + 'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication', 401, ]) }) @@ -480,7 +482,7 @@ describe('AgentlessConfigurationSource', () => { assert.strictEqual(requests.length, 1) sinon.assert.calledOnceWithExactly( log.warn, - 'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid', + 'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication', 401 ) @@ -491,6 +493,22 @@ describe('AgentlessConfigurationSource', () => { sinon.assert.notCalled(applyConfiguration) }) + it('omits a missing API key and reports the endpoint authentication failure', async () => { + delete config.apiKey + responses.push({ statusCode: 401 }) + + source().start() + await flush() + + assert.strictEqual(Object.hasOwn(requests[0].options.headers, 'DD-API-KEY'), false) + sinon.assert.calledOnceWithExactly( + log.warn, + 'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication', + 401 + ) + sinon.assert.notCalled(applyConfiguration) + }) + it('uses fixed-delay polling after a request completes', async () => { responses.push( { pending: true }, diff --git a/packages/dd-trace/test/openfeature/configuration_source.spec.js b/packages/dd-trace/test/openfeature/configuration_source.spec.js index 8ac81dba43..1a78d73687 100644 --- a/packages/dd-trace/test/openfeature/configuration_source.spec.js +++ b/packages/dd-trace/test/openfeature/configuration_source.spec.js @@ -86,21 +86,37 @@ describe('OpenFeature configuration source', () => { it(`allows the loopback endpoint ${baseUrl}`, () => { config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = baseUrl + const resolved = createSourceConfig() assert.strictEqual( - createSourceConfig().endpoint.toString(), + resolved.endpoint.toString(), `${baseUrl}/api/v2/feature-flagging/config/rules-based/server` ) + assert.strictEqual(resolved.apiKey, undefined) }) } + it('allows a cleartext custom endpoint for local development and proxies', () => { + config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = + 'http://flags.dev.internal:8080' + + const resolved = createSourceConfig() + assert.strictEqual( + resolved.endpoint.toString(), + 'http://flags.dev.internal:8080/api/v2/feature-flagging/config/rules-based/server' + ) + assert.strictEqual(resolved.apiKey, undefined) + }) + it('preserves an exact configured path and query', () => { config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = 'https://example.com/custom/ufc?tenant=one' + const resolved = createSourceConfig() assert.strictEqual( - createSourceConfig().endpoint.toString(), + resolved.endpoint.toString(), 'https://example.com/custom/ufc?tenant=one' ) + assert.strictEqual(resolved.apiKey, undefined) }) it('derives and creates the managed GovCloud endpoint without hard-coding availability', () => { @@ -156,20 +172,6 @@ describe('OpenFeature configuration source', () => { sinon.assert.notCalled(AgentlessConfigurationSource) }) - it('rejects cleartext non-loopback endpoints', () => { - config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = 'http://flags.example.test' - - const source = configurationSource.create(config, sinon.spy()) - - assert.strictEqual(source, undefined) - sinon.assert.calledOnceWithMatch( - log.error, - 'Unable to configure Feature Flagging configuration source', - sinon.match.instanceOf(Error) - ) - sinon.assert.notCalled(AgentlessConfigurationSource) - }) - it('rejects malformed endpoints without logging their sensitive value', () => { const sentinel = 'sensitive-value' config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = `https://${sentinel} value` @@ -187,7 +189,19 @@ describe('OpenFeature configuration source', () => { assert.strictEqual(log.error.firstCall.args[1].cause, undefined) }) - it('requires an API key without enabling a source', () => { + it('creates a source for a custom API without a Datadog API key', () => { + delete config.DD_API_KEY + config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = + 'https://flags.example.test/custom/ufc' + + const source = configurationSource.create(config, sinon.spy()) + + assert.ok(source instanceof AgentlessConfigurationSource) + assert.strictEqual(AgentlessConfigurationSource.firstCall.args[0].apiKey, undefined) + sinon.assert.notCalled(log.error) + }) + + it('requires a Datadog API key for the default Datadog Feature Flagging endpoint', () => { delete config.DD_API_KEY const source = configurationSource.create(config, sinon.spy()) @@ -195,7 +209,7 @@ describe('OpenFeature configuration source', () => { sinon.assert.calledOnceWithMatch( log.error, 'Unable to configure Feature Flagging configuration source', - sinon.match.instanceOf(Error) + sinon.match.has('message', 'DD_API_KEY is required for the default Datadog Feature Flagging endpoint') ) assert.strictEqual(source, undefined) sinon.assert.notCalled(AgentlessConfigurationSource) @@ -211,6 +225,18 @@ describe('OpenFeature configuration source', () => { sinon.assert.notCalled(AgentlessConfigurationSource) }) + for (const source of ['offline', 'invalid']) { + it(`does not create a source for the unsupported ${source} source`, () => { + config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = source + + const resolved = configurationSource.create(config, sinon.spy()) + + assert.strictEqual(resolved, undefined) + sinon.assert.notCalled(AgentlessConfigurationSource) + sinon.assert.notCalled(log.error) + }) + } + it('does not create an agentless source when Feature Flags are disabled', () => { config.featureFlags.DD_FEATURE_FLAGS_ENABLED = false delete config.site