escalate.so: --escalate-non-get-methods

doc/admin-guide/plugins/escalate.en.rst

@@ -43,6 +43,14 @@ when the origin server in the remap rule returns a 401,
   This option sends the "pristine" Host: header (eg, the Host: header
   that the client sent) to the escalated request.
 
+@pparam=--escalate-non-get-methods
+  In general, the escalate plugin is used with a failover origin that serves a
+  cached backup of the original content.  As a result, the default behavior is
+  to only escalate GET requests since POST, PUT, etc., are not idempotent and
+  may require side effects that are not supported by a failover origin. This
+  option overrides the default behavior and enables escalation for non-GET
+  requests in addition to GET.
+
 Installation
 ------------
 
@@ -61,3 +69,9 @@ Traffic Server would accept a request for ``cdn.example.com`` and, on a cache mi
 request to ``origin.example.com``. If the response code from that server is a 401, 404, 410,
 or 502, then Traffic Server would proxy the request to ``second-origin.example.com``, using a
 Host: header of ``cdn.example.com``.
+
+By default, only GET requests are escalated. To escalate non-GET requests as
+well, you can use::
+
+    map cdn.example.com origin.example.com \
+      @plugin=escalate.so @pparam=401,404,410,502:second-origin.example.com @pparam=--escalate-non-get-methods

plugins/escalate/escalate.cc

@@ -20,13 +20,16 @@
   See the License for the specific language governing permissions and
   limitations under the License.
  */
+#include "ts/apidefs.h"
 #include <ts/ts.h>
 #include <ts/remap.h>
+
 #include <cstdlib>
 #include <cstdio>
 #include <getopt.h>
 #include <cstring>
 #include <string>
+#include <string_view>
 #include <iterator>
 #include <map>
 
@@ -67,7 +70,8 @@ struct EscalationState {
   ~EscalationState() { TSContDestroy(cont); }
   TSCont        cont;
   StatusMapType status_map;
-  bool          use_pristine = false;
+  bool          use_pristine             = false;
+  bool          escalate_non_get_methods = false;
 };
 
 // Little helper function, to update the Host portion of a URL, and stringify the result.
@@ -92,6 +96,31 @@ MakeEscalateUrl(TSMBuffer mbuf, TSMLoc url, const char *host, size_t host_len, i
   return url_str;
 }
 
+/**
+ * Check if the method is GET.
+ *
+ * @param txn The transaction whose method is to be checked.
+ * @return True if @a txn's method is GET, false otherwise.
+ */
+static bool
+MethodIsGet(TSHttpTxn txn)
+{
+  TSMBuffer req_mbuf;
+  TSMLoc    req_hdrp;
+  if (TSHttpTxnClientReqGet(txn, &req_mbuf, &req_hdrp) != TS_SUCCESS) {
+    return false;
+  }
+  int         method_len = 0;
+  char const *method     = TSHttpHdrMethodGet(req_mbuf, req_hdrp, &method_len);
+  if (method == nullptr) {
+    return false;
+  }
+  std::string_view method_view{method, static_cast<size_t>(method_len)};
+  bool const       is_get = (method_view == TS_HTTP_METHOD_GET);
+  TSHandleMLocRelease(req_mbuf, TS_NULL_MLOC, req_hdrp);
+  return is_get;
+}
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // Main continuation for the plugin, examining an origin response for a potential retry.
 //
@@ -106,6 +135,12 @@ EscalateResponse(TSCont cont, TSEvent event, void *edata)
   TSAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR || event == TS_EVENT_HTTP_SEND_RESPONSE_HDR);
   bool const processing_connection_error = (event == TS_EVENT_HTTP_SEND_RESPONSE_HDR);
 
