Merge ~cjwatson/launchpad:charm-admin-apply-security into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: b1500e83e54dab6584ca3b4b4b0d0f8efb9a06dc
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:charm-admin-apply-security
Merge into: launchpad:master
Diff against target: 113 lines (+53/-4)
4 files modified
charm/launchpad-admin/reactive/launchpad-admin.py (+50/-1)
charm/launchpad-admin/templates/db-admin.j2 (+1/-1)
charm/launchpad-admin/templates/db-session.j2 (+1/-1)
charm/launchpad-admin/templates/db.j2 (+1/-1)
Reviewer Review Type Date Requested Status
Guruprasad Approve
Review via email: mp+440257@code.launchpad.net

Commit message

charm: Update DB permissions when configuring launchpad-admin

Description of the change

This replaces code currently run by our deployment machinery at the end of its `build` phase.

I also fixed a typo in a reactive flag name that caused hooks to do unnecessary work, since we were never considering the service to be configured.

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM ๐Ÿ‘๐Ÿผ

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/charm/launchpad-admin/reactive/launchpad-admin.py b/charm/launchpad-admin/reactive/launchpad-admin.py
2index 4d65102..ff76afc 100644
3--- a/charm/launchpad-admin/reactive/launchpad-admin.py
4+++ b/charm/launchpad-admin/reactive/launchpad-admin.py
5@@ -2,6 +2,7 @@
6 # GNU Affero General Public License version 3 (see the file LICENSE).
7
8 import os.path
9+import subprocess
10
11 from charmhelpers.core import hookenv, host, templating
12 from charms.launchpad.base import (
13@@ -28,13 +29,55 @@ def strip_password(dsn):
14 return make_dsn(**parsed_dsn)
15
16
17+def database_is_initialized() -> bool:
18+ """Has the database been initialized?
19+
20+ The launchpad-admin charm is itself used to initialize the database, so
21+ we can't assume that that's been done yet at the time our `configure`
22+ handler runs. The `LaunchpadDatabaseRevision` table is used to track
23+ schema migrations, so its presence is a good indicator of whether we
24+ have a useful database.
25+ """
26+ return (
27+ subprocess.run(
28+ [
29+ "sudo",
30+ "-H",
31+ "-u",
32+ base.user(),
33+ os.path.join(home_dir(), "bin", "db"),
34+ "-c",
35+ r"\d LaunchpadDatabaseRevision",
36+ ],
37+ stdout=subprocess.DEVNULL,
38+ stderr=subprocess.DEVNULL,
39+ ).returncode
40+ == 0
41+ )
42+
43+
44+def update_database_permissions():
45+ subprocess.run(
46+ [
47+ "sudo",
48+ "-H",
49+ "-u",
50+ base.user(),
51+ "LPCONFIG=launchpad-admin",
52+ os.path.join(base.code_dir(), "database", "schema", "security.py"),
53+ "--no-revoke",
54+ ],
55+ check=True,
56+ )
57+
58+
59 @when(
60 "launchpad.base.configured",
61 "db.master.available",
62 "db-admin.master.available",
63 "session-db.master.available",
64 )
65-@when_not("service_configured")
66+@when_not("service.configured")
67 def configure():
68 db = endpoint_from_flag("db.master.available")
69 db_admin = endpoint_from_flag("db-admin.master.available")
70@@ -82,5 +125,11 @@ def configure():
71 perms=0o755,
72 )
73
74+ if database_is_initialized():
75+ hookenv.log("Updating database permissions.")
76+ update_database_permissions()
77+ else:
78+ hookenv.log("Database has not been initialized yet.")
79+
80 set_state("service.configured")
81 hookenv.status_set("active", "Ready")
82diff --git a/charm/launchpad-admin/templates/db-admin.j2 b/charm/launchpad-admin/templates/db-admin.j2
83index aa0f73d..4749ab3 100644
84--- a/charm/launchpad-admin/templates/db-admin.j2
85+++ b/charm/launchpad-admin/templates/db-admin.j2
86@@ -6,5 +6,5 @@
87
88 set -e
89
90-psql '{{ db_admin_primary }}'
91+psql '{{ db_admin_primary }}' "$@"
92
93diff --git a/charm/launchpad-admin/templates/db-session.j2 b/charm/launchpad-admin/templates/db-session.j2
94index 4d776ae..76a47e1 100755
95--- a/charm/launchpad-admin/templates/db-session.j2
96+++ b/charm/launchpad-admin/templates/db-session.j2
97@@ -6,5 +6,5 @@
98
99 set -e
100
101-psql '{{ db_session_primary }}'
102+psql '{{ db_session_primary }}' "$@"
103
104diff --git a/charm/launchpad-admin/templates/db.j2 b/charm/launchpad-admin/templates/db.j2
105index f07f7e8..7976492 100644
106--- a/charm/launchpad-admin/templates/db.j2
107+++ b/charm/launchpad-admin/templates/db.j2
108@@ -6,5 +6,5 @@
109
110 set -e
111
112-psql '{{ db_primary }}'
113+psql '{{ db_primary }}' "$@"
114

Subscribers

People subscribed via source and target branches

to status/vote changes: