mirror of
https://github.com/github/octodns.git
synced 2024-05-11 05:55:00 +00:00
POC of config/validation errors with context. Implemented by YAML for all it's cases
This commit is contained in:
+34
-20
@@ -200,9 +200,11 @@ class Manager(object):
|
||||
except KeyError:
|
||||
self.log.exception('Invalid provider class')
|
||||
raise ManagerException(
|
||||
f'Provider {provider_name} is missing class'
|
||||
f'Provider {provider_name} is missing class, {provider_config.context}'
|
||||
)
|
||||
_class, module, version = self._get_named_class('provider', _class)
|
||||
_class, module, version = self._get_named_class(
|
||||
'provider', _class, provider_config.context
|
||||
)
|
||||
kwargs = self._build_kwargs(provider_config)
|
||||
try:
|
||||
providers[provider_name] = _class(provider_name, **kwargs)
|
||||
@@ -215,7 +217,7 @@ class Manager(object):
|
||||
except TypeError:
|
||||
self.log.exception('Invalid provider config')
|
||||
raise ManagerException(
|
||||
'Incorrect provider config for ' + provider_name
|
||||
f'Incorrect provider config for {provider_name}, {provider_config.context}'
|
||||
)
|
||||
|
||||
return providers
|
||||
@@ -228,9 +230,11 @@ class Manager(object):
|
||||
except KeyError:
|
||||
self.log.exception('Invalid processor class')
|
||||
raise ManagerException(
|
||||
f'Processor {processor_name} is missing class'
|
||||
f'Processor {processor_name} is missing class, {processor_config.context}'
|
||||
)
|
||||
_class, module, version = self._get_named_class('processor', _class)
|
||||
_class, module, version = self._get_named_class(
|
||||
'processor', _class, processor_config.context
|
||||
)
|
||||
kwargs = self._build_kwargs(processor_config)
|
||||
try:
|
||||
processors[processor_name] = _class(processor_name, **kwargs)
|
||||
@@ -243,22 +247,24 @@ class Manager(object):
|
||||
except TypeError:
|
||||
self.log.exception('Invalid processor config')
|
||||
raise ManagerException(
|
||||
'Incorrect processor config for ' + processor_name
|
||||
f'Incorrect processor config for {processor_name}, {processor_config.context}'
|
||||
)
|
||||
return processors
|
||||
|
||||
def _config_plan_outputs(self, plan_outputs_config):
|
||||
plan_outputs = {}
|
||||
for plan_output_name, plan_output_config in plan_outputs_config.items():
|
||||
context = getattr(plan_output_config, 'context', None)
|
||||
try:
|
||||
_class = plan_output_config.pop('class')
|
||||
except KeyError:
|
||||
self.log.exception('Invalid plan_output class')
|
||||
raise ManagerException(
|
||||
f'plan_output {plan_output_name} is missing class'
|
||||
)
|
||||
msg = f'plan_output {plan_output_name} is missing class'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
raise ManagerException(msg)
|
||||
_class, module, version = self._get_named_class(
|
||||
'plan_output', _class
|
||||
'plan_output', _class, context
|
||||
)
|
||||
kwargs = self._build_kwargs(plan_output_config)
|
||||
try:
|
||||
@@ -275,9 +281,11 @@ class Manager(object):
|
||||
)
|
||||
except TypeError:
|
||||
self.log.exception('Invalid plan_output config')
|
||||
raise ManagerException(
|
||||
'Incorrect plan_output config for ' + plan_output_name
|
||||
)
|
||||
msg = f'Incorrect plan_output config for {plan_output_name}'
|
||||
if context:
|
||||
msg += f', {plan_output_config.context}'
|
||||
raise ManagerException(msg)
|
||||
|
||||
return plan_outputs
|
||||
|
||||
def _try_version(self, module_name, module=None, version=None):
|
||||
@@ -308,7 +316,7 @@ class Manager(object):
|
||||
version = self._try_version(current)
|
||||
return module, version or 'n/a'
|
||||
|
||||
def _get_named_class(self, _type, _class):
|
||||
def _get_named_class(self, _type, _class, context):
|
||||
try:
|
||||
module_name, class_name = _class.rsplit('.', 1)
|
||||
module, version = self._import_module(module_name)
|
||||
@@ -316,7 +324,10 @@ class Manager(object):
|
||||
self.log.exception(
|
||||
'_get_{}_class: Unable to import module %s', _class
|
||||
)
|
||||
raise ManagerException(f'Unknown {_type} class: {_class}')
|
||||
msg = f'Unknown {_type} class: {_class}'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
raise ManagerException(msg)
|
||||
|
||||
try:
|
||||
return getattr(module, class_name), module_name, version
|
||||
@@ -326,11 +337,14 @@ class Manager(object):
|
||||
class_name,
|
||||
module,
|
||||
)
|
||||
raise ManagerException(f'Unknown {_type} class: {_class}')
|
||||
raise ManagerException(
|
||||
f'Unknown {_type} class: {_class}, {context}'
|
||||
)
|
||||
|
||||
def _build_kwargs(self, source):
|
||||
# Build up the arguments we need to pass to the provider
|
||||
kwargs = {}
|
||||
context = getattr(source, 'context', None)
|
||||
for k, v in source.items():
|
||||
try:
|
||||
if v.startswith('env/'):
|
||||
@@ -339,10 +353,10 @@ class Manager(object):
|
||||
v = environ[env_var]
|
||||
except KeyError:
|
||||
self.log.exception('Invalid provider config')
|
||||
raise ManagerException(
|
||||
'Incorrect provider config, '
|
||||
'missing env var ' + env_var
|
||||
)
|
||||
msg = f'Incorrect provider config, missing env var {env_var}'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
raise ManagerException(msg)
|
||||
except AttributeError:
|
||||
pass
|
||||
kwargs[k] = v
|
||||
|
||||
+13
-4
@@ -46,14 +46,21 @@ class Record(EqualityTupleMixin):
|
||||
reasons.append('invalid record, whitespace is not allowed')
|
||||
|
||||
fqdn = f'{name}.{zone.name}' if name else zone.name
|
||||
context = getattr(data, 'context', None)
|
||||
try:
|
||||
_type = data['type']
|
||||
except KeyError:
|
||||
raise Exception(f'Invalid record {idna_decode(fqdn)}, missing type')
|
||||
msg = f'Invalid record {idna_decode(fqdn)}, missing type'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
raise Exception(msg)
|
||||
try:
|
||||
_class = cls._CLASSES[_type]
|
||||
except KeyError:
|
||||
raise Exception(f'Unknown record type: "{_type}"')
|
||||
msg = f'Unknown record type: "{_type}"'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
raise Exception(msg)
|
||||
reasons.extend(_class.validate(name, fqdn, data))
|
||||
try:
|
||||
lenient |= data['octodns']['lenient']
|
||||
@@ -61,9 +68,11 @@ class Record(EqualityTupleMixin):
|
||||
pass
|
||||
if reasons:
|
||||
if lenient:
|
||||
cls.log.warning(ValidationError.build_message(fqdn, reasons))
|
||||
cls.log.warning(
|
||||
ValidationError.build_message(fqdn, reasons, context)
|
||||
)
|
||||
else:
|
||||
raise ValidationError(fqdn, reasons)
|
||||
raise ValidationError(fqdn, reasons, context)
|
||||
return _class(zone, name, data, source=source)
|
||||
|
||||
@classmethod
|
||||
|
||||
@@ -11,11 +11,16 @@ class RecordException(Exception):
|
||||
|
||||
class ValidationError(RecordException):
|
||||
@classmethod
|
||||
def build_message(cls, fqdn, reasons):
|
||||
def build_message(cls, fqdn, reasons, context=None):
|
||||
reasons = '\n - '.join(reasons)
|
||||
return f'Invalid record "{idna_decode(fqdn)}"\n - {reasons}'
|
||||
msg = f'Invalid record "{idna_decode(fqdn)}"'
|
||||
if context:
|
||||
msg += f', {context}'
|
||||
msg += f'\n - {reasons}'
|
||||
return msg
|
||||
|
||||
def __init__(self, fqdn, reasons):
|
||||
super().__init__(self.build_message(fqdn, reasons))
|
||||
def __init__(self, fqdn, reasons, context=None):
|
||||
super().__init__(self.build_message(fqdn, reasons, context))
|
||||
self.fqdn = fqdn
|
||||
self.reasons = reasons
|
||||
self.context = context
|
||||
|
||||
+38
-6
@@ -10,12 +10,43 @@ from yaml.representer import SafeRepresenter
|
||||
_natsort_key = natsort_keygen()
|
||||
|
||||
|
||||
# Found http://stackoverflow.com/a/21912744 which guided me on how to hook in
|
||||
# here
|
||||
class SortEnforcingLoader(SafeLoader):
|
||||
# TODO: where should this live
|
||||
class ContextDict(dict):
|
||||
# can't assign attributes to plain dict objects and it breaks lots of stuff
|
||||
# if we put the context into the dict data itself
|
||||
|
||||
def __init__(self, *args, context=None, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
self.context = context
|
||||
|
||||
|
||||
class ContextLoader(SafeLoader):
|
||||
def _construct(self, node):
|
||||
self.flatten_mapping(node)
|
||||
ret = self.construct_pairs(node)
|
||||
|
||||
start_mark = node.start_mark
|
||||
context = f'{start_mark.name}, line {start_mark.line+1}, column {start_mark.column+1}'
|
||||
return ContextDict(ret, context=context)
|
||||
|
||||
|
||||
ContextLoader.add_constructor(
|
||||
ContextLoader.DEFAULT_MAPPING_TAG, ContextLoader._construct
|
||||
)
|
||||
|
||||
|
||||
# Found http://stackoverflow.com/a/21912744 which guided me on how to hook in
|
||||
# here
|
||||
class SortEnforcingLoader(SafeLoader):
|
||||
# TODO: inheritance
|
||||
|
||||
def _construct(self, node):
|
||||
self.flatten_mapping(node)
|
||||
ret = self.construct_pairs(node)
|
||||
|
||||
start_mark = node.start_mark
|
||||
context = f'{start_mark.name}, line {start_mark.line+1}, column {start_mark.column+1}'
|
||||
|
||||
keys = [d[0] for d in ret]
|
||||
keys_sorted = sorted(keys, key=_natsort_key)
|
||||
for key in keys:
|
||||
@@ -25,9 +56,10 @@ class SortEnforcingLoader(SafeLoader):
|
||||
None,
|
||||
None,
|
||||
'keys out of order: '
|
||||
f'expected {expected} got {key} at ' + str(node.start_mark),
|
||||
f'expected {expected} got {key} at {context}',
|
||||
)
|
||||
return dict(ret)
|
||||
|
||||
return ContextDict(ret, context=context)
|
||||
|
||||
|
||||
SortEnforcingLoader.add_constructor(
|
||||
@@ -36,7 +68,7 @@ SortEnforcingLoader.add_constructor(
|
||||
|
||||
|
||||
def safe_load(stream, enforce_order=True):
|
||||
return load(stream, SortEnforcingLoader if enforce_order else SafeLoader)
|
||||
return load(stream, SortEnforcingLoader if enforce_order else ContextLoader)
|
||||
|
||||
|
||||
class SortingDumper(SafeDumper):
|
||||
|
||||
@@ -104,13 +104,13 @@ class TestManager(TestCase):
|
||||
with self.assertRaises(ManagerException) as ctx:
|
||||
name = 'bad-plan-output-missing-class.yaml'
|
||||
Manager(get_config_filename(name)).sync()
|
||||
self.assertEqual('plan_output bad is missing class', str(ctx.exception))
|
||||
self.assertTrue('plan_output bad is missing class' in str(ctx.exception))
|
||||
|
||||
def test_bad_plan_output_config(self):
|
||||
with self.assertRaises(ManagerException) as ctx:
|
||||
Manager(get_config_filename('bad-plan-output-config.yaml')).sync()
|
||||
self.assertEqual(
|
||||
'Incorrect plan_output config for bad', str(ctx.exception)
|
||||
self.assertTrue(
|
||||
'Incorrect plan_output config for bad' in str(ctx.exception)
|
||||
)
|
||||
|
||||
def test_source_only_as_a_target(self):
|
||||
|
||||
Reference in New Issue
Block a user