From 91b14dca40faf53d5c529145d35547fb7ee18943 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Thu, 11 Feb 2016 22:06:01 -0800 Subject: [PATCH] fixing the beacon module and state module to handle passing enabled properly. Also reworking how what is returned from the validating functions is handled to ensure when beacon configurations aren't validate the results indicate exactly why. --- salt/beacons/btmp.py | 4 +++- salt/beacons/diskusage.py | 6 +++--- salt/beacons/inotify.py | 37 +++++++++++++++------------------- salt/beacons/journald.py | 7 +++---- salt/beacons/load.py | 29 +++++++++++--------------- salt/beacons/network_info.py | 15 ++++++-------- salt/beacons/proxy_example.py | 5 ++--- salt/beacons/ps.py | 5 ++--- salt/beacons/service.py | 5 ++--- salt/beacons/sh.py | 5 ++--- salt/beacons/twilio_txt_msg.py | 6 +++--- salt/beacons/wtmp.py | 5 ++--- salt/modules/beacons.py | 18 +++++++++++++---- salt/states/beacon.py | 10 +++++++-- 14 files changed, 78 insertions(+), 79 deletions(-) diff --git a/salt/beacons/btmp.py b/salt/beacons/btmp.py index 41dd735ef5..135a29338e 100644 --- a/salt/beacons/btmp.py +++ b/salt/beacons/btmp.py @@ -56,8 +56,10 @@ def validate(config): ''' # Configuration for load beacon should be a list of dicts if not isinstance(config, dict): + return False, ('Configuration for btmp beacon must ' + 'be a list of dictionaries.') return False - return True + return True, 'Valid beacon configuration' # TODO: add support for only firing events for specific users and login times diff --git a/salt/beacons/diskusage.py b/salt/beacons/diskusage.py index 225e58f824..9c874985c2 100644 --- a/salt/beacons/diskusage.py +++ b/salt/beacons/diskusage.py @@ -42,9 +42,9 @@ def validate(config): ''' # Configuration for diskusage beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for diskusage beacon must be a dictionary.') - return False - return True + return False, ('Configuration for diskusage beacon ' + 'must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/inotify.py b/salt/beacons/inotify.py index e3f3308e63..f9e99ef90f 100644 --- a/salt/beacons/inotify.py +++ b/salt/beacons/inotify.py @@ -95,44 +95,39 @@ def validate(config): ] # Configuration for diskusage beacon should be a list of dicts + log.debug('config {0}'.format(config)) if not isinstance(config, dict): - log.info('Configuration for inotify beacon must be a dictionary.') - return False + return False, 'Configuration for inotify beacon must be a dictionary.' else: for config_item in config: if not isinstance(config[config_item], dict): - log.info('Configuration for inotify beacon must ' - 'be a dictionary of dictionaries.') - return False + return False, ('Configuration for inotify beacon must ' + 'be a dictionary of dictionaries.') else: if not any(j in ['mask', 'recurse', 'auto_add'] for j in config[config_item]): - log.info('Configuration for inotify beacon must ' - 'contain mask, recurse or auto_add items.') - return False + return False, ('Configuration for inotify beacon ' + 'must contain mask, recurse or auto_add items.') if 'auto_add' in config[config_item]: if not isinstance(config[config_item]['auto_add'], bool): - log.info('Configuration for inotify beacon ' - 'auto_add must be boolean.') - return False + return False, ('Configuration for inotify beacon ' + 'auto_add must be boolean.') if 'recurse' in config[config_item]: if not isinstance(config[config_item]['recurse'], bool): - log.info('Configuration for inotify beacon ' - 'recurse must be boolean.') - return False + return False, ('Configuration for inotify beacon ' + ' recurse must be boolean.') if 'mask' in config[config_item]: if not isinstance(config[config_item]['mask'], list): - log.info('Configuration for inotify beacon ' - 'mask must be list.') - return False + return False, ('Configuration for inotify beacon ' + ' mask must be list.') + for mask in config[config_item]['mask']: if mask not in VALID_MASK: - log.info('Configuration for inotify beacon ' - 'invalid mask option {0}.'.format(mask)) - return False - return True + return False, ('Configuration for inotify beacon ' + 'invalid mask option {0}.'.format(mask)) + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/journald.py b/salt/beacons/journald.py index f25fd6271a..7021d64cd5 100644 --- a/salt/beacons/journald.py +++ b/salt/beacons/journald.py @@ -54,10 +54,9 @@ def validate(config): else: for item in config: if not isinstance(config[item], dict): - log.info('Configuration for journald beacon must ' - 'be a dictionary of dictionaries.') - return False - return True + return False, ('Configuration for journald beacon must ' + 'be a dictionary of dictionaries.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/load.py b/salt/beacons/load.py index cd05ce2a61..d0eba79e19 100644 --- a/salt/beacons/load.py +++ b/salt/beacons/load.py @@ -33,33 +33,28 @@ def validate(config): # Configuration for load beacon should be a list of dicts if not isinstance(config, list): - log.info('Configuration for load beacon must be a list.') - return False + return False, ('Configuration for load beacon must be a list.') else: for config_item in config: if not isinstance(config_item, dict): - log.info('Configuration for load beacon must ' - 'be a list of dictionaries.') - return False + return False, ('Configuration for load beacon must ' + 'be a list of dictionaries.') else: if not any(j in ['1m', '5m', '15m'] for j in config_item.keys()): - log.info('Configuration for load beacon must ' - 'contain 1m, 5m and 15m items.') - return False + return False, ('Configuration for load beacon must ' + 'contain 1m, 5m and 15m items.') for item in config_item: if not isinstance(config_item[item], list): - log.info('Configuration for load beacon: ' - '1m, 5m and 15m items must be ' - 'a list of two items.') - return False + return False, ('Configuration for load beacon: ' + '1m, 5m and 15m items must be ' + 'a list of two items.') else: if len(config_item[item]) != 2: - log.info('Configuration for load beacon: ' - '1m, 5m and 15m items must be ' - 'a list of two items.') - return False - return True + return False, ('Configuration for load beacon: ' + '1m, 5m and 15m items must be ' + 'a list of two items.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/network_info.py b/salt/beacons/network_info.py index b939645544..22cddb7151 100644 --- a/salt/beacons/network_info.py +++ b/salt/beacons/network_info.py @@ -48,20 +48,17 @@ def validate(config): # Configuration for load beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for load beacon must be a dictionary.') - return False + return False, ('Configuration for load beacon must be a dictionary.') else: for item in config: if not isinstance(config[item], dict): - log.info('Configuration for load beacon must ' - 'be a dictionary of dictionaries.') - return False + return False, ('Configuration for load beacon must ' + 'be a dictionary of dictionaries.') else: if not any(j in VALID_ITEMS for j in config[item]): - log.info('Invalid configuration item in ' - 'Beacon configuration.') - return False - return True + return False, ('Invalid configuration item in ' + 'Beacon configuration.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/proxy_example.py b/salt/beacons/proxy_example.py index 8c3550f3f9..00eda2b2a9 100644 --- a/salt/beacons/proxy_example.py +++ b/salt/beacons/proxy_example.py @@ -38,9 +38,8 @@ def validate(config): Validate the beacon configuration ''' if not isinstance(config, dict): - log.info('Configuration for rest_example beacon must be a dictionary.') - return False - return True + return False, ('Configuration for rest_example beacon must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/ps.py b/salt/beacons/ps.py index 2361cafad4..50a55d56be 100644 --- a/salt/beacons/ps.py +++ b/salt/beacons/ps.py @@ -18,9 +18,8 @@ def validate(config): ''' # Configuration for ps beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for ps beacon must be a dictionary.') - return False - return True + return False, ('Configuration for ps beacon must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/service.py b/salt/beacons/service.py index 3b3903833d..70cf5e0e11 100644 --- a/salt/beacons/service.py +++ b/salt/beacons/service.py @@ -20,9 +20,8 @@ def validate(config): ''' # Configuration for service beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for service beacon must be a dictionary.') - return False - return True + return False, ('Configuration for service beacon must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/sh.py b/salt/beacons/sh.py index 9736a862bd..06128ba3b4 100644 --- a/salt/beacons/sh.py +++ b/salt/beacons/sh.py @@ -47,9 +47,8 @@ def validate(config): ''' # Configuration for sh beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for sh beacon must be a dictionary.') - return False - return True + return False, ('Configuration for sh beacon must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/twilio_txt_msg.py b/salt/beacons/twilio_txt_msg.py index 6688465905..b4b06eaa37 100644 --- a/salt/beacons/twilio_txt_msg.py +++ b/salt/beacons/twilio_txt_msg.py @@ -32,9 +32,9 @@ def validate(config): ''' # Configuration for twilio_txt_msg beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for twilio_txt_msg beacon must be a dictionary.') - return False - return True + return False, ('Configuration for twilio_txt_msg beacon ' + 'must be a dictionary.') + return True, 'Valid beacon configuration' def beacon(config): diff --git a/salt/beacons/wtmp.py b/salt/beacons/wtmp.py index 919bc923ed..0e899f7980 100644 --- a/salt/beacons/wtmp.py +++ b/salt/beacons/wtmp.py @@ -61,9 +61,8 @@ def validate(config): ''' # Configuration for wtmp beacon should be a list of dicts if not isinstance(config, dict): - log.info('Configuration for wtmp beacon must be a dictionary.') - return False - return True + return False, ('Configuration for wtmp beacon must be a dictionary.') + return True, 'Valid beacon configuration' # TODO: add support for only firing events for specific users and login times diff --git a/salt/modules/beacons.py b/salt/modules/beacons.py index 190f98437c..a4299d98e9 100644 --- a/salt/modules/beacons.py +++ b/salt/modules/beacons.py @@ -100,14 +100,19 @@ def add(name, beacon_data, **kwargs): # Attempt to validate if hasattr(beacon_module, 'validate'): - valid = beacon_module.validate(beacon_data) + _beacon_data = beacon_data + if 'enabled' in _beacon_data: + del _beacon_data['enabled'] + valid, vcomment = beacon_module.validate(_beacon_data) else: log.info('Beacon {0} does not have a validate' ' function, skipping validation.'.format(name)) valid = True if not valid: - ret['comment'] = 'Beacon {0} configuration invalid, not adding.'.format(name) + ret['result'] = False + ret['comment'] = ('Beacon {0} configuration invalid, ' + 'not adding.\n{1}'.format(name, vcomment)) return ret try: @@ -164,14 +169,19 @@ def modify(name, beacon_data, **kwargs): # Attempt to validate if hasattr(beacon_module, 'validate'): - valid = beacon_module.validate(beacon_data) + _beacon_data = beacon_data + if 'enabled' in _beacon_data: + del _beacon_data['enabled'] + valid, vcomment = beacon_module.validate(_beacon_data) else: log.info('Beacon {0} does not have a validate' ' function, skipping validation.'.format(name)) valid = True if not valid: - ret['comment'] = 'Beacon {0} configuration invalid, not modifying.'.format(name) + ret['result'] = False + ret['comment'] = ('Beacon {0} configuration invalid, ' + 'not adding.\n{1}'.format(name, vcomment)) return ret _current = current_beacons[name] diff --git a/salt/states/beacon.py b/salt/states/beacon.py index 2cf1e27818..36111b2b35 100644 --- a/salt/states/beacon.py +++ b/salt/states/beacon.py @@ -30,6 +30,9 @@ Management of the Salt beacons ''' +import logging +log = logging.getLogger(__name__) + def present(name, **kwargs): @@ -66,8 +69,11 @@ def present(name, ret['comment'] = result['comment'] return ret else: - ret['comment'].append('Modifying {0} in beacons'.format(name)) - ret['changes'] = result['changes'] + if 'changes' in result: + ret['comment'].append('Modifying {0} in beacons'.format(name)) + ret['changes'] = result['changes'] + else: + ret['comment'].append(result['comment']) else: if 'test' in __opts__ and __opts__['test']: kwargs['test'] = True