diff --git a/src/managers/config.py b/src/managers/config.py index 75078904..96481e9e 100644 --- a/src/managers/config.py +++ b/src/managers/config.py @@ -4,8 +4,9 @@ """Manager for for handling configuration building + writing.""" import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any +import yaml from ops.model import ConfigData if TYPE_CHECKING: @@ -17,22 +18,25 @@ logger = logging.getLogger(__name__) -DEFAULT_PROPERTIES = """ -opensearch.ssl.verificationMode: full -opensearch.requestHeadersWhitelist: [authorization, securitytenant] -opensearch_security.multitenancy.enabled: true -opensearch_security.multitenancy.tenants.preferred: [Private, Global] -opensearch_security.readonly_mode.roles: [kibana_read_only] -opensearch_security.cookie.secure: true -""" +DEFAULT_PROPERTIES = { + "opensearch.requestHeadersWhitelist": ["authorization", "securitytenant"], + "opensearch_security.multitenancy.enabled": True, + "opensearch_security.multitenancy.tenants.preferred": ["Private", "Global"], + "opensearch_security.readonly_mode.roles": ["kibana_read_only"], + "server.ssl.enabled": False, + "opensearch_security.cookie.secure": False, +} -TLS_PROPERTIES = """ -server.ssl.enabled: true -""" +# Overrides the DEFAULT_PROPERTIES if we have TLS enabled +TLS_PROPERTIES = { + "server.ssl.enabled": True, + "opensearch.ssl.verificationMode": "full", + "opensearch_security.cookie.secure": True, +} -LOG_PROPERTIES = """ -logging.verbose: true -""" +LOG_PROPERTIES = { + "logging.verbose": True, +} class ConfigManager: @@ -79,13 +83,16 @@ def log_level(self) -> str: return "" @property - def dashboard_properties(self) -> list[str]: - """Build the zoo.cfg content. + def dashboard_properties(self) -> dict[str, Any]: + """Build the opensearch_dashboards.yml content. + + As we are building on top of the known templates above, we do not need to care about + merging lists, for example. We will override the default properties if needed. Returns: - List of properties to be set to zoo.cfg config file + List of properties to be set to opensearch_dashboards.yml config file """ - properties = DEFAULT_PROPERTIES.split("\n") + properties = DEFAULT_PROPERTIES.copy() opensearch_user = ( self.state.opensearch_server.username if self.state.opensearch_server else "" @@ -94,43 +101,36 @@ def dashboard_properties(self) -> list[str]: self.state.opensearch_server.password if self.state.opensearch_server else "" ) - opensearch_endpoints = ( - ", ".join(f"https://{endpoint}" for endpoint in self.state.opensearch_server.endpoints) - if self.state.opensearch_server and len(self.state.opensearch_server.endpoints) > 0 - else "" - ) + if self.state.opensearch_server and len(self.state.opensearch_server.endpoints) > 0: + properties["opensearch.hosts"] = [ + f"https://{endpoint}" for endpoint in self.state.opensearch_server.endpoints + ] - opensearch_ca = self.workload.paths.opensearch_ca if self.state.opensearch_server else "" + opensearch_ca = self.workload.paths.opensearch_ca if self.state.opensearch_server else None # We are using the address exposed by Juju as service address - properties += [f"server.host: '{self.state.bind_address}'"] - properties += ( - [ - f"opensearch.username: {opensearch_user}", - f"opensearch.password: {opensearch_password}", - ] - if opensearch_user and opensearch_password - else [] - ) - - properties += ( - [f"opensearch.hosts: [{opensearch_endpoints}]"] if opensearch_endpoints else [] - ) + properties["server.host"] = str(self.state.bind_address) + if opensearch_user and opensearch_password: + properties |= { + "opensearch.username": opensearch_user, + "opensearch.password": opensearch_password, + } if opensearch_ca: - properties += [f'opensearch.ssl.certificateAuthorities: [ "{opensearch_ca}" ]'] + properties["opensearch.ssl.certificateAuthorities"] = [opensearch_ca] if self.state.unit_server.tls: - properties += TLS_PROPERTIES.split("\n") + [ - f"server.ssl.certificate: {self.workload.paths.certificate}", - f"server.ssl.key: {self.workload.paths.server_key}", - ] + properties |= TLS_PROPERTIES + properties |= { + "server.ssl.certificate": self.workload.paths.certificate, + "server.ssl.key": self.workload.paths.server_key, + } # Log-level - properties += [f"{self.log_level}: true"] + properties[self.log_level] = True # Paths - properties += [f"path.data: {self.workload.paths.data_path}"] + properties["path.data"] = self.workload.paths.data_path return properties @@ -139,61 +139,26 @@ def current_properties(self) -> list[str]: """The current configuration properties set to zoo.cfg.""" return self.workload.read(self.workload.paths.properties) - @property - def current_env(self) -> list[str]: - """The current /etc/environment variables.""" - return self.workload.read(path="/etc/environment") - - @property - def static_properties(self) -> list[str]: - """Build the zoo.cfg content, without dynamic options. - - Returns: - List of static properties to compared to current zoo.cfg - """ - return self.build_static_properties(self.dashboard_properties) - def set_dashboard_properties(self) -> None: """Writes built config file.""" self.workload.write( - content="\n".join(self.dashboard_properties), + content=yaml.dump(self.dashboard_properties), path=self.workload.paths.properties, ) - @staticmethod - def build_static_properties(properties: list[str]) -> list[str]: - """Removes dynamic config options from list of properties. - - Args: - properties: the properties to make static - - Returns: - List of static properties - """ - return [ - prop - for prop in properties - if ("clientPort" not in prop and "secureClientPort" not in prop) - ] + def load_dashboard_properties(self) -> dict[str, Any]: + """Reads built config file.""" + return yaml.safe_load( + "\n".join( + self.workload.read( + path=self.workload.paths.properties, + ) + ) + ) def config_changed(self) -> bool: """Compares expected vs actual config that would require a restart to apply.""" - server_properties = self.build_static_properties(self.current_properties) - config_properties = self.static_properties - - properties_changed = set(server_properties) ^ set(config_properties) - - if not properties_changed: + if self.load_dashboard_properties() == self.dashboard_properties: return False - - if properties_changed: - logger.info( - ( - f"Server.{self.state.unit_server.unit_id} updating properties - " - f"OLD PROPERTIES = {set(server_properties) - set(config_properties)}, " - f"NEW PROPERTIES = {set(config_properties) - set(server_properties)}" - ) - ) - self.set_dashboard_properties() - + self.set_dashboard_properties() return True diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index fc327234..1ef9b70a 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -18,17 +18,21 @@ ACTIONS = str(yaml.safe_load(Path("./actions.yaml").read_text())) METADATA = str(yaml.safe_load(Path("./metadata.yaml").read_text())) -DEFAULT_CONF = """ -opensearch.ssl.verificationMode: full -opensearch.requestHeadersWhitelist: [authorization, securitytenant] +DEFAULT_CONF = """logging.verbose: true +opensearch.requestHeadersWhitelist: +- authorization +- securitytenant +opensearch_security.cookie.secure: false opensearch_security.multitenancy.enabled: true -opensearch_security.multitenancy.tenants.preferred: [Private, Global] -opensearch_security.readonly_mode.roles: [kibana_read_only] -opensearch_security.cookie.secure: true - -server.host: '{ip}' -logging.verbose: true -path.data: /var/snap/opensearch-dashboards/common/var/lib/opensearch-dashboards""" +opensearch_security.multitenancy.tenants.preferred: +- Private +- Global +opensearch_security.readonly_mode.roles: +- kibana_read_only +path.data: /var/snap/opensearch-dashboards/common/var/lib/opensearch-dashboards +server.host: {ip} +server.ssl.enabled: false +""" @pytest.fixture @@ -46,23 +50,12 @@ def harness(): def test_log_level_changed(harness): - with ( - patch( - "managers.config.ConfigManager.build_static_properties", - return_value=["log_level=logging.verbose"], - ), - patch( - "managers.config.ConfigManager.static_properties", - return_value="log_level=logging.silent", - ), - patch("managers.config.ConfigManager.set_dashboard_properties") as set_props, - ): + with (patch("managers.config.ConfigManager.set_dashboard_properties") as set_props,): harness.charm.config_manager.config_changed() set_props.assert_called_once() with ( - patch("workload.ODWorkload.read", return_value=["log_level=logging.silent"]), - patch("managers.config.ConfigManager.current_env", return_value=["log_level"]), + patch("workload.ODWorkload.read", return_value=["log_level: logging.silent"]), patch("workload.ODWorkload.write") as write, ): assert harness.charm.config_manager.config_changed() @@ -85,19 +78,4 @@ def test_tls_enabled(harness): label=f"{PEER}.opensearch-dashboards.unit", ) - assert "server.ssl.enabled: true" in harness.charm.config_manager.dashboard_properties - - -# def test_properties_tls_uses_passwords(harness): -# with harness.hooks_disabled(): -# harness.update_relation_data( -# harness.charm.state.peer_relation.id, CHARM_KEY, {"tls": "enabled"} -# ) -# harness.update_relation_data( -# harness.charm.state.peer_relation.id, -# f"{CHARM_KEY}/0", -# {"keystore-password": "mellon", "truststore-password": "friend"}, -# ) -# -# assert "ssl.keyStore.password=mellon" in harness.charm.config_manager.dashboard_properties -# assert "ssl.trustStore.password=friend" in harness.charm.config_manager.dashboard_properties + assert harness.charm.config_manager.dashboard_properties.get("server.ssl.enabled") is True