Merge ~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress:admin-password-action into charm-k8s-wordpress:master
- Git
- lp:~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress
- admin-password-action
- Merge into master
Proposed by
Tom Haddon
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Tom Haddon | ||||
Approved revision: | 0b3c4643c8405091235a442e7ea3018c544a811e | ||||
Merged at revision: | f71d95f061c0430f3d8f1687eef5860db7b8bdfc | ||||
Proposed branch: | ~mthaddon/charm-k8s-wordpress/+git/charm-k8s-wordpress:admin-password-action | ||||
Merge into: | charm-k8s-wordpress:master | ||||
Diff against target: |
426 lines (+269/-25) 7 files modified
README.md (+2/-2) actions.yaml (+2/-0) src/charm.py (+28/-1) src/leadership.py (+218/-0) src/wordpress.py (+1/-12) tests/unit/test_charm.py (+15/-1) tests/unit/test_wordpress.py (+3/-9) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Stuart Bishop (community) | Approve | ||
Wordpress Charmers | Pending | ||
Review via email: mp+395982@code.launchpad.net |
Commit message
Add an action to retrieve initial password
Description of the change
Add an action to retrieve initial password
To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Revision history for this message
Tom Haddon (mthaddon) wrote : | # |
There's actually a problem with this. We need to use leadership data (or a peer relation) since StoredState is per unit.
Revision history for this message
Tom Haddon (mthaddon) wrote : | # |
This action can now be run from any unit.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Failed to merge change (unable to merge source repository due to conflicts), setting status to needs review.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Failed to merge change (unable to merge source repository due to conflicts), setting status to needs review.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Change successfully merged at revision f71d95f061c0430
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/README.md b/README.md |
2 | index 886c92a..bd66985 100644 |
3 | --- a/README.md |
4 | +++ b/README.md |
5 | @@ -73,9 +73,9 @@ and perform the initial setup for you. Look for this line in the output of |
6 | This is due to [issue #166](https://github.com/canonical/operator/issues/166) and will be fixed once Juju supports a Kubernetes |
7 | pod ready hook. |
8 | |
9 | -To retrieve the random admin password, run the following (until [LP#1907063](https://bugs.launchpad.net/charm-k8s-wordpress/+bug/1907063) is addressed): |
10 | +To retrieve the auto-generated admin password, run the following: |
11 | |
12 | - microk8s.kubectl exec -ti -n wordpress wordpress-operator-0 -- cat /root/initial.passwd |
13 | + juju run-action --wait wordpress/0 get-initial-password |
14 | |
15 | You should now be able to browse to https://myblog.example.com/wp-admin. |
16 | |
17 | diff --git a/actions.yaml b/actions.yaml |
18 | new file mode 100644 |
19 | index 0000000..796aa4e |
20 | --- /dev/null |
21 | +++ b/actions.yaml |
22 | @@ -0,0 +1,2 @@ |
23 | +get-initial-password: |
24 | + description: Retrieve the auto-generated initial password. |
25 | diff --git a/src/charm.py b/src/charm.py |
26 | index 8abcaad..f1a2252 100755 |
27 | --- a/src/charm.py |
28 | +++ b/src/charm.py |
29 | @@ -10,6 +10,7 @@ from ops.charm import CharmBase, CharmEvents |
30 | from ops.framework import EventBase, EventSource, StoredState |
31 | from ops.main import main |
32 | from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus |
33 | +from leadership import LeadershipSettings |
34 | |
35 | from mysql import MySQLClient |
36 | from wordpress import Wordpress, password_generator, WORDPRESS_SECRETS |
37 | @@ -112,11 +113,16 @@ class WordpressCharm(CharmBase): |
38 | def __init__(self, *args): |
39 | super().__init__(*args) |
40 | |
41 | + self.leader_data = LeadershipSettings() |
42 | + |
43 | self.framework.observe(self.on.start, self.on_config_changed) |
44 | self.framework.observe(self.on.config_changed, self.on_config_changed) |
45 | self.framework.observe(self.on.update_status, self.on_config_changed) |
46 | self.framework.observe(self.on.wordpress_initialise, self.on_wordpress_initialise) |
47 | |
48 | + # Actions. |
49 | + self.framework.observe(self.on.get_initial_password_action, self._on_get_initial_password_action) |
50 | + |
51 | self.db = MySQLClient(self, 'db') |
52 | self.framework.observe(self.on.db_relation_created, self.on_db_relation_created) |
53 | self.framework.observe(self.on.db_relation_broken, self.on_db_relation_broken) |
54 | @@ -149,7 +155,8 @@ class WordpressCharm(CharmBase): |
55 | msg = "Wordpress needs configuration" |
56 | logger.info(msg) |
57 | self.model.unit.status = MaintenanceStatus(msg) |
58 | - installed = self.wordpress.first_install(self.get_service_ip()) |
59 | + initial_password = self._get_initial_password() |
60 | + installed = self.wordpress.first_install(self.get_service_ip(), initial_password) |
61 | if not installed: |
62 | msg = "Failed to configure wordpress" |
63 | logger.info(msg) |
64 | @@ -366,6 +373,26 @@ class WordpressCharm(CharmBase): |
65 | return self.wordpress.is_vhost_ready(service_ip) |
66 | return False |
67 | |
68 | + def _get_initial_password(self): |
69 | + """Get the initial password. |
70 | + |
71 | + If a password hasn't been set yet, create one if we're the leader, |
72 | + or return an empty string if we're not.""" |
73 | + initial_password = self.leader_data["initial_password"] |
74 | + if not initial_password: |
75 | + if self.unit.is_leader: |
76 | + initial_password = password_generator() |
77 | + self.leader_data["initial_password"] = initial_password |
78 | + return initial_password |
79 | + |
80 | + def _on_get_initial_password_action(self, event): |
81 | + """Handle the get-initial-password action.""" |
82 | + initial_password = self._get_initial_password() |
83 | + if initial_password: |
84 | + event.set_results({"password": initial_password}) |
85 | + else: |
86 | + event.fail("Initial password has not been set yet.") |
87 | + |
88 | |
89 | if __name__ == "__main__": |
90 | main(WordpressCharm) |
91 | diff --git a/src/leadership.py b/src/leadership.py |
92 | new file mode 100644 |
93 | index 0000000..8e88779 |
94 | --- /dev/null |
95 | +++ b/src/leadership.py |
96 | @@ -0,0 +1,218 @@ |
97 | +# This file is part of the PostgreSQL k8s Charm for Juju. |
98 | +# Copyright 2020 Canonical Ltd. |
99 | +# |
100 | +# This program is free software: you can redistribute it and/or modify |
101 | +# it under the terms of the GNU General Public License version 3, as |
102 | +# published by the Free Software Foundation. |
103 | +# |
104 | +# This program is distributed in the hope that it will be useful, but |
105 | +# WITHOUT ANY WARRANTY; without even the implied warranties of |
106 | +# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR |
107 | +# PURPOSE. See the GNU General Public License for more details. |
108 | +# |
109 | +# You should have received a copy of the GNU General Public License |
110 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
111 | + |
112 | +# TODO: Most of all of this module should move into the Operator Framework core |
113 | + |
114 | +import collections.abc |
115 | +import subprocess |
116 | +from typing import Any, Iterable, Dict, MutableMapping, Protocol |
117 | + |
118 | +import ops |
119 | +import yaml |
120 | + |
121 | + |
122 | +class _Codec(Protocol): |
123 | + def encode(self, value: Any) -> str: |
124 | + raise NotImplementedError("encode") |
125 | + |
126 | + def decode(self, key: str, value: str) -> Any: |
127 | + raise NotImplementedError("decode") |
128 | + |
129 | + |
130 | +class _ObjectABCMeta(type(ops.framework.Object), type(collections.abc.MutableMapping)): |
131 | + """This metaclass can go once the Operator Framework drops Python 3.5 support. |
132 | + |
133 | + Per ops.framework._Metaclass docstring. |
134 | + """ |
135 | + |
136 | + pass |
137 | + |
138 | + |
139 | +class _PeerData(ops.framework.Object, collections.abc.MutableMapping, metaclass=_ObjectABCMeta): |
140 | + """A bag of data shared between peer units. |
141 | + |
142 | + Only the leader can set data. All peer units can read. |
143 | + """ |
144 | + |
145 | + def __init__(self, parent: ops.framework.Object, key: str, _store: MutableMapping, _codec: _Codec): |
146 | + super().__init__(parent, key) |
147 | + self._store = _store |
148 | + self._codec = _codec |
149 | + self._prefix = self.handle.path |
150 | + |
151 | + def _prefixed_key(self, key: str) -> str: |
152 | + return self._prefix + "/" + key |
153 | + |
154 | + def __getitem__(self, key: str) -> Any: |
155 | + if not isinstance(key, str): |
156 | + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") |
157 | + raw = self._store[self._prefixed_key(key)] |
158 | + return self._codec.decode(key, raw) |
159 | + |
160 | + def __setitem__(self, key: str, value: Any) -> None: |
161 | + if not isinstance(key, str): |
162 | + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") |
163 | + if not self.model.unit.is_leader(): |
164 | + raise RuntimeError("non-leader attempting to set peer data") |
165 | + self._store[self._prefixed_key(key)] = self._codec.encode(value) |
166 | + |
167 | + def __delitem__(self, key: str) -> None: |
168 | + if not isinstance(key, str): |
169 | + raise TypeError(f"key must be a string, got {repr(key)} {type(key)}") |
170 | + if not self.model.unit.is_leader(): |
171 | + raise RuntimeError("non-leader attempting to set peer data") |
172 | + del self._store[self._prefixed_key(key)] |
173 | + |
174 | + def __iter__(self) -> Iterable[str]: |
175 | + return iter(self._store) |
176 | + |
177 | + def __len__(self) -> int: |
178 | + return len(self._store) |
179 | + |
180 | + |
181 | +class LegacyLeaderData(_PeerData): |
182 | + """Raw Juju Leadership settings, a bag of data shared between peers. |
183 | + |
184 | + Only the leader can set data. All peer units can read. |
185 | + |
186 | + Behavior matches the Juju leader-get and leader-set tools; keys and |
187 | + values must be strings, Setting an value to the empty string is the |
188 | + same as deleting the entry, and accessing a missing entry will |
189 | + return an empty string. |
190 | + |
191 | + This class provides access to legacy Juju Leadership data, and namespace |
192 | + collisions may occur if multiple components attempt to use the same key. |
193 | + """ |
194 | + |
195 | + def __init__(self, parent, key=""): |
196 | + super().__init__(parent, key, LeadershipSettings(), _RawCodec()) |
197 | + |
198 | + def _prefixed_key(self, key: str) -> str: |
199 | + return key |
200 | + |
201 | + |
202 | +class RawLeaderData(_PeerData): |
203 | + """Raw Juju Leadership settings, a bag of data shared between peers. |
204 | + |
205 | + Only the leader can set data. All peer units can read. |
206 | + |
207 | + Behavior matches the Juju leader-get and leader-set tools; keys and |
208 | + values must be strings, Setting an value to the empty string is the |
209 | + same as deleting the entry, and accessing a missing entry will |
210 | + return an empty string. |
211 | + |
212 | + Keys are automatically prefixed to avoid namespace collisions in the |
213 | + Juju Leadership settings. |
214 | + """ |
215 | + |
216 | + def __init__(self, parent, key=""): |
217 | + super().__init__(parent, key, LeadershipSettings(), _RawCodec()) |
218 | + |
219 | + |
220 | +class RichLeaderData(_PeerData): |
221 | + """Encoded Juju Leadership settings, a bag of data shared between peers. |
222 | + |
223 | + Only the leader can set data. All peer units can read. |
224 | + |
225 | + Operates as a standard Python MutableMapping. Keys must be strings. |
226 | + Values may be anything that the yaml library can marshal. |
227 | + |
228 | + Keys are automatically prefixed to avoid namespace collisions in the |
229 | + Juju Leadership settings. |
230 | + """ |
231 | + |
232 | + def __init__(self, parent, key=""): |
233 | + super().__init__(parent, key, LeadershipSettings(), _YAMLCodec()) |
234 | + |
235 | + |
236 | +class _YAMLCodec(object): |
237 | + def encode(self, value: Any) -> str: |
238 | + return yaml.safe_dump(value) |
239 | + |
240 | + def decode(self, key: str, value: str) -> Any: |
241 | + if not value: |
242 | + # Key never existed or was deleted. If set to |
243 | + # empty string or none, value will contain |
244 | + # the YAML representation. |
245 | + raise KeyError(key) |
246 | + return yaml.safe_load(value) |
247 | + |
248 | + |
249 | +class _RawCodec(object): |
250 | + def encode(self, value: Any) -> str: |
251 | + if not isinstance(value, str): |
252 | + raise TypeError(f"{self.__class__.__name__} only supports str values, got {type(value)}") |
253 | + return value |
254 | + |
255 | + def decode(self, value: str) -> Any: |
256 | + return value |
257 | + |
258 | + |
259 | +class LeadershipSettings(collections.abc.MutableMapping): |
260 | + """Juju Leadership Settings data. |
261 | + |
262 | + This class provides direct access to the Juju Leadership Settings, |
263 | + a bag of data shared between peer units. Only the leader can set |
264 | + items. Keys all share the same namespace, so beware of collisions. |
265 | + |
266 | + This MutableMapping implements Juju behavior. Only strings are |
267 | + supported as keys and values. Deleting an entry is the same as |
268 | + setting it to the empty string. Attempting to read a missing |
269 | + key will return the empty string (this class will never raise |
270 | + a KeyError). |
271 | + """ |
272 | + |
273 | + __cls_cache = None |
274 | + |
275 | + @property |
276 | + def _cache_loaded(self) -> bool: |
277 | + return self.__class__.__cls_cache is not None |
278 | + |
279 | + @property |
280 | + def _cache(self) -> Dict[str, str]: |
281 | + # There might be multiple instances of LeadershipSettings, but |
282 | + # the backend is shared, so the cache needs to be a class |
283 | + # attribute. |
284 | + cls = self.__class__ |
285 | + if cls.__cls_cache is None: |
286 | + cmd = ["leader-get", "--format=yaml"] |
287 | + cls.__cls_cache = yaml.safe_load(subprocess.check_output(cmd).decode("UTF-8")) or {} |
288 | + return cls.__cls_cache |
289 | + |
290 | + def __getitem__(self, key: str) -> str: |
291 | + return self._cache.get(key, "") |
292 | + |
293 | + def __setitem__(self, key: str, value: str): |
294 | + if "=" in key: |
295 | + # Leave other validation to the leader-set tool |
296 | + raise RuntimeError(f"LeadershipSettings keys may not contain '=', got {key}") |
297 | + if value is None: |
298 | + value = "" |
299 | + cmd = ["leader-set", f"{key}={value}"] |
300 | + subprocess.check_call(cmd) |
301 | + if self._cache_loaded: |
302 | + if value == "": |
303 | + del self._cache[key] |
304 | + else: |
305 | + self._cache[key] = value |
306 | + |
307 | + def __delitem__(self, key: str): |
308 | + self[key] = "" |
309 | + |
310 | + def __iter__(self) -> Iterable[str]: |
311 | + return iter(self._cache) |
312 | + |
313 | + def __len__(self) -> int: |
314 | + return len(self._cache) |
315 | diff --git a/src/wordpress.py b/src/wordpress.py |
316 | index a3f586e..9ac5336 100644 |
317 | --- a/src/wordpress.py |
318 | +++ b/src/wordpress.py |
319 | @@ -44,15 +44,10 @@ class Wordpress: |
320 | def __init__(self, model_config): |
321 | self.model_config = model_config |
322 | |
323 | - def _write_initial_password(self, password, filepath): |
324 | - with open(filepath, "w") as f: |
325 | - f.write(password) |
326 | - |
327 | - def first_install(self, service_ip): |
328 | + def first_install(self, service_ip, admin_password): |
329 | """Perform initial configuration of wordpress if needed.""" |
330 | config = self.model_config |
331 | logger.info("Starting wordpress initial configuration") |
332 | - admin_password = password_generator() |
333 | payload = { |
334 | "admin_password": admin_password, |
335 | "blog_public": "checked", |
336 | @@ -61,12 +56,6 @@ class Wordpress: |
337 | payload.update(safe_load(config["initial_settings"])) |
338 | payload["admin_password2"] = payload["admin_password"] |
339 | |
340 | - # Ideally we would store this in state however juju run-action does not |
341 | - # currently support being run inside the operator pod which means the |
342 | - # StorageState will be split between workload and operator. |
343 | - # https://bugs.launchpad.net/juju/+bug/1870487 |
344 | - self._write_initial_password(payload["admin_password"], "/root/initial.passwd") |
345 | - |
346 | if not payload["blog_public"]: |
347 | payload["blog_public"] = "unchecked" |
348 | required_config = set(("user_name", "admin_email")) |
349 | diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py |
350 | index f0c6920..acf406c 100644 |
351 | --- a/tests/unit/test_charm.py |
352 | +++ b/tests/unit/test_charm.py |
353 | @@ -4,6 +4,8 @@ import copy |
354 | import mock |
355 | import unittest |
356 | |
357 | +from unittest.mock import Mock |
358 | + |
359 | from charm import WordpressCharm, create_wordpress_secrets, gather_wordpress_secrets |
360 | from wordpress import WORDPRESS_SECRETS |
361 | from ops import testing |
362 | @@ -124,7 +126,6 @@ class TestWordpressCharm(unittest.TestCase): |
363 | } |
364 | } |
365 | self.assertEqual(self.harness.charm.make_pod_resources(), expected) |
366 | - |
367 | # And now test with no tls config. |
368 | self.harness.update_config({"tls_secret_name": ""}) |
369 | expected = { |
370 | @@ -157,3 +158,16 @@ class TestWordpressCharm(unittest.TestCase): |
371 | } |
372 | } |
373 | self.assertEqual(self.harness.charm.make_pod_resources(), expected) |
374 | + |
375 | + def test_on_get_initial_password_action(self): |
376 | + action_event = Mock() |
377 | + # First test with no initial password set. |
378 | + with mock.patch.object(self.harness.charm, "_get_initial_password") as get_initial_password: |
379 | + get_initial_password.return_value = "" |
380 | + self.harness.charm._on_get_initial_password_action(action_event) |
381 | + self.assertEqual(action_event.fail.call_args, mock.call("Initial password has not been set yet.")) |
382 | + # Now test with initial password set. |
383 | + with mock.patch.object(self.harness.charm, "_get_initial_password") as get_initial_password: |
384 | + get_initial_password.return_value = "passwd" |
385 | + self.harness.charm._on_get_initial_password_action(action_event) |
386 | + self.assertEqual(action_event.set_results.call_args, mock.call({"password": "passwd"})) |
387 | diff --git a/tests/unit/test_wordpress.py b/tests/unit/test_wordpress.py |
388 | index f204cac..92712e7 100644 |
389 | --- a/tests/unit/test_wordpress.py |
390 | +++ b/tests/unit/test_wordpress.py |
391 | @@ -94,13 +94,10 @@ class WordpressTest(unittest.TestCase): |
392 | def test__init__(self): |
393 | self.assertEqual(self.test_wordpress.model_config, self.test_model_config) |
394 | |
395 | - @mock.patch("wordpress.password_generator", side_effect=dummy_password_generator) |
396 | - def test_first_install(self, password_generator_func): |
397 | + def test_first_install(self): |
398 | mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=True) |
399 | - mocked__write_initial_password = mock.MagicMock(name="_write_initial_password", return_value=None) |
400 | mocked_wordpress_configured = mock.MagicMock(name="wordpress_configured", return_value=True) |
401 | self.test_wordpress.call_wordpress = mocked_call_wordpress |
402 | - self.test_wordpress._write_initial_password = mocked__write_initial_password |
403 | self.test_wordpress.wordpress_configured = mocked_wordpress_configured |
404 | |
405 | test_payload = { |
406 | @@ -112,10 +109,7 @@ class WordpressTest(unittest.TestCase): |
407 | 'admin_email': 'root@admin.canonical.com', |
408 | 'weblog_title': 'Test Blog', |
409 | } |
410 | - self.test_wordpress.first_install(self.test_service_ip) |
411 | - |
412 | - # Test that we wrote initial admin credentials inside the operator pod. |
413 | - self.test_wordpress._write_initial_password.assert_called_with(TEST_GENERATED_PASSWORD, "/root/initial.passwd") |
414 | + self.test_wordpress.first_install(self.test_service_ip, TEST_GENERATED_PASSWORD) |
415 | |
416 | # Test that we POST'd our initial configuration options to the wordpress API. |
417 | self.test_wordpress.call_wordpress.assert_called_with( |
418 | @@ -128,7 +122,7 @@ class WordpressTest(unittest.TestCase): |
419 | self.test_wordpress.model_config["initial_settings"] = ( |
420 | "user_name: admin\n" "weblog_title: Test Blog\n" "blog_public: False" |
421 | ) |
422 | - self.test_wordpress.first_install(self.test_service_ip) |
423 | + self.test_wordpress.first_install(self.test_service_ip, TEST_GENERATED_PASSWORD) |
424 | self.test_wordpress.call_wordpress.assert_not_called() |
425 | |
426 | def test_wordpress_configured(self): |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.