[Fixes #14248] Fix proxy registry initialization (#14249)

Author: sijandh35

geonode/proxy/tests.py

@@ -26,6 +26,7 @@
 import json
 import io
 import zipfile
+from threading import Event, Thread
 
 from urllib.parse import urljoin
 
@@ -35,6 +36,7 @@
 from geonode.proxy.templatetags.proxy_lib_tags import original_link_available
 from django.test.client import RequestFactory
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.utils.timezone import now
 from unittest.mock import patch
 
 try:
@@ -70,6 +72,55 @@ def setUp(self):
         super().setUp()
         self.admin = get_user_model().objects.get(username="admin")
 
+    def test_register_host_lazy_initializes_registry(self):
+        registry = ProxyUrlsRegistry()
+
+        def initialize_registry():
+            registry.proxy_allowed_hosts = ["existing.test"]
+            registry._last_registry_load = now()
+
+        registry._initialize = MagicMock(side_effect=initialize_registry)
+
+        registry.register_host("remote.test")
+
+        registry._initialize.assert_called_once()
+        self.assertEqual({"existing.test", "remote.test"}, registry.proxy_allowed_hosts)
+
+    def test_concurrent_lazy_initialization_runs_once(self):
+        registry = ProxyUrlsRegistry()
+        initialization_started = Event()
+        release_initialization = Event()
+        errors = []
+
+        def initialize_registry():
+            initialization_started.set()
+            release_initialization.wait(5)
+            registry.proxy_allowed_hosts = ["existing.test"]
+            registry._last_registry_load = now()
+
+        registry._initialize = MagicMock(side_effect=initialize_registry)
+
+        def read_registry():
+            try:
+                registry.proxy_allowed_hosts
+            except Exception as exc:
+                errors.append(exc)
+
+        first_thread = Thread(target=read_registry)
+        second_thread = Thread(target=read_registry)
+
+        first_thread.start()
+        initialization_started.wait(5)
+        second_thread.start()
+        release_initialization.set()
+        first_thread.join(5)
+        second_thread.join(5)
+
+        self.assertFalse(first_thread.is_alive())
+        self.assertFalse(second_thread.is_alive())
+        self.assertEqual([], errors)
+        registry._initialize.assert_called_once()
+
     @patch("geonode.proxy.views.proxy_urls_registry", ProxyUrlsRegistry().set([TEST_DOMAIN]))
     def test_proxy_allowed_host(self):
         """If PROXY_ALLOWED_HOSTS is not empty and DEBUG is False requests should return no error."""

geonode/proxy/utils.py

@@ -1,3 +1,4 @@
+from threading import RLock
 from urllib.parse import urlsplit
 
 from django.conf import settings
@@ -9,50 +10,77 @@
 
 
 class ProxyUrlsRegistry:
-    _last_registry_load = None
     _registry_reload_threshold = getattr(settings, "PROXY_RELOAD_REGISTRY_THRESHOLD_DAYS", 1)
 
-    def initialize(self):
+    def __init__(self):
+        self._lock = RLock()
+        self._last_registry_load = None
+        self._proxy_allowed_hosts = None
+
+    def _needs_initialization(self):
+        return (
+            self._last_registry_load is None
+            or (now() - self._last_registry_load).days >= self._registry_reload_threshold
+        )
+
+    @property
+    def proxy_allowed_hosts(self):
+        # We register remote hosts from remote URLs at creation time, inside ImporterViewSet.create().
+        # If for some reason the creation fails we end up having stale or wrong URLs inside the registry.
+        # We check the last time the registry was updated, and after a certain delta we reinitialize the registry
+        # which removes any URLs that are not connected to remote datasets.
+        if self._needs_initialization():
+            with self._lock:
+                if self._needs_initialization():
+                    self._initialize()
+        return self._proxy_allowed_hosts
+
+    @proxy_allowed_hosts.setter
+    def proxy_allowed_hosts(self, hosts):
+        self._proxy_allowed_hosts = set(hosts)
+
+    def _initialize(self):
         from geonode.base.models import Link
         from geonode.geoserver.helpers import ogc_server_settings
 
-        self.proxy_allowed_hosts = set([site_url.hostname] + list(getattr(settings, "PROXY_ALLOWED_HOSTS", ())))
+        proxy_allowed_hosts = set([site_url.hostname] + list(getattr(settings, "PROXY_ALLOWED_HOSTS", ())))
 
         if ogc_server_settings:
-            self.register_host(ogc_server_settings.hostname)
+            proxy_allowed_hosts.add(ogc_server_settings.hostname)
 
         for link in Link.objects.filter(resource__sourcetype="REMOTE", link_type__in=PROXIED_LINK_TYPES):
             remote_host = urlsplit(link.url).hostname
-            self.register_host(remote_host)
+            proxy_allowed_hosts.add(remote_host)
 
+        self.proxy_allowed_hosts = proxy_allowed_hosts
         self._last_registry_load = now()
 
+    def initialize(self):
+        """Rebuild the registry safely for direct callers."""
+        with self._lock:
+            self._initialize()
+
     def set(self, hosts):
-        self.proxy_allowed_hosts = set(hosts)
-        self._last_registry_load = now()
+        with self._lock:
+            self.proxy_allowed_hosts = set(hosts)
+            self._last_registry_load = now()
         return self
 
     def clear(self):
-        self.proxy_allowed_hosts = set()
-        self._last_registry_load = now()
+        with self._lock:
+            self.proxy_allowed_hosts = set()
+            self._last_registry_load = now()
         return self
 
     def register_host(self, host):
-        self.proxy_allowed_hosts.add(host)
+        with self._lock:
+            self.proxy_allowed_hosts.add(host)
 
     def unregister_host(self, host):
-        self.proxy_allowed_hosts.remove(host)
+        with self._lock:
+            self.proxy_allowed_hosts.remove(host)
 
     def get_proxy_allowed_hosts(self):
-        # We register remote hosts from remote URLs at creation time, inside ImporterViewSet.create().
-        # If for some reason the creation fails we end up having stale or wrong URLs inside the registry.
-        # We check the last time the registry was updated, and after a certain delta we reinitialize the registry
-        # which removes any URLs that are not connected to remote datasets
-        if (
-            self._last_registry_load is None
-            or (now() - self._last_registry_load).days >= self._registry_reload_threshold
-        ):
-            self.initialize()
         return self.proxy_allowed_hosts