TS-4622 Use port from SRV response when connecting to origin

ci/tsqa/tests/test_hostdb.py

@@ -97,9 +97,13 @@ def handle(self):
                     'Connection: keep-alive\r\n'
                     'X-Server-Ip: {server_ip}\r\n'
                     'X-Server-Port: {server_port}\r\n'
+                    'X-Client-Ip: {client_ip}\r\n'
+                    'X-Client-Port: {client_port}\r\n'
                     '\r\n'.format(
                         server_ip=self.request.getsockname()[0],
-                        server_port=self.request.getsockname()[0],
+                        server_port=self.request.getsockname()[1],
+                        client_ip=self.request.getpeername()[0],
+                        client_port=self.request.getpeername()[1],
                     ))
             self.request.sendall(resp)
 
@@ -399,3 +403,183 @@ def test_serve_stail_for(self):
             # even though the hostdb.lookup_timeout is set to 1 (meaning it should be ~1s)
             #print end - end_working
             #self.assertTrue(end - start >= 2)
+
+
+class TestHostDBSRV(helpers.EnvironmentCase):
+    '''Tests for SRV records within hostdb
+
+        Tests:
+            - SRV record
+                - port overriding
+                - http/https lookups
+            - fallback to non SRV
+    '''
+    @classmethod
+    def setUpEnv(cls, env):
+        cls.dns_sock = socket.socket (socket.AF_INET, socket.SOCK_DGRAM)
+        cls.dns_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        cls.dns_sock.bind(('', 0))  # bind to all interfaces on an ephemeral port
+        dns_port = cls.dns_sock.getsockname()[1]
+
+        # set up dns resolver
+        cls.responses = {
+            'www.foo.com.': dnslib.server.RR.fromZone("foo.com. 1 A 127.0.0.3\nfoo.com. 1 A 127.0.0.2"),
+            'www.stale_for.com.': dnslib.server.RR.fromZone("foo.com. 1 A 127.0.0.1"),
+        }
+
+        cls.dns_server = dnslib.server.DNSServer(
+            StubDNSResolver(cls.responses),
+            port=dns_port,
+            address="localhost",
+        )
+        cls.dns_server.start_thread()
+
+        cls.configs['records.config']['CONFIG'].update({
+            'proxy.config.http.response_server_enabled': 2,  # only add server headers when there weren't any
+            'proxy.config.hostdb.lookup_timeout': 1,
+            'proxy.config.http.connect_attempts_max_retries': 1,
+            'proxy.config.diags.debug.enabled': 1,
+            'proxy.config.diags.debug.tags': 'hostdb',
+            'proxy.config.dns.resolv_conf': os.path.join(env.layout.prefix, 'resolv'),
+            'proxy.config.hostdb.serve_stale_for': 2,
+            'proxy.config.hostdb.ttl_mode': 0,
+            'proxy.config.http_ui_enabled': 3,
+            'proxy.config.dns.nameservers': '127.0.0.1:{0}'.format(dns_port),
+            'proxy.config.srv_enabled': 1,
+        })
+
+        cls.socket_servers = []
+        ss_dns_results = []
+
+        for x in xrange(0, 3):
+            ss = tsqa.endpoint.SocketServerDaemon(EchoServerIpHandler)
+            ss.start()
+            ss.ready.wait()
+            cls.socket_servers.append(ss)
+            ss_dns_results.append(dnslib.server.RR(
+                '_http._tcp.www.foo.com.',
+                dnslib.dns.QTYPE.SRV,
+                rdata = dnslib.dns.SRV(
+                    priority=10,
+                    weight=10,
+                    port=ss.port,
+                    target='127.0.0.{0}.'.format(x + 1),  # note: NUM_REALS must be < 253
+                ),
+                ttl=1,
+            ))
+        cls.responses['_http._tcp.www.foo.com.'] = ss_dns_results
+
+        cls.configs['remap.config'].add_line('map http://www.foo.com/ http://www.foo.com/')
+        cls.configs['remap.config'].add_line('map /_hostdb/ http://{hostdb}')
+
+    def _hostdb_entries(self):
+        # mapping of name -> entries
+        ret = {}
+        showall_ret = requests.get('http://127.0.0.1:{0}/_hostdb/showall?format=json'.format(
+            self.configs['records.config']['CONFIG']['proxy.config.http.server_ports']
+        ), timeout=1)
+        return showall_ret.text
+
+        for item in showall_ret:
+            ret[item['hostname']] = item
+
+        return ret
+
+    def test_ports(self):
+        '''Test port functionality of SRV responses
+
+        SRV responses include ports-- so we want to ensure that we are correctly
+        overriding the port based on the response
+        '''
+        expected_set = set([s.port for s in self.socket_servers])
+
+        actual_set = set()
+        for x in xrange(0, 10):
+            # test one that works
+            ret = requests.get(
+                'http://www.foo.com/',
+                proxies=self.proxies,
+            )
+            self.assertEqual(ret.status_code, 200)
+            actual_set.add(int(ret.headers['X-Server-Port']))
+
+        self.assertEqual(expected_set, actual_set)
+
+    # TODO: fix, seems broken...
+    @helpers.unittest.expectedFailure
+    def test_priority(self):
+        '''Test port functionality of SRV responses
+
+        SRV responses include ports-- so we want to ensure that we are correctly
+        overriding the port based on the response
+        '''
+        time.sleep(3)  # TODO: clear somehow? waiting for expiry is lame
+
+        NUM_REQUESTS = 10
+        orig_responses = self.responses['_http._tcp.www.foo.com.']
+        try:
+            self.responses['_http._tcp.www.foo.com.'][0].rdata.priority=1
+
+            request_distribution = {}
+            for x in xrange(0, NUM_REQUESTS):
+                # test one that works
+                ret = requests.get(
+                    'http://www.foo.com/',
+                    proxies=self.proxies,
+                )
+                self.assertEqual(ret.status_code, 200)
+                port = int(ret.headers['X-Server-Port'])
+                if port not in request_distribution:
+                    request_distribution[port] = 0
+                request_distribution[port] += 1
+
+            # since one has higher priority, we want to ensure that it got all requests
+            self.assertEqual(
+                request_distribution[self.responses['_http._tcp.www.foo.com.'][0].rdata.port],
+                NUM_REQUESTS,
+            )
+
+        finally:
+            self.responses['_http._tcp.www.foo.com.'] = orig_responses
+
+    # TODO: fix, seems broken...
+    @helpers.unittest.expectedFailure
+    def test_weight(self):
+        '''Test port functionality of SRV responses
+
+        SRV responses include ports-- so we want to ensure that we are correctly
+        overriding the port based on the response
+        '''
+        time.sleep(3)  # TODO: clear somehow? waiting for expiry is lame
+
+        NUM_REQUESTS = 100
+        orig_responses = self.responses['_http._tcp.www.foo.com.']
+        try:
+            self.responses['_http._tcp.www.foo.com.'][0].rdata.weight=100
+
+            request_distribution = {}
+            for x in xrange(0, NUM_REQUESTS):
+                # test one that works
+                ret = requests.get(
+                    'http://www.foo.com/',
+                    proxies=self.proxies,
+                )
+                self.assertEqual(ret.status_code, 200)
+                port = int(ret.headers['X-Server-Port'])
+                if port not in request_distribution:
+                    request_distribution[port] = 0
+                request_distribution[port] += 1
+
+            # since the first one has a significantly higher weight, we expect it to
+            # take ~10x the traffic of the other 2
+            self.assertTrue(
+                request_distribution[self.responses['_http._tcp.www.foo.com.'][0].rdata.port] >
+                (NUM_REQUESTS / len(self.responses['_http._tcp.www.foo.com.'])) * 2,
+                'Expected significantly more traffic on {0} than the rest: {1}'.format(
+                    self.responses['_http._tcp.www.foo.com.'][0].rdata.port,
+                    request_distribution,
+                ),
+            )
+
+        finally:
+            self.responses['_http._tcp.www.foo.com.'] = orig_responses

proxy/http/HttpTransact.cc

@@ -1787,7 +1787,12 @@ HttpTransact::OSDNSLookup(State *s)
   // update some state variables with hostdb information that has
   // been provided.
   ats_ip_copy(&s->server_info.dst_addr, s->host_db_info.ip());
-  s->server_info.dst_addr.port() = htons(s->hdr_info.client_request.port_get()); // now we can set the port.
+  // If the SRV response has a port number, we should honor it. Otherwise we do the port defined in remap
+  if (s->dns_info.srv_lookup_success) {
+    s->server_info.dst_addr.port() = htons(s->dns_info.srv_port);
+  } else {
+    s->server_info.dst_addr.port() = htons(s->hdr_info.client_request.port_get()); // now we can set the port.
+  }
   ats_ip_copy(&s->request_data.dest_ip, &s->server_info.dst_addr);
   get_ka_info_from_host_db(s, &s->server_info, &s->client_info, &s->host_db_info);