+  if (!es->escalate_non_get_methods && !MethodIsGet(txn)) {
+    Dbg(dbg_ctl, "Skipping escalation for non-GET method");
+    TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
+    return TS_EVENT_NONE;
+  }
+
   if (processing_connection_error) {
     TSServerState const state = TSHttpTxnServerStateGet(txn);
     if (state == TS_SRVSTATE_CONNECTION_ALIVE) {
@@ -199,6 +234,8 @@ TSRemapNewInstance(int argc, char *argv[], void **instance, char *errbuf, int er
     // Ugly, but we set the precedence before with non-command line parsing of args
     if (0 == strncasecmp(argv[i], "--pristine", 10)) {
       es->use_pristine = true;
+    } else if (0 == strncasecmp(argv[i], "--escalate-non-get-methods", 26)) {
+      es->escalate_non_get_methods = true;
     } else {
       // Each token should be a status code then a URL, separated by ':'.
       sep = strchr(argv[i], ':');

tests/gold_tests/pluginTest/escalate/escalate.test.py

@@ -1,4 +1,5 @@
 '''
+Verify escalate plugin behavior.
 '''
 #  Licensed to the Apache Software Foundation (ASF) under one
 #  or more contributor license agreements.  See the NOTICE file
@@ -28,7 +29,7 @@
 
 class EscalateTest:
     """
-    Test the escalate plugin.
+    Test the escalate plugin default behavior (GET requests only).
     """
 
     _replay_original_file: str = 'escalate_original.replay.yaml'
@@ -65,6 +66,10 @@ def _setup_servers(self, tr: 'Process') -> None:
             'uuid: GET_chunked', "Verify the origin server GET request for chunked content.")
         self._server_origin.Streams.All += Testers.ContainsExpression(
             'uuid: GET_failed', "Verify the origin server received the GET request that it returns a 502 with.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: HEAD_fail_not_escalated', "Verify the origin server received the HEAD request that should not be escalated.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: POST_fail_not_escalated', "Verify the origin server received the POST request that should not be escalated.")
         self._server_origin.Streams.All += Testers.ExcludesExpression(
             'uuid: GET_down_origin', "Verify the origin server did not receive the down origin request.")
 
@@ -77,6 +82,13 @@ def _setup_servers(self, tr: 'Process') -> None:
             'x-request: first', "Verify the failover server did not receive the GET request.")
         self._server_failover.Streams.All += Testers.ExcludesExpression(
             'uuid: GET_chunked', "Verify the failover server did not receive the GET request for chunked content.")
+        # By default, non-GET methods should NOT be escalated to failover
+        self._server_failover.Streams.All += Testers.ExcludesExpression(
+            'uuid: HEAD_fail_not_escalated',
+            "Verify the failover server did not receive the HEAD request that should not be escalated.")
+        self._server_failover.Streams.All += Testers.ExcludesExpression(
+            'uuid: POST_fail_not_escalated',
+            "Verify the failover server did not receive the POST request that should not be escalated.")
 
     def _setup_ts(self, tr: 'Process') -> None:
         '''Set up Traffic Server.
@@ -119,12 +131,128 @@ def _setup_client(self, tr: 'Process') -> None:
 
         client.Streams.All += Testers.ExcludesExpression(r'\[ERROR\]', 'Verify there were no errors in the replay.')
         client.Streams.All += Testers.ExcludesExpression('400 Bad', 'Verify none of the 400 responses make it to the client.')
-        client.Streams.All += Testers.ExcludesExpression('502 Bad', 'Verify none of the 502 responses make it to the client.')
         client.Streams.All += Testers.ExcludesExpression('500 Internal', 'Verify none of the 500 responses make it to the client.')
+        # GET requests should be escalated and return 200
         client.Streams.All += Testers.ContainsExpression('x-response: first', 'Verify that the first response was received.')
         client.Streams.All += Testers.ContainsExpression('x-response: second', 'Verify that the second response was received.')
         client.Streams.All += Testers.ContainsExpression('x-response: third', 'Verify that the third response was received.')
         client.Streams.All += Testers.ContainsExpression('x-response: fourth', 'Verify that the fourth response was received.')
+        # Non-GET requests should NOT be escalated and return 502 (default behavior)
+        client.Streams.All += Testers.ContainsExpression(
+            'x-response: head_fail_not_escalated', 'Verify that the HEAD response was received (502).')
+        client.Streams.All += Testers.ContainsExpression(
+            'x-response: post_fail_not_escalated', 'Verify that the POST response was received (502).')
+        client.Streams.All += Testers.ContainsExpression(
+            '502 Bad Gateway', 'Verify that non-GET requests return 502 (not escalated by default).')
+
+
+class EscalateNonGetMethodsTest:
+    """
+    Test the escalate plugin with --escalate-non-get-methods option to verify non-GET requests are also escalated.
+    """
+
+    _replay_get_method_file: str = 'escalate_non_get_methods.replay.yaml'
+    _replay_failover_file: str = 'escalate_failover.replay.yaml'
+
+    def __init__(self):
+        '''Configure the test run for escalating non-GET methods testing.'''
+        tr = Test.AddTestRun('Test escalate plugin with --escalate-non-get-methods option.')
+        self._setup_dns(tr)
+        self._setup_servers(tr)
+        self._setup_ts(tr)
+        self._setup_client(tr)
+
+    def _setup_dns(self, tr: 'TestRun') -> None:
+        '''Set up the DNS server.'''
+        self._dns = tr.MakeDNServer("dns_non_get_methods", default='127.0.0.1')
+
+    def _setup_servers(self, tr: 'TestRun') -> None:
+        '''Set up the origin and failover servers for non-GET methods testing.'''
+        tr.Setup.Copy(self._replay_get_method_file)
+        tr.Setup.Copy(self._replay_failover_file)
+        self._server_origin = tr.AddVerifierServerProcess("server_origin_non_get_methods", self._replay_get_method_file)
+        self._server_failover = tr.AddVerifierServerProcess("server_failover_non_get_methods", self._replay_failover_file)
+
+        # Verify the origin server received all requests
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: GET', "Verify the origin server received the first GET request.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: GET_chunked', "Verify the origin server received the chunked GET request.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: GET_failed', "Verify the origin server received the failed GET request.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: POST_success', "Verify the origin server received the successful POST request.")
+        self._server_origin.Streams.All += Testers.ContainsExpression(
+            'uuid: HEAD_fail_escalated', "Verify the origin server received the HEAD request that will be escalated.")
+
+        # The down origin request should NOT be received by this server
+        self._server_origin.Streams.All += Testers.ExcludesExpression(
+            'uuid: GET_down_origin', "Verify the origin server did not receive the down origin request.")
+
+        # Verify failover server receives escalated requests including non-GET methods
+        self._server_failover.Streams.All += Testers.ContainsExpression(
+            'uuid: GET_failed', "Verify the failover server received the failed GET request.")
+        self._server_failover.Streams.All += Testers.ContainsExpression(
+            'uuid: GET_down_origin', "Verify the failover server received the down origin GET request.")
+        # With --escalate-non-get-methods, the HEAD request should now be escalated
+        self._server_failover.Streams.All += Testers.ContainsExpression(
+            'uuid: HEAD_fail_escalated', "Verify the failover server received the HEAD that is now escalated.")
+        # The successful POST should also not reach failover (since it succeeds on origin)
+        self._server_failover.Streams.All += Testers.ExcludesExpression(
+            'uuid: POST_success', "Verify the failover server did not receive the successful POST request.")
+
+    def _setup_ts(self, tr: 'TestRun') -> None:
+        '''Set up Traffic Server with --escalate-non-get-methods option.'''
+        self._ts = tr.MakeATSProcess("ts_non_get_methods", enable_cache=False)
+
+        self._ts.Disk.records_config.update(
+            {
+                'proxy.config.diags.debug.enabled': 1,
+                'proxy.config.diags.debug.tags': 'http|escalate',
+                'proxy.config.dns.nameservers': f'127.0.0.1:{self._dns.Variables.Port}',
+                'proxy.config.dns.resolv_conf': 'NULL',
+                'proxy.config.http.redirect.actions': 'self:follow',
+                'proxy.config.http.number_of_redirections': 4,
+            })
+
+        # Set up a dead port for the down origin scenario
+        dead_port = get_port(self._ts, "dead_port")
+
+        # Configure escalate plugin with --escalate-non-get-methods option
+        self._ts.Disk.remap_config.AddLines(
+            [
+                f'map http://origin.server.com http://backend.origin.server.com:{self._server_origin.Variables.http_port} '
+                f'@plugin=escalate.so @pparam=500,502:failover.server.com:{self._server_failover.Variables.http_port} @pparam=--escalate-non-get-methods',
+                f'map http://down_origin.server.com http://backend.down_origin.server.com:{dead_port} '
+                f'@plugin=escalate.so @pparam=500,502:failover.server.com:{self._server_failover.Variables.http_port} @pparam=--escalate-non-get-methods',
+            ])
+
+    def _setup_client(self, tr: 'TestRun') -> None:
+        '''Set up the client for non-GET methods testing.'''
+        client = tr.AddVerifierClientProcess(
+            "client_non_get_methods", self._replay_get_method_file, http_ports=[self._ts.Variables.port])
+
+        client.StartBefore(self._dns)
+        client.StartBefore(self._server_origin)
+        client.StartBefore(self._server_failover)
+        client.StartBefore(self._ts)
+
+        # Verify that successful responses are returned for successful requests and escalated failures
+        client.Streams.All += Testers.ContainsExpression('x-response: first', 'Verify first GET response received.')
+        client.Streams.All += Testers.ContainsExpression('x-response: second', 'Verify second GET response received.')
+        client.Streams.All += Testers.ContainsExpression('x-response: third', 'Verify third GET response received (escalated).')
+        client.Streams.All += Testers.ContainsExpression('x-response: fourth', 'Verify fourth GET response received (escalated).')
+        client.Streams.All += Testers.ContainsExpression('x-response: post_success', 'Verify successful POST response received.')
+        client.Streams.All += Testers.ContainsExpression(
+            'x-response: head_fail_escalated', 'Verify escalated HEAD response received.')
+
+        # With --escalate-non-get-methods, POST and HEAD failures should now be escalated and return 200
+        client.Streams.All += Testers.ExcludesExpression(
+            '502 Bad Gateway', 'Verify failed POST and HEAD requests are now escalated')
+
+        # The test should complete without errors
+        client.Streams.All += Testers.ExcludesExpression(r'\[ERROR\]', 'Verify there were no errors in the replay.')
 
 
 EscalateTest()
+EscalateNonGetMethodsTest()

tests/gold_tests/pluginTest/escalate/escalate_failover.replay.yaml

@@ -124,3 +124,28 @@ sessions:
         - [ X-Response, fourth ]
       content:
         size: 320000
+
+  # HEAD request response for escalated requests (with --escalate-non-get-methods)
+  - client-request:
+      method: "HEAD"
+      version: "1.1"
+      url: /api/head/data
+      headers:
+        fields:
+        - [ Host, origin.server.com ]
+        - [ X-Request, head_fail_escalated ]
+        - [ uuid, HEAD_fail_escalated ]
+
+    proxy-request:
+      method: "HEAD"
+      headers:
+        fields:
+        - [ X-Request, { value: head_fail_escalated, as: equal } ]
+
+    server-response:
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Content-Length, 0 ]
+        - [ X-Response, head_fail_escalated ]