Alberto Donato has proposed merging ~ack/maas:vault-client-error-wrap into
maas:master.
Commit message:
wrap hvac.exceptions with local exceptions (for better error messages)
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/433305
--
Your team MAAS Maintainers is requested to review the proposed merge of
~ack/maas:vault-client-error-wrap into maas:master.
diff --git a/src/maascli/snap.py b/src/maascli/snap.py
index 0459598..a8b8068 100644
--- a/src/maascli/snap.py
+++ b/src/maascli/snap.py
@@ -16,11 +16,9 @@ from textwrap import dedent
import threading
import time
-from hvac.exceptions import VaultError
import netifaces
import psycopg2
from psycopg2.extensions import parse_dsn
-from requests.exceptions import ConnectionError
import tempita
from maascli.command import Command, CommandError
@@ -33,7 +31,7 @@ from maascli.init import (
prompt_for_choices,
read_input,
)
-from maasserver.vault import prepare_wrapped_approle
+from maasserver.vault import prepare_wrapped_approle, VaultError
ARGUMENTS = OrderedDict(
[
@@ -592,7 +590,7 @@ def get_vault_settings(options) -> dict:
secrets_path=options.vault_secrets_path,
secrets_mount=options.vault_secrets_mount,
)
- except (VaultError, ConnectionError) as e:
+ except VaultError as e:
raise CommandError(e)
return {
diff --git a/src/maascli/tests/test_snap.py b/src/maascli/tests/test_snap.py
index dca2c72..00f5262 100644
--- a/src/maascli/tests/test_snap.py
+++ b/src/maascli/tests/test_snap.py
@@ -13,13 +13,13 @@ import time
from unittest.mock import MagicMock
from fixtures import EnvironmentVariableFixture
-from hvac.exceptions import VaultError
import netifaces
import pytest
from maascli import snap
from maascli.command import CommandError
from maascli.parser import ArgumentParser
+from maasserver.vault import VaultError
from maastesting.factory import factory
from maastesting.testcase import MAASTestCase
diff --git a/src/maasserver/djangosettings/pytest_tests/test_settings.py b/src/maasserver/djangosettings/pytest_tests/test_settings.py
index 72ec38d..d655bff 100644
--- a/src/maasserver/djangosettings/pytest_tests/test_settings.py
+++ b/src/maasserver/djangosettings/pytest_tests/test_settings.py
@@ -10,7 +10,6 @@ from unittest.mock import MagicMock
from django.core.exceptions import ImproperlyConfigured
from django.db import connections
-from hvac.exceptions import InvalidPath, VaultError
from psycopg2.extensions import ISOLATION_LEVEL_REPEATABLE_READ
import pytest
@@ -21,6 +20,7 @@ from maasserver.djangosettings.settings import (
_get_local_timezone,
_read_timezone,
)
+from maasserver.vault import UnknownSecretPath, VaultError
from maastesting.factory import factory
@@ -125,7 +125,7 @@ class TestGetDefaultDbConfig:
def side_effect(given_path):
if given_path == db_creds_vault_path:
return expected
- raise InvalidPath()
+ raise UnknownSecretPath(given_path)
vault_client.get.side_effect = side_effect
get_vault_mock = mocker.patch.object(
@@ -158,7 +158,7 @@ class TestGetDefaultDbConfig:
"database_pass": factory.make_name("uuid"),
}
vault_client = MagicMock()
- vault_client.get.side_effect = [InvalidPath()]
+ vault_client.get.side_effect = [UnknownSecretPath("some-path")]
mocker.patch.object(
settings, "get_region_vault_client"
).return_value = vault_client
diff --git a/src/maasserver/djangosettings/settings.py b/src/maasserver/djangosettings/settings.py
index ba47b2f..4767bde 100644
--- a/src/maasserver/djangosettings/settings.py
+++ b/src/maasserver/djangosettings/settings.py
@@ -6,12 +6,15 @@ import logging
import os
from django.core.exceptions import ImproperlyConfigured
-from hvac.exceptions import InvalidPath, VaultError
from maasserver.config import get_db_creds_vault_path, RegionConfiguration
from maasserver.djangosettings import fix_up_databases
from maasserver.djangosettings.monkey import patch_get_script_prefix
-from maasserver.vault import get_region_vault_client
+from maasserver.vault import (
+ get_region_vault_client,
+ UnknownSecretPath,
+ VaultError,
+)
from provisioningserver.utils.env import MAAS_ID
@@ -64,7 +67,7 @@ def _get_default_db_config(config: RegionConfiguration) -> dict:
raise ImproperlyConfigured(
f"Incomplete Vault-stored DB credentials, missing key {e}"
)
- except InvalidPath:
+ except UnknownSecretPath:
# Vault does not have DB credentials, but is available. No need to report anything, use local credentials.
pass
except VaultError as e:
diff --git a/src/maasserver/management/commands/config_vault.py b/src/maasserver/management/commands/config_vault.py
index a9ad295..ca9f551 100644
--- a/src/maasserver/management/commands/config_vault.py
+++ b/src/maasserver/management/commands/config_vault.py
@@ -9,8 +9,6 @@ import time
from typing import Optional
from django.core.management.base import BaseCommand, CommandError
-from hvac.exceptions import VaultError
-from requests.exceptions import ConnectionError
import yaml
from maascli.init import prompt_yes_no
@@ -29,6 +27,7 @@ from maasserver.utils.orm import transactional, with_connection
from maasserver.vault import (
configure_region_with_vault,
get_region_vault_client,
+ VaultError,
WrappedSecretError,
)
from provisioningserver.utils.env import MAAS_ID
@@ -95,7 +94,7 @@ class Command(BaseCommand):
"""
)
- except (ConnectionError, VaultError, WrappedSecretError) as e:
+ except (VaultError, WrappedSecretError) as e:
raise CommandError(e)
def _get_online_regions(self) -> list[str]:
diff --git a/src/maasserver/management/commands/pytest_tests/test_config_vault.py b/src/maasserver/management/commands/pytest_tests/test_config_vault.py
index 7f4bd11..f79881c 100644
--- a/src/maasserver/management/commands/pytest_tests/test_config_vault.py
+++ b/src/maasserver/management/commands/pytest_tests/test_config_vault.py
@@ -1,9 +1,7 @@
from unittest.mock import MagicMock
from django.core.management import CommandError
-from hvac.exceptions import VaultError
import pytest
-from requests.exceptions import ConnectionError
import yaml
from maasserver.management.commands import config_vault
@@ -15,7 +13,7 @@ from maasserver.models import (
Secret,
)
from maasserver.testing.factory import factory
-from maasserver.vault import WrappedSecretError
+from maasserver.vault import VaultError, WrappedSecretError
from provisioningserver.utils.env import MAAS_ID
@@ -58,7 +56,7 @@ class TestConfigVaultConfigurateCommand:
def test_wraps_specific_exceptions_only(self, configure_mock):
handler = Command()
- side_effects = [ConnectionError(), VaultError(), WrappedSecretError()]
+ side_effects = [VaultError("error"), WrappedSecretError()]
configure_mock.side_effect = side_effects
kwargs = self._configure_kwargs()
diff --git a/src/maasserver/pytest_tests/test_vault.py b/src/maasserver/pytest_tests/test_vault.py
index cc2ff6d..d050999 100644
--- a/src/maasserver/pytest_tests/test_vault.py
+++ b/src/maasserver/pytest_tests/test_vault.py
@@ -2,7 +2,7 @@ from datetime import datetime, timedelta, timezone
from unittest.mock import ANY, MagicMock
from django.core.exceptions import ImproperlyConfigured
-from hvac.exceptions import VaultError
+from hvac.exceptions import VaultError as HVACVaultError
import pytest
from maasserver import vault
@@ -12,8 +12,9 @@ from maasserver.vault import (
clear_vault_client_caches,
get_region_vault_client,
get_region_vault_client_if_enabled,
- hvac,
+ UnknownSecretPath,
VaultClient,
+ VaultError,
WrappedSecretError,
)
@@ -62,7 +63,7 @@ class TestVaultClient:
assert vault_client.get("mysecret") == value
def test_get_not_found(self, vault_client):
- with pytest.raises(hvac.exceptions.InvalidPath):
+ with pytest.raises(UnknownSecretPath):
vault_client.get("mysecret")
def test_delete(self, mock_hvac_client):
@@ -134,7 +135,7 @@ class TestVaultClient:
client=mock_hvac_client,
)
ensure_auth = mocker.patch.object(client, "_ensure_auth")
- ensure_auth.side_effect = [VaultError()]
+ ensure_auth.side_effect = [HVACVaultError()]
with pytest.raises(VaultError):
client.check_authentication()
ensure_auth.assert_called_once()
@@ -250,7 +251,7 @@ class TestUnwrapSecret:
):
mocker.patch.object(
mock_hvac_client.sys, "unwrap"
- ).side_effect = VaultError("Test")
+ ).side_effect = HVACVaultError("Test")
with pytest.raises(VaultError):
vault.unwrap_secret("http://vault:8200";, "token")
diff --git a/src/maasserver/regiondservices/tests/test_vault_secrets_cleanup.py b/src/maasserver/regiondservices/tests/test_vault_secrets_cleanup.py
index 000ae35..e77215f 100644
--- a/src/maasserver/regiondservices/tests/test_vault_secrets_cleanup.py
+++ b/src/maasserver/regiondservices/tests/test_vault_secrets_cleanup.py
@@ -4,7 +4,6 @@
from unittest.mock import call
from django.db import transaction
-from hvac.exceptions import VaultError
from twisted.internet import reactor
from twisted.internet.defer import inlineCallbacks
@@ -17,6 +16,7 @@ from maasserver.secrets import SecretManager
from maasserver.testing.testcase import MAASTransactionServerTestCase
from maasserver.testing.vault import FakeVaultClient
from maasserver.utils.threads import deferToDatabase
+from maasserver.vault import VaultError
from maastesting.crochet import wait_for
wait_for_reactor = wait_for()
diff --git a/src/maasserver/regiondservices/vault_secrets_cleanup.py b/src/maasserver/regiondservices/vault_secrets_cleanup.py
index 2bd693f..9a5b43f 100644
--- a/src/maasserver/regiondservices/vault_secrets_cleanup.py
+++ b/src/maasserver/regiondservices/vault_secrets_cleanup.py
@@ -3,13 +3,16 @@ from datetime import timedelta
import logging
from django.db import transaction
-from hvac.exceptions import VaultError
from twisted.internet.defer import inlineCallbacks
from maasserver.models import VaultSecret
from maasserver.utils.orm import transactional
from maasserver.utils.threads import deferToDatabase
-from maasserver.vault import get_region_vault_client_if_enabled, VaultClient
+from maasserver.vault import (
+ get_region_vault_client_if_enabled,
+ VaultClient,
+ VaultError,
+)
from provisioningserver.utils.services import SingleInstanceService
from provisioningserver.utils.twisted import synchronous
diff --git a/src/maasserver/secrets.py b/src/maasserver/secrets.py
index 0607581..300413f 100644
--- a/src/maasserver/secrets.py
+++ b/src/maasserver/secrets.py
@@ -1,10 +1,13 @@
from typing import Any, Literal, NamedTuple, Optional
from django.db.models import Model
-from hvac.exceptions import InvalidPath
from maasserver.models import Node, RootKey, Secret, VaultSecret
-from maasserver.vault import get_region_vault_client_if_enabled, VaultClient
+from maasserver.vault import (
+ get_region_vault_client_if_enabled,
+ UnknownSecretPath,
+ VaultClient,
+)
SIMPLE_SECRET_KEY = "secret"
@@ -182,5 +185,5 @@ class SecretManager:
def _get_secret_from_vault(self, path: str):
try:
return self._vault_client.get(path)
- except InvalidPath:
+ except UnknownSecretPath:
raise SecretNotFound(path)
diff --git a/src/maasserver/tests/test_start_up.py b/src/maasserver/tests/test_start_up.py
index 0031760..7391da7 100644
--- a/src/maasserver/tests/test_start_up.py
+++ b/src/maasserver/tests/test_start_up.py
@@ -5,7 +5,6 @@ from pathlib import Path
import random
from unittest.mock import call, MagicMock
-from hvac.exceptions import InvalidPath, VaultError
from testtools.matchers import HasLength
from maasserver import deprecations, eventloop, locks, start_up
@@ -24,6 +23,7 @@ from maasserver.testing.testcase import (
MAASTransactionServerTestCase,
)
from maasserver.utils.orm import post_commit_hooks
+from maasserver.vault import UnknownSecretPath, VaultError
from maastesting import get_testing_timeout
from maastesting.matchers import (
MockCalledOnceWith,
@@ -39,7 +39,7 @@ from provisioningserver.utils.env import (
MAAS_SECRET,
MAAS_SHARED_SECRET,
)
-from provisioningserver.utils.testing import MAASIDFixture
+from provisioningserver.utils.testing import MAASIDFixture, MAASUUIDFixture
class TestStartUp(MAASTransactionServerTestCase):
@@ -101,6 +101,7 @@ class TestInnerStartUp(MAASServerTestCase):
def setUp(self):
super().setUp()
self.useFixture(MAASIDFixture(None))
+ self.useFixture(MAASUUIDFixture("uuid"))
self.patch_autospec(start_up, "dns_kms_setting_changed")
self.patch(ipaddr, "get_ip_addr").return_value = {}
self.versions_info = DebVersionsInfo(
@@ -388,7 +389,7 @@ class TestVaultMigrateDbCredentials(MAASServerTestCase):
def get_side_effect(path):
if path == self.creds_path:
return {"name": db_name, "user": db_user, "pass": db_pass}
- raise InvalidPath(path)
+ raise UnknownSecretPath(path)
client = MagicMock()
client.get.side_effect = get_side_effect
diff --git a/src/maasserver/vault.py b/src/maasserver/vault.py
index 5b4e80b..269c983 100644
--- a/src/maasserver/vault.py
+++ b/src/maasserver/vault.py
@@ -1,10 +1,11 @@
from datetime import datetime, timedelta, timezone
-from functools import cache
-from typing import Any, NamedTuple, Optional
+from functools import cache, wraps
+from typing import Any, Callable, NamedTuple, Optional
from dateutil.parser import isoparse
from django.core.exceptions import ImproperlyConfigured
import hvac
+import requests
from maasserver.config import RegionConfiguration
@@ -15,10 +16,34 @@ TOKEN_BEFORE_EXPIRY_LIMIT = timedelta(seconds=10)
SecretValue = dict[str, Any]
+class VaultError(Exception):
+ """Raised to wrap the hvac.exception.VaultError one."""
+
+
+class UnknownSecretPath(Exception):
+ """Raised when the path for a secret is unknown."""
+
+
class WrappedSecretError(Exception):
"""Raised when the provided token could not be used to obtain secret_id by unwrapping"""
+def wrap_errors(func: Callable) -> Callable:
+ """Wrap hvac exceptions with local ones."""
+
+ @wraps(func)
+ def wrapper(*args, **kwargs):
+ try:
+ return func(*args, **kwargs)
+ except requests.exceptions.ConnectionError as e:
+ raise VaultError("Vault connection failed") from e
+ except hvac.exceptions.VaultError as e:
+ raise VaultError("Vault request failed") from e
+
+ return wrapper
+
+
+@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)
@@ -50,6 +75,7 @@ class VaultClient:
# ensure a token is created at the first request
self._token_expire = self._utcnow()
+ @wrap_errors
def set(self, path: str, value: SecretValue):
self._ensure_auth()
self._kv.create_or_update_secret(
@@ -58,13 +84,18 @@ class VaultClient:
mount_point=self._secrets_mount,
)
+ @wrap_errors
def get(self, path: str) -> SecretValue:
self._ensure_auth()
- return self._kv.read_secret(
- self._secret_path(path),
- mount_point=self._secrets_mount,
- )["data"]["data"]
-
+ try:
+ return self._kv.read_secret(
+ self._secret_path(path),
+ mount_point=self._secrets_mount,
+ )["data"]["data"]
+ except hvac.exceptions.InvalidPath:
+ raise UnknownSecretPath(path)
+
+ @wrap_errors
def delete(self, path: str):
self._ensure_auth()
return self._kv.delete_metadata_and_all_versions(
@@ -72,6 +103,7 @@ class VaultClient:
mount_point=self._secrets_mount,
)
+ @wrap_errors
def check_authentication(self):
"""Checks if vault is available, throws otherwise"""
self._ensure_auth(force=True)
--
Mailing list: https://launchpad.net/~sts-sponsors
Post to : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help : https://help.launchpad.net/ListHelp
