sts-sponsors team mailing list archive

[Adam Collard <mp+435287@xxxxxxxxxxxxxxxxxx>]

Adam Collard has proposed merging ~adam-collard/maas:3.3-vault-proxy into maas:3.3.

Commit message:
Set no_proxy for vault, log exception connecting

Cherry-pick b24a6b7e530179eb819940492ad17cfc49fc4f9e

Requested reviews:
  Adam Collard (adam-collard)
Related bugs:
  Bug #2002111 in MAAS: "Connection to local Vault fails if proxy is configured"
  https://bugs.launchpad.net/maas/+bug/2002111

For more details, see:
https://code.launchpad.net/~adam-collard/maas/+git/maas/+merge/435287
-- 
Your team MAAS Committers is subscribed to branch maas:3.3.

diff --git a/src/maasserver/pytest_tests/test_vault.py b/src/maasserver/pytest_tests/test_vault.py
index c6cc302..7277dbb 100644
--- a/src/maasserver/pytest_tests/test_vault.py
+++ b/src/maasserver/pytest_tests/test_vault.py
@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta, timezone
+from os import environ
 from unittest.mock import ANY, MagicMock
 
 from django.core.exceptions import ImproperlyConfigured
@@ -8,6 +9,7 @@ import pytest
 from maasserver import vault
 from maasserver.models import Config
 from maasserver.vault import (
+    _create_hvac_client,
     _get_region_vault_client,
     clear_vault_client_caches,
     get_region_vault_client,
@@ -141,6 +143,27 @@ class TestVaultClient:
         ensure_auth.assert_called_once()
 
 
+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
+
+
 class TestGetRegionVaultClient:
     def test_cached(self, mocker):
         mock_get_client = mocker.patch.object(
diff --git a/src/maasserver/vault.py b/src/maasserver/vault.py
index 269c983..9ff213f 100644
--- a/src/maasserver/vault.py
+++ b/src/maasserver/vault.py
@@ -1,5 +1,7 @@
 from datetime import datetime, timedelta, timezone
 from functools import cache, wraps
+import logging
+import os
 from typing import Any, Callable, NamedTuple, Optional
 
 from dateutil.parser import isoparse
@@ -36,6 +38,7 @@ def wrap_errors(func: Callable) -> Callable:
         try:
             return func(*args, **kwargs)
         except requests.exceptions.ConnectionError as e:
+            logging.exception(e)
             raise VaultError("Vault connection failed") from e
         except hvac.exceptions.VaultError as e:
             raise VaultError("Vault request failed") from e
@@ -43,10 +46,22 @@ def wrap_errors(func: Callable) -> Callable:
     return wrapper
 
 
+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
+    return hvac.Client(url=url, **kwargs)
+
+
 @wrap_errors
 def unwrap_secret(url: str, wrapped_token: str) -> str:
     """Helper function to unwrap approle secret id from wrapped token"""
-    client = hvac.Client(url=url, token=wrapped_token)
+    client = _create_hvac_client(url, token=wrapped_token)
     try:
         return client.sys.unwrap()["data"]["secret_id"]
     except KeyError:
@@ -67,7 +82,7 @@ class VaultClient:
         secrets_mount: str = "secret",
         client: Optional[hvac.Client] = None,
     ):
-        self._client = client or hvac.Client(url=url)
+        self._client = client or _create_hvac_client(url)
         self._secrets_mount = secrets_mount
         self._secrets_base_path = secrets_base_path
         self._role_id = role_id