From 74b6e24beab6526074cbcf9489487e545e81333e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 27 Mar 2026 12:21:45 +0100 Subject: [PATCH] Add Hashicorp Vault token renewal support Add a celery beat for renewal of Hashicorp Vault tokens. This eliminates the need for manual token rotation when using short-lived renewable tokens. The approach: - HashicorpVault checks token renewable status on startup (warning if not renewable) - New `renew_vault_token` Celery Beat task calls renew-self - Configured via `vault_token_renewal_interval` in galaxy.yml (default 0 = disabled) - Requires Celery Beat to be running Fixes https://github.com/galaxyproject/galaxy/issues/22187 --- doc/source/admin/special_topics/vault.md | 27 ++ lib/galaxy/celery/__init__.py | 3 + lib/galaxy/celery/tasks.py | 10 + lib/galaxy/config/schemas/config_schema.yml | 12 + lib/galaxy/dependencies/dev-requirements.txt | 1 + .../dependencies/pinned-test-requirements.txt | 1 + lib/galaxy/security/vault.py | 74 +++++- pyproject.toml | 1 + test/integration/test_hashicorp_vault.py | 239 ++++++++++++++++++ test/unit/data/security/test_vault.py | 107 ++++++++ 10 files changed, 472 insertions(+), 3 deletions(-) create mode 100644 test/integration/test_hashicorp_vault.py diff --git a/doc/source/admin/special_topics/vault.md b/doc/source/admin/special_topics/vault.md index a130e6ac97e..fa12f31de4c 100644 --- a/doc/source/admin/special_topics/vault.md +++ b/doc/source/admin/special_topics/vault.md @@ -49,6 +49,33 @@ vault_address: http://localhost:8200 vault_token: vault_application_token ``` +### Token Renewal + +Galaxy supports automatic renewal of Hashicorp Vault tokens via a Celery Beat periodic task. This is the recommended approach for production deployments using renewable tokens with a short TTL. + +On startup, Galaxy checks whether the configured token is renewable and logs a warning if it is not. + +To enable automatic renewal, set `vault_token_renewal_interval` in `galaxy.yml`: + +```yaml +galaxy: + vault_token_renewal_interval: 1800 # renew every 30 minutes +``` + +This requires Celery Beat to be running. The periodic task calls Vault's `renew-self` endpoint at the configured interval. + +**Recommended token creation for production use:** + +Create a renewable token with a short TTL but a long max TTL: + +```bash +vault token create -policy=galaxy -ttl=1h -explicit-max-ttl=720h -renewable +``` + +This creates a token that must be renewed every hour, but can be renewed for up to 30 days. Set `vault_token_renewal_interval` to half the TTL (e.g. 1800 for a 1-hour TTL). + +If the token is not renewable, Galaxy logs a warning at startup but continues to operate normally. If renewal fails at runtime, the Celery task will retry at the next scheduled interval. + ## Vault configuration for database ```yaml diff --git a/lib/galaxy/celery/__init__.py b/lib/galaxy/celery/__init__.py index a15939cbd8a..72e977b828d 100644 --- a/lib/galaxy/celery/__init__.py +++ b/lib/galaxy/celery/__init__.py @@ -248,6 +248,9 @@ def setup_periodic_tasks(config, celery_app): if config.enable_failed_jobs_working_directory_cleanup: schedule_task("cleanup_jwds", config.failed_jobs_working_directory_cleanup_interval) + if config.vault_token_renewal_interval: + schedule_task("renew_vault_token", config.vault_token_renewal_interval) + if beat_schedule: celery_app.conf.beat_schedule = beat_schedule diff --git a/lib/galaxy/celery/tasks.py b/lib/galaxy/celery/tasks.py index 507fd42ccbd..ff7270ee49d 100644 --- a/lib/galaxy/celery/tasks.py +++ b/lib/galaxy/celery/tasks.py @@ -70,6 +70,10 @@ from galaxy.schema.tasks import ( WriteHistoryTo, WriteInvocationTo, ) +from galaxy.security.vault import ( + renew_vault_token_if_needed, + Vault, +) from galaxy.short_term_storage import ShortTermStorageMonitor from galaxy.structured_app import MinimalManagerApp from galaxy.tools import create_tool_from_representation @@ -638,6 +642,12 @@ def cleanup_jwds(sa_session: galaxy_scoped_session, object_store: BaseObjectStor log.info("Deleted job working directory for job %s", job.id) +@galaxy_task(action="renewing Hashicorp Vault token") +def renew_vault_token(vault: Vault): + """Renew the Hashicorp Vault token if configured and renewable.""" + renew_vault_token_if_needed(vault) + + @galaxy_task(action="execute workflow completion hook") def execute_workflow_completion_hook( invocation_id: int, diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 6b8a553782c..564b813659c 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -4179,6 +4179,18 @@ mapping: desc: | Vault config file. + vault_token_renewal_interval: + type: int + default: 0 + required: false + desc: | + Time (in seconds) between Hashicorp Vault token renewal attempts. + Set to 0 to disable automatic token renewal (the default). + When enabled, a Celery Beat periodic task will call Vault's + renew-self endpoint at this interval. Recommended value: half the + token TTL (e.g. 1800 for a 1-hour TTL token). Requires Celery + Beat to be running. + url_headers_config_file: type: str default: url_headers_conf.yml diff --git a/lib/galaxy/dependencies/dev-requirements.txt b/lib/galaxy/dependencies/dev-requirements.txt index 20e51692f62..b03ee27d28e 100644 --- a/lib/galaxy/dependencies/dev-requirements.txt +++ b/lib/galaxy/dependencies/dev-requirements.txt @@ -62,6 +62,7 @@ grpcio==1.78.0 grpcio-status==1.78.0 h11==0.16.0 html5rdf==1.2.1 +hvac==2.4.0 id==1.6.1 idna==3.11 imagesize==2.0.0 diff --git a/lib/galaxy/dependencies/pinned-test-requirements.txt b/lib/galaxy/dependencies/pinned-test-requirements.txt index 8593d8208e7..0525860c3f0 100644 --- a/lib/galaxy/dependencies/pinned-test-requirements.txt +++ b/lib/galaxy/dependencies/pinned-test-requirements.txt @@ -51,6 +51,7 @@ grpcio==1.78.0 grpcio-status==1.78.0 h11==0.16.0 html5rdf==1.2.1 +hvac==2.4.0 idna==3.11 importlib-metadata==8.7.1 ; python_full_version < '3.12' iniconfig==2.3.0 diff --git a/lib/galaxy/security/vault.py b/lib/galaxy/security/vault.py index 09d87a8acae..fcdfbb041fc 100644 --- a/lib/galaxy/security/vault.py +++ b/lib/galaxy/security/vault.py @@ -98,7 +98,7 @@ class NullVault(Vault): class HashicorpVault(Vault): - def __init__(self, config): + def __init__(self, config, token_renewal_enabled=False): if not hvac: raise InvalidVaultConfigException( "Hashicorp vault library 'hvac' is not available. Make sure hvac is installed." @@ -106,6 +106,41 @@ class HashicorpVault(Vault): self.vault_address = config.get("vault_address") self.vault_token = config.get("vault_token") self.client = hvac.Client(url=self.vault_address, token=self.vault_token) + if token_renewal_enabled: + self._check_token_renewable() + + def _check_token_renewable(self): + try: + token_info = self.client.auth.token.lookup_self() + data = token_info.get("data", {}) + renewable = data.get("renewable", False) + ttl = data.get("ttl", 0) + if not renewable: + log.error( + "Hashicorp Vault token is not renewable, but vault_token_renewal_interval is set. " + "The token will expire and cannot be renewed. " + "Generate a renewable token with: vault token create -policy= -ttl=1h -explicit-max-ttl=720h -renewable" + ) + elif ttl > 0: + log.info("Hashicorp Vault token is renewable (TTL: %ds).", ttl) + else: + log.info("Hashicorp Vault token is renewable (no TTL).") + except Exception: + log.exception("Failed to look up Hashicorp Vault token info.") + + def renew_token(self): + """Renew the Vault token. Intended to be called periodically by a Celery Beat task.""" + result = self.client.auth.token.renew_self() + auth_data = result.get("auth", {}) + new_ttl = auth_data.get("lease_duration", 0) + renewable = auth_data.get("renewable", False) + if not renewable: + log.error( + "Hashicorp Vault token is no longer renewable (max TTL likely reached). " + "A new token must be configured." + ) + else: + log.debug("Hashicorp Vault token renewed successfully (new TTL: %ds).", new_ttl) def read_secret(self, key: str) -> Optional[str]: try: @@ -114,9 +149,24 @@ class HashicorpVault(Vault): except hvac.exceptions.InvalidPath: log.exception(f"Failed to read secret from Hashicorp Vault at key: {key}") return None + except hvac.exceptions.Forbidden: + log.error( + "Permission denied reading secret at key: %s. " + "The Vault token may have expired. Check token renewal configuration.", + key, + ) + return None def write_secret(self, key: str, value: str) -> None: - self.client.secrets.kv.v2.create_or_update_secret(path=key, secret={"value": value}) + try: + self.client.secrets.kv.v2.create_or_update_secret(path=key, secret={"value": value}) + except hvac.exceptions.Forbidden: + log.error( + "Permission denied writing secret at key: %s. " + "The Vault token may have expired. Check token renewal configuration.", + key, + ) + raise def list_secrets(self, key: str) -> list[str]: raise NotImplementedError() @@ -258,7 +308,8 @@ class VaultFactory: def from_vault_type(app, vault_type: Optional[str], cfg: dict) -> Vault: vault: Vault if vault_type == "hashicorp": - vault = HashicorpVault(cfg) + token_renewal_enabled = app.config.vault_token_renewal_interval > 0 + vault = HashicorpVault(cfg, token_renewal_enabled=token_renewal_enabled) elif vault_type == "database": vault = DatabaseVault(app.model.context, cfg) else: @@ -277,3 +328,20 @@ class VaultFactory: def is_vault_configured(vault: Vault) -> bool: return not isinstance(vault, NullVault) + + +def _unwrap_vault(vault: Vault) -> Vault: + """Unwrap decorator layers to get the underlying vault implementation.""" + while hasattr(vault, "vault"): + vault = vault.vault + return vault + + +def renew_vault_token_if_needed(vault: Vault) -> None: + """Renew the Hashicorp Vault token if the vault is a HashicorpVault. + + Intended to be called from a Celery Beat periodic task. + """ + inner = _unwrap_vault(vault) + if isinstance(inner, HashicorpVault): + inner.renew_token() diff --git a/pyproject.toml b/pyproject.toml index 63778e6bdb3..8d02fa693b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -128,6 +128,7 @@ test = [ "cwltest>=2.5.20240906231108", # Python 3.13 support "fluent-logger", "gcsfs", + "hvac", "lxml!=4.2.2", "onedatafilerestclient==21.2.5.2", "pkce", diff --git a/test/integration/test_hashicorp_vault.py b/test/integration/test_hashicorp_vault.py new file mode 100644 index 00000000000..dc0c6dae041 --- /dev/null +++ b/test/integration/test_hashicorp_vault.py @@ -0,0 +1,239 @@ +"""Integration tests for Hashicorp Vault token renewal using a real Vault Docker container. + +Requires Docker to be available. The test starts a hashicorp/vault container in dev mode, +creates a renewable token, and verifies the renewal logic works end-to-end. +""" + +import os +import subprocess +import tempfile +import threading +import time + +import requests + +from galaxy.security.vault import ( + _unwrap_vault, + HashicorpVault, +) +from galaxy.util.wait import wait_on +from galaxy_test.base.populators import CredentialsPopulator +from galaxy_test.driver import integration_util +from galaxy_test.driver.integration_util import ( + docker_rm, + docker_run, + skip_unless_docker, +) + +VAULT_DEV_ROOT_TOKEN = "vault-integration-test-token" +VAULT_PORT = 18200 +VAULT_IMAGE = "hashicorp/vault" +CREDENTIALS_TOOL = "secret_tool" +CREDENTIALS_VARIABLES = [{"name": "server", "value": "http://localhost:8080"}] +CREDENTIALS_SECRETS = [{"name": "username", "value": "user"}, {"name": "password", "value": "pass"}] + + +class VaultClient: + """Thin wrapper around the Vault HTTP API for test setup.""" + + def __init__(self, addr, token): + self.addr = addr + self.session = requests.Session() + self.session.headers.update( + { + "X-Vault-Token": token, + "Content-Type": "application/json", + } + ) + + def wait_ready(self, timeout=30): + def check(): + try: + self.session.get(f"{self.addr}/v1/sys/health", timeout=2).raise_for_status() + return True + except Exception: + return None + + wait_on(check, "Vault to become ready", timeout) + + def create_policy(self, name, hcl): + self.session.put(f"{self.addr}/v1/sys/policies/acl/{name}", json={"policy": hcl}) + + def create_renewable_token(self, ttl="1h", max_ttl="24h", policies=None): + result = self.session.post( + f"{self.addr}/v1/auth/token/create", + json={ + "ttl": ttl, + "explicit_max_ttl": max_ttl, + "renewable": True, + "policies": policies or ["default"], + }, + ).json() + return result["auth"]["client_token"] + + +def _write_vault_config(vault_addr, vault_token, path_prefix="/galaxy_integration_test"): + fd, path = tempfile.mkstemp(prefix="vault_hashicorp_integ_", suffix=".yml") + with os.fdopen(fd, "w") as f: + f.write( + f"type: hashicorp\n" + f"path_prefix: {path_prefix}\n" + f"vault_address: {vault_addr}\n" + f"vault_token: {vault_token}\n" + ) + return path + + +def _start_vault_container(container_name): + try: + docker_rm(container_name) + except subprocess.CalledProcessError: + pass + docker_run( + VAULT_IMAGE, + container_name, + "server", + "-dev", + f"-dev-root-token-id={VAULT_DEV_ROOT_TOKEN}", + "-dev-listen-address=0.0.0.0:8200", + ports=[(VAULT_PORT, 8200)], + env_vars={"VAULT_ADDR": "http://0.0.0.0:8200"}, + ) + vault_addr = f"http://127.0.0.1:{VAULT_PORT}" + client = VaultClient(vault_addr, VAULT_DEV_ROOT_TOKEN) + client.wait_ready() + client.create_policy( + "galaxy", + r'path "secret/*" { capabilities = ["create","read","update","delete","list"] }', + ) + return vault_addr, client + + +@skip_unless_docker() +class TestHashicorpVaultRenewalGalaxyIntegration(integration_util.IntegrationTestCase): + """Full Galaxy + Vault + Celery Beat integration test. + + Starts a Galaxy instance backed by a Hashicorp Vault Docker container. + An in-process Celery Beat scheduler fires ``renew_vault_token`` every + 2 s with ``task_always_eager`` so tasks execute immediately in the Beat + thread via the Galaxy app's DI container. + + The test stores credentials (which write secrets to the vault) via the + Galaxy API, then verifies the operation still succeeds past the token + TTL thanks to Beat renewal. + """ + + container_name = "galaxy_test_hashicorp_vault_galaxy" + vault_addr: str + vault_config_path: str + _vault_client: VaultClient + _vault_token: str + _beat_service = None + _beat_thread = None + RENEWAL_INTERVAL = 2 # seconds + + @classmethod + def setUpClass(cls): + cls.vault_addr, cls._vault_client = _start_vault_container(cls.container_name) + super().setUpClass() + cls._start_beat() + + @classmethod + def tearDownClass(cls): + cls._stop_beat() + super().tearDownClass() + docker_rm(cls.container_name) + if hasattr(cls, "vault_config_path") and os.path.exists(cls.vault_config_path): + os.unlink(cls.vault_config_path) + + @classmethod + def handle_galaxy_config_kwds(cls, config): + super().handle_galaxy_config_kwds(config) + cls._vault_token = cls._vault_client.create_renewable_token( + ttl="1h", + max_ttl="24h", + policies=["galaxy"], + ) + cls.vault_config_path = _write_vault_config( + cls.vault_addr, + cls._vault_token, + path_prefix="/galaxy", + ) + config["vault_config_file"] = cls.vault_config_path + config["vault_token_renewal_interval"] = cls.RENEWAL_INTERVAL + + def setUp(self): + super().setUp() + self.credentials_populator = CredentialsPopulator(self.galaxy_interactor) + + # ---- Beat ---------------------------------------------------------------- + + @classmethod + def _start_beat(cls): + from celery.beat import Service as BeatService + + from galaxy.celery import celery_app + + module_name = celery_app.trim_module_name("galaxy.celery.tasks") + schedule = dict(celery_app.conf.beat_schedule or {}) + schedule["renew-vault-token"] = { + "task": f"{module_name}.renew_vault_token", + "schedule": cls.RENEWAL_INTERVAL, + } + celery_app.conf.beat_schedule = schedule + celery_app.conf.task_always_eager = True + + cls._beat_service = BeatService(celery_app, max_interval=cls.RENEWAL_INTERVAL) + cls._beat_thread = threading.Thread(target=cls._beat_service.start, daemon=True) + cls._beat_thread.start() + time.sleep(1) + + @classmethod + def _stop_beat(cls): + if cls._beat_service: + cls._beat_service.stop() + if cls._beat_thread: + cls._beat_thread.join(timeout=5) + from galaxy.celery import celery_app + + celery_app.conf.task_always_eager = False + + # ---- helpers ------------------------------------------------------------- + + def _swap_token(self, new_token): + inner = _unwrap_vault(self._app.vault) + assert isinstance(inner, HashicorpVault) + inner.client.token = new_token + inner.vault_token = new_token + + # ---- tests --------------------------------------------------------------- + + def test_beat_renews_token_and_secrets_survive(self): + """Beat fires renew_vault_token every 2 s, keeping the token alive past its 3 s TTL.""" + # Swap in a short-lived token after Galaxy is running. + short_token = self._vault_client.create_renewable_token( + ttl="3s", + max_ttl="1h", + policies=["galaxy"], + ) + self._swap_token(short_token) + + # Store credentials — secrets go to Vault. + self.credentials_populator.create_credentials( + tool_id=CREDENTIALS_TOOL, + variables=CREDENTIALS_VARIABLES, + secrets=CREDENTIALS_SECRETS, + ) + + # Beat renews every 2 s. Sleep 4 s — past the 3 s TTL. + time.sleep(4) + + # Creating credentials again exercises a vault write — this would fail + # with Forbidden if the token had expired. + self.credentials_populator.create_credentials( + tool_id=CREDENTIALS_TOOL, + variables=CREDENTIALS_VARIABLES, + secrets=CREDENTIALS_SECRETS, + ) + + self._swap_token(self._vault_token) diff --git a/test/unit/data/security/test_vault.py b/test/unit/data/security/test_vault.py index 39794f8e5ff..baeab3900d8 100644 --- a/test/unit/data/security/test_vault.py +++ b/test/unit/data/security/test_vault.py @@ -1,6 +1,11 @@ +import logging import os import string import tempfile +from unittest.mock import ( + MagicMock, + patch, +) import pytest from cryptography.fernet import InvalidToken @@ -10,9 +15,15 @@ from galaxy.model.unittest_utils.data_app import ( GalaxyDataTestConfig, ) from galaxy.security.vault import ( + _unwrap_vault, + HashicorpVault, InvalidVaultKeyException, + NullVault, + renew_vault_token_if_needed, Vault, VaultFactory, + VaultKeyPrefixWrapper, + VaultKeyValidationWrapper, ) from galaxy.util.unittest import TestCase @@ -73,6 +84,16 @@ class TestHashicorpVault(AbstractTestCases.VaultTestBase): app = GalaxyDataTestApp(config=config) self.vault = VaultFactory.from_app(app) + def test_renew_token(self): + """Test that vault token renewal works against a real Hashicorp Vault.""" + inner = _unwrap_vault(self.vault) + assert isinstance(inner, HashicorpVault), f"Expected HashicorpVault, got {type(inner)}" + inner.renew_token() + + def test_renew_vault_token_if_needed(self): + """Test the full renewal path through renew_vault_token_if_needed with a real Vault.""" + renew_vault_token_if_needed(self.vault) + def tearDown(self) -> None: os.remove(self.vault_temp_conf) @@ -110,3 +131,89 @@ class TestDatabaseVault(AbstractTestCases.VaultTestBase): vault = VaultFactory.from_app(app) with self.assertRaises(InvalidToken): vault.read_secret("my/incorrect/secret") + + +@patch("galaxy.security.vault.hvac") +class TestHashicorpVaultTokenRenewal: + def _make_vault(self, hvac_mock, token_lookup_data=None, token_renewal_enabled=False): + # Set up real exception classes so except clauses work with the mock + hvac_mock.exceptions.Forbidden = type("Forbidden", (Exception,), {}) + hvac_mock.exceptions.InvalidPath = type("InvalidPath", (Exception,), {}) + + mock_client = MagicMock() + hvac_mock.Client.return_value = mock_client + if token_lookup_data is not None: + mock_client.auth.token.lookup_self.return_value = {"data": token_lookup_data} + config = { + "vault_address": "http://localhost:8200", + "vault_token": "s.test-token", + } + vault = HashicorpVault(config, token_renewal_enabled=token_renewal_enabled) + return vault, mock_client + + def test_startup_errors_non_renewable_token(self, hvac_mock, caplog): + """When renewal is enabled but the token isn't renewable, log an error.""" + with caplog.at_level(logging.ERROR, logger="galaxy.security.vault"): + self._make_vault( + hvac_mock, + token_lookup_data={"renewable": False, "ttl": 3600}, + token_renewal_enabled=True, + ) + assert "not renewable" in caplog.text + + def test_startup_skips_check_when_renewal_disabled(self, hvac_mock): + """No Vault API call when renewal is not configured.""" + vault, mock_client = self._make_vault( + hvac_mock, + token_lookup_data={"renewable": False, "ttl": 3600}, + token_renewal_enabled=False, + ) + mock_client.auth.token.lookup_self.assert_not_called() + + def test_startup_handles_lookup_failure(self, hvac_mock): + """Vault unreachable at startup should not crash Galaxy.""" + mock_client = MagicMock() + hvac_mock.Client.return_value = mock_client + mock_client.auth.token.lookup_self.side_effect = Exception("connection refused") + config = { + "vault_address": "http://localhost:8200", + "vault_token": "s.test-token", + } + vault = HashicorpVault(config, token_renewal_enabled=True) + assert vault.client is mock_client + + def test_renew_vault_token_if_needed_unwraps_decorators(self, hvac_mock): + vault, mock_client = self._make_vault( + hvac_mock, + token_lookup_data={"renewable": True, "ttl": 3600}, + ) + mock_client.auth.token.renew_self.return_value = {"auth": {"lease_duration": 3600, "renewable": True}} + # Wrap in decorators like VaultFactory does + wrapped = VaultKeyValidationWrapper(VaultKeyPrefixWrapper(vault, prefix="/galaxy")) + renew_vault_token_if_needed(wrapped) + mock_client.auth.token.renew_self.assert_called_once() + + def test_renew_token_propagates_exception_on_failure(self, hvac_mock): + vault, mock_client = self._make_vault( + hvac_mock, + token_lookup_data={"renewable": True, "ttl": 3600}, + ) + mock_client.auth.token.renew_self.side_effect = Exception("Vault sealed") + with pytest.raises(Exception, match="Vault sealed"): + vault.renew_token() + + def test_renew_vault_token_if_needed_noop_for_non_hashicorp(self, hvac_mock): + # Should not raise for non-HashicorpVault + renew_vault_token_if_needed(NullVault()) + + def test_read_secret_forbidden_returns_none(self, hvac_mock): + vault, mock_client = self._make_vault(hvac_mock) + mock_client.secrets.kv.read_secret_version.side_effect = hvac_mock.exceptions.Forbidden + result = vault.read_secret("some/key") + assert result is None + + def test_write_secret_forbidden_raises(self, hvac_mock): + vault, mock_client = self._make_vault(hvac_mock) + mock_client.secrets.kv.v2.create_or_update_secret.side_effect = hvac_mock.exceptions.Forbidden + with pytest.raises(hvac_mock.exceptions.Forbidden): + vault.write_secret("some/key", "value")