Igor Brovtsin has proposed merging
~igor-brovtsin/maas:vault-startup-clear-caches-early into maas:master.
Commit message:
fix(vault): Clean Vault caches before fetching RPC shared secret
Otherwise, RPC secret will be fetched from the DB after migration to Vault
(until `regiond` process is restarted). Since there is no RPC secret in the DB
after migration, region will generate a new secret, preventing clients access.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/433629
--
Your team MAAS Maintainers is requested to review the proposed merge of
~igor-brovtsin/maas:vault-startup-clear-caches-early into maas:master.
diff --git a/src/maasserver/start_up.py b/src/maasserver/start_up.py
index 135a3a6..c572b1f 100644
--- a/src/maasserver/start_up.py
+++ b/src/maasserver/start_up.py
@@ -120,6 +120,11 @@ def start_up(master=False):
"""
while True:
try:
+ # Since start_up now can be called multiple times in a process lifetime,
+ # vault client caches should be cleared in order to re-read the configuration.
+ # This prevents fetching shared secret from DB when Vault is already enabled.
+ clear_vault_client_caches()
+
# Ensure the shared secret is configured
secret = yield security.get_shared_secret()
MAAS_SECRET.set(secret)
@@ -195,9 +200,6 @@ def inner_start_up(master=False):
# Only perform the following if the master process for the
# region controller.
if master:
- # Since start_up now can be called multiple times in a process lifetime,
- # vault client caches should be cleared in order to re-read the configuration.
- clear_vault_client_caches()
# Migrate DB credentials to Vault and set the flag if Vault client is configured
client = get_region_vault_client()
if client is not None:
diff --git a/src/maasserver/tests/test_start_up.py b/src/maasserver/tests/test_start_up.py
index 7391da7..a49c0ac 100644
--- a/src/maasserver/tests/test_start_up.py
+++ b/src/maasserver/tests/test_start_up.py
@@ -7,13 +7,14 @@ from unittest.mock import call, MagicMock
from testtools.matchers import HasLength
-from maasserver import deprecations, eventloop, locks, start_up
+from maasserver import deprecations, eventloop, locks, start_up, vault
from maasserver.config import RegionConfiguration
from maasserver.models.config import Config
from maasserver.models.controllerinfo import ControllerInfo
from maasserver.models.node import RegionController
from maasserver.models.notification import Notification
from maasserver.models.signals import bootsources
+from maasserver.secrets import SecretManager
from maasserver.start_up import migrate_db_credentials_if_necessary
from maasserver.testing.config import RegionConfigurationFixture
from maasserver.testing.eventloop import RegionEventLoopFixture
@@ -22,6 +23,7 @@ from maasserver.testing.testcase import (
MAASServerTestCase,
MAASTransactionServerTestCase,
)
+from maasserver.testing.vault import FakeVaultClient
from maasserver.utils.orm import post_commit_hooks
from maasserver.vault import UnknownSecretPath, VaultError
from maastesting import get_testing_timeout
@@ -32,6 +34,7 @@ from maastesting.matchers import (
)
from provisioningserver.config import ConfigurationFile
from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
+from provisioningserver.security import to_hex
from provisioningserver.utils import ipaddr
from provisioningserver.utils.deb import DebVersionsInfo
from provisioningserver.utils.env import (
@@ -96,6 +99,30 @@ class TestStartUp(MAASTransactionServerTestCase):
# It also slept once, for 3 seconds, between those attempts.
self.expectThat(start_up.pause, MockCalledOnceWith(3.0))
+ def test_start_up_fetches_secret_from_vault_after_migration(self):
+ vault.clear_vault_client_caches()
+ # Prepare fake vault
+ expected_shared_secret = b"EXPECTED"
+ client = FakeVaultClient()
+ self.patch(vault, "_get_region_vault_client").return_value = client
+ SecretManager(client).set_simple_secret(
+ "rpc-shared", to_hex(expected_shared_secret)
+ )
+ # Cache vault being disabled
+ Config.objects.set_config("vault_enabled", False)
+ assert vault.get_region_vault_client_if_enabled() is None
+ # Enable vault as if migration was finished before the call
+ Config.objects.set_config("vault_enabled", True)
+ # No need to do actual startup
+ self.patch(start_up, "inner_start_up")
+ self.patch(start_up, "pause")
+
+ with post_commit_hooks:
+ start_up.start_up()
+
+ print(MAAS_SECRET.get(), expected_shared_secret)
+ assert MAAS_SECRET.get() == expected_shared_secret
+
class TestInnerStartUp(MAASServerTestCase):
def setUp(self):
@@ -247,12 +274,6 @@ class TestInnerStartUp(MAASServerTestCase):
self.assertEqual(region.version, "1:3.1.0-1234-g.deadbeef")
self.assertEqual(region.info.install_type, "deb")
- def test_clears_vault_cache(self):
- clear_caches_mock = self.patch(start_up, "clear_vault_client_caches")
- with post_commit_hooks:
- start_up.inner_start_up(master=True)
- clear_caches_mock.assert_called_once()
-
def test_sets_vault_flag_disabled(self):
self.patch(start_up, "get_region_vault_client").return_value = None
with post_commit_hooks:
--
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
