From 943150ee512e8882408bd37c6a6cc3e235368d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mois=C3=A9s=20Guimar=C3=A3es=20de=20Medeiros?= Date: Thu, 3 Jan 2019 10:22:21 +0100 Subject: [PATCH] Implements KeyManager's option discovery. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The KeyManager itself should be responsible for advertising the correct set of options for discovery, not relying on the global option listing method to know which variable holds the options and how are they grouped. Change-Id: I1764c383206df835b7d654f2f776663bd6d4d25b Signed-off-by: Moisés Guimarães de Medeiros --- castellan/key_manager/barbican_key_manager.py | 17 ++++--- castellan/key_manager/key_manager.py | 13 +++++ castellan/key_manager/vault_key_manager.py | 27 ++++++----- castellan/options.py | 47 ++++++++++++------- castellan/tests/unit/test_options.py | 14 +++--- ...ger-option-discovery-13a46c1dfc036a3f.yaml | 15 ++++++ 6 files changed, 89 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/implements-keymanager-option-discovery-13a46c1dfc036a3f.yaml diff --git a/castellan/key_manager/barbican_key_manager.py b/castellan/key_manager/barbican_key_manager.py index 324dc034..8a53e3e5 100644 --- a/castellan/key_manager/barbican_key_manager.py +++ b/castellan/key_manager/barbican_key_manager.py @@ -47,7 +47,7 @@ from oslo_utils import timeutils from six.moves import urllib -barbican_opts = [ +_barbican_opts = [ cfg.StrOpt('barbican_endpoint', help='Use this endpoint to connect to Barbican, for example: ' '"http://localhost:9311/"'), @@ -78,7 +78,7 @@ barbican_opts = [ ] -BARBICAN_OPT_GROUP = 'barbican' +_BARBICAN_OPT_GROUP = 'barbican' LOG = logging.getLogger(__name__) @@ -98,8 +98,8 @@ class BarbicanKeyManager(key_manager.KeyManager): self._barbican_client = None self._base_url = None self.conf = configuration - self.conf.register_opts(barbican_opts, group=BARBICAN_OPT_GROUP) - loading.register_session_conf_options(self.conf, BARBICAN_OPT_GROUP) + self.conf.register_opts(_barbican_opts, group=_BARBICAN_OPT_GROUP) + loading.register_session_conf_options(self.conf, _BARBICAN_OPT_GROUP) def _get_barbican_client(self, context): """Creates a client to connect to the Barbican service. @@ -634,7 +634,7 @@ class BarbicanKeyManager(key_manager.KeyManager): except (barbican_exceptions.HTTPAuthError, barbican_exceptions.HTTPClientError, barbican_exceptions.HTTPServerError) as e: - LOG.error(_("Error listing objects: %s"), e) + LOG.error("Error listing objects: %s", e) raise exception.KeyManagerError(reason=e) for secret in secrets: @@ -644,7 +644,10 @@ class BarbicanKeyManager(key_manager.KeyManager): except (barbican_exceptions.HTTPAuthError, barbican_exceptions.HTTPClientError, barbican_exceptions.HTTPServerError) as e: - LOG.warning(_("Error occurred while retrieving object " - "metadata, not adding it to the list: %s"), e) + LOG.warning("Error occurred while retrieving object " + "metadata, not adding it to the list: %s", e) return objects + + def list_options_for_discovery(self): + return [(_BARBICAN_OPT_GROUP, _barbican_opts)] diff --git a/castellan/key_manager/key_manager.py b/castellan/key_manager/key_manager.py index 07b88294..7e3040ca 100644 --- a/castellan/key_manager/key_manager.py +++ b/castellan/key_manager/key_manager.py @@ -123,3 +123,16 @@ class KeyManager(object): found, an empty list should be returned instead. """ return [] + + def list_options_for_discovery(self): + """Lists the KeyManager's configure options. + + KeyManagers should advertise all supported options through this + method for the purpose of sample generation by oslo-config-generator. + Each item in the advertised list should be tuple composed by the group + name and a list of options belonging to that group. None should be used + as the group name for the DEFAULT group. + + :returns: the list of supported options of a KeyManager. + """ + return [] diff --git a/castellan/key_manager/vault_key_manager.py b/castellan/key_manager/vault_key_manager.py index ec198258..82e59d0b 100644 --- a/castellan/key_manager/vault_key_manager.py +++ b/castellan/key_manager/vault_key_manager.py @@ -43,10 +43,10 @@ from castellan.common.objects import x_509 from castellan.i18n import _ from castellan.key_manager import key_manager -DEFAULT_VAULT_URL = "http://127.0.0.1:8200" -DEFAULT_MOUNTPOINT = "secret" +_DEFAULT_VAULT_URL = "http://127.0.0.1:8200" +_DEFAULT_MOUNTPOINT = "secret" -vault_opts = [ +_vault_opts = [ cfg.StrOpt('root_token_id', help='root token for vault'), cfg.StrOpt('approle_role_id', @@ -54,13 +54,13 @@ vault_opts = [ cfg.StrOpt('approle_secret_id', help='AppRole secret_id for authentication with vault'), cfg.StrOpt('kv_mountpoint', - default=DEFAULT_MOUNTPOINT, + default=_DEFAULT_MOUNTPOINT, help='Mountpoint of KV store in Vault to use, for example: ' - '{}'.format(DEFAULT_MOUNTPOINT)), + '{}'.format(_DEFAULT_MOUNTPOINT)), cfg.StrOpt('vault_url', - default=DEFAULT_VAULT_URL, + default=_DEFAULT_VAULT_URL, help='Use this endpoint to connect to Vault, for example: ' - '"%s"' % DEFAULT_VAULT_URL), + '"%s"' % _DEFAULT_VAULT_URL), cfg.StrOpt('ssl_ca_crt_file', help='Absolute path to ca cert file'), cfg.BoolOpt('use_ssl', @@ -68,7 +68,7 @@ vault_opts = [ help=_('SSL Enabled/Disabled')), ] -VAULT_OPT_GROUP = 'vault' +_VAULT_OPT_GROUP = 'vault' _EXCEPTIONS_BY_CODE = [ requests.codes['internal_server_error'], @@ -94,8 +94,8 @@ class VaultKeyManager(key_manager.KeyManager): def __init__(self, configuration): self._conf = configuration - self._conf.register_opts(vault_opts, group=VAULT_OPT_GROUP) - loading.register_session_conf_options(self._conf, VAULT_OPT_GROUP) + self._conf.register_opts(_vault_opts, group=_VAULT_OPT_GROUP) + loading.register_session_conf_options(self._conf, _VAULT_OPT_GROUP) self._session = requests.Session() self._root_token_id = self._conf.vault.root_token_id self._approle_role_id = self._conf.vault.approle_role_id @@ -412,7 +412,10 @@ class VaultKeyManager(key_manager.KeyManager): if object_type is None or isinstance(obj, object_type): objects.append(obj) except exception.ManagedObjectNotFoundError as e: - LOG.warning(_("Error occurred while retrieving object " - "metadata, not adding it to the list: %s"), e) + LOG.warning("Error occurred while retrieving object " + "metadata, not adding it to the list: %s", e) pass return objects + + def list_options_for_discovery(self): + return [(_VAULT_OPT_GROUP, _vault_opts)] diff --git a/castellan/options.py b/castellan/options.py index 2caed9a1..64dd106f 100644 --- a/castellan/options.py +++ b/castellan/options.py @@ -12,10 +12,12 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from stevedore import ExtensionManager + from oslo_config import cfg from oslo_log import log -from castellan import key_manager as km +from castellan import key_manager try: from castellan.key_manager import barbican_key_manager as bkm except ImportError: @@ -66,11 +68,16 @@ def set_defaults(conf, backend=None, barbican_endpoint=None, :param barbican_endpoint_type: Use this to specify the type of URL. : Valid values are: public, internal or admin. """ - conf.register_opts(km.key_manager_opts, group='key_manager') - if bkm: - conf.register_opts(bkm.barbican_opts, group=bkm.BARBICAN_OPT_GROUP) - if vkm: - conf.register_opts(vkm.vault_opts, group=vkm.VAULT_OPT_GROUP) + conf.register_opts(key_manager.key_manager_opts, group='key_manager') + + ext_mgr = ExtensionManager( + "castellan.drivers", + invoke_on_load=True, + invoke_args=[cfg.CONF]) + + for km in ext_mgr.names(): + for group, opts in ext_mgr[km].obj.list_options_for_discovery(): + conf.register_opts(opts, group=group) # Use the new backend option if set or fall back to the older api_class default_backend = backend or api_class @@ -80,25 +87,25 @@ def set_defaults(conf, backend=None, barbican_endpoint=None, if bkm is not None: if barbican_endpoint is not None: conf.set_default('barbican_endpoint', barbican_endpoint, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if barbican_api_version is not None: conf.set_default('barbican_api_version', barbican_api_version, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if auth_endpoint is not None: conf.set_default('auth_endpoint', auth_endpoint, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if retry_delay is not None: conf.set_default('retry_delay', retry_delay, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if number_of_retries is not None: conf.set_default('number_of_retries', number_of_retries, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if verify_ssl is not None: conf.set_default('verify_ssl', verify_ssl, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if barbican_endpoint_type is not None: conf.set_default('barbican_endpoint_type', barbican_endpoint_type, - group=bkm.BARBICAN_OPT_GROUP) + group=bkm._BARBICAN_OPT_GROUP) if vkm is not None: if vault_root_token_id is not None: @@ -151,12 +158,16 @@ def list_opts(): :returns: a list of (group_name, opts) tuples """ key_manager_opts = [] - key_manager_opts.extend(km.key_manager_opts) + key_manager_opts.extend(key_manager.key_manager_opts) key_manager_opts.extend(utils.credential_opts) opts = [('key_manager', key_manager_opts)] - if bkm is not None: - opts.append((bkm.BARBICAN_OPT_GROUP, bkm.barbican_opts)) - if vkm is not None: - opts.append((vkm.VAULT_OPT_GROUP, vkm.vault_opts)) + ext_mgr = ExtensionManager( + "castellan.drivers", + invoke_on_load=True, + invoke_args=[cfg.CONF]) + + for driver in ext_mgr.names(): + opts.extend(ext_mgr[driver].obj.list_options_for_discovery()) + return opts diff --git a/castellan/tests/unit/test_options.py b/castellan/tests/unit/test_options.py index bd8c3ffa..3b82c49f 100644 --- a/castellan/tests/unit/test_options.py +++ b/castellan/tests/unit/test_options.py @@ -40,34 +40,34 @@ class TestOptions(base.TestCase): barbican_endpoint = 'http://test-server.org:9311/' options.set_defaults(conf, barbican_endpoint=barbican_endpoint) self.assertEqual(barbican_endpoint, - conf.get(bkm.BARBICAN_OPT_GROUP).barbican_endpoint) + conf.barbican.barbican_endpoint) barbican_api_version = 'vSomething' options.set_defaults(conf, barbican_api_version=barbican_api_version) self.assertEqual(barbican_api_version, - conf.get(bkm.BARBICAN_OPT_GROUP).barbican_api_version) + conf.barbican.barbican_api_version) auth_endpoint = 'http://test-server.org/identity' options.set_defaults(conf, auth_endpoint=auth_endpoint) self.assertEqual(auth_endpoint, - conf.get(bkm.BARBICAN_OPT_GROUP).auth_endpoint) + conf.barbican.auth_endpoint) retry_delay = 3 options.set_defaults(conf, retry_delay=retry_delay) self.assertEqual(retry_delay, - conf.get(bkm.BARBICAN_OPT_GROUP).retry_delay) + conf.barbican.retry_delay) number_of_retries = 10 options.set_defaults(conf, number_of_retries=number_of_retries) self.assertEqual(number_of_retries, - conf.get(bkm.BARBICAN_OPT_GROUP).number_of_retries) + conf.barbican.number_of_retries) verify_ssl = True options.set_defaults(conf, verify_ssl=True) self.assertEqual(verify_ssl, - conf.get(bkm.BARBICAN_OPT_GROUP).verify_ssl) + conf.barbican.verify_ssl) barbican_endpoint_type = 'internal' options.set_defaults(conf, barbican_endpoint_type='internal') - result_type = conf.get(bkm.BARBICAN_OPT_GROUP).barbican_endpoint_type + result_type = conf.barbican.barbican_endpoint_type self.assertEqual(barbican_endpoint_type, result_type) diff --git a/releasenotes/notes/implements-keymanager-option-discovery-13a46c1dfc036a3f.yaml b/releasenotes/notes/implements-keymanager-option-discovery-13a46c1dfc036a3f.yaml new file mode 100644 index 00000000..6d5632b2 --- /dev/null +++ b/releasenotes/notes/implements-keymanager-option-discovery-13a46c1dfc036a3f.yaml @@ -0,0 +1,15 @@ +--- +features: + - | + Enhance the global option listing to discover available key managers and + their options. The purpose of this feature is to have a correct listing of + the supported key managers, now each key manager is responsible for + advertising the oslo.config groups/options they consume. +other: + - | + The visibility of module variables and constants related to oslo.config + options changed to private in both barbican and vault key managers. The + key managers are only responsible for overloading the method + list_options_for_discovery() in order to advertise their own options. + This way, the global options doesn't need to know which variables to look + for.