From 53f664d0d19f0f066284fa1e79856b852d3c5c1a Mon Sep 17 00:00:00 2001 From: utkarshbhatthere Date: Fri, 26 May 2023 16:47:16 +0530 Subject: [PATCH] Fixes SSL conflicts between relation and config data. The fix adds event based handling of SSL configuration using charm config and cleanup of SSL for relation and config based key/certs. It also adds logical abstractions to analyse SSL setup and emit relevant events. Closes-Bug: 1952282 Change-Id: Ic486434526f639f5985cfe355e303c1d6ff5fa0d Signed-off-by: utkarshbhatthere func-test-pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/1090 --- src/ceph_dashboard_commands.py | 152 ++++++ src/charm.py | 619 ++++++++++++++---------- src/charm_option.py | 108 +++++ tests/bundles/focal-yoga.yaml | 2 +- tests/bundles/focal.yaml | 2 +- tests/bundles/jammy-antelope.yaml | 2 +- tests/bundles/jammy-bobcat.yaml | 2 +- tests/bundles/jammy-yoga.yaml | 2 +- tests/bundles/lunar-antelope.yaml | 4 +- tests/bundles/mantic-bobcat.yaml | 2 +- unit_tests/test_ceph_dashboard_charm.py | 95 ++-- 11 files changed, 701 insertions(+), 289 deletions(-) create mode 100644 src/ceph_dashboard_commands.py create mode 100644 src/charm_option.py diff --git a/src/ceph_dashboard_commands.py b/src/ceph_dashboard_commands.py new file mode 100644 index 0000000..d047e89 --- /dev/null +++ b/src/ceph_dashboard_commands.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical +# See LICENSE file for licensing details. +# +# Learn more at: https://juju.is/docs/sdk + +import json +import socket +from typing import List +from functools import partial + +import subprocess +import logging + +from charm_option import CharmCephOption + +logger = logging.getLogger(__name__) + + +def _run_cmd(cmd: List[str]): + """Run command in subprocess + + `cmd` The command to run + """ + return subprocess.check_output( + cmd, stderr=subprocess.STDOUT + ).decode('UTF-8') + + +def exec_option_ceph_cmd(option: CharmCephOption, value: str) -> None: + """Execute internal ceph command for the CharmCephOption""" + _run_cmd(option.ceph_command(value)) + + +def ceph_dashboard_delete_user(user: str) -> None: + """Delete Ceph dashboard user.""" + cmd = ['ceph', 'dashboard', 'ac-user-delete', user] + _run_cmd(cmd) + + +def ceph_dashboard_add_user(user: str, filename: str, role: str) -> str: + """Create Ceph dashboard user.""" + cmd = [ + 'ceph', 'dashboard', 'ac-user-create', '--enabled', + '-i', filename, user, role + ] + return _run_cmd(cmd) + + +def ceph_dashboard_config_saml( + base_url: str, idp_meta: str, + username_attr: str, idp_entity_id: str +) -> None: + """Configure SSO SAML2""" + cmd = [ + 'ceph', 'dashboard', 'sso', 'setup', 'saml2', + base_url, idp_meta + ] + if username_attr: + cmd.append(username_attr) + + if idp_entity_id: + cmd.append(idp_entity_id) + _run_cmd(cmd) + + +def ceph_config_get(key: str) -> str: + "Fetch Value for a particular ceph-config key." + cmd = [ + "ceph", "config-key", "get", key + ] + try: + return _run_cmd(cmd) + except subprocess.CalledProcessError: + logger.error("Failed to fetch key %s", key) + + +def ceph_config_list() -> list: + "Fetch list of ceph-config keys." + cmd = [ + "ceph", "config-key", "ls" + ] + + # CLI returns empty list if no config-key is configured. + return json.loads(_run_cmd(cmd)) + + +def ceph_config_set(key: str, value: str) -> None: + "Remove the provided key/value pair" + cmd = ["ceph", "config-key", "set", key, value] + + logging.debug("Setting config-key: %s", key) + _run_cmd(cmd) + + +def ceph_config_reset(key: str) -> None: + "Remove the provided key/value pair" + cmd = ["ceph", "config-key", "rm", key] + + logging.debug("Removing config-key: %s", key) + _run_cmd(cmd) + + +def dashboard_set(prop: str, value: str) -> str: + "Configure ceph dashboard properties" + logger.debug("Setting Dashboard %s as %s", prop, value) + return _run_cmd(["ceph", "dashboard", prop, value]) + + +def apply_setting(ceph_setting: str, value: List[str]) -> str: + """Apply a dashboard setting""" + cmd = ["ceph", "dashboard", ceph_setting] + cmd.extend(value) + return _run_cmd(cmd) + + +get_ceph_dashboard_ssl_key = partial(ceph_config_get, "mgr/dashboard/key") +get_ceph_dashboard_ssl_crt = partial(ceph_config_get, "mgr/dashboard/crt") +get_ceph_dashboard_host_ssl_key = partial( + ceph_config_get, f"mgr/dashboard/{socket.gethostname()}/key" +) +get_ceph_dashboard_host_ssl_crt = partial( + ceph_config_get, f"mgr/dashboard/{socket.gethostname()}/crt" +) + + +def check_ceph_dashboard_ssl_enabled() -> bool: + """Check if ssl config-key is set to true""" + ssl_status = ceph_config_get("config/mgr/mgr/dashboard/ssl") + return ssl_status == "true" + + +def check_ceph_dashboard_ssl_configured( + is_check_host_key: bool = False) -> bool: + """Check if SSL key and certificate are configured on ceph dashboard.""" + if is_check_host_key: + keys = [ + f"mgr/dashboard/{socket.gethostname()}/crt", + f"mgr/dashboard/{socket.gethostname()}/key", + ] + else: + keys = [ # List of keys to check for ssl configuration + "mgr/dashboard/crt", + "mgr/dashboard/key" + ] + + for key in keys: + value = ceph_config_get(key) + if value is None: + return False + + return True diff --git a/src/charm.py b/src/charm.py index 4fa7aa0..5ef66ec 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,41 +7,60 @@ """Charm for the Ceph Dashboard.""" import json -import logging -import tempfile - -from ops.framework import StoredState -from ops.main import main -from ops.model import ActiveStatus, BlockedStatus, StatusBase -from ops.charm import ActionEvent -from typing import List, Union, Tuple - import base64 -import interface_tls_certificates.ca_client as ca_client -import interface_openstack_loadbalancer.loadbalancer as ops_lb_interface +import logging import re import secrets import socket import string import subprocess -import tenacity -import ops_openstack.plugins.classes +import tempfile +from pathlib import Path +from typing import List, Tuple, Union + +import charmhelpers.core.host as ch_host +import charms_ceph.utils as ceph_utils +import cryptography.hazmat.primitives.serialization as serialization import interface_ceph_iscsi_admin_access.admin_access as admin_access import interface_dashboard -import interface_grafana_dashboard +import interface_grafana_dashboard as grafana_interface import interface_http +import interface_openstack_loadbalancer.loadbalancer as ops_lb_interface import interface_radosgw_user -import cryptography.hazmat.primitives.serialization as serialization -import charms_ceph.utils as ceph_utils -import charmhelpers.core.host as ch_host +import interface_tls_certificates.ca_client as ca_client +import ops_openstack.plugins.classes +import tenacity -from pathlib import Path +from ops.charm import ActionEvent, CharmEvents +from ops.framework import EventBase, EventSource, StoredState +from ops.main import main +from ops.model import ActiveStatus, BlockedStatus, StatusBase + +# Charm Src +import ceph_dashboard_commands as cmds +from charm_option import CharmCephOptionList logger = logging.getLogger(__name__) TLS_Config = Tuple[Union[bytes, None], Union[bytes, None], Union[bytes, None]] +# Maintenance Events +class DisableSSL(EventBase): + """Charm Event to disable SSL and clean certificates.""" + + +class EnableSSLFromConfig(EventBase): + """Charm Event to configure SSL using Charm config values.""" + + +class CephCharmEvents(CharmEvents): + """Custom charm events.""" + + disable_ssl = EventSource(DisableSSL) + enable_ssl_from_config = EventSource(EnableSSLFromConfig) + + class CephDashboardCharm(ops_openstack.core.OSBaseCharm): """Ceph Dashboard charm.""" @@ -59,183 +78,106 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): DASH_DIR = Path('src/dashboards') LB_SERVICE_NAME = "ceph-dashboard" - class CharmCephOption(): - """Manage a charm option to ceph command to manage that option""" + # Charm Events + on = CephCharmEvents() - def __init__(self, charm_option_name, ceph_option_name, - min_version=None): - self.charm_option_name = charm_option_name - self.ceph_option_name = ceph_option_name - self.min_version = min_version - - def is_supported(self) -> bool: - """Is the option supported on this unit""" - if self.min_version: - return self.minimum_supported(self.min_version) - return True - - def minimum_supported(self, supported_version: str) -> bool: - """Check if installed Ceph release is >= to supported_version""" - return ch_host.cmp_pkgrevno('ceph-common', supported_version) >= 0 - - def convert_option(self, value: Union[bool, str, int]) -> List[str]: - """Convert a value to the corresponding value part of the ceph - dashboard command""" - return [str(value)] - - def ceph_command(self, value: List[str]) -> List[str]: - """Shell command to set option to desired value""" - cmd = ['ceph', 'dashboard', self.ceph_option_name] - cmd.extend(self.convert_option(value)) - return cmd - - class DebugOption(CharmCephOption): - - def convert_option(self, value): - """Convert charm True/False to enable/disable""" - if value: - return ['enable'] - else: - return ['disable'] - - class MOTDOption(CharmCephOption): - - def convert_option(self, value): - """Split motd charm option into ['severity', 'time', 'message']""" - if value: - return value.split('|') - else: - return ['clear'] - - CHARM_TO_CEPH_OPTIONS = [ - DebugOption('debug', 'debug'), - CharmCephOption( - 'enable-password-policy', - 'set-pwd-policy-enabled'), - CharmCephOption( - 'password-policy-check-length', - 'set-pwd-policy-check-length-enabled'), - CharmCephOption( - 'password-policy-check-oldpwd', - 'set-pwd-policy-check-oldpwd-enabled'), - CharmCephOption( - 'password-policy-check-username', - 'set-pwd-policy-check-username-enabled'), - CharmCephOption( - 'password-policy-check-exclusion-list', - 'set-pwd-policy-check-exclusion-list-enabled'), - CharmCephOption( - 'password-policy-check-complexity', - 'set-pwd-policy-check-complexity-enabled'), - CharmCephOption( - 'password-policy-check-sequential-chars', - 'set-pwd-policy-check-sequential-chars-enabled'), - CharmCephOption( - 'password-policy-check-repetitive-chars', - 'set-pwd-policy-check-repetitive-chars-enabled'), - CharmCephOption( - 'password-policy-min-length', - 'set-pwd-policy-min-length'), - CharmCephOption( - 'password-policy-min-complexity', - 'set-pwd-policy-min-complexity'), - CharmCephOption( - 'audit-api-enabled', - 'set-audit-api-enabled'), - CharmCephOption( - 'audit-api-log-payload', - 'set-audit-api-log-payload'), - MOTDOption( - 'motd', - 'motd', - min_version='15.2.14') - ] + CHARM_TO_CEPH_OPTIONS = CharmCephOptionList().get() def __init__(self, *args) -> None: """Setup adapters and observers.""" super().__init__(*args) super().register_status_check(self.check_dashboard) self.framework.observe( - self.on.config_changed, - self._configure_dashboard) - self.mon = interface_dashboard.CephDashboardRequires( - self, - 'dashboard') - self.ca_client = ca_client.CAClient( - self, - 'certificates') + self.on.config_changed, self._configure_dashboard + ) + self.mon = interface_dashboard.CephDashboardRequires(self, "dashboard") self.radosgw_user = interface_radosgw_user.RadosGWUserRequires( - self, - 'radosgw-dashboard', - request_system_role=True) + self, "radosgw-dashboard", request_system_role=True + ) self.iscsi_user = admin_access.CephISCSIAdminAccessRequires( - self, - 'iscsi-dashboard') + self, "iscsi-dashboard" + ) self.framework.observe( - self.mon.on.mon_ready, - self._configure_dashboard) + self.mon.on.mon_ready, self._configure_dashboard + ) self.framework.observe( - self.ca_client.on.ca_available, - self._configure_dashboard) + self.radosgw_user.on.gw_user_ready, self._configure_dashboard + ) self.framework.observe( - self.ca_client.on.tls_server_config_ready, - self._configure_dashboard) - self.framework.observe( - self.radosgw_user.on.gw_user_ready, - self._configure_dashboard) - self.framework.observe( - self.iscsi_user.on.admin_access_ready, - self._configure_dashboard) + self.iscsi_user.on.admin_access_ready, self._configure_dashboard + ) self.framework.observe(self.on.add_user_action, self._add_user_action) self.framework.observe( - self.on.delete_user_action, - self._delete_user_action) + self.on.delete_user_action, self._delete_user_action + ) self.ingress = ops_lb_interface.OSLoadbalancerRequires( - self, - 'loadbalancer') - self.grafana_dashboard = \ - interface_grafana_dashboard.GrafanaDashboardProvides( - self, - 'grafana-dashboard') + self, "loadbalancer" + ) + self.grafana_dashboard = grafana_interface.GrafanaDashboardProvides( + self, "grafana-dashboard" + ) self.alertmanager = interface_http.HTTPRequires( - self, - 'alertmanager-service') - self.prometheus = interface_http.HTTPRequires( - self, - 'prometheus') + self, "alertmanager-service" + ) + self.prometheus = interface_http.HTTPRequires(self, "prometheus") self.framework.observe( - self.grafana_dashboard.on.dash_ready, - self._configure_dashboard) + self.grafana_dashboard.on.dash_ready, self._configure_dashboard + ) self.framework.observe( - self.alertmanager.on.http_ready, - self._configure_dashboard) + self.alertmanager.on.http_ready, self._configure_dashboard + ) self.framework.observe( - self.prometheus.on.http_ready, - self._configure_dashboard) + self.prometheus.on.http_ready, self._configure_dashboard + ) self.framework.observe( - self.ingress.on.lb_relation_ready, - self._request_loadbalancer) + self.ingress.on.lb_relation_ready, self._request_loadbalancer + ) self.framework.observe( - self.ingress.on.lb_configured, - self._configure_dashboard) + self.ingress.on.lb_configured, self._configure_dashboard + ) + + # Certificates Relation + self.ca_client = ca_client.CAClient(self, "certificates") + self.framework.observe( + self.ca_client.on.ca_available, self._request_certificates + ) + self.framework.observe( + self.ca_client.on.tls_server_config_ready, + self._enable_ssl_from_relation + ) + self.framework.observe( + self.on["certificates"].relation_departed, + self._certificates_relation_departed, + ) + + # Charm Custom Events + self.framework.observe(self.on.disable_ssl, self._clean_ssl_conf) + self.framework.observe( + self.on.enable_ssl_from_config, self._enable_ssl_from_config + ) + self._stored.set_default(is_started=False) - def _request_loadbalancer(self, _) -> None: + def _request_loadbalancer(self, _event) -> None: """Send request to create loadbalancer""" self.ingress.request_loadbalancer( self.LB_SERVICE_NAME, self.TLS_PORT, self.TLS_PORT, self._get_bind_ip(), - 'http') + 'http', + ) def _register_dashboards(self) -> None: """Register all dashboards with grafana""" + if not self.unit.is_leader(): + return # Do nothing on non leader units. + for dash_file in self.DASH_DIR.glob("*.json"): self.grafana_dashboard.register_dashboard( dash_file.stem, json.loads(dash_file.read_text())) - logging.info( + logging.debug( "register_grafana_dashboard: {}".format(dash_file)) def _update_legacy_radosgw_creds(self, access_key: str, @@ -285,11 +227,10 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): creds[0]['access_key'], creds[0]['secret_key']) - def request_certificates(self) -> None: + def _request_certificates(self, event) -> None: """Request TLS certificates.""" if not self.ca_client.is_joined: - logging.debug( - "Cannot request certificates, relation not present.") + logging.debug("Cannot request certificates, relation not present.") return addresses = set() if self.ingress.relations: @@ -302,6 +243,7 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): logging.debug( ("Defering certificate request until loadbalancer has " "responded.")) + event.defer() return for binding_name in ['public']: binding = self.model.get_binding(binding_name) @@ -358,45 +300,32 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): 'charm config')), (self._check_grafana_config, 'Charm config option grafana-api-url ' 'not set'), - (self._check_dashboard_responding, 'Dashboard not responding')] + (self._check_dashboard_responding, 'Dashboard not responding') + ] for check_f, msg in checks: if not check_f(): return BlockedStatus(msg) - return ActiveStatus() + + # Check if both relation based and config based certs are supplied. + return self._status_check_conflicting_ssl_sources() def kick_dashboard(self) -> None: """Disable and re-enable dashboard""" ceph_utils.mgr_disable_dashboard() ceph_utils.mgr_enable_dashboard() - def _run_cmd(self, cmd: List[str]) -> str: - """Run command in subprocess - - `cmd` The command to run - """ - try: - output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) - return output.decode('UTF-8') - except subprocess.CalledProcessError as exc: - logging.exception("Command failed: {}".format(exc.output)) - - def _apply_setting(self, ceph_setting: str, value: List[str]) -> str: - """Apply a dashboard setting""" - cmd = ['ceph', 'dashboard', ceph_setting] - cmd.extend(value) - return self._run_cmd(cmd) - - def _apply_file_setting(self, ceph_setting: str, - file_contents: str, - extra_args: List[str] = None) -> None: + def _apply_file_setting( + self, ceph_setting: str, file_contents: str, + extra_args: List[str] = None + ) -> None: """Apply a setting via a file""" - with tempfile.NamedTemporaryFile(mode='w', delete=True) as _file: + with tempfile.NamedTemporaryFile(mode="w", delete=True) as _file: _file.write(file_contents) _file.flush() - settings = ['-i', _file.name] + settings = ["-i", _file.name] if extra_args: settings.extend(extra_args) - self._apply_setting(ceph_setting, settings) + cmds.apply_setting(ceph_setting, settings) def _apply_ceph_config_from_charm_config(self) -> None: """Read charm config and apply settings to dashboard config""" @@ -409,18 +338,44 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): option.charm_option_name)) continue if option.is_supported(): - self._run_cmd(option.ceph_command(value)) + cmds.exec_option_ceph_cmd(option, value) else: logging.warning( "Skipping charm option {}, not supported".format( option.charm_option_name)) + def _configure_service_apis(self) -> None: + """Configure related service APIs in ceph dashboard""" + if self.unit.is_leader(): + grafana_ep = self.config.get("grafana-api-url") + if grafana_ep: + cmds.dashboard_set("set-grafana-api-url", grafana_ep) + + alertmanager_conn = self.alertmanager.get_service_ep_data() + if alertmanager_conn: + cmds.dashboard_set( + "set-alertmanager-api-host", + "http://{}:{}".format( + alertmanager_conn["hostname"], + alertmanager_conn["port"] + ), + ) + + prometheus_conn = self.prometheus.get_service_ep_data() + if prometheus_conn: + cmds.dashboard_set( + "set-prometheus-api-host", + "http://{}:{}".format( + prometheus_conn["hostname"], prometheus_conn["port"] + ), + ) + def _configure_dashboard(self, event) -> None: """Configure dashboard""" - self.request_certificates() if not self.mon.mons_ready: logging.info("Not configuring dashboard, mons not ready") return + if not ceph_utils.is_dashboard_enabled(): if self.unit.is_leader(): ceph_utils.mgr_enable_dashboard() @@ -428,34 +383,34 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): logging.info("Dashboard not enabled, deferring event.") return - self._apply_ceph_config_from_charm_config() - self._configure_tls() - ceph_utils.mgr_config_set( - 'mgr/dashboard/{hostname}/server_addr'.format( - hostname=socket.gethostname()), - str(self._get_bind_ip())) if self.unit.is_leader(): - grafana_ep = self.config.get('grafana-api-url') - if grafana_ep: - self._run_cmd([ - 'ceph', 'dashboard', 'set-grafana-api-url', grafana_ep]) - alertmanager_conn = self.alertmanager.get_service_ep_data() - if alertmanager_conn: - alertmanager_ep = 'http://{}:{}'.format( - alertmanager_conn['hostname'], - alertmanager_conn['port']) - self._run_cmd([ - 'ceph', 'dashboard', 'set-alertmanager-api-host', - alertmanager_ep]) - prometheus_conn = self.prometheus.get_service_ep_data() - if prometheus_conn: - prometheus_ep = 'http://{}:{}'.format( - prometheus_conn['hostname'], - prometheus_conn['port']) - self._run_cmd([ - 'ceph', 'dashboard', 'set-prometheus-api-host', - prometheus_ep]) - self._configure_saml() + # If charm config ssl is present. + if self._is_charm_ssl_from_config(): + if not cmds.check_ceph_dashboard_ssl_configured(): + # Configure SSL using charm config. + self.on.enable_ssl_from_config.emit() + else: # charm config is not present. + # Since certificates relation can provide unique certs to each + # unit, the below check should only be performed on leader as + # the central key/cert pair matches leader unit. + key, cert, _ = self._get_tls_from_relation() + if not self.is_ceph_dashboard_ssl_key_cert_same(key, cert): + # clean SSL if not configured using relation + self.on.disable_ssl.emit() + # apply charm config + self._apply_ceph_config_from_charm_config() + + self._configure_saml() + + ceph_utils.mgr_config_set( + "mgr/dashboard/{hostname}/server_addr".format( + hostname=socket.gethostname() + ), + str(self._get_bind_ip()), + ) + + # configure grafana, prometheus and alertmanager API endpoints + self._configure_service_apis() self._register_dashboards() self._manage_radosgw() @@ -468,6 +423,64 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): binding = self.model.get_binding('public') return str(binding.network.ingress_address) + def _clean_ssl_conf(self, _event) -> None: + """Clean ssl conf for ceph-dashboard.""" + + # NOTE: Clearing up of SSL key/cert is done centrally so that it can + # be performed with consistency for all units at once. + if self.unit.is_leader(): + # Disable ssl + cmds.ceph_config_set("config/mgr/mgr/dashboard/ssl", "false") + + config_keys = cmds.ceph_config_list() + for config in config_keys: + # clear all certificates. + if re.match("mgr/dashboard.*/crt", config): + cmds.ceph_config_reset(config) + # clear all keys. + if re.match("mgr/dashboard.*/key", config): + cmds.ceph_config_reset(config) + + def is_ceph_dashboard_ssl_key_cert_same( + self, key: str, cert: str, check_host: bool = False + ) -> Union[bool, None]: + """Checks if provided ssl key/cert match with configured key/cert. + + Since this method can result in falsy values even if the provided pair + is empty (None). It is advised to use this method for falsy checks + carefully. + + :returns: None if ssl is not configured or provided key/cert are empty. + """ + if not cmds.check_ceph_dashboard_ssl_configured(): + # Ceph Dashboard SSL not configured. + return None + + # Provided key/crt from param + if key is None or cert is None: + logger.debug("Empty key/cert pair : \n" + "Key %s, \nCerts: %s", (key is None), (cert is None)) + return None + + # Decode to ascii strings if bytes. + if isinstance(key, bytes): + key = key.decode() + if isinstance(cert, bytes): + cert = cert.decode() + + # Configured key/crt from ceph-dashboard + if not check_host: + ssl_key = cmds.get_ceph_dashboard_ssl_key() + ssl_crt = cmds.get_ceph_dashboard_ssl_crt() + else: + ssl_key = cmds.get_ceph_dashboard_host_ssl_key() + ssl_crt = cmds.get_ceph_dashboard_host_ssl_crt() + + if ssl_key == key and ssl_crt == cert: + return True + else: + return False + def _get_tls_from_config(self) -> TLS_Config: """Extract TLS config from charm config.""" raw_key = self.config.get("ssl_key") @@ -475,6 +488,7 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): raw_ca_cert = self.config.get("ssl_ca") if not (raw_key and raw_key): return None, None, None + key = base64.b64decode(raw_key) cert = base64.b64decode(raw_cert) if raw_ca_cert: @@ -483,8 +497,18 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): ca_cert = None return key, cert, ca_cert + def _is_relation_active(self, relation_name: str) -> bool: + """Check if any instance of the relation is present.""" + return any( + relation.id for relation in self.model.relations[relation_name] + ) + def _get_tls_from_relation(self) -> TLS_Config: - """Extract TLS config from certificatees relation.""" + """Extract TLS config from certificates relation.""" + # If 'certificates' relation is not present return None. + if not self._is_relation_active('certificates'): + return None, None, None + if not self.ca_client.is_server_cert_ready: return None, None, None key = self.ca_client.server_key.private_bytes( @@ -507,8 +531,8 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): root_ca_chain = bytes() ca_cert = ( self.ca_client.ca_certificate.public_bytes( - encoding=serialization.Encoding.PEM) + - root_ca_chain) + encoding=serialization.Encoding.PEM + ) + root_ca_chain) return key, cert, ca_cert def _update_iscsigw_creds(self, creds): @@ -533,20 +557,22 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): for c in creds: self._update_iscsigw_creds(c) - def _configure_tls(self) -> None: - """Configure TLS.""" - logging.debug("Attempting to collect TLS config from relation") - key, cert, ca_cert = self._get_tls_from_relation() - ca_cert_path = self.TLS_VAULT_CA_CERT_PATH - if not (key and cert): - logging.debug("Attempting to collect TLS config from charm " - "config") - key, cert, ca_cert = self._get_tls_from_config() - ca_cert_path = self.TLS_CHARM_CA_CERT_PATH - if not (key and cert): - logging.warn( - "Not configuring TLS, not all data present") - return + def _certificates_relation_departed(self, event) -> None: + """Certificates relation departed handle""" + if self.unit.is_leader(): + # Clear SSL if not configured using charm config. + # NOTE: Since certificates relation has departed, check has to be + # done using the charm config key/certs. + key, cert, _ = self._get_tls_from_config() + if not self.is_ceph_dashboard_ssl_key_cert_same(key, cert): + self._clean_ssl_conf(event) + + # Possible handover to charm-config SSL. + if self._is_charm_ssl_from_config(): + self.on.enable_ssl_from_config.emit() + + def _configure_tls(self, key, cert, ca_cert, ca_cert_path) -> None: + """Configure TLS using provided credentials""" self.TLS_KEY_PATH.write_bytes(key) self.TLS_CERT_PATH.write_bytes(cert) if ca_cert: @@ -576,21 +602,20 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): self.kick_dashboard() def _configure_saml(self) -> None: + if not self.unit.is_leader(): + logger.debug("Unit not leader, skipping saml config") + return + base_url = self.config.get('saml-base-url') idp_metadata = self.config.get('saml-idp-metadata') + username_attr = self.config.get('saml-username-attribute') + idp_entity_id = self.config.get('saml-idp-entity-id') if not base_url or not idp_metadata: return - cmd = ['ceph', 'dashboard', 'sso', 'setup', 'saml2', - base_url, idp_metadata] - username_attr = self.config.get('saml-username-attribute') - if username_attr: - cmd.append(username_attr) - idp_entity_id = self.config.get('saml-idp-entity-id') - if idp_entity_id: - cmd.append(idp_entity_id) - - self._run_cmd(cmd) + cmds.ceph_dashboard_config_saml( + base_url, idp_metadata, username_attr, idp_entity_id + ) def _gen_user_password(self, length: int = 12) -> str: """Generate a password""" @@ -607,12 +632,10 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): event.fail("Config missing") else: password = self._gen_user_password() - with tempfile.NamedTemporaryFile(mode='w', delete=True) as fp: + with tempfile.NamedTemporaryFile(mode="w", delete=True) as fp: fp.write(password) fp.flush() - cmd_out = subprocess.check_output([ - 'ceph', 'dashboard', 'ac-user-create', '--enabled', - '-i', fp.name, username, role]).decode('UTF-8') + cmd_out = cmds.ceph_dashboard_add_user(username, fp.name, role) if re.match('User.*already exists', cmd_out): event.fail("User already exists") else: @@ -622,11 +645,111 @@ class CephDashboardCharm(ops_openstack.core.OSBaseCharm): """Delete a user""" username = event.params["username"] try: - self._run_cmd(['ceph', 'dashboard', 'ac-user-delete', username]) + cmds.ceph_dashboard_delete_user(username) event.set_results({"message": "User {} deleted".format(username)}) except subprocess.CalledProcessError as exc: event.fail(exc.output) + def _is_charm_ssl_from_relation(self) -> bool: + """Check if ssl cert/key are provided by certificates relation.""" + key, cert, _ = self._get_tls_from_relation() + # True if both key and cert are present false otherwise. + return key and cert + + def _is_charm_ssl_from_config(self) -> bool: + """Check if ssl cert/key are configured in charm config.""" + key, cert, _ = self._get_tls_from_config() + # True if both key and cert are present false otherwise. + return key and cert + + def _is_charm_ssl_multiple_sources(self) -> bool: + """Check if SSL key/cert are available from multiple sources.""" + return self._is_charm_ssl_from_config() \ + and self._is_charm_ssl_from_relation() + + def _status_check_conflicting_ssl_sources(self): + """Generate status check message for multiple ssl key/cert scenario.""" + # If conflicting SSL source is not present + if not self._is_charm_ssl_multiple_sources(): + return ActiveStatus() + + # If both are waiting. + if not cmds.check_ceph_dashboard_ssl_configured(): + return BlockedStatus( + "Conflict: SSL configuration available from 'certificates' " + "relation and Charm config, refusing to guess. " + "Remove conflicting source to proceed." + ) + + key, cert, _ = self._get_tls_from_config() + if self.is_ceph_dashboard_ssl_key_cert_same(key, cert): + # SSL currently configured from charm config. + return BlockedStatus( + "Conflict: Active SSL from Charm config, 'certificates' " + "relation is ignored. Remove conflicting source to proceed." + ) + + key, cert, _ = self._get_tls_from_relation() + # 'Certificates' relation provides unique key/cert to each host. + # Hence cert check is performed for host. + if self.is_ceph_dashboard_ssl_key_cert_same( + key, cert, check_host=True + ): + # SSL currently configured from relation. + return BlockedStatus( + "Conflict: Active SSL from 'certificates' relation, Charm " + "config is ignored. Remove conflicting source to proceed." + ) + + return BlockedStatus("Unknown SSL source.") + + def _configure_tls_from_charm_config(self) -> None: + """Configure TLS using charm config values.""" + logging.debug("Attempting to collect TLS config from charm config") + key, cert, ca_cert = self._get_tls_from_config() + if not (key and cert): + logging.error("Not configuring, not all config data present") + return + + # Configure TLS + self._configure_tls(key, cert, ca_cert, self.TLS_CHARM_CA_CERT_PATH) + + def _configure_tls_from_relation(self) -> None: + """Configure TLS from certificates relation""" + logging.debug("Attempting to collect TLS config from relation") + key, cert, ca_cert = self._get_tls_from_relation() + if not (key and cert): + logging.error("Not configuring TLS, not all relation data present") + return + + # Configure TLS + self._configure_tls(key, cert, ca_cert, self.TLS_VAULT_CA_CERT_PATH) + + # Custom SSL Event Handles + def _enable_ssl_from_config(self, _event) -> None: + """Configure Ceph Dashboard SSL with available key/cert from charm.""" + if all([ + cmds.check_ceph_dashboard_ssl_configured(), + cmds.check_ceph_dashboard_ssl_configured(is_check_host_key=True) + ]): + # SSL is already configured for both central and host key/cert. + return + + self._configure_tls_from_charm_config() + + # Certificates relation handle. + def _enable_ssl_from_relation(self, event) -> None: + """Configure Ceph Dashboard SSL using key/cert from relation.""" + if cmds.check_ceph_dashboard_ssl_configured(): + key, cert, _ = self._get_tls_from_config() + if self.is_ceph_dashboard_ssl_key_cert_same(key, cert): + # Charm relation event deferred until conflicting charm config + # ssl is removed. Operator is informed through unit status. + event.defer() + return # SSL is already configured. + + self._configure_tls_from_relation() + if __name__ == "__main__": main(CephDashboardCharm) diff --git a/src/charm_option.py b/src/charm_option.py new file mode 100644 index 0000000..08f72af --- /dev/null +++ b/src/charm_option.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical +# See LICENSE file for licensing details. +# +# Learn more at: https://juju.is/docs/sdk + +import charmhelpers.core.host as ch_host +from typing import List, Union + + +class CharmCephOption(): + """Manage a charm option to ceph command to manage that option""" + + def __init__( + self, charm_option_name, ceph_option_name, min_version=None + ): + self.charm_option_name = charm_option_name + self.ceph_option_name = ceph_option_name + self.min_version = min_version + + def is_supported(self) -> bool: + """Is the option supported on this unit""" + if self.min_version: + return self.minimum_supported(self.min_version) + return True + + def minimum_supported(self, supported_version: str) -> bool: + """Check if installed Ceph release is >= to supported_version""" + return ch_host.cmp_pkgrevno('ceph-common', supported_version) >= 0 + + def convert_option(self, value: Union[bool, str, int]) -> List[str]: + """Convert a value to the corresponding value part of the ceph + dashboard command""" + return [str(value)] + + def ceph_command(self, value: List[str]) -> List[str]: + """Shell command to set option to desired value""" + cmd = ['ceph', 'dashboard', self.ceph_option_name] + cmd.extend(self.convert_option(value)) + return cmd + + +class DebugOption(CharmCephOption): + + def convert_option(self, value): + """Convert charm True/False to enable/disable""" + if value: + return ['enable'] + else: + return ['disable'] + + +class MOTDOption(CharmCephOption): + + def convert_option(self, value): + """Split motd charm option into ['severity', 'time', 'message']""" + if value: + return value.split('|') + else: + return ['clear'] + + +class CharmCephOptionList(): + def get(self) -> List: + """Get Charm options list""" + return [ + DebugOption('debug', 'debug'), + CharmCephOption( + 'enable-password-policy', + 'set-pwd-policy-enabled'), + CharmCephOption( + 'password-policy-check-length', + 'set-pwd-policy-check-length-enabled'), + CharmCephOption( + 'password-policy-check-oldpwd', + 'set-pwd-policy-check-oldpwd-enabled'), + CharmCephOption( + 'password-policy-check-username', + 'set-pwd-policy-check-username-enabled'), + CharmCephOption( + 'password-policy-check-exclusion-list', + 'set-pwd-policy-check-exclusion-list-enabled'), + CharmCephOption( + 'password-policy-check-complexity', + 'set-pwd-policy-check-complexity-enabled'), + CharmCephOption( + 'password-policy-check-sequential-chars', + 'set-pwd-policy-check-sequential-chars-enabled'), + CharmCephOption( + 'password-policy-check-repetitive-chars', + 'set-pwd-policy-check-repetitive-chars-enabled'), + CharmCephOption( + 'password-policy-min-length', + 'set-pwd-policy-min-length'), + CharmCephOption( + 'password-policy-min-complexity', + 'set-pwd-policy-min-complexity'), + CharmCephOption( + 'audit-api-enabled', + 'set-audit-api-enabled'), + CharmCephOption( + 'audit-api-log-payload', + 'set-audit-api-log-payload'), + MOTDOption( + 'motd', + 'motd', + min_version='15.2.14') + ] diff --git a/tests/bundles/focal-yoga.yaml b/tests/bundles/focal-yoga.yaml index 4fea677..7d7d1bf 100644 --- a/tests/bundles/focal-yoga.yaml +++ b/tests/bundles/focal-yoga.yaml @@ -22,7 +22,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/tests/bundles/focal.yaml b/tests/bundles/focal.yaml index 3c3cde6..fedc701 100644 --- a/tests/bundles/focal.yaml +++ b/tests/bundles/focal.yaml @@ -18,7 +18,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/tests/bundles/jammy-antelope.yaml b/tests/bundles/jammy-antelope.yaml index b8def79..4263cbe 100644 --- a/tests/bundles/jammy-antelope.yaml +++ b/tests/bundles/jammy-antelope.yaml @@ -20,7 +20,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/tests/bundles/jammy-bobcat.yaml b/tests/bundles/jammy-bobcat.yaml index 42462d1..62c023f 100644 --- a/tests/bundles/jammy-bobcat.yaml +++ b/tests/bundles/jammy-bobcat.yaml @@ -20,7 +20,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/tests/bundles/jammy-yoga.yaml b/tests/bundles/jammy-yoga.yaml index dd81eb9..5a38b13 100644 --- a/tests/bundles/jammy-yoga.yaml +++ b/tests/bundles/jammy-yoga.yaml @@ -23,7 +23,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/tests/bundles/lunar-antelope.yaml b/tests/bundles/lunar-antelope.yaml index fcfa66d..160e683 100644 --- a/tests/bundles/lunar-antelope.yaml +++ b/tests/bundles/lunar-antelope.yaml @@ -22,7 +22,8 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable + series: jammy mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M @@ -31,6 +32,7 @@ applications: vault-mysql-router: charm: ch:mysql-router channel: latest/edge + series: jammy ceph-dashboard: charm: ../../ceph-dashboard.charm options: diff --git a/tests/bundles/mantic-bobcat.yaml b/tests/bundles/mantic-bobcat.yaml index 2f4d4f9..bed13e0 100644 --- a/tests/bundles/mantic-bobcat.yaml +++ b/tests/bundles/mantic-bobcat.yaml @@ -22,7 +22,7 @@ applications: vault: num_units: 1 charm: ch:vault - channel: latest/edge + channel: 1.8/stable mysql-innodb-cluster: charm: ch:mysql-innodb-cluster constraints: mem=3072M diff --git a/unit_tests/test_ceph_dashboard_charm.py b/unit_tests/test_ceph_dashboard_charm.py index 96c749c..904b861 100644 --- a/unit_tests/test_ceph_dashboard_charm.py +++ b/unit_tests/test_ceph_dashboard_charm.py @@ -151,6 +151,12 @@ class _CephDashboardCharm(charm.CephDashboardCharm): def _get_bind_ip(self): return '10.0.0.10' + def _clean_ssl_conf(self, _event): + return # empty stub + + def _is_relation_active(self, _event): + return True + class TestCephDashboardCharmBase(CharmTestCase): @@ -158,7 +164,7 @@ class TestCephDashboardCharmBase(CharmTestCase): 'ceph_utils', 'ch_host', 'socket', - 'subprocess', + 'subprocess', # charm's subprocess import 'ch_host', ] @@ -207,9 +213,11 @@ class TestCephDashboardCharmBase(CharmTestCase): self.harness.begin() self.assertFalse(self.harness.charm._stored.is_started) - def test_charm_config(self): + @patch('ceph_dashboard_commands.subprocess') + @patch('charm_option.ch_host') + def test_charm_config(self, option_ch_host, subprocess): self.ceph_utils.is_dashboard_enabled.return_value = True - self.ch_host.cmp_pkgrevno.return_value = 0 + option_ch_host.cmp_pkgrevno.return_value = 0 basic_boolean = [ ('enable-password-policy', 'set-pwd-policy-enabled'), ('password-policy-check-length', @@ -254,13 +262,16 @@ class TestCephDashboardCharmBase(CharmTestCase): { 'mon-ready': 'True'}) _harness.begin() + _harness.set_leader(True) + _harness.charm.is_ceph_dashboard_ssl_key_cert_same \ + = lambda *_: True expected_cmd = base_cmd + expected_options - self.subprocess.check_output.reset_mock() + subprocess.check_output.reset_mock() _harness.update_config( key_values={charm_option: charm_value}) - self.subprocess.check_output.assert_called_once_with( + subprocess.check_output.assert_called_once_with( expected_cmd, - stderr=self.subprocess.STDOUT) + stderr=subprocess.STDOUT) def test__on_ca_available(self): rel_id = self.harness.add_relation('certificates', 'vault') @@ -332,7 +343,8 @@ class TestCephDashboardCharmBase(CharmTestCase): self.ceph_utils.mgr_disable_dashboard.assert_called_once_with() self.ceph_utils.mgr_enable_dashboard.assert_called_once_with() - def test__configure_dashboard(self): + @patch('ceph_dashboard_commands.subprocess') + def test_configure_dashboard(self, subprocess): self.ceph_utils.is_dashboard_enabled.return_value = True rel_id = self.harness.add_relation('dashboard', 'ceph-mon') self.harness.begin() @@ -377,9 +389,11 @@ class TestCephDashboardCharmBase(CharmTestCase): self.harness.charm._get_bind_ip(), '10.0.0.10') + @patch('ceph_dashboard_commands.check_ceph_dashboard_ssl_configured') @patch('socket.gethostname') - def test_certificates_relation(self, _gethostname): + def test_certificates_relation(self, _gethostname, ssl_configured): self.ceph_utils.is_dashboard_enabled.return_value = True + ssl_configured.return_value = False mock_TLS_KEY_PATH = MagicMock() mock_TLS_CERT_PATH = MagicMock() mock_TLS_VAULT_CA_CERT_PATH = MagicMock() @@ -434,6 +448,8 @@ class TestCephDashboardCharmBase(CharmTestCase): 'ip': ['10.10.0.101'], 'port': 8443, 'protocol': 'http'}}})}) + # Reemit deferred events. + self.harness.framework.reemit() self.assertNotEqual( self.harness.get_relation_data( cert_rel_id, @@ -464,8 +480,10 @@ class TestCephDashboardCharmBase(CharmTestCase): self.ceph_utils.mgr_disable_dashboard.assert_called_once_with() self.ceph_utils.mgr_enable_dashboard.assert_called_once_with() - def test_certificates_from_config(self): + @patch('ceph_dashboard_commands.check_ceph_dashboard_ssl_configured') + def test_certificates_from_config(self, ssl_configured): self.ceph_utils.is_dashboard_enabled.return_value = True + ssl_configured.return_value = False mock_TLS_KEY_PATH = MagicMock() mock_TLS_CERT_PATH = MagicMock() mock_TLS_CHARM_CA_CERT_PATH = MagicMock() @@ -503,7 +521,8 @@ class TestCephDashboardCharmBase(CharmTestCase): self.ceph_utils.mgr_disable_dashboard.assert_called_once_with() self.ceph_utils.mgr_enable_dashboard.assert_called_once_with() - def test_rados_gateway(self): + @patch('ceph_dashboard_commands.subprocess') + def test_rados_gateway(self, subprocess): self.ceph_utils.is_dashboard_enabled.return_value = True self.ch_host.cmp_pkgrevno.return_value = 1 mon_rel_id = self.harness.add_relation('dashboard', 'ceph-mon') @@ -541,16 +560,17 @@ class TestCephDashboardCharmBase(CharmTestCase): 'access-key': 'XNUZVPL364U0BL1OXWJZ', 'secret-key': 'SgBo115xJcW90nkQ5EaNQ6fPeyeUUT0GxhwQbLFo', 'uid': 'radosgw-user-9'}) - self.subprocess.check_output.assert_has_calls([ + subprocess.check_output.assert_has_calls([ call(['ceph', 'dashboard', 'set-rgw-api-access-key', '-i', ANY], - stderr=self.subprocess.STDOUT), + stderr=subprocess.STDOUT), call().decode('UTF-8'), call(['ceph', 'dashboard', 'set-rgw-api-secret-key', '-i', ANY], - stderr=self.subprocess.STDOUT), + stderr=subprocess.STDOUT), call().decode('UTF-8'), ]) - def test_rados_gateway_multi_relations_pacific(self): + @patch('ceph_dashboard_commands.subprocess') + def test_rados_gateway_multi_relations_pacific(self, subprocess): self.ceph_utils.is_dashboard_enabled.return_value = True self.ch_host.cmp_pkgrevno.return_value = 1 rel_id1 = self.harness.add_relation('radosgw-dashboard', 'ceph-eu') @@ -589,7 +609,7 @@ class TestCephDashboardCharmBase(CharmTestCase): 'access-key': 'XNUZVPL364U0BL1OXWJZ', 'secret-key': 'SgBo115xJcW90nkQ5EaNQ6fPeyeUUT0GxhwQbLFo', 'uid': 'radosgw-user-9'}) - self.subprocess.check_output.reset_mock() + subprocess.check_output.reset_mock() self.harness.update_relation_data( rel_id2, 'ceph-us', @@ -597,16 +617,17 @@ class TestCephDashboardCharmBase(CharmTestCase): 'access-key': 'JGHKJGDKJGJGJHGYYYYM', 'secret-key': 'iljkdfhHKHKd88LKxNLSKDiijfjfjfldjfjlf44', 'uid': 'radosgw-user-10'}) - self.subprocess.check_output.assert_has_calls([ + subprocess.check_output.assert_has_calls([ call(['ceph', 'dashboard', 'set-rgw-api-access-key', '-i', ANY], - stderr=self.subprocess.STDOUT), + stderr=subprocess.STDOUT), call().decode('UTF-8'), call(['ceph', 'dashboard', 'set-rgw-api-secret-key', '-i', ANY], - stderr=self.subprocess.STDOUT), + stderr=subprocess.STDOUT), call().decode('UTF-8'), ]) - def test_rados_gateway_multi_relations_octopus(self): + @patch('ceph_dashboard_commands.subprocess') + def test_rados_gateway_multi_relations_octopus(self, subprocess): self.ch_host.cmp_pkgrevno.return_value = -1 rel_id1 = self.harness.add_relation('radosgw-dashboard', 'ceph-eu') rel_id2 = self.harness.add_relation('radosgw-dashboard', 'ceph-us') @@ -635,7 +656,7 @@ class TestCephDashboardCharmBase(CharmTestCase): 'access-key': 'XNUZVPL364U0BL1OXWJZ', 'secret-key': 'SgBo115xJcW90nkQ5EaNQ6fPeyeUUT0GxhwQbLFo', 'uid': 'radosgw-user-9'}) - self.subprocess.check_output.reset_mock() + subprocess.check_output.reset_mock() self.harness.update_relation_data( rel_id2, 'ceph-us', @@ -643,7 +664,7 @@ class TestCephDashboardCharmBase(CharmTestCase): 'access-key': 'JGHKJGDKJGJGJHGYYYYM', 'secret-key': 'iljkdfhHKHKd88LKxNLSKDiijfjfjfldjfjlf44', 'uid': 'radosgw-user-10'}) - self.assertFalse(self.subprocess.check_output.called) + self.assertFalse(subprocess.check_output.called) @patch.object(charm.secrets, 'choice') def test__gen_user_password(self, _choice): @@ -653,10 +674,11 @@ class TestCephDashboardCharmBase(CharmTestCase): self.harness.charm._gen_user_password(), 'rrrrrrrrrrrr') + @patch('ceph_dashboard_commands.subprocess') @patch.object(charm.tempfile, 'NamedTemporaryFile') @patch.object(charm.secrets, 'choice') - def test__add_user_action(self, _choice, _NTFile): - self.subprocess.check_output.return_value = b'' + def test_add_user_action(self, _choice, _NTFile, subprocess): + subprocess.check_output.return_value = b'Byte String' _NTFile.return_value.__enter__.return_value.name = 'tempfilename' _choice.return_value = 'r' self.harness.begin() @@ -665,27 +687,31 @@ class TestCephDashboardCharmBase(CharmTestCase): 'username': 'auser', 'role': 'administrator'} self.harness.charm._add_user_action(action_event) - self.subprocess.check_output.assert_called_once_with( - ['ceph', 'dashboard', 'ac-user-create', '--enabled', - '-i', 'tempfilename', 'auser', 'administrator']) + subprocess.check_output.assert_called_once_with( + ['ceph', 'dashboard', 'ac-user-create', '--enabled', '-i', + 'tempfilename', 'auser', 'administrator'], + stderr=subprocess.STDOUT + ) - def test__delete_user_action(self): - self.subprocess.check_output.return_value = b'' + @patch('ceph_dashboard_commands.subprocess') + def test__delete_user_action(self, subprocess): + subprocess.check_output.return_value = b'' self.harness.begin() action_event = MagicMock() action_event.params = { 'username': 'auser'} self.harness.charm._delete_user_action(action_event) - self.subprocess.check_output.assert_called_once_with( + subprocess.check_output.assert_called_once_with( ['ceph', 'dashboard', 'ac-user-delete', 'auser'], - stderr=self.subprocess.STDOUT) + stderr=subprocess.STDOUT) - def test_saml(self): - self.subprocess.check_output.return_value = b'' + @patch('ceph_dashboard_commands.subprocess') + def test_saml(self, subprocess): + subprocess.check_output.return_value = b'' self.harness.begin() self.harness.charm.PACKAGES.append('python3-onelogin-saml2') self.harness.charm._configure_saml() - self.subprocess.check_output.assert_not_called() + subprocess.check_output.assert_not_called() base_url = 'https://saml-base' idp_meta = 'file://idp.xml' @@ -701,8 +727,9 @@ class TestCephDashboardCharmBase(CharmTestCase): } ) + self.harness.set_leader() self.harness.charm._configure_saml() - self.subprocess.check_output.assert_called_with( + subprocess.check_output.assert_called_with( ['ceph', 'dashboard', 'sso', 'setup', 'saml2', base_url, idp_meta, username_attr, entity_id], stderr=ANY