Fix ip_allow optional methods specification

proxy/IPAllow.cc

@@ -402,8 +402,9 @@ IpAllow::YAMLLoadEntry(const YAML::Node &entry)
     return swoc::Errata(ERRATA_ERROR, "{} {} - item ignored, required '{}' key not found.", this, entry.Mark(), YAML_TAG_IP_ADDRS);
   }
 
-  if (node = entry[YAML_TAG_METHODS]; node) {
-    if (auto errata = this->YAMLLoadMethod(node, *record); !errata.is_ok()) {
+  if (auto methodNode = entry[YAML_TAG_METHODS]) {
+    // methods are specified.
+    if (auto errata = this->YAMLLoadMethod(methodNode, *record); !errata.is_ok()) {
       return errata;
     }
   } else {

tests/gold_tests/ip_allow/ip_allow.test.py

@@ -201,27 +201,37 @@
 '''
 
 
-class Test_ip_allow_h3:
-    """Configure a test to verify ip_allow behavior for h3 connections."""
+class Test_ip_allow:
+    """Configure a test to verify ip_allow behavior."""
 
-    replay_file = "replays/h3.replay.yaml"
     client_counter: int = 0
     ts_counter: int = 0
     server_counter: int = 0
 
-    def __init__(self, name: str, ip_allow_config: str, gold_file="", replay_keys="", expect_request_rejected=False):
+    def __init__(
+            self,
+            name: str,
+            replay_file: str,
+            ip_allow_config: str,
+            gold_file="",
+            replay_keys="",
+            is_h3=False,
+            expect_request_rejected=False):
         """Initialize the test.
 
         :param name: The name of the test.
         :param ip_allow_config: The ip_allow configuration to be used.
+        :param replay_file: The replay file to be used.
         :param gold_file: (Optional) Gold file to be checked.
         :param replay_keys: (Optional) Keys to be used by pv.
         :param expect_request_rejected: (Optional) Whether or not the client request is expected to be rejected.
         """
         self.name = name
+        self.replay_file = replay_file
+        self.ip_allow_config = ip_allow_config
         self.gold_file = gold_file
         self.replay_keys = replay_keys
-        self.ip_allow_config = ip_allow_config
+        self.is_h3 = is_h3
         self.expect_request_rejected = expect_request_rejected
 
     def _configure_server(self, tr: 'TestRun'):
@@ -230,19 +240,19 @@ def _configure_server(self, tr: 'TestRun'):
         :param tr: The TestRun object to associate the server process with.
         """
         server = tr.AddVerifierServerProcess(
-            f"server_{Test_ip_allow_h3.server_counter}",
+            f"server_{Test_ip_allow.server_counter}",
             self.replay_file)
-        Test_ip_allow_h3.server_counter += 1
+        Test_ip_allow.server_counter += 1
         self._server = server
 
     def _configure_traffic_server(self, tr: 'TestRun'):
         """Configure Traffic Server.
 
         :param tr: The TestRun object to associate the ts process with.
         """
-        ts = tr.MakeATSProcess(f"ts-{Test_ip_allow_h3.ts_counter}", enable_quic=True, enable_tls=True)
+        ts = tr.MakeATSProcess(f"ts-{Test_ip_allow.ts_counter}", enable_quic=self.is_h3, enable_tls=True)
 
-        Test_ip_allow_h3.ts_counter += 1
+        Test_ip_allow.ts_counter += 1
         self._ts = ts
         # Configure TLS for Traffic Server.
         self._ts.addDefaultSSLFiles()
@@ -252,10 +262,12 @@ def _configure_traffic_server(self, tr: 'TestRun'):
         self._ts.Disk.records_config.update({
             'proxy.config.diags.debug.enabled': 1,
             'proxy.config.diags.debug.tags': 'v_quic|quic|http|ip_allow',
+            'proxy.config.http.push_method_enabled': 1,
             'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
             'proxy.config.quic.no_activity_timeout_in': 0,
             'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
             'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE',
+            'proxy.config.http.connect_ports': f"{self._server.Variables.http_port}",
         })
 
         self._ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.http_port}')
@@ -275,11 +287,12 @@ def run(self):
         tr.Processes.Default.StartBefore(self._ts)
 
         tr.AddVerifierClientProcess(
-            f'client-{Test_ip_allow_h3.client_counter}',
+            f'client-{Test_ip_allow.client_counter}',
             self.replay_file,
+            https_ports=[self._ts.Variables.ssl_port],
             http3_ports=[self._ts.Variables.ssl_port],
             keys=self.replay_keys)
-        Test_ip_allow_h3.client_counter += 1
+        Test_ip_allow.client_counter += 1
 
         if self.expect_request_rejected:
             # The client request should time out because ATS rejects it and does
@@ -301,11 +314,34 @@ def run(self):
 if Condition.HasATSFeature('TS_USE_QUIC') and Condition.HasCurlFeature('http3'):
 
     # TEST 4: Perform a request in h3 with ip_allow configured to allow all IPs.
-    test0 = Test_ip_allow_h3(
-        "", ip_allow_config=IP_ALLOW_CONFIG_ALLOW_ALL, expect_request_rejected=False)
+    test0 = Test_ip_allow(
+        "h3_allow_all",
+        replay_file='replays/h3.replay.yaml',
+        ip_allow_config=IP_ALLOW_CONFIG_ALLOW_ALL,
+        is_h3=True,
+        expect_request_rejected=False)
     test0.run()
 
     # TEST 5: Perform a request in h3 with ip_allow configured to deny all IPs.
-    test1 = Test_ip_allow_h3(
-        "", ip_allow_config=IP_ALLOW_CONFIG_DENY_ALL, expect_request_rejected=True)
+    test1 = Test_ip_allow(
+        "h3_deny_all",
+        replay_file='replays/h3.replay.yaml',
+        ip_allow_config=IP_ALLOW_CONFIG_DENY_ALL,
+        is_h3=True,
+        expect_request_rejected=True)
     test1.run()
+
+
+# TEST 6: Verify rules are applied to all methods if methods is not specified.
+IP_ALLOW_CONFIG_METHODS_UNSPECIFIED = '''ip_allow:
+  - apply: in
+    ip_addrs: 0/0
+    action: allow
+'''
+test_ip_allow_optional_methods = Test_ip_allow(
+    "ip_allow_optional_methods",
+    replay_file='replays/https_multiple_methods.replay.yaml',
+    ip_allow_config=IP_ALLOW_CONFIG_METHODS_UNSPECIFIED,
+    is_h3=False,
+    expect_request_rejected=False)
+test_ip_allow_optional_methods.run()

tests/gold_tests/ip_allow/replays/https_multiple_methods.replay.yaml

@@ -0,0 +1,109 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+# The replay file executes various HTTP requests to verify the ip_allow policy
+# applies by default to all methods.
+
+meta:
+  version: "1.0"
+
+  blocks:
+  - standard_response: &standard_response
+      server-response:
+        status: 200
+        reason: OK
+        headers:
+          fields:
+          - [ Content-Length, 20 ]
+sessions:
+- protocol:
+  - name: http
+    version: 1
+  - name: tls
+    sni: test_sni
+  transactions:
+
+  # GET
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /test/ip_allow/test_get
+      headers:
+        fields:
+        - [ Content-Length, 0 ]
+        - [ uuid, 1 ]
+
+    <<: *standard_response
+
+    proxy-response:
+      status: 200
+
+  # PUSH
+  - client-request:
+      method: "PUSH"
+      version: "1.1"
+      url: /test/ip_allow/test_put
+      headers:
+        fields:
+        - [ Host, example.com ]
+        - [ uuid, push ]
+        - [ X-Request, push ]
+        - [ Content-Length, 113 ]
+      content:
+        encoding: plain
+        data: "HTTP/1.1 200 OK\nServer: ATS/10.0.0\nAccept-Ranges: bytes\nContent-Length: 6\nCache-Control: public,max-age=2\n\nCACHED"
+
+    # Doesn't matter as the server should not get the request.
+    <<: *standard_response
+
+    # Verify that ATS confirmed that the PUSH was successful, which it does
+    # with a 201 response.
+    proxy-response:
+      status: 201
+
+  # POST
+  - client-request:
+      method: "POST"
+      version: "1.1"
+      url: "/test/ip_allow/test_post"
+      headers:
+        fields:
+          - [Host, www.tls.com]
+          - [Content-Length, 10]
+          - [uuid, post]
+
+    <<: *standard_response
+
+    proxy-response:
+      status: 200
+
+  transactions:
+  # CONNECT
+  - client-request:
+      method: CONNECT
+      version: "1.1"
+      url: www.example.com:80
+      headers:
+        fields:
+          - [uuid, connect]
+          - [Host, www.example.com:80]
+    # This is a CONNECT request so it should not reach the origin server
+    <<: *standard_response
+
+    # ATS returns a 200 responses to client when it establishes a tunnel
+    # between the client and server
+    proxy-response:
+      status: 200