Adam Collard has proposed merging ~adam-collard/maas:vault-proxy into maas:master.
Commit message: LP:2002111 explicitly set proxies for hvac.Client Requested reviews: MAAS Maintainers (maas-maintainers) For more details, see: https://code.launchpad.net/~adam-collard/maas/+git/maas/+merge/435584 -- Your team MAAS Committers is subscribed to branch maas:master.
diff --git a/src/maasserver/pytest_tests/test_vault.py b/src/maasserver/pytest_tests/test_vault.py index 7277dbb..d0c28e8 100644 --- a/src/maasserver/pytest_tests/test_vault.py +++ b/src/maasserver/pytest_tests/test_vault.py @@ -1,10 +1,11 @@ +from contextlib import suppress from datetime import datetime, timedelta, timezone -from os import environ from unittest.mock import ANY, MagicMock from django.core.exceptions import ImproperlyConfigured from hvac.exceptions import VaultError as HVACVaultError import pytest +from requests.adapters import HTTPAdapter from maasserver import vault from maasserver.models import Config @@ -144,24 +145,54 @@ class TestVaultClient: class TestNoProxySettingForHVAC: - def test_no_proxy_set(self, monkeypatch): - url = "http://url.to.vault:8200"; - monkeypatch.delenv("no_proxy", raising=False) - _create_hvac_client(url) - assert environ.get("no_proxy") == url - - def test_no_proxy_appended(self, monkeypatch): - url = "http://url.to.vault:8200"; - monkeypatch.setenv("no_proxy", "localhost,127.0.0.1") - _create_hvac_client(url) - assert environ.get("no_proxy") == f"localhost,127.0.0.1,{url}" - - def test_idempotency(self, monkeypatch): - url = "http://url.to.vault:8200"; - monkeypatch.delenv("no_proxy", raising=False) - _create_hvac_client(url) - _create_hvac_client(url) - assert environ.get("no_proxy") == url + def test_proxy_for_vault_scheme_set_to_None(self): + """HVAC client should be configured to not use a proxy.""" + http_client = _create_hvac_client("http://url.to.vault:8200";) + assert http_client.session.proxies == {"http": None} + https_client = _create_hvac_client("https://url.to.vault:8200";) + assert https_client.session.proxies == {"https": None} + + def test_proxy_for_with_env(self, monkeypatch): + """HVAC client should ignore standard proxy environment variables.""" + monkeypatch.setenv("http_proxy", "http://squid.proxy:3128";) + monkeypatch.setenv("https_proxy", "http://squid.proxy:3128";) + monkeypatch.setenv("no_proxy", "127.0.0.1.localhost") + + http_client = _create_hvac_client("http://url.to.vault:8200";) + assert http_client.session.proxies == {"http": None} + https_client = _create_hvac_client("https://url.to.vault:8200";) + assert https_client.session.proxies == {"https": None} + + def test_request_honours_proxies(self, monkeypatch): + """Verify that the request made by requests follows the rules.""" + monkeypatch.setenv("http_proxy", "http://squid.proxy:3128";) + monkeypatch.setenv("https_proxy", "http://squid.proxy:3128";) + monkeypatch.setenv("no_proxy", "127.0.0.1.localhost") + http_client = _create_hvac_client("http://url.to.vault:8200";) + + class ProxyRecordingAdapter(HTTPAdapter): + """ + A basic subclass of the HTTPAdapter that records the arguments used to + ``send``. + """ + + def __init__(self2, *args, **kwargs): + super().__init__(*args, **kwargs) + self2.proxies = [] + + def send(self2, request, **kwargs): + self2.proxies.append(kwargs.get("proxies")) + return + + adapter = ProxyRecordingAdapter() + http_client.session.mount("http://";, adapter) + + # Since we return None from ProxyRecordingAdapter.send, it throws + # AttributeErrors, we just want to see the request that was + # attempted + with suppress(AttributeError): + http_client.seal_status + assert "http" not in adapter.proxies[0] class TestGetRegionVaultClient: diff --git a/src/maasserver/vault.py b/src/maasserver/vault.py index 9ff213f..884bee6 100644 --- a/src/maasserver/vault.py +++ b/src/maasserver/vault.py @@ -1,8 +1,8 @@ from datetime import datetime, timedelta, timezone from functools import cache, wraps import logging -import os from typing import Any, Callable, NamedTuple, Optional +from urllib.parse import urlparse from dateutil.parser import isoparse from django.core.exceptions import ImproperlyConfigured @@ -50,11 +50,7 @@ def _create_hvac_client(url: str, **kwargs) -> hvac.Client: """Create HVAC client for the given URL, with no proxies set.""" # This is gross, and unfortunately necessary due to bootsources.get_simplestreams_env # and https://github.com/psf/requests/issues/2018 - if no_proxy := os.environ.get("no_proxy"): - if url not in no_proxy.split(","): - os.environ["no_proxy"] = f"{no_proxy},{url}" - else: - os.environ["no_proxy"] = url + kwargs["proxies"] = {urlparse(url).scheme: None} return hvac.Client(url=url, **kwargs)
-- Mailing list: https://launchpad.net/~sts-sponsors Post to : [email protected] Unsubscribe : https://launchpad.net/~sts-sponsors More help : https://help.launchpad.net/ListHelp
