Skip to content

Commit e62b427

Browse files
authored
fix(rcm): check if the product callback exists [backport to 1.9] (#5259)
Backport to 1.9 of: #5249 ## Checklist - [X] Change(s) are motivated and described in the PR description. - [X] Testing strategy is described if automated tests are not included in the PR. - [X] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [X] Change is maintainable (easy to change, telemetry, documentation). - [X] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [X] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [X] Author is aware of the performance implications of this PR as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment. --------- Signed-off-by: Juanjo Alvarez <[email protected]>
1 parent 762a5f7 commit e62b427

File tree

3 files changed

+41
-30
lines changed

3 files changed

+41
-30
lines changed

ddtrace/appsec/_remoteconfiguration.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,14 @@
3535
log = get_logger(__name__)
3636

3737

38-
def enable_appsec_rc():
39-
# type: () -> None
38+
def enable_appsec_rc(test_tracer=None):
39+
# type: (Optional[Tracer]) -> None
40+
# Tracer is a parameter for testing propose
4041
# Import tracer here to avoid a circular import
41-
from ddtrace import tracer
42+
if test_tracer is None:
43+
from ddtrace import tracer
44+
else:
45+
tracer = test_tracer
4246

4347
if _appsec_rc_features_is_enabled():
4448
from ddtrace.internal.remoteconfig import RemoteConfig

ddtrace/internal/remoteconfig/client.py

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ def base64_to_struct(val, cls):
239239
self.converter.register_structure_hook(SignedRoot, base64_to_struct)
240240
self.converter.register_structure_hook(SignedTargets, base64_to_struct)
241241

242-
self._products = dict() # type: MutableMapping[str, ProductCallback]
242+
self._products = dict() # type: MutableMapping[str, Optional[ProductCallback]]
243243
self._applied_configs = dict() # type: Mapping[str, ConfigMetadata]
244244
self._last_targets_version = 0
245245
self._last_error = None # type: Optional[str]
@@ -348,7 +348,8 @@ def _process_response(self, data):
348348
if last_targets_version is None or targets is None:
349349
log.debug("No targets in configuration payload")
350350
for callback in self._products.values():
351-
callback(None, None)
351+
if callback:
352+
callback(None, None)
352353
return
353354

354355
client_configs = {k: v for k, v in targets.items() if k in payload.client_configs}
@@ -374,34 +375,35 @@ def _process_response(self, data):
374375
log.debug("Disable configuration: %s", target)
375376
callback_action = False
376377

377-
callback = self._products[config.product_name]
378-
379-
try:
380-
callback(config, callback_action)
381-
except Exception:
382-
log.debug("error while removing product %s config %r", config.product_name, config)
383-
continue
378+
callback = self._products.get(config.product_name)
379+
if callback:
380+
try:
381+
callback(config, callback_action)
382+
except Exception:
383+
log.debug("error while removing product %s config %r", config.product_name, config)
384+
continue
384385

385386
# 3. Load new configurations
386387
for target, config in client_configs.items():
387-
callback = self._products[config.product_name]
388-
389-
applied_config = self._applied_configs.get(target)
390-
if applied_config == config:
391-
continue
392-
393-
config_content = _extract_target_file(payload, target, config)
394-
if config_content is None:
395-
continue
396-
397-
try:
398-
log.debug("Load new configuration: %s. content %s", target, config_content)
399-
callback(config, config_content)
400-
except Exception:
401-
log.debug("error while loading product %s config %r", config.product_name, config)
402-
continue
403-
else:
404-
applied_configs[target] = config
388+
callback = self._products.get(config.product_name)
389+
390+
if callback:
391+
applied_config = self._applied_configs.get(target)
392+
if applied_config == config:
393+
continue
394+
395+
config_content = _extract_target_file(payload, target, config)
396+
if config_content is None:
397+
continue
398+
399+
try:
400+
log.debug("Load new configuration: %s. content %s", target, config_content)
401+
callback(config, config_content)
402+
except Exception:
403+
log.debug("error while loading product %s config %r", config.product_name, config)
404+
continue
405+
else:
406+
applied_configs[target] = config
405407

406408
self._last_targets_version = last_targets_version
407409
self._applied_configs = applied_configs
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
Fix for KeyError exceptions when when `ASM_FEATURES` (1-click activation) disabled all ASM products. This could
5+
cause 1-click activation to work incorrectly in some cases.

0 commit comments

Comments
 (0)