From ca2c7112a133f03b9e86d1466379dbebddc4246f Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 10 Feb 2024 20:58:21 -0500 Subject: [PATCH 1/7] Secrets handlers support Add functionality that enables configurable secrets sources, with a hard-coded `env` that provides the existing environmental variable support. --- octodns/manager.py | 29 +++++++++++++------------ octodns/secret/__init__.py | 3 +++ octodns/secret/base.py | 11 ++++++++++ octodns/secret/environ.py | 32 ++++++++++++++++++++++++++++ octodns/secret/exception.py | 7 ++++++ tests/test_octodns_manager.py | 4 +++- tests/test_octodns_secret_environ.py | 31 +++++++++++++++++++++++++++ 7 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 octodns/secret/__init__.py create mode 100644 octodns/secret/base.py create mode 100644 octodns/secret/environ.py create mode 100644 octodns/secret/exception.py create mode 100644 tests/test_octodns_secret_environ.py diff --git a/octodns/manager.py b/octodns/manager.py index 7b26f83..d4d23bf 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -10,7 +10,6 @@ from importlib.metadata import PackageNotFoundError from importlib.metadata import version as module_version from json import dumps from logging import getLogger -from os import environ from sys import stdout from . import __version__ @@ -20,6 +19,7 @@ from .processor.meta import MetaProcessor from .provider.base import BaseProvider from .provider.plan import Plan from .provider.yaml import SplitYamlProvider, YamlProvider +from .secret.environ import EnvironSecrets from .yaml import safe_load from .zone import Zone @@ -119,6 +119,8 @@ class Manager(object): manager_config, enable_checksum ) + self.secret_handlers = {'env': EnvironSecrets('env')} + self.auto_arpa = self._config_auto_arpa(manager_config, auto_arpa) self.global_processors = manager_config.get('processors', []) @@ -377,22 +379,21 @@ class Manager(object): if isinstance(v, dict): v = self._build_kwargs(v) elif isinstance(v, str): - if v.startswith('env/'): - # expand env variables + if '/' in v: + handler, name = v.split('/', 1) try: - env_var = v[4:] - v = environ[env_var] + handler = self.secret_handlers[handler] except KeyError: - self.log.exception('Invalid provider config') - raise ManagerException( - f'Incorrect provider config, missing env var {env_var}, {source.context}' + # we don't have a matching handler, but don't want to + # make that an error b/c config values will often + # contain /. We don't want to print the values in case + # they're sensitive so just provide the key, and even + # that only at debug level. + self.log.debug( + '_build_kwargs: no handler found for the value of {k}' ) - try: - # try converting the value to a number to see if it - # converts - v = float(v) - except ValueError: - pass + else: + v = handler.fetch(name, source) kwargs[k] = v diff --git a/octodns/secret/__init__.py b/octodns/secret/__init__.py new file mode 100644 index 0000000..407eb4e --- /dev/null +++ b/octodns/secret/__init__.py @@ -0,0 +1,3 @@ +# +# +# diff --git a/octodns/secret/base.py b/octodns/secret/base.py new file mode 100644 index 0000000..bb7a93b --- /dev/null +++ b/octodns/secret/base.py @@ -0,0 +1,11 @@ +# +# +# + +from logging import getLogger + + +class BaseSecrets: + def __init__(self, name): + self.log = getLogger(f'{self.__class__.__name__}[{name}]') + self.name = name diff --git a/octodns/secret/environ.py b/octodns/secret/environ.py new file mode 100644 index 0000000..ddcb83c --- /dev/null +++ b/octodns/secret/environ.py @@ -0,0 +1,32 @@ +# +# +# + +from os import environ + +from .base import BaseSecrets +from .exception import SecretException + + +class EnvironSecretException(SecretException): + pass + + +class EnvironSecrets(BaseSecrets): + def fetch(self, name, source): + # expand env variables + try: + v = environ[name] + except KeyError: + self.log.exception('Invalid provider config') + raise EnvironSecretException( + f'Incorrect provider config, missing env var {name}, {source.context}' + ) + try: + # try converting the value to a number to see if it + # converts + v = float(v) + except ValueError: + pass + + return v diff --git a/octodns/secret/exception.py b/octodns/secret/exception.py new file mode 100644 index 0000000..3a64c86 --- /dev/null +++ b/octodns/secret/exception.py @@ -0,0 +1,7 @@ +# +# +# + + +class SecretException(Exception): + pass diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 5283769..008f23a 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -26,6 +26,7 @@ from octodns.manager import ( ) from octodns.processor.base import BaseProcessor from octodns.record import Create, Delete, Record, Update +from octodns.secret.environ import EnvironSecretException from octodns.yaml import safe_load from octodns.zone import Zone @@ -68,7 +69,8 @@ class TestManager(TestCase): self.assertTrue('provider config' in str(ctx.exception)) def test_missing_env_config(self): - with self.assertRaises(ManagerException) as ctx: + # details of the EnvironSecrets will be tested in dedicated tests + with self.assertRaises(EnvironSecretException) as ctx: Manager(get_config_filename('missing-provider-env.yaml')).sync() self.assertTrue('missing env var' in str(ctx.exception)) diff --git a/tests/test_octodns_secret_environ.py b/tests/test_octodns_secret_environ.py new file mode 100644 index 0000000..6b97aec --- /dev/null +++ b/tests/test_octodns_secret_environ.py @@ -0,0 +1,31 @@ +# +# +# + +from os import environ +from unittest import TestCase + +from octodns.context import ContextDict +from octodns.secret.environ import EnvironSecretException, EnvironSecrets + + +class TestEnvironSecrets(TestCase): + def test_environ_secrets(self): + # put some secrets into our env + environ['THIS_EXISTS'] = 'and has a val' + environ['THIS_IS_AN_INT'] = '42' + environ['THIS_IS_A_FLOAT'] = '43.44' + + es = EnvironSecrets('env') + + source = ContextDict({}, context='xyz') + self.assertEqual('and has a val', es.fetch('THIS_EXISTS', source)) + self.assertEqual(42, es.fetch('THIS_IS_AN_INT', source)) + self.assertEqual(43.44, es.fetch('THIS_IS_A_FLOAT', source)) + + with self.assertRaises(EnvironSecretException) as ctx: + es.fetch('DOES_NOT_EXIST', source) + self.assertEqual( + 'Incorrect provider config, missing env var DOES_NOT_EXIST, xyz', + str(ctx.exception), + ) From 60bc4193f833911c07fbac327593aeba46e48088 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 10 Feb 2024 21:45:43 -0500 Subject: [PATCH 2/7] configured secret handlers & tests of that configuration --- octodns/manager.py | 38 +++++++++++++++++++++++++++++ tests/helpers.py | 10 ++++++++ tests/test_octodns_manager.py | 45 +++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/octodns/manager.py b/octodns/manager.py index d4d23bf..dd36e9a 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -119,7 +119,13 @@ class Manager(object): manager_config, enable_checksum ) + # add our hard-coded environ handler first so that other secret + # providers can pull in env variables w/it self.secret_handlers = {'env': EnvironSecrets('env')} + secret_handlers_config = self.config.get('secret_handlers', {}) + self.secret_handlers.update( + self._config_secret_handlers(secret_handlers_config) + ) self.auto_arpa = self._config_auto_arpa(manager_config, auto_arpa) @@ -221,6 +227,38 @@ class Manager(object): self.log.info('_config_auto_arpa: auto_arpa=%s', auto_arpa) return auto_arpa + def _config_secret_handlers(self, secret_handlers_config): + self.log.debug('_config_secret_handlers: configuring secret_handlers') + secret_handlers = {} + for sh_name, sh_config in secret_handlers_config.items(): + # Get our class and remove it from the secret handler config + try: + _class = sh_config.pop('class') + except KeyError: + self.log.exception('Invalid secret handler class') + raise ManagerException( + f'Secret Handler {sh_name} is missing class, {sh_config.context}' + ) + _class, module, version = self._get_named_class( + 'secret handler', _class, sh_config.context + ) + kwargs = self._build_kwargs(sh_config) + try: + secret_handlers[sh_name] = _class(sh_name, **kwargs) + self.log.info( + '__init__: secret_handler=%s (%s %s)', + sh_name, + module, + version, + ) + except TypeError: + self.log.exception('Invalid secret handler config') + raise ManagerException( + f'Incorrect secret handler config for {sh_name}, {sh_config.context}' + ) + + return secret_handlers + def _config_providers(self, providers_config): self.log.debug('_config_providers: configuring providers') providers = {} diff --git a/tests/helpers.py b/tests/helpers.py index 019395b..5ec8761 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -9,6 +9,7 @@ from tempfile import mkdtemp from octodns.processor.base import BaseProcessor from octodns.provider.base import BaseProvider from octodns.provider.yaml import YamlProvider +from octodns.secret.base import BaseSecrets class SimpleSource(object): @@ -134,3 +135,12 @@ class CountingProcessor(BaseProcessor): def process_source_zone(self, zone, *args, **kwargs): self.count += len(zone.records) return zone + + +class DummySecrets(BaseSecrets): + def __init__(self, name, prefix): + super().__init__(name) + self.prefix = prefix + + def fetch(self, name, source): + return f'{self.prefix}{name}' diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 008f23a..20be8a6 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -17,6 +17,7 @@ from helpers import ( ) from octodns import __version__ +from octodns.context import ContextDict from octodns.idna import IdnaDict, idna_encode from octodns.manager import ( MainThreadExecutor, @@ -1204,6 +1205,50 @@ class TestManager(TestCase): ), ) + def test_config_secret_handlers(self): + # config doesn't matter here + manager = Manager(get_config_filename('simple.yaml')) + + # no config + self.assertEqual({}, manager._config_secret_handlers({})) + + # missing class + with self.assertRaises(ManagerException) as ctx: + cfg = {'secr3t': ContextDict({}, context='xyz')} + manager._config_secret_handlers(cfg) + self.assertEqual( + 'Secret Handler secr3t is missing class, xyz', str(ctx.exception) + ) + + # bad param + with self.assertRaises(ManagerException) as ctx: + cfg = { + 'secr3t': ContextDict( + { + 'class': 'octodns.secret.environ.EnvironSecrets', + 'bad': 'param', + }, + context='xyz', + ) + } + manager._config_secret_handlers(cfg) + self.assertEqual( + 'Incorrect secret handler config for secr3t, xyz', + str(ctx.exception), + ) + + # valid with a param that gets used/tested + cfg = { + 'secr3t': ContextDict( + {'class': 'helpers.DummySecrets', 'prefix': 'pre-'}, + context='xyz', + ) + } + shs = manager._config_secret_handlers(cfg) + sh = shs.get('secr3t') + self.assertTrue(sh) + self.assertEqual('pre-thing', sh.fetch('thing', None)) + class TestMainThreadExecutor(TestCase): def test_success(self): From 6d778b3b67dcf0ce359577ebbb250e251a249cfd Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 18 Feb 2024 14:25:59 -0800 Subject: [PATCH 3/7] Include tests of secret_handlers from config yaml and some of that's implementation details --- octodns/manager.py | 3 ++- tests/config/secrets.yaml | 19 +++++++++++++++++++ tests/helpers.py | 1 + tests/test_octodns_manager.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 tests/config/secrets.yaml diff --git a/octodns/manager.py b/octodns/manager.py index dd36e9a..7c84deb 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -428,7 +428,8 @@ class Manager(object): # they're sensitive so just provide the key, and even # that only at debug level. self.log.debug( - '_build_kwargs: no handler found for the value of {k}' + '_build_kwargs: failed to find handler for key "%sp ', + k, ) else: v = handler.fetch(name, source) diff --git a/tests/config/secrets.yaml b/tests/config/secrets.yaml new file mode 100644 index 0000000..d46e74f --- /dev/null +++ b/tests/config/secrets.yaml @@ -0,0 +1,19 @@ +--- +secret_handlers: + dummy: + class: helpers.DummySecrets + prefix: in_config/ + requires-env: + class: helpers.DummySecrets + # things can pull from env, it prexists + prefix: env/FROM_ENV_WILL_WORK + requires-dummy: + class: helpers.DummySecrets + # things can't pull from other handlers, the order they're configured in is + # indeterminent so it's not safe, they're also all added at once + prefix: dummy/FROM_DUMMY_WONT_WORK + +# Not needed, but required key +providers: {} +# Not needed, but required key +zones: {} diff --git a/tests/helpers.py b/tests/helpers.py index 5ec8761..b19d236 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -140,6 +140,7 @@ class CountingProcessor(BaseProcessor): class DummySecrets(BaseSecrets): def __init__(self, name, prefix): super().__init__(name) + self.log.info('__init__: name=%s, prefix=%s', name, prefix) self.prefix = prefix def fetch(self, name, source): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 20be8a6..04eb68c 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -8,6 +8,7 @@ from unittest import TestCase from unittest.mock import MagicMock, patch from helpers import ( + DummySecrets, DynamicProvider, GeoProvider, NoSshFpProvider, @@ -1249,6 +1250,37 @@ class TestManager(TestCase): self.assertTrue(sh) self.assertEqual('pre-thing', sh.fetch('thing', None)) + # test configuring secret handlers + environ['FROM_ENV_WILL_WORK'] = 'fetched_from_env/' + manager = Manager(get_config_filename('secrets.yaml')) + + # dummy was configured + self.assertTrue('dummy' in manager.secret_handlers) + dummy = manager.secret_handlers['dummy'] + self.assertIsInstance(dummy, DummySecrets) + # and has the prefix value explicitly stated in the yaml + self.assertEqual('in_config/hello', dummy.fetch('hello', None)) + + # requires-env was configured + self.assertTrue('requires-env' in manager.secret_handlers) + requires_env = manager.secret_handlers['requires-env'] + self.assertIsInstance(requires_env, DummySecrets) + # and successfully pulled a value from env as its prefix + self.assertEqual( + 'fetched_from_env/hello', requires_env.fetch('hello', None) + ) + + # requires-dummy was created + self.assertTrue('requires-dummy' in manager.secret_handlers) + requires_dummy = manager.secret_handlers['requires-dummy'] + self.assertIsInstance(requires_dummy, DummySecrets) + # but failed to fetch a secret from dummy so we just get the configured + # value as it was in the yaml for prefix + self.assertEqual( + 'dummy/FROM_DUMMY_WONT_WORK:hello', + requires_dummy.fetch(':hello', None), + ) + class TestMainThreadExecutor(TestCase): def test_success(self): From c283654e2a9d79f273e1eaddce6c7f0a55980adf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 18 Feb 2024 14:27:34 -0800 Subject: [PATCH 4/7] CHANGELOG for secret_handlers --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a36082..41f80dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * Record.octodns added to formally make that data "API" and Record._octodns is deprecated. The latter has been converted to properties that return the former and emit deprecation warnings. +* Beta support for custom secret providers added to Manager. ## v1.4.0 - 2023-12-04 - Minor Meta From 7f1a68a9db666df04ed87adb697fd59cebccee31 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 28 Feb 2024 13:12:05 -0800 Subject: [PATCH 5/7] Fix/merge CHANGELOG.md --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4ed48a..9e9fa3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## v1.6.x - 2024-??-?? - ??? + +* Beta support for custom secret providers added to Manager. + ## v1.5.0 - 2024-02-26 - Checksums, nested expansion, & flexable values * Beta support for Manager.enable_checksum and octodns-sync --checksum Allows a @@ -17,7 +21,6 @@ * Record.octodns added to formally make that data "API" and Record._octodns is deprecated. The latter has been converted to properties that return the former and emit deprecation warnings. -* Beta support for custom secret providers added to Manager. ## v1.4.0 - 2023-12-04 - Minor Meta From b539cb0a4f6c76a0475b937a68460ef262ba67ae Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 11 Mar 2024 14:51:06 -0700 Subject: [PATCH 6/7] more consistent naming, pural secrets --- octodns/secret/environ.py | 6 +++--- octodns/secret/exception.py | 2 +- tests/test_octodns_manager.py | 4 ++-- tests/test_octodns_secret_environ.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/octodns/secret/environ.py b/octodns/secret/environ.py index ddcb83c..22c4b9d 100644 --- a/octodns/secret/environ.py +++ b/octodns/secret/environ.py @@ -5,10 +5,10 @@ from os import environ from .base import BaseSecrets -from .exception import SecretException +from .exception import SecretsException -class EnvironSecretException(SecretException): +class EnvironSecretsException(SecretsException): pass @@ -19,7 +19,7 @@ class EnvironSecrets(BaseSecrets): v = environ[name] except KeyError: self.log.exception('Invalid provider config') - raise EnvironSecretException( + raise EnvironSecretsException( f'Incorrect provider config, missing env var {name}, {source.context}' ) try: diff --git a/octodns/secret/exception.py b/octodns/secret/exception.py index 3a64c86..53077bc 100644 --- a/octodns/secret/exception.py +++ b/octodns/secret/exception.py @@ -3,5 +3,5 @@ # -class SecretException(Exception): +class SecretsException(Exception): pass diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 53b0ec7..05d20aa 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -28,7 +28,7 @@ from octodns.manager import ( ) from octodns.processor.base import BaseProcessor from octodns.record import Create, Delete, Record, Update -from octodns.secret.environ import EnvironSecretException +from octodns.secret.environ import EnvironSecretsException from octodns.yaml import safe_load from octodns.zone import Zone @@ -72,7 +72,7 @@ class TestManager(TestCase): def test_missing_env_config(self): # details of the EnvironSecrets will be tested in dedicated tests - with self.assertRaises(EnvironSecretException) as ctx: + with self.assertRaises(EnvironSecretsException) as ctx: Manager(get_config_filename('missing-provider-env.yaml')).sync() self.assertTrue('missing env var' in str(ctx.exception)) diff --git a/tests/test_octodns_secret_environ.py b/tests/test_octodns_secret_environ.py index 6b97aec..ff09c32 100644 --- a/tests/test_octodns_secret_environ.py +++ b/tests/test_octodns_secret_environ.py @@ -6,7 +6,7 @@ from os import environ from unittest import TestCase from octodns.context import ContextDict -from octodns.secret.environ import EnvironSecretException, EnvironSecrets +from octodns.secret.environ import EnvironSecrets, EnvironSecretsException class TestEnvironSecrets(TestCase): @@ -23,7 +23,7 @@ class TestEnvironSecrets(TestCase): self.assertEqual(42, es.fetch('THIS_IS_AN_INT', source)) self.assertEqual(43.44, es.fetch('THIS_IS_A_FLOAT', source)) - with self.assertRaises(EnvironSecretException) as ctx: + with self.assertRaises(EnvironSecretsException) as ctx: es.fetch('DOES_NOT_EXIST', source) self.assertEqual( 'Incorrect provider config, missing env var DOES_NOT_EXIST, xyz', From afee52b181e7bf63371355bbab01ddd88a2e9934 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 11 Mar 2024 14:53:06 -0700 Subject: [PATCH 7/7] Fix method name in exception log --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 524b397..903f797 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -402,7 +402,7 @@ class Manager(object): return getattr(module, class_name), module_name, version except AttributeError: self.log.exception( - '_get_{}_class: Unable to get class %s from module %s', + '_get_named_class: Unable to get class %s from module %s', class_name, module, )