Merge pull request #588 from gerrod3/proxy-sync-fix

Fix syncing with proxies

Author: gerrod3

CHANGES/581.bugfix

@@ -0,0 +1 @@
+Fixed syncing ignoring remote proxy.

pulp_python/app/tasks/sync.py

@@ -3,7 +3,7 @@
 from aiohttp import ClientResponseError, ClientError
 from lxml.etree import LxmlError
 from gettext import gettext as _
-from os import environ
+from os import environ, path
 
 from rest_framework import serializers
 
@@ -26,7 +26,7 @@
 from bandersnatch.master import Master
 from bandersnatch.configuration import BandersnatchConfig
 from packaging.requirements import Requirement
-from urllib.parse import urljoin
+from urllib.parse import urljoin, urlsplit, urlunsplit
 
 logger = logging.getLogger(__name__)
 
@@ -111,14 +111,22 @@ async def run(self):
         """
         If includes is specified, then only sync those,else try to sync all other packages
         """
+        # Prevent bandersnatch from reading actual .netrc file, set to nonexistent file
+        # See discussion on https://github.com/pulp/pulp_python/issues/581
+        environ["NETRC"] = f"{path.curdir}/.fake-netrc"
         # TODO Change Bandersnatch internal API to take proxy settings in from config parameters
-        if self.remote.proxy_url:
-            environ['http_proxy'] = self.remote.proxy_url
-            environ['https_proxy'] = self.remote.proxy_url
+        if proxy_url := self.remote.proxy_url:
+            if self.remote.proxy_username or self.remote.proxy_password:
+                parsed_proxy = urlsplit(proxy_url)
+                creds = f"{self.remote.proxy_username}:{self.remote.proxy_password}"
+                netloc = f"{creds}@{parsed_proxy.netloc}"
+                proxy_url = urlunsplit((parsed_proxy.scheme, netloc, "", "", ""))
+            environ['http_proxy'] = proxy_url
+            environ['https_proxy'] = proxy_url
         # Bandersnatch includes leading slash when forming API urls
         url = self.remote.url.rstrip("/")
         # local & global timeouts defaults to 10secs and 5 hours
-        async with PulpMaster(url) as master:
+        async with Master(url) as master:
             deferred_download = self.remote.policy != Remote.IMMEDIATE
             workers = self.remote.download_concurrency or self.remote.DEFAULT_DOWNLOAD_CONCURRENCY
             async with ProgressReport(
@@ -140,18 +148,6 @@ async def run(self):
                 await pmirror.synchronize(packages_to_sync)
 
 
-class PulpMaster(Master):
-    """
-    Temporary subclass of bandersnatch.Master until features are in bandersnatch.
-    """
-
-    async def __aenter__(self) -> "Master":
-        """Ensure Pulp does not try to read the .netrc file."""
-        await super().__aenter__()
-        self.session._trust_env = False
-        return self
-
-
 class PulpMirror(Mirror):
     """
     Pulp Mirror Class to perform syncing using Bandersnatch

pulp_python/tests/functional/api/test_sync.py

@@ -1,9 +1,10 @@
 # coding=utf-8
 """Tests that sync python plugin repositories."""
+import pytest
 import unittest
 
 from pulp_smash import config
-from pulp_smash.pulp3.bindings import monitor_task, PulpTaskError, delete_orphans
+from pulp_smash.pulp3.bindings import monitor_task, PulpTaskError
 from pulp_smash.pulp3.utils import (
     gen_repo,
     get_added_content_summary,
@@ -582,9 +583,6 @@ def tearDown(cls):
         """Destroy class-wide variables per test"""
         cls.remote_api.delete(cls.remote.pulp_href)
         cls.repo_api.delete(cls.repo.pulp_href)
-        # This is the last test case to be ran, delete orphans
-        # TODO: Move this to pytest hook when converting to pytest style
-        delete_orphans()
 
     def test_no_windows_sync(self):
         """Tests that no windows packages are synced"""
@@ -643,6 +641,52 @@ def test_no_platform_sync(self):
         )
 
 
+@pytest.mark.parallel
+def test_proxy_sync(
+    python_repo,
+    python_repo_api_client,
+    python_remote_factory,
+    python_content_api_client,
+    http_proxy,
+):
+    """Test syncing with a proxy."""
+    body = gen_python_remote(proxy_url=http_proxy.proxy_url)
+    remote = python_remote_factory(**body)
+    sync_resp = python_repo_api_client.sync(python_repo.pulp_href, {"remote": remote.pulp_href})
+    monitor_task(sync_resp.task)
+
+    repo = python_repo_api_client.read(python_repo.pulp_href)
+    assert repo.latest_version_href[-2] == "1"
+
+    content_resp = python_content_api_client.list(repository_version=repo.latest_version_href)
+    assert content_resp.count == 2
+
+
+@pytest.mark.parallel
+def test_proxy_auth_sync(
+    python_repo,
+    python_repo_api_client,
+    python_remote_factory,
+    python_content_api_client,
+    http_proxy_with_auth,
+):
+    """Test syncing with a proxy with auth."""
+    body = gen_python_remote(
+        proxy_url=http_proxy_with_auth.proxy_url,
+        proxy_username=http_proxy_with_auth.username,
+        proxy_password=http_proxy_with_auth.password,
+    )
+    remote = python_remote_factory(**body)
+    sync_resp = python_repo_api_client.sync(python_repo.pulp_href, {"remote": remote.pulp_href})
+    monitor_task(sync_resp.task)
+
+    repo = python_repo_api_client.read(python_repo.pulp_href)
+    assert repo.latest_version_href[-2] == "1"
+
+    content_resp = python_content_api_client.list(repository_version=repo.latest_version_href)
+    assert content_resp.count == 2
+
+
 def sync_to_remote(self, body, create=False, mirror=False):
     """Takes a body and creates/updates a remote object, then it performs a sync"""
     if create: