Tom Haddon has proposed merging ~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress:admin-password-action into charm-k8s-wordpress:master.
Commit message: Add an action to retrieve initial password Requested reviews: Canonical IS Reviewers (canonical-is-reviewers) Wordpress Charmers (wordpress-charmers) Related bugs: Bug #1907063 in charm-k8s-wordpress: "Add a juju action to retrieve admin password" https://bugs.launchpad.net/charm-k8s-wordpress/+bug/1907063 For more details, see: https://code.launchpad.net/~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress/+merge/395982 Add an action to retrieve initial password -- Your team Wordpress Charmers is requested to review the proposed merge of ~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress:admin-password-action into charm-k8s-wordpress:master.
diff --git a/README.md b/README.md index 886c92a..bd66985 100644 --- a/README.md +++ b/README.md @@ -73,9 +73,9 @@ and perform the initial setup for you. Look for this line in the output of This is due to [issue #166](https://github.com/canonical/operator/issues/166) and will be fixed once Juju supports a Kubernetes pod ready hook. -To retrieve the random admin password, run the following (until [LP#1907063](https://bugs.launchpad.net/charm-k8s-wordpress/+bug/1907063) is addressed): +To retrieve the auto-generated admin password, run the following: - microk8s.kubectl exec -ti -n wordpress wordpress-operator-0 -- cat /root/initial.passwd + juju run-action --wait wordpress/0 get-initial-password You should now be able to browse to https://myblog.example.com/wp-admin. diff --git a/actions.yaml b/actions.yaml new file mode 100644 index 0000000..796aa4e --- /dev/null +++ b/actions.yaml @@ -0,0 +1,2 @@ +get-initial-password: + description: Retrieve the auto-generated initial password. diff --git a/src/charm.py b/src/charm.py index edade47..1c2f1de 100755 --- a/src/charm.py +++ b/src/charm.py @@ -12,6 +12,7 @@ from ops.charm import CharmBase, CharmEvents from ops.framework import EventBase, EventSource, StoredState from ops.main import main from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus +from leadership import LeadershipSettings logger = logging.getLogger() @@ -110,11 +111,16 @@ class WordpressCharm(CharmBase): def __init__(self, *args): super().__init__(*args) + self.leader_data = LeadershipSettings() + self.framework.observe(self.on.start, self.on_config_changed) self.framework.observe(self.on.config_changed, self.on_config_changed) self.framework.observe(self.on.update_status, self.on_config_changed) self.framework.observe(self.on.wordpress_initialise, self.on_wordpress_initialise) + # Actions. + self.framework.observe(self.on.get_initial_password_action, self._on_get_initial_password_action) + self.state.set_default( initialised=False, valid=False, ) @@ -146,7 +152,8 @@ class WordpressCharm(CharmBase): msg = "Wordpress needs configuration" logger.info(msg) self.model.unit.status = MaintenanceStatus(msg) - installed = self.wordpress.first_install(self.get_service_ip()) + initial_password = self._get_initial_password() + installed = self.wordpress.first_install(self.get_service_ip(), initial_password) if not installed: msg = "Failed to configure wordpress" logger.info(msg) @@ -287,6 +294,26 @@ class WordpressCharm(CharmBase): return self.wordpress.is_vhost_ready(service_ip) return False + def _get_initial_password(self): + """Get the initial password. + + If a password hasn't been set yet, create one if we're the leader, + or return an empty string if we're not.""" + initial_password = self.leader_data["initial_password"] + if not initial_password: + if self.unit.is_leader: + initial_password = password_generator() + self.leader_data["initial_password"] = initial_password + return initial_password + + def _on_get_initial_password_action(self, event): + """Handle the get-initial-password action.""" + initial_password = self._get_initial_password() + if initial_password: + event.set_results({"password": initial_password}) + else: + event.fail("Initial password has not been set yet.") + if __name__ == "__main__": main(WordpressCharm) diff --git a/src/leadership.py b/src/leadership.py new file mode 100644 index 0000000..8e88779 --- /dev/null +++ b/src/leadership.py @@ -0,0 +1,218 @@ +# This file is part of the PostgreSQL k8s Charm for Juju. +# Copyright 2020 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranties of +# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR +# PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# TODO: Most of all of this module should move into the Operator Framework core + +import collections.abc +import subprocess +from typing import Any, Iterable, Dict, MutableMapping, Protocol + +import ops +import yaml + + +class _Codec(Protocol): + def encode(self, value: Any) -> str: + raise NotImplementedError("encode") + + def decode(self, key: str, value: str) -> Any: + raise NotImplementedError("decode") + + +class _ObjectABCMeta(type(ops.framework.Object), type(collections.abc.MutableMapping)): + """This metaclass can go once the Operator Framework drops Python 3.5 support. + + Per ops.framework._Metaclass docstring. + """ + + pass + + +class _PeerData(ops.framework.Object, collections.abc.MutableMapping, metaclass=_ObjectABCMeta): + """A bag of data shared between peer units. + + Only the leader can set data. All peer units can read. + """ + + def __init__(self, parent: ops.framework.Object, key: str, _store: MutableMapping, _codec: _Codec): + super().__init__(parent, key) + self._store = _store + self._codec = _codec + self._prefix = self.handle.path + + def _prefixed_key(self, key: str) -> str: + return self._prefix + "/" + key + + def __getitem__(self, key: str) -> Any: + if not isinstance(key, str): + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") + raw = self._store[self._prefixed_key(key)] + return self._codec.decode(key, raw) + + def __setitem__(self, key: str, value: Any) -> None: + if not isinstance(key, str): + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") + if not self.model.unit.is_leader(): + raise RuntimeError("non-leader attempting to set peer data") + self._store[self._prefixed_key(key)] = self._codec.encode(value) + + def __delitem__(self, key: str) -> None: + if not isinstance(key, str): + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") + if not self.model.unit.is_leader(): + raise RuntimeError("non-leader attempting to set peer data") + del self._store[self._prefixed_key(key)] + + def __iter__(self) -> Iterable[str]: + return iter(self._store) + + def __len__(self) -> int: + return len(self._store) + + +class LegacyLeaderData(_PeerData): + """Raw Juju Leadership settings, a bag of data shared between peers. + + Only the leader can set data. All peer units can read. + + Behavior matches the Juju leader-get and leader-set tools; keys and + values must be strings, Setting an value to the empty string is the + same as deleting the entry, and accessing a missing entry will + return an empty string. + + This class provides access to legacy Juju Leadership data, and namespace + collisions may occur if multiple components attempt to use the same key. + """ + + def __init__(self, parent, key=""): + super().__init__(parent, key, LeadershipSettings(), _RawCodec()) + + def _prefixed_key(self, key: str) -> str: + return key + + +class RawLeaderData(_PeerData): + """Raw Juju Leadership settings, a bag of data shared between peers. + + Only the leader can set data. All peer units can read. + + Behavior matches the Juju leader-get and leader-set tools; keys and + values must be strings, Setting an value to the empty string is the + same as deleting the entry, and accessing a missing entry will + return an empty string. + + Keys are automatically prefixed to avoid namespace collisions in the + Juju Leadership settings. + """ + + def __init__(self, parent, key=""): + super().__init__(parent, key, LeadershipSettings(), _RawCodec()) + + +class RichLeaderData(_PeerData): + """Encoded Juju Leadership settings, a bag of data shared between peers. + + Only the leader can set data. All peer units can read. + + Operates as a standard Python MutableMapping. Keys must be strings. + Values may be anything that the yaml library can marshal. + + Keys are automatically prefixed to avoid namespace collisions in the + Juju Leadership settings. + """ + + def __init__(self, parent, key=""): + super().__init__(parent, key, LeadershipSettings(), _YAMLCodec()) + + +class _YAMLCodec(object): + def encode(self, value: Any) -> str: + return yaml.safe_dump(value) + + def decode(self, key: str, value: str) -> Any: + if not value: + # Key never existed or was deleted. If set to + # empty string or none, value will contain + # the YAML representation. + raise KeyError(key) + return yaml.safe_load(value) + + +class _RawCodec(object): + def encode(self, value: Any) -> str: + if not isinstance(value, str): + raise TypeError(f"{self.__class__.__name__} only supports str values, got {type(value)}") + return value + + def decode(self, value: str) -> Any: + return value + + +class LeadershipSettings(collections.abc.MutableMapping): + """Juju Leadership Settings data. + + This class provides direct access to the Juju Leadership Settings, + a bag of data shared between peer units. Only the leader can set + items. Keys all share the same namespace, so beware of collisions. + + This MutableMapping implements Juju behavior. Only strings are + supported as keys and values. Deleting an entry is the same as + setting it to the empty string. Attempting to read a missing + key will return the empty string (this class will never raise + a KeyError). + """ + + __cls_cache = None + + @property + def _cache_loaded(self) -> bool: + return self.__class__.__cls_cache is not None + + @property + def _cache(self) -> Dict[str, str]: + # There might be multiple instances of LeadershipSettings, but + # the backend is shared, so the cache needs to be a class + # attribute. + cls = self.__class__ + if cls.__cls_cache is None: + cmd = ["leader-get", "--format=yaml"] + cls.__cls_cache = yaml.safe_load(subprocess.check_output(cmd).decode("UTF-8")) or {} + return cls.__cls_cache + + def __getitem__(self, key: str) -> str: + return self._cache.get(key, "") + + def __setitem__(self, key: str, value: str): + if "=" in key: + # Leave other validation to the leader-set tool + raise RuntimeError(f"LeadershipSettings keys may not contain '=', got {key}") + if value is None: + value = "" + cmd = ["leader-set", f"{key}={value}"] + subprocess.check_call(cmd) + if self._cache_loaded: + if value == "": + del self._cache[key] + else: + self._cache[key] = value + + def __delitem__(self, key: str): + self[key] = "" + + def __iter__(self) -> Iterable[str]: + return iter(self._cache) + + def __len__(self) -> int: + return len(self._cache) diff --git a/src/wordpress.py b/src/wordpress.py index a3f586e..9ac5336 100644 --- a/src/wordpress.py +++ b/src/wordpress.py @@ -44,15 +44,10 @@ class Wordpress: def __init__(self, model_config): self.model_config = model_config - def _write_initial_password(self, password, filepath): - with open(filepath, "w") as f: - f.write(password) - - def first_install(self, service_ip): + def first_install(self, service_ip, admin_password): """Perform initial configuration of wordpress if needed.""" config = self.model_config logger.info("Starting wordpress initial configuration") - admin_password = password_generator() payload = { "admin_password": admin_password, "blog_public": "checked", @@ -61,12 +56,6 @@ class Wordpress: payload.update(safe_load(config["initial_settings"])) payload["admin_password2"] = payload["admin_password"] - # Ideally we would store this in state however juju run-action does not - # currently support being run inside the operator pod which means the - # StorageState will be split between workload and operator. - # https://bugs.launchpad.net/juju/+bug/1870487 - self._write_initial_password(payload["admin_password"], "/root/initial.passwd") - if not payload["blog_public"]: payload["blog_public"] = "unchecked" required_config = set(("user_name", "admin_email")) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1840a4c..29d8f81 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -4,6 +4,8 @@ import copy import mock import unittest +from unittest.mock import Mock + from charm import WordpressCharm, create_wordpress_secrets, gather_wordpress_secrets from wordpress import WORDPRESS_SECRETS from ops import testing @@ -124,3 +126,16 @@ class TestWordpressCharm(unittest.TestCase): } } self.assertEqual(self.harness.charm.make_pod_resources(), expected) + + def test_on_get_initial_password_action(self): + action_event = Mock() + # First test with no initial password set. + with mock.patch.object(self.harness.charm, "_get_initial_password") as get_initial_password: + get_initial_password.return_value = "" + self.harness.charm._on_get_initial_password_action(action_event) + self.assertEqual(action_event.fail.call_args, mock.call("Initial password has not been set yet.")) + # Now test with initial password set. + with mock.patch.object(self.harness.charm, "_get_initial_password") as get_initial_password: + get_initial_password.return_value = "passwd" + self.harness.charm._on_get_initial_password_action(action_event) + self.assertEqual(action_event.set_results.call_args, mock.call({"password": "passwd"})) diff --git a/tests/unit/test_wordpress.py b/tests/unit/test_wordpress.py index f204cac..92712e7 100644 --- a/tests/unit/test_wordpress.py +++ b/tests/unit/test_wordpress.py @@ -94,13 +94,10 @@ class WordpressTest(unittest.TestCase): def test__init__(self): self.assertEqual(self.test_wordpress.model_config, self.test_model_config) - @mock.patch("wordpress.password_generator", side_effect=dummy_password_generator) - def test_first_install(self, password_generator_func): + def test_first_install(self): mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=True) - mocked__write_initial_password = mock.MagicMock(name="_write_initial_password", return_value=None) mocked_wordpress_configured = mock.MagicMock(name="wordpress_configured", return_value=True) self.test_wordpress.call_wordpress = mocked_call_wordpress - self.test_wordpress._write_initial_password = mocked__write_initial_password self.test_wordpress.wordpress_configured = mocked_wordpress_configured test_payload = { @@ -112,10 +109,7 @@ class WordpressTest(unittest.TestCase): 'admin_email': 'r...@admin.canonical.com', 'weblog_title': 'Test Blog', } - self.test_wordpress.first_install(self.test_service_ip) - - # Test that we wrote initial admin credentials inside the operator pod. - self.test_wordpress._write_initial_password.assert_called_with(TEST_GENERATED_PASSWORD, "/root/initial.passwd") + self.test_wordpress.first_install(self.test_service_ip, TEST_GENERATED_PASSWORD) # Test that we POST'd our initial configuration options to the wordpress API. self.test_wordpress.call_wordpress.assert_called_with( @@ -128,7 +122,7 @@ class WordpressTest(unittest.TestCase): self.test_wordpress.model_config["initial_settings"] = ( "user_name: admin\n" "weblog_title: Test Blog\n" "blog_public: False" ) - self.test_wordpress.first_install(self.test_service_ip) + self.test_wordpress.first_install(self.test_service_ip, TEST_GENERATED_PASSWORD) self.test_wordpress.call_wordpress.assert_not_called() def test_wordpress_configured(self):
-- Mailing list: https://launchpad.net/~wordpress-charmers Post to : wordpress-charmers@lists.launchpad.net Unsubscribe : https://launchpad.net/~wordpress-charmers More help : https://help.launchpad.net/ListHelp
