From 3ce26f7b947f926bf9d084616ec5b53371297d66 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Wed, 8 Apr 2026 10:26:25 +0200 Subject: [PATCH 1/3] Avoid eager vault lookup for YAML secrets Add load_yaml_file and reuse it in the CLI paths that read OpenStack, RabbitMQ, Octavia, and database secrets from YAML files. Several commands called get_vault() before checking whether the file was actually vault-encrypted. In deployments that keep secrets as plain YAML, this produced repeated 'Unable to get ansible vault password' errors for a path that is never needed. Read the file first and only initialize the vault when the content is encrypted. This removes noisy false-error output during checks and deployments while preserving decryption for encrypted secrets. AI-assisted: Codex Signed-off-by: Roger Luethi --- osism/commands/loadbalancer.py | 18 ++----------- osism/commands/status.py | 18 ++----------- osism/tasks/conductor/utils.py | 29 ++++++++++++++++++++ osism/tasks/openstack.py | 49 ++-------------------------------- osism/utils/rabbitmq.py | 19 ++----------- 5 files changed, 37 insertions(+), 96 deletions(-) diff --git a/osism/commands/loadbalancer.py b/osism/commands/loadbalancer.py index e5fe0e41..168e4b81 100644 --- a/osism/commands/loadbalancer.py +++ b/osism/commands/loadbalancer.py @@ -39,23 +39,9 @@ def _load_octavia_database_password(): return None try: - from osism.tasks.conductor.utils import get_vault + from osism.tasks.conductor.utils import load_yaml_file - vault = get_vault() - - with open(secrets_path, "rb") as f: - file_data = f.read() - - if vault.is_encrypted(file_data): - decrypted_data = vault.decrypt(file_data).decode() - logger.debug(f"Successfully decrypted secrets file: {secrets_path}") - else: - decrypted_data = file_data.decode() - logger.debug( - f"Secrets file is not encrypted (development mode): {secrets_path}" - ) - - secrets = yaml.safe_load(decrypted_data) + secrets = load_yaml_file(secrets_path) if not secrets or not isinstance(secrets, dict): logger.error("Empty or invalid secrets file") diff --git a/osism/commands/status.py b/osism/commands/status.py index be022a46..7e967d40 100644 --- a/osism/commands/status.py +++ b/osism/commands/status.py @@ -104,8 +104,6 @@ def _load_kolla_configuration(self): def _load_database_password(self): """Load and decrypt the database password from secrets.yml""" - from osism.tasks.conductor.utils import get_vault - secrets_path = "/opt/configuration/environments/kolla/secrets.yml" if not os.path.exists(secrets_path): @@ -113,21 +111,9 @@ def _load_database_password(self): return None try: - vault = get_vault() - - with open(secrets_path, "rb") as f: - file_data = f.read() - - if vault.is_encrypted(file_data): - decrypted_data = vault.decrypt(file_data).decode() - logger.debug(f"Successfully decrypted secrets file: {secrets_path}") - else: - decrypted_data = file_data.decode() - logger.debug( - f"Secrets file is not encrypted (development mode): {secrets_path}" - ) + from osism.tasks.conductor.utils import load_yaml_file - secrets = yaml.safe_load(decrypted_data) + secrets = load_yaml_file(secrets_path) if not secrets or not isinstance(secrets, dict): logger.error("Empty or invalid secrets file") diff --git a/osism/tasks/conductor/utils.py b/osism/tasks/conductor/utils.py index dc8f74e4..25cee2f1 100644 --- a/osism/tasks/conductor/utils.py +++ b/osism/tasks/conductor/utils.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 +import os + from ansible import constants as ansible_constants from ansible.parsing.vault import VaultLib, VaultSecret from loguru import logger @@ -7,6 +9,7 @@ from osism import utils import sushy import urllib3 +import yaml def deep_compare(a, b, updates): @@ -88,6 +91,32 @@ def get_vault(): return vault +def load_yaml_file(path): + """Load a YAML file and only request the vault secret when needed.""" + if not os.path.exists(path): + logger.error(f"YAML file not found: {path}") + return None + + try: + with open(path, "rb") as f: + file_data = f.read() + + if VaultLib().is_encrypted(file_data): + decrypted_data = get_vault().decrypt(file_data).decode() + logger.debug(f"Successfully decrypted vault-encrypted YAML file: {path}") + else: + decrypted_data = file_data.decode() + logger.debug(f"YAML file is not encrypted: {path}") + + return yaml.safe_load(decrypted_data) + except yaml.YAMLError as exc: + logger.error(f"Failed to parse YAML file {path}: {exc}") + return None + except Exception as exc: + logger.error(f"Failed to load YAML file {path}: {exc}") + return None + + def get_redfish_connection( hostname, username=None, password=None, ignore_ssl_errors=True, timeout=None ): diff --git a/osism/tasks/openstack.py b/osism/tasks/openstack.py index 86809140..ed00f1fc 100644 --- a/osism/tasks/openstack.py +++ b/osism/tasks/openstack.py @@ -10,7 +10,7 @@ from osism import settings, utils from osism.tasks import Config, run_command -from osism.tasks.conductor.utils import get_vault +from osism.tasks.conductor.utils import load_yaml_file app = Celery("openstack") app.config_from_object(Config) @@ -432,52 +432,7 @@ def get_cloud_password(cloud): logger.warning(f"Secrets file not found: {secrets_path}") return None - # Get vault instance for decryption - vault = get_vault() - - # Load the secrets file - with open(secrets_path, "rb") as f: - file_data = f.read() - - decrypted_secrets = None - - # Try to decrypt the file if it's vault encrypted - try: - if vault.is_encrypted(file_data): - # File is encrypted, decrypt it - decrypted_data = vault.decrypt(file_data).decode() - logger.debug(f"Successfully decrypted secrets file: {secrets_path}") - else: - # File is not encrypted, use as-is - decrypted_data = file_data.decode() - logger.debug( - f"Secrets file is not encrypted (development mode): {secrets_path}" - ) - - # Parse the YAML content safely - try: - decrypted_secrets = yaml.safe_load(decrypted_data) - except yaml.YAMLError as yaml_exc: - logger.error( - f"Failed to parse YAML content from secrets file: {yaml_exc}" - ) - return None - - except Exception as decrypt_exc: - # If decryption fails, try reading as plain YAML (development fallback) - logger.warning( - f"Failed to decrypt secrets file, attempting to read as plain YAML: {decrypt_exc}" - ) - try: - with open(secrets_path, "r") as f: - decrypted_secrets = yaml.safe_load(f) - logger.debug( - f"Successfully loaded unencrypted secrets file (development mode): {secrets_path}" - ) - except Exception as plain_exc: - logger.error(f"Failed to read secrets file as plain YAML: {plain_exc}") - return None - + decrypted_secrets = load_yaml_file(secrets_path) if not decrypted_secrets or not isinstance(decrypted_secrets, dict): logger.warning( f"Empty or invalid secrets file after decryption: {secrets_path}" diff --git a/osism/utils/rabbitmq.py b/osism/utils/rabbitmq.py index 0cbfe3a4..1a343b95 100644 --- a/osism/utils/rabbitmq.py +++ b/osism/utils/rabbitmq.py @@ -6,7 +6,6 @@ import subprocess from loguru import logger -import yaml from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path @@ -150,23 +149,9 @@ def load_rabbitmq_password(): return None try: - from osism.tasks.conductor.utils import get_vault + from osism.tasks.conductor.utils import load_yaml_file - vault = get_vault() - - with open(secrets_path, "rb") as f: - file_data = f.read() - - if vault.is_encrypted(file_data): - decrypted_data = vault.decrypt(file_data).decode() - logger.debug(f"Successfully decrypted secrets file: {secrets_path}") - else: - decrypted_data = file_data.decode() - logger.debug( - f"Secrets file is not encrypted (development mode): {secrets_path}" - ) - - secrets = yaml.safe_load(decrypted_data) + secrets = load_yaml_file(secrets_path) if not secrets or not isinstance(secrets, dict): logger.error("Empty or invalid secrets file") From 9d25b3e1fb910f6d7061ba7a80948c7c43be5f07 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Tue, 14 Apr 2026 12:53:23 +0200 Subject: [PATCH 2/3] Downgrade load_yaml_file log messages to debug Callers already log at their own preferred level (error or warning) when load_yaml_file returns None. Having load_yaml_file also log at error produced double logging for the same failure. Downgrade the messages in load_yaml_file to debug so that callers remain in control of the log level. This matters because not all callers treat a failure the same way: get_cloud_password has a fallback to secure.yml, so a missing secrets.yml is not an error there. An alternative would be to raise specific exceptions from load_yaml_file instead of returning None so that callers can distinguish file-not-found, decryption errors, and YAML parse errors. That would require a larger refactoring of all call sites. AI-assisted: Claude Code Signed-off-by: Roger Luethi --- osism/tasks/conductor/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osism/tasks/conductor/utils.py b/osism/tasks/conductor/utils.py index 25cee2f1..88325cef 100644 --- a/osism/tasks/conductor/utils.py +++ b/osism/tasks/conductor/utils.py @@ -94,7 +94,7 @@ def get_vault(): def load_yaml_file(path): """Load a YAML file and only request the vault secret when needed.""" if not os.path.exists(path): - logger.error(f"YAML file not found: {path}") + logger.debug(f"YAML file not found: {path}") return None try: @@ -110,10 +110,10 @@ def load_yaml_file(path): return yaml.safe_load(decrypted_data) except yaml.YAMLError as exc: - logger.error(f"Failed to parse YAML file {path}: {exc}") + logger.debug(f"Failed to parse YAML file {path}: {exc}") return None except Exception as exc: - logger.error(f"Failed to load YAML file {path}: {exc}") + logger.debug(f"Failed to load YAML file {path}: {exc}") return None From dbdb131790372522dd9f5cfbdcdddc58ccc2552a Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Tue, 14 Apr 2026 13:02:16 +0200 Subject: [PATCH 3/3] Narrow exception handling in load_yaml_file Replace the broad except Exception with specific catches for yaml.YAMLError, AnsibleError (vault decryption), and OSError (file I/O). Unexpected exceptions now propagate to the caller, which is the Pythonic convention -- all callers already have their own except Exception safety nets with context-appropriate messages. AI-assisted: Claude Code Signed-off-by: Roger Luethi --- osism/tasks/conductor/utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osism/tasks/conductor/utils.py b/osism/tasks/conductor/utils.py index 88325cef..46e34c73 100644 --- a/osism/tasks/conductor/utils.py +++ b/osism/tasks/conductor/utils.py @@ -3,6 +3,7 @@ import os from ansible import constants as ansible_constants +from ansible.errors import AnsibleError from ansible.parsing.vault import VaultLib, VaultSecret from loguru import logger @@ -112,8 +113,11 @@ def load_yaml_file(path): except yaml.YAMLError as exc: logger.debug(f"Failed to parse YAML file {path}: {exc}") return None - except Exception as exc: - logger.debug(f"Failed to load YAML file {path}: {exc}") + except AnsibleError as exc: + logger.debug(f"Failed to decrypt YAML file {path}: {exc}") + return None + except OSError as exc: + logger.debug(f"Failed to read YAML file {path}: {exc}") return None