fix(test): rework reload_nginx

This change comes after a deep investigation into why certain jobs were
failing when upgrading to the latest dd-trace-cpp version.

The root cause turned out to be increased contention in the default curl HTTP
client due to telemetry refactoring. This caused NGINX to take significantly
longer to shut down, which in turn led to timeouts during reload_nginx.

While investigating, I also realized the logic in `reload_nginx` was flawed.
It was only checking whether a worker process was running, rather than verifying
that the old worker (by PID) had exited. Since NGINX spawns a new worker and
sends a SIGQUIT to the old one during reloads, our previous check was resulting
in false positives. I've updated the logic to explicitly wait for the old worker
PID to terminate and confirm the new worker is up before proceeding.

Additionally, while fixing this, I encountered issues with some AppSec tests.
These tests were intentionally causing the worker to fail during initialization
(e.g. by using invalid config paths), but this left NGINX in a broken state for
subsequent tests. To resolve this, the tests for
`datadog_appsec_http_blocked_template_json`,
`datadog_appsec_http_blocked_template_html`, and
`datadog_appsec_ruleset_file` now validate the existence of their required files
during the configuration process instead of at worker startup.

Author: dmehala

src/security/directives.h

@@ -33,7 +33,7 @@ constexpr directive appsec_directives[] = {
     {
         "datadog_appsec_ruleset_file",
         NGX_HTTP_MAIN_CONF | NGX_CONF_TAKE1,
-        ngx_conf_set_str_slot,
+        ngx_conf_set_path_slot,
         NGX_HTTP_MAIN_CONF_OFFSET,
         offsetof(datadog_main_conf_t, appsec_ruleset_file),
         nullptr,
@@ -42,7 +42,7 @@ constexpr directive appsec_directives[] = {
     {
         "datadog_appsec_http_blocked_template_json",
         NGX_HTTP_MAIN_CONF | NGX_CONF_TAKE1,
-        ngx_conf_set_str_slot,
+        ngx_conf_set_path_slot,
         NGX_HTTP_MAIN_CONF_OFFSET,
         offsetof(datadog_main_conf_t, appsec_http_blocked_template_json),
         nullptr,
@@ -51,7 +51,7 @@ constexpr directive appsec_directives[] = {
     {
         "datadog_appsec_http_blocked_template_html",
         NGX_HTTP_MAIN_CONF | NGX_CONF_TAKE1,
-        ngx_conf_set_str_slot,
+        ngx_conf_set_path_slot,
         NGX_HTTP_MAIN_CONF_OFFSET,
         offsetof(datadog_main_conf_t, appsec_http_blocked_template_html),
         nullptr,

src/security/library.cpp

@@ -491,23 +491,26 @@ FinalizedConfigSettings::FinalizedConfigSettings(
                                                   : enable_status::DISABLED;
   }
 
-  if (ngx_conf.appsec_ruleset_file.len > 0) {
-    ruleset_file_ = to_string_view(ngx_conf.appsec_ruleset_file);
+  if (ngx_conf.appsec_ruleset_file != nullptr &&
+      ngx_conf.appsec_ruleset_file->name.len > 0) {
+    ruleset_file_ = to_string_view(ngx_conf.appsec_ruleset_file->name);
   } else {
     ruleset_file_ = get_env_str(evs, "DD_APPSEC_RULES"sv).value_or("");
   }
 
-  if (ngx_conf.appsec_http_blocked_template_json.len > 0) {
+  if (ngx_conf.appsec_http_blocked_template_json != nullptr &&
+      ngx_conf.appsec_http_blocked_template_json->name.len > 0) {
     blocked_template_json_ =
-        to_string_view(ngx_conf.appsec_http_blocked_template_json);
+        to_string_view(ngx_conf.appsec_http_blocked_template_json->name);
   } else {
     blocked_template_json_ =
         get_env_str(evs, "DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON"sv).value_or("");
   }
 
-  if (ngx_conf.appsec_http_blocked_template_html.len > 0) {
+  if (ngx_conf.appsec_http_blocked_template_html != nullptr &&
+      ngx_conf.appsec_http_blocked_template_html->name.len > 0) {
     blocked_template_html_ =
-        to_string_view(ngx_conf.appsec_http_blocked_template_html);
+        to_string_view(ngx_conf.appsec_http_blocked_template_html->name);
   } else {
     blocked_template_html_ =
         get_env_str(evs, "DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML"sv).value_or("");

src/tracing_library.cpp

@@ -73,7 +73,9 @@ dd::Expected<dd::Tracer> TracingLibrary::make_tracer(
 #ifdef WITH_WAF
   const bool appsec_fully_disabled = (nginx_conf.appsec_enabled == 0);
   if (!appsec_fully_disabled) {
-    const bool has_custom_ruleset = (nginx_conf.appsec_ruleset_file.len > 0);
+    const bool has_custom_ruleset =
+        (nginx_conf.appsec_ruleset_file != nullptr &&
+         nginx_conf.appsec_ruleset_file->len > 0);
     const bool appsec_enabling_explicit =
         (nginx_conf.appsec_enabled != NGX_CONF_UNSET);
     security::register_with_remote_cfg(

test/cases/orchestration.py

@@ -29,6 +29,21 @@ def quit_signal_handler(signum, frame):
     faulthandler.dump_traceback()
 
 
+def wait_until(predicate_func, timeout_seconds):
+    """Wait until `predicate_func` is True."""
+    before = time.monotonic()
+    while True:
+        if predicate_func():
+            break
+
+        now = time.monotonic()
+        if now - before >= timeout_seconds:
+            raise Exception(
+                f"{timeout_seconds} seconds timeout exceeded while waiting for nginx workers to stop.  {now - before} seconds elapsed."
+            )
+        time.sleep(0.5)
+
+
 signal.signal(signal.SIGQUIT, quit_signal_handler)
 
 # Since we override the environment variables of child processes,
@@ -788,17 +803,22 @@ def reload_nginx(self, wait_for_workers_to_terminate=True):
             # The polling interval was chosen based on a system where:
             # - nginx_worker_pids ran in ~0.05 seconds
             # - the workers terminated after ~6 seconds
-            poll_period_seconds = 0.5
-            timeout_seconds = 10
-            before = time.monotonic()
-            while old_worker_pids & nginx_worker_pids(nginx_container,
-                                                      self.verbose):
-                now = time.monotonic()
-                if now - before >= timeout_seconds:
-                    raise Exception(
-                        f"{timeout_seconds} seconds timeout exceeded while waiting for nginx workers to stop.  {now - before} seconds elapsed."
-                    )
-                time.sleep(poll_period_seconds)
+            def old_worker_stops(worker_pid):
+                _worker_pid = worker_pid
+
+                def check():
+                    pids = nginx_worker_pids(nginx_container, self.verbose)
+                    return _worker_pid not in pids
+
+                return check
+
+            wait_until(old_worker_stops(old_worker_pids), timeout_seconds=10)
+
+            def new_worker_starts():
+                pids = nginx_worker_pids(nginx_container, self.verbose)
+                return len(pids) == 1
+
+            wait_until(new_worker_starts, timeout_seconds=10)
 
     def nginx_replace_config(self, nginx_conf_text, file_name):
         """Replace nginx's config and reload nginx.