commit 7a54f2cb0803fc53c69d456d6d39463b751d9666
parent 2133cdf423de8207504ccdceb6b95610397d739c
Author: Asher Morgan <asher@ashermorgan.net>
Date: Sat, 12 Sep 2026 14:56:09 -0700
Improve error handling
Diffstat:
4 files changed, 81 insertions(+), 39 deletions(-)
diff --git a/aoh/config.py b/aoh/config.py
@@ -1,5 +1,6 @@
import logging
import os
+import sys
from urllib.parse import urlsplit
from dotenv import load_dotenv
@@ -15,22 +16,22 @@ AOH_LOG_LEVEL = os.getenv('AOH_LOG_LEVEL', 'ERROR')
try:
AOH_MAX_RUNNERS = int(os.getenv('AOH_MAX_RUNNERS', '0'))
except ValueError:
- logger.warning('$AOH_MAX_RUNNERS must be a non-negative integer')
- AOH_MAX_RUNNERS = 0
+ logger.critical('$AOH_MAX_RUNNERS must be a non-negative integer')
+ sys.exit(1)
if AOH_MAX_RUNNERS < 0:
- logger.warning('$AOH_MAX_RUNNERS must be a non-negative integer')
- AOH_MAX_RUNNERS = 0
+ logger.critical('$AOH_MAX_RUNNERS must be a non-negative integer')
+ sys.exit(1)
AOH_ORIGIN = os.getenv('AOH_ORIGIN')
if AOH_ORIGIN is not None:
try:
parts = urlsplit(AOH_ORIGIN)
if parts.scheme == '' or parts.path != '':
- logger.warning('$AOH_ORIGIN is not a valid origin')
- AOH_ORIGIN = None
+ logger.critical('$AOH_ORIGIN is not a valid origin')
+ sys.exit(1)
except ValueError:
- logger.warning('$AOH_ORIGIN is not a valid origin')
- AOH_ORIGIN = None
+ logger.critical('$AOH_ORIGIN is not a valid origin')
+ sys.exit(1)
AOH_PLAYBOOKS_FILE = os.getenv('AOH_PLAYBOOKS_FILE', 'playbooks.yml')
if not os.path.exists(AOH_PLAYBOOKS_FILE):
diff --git a/aoh/playbook.py b/aoh/playbook.py
@@ -8,7 +8,7 @@ from aoh.security import CLI_OPT_BLACKLIST
_PLAYBOOKS_DIR = os.path.dirname(AOH_PLAYBOOKS_FILE)
-class PlaybookError(Exception):
+class PlaybookFileError(Exception):
"""Raised for AoH playbook configuration errors."""
@@ -34,11 +34,12 @@ class Playbook:
for key, _type in _types.items():
if key in dict:
if type(dict[key]) != _type:
- raise PlaybookError(f'"{key}" field must be of type '
- f'{_type}')
+ raise PlaybookFileError(f'"{key}" field must be of type '
+ f'{_type}')
if _type is list and any(type(x) != str for x in dict[key]):
- raise PlaybookError(f'"{key}" elements must be of type str')
+ raise PlaybookFileError(f'"{key}" elements must be of type '
+ 'str')
self.name = name
self.path = dict.get('path', name)
@@ -55,32 +56,41 @@ class Playbook:
self.web_description = dict.get('web_description')
self.path = os.path.abspath(os.path.join(_PLAYBOOKS_DIR, self.path))
- self.config = os.path.abspath(os.path.join(_PLAYBOOKS_DIR, self.config))
+ if self.config:
+ self.config = os.path.abspath(os.path.join(_PLAYBOOKS_DIR,
+ self.config))
if self.limit:
if '--limit' in self.allow_opts or '-l' in self.allow_opts:
- raise PlaybookError('allow_opts cannot contain "--limit" if '
- 'limit is true')
+ raise PlaybookFileError('allow_opts cannot contain "--limit" '
+ 'if limit is true')
if '--limit' in self.extra_args or '-l' in self.extra_args:
- raise PlaybookError('extra_args cannot contain "--limit" if '
- 'limit is true')
+ raise PlaybookFileError('extra_args cannot contain "--limit" '
+ 'if limit is true')
for opt in CLI_OPT_BLACKLIST:
if opt in self.allow_opts:
- raise PlaybookError(f'The "{opt}" option is not supported and '
- 'cannot be included in allow_opts')
+ raise PlaybookFileError(f'The "{opt}" option is not supported '
+ 'and cannot be included in allow_opts')
if opt in self.extra_args:
- raise PlaybookError(f'The "{opt}" option is not supported and '
- 'cannot be included in extra_args')
+ raise PlaybookFileError(f'The "{opt}" option is not supported '
+ 'and cannot be included in extra_args')
def get_playbook(name):
"""Lookup a playbook configuration."""
with open(AOH_PLAYBOOKS_FILE, 'r') as f:
- CONFIG = yaml.safe_load(f)
- if name in CONFIG:
- return Playbook(name, CONFIG[name])
+ try:
+ PLAYBOOKS = yaml.safe_load(f)
+ except yaml.YAMLError:
+ raise PlaybookFileError('Failed to parse $AOH_PLAYBOOKS_FILE')
+
+ if type(PLAYBOOKS) != dict:
+ raise PlaybookFileError('$AOH_PASSWORDS_FILE must contain a '
+ 'dictionary of playbook entries')
+ if name in PLAYBOOKS:
+ return Playbook(name, PLAYBOOKS[name] or {})
else:
return None
diff --git a/aoh/security.py b/aoh/security.py
@@ -42,6 +42,10 @@ _CONFIG_PASSWORD_OPTS = {
}
+class PasswordFileError(Exception):
+ """Raised for AoH password configuration errors."""
+
+
def _get_opts(args):
"""Identify options present in a list of CLI arguments."""
@@ -106,5 +110,21 @@ def validate_aoh_password(playbook, password):
return False
with open(AOH_PASSWORDS_FILE, 'r') as f:
- hash = yaml.safe_load(f).get(playbook.name)
- return bcrypt.checkpw(password.encode(), hash.encode())
+ try:
+ PASSWORDS = yaml.safe_load(f)
+ except yaml.YAMLError:
+ raise PasswordFileError('Failed to parse $AOH_PASSWORDS_FILE')
+
+ if type(PASSWORDS) != dict:
+ raise PasswordFileError('$AOH_PASSWORDS_FILE must contain a '
+ 'dictionary of password hashes')
+ hash = PASSWORDS.get(playbook.name)
+ if hash is None:
+ raise PasswordFileError('Password hash not found')
+ if type(hash) != str:
+ raise PasswordFileError('Password hashes must be of type str')
+
+ try:
+ return bcrypt.checkpw(password.encode(), hash.encode())
+ except ValueError:
+ raise PasswordFileError('Invalid bcrypt hash')
diff --git a/aoh/server.py b/aoh/server.py
@@ -5,9 +5,10 @@ from flask import Flask, abort, render_template, request, session
from flask_apscheduler import APScheduler
from aoh.config import AOH_MAX_RUNNERS, AOH_ORIGIN
-from aoh.playbook import PlaybookError, get_playbook
+from aoh.playbook import PlaybookFileError, get_playbook
from aoh.runner import Runner, RunnerError
from aoh.security import (
+ PasswordFileError,
get_required_passwords,
validate_aoh_password,
validate_args,
@@ -36,7 +37,7 @@ def gc():
for id, runner in runners:
if runner and runner.has_timed_out(_GC_THRESHOLD):
logger.info(f'{runner.playbook.name} (#{id[:8]}): Runner torn down '
- 'due to inactivity.')
+ 'due to inactivity')
runner.teardown()
_RUNNERS.pop(id, None)
@@ -48,10 +49,20 @@ def inject_stage_and_region():
}
+@app.errorhandler(PasswordFileError)
+@app.errorhandler(PlaybookFileError)
@app.errorhandler(RunnerError)
-@app.errorhandler(PlaybookError)
def error(e):
- logger.error(e)
+ path = (request.view_args or {}).get('path', '')
+ short_id = (request.view_args or {}).get('short_id')
+ if path and short_id:
+ prefix = f'{path} (#{short_id}): '
+ elif path:
+ prefix = f'{path}: '
+ else:
+ prefix = ''
+
+ logger.error(prefix + str(e))
return {'msg': 'Internal server error'}, 500
@@ -68,8 +79,8 @@ def new_runner(path):
return {'err': f'Playbook not found: {path}'}, 400
if AOH_MAX_RUNNERS != 0 and len(_RUNNERS) >= AOH_MAX_RUNNERS:
- logger.info(f'{path}: No runners available.')
- return { 'err': 'No runners available.' }, 503
+ logger.info(f'{path}: No runners available')
+ return {'err': 'No runners available'}, 503
exp_pw_types = get_required_passwords(playbook, request.json['args'])
act_pws = request.json.get('passwords', {})
@@ -82,13 +93,13 @@ def new_runner(path):
}, 401
if not validate_aoh_password(playbook, act_pws.get('aoh_password')):
- logger.info(f'{path}: Received incorrect password.')
- return {'err': 'Incorrect AoH password.'}, 401
+ logger.info(f'{path}: Received incorrect password')
+ return {'err': 'Incorrect AoH password'}, 401
act_pws.pop('aoh_password', None) # Don't pass AoH password on to runner
if not validate_args(playbook, request.json['args']):
logger.info(f"{path}: Received bad arguments: {request.json['args']}")
- return {'err': 'Bad or banned arguments passed.'}, 400
+ return {'err': 'Bad or banned arguments passed'}, 400
runner = Runner(playbook, request.json['host'], request.json['args'],
act_pws)
@@ -98,7 +109,7 @@ def new_runner(path):
session['runner'] = runner.id
short_id = runner.id[:8]
- logger.info(f'{path} (#{short_id}): Created runner.')
+ logger.info(f'{path} (#{short_id}): Created runner')
return {}, 201, {'Location': f'/runners/{path}/{short_id}'}
@@ -107,7 +118,7 @@ def new_runner(path):
def runner_update(path, short_id):
id = session.get('runner')
if not id or not id.startswith(short_id):
- return {'err': 'Invalid runner ID.'}, 404
+ return {'err': 'Invalid runner ID'}, 404
runner = _RUNNERS.get(id)
if not runner:
@@ -116,12 +127,12 @@ def runner_update(path, short_id):
if runner.playbook.name != path:
return {'err': f'Playbook not found: {path}'}, 400
- logger.debug(f'{path} (#{short_id}): Received client message.')
+ logger.debug(f'{path} (#{short_id}): Received client message')
res = runner.process_message(request.json)
if res['finished']:
logger.info(f'{runner.playbook.name} (#{short_id}): Runner torn down '
- 'due to completion.')
+ 'due to completion')
runner.teardown()
_RUNNERS.pop(id, None)
