From 39b9b42f981e9526f0292d6be300e56d9a17c0ae Mon Sep 17 00:00:00 2001 From: Isman Firmansyah Date: Wed, 22 Jun 2022 21:06:31 +0700 Subject: [PATCH] refactor(pycloudlib): add more typehint on manager, secret, and config modules (#1582) * refactor: add more typehints to manager module and config/secret subpackages * refactor: replace typing Optional with Union * docs(pycloudlib): adjust docstrings for manager, config, and secret modules * test(pycloudlib): add missing testcases for k8s configs and secrets * refactor: fix return type for create_secret function * refactor: fix incompatible types on function calls * docs: fix typo on pip install * test: handle unsupported config/secret adapter --- jans-pycloudlib/README.md | 40 +++- jans-pycloudlib/docs/conf.py | 5 + jans-pycloudlib/docs/config.rst | 6 +- jans-pycloudlib/docs/manager.rst | 14 +- jans-pycloudlib/docs/secret.rst | 3 + .../jans/pycloudlib/config/base_config.py | 40 ++-- .../jans/pycloudlib/config/consul_config.py | 88 +++---- .../jans/pycloudlib/config/google_config.py | 73 +++--- .../pycloudlib/config/kubernetes_config.py | 33 ++- jans-pycloudlib/jans/pycloudlib/manager.py | 222 +++++++++++------- .../jans/pycloudlib/secret/base_secret.py | 38 +-- .../jans/pycloudlib/secret/google_secret.py | 102 ++++---- .../pycloudlib/secret/kubernetes_secret.py | 35 ++- .../jans/pycloudlib/secret/vault_secret.py | 90 +++---- jans-pycloudlib/pyproject.toml | 18 ++ jans-pycloudlib/tests/conftest.py | 19 +- jans-pycloudlib/tests/test_config.py | 64 +++-- jans-pycloudlib/tests/test_manager.py | 104 ++++---- jans-pycloudlib/tests/test_secret.py | 69 +++--- 19 files changed, 600 insertions(+), 463 deletions(-) diff --git a/jans-pycloudlib/README.md b/jans-pycloudlib/README.md index 1bdec3b9519..d7862442cab 100644 --- a/jans-pycloudlib/README.md +++ b/jans-pycloudlib/README.md @@ -2,12 +2,44 @@ Utilities for Janssen cloud deployment. Used in Janssen docker images. -## Developer Guide +## Developer guide -Refer to https://github.com/JanssenProject/jans-pycloudlib/wiki for guidance on how to use/develop this library. +### Run testcases + +Testcase files are available under `tests` directory. +To run testcase suite, follow steps below: + +1. Install [tox](https://tox.wiki/en/latest/) by running `pip install tox` +1. Run `tox` executable to run all testcase suites (note, to run a single testcase suite, run `tox -- tests/path/to/testfile` instead; see avaiable test files under `tests` directory) + +### Check docstrings + +The sourcecode of `jans.pycloudlib` are heavily documented internally using docstrings. +To check whether they are missing docstrings, run the following steps: + +1. Install [pydocstyle](http://www.pydocstyle.org/en/stable/) by running `pip install pydocstyle[toml]` +1. Check docstrings by running `pydocstyle` +1. Adjust docstrings if any error is reported by `pydocstyle` + +### Check Python types + +We are adding more typehints into the `jans.pycloudlib` sourcecode, gradually. + +1. Install [mypy](https://mypy.readthedocs.io/en/stable/index.html) by running `pip install mypy` +1. Check typehints by running `mypy --install-types /path/to/python/file` +1. Fix errors reported by `mypy` + +### Building internal docs + +Internal docs are generated from sphinx-based docs at `docs` directory. +To generate/preview docs, run the following steps: + +1. Install [sphinx-autobuild](https://github.com/executablebooks/sphinx-autobuild) by running `pip install sphinx-autobuild` +1. Generate docs by running `sphinx-autobuild --watch=jans docs docs/_build/html` +1. Visit http://localhost:8000 to see the generated docs (they are reloaded automatically when sourcecode is modified) + +## Refs -# Refs - https://www.linuxfoundation.org/press-release/2020/12/the-janssen-project-takes-on-worlds-most-demanding-digital-trust-challenges-at-linux-foundation/ - https://betanews.com/2020/12/08/linux-foundation-open-source-identity-management/ - https://www.techrepublic.com/article/linux-foundation-debuts-new-secure-open-source-cloud-native-access-management-software-platform/ - diff --git a/jans-pycloudlib/docs/conf.py b/jans-pycloudlib/docs/conf.py index b17f6cb1adf..0af9fa7d4f6 100644 --- a/jans-pycloudlib/docs/conf.py +++ b/jans-pycloudlib/docs/conf.py @@ -81,3 +81,8 @@ def find_version(*file_paths): # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". html_static_path = ['_static'] + +autodoc_default_options = { + "undoc-members": True, + "exclude-members": "_abc_impl", +} diff --git a/jans-pycloudlib/docs/config.rst b/jans-pycloudlib/docs/config.rst index 01d0ba1910b..c990486be55 100644 --- a/jans-pycloudlib/docs/config.rst +++ b/jans-pycloudlib/docs/config.rst @@ -12,7 +12,7 @@ Consul .. autoclass:: jans.pycloudlib.config.consul_config.ConsulConfig :members: :private-members: - :undoc-members: + :show-inheritance: Kubernetes ========== @@ -20,7 +20,7 @@ Kubernetes .. autoclass:: jans.pycloudlib.config.kubernetes_config.KubernetesConfig :members: :private-members: - :undoc-members: + :show-inheritance: Google ====== @@ -28,4 +28,4 @@ Google .. autoclass:: jans.pycloudlib.config.google_config.GoogleConfig :members: :private-members: - :undoc-members: + :show-inheritance: diff --git a/jans-pycloudlib/docs/manager.rst b/jans-pycloudlib/docs/manager.rst index 0716403c061..4749e224932 100644 --- a/jans-pycloudlib/docs/manager.rst +++ b/jans-pycloudlib/docs/manager.rst @@ -3,16 +3,28 @@ Manager .. module:: jans.pycloudlib.manager +.. autoclass:: AdapterProtocol + +.. autoclass:: BaseConfiguration + :members: + :private-members: + :undoc-members: + .. autoclass:: ConfigManager :members: :private-members: :undoc-members: + :show-inheritance: .. autoclass:: SecretManager :members: :private-members: :undoc-members: + :show-inheritance: -.. autodata:: jans.pycloudlib.manager._Manager +.. autoclass:: jans.pycloudlib.manager._Manager + :members: + :private-members: + :undoc-members: .. autofunction:: get_manager diff --git a/jans-pycloudlib/docs/secret.rst b/jans-pycloudlib/docs/secret.rst index 5d39d2078ed..bf4d2d23079 100644 --- a/jans-pycloudlib/docs/secret.rst +++ b/jans-pycloudlib/docs/secret.rst @@ -13,6 +13,7 @@ Vault :members: :private-members: :undoc-members: + :show-inheritance: Kubernetes ========== @@ -21,6 +22,7 @@ Kubernetes :members: :private-members: :undoc-members: + :show-inheritance: Google ====== @@ -29,3 +31,4 @@ Google :members: :private-members: :undoc-members: + :show-inheritance: diff --git a/jans-pycloudlib/jans/pycloudlib/config/base_config.py b/jans-pycloudlib/jans/pycloudlib/config/base_config.py index ddf89c18d71..17fc448359b 100644 --- a/jans-pycloudlib/jans/pycloudlib/config/base_config.py +++ b/jans-pycloudlib/jans/pycloudlib/config/base_config.py @@ -1,48 +1,52 @@ """This module contains base class for config adapter.""" -from typing import Any -from typing import NoReturn +import typing as _t +from abc import ABC +from abc import abstractmethod -class BaseConfig: +class BaseConfig(ABC): """Base class for config adapter. Must be sub-classed per implementation details. """ - type = "config" + @property + def type(self) -> str: + """Name of the configuration type. - def get(self, key: str, default: Any = "") -> NoReturn: + This attribute always returns ``config``. + """ + return "config" + + @abstractmethod + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get specific config. Subclass **MUST** implement this method. """ - raise NotImplementedError - def set(self, key: str, value: Any) -> NoReturn: + @abstractmethod + def set(self, key: str, value: _t.Any) -> bool: """Set specific config. Subclass **MUST** implement this method. """ - raise NotImplementedError - - def all(self) -> NoReturn: # pragma: no cover - """Get all config (deprecated in favor of ``get_all``). - Subclass **MUST** implement this method. - """ + def all(self) -> dict[str, _t.Any]: # noqa: A003 + """Get all config (deprecated in favor of ``get_all``).""" return self.get_all() - def set_all(self, data: dict) -> NoReturn: + @abstractmethod + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set all config. Subclass **MUST** implement this method. """ - raise NotImplementedError - def get_all(self) -> NoReturn: - """Get all secrets. + @abstractmethod + def get_all(self) -> dict[str, _t.Any]: + """Get all configs. Subclass **MUST** implement this method. """ - raise NotImplementedError diff --git a/jans-pycloudlib/jans/pycloudlib/config/consul_config.py b/jans-pycloudlib/jans/pycloudlib/config/consul_config.py index 7323fdb8bde..4002639a7dd 100644 --- a/jans-pycloudlib/jans/pycloudlib/config/consul_config.py +++ b/jans-pycloudlib/jans/pycloudlib/config/consul_config.py @@ -2,9 +2,7 @@ import logging import os -from typing import Any -from typing import Tuple -from typing import Union +import typing as _t from consul import Consul @@ -14,25 +12,30 @@ logger = logging.getLogger(__name__) +MaybeCert = _t.Union[tuple[str, str], None] +MaybeCacert = _t.Union[bool, str] + class ConsulConfig(BaseConfig): """This class interacts with Consul backend. - The following environment variables are used to instantiate the client: - - - ``CN_CONFIG_CONSUL_HOST`` - - ``CN_CONFIG_CONSUL_PORT`` - - ``CN_CONFIG_CONSUL_CONSISTENCY`` - - ``CN_CONFIG_CONSUL_SCHEME`` - - ``CN_CONFIG_CONSUL_VERIFY`` - - ``CN_CONFIG_CONSUL_CACERT_FILE`` - - ``CN_CONFIG_CONSUL_CERT_FILE`` - - ``CN_CONFIG_CONSUL_KEY_FILE`` - - ``CN_CONFIG_CONSUL_TOKEN_FILE`` - - ``CN_CONFIG_CONSUL_NAMESPACE`` + .. important:: The instance of this class is configured via environment variables. + + Supported environment variables: + + - ``CN_CONFIG_CONSUL_HOST``: hostname or IP of Consul (default to ``localhost``). + - ``CN_CONFIG_CONSUL_PORT``: port of Consul (default to ``8500``). + - ``CN_CONFIG_CONSUL_CONSISTENCY``: Consul consistency mode (choose one of ``default``, ``consistent``, or ``stale``). Default to ``stale`` mode. + - ``CN_CONFIG_CONSUL_SCHEME``: supported Consul scheme (``http`` or ``https``). + - ``CN_CONFIG_CONSUL_VERIFY``: whether to verify cert or not (default to ``false``). + - ``CN_CONFIG_CONSUL_CACERT_FILE``: path to Consul CA cert file (default to ``/etc/certs/consul_ca.crt``). This file will be used if it exists and ``CN_CONFIG_CONSUL_VERIFY`` set to ``true``. + - ``CN_CONFIG_CONSUL_CERT_FILE``: path to Consul cert file (default to ``/etc/certs/consul_client.crt``). + - ``CN_CONFIG_CONSUL_KEY_FILE``: path to Consul key file (default to ``/etc/certs/consul_client.key``). + - ``CN_CONFIG_CONSUL_TOKEN_FILE``: path to file contains ACL token (default to ``/etc/certs/consul_token``). + - ``CN_CONFIG_CONSUL_NAMESPACE``: namespace used to create the config tree, i.e. ``jans/config`` (default to ``jans``). """ - def __init__(self): + def __init__(self) -> None: self.settings = { k: v for k, v in os.environ.items() @@ -44,7 +47,7 @@ def __init__(self): ) self.settings.setdefault( - "CN_CONFIG_CONSUL_PORT", 8500, + "CN_CONFIG_CONSUL_PORT", "8500", ) self.settings.setdefault( @@ -56,7 +59,7 @@ def __init__(self): ) self.settings.setdefault( - "CN_CONFIG_CONSUL_VERIFY", False, + "CN_CONFIG_CONSUL_VERIFY", "false", ) self.settings.setdefault( @@ -80,7 +83,7 @@ def __init__(self): self.prefix = f"{self.settings['CN_CONFIG_CONSUL_NAMESPACE']}/config/" cert, verify = self._verify_cert( self.settings["CN_CONFIG_CONSUL_SCHEME"], - self.settings["CN_CONFIG_CONSUL_VERIFY"], + as_boolean(self.settings["CN_CONFIG_CONSUL_VERIFY"]), self.settings["CN_CONFIG_CONSUL_CACERT_FILE"], self.settings["CN_CONFIG_CONSUL_CERT_FILE"], self.settings["CN_CONFIG_CONSUL_KEY_FILE"], @@ -90,7 +93,7 @@ def __init__(self): self.client = Consul( host=self.settings["CN_CONFIG_CONSUL_HOST"], - port=self.settings["CN_CONFIG_CONSUL_PORT"], + port=int(self.settings["CN_CONFIG_CONSUL_PORT"]), token=self._token_from_file(self.settings["CN_CONFIG_CONSUL_TOKEN_FILE"]), scheme=self.settings["CN_CONFIG_CONSUL_SCHEME"], consistency=self.settings["CN_CONFIG_CONSUL_CONSISTENCY"], @@ -113,14 +116,14 @@ def _unmerge_path(self, key: str) -> str: """Remove prefix from the key. For example, given the namespace is ``jans``, prefix will be set as ``jans/config`` - and an absolute path``jans/config/random``, calling this method returns ``random`` key. + and an absolute path ``jans/config/random``, calling this method returns ``random`` key. :param key: Key name as relative path. :returns: Relative path to key. """ return key[len(self.prefix):] - def get(self, key: str, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -133,38 +136,31 @@ def get(self, key: str, default: Any = "") -> Any: # this is a bytes return result["Value"].decode() - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. :param value: Value of the key. :returns: A ``bool`` to mark whether config is set or not. """ - return self.client.kv.put(self._merge_path(key), safe_value(value)) + return bool(self.client.kv.put(self._merge_path(key), safe_value(value))) - def all(self) -> dict: # pragma: no cover - """Get all key-value pairs. - - :returns: A ``dict`` of key-value pairs (if any). - """ - return self.get_all() - - def _request_warning(self, scheme: str, verify: bool) -> None: + def _request_warning(self, scheme: str, verify: _t.Union[bool, str]) -> None: """Emit warning about unverified request to unsecure Consul address. :param scheme: Scheme of Consul address. :param verify: Mark whether client needs to verify the address. """ - if scheme == "https" and verify is False: - import urllib3 + import urllib3 + if scheme == "https" and verify is False: urllib3.disable_warnings() logger.warning( "All requests to Consul will be unverified. " "Please adjust CN_CONFIG_CONSUL_SCHEME and " "CN_CONFIG_CONSUL_VERIFY environment variables.") - def _token_from_file(self, path) -> str: + def _token_from_file(self, path: str) -> str: """Get the token string from a path. :param path: Path to file contains token string. @@ -177,7 +173,14 @@ def _token_from_file(self, path) -> str: token = fr.read().strip() return token - def _verify_cert(self, scheme, verify, cacert_file, cert_file, key_file) -> Tuple[Union[None, tuple], Union[bool, str]]: + def _verify_cert( + self, + scheme: str, + verify: bool, + cacert_file: str, + cert_file: str, + key_file: str, + ) -> tuple[MaybeCert, MaybeCacert]: """Verify client cert and key. :param scheme: Scheme of Consul address. @@ -188,19 +191,18 @@ def _verify_cert(self, scheme, verify, cacert_file, cert_file, key_file) -> Tupl :returns: A pair of cert key files (if exist) and verification. """ cert = None + maybe_cacert: MaybeCacert = as_boolean(verify) if scheme == "https": - verify = as_boolean(verify) - # verify using CA cert (if any) - if all([verify, os.path.isfile(cacert_file)]): - verify = cacert_file + if all([maybe_cacert, os.path.isfile(cacert_file)]): + maybe_cacert = cacert_file if all([os.path.isfile(cert_file), os.path.isfile(key_file)]): cert = (cert_file, key_file) - return cert, verify + return cert, maybe_cacert - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set key-value pairs. :param data: Key-value pairs. @@ -210,7 +212,7 @@ def set_all(self, data: dict) -> bool: self.set(k, v) return True - def get_all(self): + def get_all(self) -> dict[str, _t.Any]: """Get all key-value pairs. :returns: A ``dict`` of key-value pairs (if any). diff --git a/jans-pycloudlib/jans/pycloudlib/config/google_config.py b/jans-pycloudlib/jans/pycloudlib/config/google_config.py index a546ec33a52..9d55985f409 100644 --- a/jans-pycloudlib/jans/pycloudlib/config/google_config.py +++ b/jans-pycloudlib/jans/pycloudlib/config/google_config.py @@ -1,10 +1,13 @@ """This module contains config adapter class to interact with Google Secret.""" +from __future__ import annotations + import sys import logging import os import json -from typing import Any +import typing as _t +from functools import cached_property from google.cloud import secretmanager from google.api_core.exceptions import AlreadyExists, NotFound @@ -12,36 +15,39 @@ from jans.pycloudlib.utils import safe_value from jans.pycloudlib.config.base_config import BaseConfig +if _t.TYPE_CHECKING: # pragma: no cover + # imported objects for function type hint, completion, etc. + # these won't be executed in runtime + from google.cloud import secretmanager_v1 + logger = logging.getLogger(__name__) class GoogleConfig(BaseConfig): """This class interacts with Google Secret backend. - The following environment variables are used to instantiate the client: + .. important:: The instance of this class is configured via environment variables. + + Supported environment variables: - - ``GOOGLE_APPLICATION_CREDENTIALS`` json file that should be injected in upstream images - - ``GOOGLE_PROJECT_ID`` - - ``CN_CONFIG_GOOGLE_SECRET_VERSION_ID`` - - ``CN_CONFIG_GOOGLE_SECRET_NAME_PREFIX`` + - ``CN_CONFIG_GOOGLE_SECRET_VERSION_ID``: Janssen configuration secret version ID in Google Secret Manager. Defaults to ``latest``, which is recommended. + - ``CN_CONFIG_GOOGLE_SECRET_NAME_PREFIX``: Prefix for Janssen configuration secret in Google Secret Manager. Defaults to ``jans``. If left intact ``jans-configuration`` secret will be created. + - ``GOOGLE_APPLICATION_CREDENTIALS``: JSON file (contains Google credentials) that should be injected into container. + - ``GOOGLE_PROJECT_ID``: ID of Google project. """ - def __init__(self): - self.project_id = os.getenv("GOOGLE_PROJECT_ID") + def __init__(self) -> None: + self.project_id = os.getenv("GOOGLE_PROJECT_ID", "") self.version_id = os.getenv("CN_CONFIG_GOOGLE_SECRET_VERSION_ID", "latest") # secrets key value by default self.google_secret_name = os.getenv("CN_CONFIG_GOOGLE_SECRET_NAME_PREFIX", "jans") + "-configuration" - # Create the Secret Manager client. - self.client = secretmanager.SecretManagerServiceClient() - def all(self) -> dict: # pragma: no cover - """Access the payload for the given secret version if one exists. - - This method is deprecated, use ``get_all`` instead. - """ - return self.get_all() + @cached_property + def client(self) -> secretmanager.SecretManagerServiceClient: + """Create the Secret Manager client.""" + return secretmanager.SecretManagerServiceClient() - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """Access the payload for the given secret version if one exists. The version can be a version number as a string (e.g. "5") or an alias (e.g. "latest"). @@ -76,7 +82,7 @@ def get_all(self) -> dict: return data - def get(self, key, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -86,7 +92,7 @@ def get(self, key, default: Any = "") -> Any: result = self.get_all() return result.get(key) or default - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -101,7 +107,7 @@ def set(self, key: str, value: Any) -> bool: secret_version_bool = self.add_secret_version(safe_value(all_)) return secret_version_bool - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Push a full dictionary to secrets. :param data: full dictionary to push. Used in initial creation of config and secret @@ -115,15 +121,19 @@ def set_all(self, data: dict) -> bool: secret_version_bool = self.add_secret_version(safe_value(all_)) return secret_version_bool - def create_secret(self) -> bool: + def create_secret(self) -> _t.Union[secretmanager_v1.types.Secret, None]: """Create a new secret with the given name. A secret is a logical wrapper around a collection of secret versions. Secret versions hold the actual secret material. + + .. versionchanged:: 1.0.1 + Returns ``google.cloud.secretmanager_v1.types.Secret`` instead of boolean. """ # Build the resource name of the parent project. parent = f"projects/{self.project_id}" - response = False + + response = None try: # Create the secret. response = self.client.create_secret( @@ -133,14 +143,12 @@ def create_secret(self) -> bool: "secret": {"replication": {"automatic": {}}}, } ) - logger.info("Created secret: {}".format(response.name)) - + logger.info(f"Created secret: {response.name}") except AlreadyExists: logger.warning(f'Secret {self.google_secret_name} already exists. A new version will be created.') + return response - return bool(response) - - def add_secret_version(self, payload: str) -> bool: + def add_secret_version(self, payload: _t.AnyStr) -> bool: """Add a new secret version to the given secret with the provided payload. :param payload: payload @@ -148,13 +156,16 @@ def add_secret_version(self, payload: str) -> bool: # Build the resource name of the parent secret. parent = self.client.secret_path(self.project_id, self.google_secret_name) - # Convert the string payload into a bytes. This step can be omitted if you - # pass in bytes instead of a str for the payload argument. - payload = payload.encode("UTF-8") + if isinstance(payload, str): + # Convert the string payload into a bytes (if it's a string). + # This step can be omitted if you pass in bytes instead of a str for the payload argument. + payload_bytes = payload.encode("UTF-8") + else: + payload_bytes = payload # Add the secret version. response = self.client.add_secret_version( - request={"parent": parent, "payload": {"data": payload}} + request={"parent": parent, "payload": {"data": payload_bytes}} ) logger.info("Added secret version: {}".format(response.name)) diff --git a/jans-pycloudlib/jans/pycloudlib/config/kubernetes_config.py b/jans-pycloudlib/jans/pycloudlib/config/kubernetes_config.py index b9d9b27c5b1..f3e3f8ac077 100644 --- a/jans-pycloudlib/jans/pycloudlib/config/kubernetes_config.py +++ b/jans-pycloudlib/jans/pycloudlib/config/kubernetes_config.py @@ -1,7 +1,7 @@ """This module contains config adapter class to interact with Kubernetes ConfigMap.""" import os -from typing import Any +import typing as _t import kubernetes.client import kubernetes.config @@ -14,14 +14,16 @@ class KubernetesConfig(BaseConfig): """This class interacts with Kubernetes ConfigMap backend. - The following environment variables are used to instantiate the client: + .. important:: The instance of this class is configured via environment variables. - - ``CN_CONFIG_KUBERNETES_NAMESPACE`` - - ``CN_CONFIG_KUBERNETES_CONFIGMAP`` - - ``CN_CONFIG_KUBERNETES_USE_KUBE_CONFIG`` + Supported environment variables: + + - ``CN_CONFIG_KUBERNETES_NAMESPACE``: Kubernetes namespace (default to ``default``). + - ``CN_CONFIG_KUBERNETES_CONFIGMAP``: Kubernetes configmaps name (default to ``jans``). + - ``CN_CONFIG_KUBERNETES_USE_KUBE_CONFIG``: Load credentials from ``$HOME/.kube/config``, only useful for non-container environment (default to ``false``). """ - def __init__(self): + def __init__(self) -> None: self.settings = { k: v for k, v in os.environ.items() @@ -35,13 +37,13 @@ def __init__(self): "CN_CONFIG_KUBERNETES_CONFIGMAP", "jans", ) - self.settings.setdefault("CN_CONFIG_KUBERNETES_USE_KUBE_CONFIG", False) + self.settings.setdefault("CN_CONFIG_KUBERNETES_USE_KUBE_CONFIG", "false") self._client = None self.name_exists = False self.kubeconfig_file = os.path.expanduser("~/.kube/config") - def get(self, key: str, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -52,7 +54,7 @@ def get(self, key: str, default: Any = "") -> Any: return result.get(key) or default @property - def client(self): + def client(self) -> kubernetes.client.CoreV1Api: """Lazy-loaded client to interact with Kubernetes API.""" if not self._client: if as_boolean(self.settings["CN_CONFIG_KUBERNETES_USE_KUBE_CONFIG"]): @@ -90,7 +92,7 @@ def _prepare_configmap(self) -> None: else: raise - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -111,14 +113,7 @@ def set(self, key: str, value: Any) -> bool: ) return bool(ret) - def all(self) -> dict: # pragma: no cover - """Get all key-value pairs. - - :returns: A ``dict`` of key-value pairs (if any). - """ - return self.get_all() - - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """Get all key-value pairs. :returns: A ``dict`` of key-value pairs (if any). @@ -130,7 +125,7 @@ def get_all(self) -> dict: ) return result.data or {} - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set all key-value pairs. :returns: A ``bool`` indicating operation is succeed or not. diff --git a/jans-pycloudlib/jans/pycloudlib/manager.py b/jans-pycloudlib/jans/pycloudlib/manager.py index 9ae79d69943..68aee6981fe 100644 --- a/jans-pycloudlib/jans/pycloudlib/manager.py +++ b/jans-pycloudlib/jans/pycloudlib/manager.py @@ -1,10 +1,11 @@ """This module contains config and secret helpers.""" import os -from collections import namedtuple -from typing import Any -from typing import AnyStr -from typing import NamedTuple +import typing as _t +from abc import ABC +from abc import abstractproperty +from dataclasses import dataclass +from functools import cached_property from jans.pycloudlib.config import ConsulConfig from jans.pycloudlib.config import KubernetesConfig @@ -15,29 +16,47 @@ from jans.pycloudlib.utils import decode_text from jans.pycloudlib.utils import encode_text +ConfigAdapter = _t.Union[ConsulConfig, KubernetesConfig, GoogleConfig] +SecretAdapter = _t.Union[VaultSecret, KubernetesSecret, GoogleSecret] -class ConfigManager: - """This class acts as a proxy to specific config adapter class. - Supported config adapter class: +class AdapterProtocol(_t.Protocol): # pragma: no cover + """Custom class to define adapter contracts (only useful for type check).""" - - :class:`~jans.pycloudlib.config.consul_config.ConsulConfig` - - :class:`~jans.pycloudlib.config.kubernetes_config.KubernetesConfig` - - :class:`~jans.pycloudlib.config.google_config.GoogleConfig` - """ + def get(self, key: str, default: _t.Any = "") -> _t.Any: # noqa: D102 + ... + + def set(self, key: str, value: _t.Any) -> bool: # noqa: D102 + ... + + def all(self) -> dict[str, _t.Any]: # noqa: A003,D102 + ... + + def get_all(self) -> dict[str, _t.Any]: # noqa: D102 + ... + + def set_all(self, data: dict[str, _t.Any]) -> bool: # noqa: D102 + ... + + +class BaseConfiguration(ABC): + """Base class to provide contracts for managing configuration (configs or secrets).""" + + @abstractproperty + def adapter(self) -> AdapterProtocol: # pragma: no cover + """Abstract attribute as a container of adapter instance. - def __init__(self): - _adapter = os.environ.get("CN_CONFIG_ADAPTER", "consul",) - if _adapter == "consul": - self.adapter = ConsulConfig() - elif _adapter == "kubernetes": - self.adapter = KubernetesConfig() - elif _adapter == "google": - self.adapter = GoogleConfig() - else: - self.adapter = None - - def get(self, key: str, default: Any = "") -> Any: + The adapter is used in the following public methods: + + - ``get`` + - ``get_all`` + - ``set`` + - ``set_all`` + + Any subclass **MUST** returns an instance of adapter or raise exception. + """ + + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -46,7 +65,7 @@ def get(self, key: str, default: Any = "") -> Any: """ return self.adapter.get(key, default) - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -55,21 +74,21 @@ def set(self, key: str, value: Any) -> bool: """ return self.adapter.set(key, value) - def all(self) -> dict: # noqa: A003 + def all(self) -> dict[str, _t.Any]: # noqa: A003 """Get all key-value pairs (deprecated in favor of ``get_all``). - :returns: A ``dict`` of key-value pairs (if any). + :returns: A ``dict`` of key-value pairs. """ return self.get_all() - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """Get all key-value pairs. - :returns: A ``dict`` of key-value pairs (if any). + :returns: A ``dict`` of key-value pairs. """ return self.adapter.get_all() - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set all key-value pairs. :param data: Key-value pairs. @@ -77,69 +96,86 @@ def set_all(self, data: dict) -> bool: return self.adapter.set_all(data) -class SecretManager: - """This class acts as a proxy to specific secret adapter class. +class ConfigManager(BaseConfiguration): + """A subclass of :class:`~jans.pycloudlib.manager.BaseConfiguration` to manage configs. - Supported secret adapter class: - - - :class:`~jans.pycloudlib.secret.vault_secret.VaultSecret` - - :class:`~jans.pycloudlib.secret.kubernetes_secret.KubernetesSecret` - - :class:`~jans.pycloudlib.secret.google_secret.GoogleSecret` + This class acts as a proxy to specific config adapter class. """ - def __init__(self): - _adapter = os.environ.get("CN_SECRET_ADAPTER", "vault",) - if _adapter == "vault": - self.adapter = VaultSecret() - elif _adapter == "kubernetes": - self.adapter = KubernetesSecret() - elif _adapter == "google": - self.adapter = GoogleSecret() - else: - self.adapter = None - - def get(self, key: str, default: Any = "") -> Any: - """Get value based on given key. + @cached_property + def adapter(self) -> ConfigAdapter: # noqa: D412 + """Get an instance of adapter class. - :param key: Key name. - :param default: Default value if key is not exist. - :returns: Value based on given key or default one. - """ - return self.adapter.get(key, default) + Example: - def set(self, key: str, value: Any) -> bool: # noqa: A003 - """Set key with given value. + .. code-block:: python - :param key: Key name. - :param value: Value of the key. - :returns: A ``bool`` to mark whether config is set or not. - """ - return self.adapter.set(key, value) + os.environ["CN_CONFIG_ADAPTER"] = "consul" + ConfigManager().adapter # returns an instance of adapter class - def all(self) -> dict: # noqa: A003 - """Get all key-value pairs (deprecated in favor of ``get_all``). + .. important:: The adapter name is pre-populated from ``CN_CONFIG_ADAPTER`` environment variable. - :returns: A ``dict`` of key-value pairs (if any). + Supported config adapter name: + + - ``consul``: returns an instance of :class:`~jans.pycloudlib.config.consul_config.ConsulConfig` + - ``kubernetes``: returns an instance of :class:`~jans.pycloudlib.config.kubernetes_config.KubernetesConfig` + - ``google``: returns an instance of :class:`~jans.pycloudlib.config.google_config.GoogleConfig` """ - return self.get_all() + adapter = os.environ.get("CN_CONFIG_ADAPTER", "consul") - def get_all(self) -> dict: - """Get all key-value pairs. + if adapter == "consul": + return ConsulConfig() - :returns: A ``dict`` of key-value pairs (if any). - """ - return self.adapter.get_all() + if adapter == "kubernetes": + return KubernetesConfig() - def set_all(self, data: dict) -> bool: - """Set all key-value pairs. + if adapter == "google": + return GoogleConfig() - :param data: Key-value pairs. + raise ValueError(f"Unsupported config adapter {adapter!r}") + + +class SecretManager(BaseConfiguration): + """A subclass of :class:`~jans.pycloudlib.manager.BaseConfiguration` to manage secrets. + + This class acts as a proxy to specific secret adapter class. + """ + + @cached_property + def adapter(self) -> SecretAdapter: # noqa: D412 + """Get an instance of adapter class. + + Example: + + .. code-block:: python + + os.environ["CN_SECRET_ADAPTER"] = "vault" + SecretManager().adapter # returns an instance of adapter class + + .. important:: The adapter name is pre-populated from ``CN_SECRET_ADAPTER`` environment variable (i.e. ``CN_SECRET_ADAPTER=vault``). + + Supported config adapter name: + + - ``vault``: returns an instance of :class:`~jans.pycloudlib.secret.vault_secret.VaultSecret` + - ``kubernetes``: returns an instance of :class:`~jans.pycloudlib.secret.kubernetes_secret.KubernetesSecret` + - ``google``: returns an instance of :class:`~jans.pycloudlib.secret.google_secret.GoogleSecret` """ - return self.adapter.set_all(data) + adapter = os.environ.get("CN_SECRET_ADAPTER", "vault") + + if adapter == "vault": + return VaultSecret() + + if adapter == "kubernetes": + return KubernetesSecret() + + if adapter == "google": + return GoogleSecret() + + raise ValueError(f"Unsupported secret adapter {adapter!r}") def to_file( self, key: str, dest: str, decode: bool = False, binary_mode: bool = False - ) -> AnyStr: # noqa: D412 + ) -> None: # noqa: D412 """Pull secret and write to a file. Example: @@ -190,7 +226,7 @@ def to_file( def from_file( self, key: str, src: str, encode: bool = False, binary_mode: bool = False ) -> None: # noqa: D412 - """Put secret from a file. + """Read from a file and put to secret. Example: @@ -231,18 +267,34 @@ def from_file( self.adapter.set(key, value) -#: Object as a placeholder of config and secret manager. -#: This object is not intended for direct use, use ``get_manager`` -#: function instead. -_Manager = namedtuple("_Manager", ["config", "secret"]) +@dataclass +class _Manager: + """Class acts as a container of config and secret manager. + + This object is not intended for direct use, use :func:`~jans.pycloudlib.manager.get_manager` function instead. + """ + + #: An instance of :class:`~jans.pycloudlib.manager.ConfigManager` + config: ConfigManager + + #: An instance of :class:`~jans.pycloudlib.manager.SecretManager` + secret: SecretManager + + +def get_manager() -> _Manager: + """Create an instance of :class:`~jans.pycloudlib.manager._Manager` class. + + The instance has ``config`` and ``secret`` attributes to interact with + configs and secrets, for example: + .. code-block:: python -def get_manager() -> NamedTuple: - """Get config and secret manager instances. + manager = get_manager() + manager.config.get("hostname") + manager.secret.get("ssl-cert") - :returns: A ``namedtuple`` consists of :class:`~jans.pycloudlib.manager.ConfigManager` - and :class:`~jans.pycloudlib.manager.SecretManager` instances. + :returns: An instance of :class:`~jans.pycloudlib.manager._Manager`. """ config_mgr = ConfigManager() secret_mgr = SecretManager() - return _Manager(config=config_mgr, secret=secret_mgr) + return _Manager(config_mgr, secret_mgr) diff --git a/jans-pycloudlib/jans/pycloudlib/secret/base_secret.py b/jans-pycloudlib/jans/pycloudlib/secret/base_secret.py index 652a6fa817d..5aeeb776aee 100644 --- a/jans-pycloudlib/jans/pycloudlib/secret/base_secret.py +++ b/jans-pycloudlib/jans/pycloudlib/secret/base_secret.py @@ -1,48 +1,52 @@ """This module contains base class for secret adapter.""" -from typing import Any -from typing import NoReturn +import typing as _t +from abc import ABC +from abc import abstractmethod -class BaseSecret: +class BaseSecret(ABC): """Base class for secret adapter. Must be sub-classed per implementation details. """ - type = "secret" + @property + def type(self) -> str: + """Name of the configuration type. - def get(self, key: str, default: Any = "") -> NoReturn: + This attribute always returns ``secret``. + """ + return "secret" + + @abstractmethod + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get specific secret. Subclass **MUST** implement this method. """ - raise NotImplementedError - def set(self, key: str, value: Any) -> NoReturn: + @abstractmethod + def set(self, key: str, value: _t.Any) -> bool: """Set specific secret. Subclass **MUST** implement this method. """ - raise NotImplementedError - - def all(self) -> NoReturn: # pragma: no cover - """Get all secrets (deprecated in favor of ``get_all``). - Subclass **MUST** implement this method. - """ + def all(self) -> dict[str, _t.Any]: # pragma: no cover + """Get all secrets (deprecated in favor of ``get_all``).""" return self.get_all() - def set_all(self, data: dict) -> NoReturn: + @abstractmethod + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set all secrets. Subclass **MUST** implement this method. """ - raise NotImplementedError - def get_all(self) -> NoReturn: + @abstractmethod + def get_all(self) -> dict[str, _t.Any]: """Get all secrets. Subclass **MUST** implement this method. """ - raise NotImplementedError diff --git a/jans-pycloudlib/jans/pycloudlib/secret/google_secret.py b/jans-pycloudlib/jans/pycloudlib/secret/google_secret.py index 926df815568..0e209e6f98f 100644 --- a/jans-pycloudlib/jans/pycloudlib/secret/google_secret.py +++ b/jans-pycloudlib/jans/pycloudlib/secret/google_secret.py @@ -1,14 +1,17 @@ """This module contains secret adapter class to interact with Google Secret.""" +from __future__ import annotations + import hashlib import sys import os import json import logging import lzma +import typing as _t import zlib from binascii import hexlify, unhexlify -from typing import Any +from functools import cached_property from cryptography.hazmat.primitives.ciphers.aead import AESGCM from cryptography.exceptions import InvalidTag @@ -18,32 +21,42 @@ from jans.pycloudlib.secret.base_secret import BaseSecret from jans.pycloudlib.utils import safe_value +if _t.TYPE_CHECKING: # pragma: no cover + # imported objects for function type hint, completion, etc. + # these won't be executed in runtime + from google.cloud import secretmanager_v1 + logger = logging.getLogger(__name__) class GoogleSecret(BaseSecret): """This class interacts with Google Secret backend. - The following environment variables are used to instantiate the client: + .. important:: The instance of this class is configured via environment variables. + + Supported environment variables: - - ``GOOGLE_APPLICATION_CREDENTIALS`` json file that should be injected in upstream images - - ``GOOGLE_PROJECT_ID`` - - ``CN_SECRET_GOOGLE_SECRET_VERSION_ID`` - - ``CN_SECRET_GOOGLE_SECRET_MANAGER_PASSPHRASE`` - - ``CN_SECRET_GOOGLE_SECRET_NAME_PREFIX`` + - ``CN_SECRET_GOOGLE_SECRET_VERSION_ID``: Janssen secret version ID in Google Secret Manager. Defaults to ``latest``, which is recommended. + - ``CN_SECRET_GOOGLE_SECRET_MANAGER_PASSPHRASE``: Passphrase for Janssen secret in Google Secret Manager. This is recommended to be changed and defaults to ``secret``. + - ``CN_SECRET_GOOGLE_SECRET_NAME_PREFIX``: Prefix for Janssen secret in Google Secret Manager. Defaults to ``jans``. If left ``jans-secret`` secret will be created. + - ``GOOGLE_APPLICATION_CREDENTIALS``: JSON file (contains Google credentials) that should be injected into container. + - ``GOOGLE_PROJECT_ID``: ID of Google project. """ - def __init__(self): - self.project_id = os.getenv("GOOGLE_PROJECT_ID") + def __init__(self) -> None: + self.project_id = os.getenv("GOOGLE_PROJECT_ID", "") self.version_id = os.getenv("CN_SECRET_GOOGLE_SECRET_VERSION_ID", "latest") self.salt = os.urandom(16) self.passphrase = os.getenv("CN_SECRET_GOOGLE_SECRET_MANAGER_PASSPHRASE", "secret") # secrets key value by default self.google_secret_name = os.getenv("CN_SECRET_GOOGLE_SECRET_NAME_PREFIX", "jans") + "-secret" - # Create the Secret Manager client. - self.client = secretmanager.SecretManagerServiceClient() self.key = self._set_key() + @cached_property + def client(self) -> secretmanager.SecretManagerServiceClient: + """Create the Secret Manager client.""" + return secretmanager.SecretManagerServiceClient() + def _set_key(self) -> bytes: """Return key for for encrypting and decrypting payload. @@ -59,9 +72,10 @@ def _encrypt(self, plaintext: str) -> str: """ aes = AESGCM(self.key) iv = os.urandom(16) - plaintext = plaintext.encode("utf8") - plaintext = lzma.compress(plaintext) - ciphertext = aes.encrypt(iv, plaintext, None) + + text_bytes = plaintext.encode("utf8") + text_bytes = lzma.compress(text_bytes) + ciphertext = aes.encrypt(iv, text_bytes, None) logger.info(f'Size of encrypted secret payload : {sys.getsizeof(ciphertext)} bytes') return "%s-%s-%s" % ( hexlify(self.salt).decode("utf8"), hexlify(iv).decode("utf8"), hexlify(ciphertext).decode("utf8")) @@ -72,25 +86,19 @@ def _decrypt(self, ciphertext: str) -> str: :param ciphertext: encrypted string to decrypt :return: decrypted payload """ - self.salt, iv, ciphertext = map(unhexlify, ciphertext.split("-")) + self.salt, iv, cipher_bytes = map(unhexlify, ciphertext.split("-")) self.key = self._set_key() - aes = AESGCM(self.key) - plaintext = "" + + plaintext = b"" try: - plaintext = aes.decrypt(iv, ciphertext, None) + aes = AESGCM(self.key) + plaintext = aes.decrypt(iv, cipher_bytes, None) plaintext = lzma.decompress(plaintext) except InvalidTag: logger.error("Wrong passphrase used.") return plaintext.decode("utf8") - def all(self) -> dict: # pragma: no cover - """Access the payload for the given secret version if one exists. - - This method is deprecated, use ``get_all`` instead. - """ - return self.get_all() - - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """Access the payload for the given secret version if one exists. The version can be a version number as a string (e.g. "5") @@ -125,7 +133,7 @@ def get_all(self) -> dict: " set to jans.") return data - def get(self, key, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -135,7 +143,7 @@ def get(self, key, default: Any = "") -> Any: result = self.get_all() return result.get(key) or default - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -151,7 +159,7 @@ def set(self, key: str, value: Any) -> bool: self._encrypt(safe_value(all_))) return secret_version_bool - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Push a full dictionary to secrets. :param data: full dictionary to push. Used in initial creation of config and secret @@ -166,15 +174,19 @@ def set_all(self, data: dict) -> bool: self._encrypt(safe_value(all_))) return secret_version_bool - def create_secret(self) -> bool: + def create_secret(self) -> _t.Union[secretmanager_v1.types.Secret, None]: """Create a new secret with the given name. A secret is a logical wrapper around a collection of secret versions. Secret versions hold the actual secret material. + + .. versionchanged:: 1.0.1 + Returns ``google.cloud.secretmanager_v1.types.Secret`` instead of boolean. """ # Build the resource name of the parent project. parent = f"projects/{self.project_id}" - response = False + + response = None try: # Create the secret. response = self.client.create_secret( @@ -184,14 +196,12 @@ def create_secret(self) -> bool: "secret": {"replication": {"automatic": {}}}, } ) - logger.info("Created secret: {}".format(response.name)) - + logger.info(f"Created secret: {response.name}") except AlreadyExists: logger.warning(f'Secret {self.google_secret_name} already exists. A new version will be created.') + return response - return bool(response) - - def add_secret_version(self, payload: str) -> bool: + def add_secret_version(self, payload: _t.AnyStr) -> bool: """Add a new secret version to the given secret with the provided payload. :param payload: encrypted payload @@ -199,16 +209,24 @@ def add_secret_version(self, payload: str) -> bool: # Build the resource name of the parent secret. parent = self.client.secret_path(self.project_id, self.google_secret_name) - # Convert the string payload into a bytes. This step can be omitted if you - # pass in bytes instead of a str for the payload argument. - payload = zlib.compress(payload.encode("UTF-8")) - logger.info(f'Size of final compressed secret payload : {sys.getsizeof(payload)} bytes') + if isinstance(payload, str): + # Convert the string payload into a bytes. This step can be omitted if you + # pass in bytes instead of a str for the payload argument. + payload_bytes = payload.encode("UTF-8") + else: + payload_bytes = payload + + # compress the payload + payload_bytes = zlib.compress(payload_bytes) + + logger.info(f'Size of final compressed secret payload : {sys.getsizeof(payload_bytes)} bytes') + # Add the secret version. response = self.client.add_secret_version( - request={"parent": parent, "payload": {"data": payload}} + request={"parent": parent, "payload": {"data": payload_bytes}} ) - logger.info("Added secret version: {}".format(response.name)) + logger.info(f"Added secret version: {response.name}") return bool(response) def delete(self) -> None: diff --git a/jans-pycloudlib/jans/pycloudlib/secret/kubernetes_secret.py b/jans-pycloudlib/jans/pycloudlib/secret/kubernetes_secret.py index eadb7347531..9f63eb44a81 100644 --- a/jans-pycloudlib/jans/pycloudlib/secret/kubernetes_secret.py +++ b/jans-pycloudlib/jans/pycloudlib/secret/kubernetes_secret.py @@ -2,7 +2,7 @@ import base64 import os -from typing import Any +import typing as _t import kubernetes.client import kubernetes.config @@ -15,14 +15,16 @@ class KubernetesSecret(BaseSecret): """This class interacts with Kubernetes Secret backend. - The following environment variables are used to instantiate the client: + .. important:: The instance of this class is configured via environment variables. - - ``CN_SECRET_KUBERNETES_NAMESPACE`` - - ``CN_SECRET_KUBERNETES_SECRET`` - - ``CN_SECRET_KUBERNETES_USE_KUBE_CONFIG`` + Supported environment variables: + + - ``CN_SECRET_KUBERNETES_NAMESPACE``: Kubernetes namespace (default to ``default``). + - ``CN_SECRET_KUBERNETES_SECRET``: Kubernetes secrets name (default to ``jans``). + - ``CN_SECRET_KUBERNETES_USE_KUBE_CONFIG``: Load credentials from ``$HOME/.kube/config``, only useful for non-container environment (default to ``false``). """ - def __init__(self): + def __init__(self) -> None: self.settings = { k: v for k, v in os.environ.items() @@ -34,14 +36,14 @@ def __init__(self): self.settings.setdefault( "CN_SECRET_KUBERNETES_SECRET", "jans", ) - self.settings.setdefault("CN_SECRET_KUBERNETES_USE_KUBE_CONFIG", False) + self.settings.setdefault("CN_SECRET_KUBERNETES_USE_KUBE_CONFIG", "false") self._client = None self.name_exists = False self.kubeconfig_file = os.path.expanduser("~/.kube/config") @property - def client(self): + def client(self) -> kubernetes.client.CoreV1Api: """Lazy-loaded client to interact with Kubernetes API.""" if not self._client: if as_boolean(self.settings["CN_SECRET_KUBERNETES_USE_KUBE_CONFIG"]): @@ -51,14 +53,14 @@ def client(self): self._client = kubernetes.client.CoreV1Api() return self._client - def get(self, key, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. :param default: Default value if key is not exist. :returns: Value based on given key or default one. """ - result = self.all() + result = self.get_all() return result.get(key) or default def _prepare_secret(self) -> None: @@ -89,7 +91,7 @@ def _prepare_secret(self) -> None: else: raise - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -110,14 +112,7 @@ def set(self, key: str, value: Any) -> bool: ) return bool(ret) - def all(self) -> dict: # pragma: no cover - """Get all key-value pairs. - - :returns: A ``dict`` of key-value pairs (if any). - """ - return self.get_all() - - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """Get all key-value pairs. :returns: A ``dict`` of key-value pairs (if any). @@ -131,7 +126,7 @@ def get_all(self) -> dict: data = result.data or {} return {k: base64.b64decode(v).decode() for k, v in data.items()} - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set all key-value pairs. :param key: Key name. diff --git a/jans-pycloudlib/jans/pycloudlib/secret/vault_secret.py b/jans-pycloudlib/jans/pycloudlib/secret/vault_secret.py index e641100112a..61a9cb6aa70 100644 --- a/jans-pycloudlib/jans/pycloudlib/secret/vault_secret.py +++ b/jans-pycloudlib/jans/pycloudlib/secret/vault_secret.py @@ -1,10 +1,8 @@ """This module contains secret adapter class to interact with Vault.""" +import typing as _t import logging import os -from typing import Any -from typing import Tuple -from typing import Union import hvac @@ -14,25 +12,30 @@ logger = logging.getLogger(__name__) +MaybeCert = _t.Union[tuple[str, str], None] +MaybeCacert = _t.Union[bool, str] + class VaultSecret(BaseSecret): """This class interacts with Vault backend. - The following environment variables are used to instantiate the client: - - - ``CN_SECRET_VAULT_HOST`` - - ``CN_SECRET_VAULT_PORT`` - - ``CN_SECRET_VAULT_SCHEME`` - - ``CN_SECRET_VAULT_VERIFY`` - - ``CN_SECRET_VAULT_ROLE_ID_FILE`` - - ``CN_SECRET_VAULT_SECRET_ID_FILE`` - - ``CN_SECRET_VAULT_CERT_FILE`` - - ``CN_SECRET_VAULT_KEY_FILE`` - - ``CN_SECRET_VAULT_CACERT_FILE`` - - ``CN_SECRET_VAULT_NAMESPACE`` + .. important:: The instance of this class is configured via environment variables. + + Supported environment variables: + + - ``CN_SECRET_VAULT_SCHEME``: supported Vault scheme (``http`` or ``https``). + - ``CN_SECRET_VAULT_HOST``: hostname or IP of Vault (default to ``localhost``). + - ``CN_SECRET_VAULT_PORT``: port of Vault (default to ``8200``). + - ``CN_SECRET_VAULT_VERIFY``: whether to verify cert or not (default to ``false``). + - ``CN_SECRET_VAULT_ROLE_ID_FILE``: path to file contains Vault AppRole role ID (default to ``/etc/certs/vault_role_id``). + - ``CN_SECRET_VAULT_SECRET_ID_FILE``: path to file contains Vault AppRole secret ID (default to ``/etc/certs/vault_secret_id``). + - ``CN_SECRET_VAULT_CERT_FILE``: path to Vault cert file (default to ``/etc/certs/vault_client.crt``). + - ``CN_SECRET_VAULT_KEY_FILE``: path to Vault key file (default to ``/etc/certs/vault_client.key``). + - ``CN_SECRET_VAULT_CACERT_FILE``: path to Vault CA cert file (default to ``/etc/certs/vault_ca.crt``). This file will be used if it exists and ``CN_SECRET_VAULT_VERIFY`` set to ``true``. + - ``CN_SECRET_VAULT_NAMESPACE``: namespace used to create the config tree, i.e. ``secret/jans`` (default to ``jans``). """ - def __init__(self): + def __init__(self) -> None: self.settings = { k: v for k, v in os.environ.items() @@ -42,13 +45,13 @@ def __init__(self): "CN_SECRET_VAULT_HOST", "localhost", ) self.settings.setdefault( - "CN_SECRET_VAULT_PORT", 8200, + "CN_SECRET_VAULT_PORT", "8200", ) self.settings.setdefault( "CN_SECRET_VAULT_SCHEME", "http", ) self.settings.setdefault( - "CN_SECRET_VAULT_VERIFY", False, + "CN_SECRET_VAULT_VERIFY", "false", ) self.settings.setdefault( "CN_SECRET_VAULT_ROLE_ID_FILE", "/etc/certs/vault_role_id", @@ -70,7 +73,7 @@ def __init__(self): cert, verify = self._verify_cert( self.settings["CN_SECRET_VAULT_SCHEME"], - self.settings["CN_SECRET_VAULT_VERIFY"], + as_boolean(self.settings["CN_SECRET_VAULT_VERIFY"]), self.settings["CN_SECRET_VAULT_CACERT_FILE"], self.settings["CN_SECRET_VAULT_CERT_FILE"], self.settings["CN_SECRET_VAULT_KEY_FILE"], @@ -82,7 +85,7 @@ def __init__(self): url="{}://{}:{}".format( self.settings["CN_SECRET_VAULT_SCHEME"], self.settings["CN_SECRET_VAULT_HOST"], - self.settings["CN_SECRET_VAULT_PORT"], + int(self.settings["CN_SECRET_VAULT_PORT"]), ), cert=cert, verify=verify, @@ -90,7 +93,7 @@ def __init__(self): self.prefix = f"secret/{self.settings['CN_SECRET_VAULT_NAMESPACE']}" @property - def role_id(self): + def role_id(self) -> str: """Get the Role ID from file. The file location is determined by @@ -104,7 +107,7 @@ def role_id(self): return role_id @property - def secret_id(self): + def secret_id(self) -> str: """Get the Secret ID from file. The file location is determined by @@ -114,7 +117,7 @@ def secret_id(self): with open(self.settings["CN_SECRET_VAULT_SECRET_ID_FILE"]) as f: secret_id = f.read() except FileNotFoundError: - secret_id = "" + secret_id = "" # nosec: B105 return secret_id def _authenticate(self) -> None: @@ -125,7 +128,7 @@ def _authenticate(self) -> None: creds = self.client.auth.approle.login(self.role_id, self.secret_id, use_token=False) self.client.token = creds["auth"]["client_token"] - def get(self, key: str, default: Any = "") -> Any: + def get(self, key: str, default: _t.Any = "") -> _t.Any: """Get value based on given key. :param key: Key name. @@ -139,7 +142,7 @@ def get(self, key: str, default: Any = "") -> Any: return default return sc["data"]["value"] - def set(self, key: str, value: Any) -> bool: + def set(self, key: str, value: _t.Any) -> bool: """Set key with given value. :param key: Key name. @@ -155,32 +158,32 @@ def set(self, key: str, value: Any) -> bool: response = self.client._adapter.post( f"/v1/{self.prefix}/{key}", json=val, ) - return response.status_code == 204 - - def all(self) -> dict: # pragma: no cover - """Get all key-value pairs. - - :returns: A ``dict`` of key-value pairs (if any). - """ - return self.get_all() + return bool(response.status_code == 204) - def _request_warning(self, scheme: str, verify: bool) -> None: + def _request_warning(self, scheme: str, verify: _t.Union[bool, str]) -> None: """ Emit warning about unverified request to unsecure Consul address. :param scheme: Scheme of Vault address. :param verify: Mark whether client needs to verify the address. """ - if scheme == "https" and verify is False: - import urllib3 + import urllib3 + if scheme == "https" and verify is False: urllib3.disable_warnings() logger.warning( "All requests to Vault will be unverified. " "Please adjust CN_SECRET_VAULT_SCHEME and " "CN_SECRET_VAULT_VERIFY environment variables.") - def _verify_cert(self, scheme, verify, cacert_file, cert_file, key_file) -> Tuple[Union[None, tuple], Union[bool, str]]: + def _verify_cert( + self, + scheme: str, + verify: bool, + cacert_file: str, + cert_file: str, + key_file: str, + ) -> tuple[MaybeCert, MaybeCacert]: """Verify client cert and key. :param scheme: Scheme of Consul address. @@ -191,19 +194,18 @@ def _verify_cert(self, scheme, verify, cacert_file, cert_file, key_file) -> Tupl :returns: A pair of cert key files (if exist) and verification. """ cert = None + maybe_cacert: MaybeCacert = as_boolean(verify) if scheme == "https": - verify = as_boolean(verify) - # verify using CA cert (if any) - if all([verify, os.path.isfile(cacert_file)]): - verify = cacert_file + if all([maybe_cacert, os.path.isfile(cacert_file)]): + maybe_cacert = cacert_file if all([os.path.isfile(cert_file), os.path.isfile(key_file)]): cert = (cert_file, key_file) - return cert, verify + return cert, maybe_cacert - def set_all(self, data: dict) -> bool: + def set_all(self, data: dict[str, _t.Any]) -> bool: """Set key-value pairs. :param data: Key-value pairs. @@ -213,7 +215,7 @@ def set_all(self, data: dict) -> bool: self.set(k, v) return True - def get_all(self) -> dict: + def get_all(self) -> dict[str, _t.Any]: """ Get all key-value pairs. diff --git a/jans-pycloudlib/pyproject.toml b/jans-pycloudlib/pyproject.toml index ecfbc37cdfa..f3b31daab07 100644 --- a/jans-pycloudlib/pyproject.toml +++ b/jans-pycloudlib/pyproject.toml @@ -4,3 +4,21 @@ inherit = false match = '(?!test_|setup|conf).*\.py' # match_dir = '[^\.].*' add_ignore = "D107" + +[tool.mypy] +strict = true +follow_imports = "silent" +implicit_reexport = true +show_error_context = true +pretty = true +show_column_numbers = true +show_error_codes = true + +[[tool.mypy.overrides]] +module = [ + "consul", + "hvac", + "kubernetes.*", + "urllib3", +] +ignore_missing_imports = true diff --git a/jans-pycloudlib/tests/conftest.py b/jans-pycloudlib/tests/conftest.py index ea07d4cfd9d..69eb3ef1ddd 100644 --- a/jans-pycloudlib/tests/conftest.py +++ b/jans-pycloudlib/tests/conftest.py @@ -1,15 +1,6 @@ import pytest -@pytest.fixture() -def gconfig(): - from jans.pycloudlib.config.base_config import BaseConfig - - class GConfig(BaseConfig): - pass - yield GConfig() - - @pytest.fixture() def gconsul_config(): from jans.pycloudlib.config import ConsulConfig @@ -31,15 +22,6 @@ def gk8s_config(): yield config -@pytest.fixture -def gsecret(): - from jans.pycloudlib.secret.base_secret import BaseSecret - - class GSecret(BaseSecret): - pass - yield GSecret() - - @pytest.fixture() def gvault_secret(): from jans.pycloudlib.secret import VaultSecret @@ -84,6 +66,7 @@ def get_secret(key, default=""): "sql_password": "secret", "couchbase_password": "secret", "couchbase_superuser_password": "secret", + "random": ENCODED_PW, } return ctx.get(key) or default diff --git a/jans-pycloudlib/tests/test_config.py b/jans-pycloudlib/tests/test_config.py index 6c5a1d3cc21..2b71c80643e 100644 --- a/jans-pycloudlib/tests/test_config.py +++ b/jans-pycloudlib/tests/test_config.py @@ -5,36 +5,6 @@ KubeResult = namedtuple("KubeResult", ["data"]) - -# =========== -# base config -# =========== - - -def test_config_get(gconfig): - with pytest.raises(NotImplementedError) as exc: - gconfig.get("foo") - assert "" in str(exc.value) - - -def test_config_set(gconfig): - with pytest.raises(NotImplementedError) as exc: - gconfig.set("foo", "bar") - assert "" in str(exc.value) - - -def test_config_get_all(gconfig): - with pytest.raises(NotImplementedError) as exc: - gconfig.all() - assert "" in str(exc.value) - - -def test_config_set_all(gconfig): - with pytest.raises(NotImplementedError) as exc: - gconfig.set_all({}) - assert "" in str(exc.value) - - # ============= # consul config # ============= @@ -95,7 +65,7 @@ def test_consul_config_set(gconsul_config, monkeypatch): assert gconsul_config.set("foo", "bar") is True -def test_consul_config_all(gconsul_config, monkeypatch): +def test_consul_config_get_all(gconsul_config, monkeypatch): monkeypatch.setattr( "consul.Consul.KV.get", lambda cls, k, recurse: ( @@ -106,21 +76,30 @@ def test_consul_config_all(gconsul_config, monkeypatch): ], ), ) - assert gconsul_config.all() == {"foo": "bar", "lorem": "ipsum"} + assert gconsul_config.get_all() == {"foo": "bar", "lorem": "ipsum"} -def test_consul_config_all_empty(gconsul_config, monkeypatch): +def test_consul_config_get_all_empty(gconsul_config, monkeypatch): monkeypatch.setattr( "consul.Consul.KV.get", lambda cls, k, recurse: (1, []), ) - assert gconsul_config.all() == {} + assert gconsul_config.get_all() == {} def test_consul_config_request_warning(gconsul_config, caplog): gconsul_config._request_warning("https", False) assert "All requests to Consul will be unverified" in caplog.records[0].message + +def test_consul_config_set_all(gconsul_config, monkeypatch): + monkeypatch.setattr( + "consul.Consul.KV.put", + lambda cls, k, v: True, + ) + assert gconsul_config.set_all({"foo": "bar"}) is True + + # ================= # kubernetes config # ================= @@ -201,3 +180,20 @@ def test_k8s_config_incluster(): with pytest.raises(kubernetes.config.config_exception.ConfigException): config.client + + +def test_k8s_config_set_all(gk8s_config, monkeypatch): + monkeypatch.setattr( + "kubernetes.client.CoreV1Api.read_namespaced_config_map", + lambda cls, n, ns: KubeResult(data={"foo": "bar"}) + ) + monkeypatch.setattr( + "kubernetes.client.CoreV1Api.patch_namespaced_config_map", + lambda cls, n, ns, body: KubeResult(data={"foo": "bar"}) + ) + assert gk8s_config.set_all({"foo": "bar"}) is True + + +def test_k8s_config_type(gk8s_config): + # gk8s_config is a subclass of BaseConfig + assert gk8s_config.type == "config" diff --git a/jans-pycloudlib/tests/test_manager.py b/jans-pycloudlib/tests/test_manager.py index 28bee60587a..e9f13d54d0c 100644 --- a/jans-pycloudlib/tests/test_manager.py +++ b/jans-pycloudlib/tests/test_manager.py @@ -1,27 +1,44 @@ import pytest -class GAdapter(object): - def get(self, k, default=None): - return "GET" +def test_base_configuration(): + from functools import cached_property + from jans.pycloudlib.manager import BaseConfiguration - def set(self, k, v): # noqa: A003 - return "SET" + class Configuration(BaseConfiguration): + @cached_property + def adapter(self): + return Adapter() - def all(self): # noqa: A003 - return {} + class Adapter: + def get(self, k, default=None): + return "random" - def get_all(self): - return {} + def set(self, k, v): # noqa: A003 + return True - def set_all(self, data): - return True + def all(self): # noqa: A003 + return {} + + def get_all(self): + return {} + + def set_all(self, data): + return True + + config = Configuration() + + assert config.get("foo") == "random" + assert config.set("foo", "bar") is True + assert config.all() == {} # ``all`` method is deprecated + assert config.get_all() == {} + assert config.set_all({"foo": "bar"}) is True @pytest.mark.parametrize("adapter, adapter_cls", [ ("consul", "ConsulConfig"), ("kubernetes", "KubernetesConfig"), - ("random", "NoneType"), + ("google", "GoogleConfig"), ]) def test_config_manager(monkeypatch, adapter, adapter_cls): from jans.pycloudlib.manager import ConfigManager @@ -31,10 +48,19 @@ def test_config_manager(monkeypatch, adapter, adapter_cls): assert manager.adapter.__class__.__name__ == adapter_cls +def test_config_manager_invalid_adapter(monkeypatch): + from jans.pycloudlib.manager import ConfigManager + + monkeypatch.setenv("CN_CONFIG_ADAPTER", "random") + with pytest.raises(ValueError) as exc: + _ = ConfigManager().get("config1") + assert "Unsupported config adapter" in str(exc.value) + + @pytest.mark.parametrize("adapter, adapter_cls", [ ("vault", "VaultSecret"), ("kubernetes", "KubernetesSecret"), - ("random", "NoneType"), + ("google", "GoogleSecret") ]) def test_secret_manager(monkeypatch, adapter, adapter_cls): from jans.pycloudlib.manager import SecretManager @@ -44,58 +70,44 @@ def test_secret_manager(monkeypatch, adapter, adapter_cls): assert manager.adapter.__class__.__name__ == adapter_cls -def test_config_manager_methods(): - from jans.pycloudlib.manager import ConfigManager - - gadapter = GAdapter() - manager = ConfigManager() - manager.adapter = gadapter - - assert manager.get("foo") == gadapter.get("foo") - assert manager.set("foo", "bar") == gadapter.set("foo", "bar") - assert manager.all() == gadapter.all() - - -def test_secret_manager_methods(): +def test_secret_manager_invalid_adapter(monkeypatch): from jans.pycloudlib.manager import SecretManager - gadapter = GAdapter() - manager = SecretManager() - manager.adapter = gadapter - - assert manager.get("foo") == gadapter.get("foo") - assert manager.set("foo", "bar") == gadapter.set("foo", "bar") - assert manager.all() == gadapter.all() + monkeypatch.setenv("CN_SECRET_ADAPTER", "random") + with pytest.raises(ValueError) as exc: + _ = SecretManager().get("secret1") + assert "Unsupported secret adapter" in str(exc.value) -@pytest.mark.parametrize("value, expected, decode, binary_mode", [ - ("secret", "secret", False, False), - ("fHL54sT5qHk=", "secret", True, False), - ("secret", b"secret", False, True), - ("fHL54sT5qHk=", b"secret", True, True), +@pytest.mark.parametrize("key, expected, decode, binary_mode", [ + ("sql_password", "secret", False, False), + ("encoded_ox_ldap_pw", "secret", True, False), + ("encoded_ox_ldap_pw", "secret", False, True), + ("encoded_ox_ldap_pw", "secret", True, True), ]) def test_manager_secret_to_file( gmanager, tmpdir, monkeypatch, - value, + key, expected, decode, binary_mode, ): dst = tmpdir.join("secret.txt") - gmanager.secret.to_file("encoded_ox_ldap_pw", str(dst), decode, binary_mode) - assert dst.read() + gmanager.secret.to_file(key, str(dst), decode, binary_mode) + assert dst.read() == expected -@pytest.mark.parametrize("value, expected, encode, binary_mode", [ - ("secret", "fHL54sT5qHk=", True, False), - (b"secret", "fHL54sT5qHk=", True, True), +@pytest.mark.parametrize("key, value, expected, encode, binary_mode", [ + ("random", "secret", "fHL54sT5qHk=", True, False), + ("random", b"secret", "fHL54sT5qHk=", True, True), ]) def test_manager_secret_from_file( gmanager, tmpdir, monkeypatch, + key, value, expected, encode, @@ -104,5 +116,5 @@ def test_manager_secret_from_file( dst = tmpdir.join("secret_file") dst.write(value) - gmanager.secret.from_file("encoded_ox_ldap_pw", str(dst), encode, binary_mode) - assert gmanager.secret.get("encoded_ox_ldap_pw") == expected + gmanager.secret.from_file(key, str(dst), encode, binary_mode) + assert gmanager.secret.get(key) == expected diff --git a/jans-pycloudlib/tests/test_secret.py b/jans-pycloudlib/tests/test_secret.py index c4482a18d40..74b18b357ed 100644 --- a/jans-pycloudlib/tests/test_secret.py +++ b/jans-pycloudlib/tests/test_secret.py @@ -3,38 +3,8 @@ import pytest - KubeResult = namedtuple("KubeResult", ["data"]) - - -# =========== -# base secret -# =========== - - -def test_secret_get(gsecret): - with pytest.raises(NotImplementedError) as exc: - gsecret.get("foo") - assert "" in str(exc.value) - - -def test_secret_set(gsecret): - with pytest.raises(NotImplementedError) as exc: - gsecret.set("foo", "bar") - assert "" in str(exc.value) - - -def test_secret_get_all(gsecret): - with pytest.raises(NotImplementedError) as exc: - gsecret.get_all() - assert "" in str(exc.value) - - -def test_secret_set_all(gsecret): - with pytest.raises(NotImplementedError) as exc: - gsecret.set_all({}) - assert "" in str(exc.value) - +VaultResponse = namedtuple("Response", ["status_code"]) # ============ @@ -133,22 +103,18 @@ def test_vault_secret_get_default(gvault_secret, monkeypatch): def test_vault_secret_set(gvault_secret, monkeypatch): - from collections import namedtuple - - Response = namedtuple("Response", ["status_code"]) - monkeypatch.setattr( "hvac.Client.is_authenticated", lambda cls: True, ) monkeypatch.setattr( "hvac.adapters.Request.post", - lambda cls, url, json: Response(204), + lambda cls, url, json: VaultResponse(204), ) assert gvault_secret.set("foo", "bar") is True -def test_vault_secret_all(gvault_secret, monkeypatch): +def test_vault_secret_get_all(gvault_secret, monkeypatch): monkeypatch.setattr( "hvac.Client.is_authenticated", lambda cls: True, @@ -165,7 +131,7 @@ def test_vault_secret_all(gvault_secret, monkeypatch): assert gvault_secret.all() == {"foo": "bar"} -def test_vault_secret_all_empty(gvault_secret, monkeypatch): +def test_vault_secret_get_all_empty(gvault_secret, monkeypatch): monkeypatch.setattr( "hvac.Client.is_authenticated", lambda cls: True, @@ -182,6 +148,18 @@ def test_vault_secret_request_warning(gvault_secret, caplog): assert "All requests to Vault will be unverified" in caplog.records[0].message +def test_vault_secret_set_all(gvault_secret, monkeypatch): + monkeypatch.setattr( + "hvac.Client.is_authenticated", + lambda cls: True, + ) + monkeypatch.setattr( + "hvac.adapters.Request.post", + lambda cls, url, json: VaultResponse(204), + ) + assert gvault_secret.set_all({"a": 1}) is True + + # ================= # kubernetes secret # ================= @@ -271,3 +249,18 @@ def test_k8s_secret_incluster(): with pytest.raises(kubernetes.config.config_exception.ConfigException): secret.client + + +def test_k8s_secret_set_all(gk8s_secret, monkeypatch): + gk8s_secret.name_exists = True + + monkeypatch.setattr( + "kubernetes.client.CoreV1Api.patch_namespaced_secret", + lambda cls, n, ns, body: KubeResult(data={}) + ) + assert gk8s_secret.set_all({"foo": "bar"}) is True + + +def test_k8s_secret_type(gk8s_secret): + # gk8s_secret is a subclass of BaseSecret + assert gk8s_secret.type == "secret"