Skip to content

fix resource renaming not enabled if appsec enabled via env#401

Merged
cataphract merged 3 commits into
masterfrom
glopes/fix-resource-renaming
Jun 16, 2026
Merged

fix resource renaming not enabled if appsec enabled via env#401
cataphract merged 3 commits into
masterfrom
glopes/fix-resource-renaming

Conversation

@cataphract

Copy link
Copy Markdown
Contributor

When resource renaming is not explicitly enabled or disabled, it should get enabled if appsec is explicitly enabled (not via remote config). However, we were only checking for the enablement via nginx config file, not the env var. Fix that

@cataphract
cataphract requested a review from a team as a code owner June 11, 2026 12:16
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jun 11, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 68.70% (+0.30%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 60b53ae | Docs | Datadog PR Page | Give us feedback!


def test_appsec_env_enables_fallback_mode_by_default(self):
"""Verify that DD_APPSEC_ENABLED=true enables endpoint renaming by default."""
if self.waf_disabled:

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.

opt: Use a decorator to factorize this:

In case.py:

import functools

def skip_if_waf_disabled(func):
    @functools.wraps(func)
    def wrapper(self, *args, **kwargs):
        if self.waf_disabled:
            self.skipTest("WAF is disabled - appsec test requires WAF support")
        return func(self, *args, **kwargs)
    return wrapper

Then:

@case.skip_if_waf_disabled
def test_appsec_enables_fallback_mode_by_default(self):

@case.skip_if_waf_disabled
def test_appsec_env_enables_fallback_mode_by_default(self):


return status, nginx_spans[0]

def send_request_and_get_span_with_env(self, url, config_name, extra_env):

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.

nit: please add type hints for function parameters (and return values)

"log_lines": log_lines,
})

return 200, nginx_spans[0]

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.

nit: rather return status (even if always 200 because of the assert line 54)?


log_lines = self.orch.sync_service('agent')
spans = formats.parse_spans(log_lines)
nginx_spans = [

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.

nit: should we have a similar spans filtering in send_request_and_get_span() above?

@cataphract
cataphract force-pushed the glopes/fix-resource-renaming branch from 90b5d98 to 57feaa3 Compare June 15, 2026 13:42
@cataphract
cataphract merged commit 28b2d1b into master Jun 16, 2026
185 checks passed
@cataphract
cataphract deleted the glopes/fix-resource-renaming branch June 16, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants