Add defensive behavior for strict environments like sandboxes

morango/sync/syncsession.py

@@ -69,15 +69,25 @@ def _get_server_ip(hostname):
 
 
 def _get_client_ip_for_server(server_host, server_port):
-    s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    default_ip = "127.0.0.1"
+
+    # strict apparmor rules or sandboxing could prevent this
+    try:
+        s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    except OSError:
+        logger.warning(
+            f"Could not create socket to determine external IP address to reach {server_host}"
+        )
+        return default_ip
+
     try:
         s.connect((server_host, server_port))
-        IP = s.getsockname()[0]
-    except:  # noqa: E722
-        IP = "127.0.0.1"
+        return s.getsockname()[0]
+    except (OSError, ValueError, OverflowError):  # ValueError covers UnicodeError
+        logger.warning(f"Could not determine external IP address to reach {server_host}")
+        return default_ip
     finally:
         s.close()
-    return IP
 
 
 def ignore_404(target):

tests/testapp/tests/helpers.py

@@ -3,6 +3,7 @@
 """
 
 import json
+import socket
 import uuid
 
 import factory
@@ -35,6 +36,34 @@
 from .compat import EnvironmentVarGuard
 
 
+def get_free_tcp_port():
+    # sandboxing can cause 0 to fail (which is to try for any open port), so fallback to a hardcoded
+    # list afterward
+    ports = [0, 3000, 8000, 8001, 8080, 8081, 8888, 10001, 10002, 10003]
+    for p in ports:
+        try:
+            with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as tcp:
+                tcp.bind(("127.0.0.1", p))
+                _, port = tcp.getsockname()
+            return port
+        except OSError:
+            pass
+
+    raise RuntimeError("Could not find open port")
+
+
+class FreePortLiveServerTestCase(LiveServerTestCase):
+    """
+    LiveServerTestCase defaults to port 0, which sandboxing can deny, so pick an explicit port
+    for each test class instead
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        cls.port = get_free_tcp_port()
+        super(FreePortLiveServerTestCase, cls).setUpClass()
+
+
 class FacilityModelFactory(factory.django.DjangoModelFactory):
     class Meta:
         model = Facility
@@ -325,7 +354,7 @@ def create_buffer_and_store_dummy_data(transfer_session_id):
     return data
 
 
-class BaseClientTestCase(LiveServerTestCase):
+class BaseClientTestCase(FreePortLiveServerTestCase):
     profile = "facilitydata"
 
     def setUp(self):

tests/testapp/tests/integration/test_syncsession.py

@@ -1,7 +1,6 @@
 import contextlib
 import json
 import os
-import socket
 import subprocess
 import sys
 import time
@@ -29,6 +28,7 @@
 from morango.sync.controller import MorangoProfileController
 
 from ..compat import EnvironmentVarGuard
+from ..helpers import get_free_tcp_port
 
 SECOND_TEST_DATABASE = "default2"
 SECOND_SYSTEM_ID = "default2"
@@ -45,14 +45,6 @@ def second_environment():
     assert instance1.id != instance2.id
 
 
-def get_free_tcp_port():
-    tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-    tcp.bind(("", 0))
-    addr, port = tcp.getsockname()
-    tcp.close()
-    return port
-
-
 class LiveServer:
     def __init__(self):
         self.env = os.environ.copy()

tests/testapp/tests/sync/test_syncsession.py

@@ -2,7 +2,7 @@
 import uuid
 
 import mock
-from django.test.testcases import LiveServerTestCase
+from django.test import SimpleTestCase
 from django.test.utils import override_settings
 from requests.exceptions import ChunkedEncodingError
 from requests.exceptions import HTTPError
@@ -27,6 +27,7 @@
 from morango.sync.context import NetworkSessionContext
 from morango.sync.controller import MorangoProfileController
 from morango.sync.session import SessionWrapper
+from morango.sync.syncsession import _get_client_ip_for_server
 from morango.sync.syncsession import NetworkSyncConnection
 from morango.sync.syncsession import PullClient
 from morango.sync.syncsession import PushClient
@@ -35,6 +36,7 @@
 
 from ..helpers import BaseClientTestCase
 from ..helpers import BaseTransferClientTestCase
+from ..helpers import FreePortLiveServerTestCase
 
 
 def mock_patch_decorator(func):
@@ -52,7 +54,31 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
-class NetworkSyncConnectionTestCase(LiveServerTestCase):
+class GetClientIPForServerTestCase(SimpleTestCase):
+    logger_name = "morango.sync.syncsession"
+
+    @mock.patch("morango.sync.syncsession.socket.socket")
+    def test_socket_creation_fails(self, mock_socket):
+        mock_socket.side_effect = OSError("Too many open files")
+        with self.assertLogs(self.logger_name, "WARNING") as logs:
+            ip = _get_client_ip_for_server("example.com", 80)
+        self.assertEqual(ip, "127.0.0.1")
+        self.assertIn("Could not create socket", logs.output[0])
+
+    @mock.patch("morango.sync.syncsession.socket.socket")
+    def test_connect_fails(self, mock_socket):
+        for error in (OSError("unreachable"), UnicodeError("label too long"), OverflowError()):
+            with self.subTest(error=type(error).__name__):
+                mock_socket.reset_mock()
+                mock_socket.return_value.connect.side_effect = error
+                with self.assertLogs(self.logger_name, "WARNING") as logs:
+                    ip = _get_client_ip_for_server("example.com", 80)
+                self.assertEqual(ip, "127.0.0.1")
+                self.assertIn("Could not determine external IP address", logs.output[0])
+                mock_socket.return_value.close.assert_called_once_with()
+
+
+class NetworkSyncConnectionTestCase(FreePortLiveServerTestCase):
     def setUp(self):
         super(NetworkSyncConnectionTestCase, self).setUp()
         self.profile = "facilitydata"

tests/testapp/tests/test_uuid_utilities.py

@@ -144,6 +144,7 @@ def test_consistent_0_5_instance_id(self, *args):
 
         with EnvironmentVarGuard() as env:
             env["MORANGO_SYSTEM_ID"] = "magicsysid"
+            env["MORANGO_NODE_ID"] = ""  # ensure envvar override does not kick in
 
             DatabaseIDModel.objects.all().update(current=False)
             DatabaseIDModel.objects.create(id="7fe445b75cea11858c00fb97bdee8878", current=True)