launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29241
[Merge] ~cjwatson/launchpad:charm-postgresql-relation into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:charm-postgresql-relation into launchpad:master.
Commit message:
charm: Implement PostgreSQL relations
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/430399
Rather than having to configure connection strings manually, this now allows simply using `juju relate launchpad-appserver:db postgresql:db; juju relate launchpad-appserver:session-db postgresql:db` to connect it to a PostgreSQL deployment.
The username/password arrangements are currently quite awkward, because Launchpad assumes that it will have a separate password for each role it needs to use, rather than having a password for a parent role and then using `SET ROLE` to switch. We can update the charm once Launchpad has some improved mechanisms, though.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-postgresql-relation into launchpad:master.
diff --git a/charm/dependencies.txt b/charm/dependencies.txt
index a4a6420..26b54bc 100644
--- a/charm/dependencies.txt
+++ b/charm/dependencies.txt
@@ -1,2 +1,2 @@
-ols-layers git+ssh://git.launchpad.net/~ubuntuone-pqm-team/ols-charm-deps/+git/ols-layers;revno=df20c87d
+ols-layers git+ssh://git.launchpad.net/~ubuntuone-pqm-team/ols-charm-deps/+git/ols-layers;revno=8e3b4816
charm-wheels git+ssh://git.launchpad.net/~ubuntuone-hackers/ols-charm-deps/+git/wheels;revno=fe523e25
diff --git a/charm/launchpad-appserver/charmcraft.yaml b/charm/launchpad-appserver/charmcraft.yaml
index 2a40540..824480b 100644
--- a/charm/launchpad-appserver/charmcraft.yaml
+++ b/charm/launchpad-appserver/charmcraft.yaml
@@ -11,6 +11,7 @@ parts:
source: .
plugin: reactive
build-snaps: [charm]
+ build-packages: [libpq-dev]
build-environment:
- CHARM_LAYERS_DIR: tmp/deps/ols-layers/layer
- CHARM_INTERFACES_DIR: tmp/deps/ols-layers/interface
diff --git a/charm/launchpad-appserver/config.yaml b/charm/launchpad-appserver/config.yaml
index 8967f67..43f4a45 100644
--- a/charm/launchpad-appserver/config.yaml
+++ b/charm/launchpad-appserver/config.yaml
@@ -1,12 +1,4 @@
options:
- dbuser:
- type: string
- description: The database user used with the main database.
- default: ""
- dbuser_session:
- type: string
- description: The database user used with the session database.
- default: ""
devmode:
type: boolean
description: Is this server running in dev mode?
diff --git a/charm/launchpad-appserver/layer.yaml b/charm/launchpad-appserver/layer.yaml
index bcff867..ae77d24 100644
--- a/charm/launchpad-appserver/layer.yaml
+++ b/charm/launchpad-appserver/layer.yaml
@@ -1,3 +1,12 @@
includes:
- layer:launchpad-base
repo: https://git.launchpad.net/launchpad
+options:
+ ols-pg:
+ databases:
+ db:
+ name: launchpad_dev
+ roles: launchpad_main
+ session-db:
+ name: session_dev
+ roles: session
diff --git a/charm/launchpad-appserver/metadata.yaml b/charm/launchpad-appserver/metadata.yaml
index c778bcf..7f7b48e 100644
--- a/charm/launchpad-appserver/metadata.yaml
+++ b/charm/launchpad-appserver/metadata.yaml
@@ -13,3 +13,6 @@ tags:
series:
- focal
subordinate: false
+requires:
+ session-db:
+ interface: pgsql
diff --git a/charm/launchpad-appserver/reactive/launchpad-appserver.py b/charm/launchpad-appserver/reactive/launchpad-appserver.py
index 91745af..30b1f40 100644
--- a/charm/launchpad-appserver/reactive/launchpad-appserver.py
+++ b/charm/launchpad-appserver/reactive/launchpad-appserver.py
@@ -10,6 +10,8 @@ from charms.launchpad.base import (
configure_lazr,
get_service_config,
lazr_config_files,
+ strip_dsn_authentication,
+ update_pgpass,
)
from charms.reactive import (
clear_flag,
@@ -19,7 +21,8 @@ from charms.reactive import (
when,
when_not,
)
-from ols import base
+from ols import base, postgres
+from psycopg2.extensions import parse_dsn
def reload_or_restart(service):
@@ -84,10 +87,17 @@ def config_files():
return files
-@when("launchpad.base.configured")
+@when("launchpad.base.configured", "session-db.master.available")
@when_not("service.configured")
-def configure():
+def configure(session_db):
config = get_service_config()
+ session_db_primary, _ = postgres.get_db_uris(session_db)
+ # XXX cjwatson 2022-09-23: Mangle the connection string into a form
+ # Launchpad understands. In the long term it would be better to have
+ # Launchpad be able to consume unmodified connection strings.
+ update_pgpass(session_db_primary)
+ config["db_session"] = strip_dsn_authentication(session_db_primary)
+ config["db_session_user"] = parse_dsn(session_db_primary)["user"]
# XXX cjwatson 2022-09-07: Some config items have no reasonable default.
# We should set the workload status to blocked in that case.
configure_lazr(
diff --git a/charm/launchpad-appserver/templates/launchpad-appserver-lazr.conf b/charm/launchpad-appserver/templates/launchpad-appserver-lazr.conf
index dfa56bd..bafc316 100644
--- a/charm/launchpad-appserver/templates/launchpad-appserver-lazr.conf
+++ b/charm/launchpad-appserver/templates/launchpad-appserver-lazr.conf
@@ -10,14 +10,11 @@
extends: ../launchpad-base-lazr.conf
[launchpad]
-{%- if dbuser %}
-dbuser: {{ dbuser }}
-{%- endif %}
devmode: {{ devmode }}
min_legitimate_account_age: {{ min_legitimate_account_age }}
min_legitimate_karma: {{ min_legitimate_karma }}
[launchpad_session]
-{%- if dbuser_session %}
-dbuser: {{ dbuser_session }}
-{%- endif %}
+database: {{ db_session }}
+dbuser: {{ db_session_user }}
+cookie: {{ session_cookie_name }}
diff --git a/charm/layer/launchpad-base/config.yaml b/charm/layer/launchpad-base/config.yaml
index cb665fe..c33c239 100644
--- a/charm/layer/launchpad-base/config.yaml
+++ b/charm/layer/launchpad-base/config.yaml
@@ -3,17 +3,12 @@ options:
type: string
description: URL of file used to control whether cron scripts run.
default: "file:cronscripts.ini"
- db_primary:
+ databases:
type: string
- description: Connection string for primary database.
- default: ""
- db_session:
- type: string
- description: Connection string for session database.
- default: ""
- db_standby:
- type: string
- description: Connection string for standby database, if any.
+ description: >
+ YAML-encoded information about database relations, overriding the
+ defaults. For example, setting this to "db: {name: launchpad_prod}"
+ changes the name of the database used by the "db" relation.
default: ""
domain:
type: string
diff --git a/charm/layer/launchpad-base/layer.yaml b/charm/layer/launchpad-base/layer.yaml
index ae16aa0..0a62526 100644
--- a/charm/layer/launchpad-base/layer.yaml
+++ b/charm/layer/launchpad-base/layer.yaml
@@ -1,6 +1,6 @@
includes:
- layer:basic
- - layer:ols
+ - layer:ols-pg
options:
apt:
packages:
diff --git a/charm/layer/launchpad-base/lib/charms/launchpad/base.py b/charm/layer/launchpad-base/lib/charms/launchpad/base.py
index 67bfdcb..da811fe 100644
--- a/charm/layer/launchpad-base/lib/charms/launchpad/base.py
+++ b/charm/layer/launchpad-base/lib/charms/launchpad/base.py
@@ -1,10 +1,15 @@
# Copyright 2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+import grp
import os.path
+import pwd
+import re
+from dataclasses import dataclass
from charmhelpers.core import hookenv, host, templating
from ols import base
+from psycopg2.extensions import make_dsn, parse_dsn
def home_dir():
@@ -80,3 +85,108 @@ def configure_rsync(config, template, name):
templating.render(template, rsync_path, config, perms=0o644)
elif os.path.exists(rsync_path):
os.unlink(rsync_path)
+
+
+@dataclass
+class PgPassLine:
+ hostname: str
+ port: str
+ database: str
+ username: str
+ password: str
+
+
+def update_pgpass(dsn):
+ # See https://www.postgresql.org/docs/current/libpq-pgpass.html.
+
+ def unescape(entry):
+ return re.sub(r"\\(.)", r"\1", entry)
+
+ def escape(entry):
+ return re.sub(r"([:\\])", r"\\\1", entry)
+
+ parsed_dsn = parse_dsn(dsn)
+ pgpass_path = os.path.join(home_dir(), ".pgpass")
+ pgpass = []
+ try:
+ with open(pgpass_path) as f:
+ for line in f:
+ if line.startswith("#"):
+ continue
+ match = re.match(
+ r"""
+ ^
+ (?P<hostname>(?:[^:\\]|\\.)*):
+ (?P<port>(?:[^:\\]|\\.)*):
+ (?P<database>(?:[^:\\]|\\.)*):
+ (?P<username>(?:[^:\\]|\\.)*):
+ (?P<password>(?:[^:\\]|\\.)*)
+ $
+ """,
+ line.rstrip("\n"),
+ flags=re.X,
+ )
+ if match is None:
+ continue
+ pgpass.append(
+ PgPassLine(
+ hostname=unescape(match.group("hostname")),
+ port=unescape(match.group("port")),
+ database=unescape(match.group("database")),
+ username=unescape(match.group("username")),
+ password=unescape(match.group("password")),
+ )
+ )
+ except OSError:
+ pass
+
+ modified = False
+ for line in pgpass:
+ if (
+ line.hostname in ("*", parsed_dsn["host"])
+ and line.port in ("*", parsed_dsn["port"])
+ and line.database in ("*", parsed_dsn["dbname"])
+ and line.username in ("*", parsed_dsn["user"])
+ ):
+ if line.password != parsed_dsn["password"]:
+ line.password = parsed_dsn["password"]
+ modified = True
+ break
+ else:
+ pgpass.append(
+ PgPassLine(
+ hostname=parsed_dsn["host"],
+ port=parsed_dsn["port"],
+ database=parsed_dsn["dbname"],
+ username=parsed_dsn["user"],
+ password=parsed_dsn["password"],
+ )
+ )
+ modified = True
+
+ if modified:
+ uid = pwd.getpwnam(base.user()).pw_uid
+ gid = grp.getgrnam(base.user()).gr_gid
+ with open(pgpass_path, "w") as f:
+ for line in pgpass:
+ print(
+ ":".join(
+ [
+ escape(line.hostname),
+ escape(line.port),
+ escape(line.database),
+ escape(line.username),
+ escape(line.password),
+ ]
+ ),
+ file=f,
+ )
+ os.fchown(f.fileno(), uid, gid)
+ os.fchmod(f.fileno(), 0o600)
+
+
+def strip_dsn_authentication(dsn):
+ parsed_dsn = parse_dsn(dsn)
+ parsed_dsn.pop("user", None)
+ parsed_dsn.pop("password", None)
+ return make_dsn(**parsed_dsn)
diff --git a/charm/layer/launchpad-base/metadata.yaml b/charm/layer/launchpad-base/metadata.yaml
new file mode 100644
index 0000000..6de4a93
--- /dev/null
+++ b/charm/layer/launchpad-base/metadata.yaml
@@ -0,0 +1,3 @@
+requires:
+ db:
+ interface: pgsql
diff --git a/charm/layer/launchpad-base/reactive/launchpad-base.py b/charm/layer/launchpad-base/reactive/launchpad-base.py
index 8dd3b69..cd4bef1 100644
--- a/charm/layer/launchpad-base/reactive/launchpad-base.py
+++ b/charm/layer/launchpad-base/reactive/launchpad-base.py
@@ -8,9 +8,12 @@ from charms.launchpad.base import (
configure_rsync,
ensure_lp_directories,
get_service_config,
+ strip_dsn_authentication,
+ update_pgpass,
)
from charms.reactive import hook, remove_state, set_state, when, when_not
-from ols import base
+from ols import base, postgres
+from psycopg2.extensions import parse_dsn
# Monkey-patch layer:ols.
@@ -25,11 +28,25 @@ def create_virtualenv(wheels_dir, codedir, python_exe):
base.create_virtualenv = create_virtualenv
-@when("ols.configured")
+@when("ols.configured", "db.master.available")
@when_not("launchpad.base.configured")
-def configure():
+def configure(db):
ensure_lp_directories()
config = get_service_config()
+ db_primary, db_standby = postgres.get_db_uris(db)
+ # XXX cjwatson 2022-09-23: Mangle the connection strings into forms
+ # Launchpad understands. In the long term it would be better to have
+ # Launchpad be able to consume unmodified connection strings.
+ for dsn in [db_primary] + db_standby:
+ update_pgpass(dsn)
+ config["db_primary"] = strip_dsn_authentication(db_primary)
+ config["db_standby"] = ",".join(
+ strip_dsn_authentication(dsn) for dsn in db_standby
+ )
+ # XXX cjwatson 2022-09-23: This is a layering violation, since it's
+ # specific to the appserver. We need to teach Launchpad to be able to
+ # log in as one role and then switch to another.
+ config["db_user"] = parse_dsn(db_primary)["user"]
# XXX cjwatson 2022-09-07: Some config items have no reasonable default.
# We should set the workload status to blocked in that case.
configure_lazr(
diff --git a/charm/layer/launchpad-base/templates/launchpad-base-lazr.conf b/charm/layer/launchpad-base/templates/launchpad-base-lazr.conf
index e66d6ec..4b50d48 100644
--- a/charm/layer/launchpad-base/templates/launchpad-base-lazr.conf
+++ b/charm/layer/launchpad-base/templates/launchpad-base-lazr.conf
@@ -25,14 +25,11 @@ oops_prefix: {{ oops_prefix }}
[launchpad]
config_overlay_dir: {{ secrets_dir }}
+dbuser: {{ db_user }}
http_proxy: {{ http_proxy or "none" }}
openid_alternate_provider_roots: {{ openid_alternate_provider_roots or "none" }}
openid_provider_root: {{ openid_provider_root }}
-[launchpad_session]
-database: {{ db_session }}
-cookie: {{ session_cookie_name }}
-
[vhost.mainsite]
hostname: {{ domain }}
althostnames: localhost, www.{{ domain }}
