Colin Watson has proposed merging ~cjwatson/launchpad-layers:db-layer into launchpad-layers:main with ~cjwatson/launchpad-layers:payload-layer as a prerequisite.
Commit message: Split out a launchpad-db layer Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad-layers/+git/launchpad-layers/+merge/442155 `launchpad-loggerhead` needs most of the `launchpad-base` layer, but doesn't have any direct database connectivity, so we don't want to require it to have a `db` relation before it will do anything. Split out database interaction into a separate layer. This will require applications to change: they will need to import some functions from `charms.launchpad.db` rather than `charms.launchpad.base`, they will need to react to `launchpad.db.configured` rather than `launchpad.base.configured`, and their LAZR configuration files will need to extend `launchpad-db-lazr.conf` rather than `launchpad-base-lazr.conf`. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-layers:db-layer into launchpad-layers:main.
diff --git a/launchpad-base/config.yaml b/launchpad-base/config.yaml index b804392..3263531 100644 --- a/launchpad-base/config.yaml +++ b/launchpad-base/config.yaml @@ -43,17 +43,6 @@ options: type: string description: URL of file used to control whether cron scripts run. default: "file:cronscripts.ini" - databases: - type: string - 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: "" - db_statement_timeout: - type: int - description: SQL statement timeout in milliseconds. - default: default_batch_size: type: int description: The default size used in a batched listing of results. diff --git a/launchpad-base/layer.yaml b/launchpad-base/layer.yaml index 5abc6ac..ed3e8dc 100644 --- a/launchpad-base/layer.yaml +++ b/launchpad-base/layer.yaml @@ -1,6 +1,5 @@ includes: - layer:basic - - layer:ols-pg - interface:rabbitmq - layer:launchpad-payload repo: https://git.launchpad.net/launchpad-layers diff --git a/launchpad-base/lib/charms/launchpad/base.py b/launchpad-base/lib/charms/launchpad/base.py index 2d441de..043cd4f 100644 --- a/launchpad-base/lib/charms/launchpad/base.py +++ b/launchpad-base/lib/charms/launchpad/base.py @@ -1,22 +1,16 @@ # 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 import subprocess -from dataclasses import dataclass from email.utils import parseaddr from urllib.parse import urlparse from charmhelpers.core import hookenv, host, templating from charms.launchpad.payload import config_file_path from charms.launchpad.payload import get_service_config as get_payload_config -from charms.launchpad.payload import home_dir from charms.reactive import endpoint_from_flag from ols import base -from psycopg2.extensions import make_dsn, parse_dsn def oopses_dir(): @@ -136,107 +130,3 @@ 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("password", None) - return make_dsn(**parsed_dsn) diff --git a/launchpad-base/metadata.yaml b/launchpad-base/metadata.yaml index eafdcc0..f2480ad 100644 --- a/launchpad-base/metadata.yaml +++ b/launchpad-base/metadata.yaml @@ -1,5 +1,3 @@ requires: - db: - interface: pgsql rabbitmq: interface: rabbitmq diff --git a/launchpad-base/reactive/launchpad-base.py b/launchpad-base/reactive/launchpad-base.py index 186e7df..d8e3376 100644 --- a/launchpad-base/reactive/launchpad-base.py +++ b/launchpad-base/reactive/launchpad-base.py @@ -7,8 +7,6 @@ from charms.launchpad.base import ( configure_rsync, ensure_lp_directories, get_service_config, - strip_dsn_authentication, - update_pgpass, ) from charms.launchpad.payload import configure_lazr from charms.reactive import ( @@ -22,7 +20,7 @@ from charms.reactive import ( when_not, when_not_all, ) -from ols import base, postgres +from ols import base @when("rabbitmq.connected") @@ -59,30 +57,15 @@ def rabbitmq_unavailable(): clear_flag("launchpad.rabbitmq.available") -@when( - "launchpad.payload.configured", - "db.master.available", - "launchpad.rabbitmq.available", -) +@when("launchpad.payload.configured", "launchpad.rabbitmq.available") @when_not("launchpad.base.configured") def configure(): - db = endpoint_from_flag("db.master.available") rabbitmq = endpoint_from_flag("rabbitmq.available") # Interactive use shouldn't be frequent, but it's still needed # sometimes, so make it less annoying. change_shell(base.user(), "/bin/bash") 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 - ) config["rabbitmq_broker_urls"] = sorted(get_rabbitmq_uris(rabbitmq)) configure_lazr( config, @@ -102,30 +85,16 @@ def configure(): @when("launchpad.base.configured") -@when_not_all( - "launchpad.payload.configured", - "db.master.available", - "launchpad.rabbitmq.available", -) +@when_not_all("launchpad.payload.configured", "launchpad.rabbitmq.available") def deconfigure(): clear_flag("launchpad.base.configured") - clear_flag("service.configured") @when("config.changed") def config_changed(): clear_flag("launchpad.base.configured") - clear_flag("service.configured") - - -@when("db.database.changed", "launchpad.base.configured") -def db_changed(): - clear_flag("launchpad.base.configured") - clear_flag("service.configured") - clear_flag("db.database.changed") @hook("{requires:rabbitmq}-relation-changed") def rabbitmq_relation_changed(*args): clear_flag("launchpad.base.configured") - clear_flag("service.configured") diff --git a/launchpad-base/templates/launchpad-base-lazr.conf b/launchpad-base/templates/launchpad-base-lazr.conf index b9707cb..a73f081 100644 --- a/launchpad-base/templates/launchpad-base-lazr.conf +++ b/launchpad-base/templates/launchpad-base-lazr.conf @@ -44,10 +44,6 @@ git_ssh_root: git+ssh://{{ domain_git }}/ {%- endif %} [database] -{{- opt("db_statement_timeout", db_statement_timeout) }} -rw_main_primary: {{ db_primary }} -rw_main_standby: {{ db_standby or db_primary }} -set_role_after_connecting: True {{- opt("soft_request_timeout", soft_request_timeout) }} [error_reports] diff --git a/launchpad-db/config.yaml b/launchpad-db/config.yaml new file mode 100644 index 0000000..cd088e2 --- /dev/null +++ b/launchpad-db/config.yaml @@ -0,0 +1,12 @@ +options: + databases: + type: string + 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: "" + db_statement_timeout: + type: int + description: SQL statement timeout in milliseconds. + default: diff --git a/launchpad-db/layer.yaml b/launchpad-db/layer.yaml new file mode 100644 index 0000000..3ea5d6a --- /dev/null +++ b/launchpad-db/layer.yaml @@ -0,0 +1,4 @@ +includes: + - layer:launchpad-base + - layer:ols-pg +repo: https://git.launchpad.net/launchpad-layers diff --git a/launchpad-db/lib/charms/launchpad/db.py b/launchpad-db/lib/charms/launchpad/db.py new file mode 100644 index 0000000..22e1ead --- /dev/null +++ b/launchpad-db/lib/charms/launchpad/db.py @@ -0,0 +1,121 @@ +# Copyright 2022-2023 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 charms.launchpad.base import lazr_config_files as base_config_files +from charms.launchpad.payload import config_file_path, home_dir +from ols import base +from psycopg2.extensions import make_dsn, parse_dsn + + +def lazr_config_files(): + return base_config_files() + [config_file_path("launchpad-db-lazr.conf")] + + +@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("password", None) + return make_dsn(**parsed_dsn) diff --git a/launchpad-db/metadata.yaml b/launchpad-db/metadata.yaml new file mode 100644 index 0000000..6de4a93 --- /dev/null +++ b/launchpad-db/metadata.yaml @@ -0,0 +1,3 @@ +requires: + db: + interface: pgsql diff --git a/launchpad-db/reactive/launchpad-db.py b/launchpad-db/reactive/launchpad-db.py new file mode 100644 index 0000000..b83a99f --- /dev/null +++ b/launchpad-db/reactive/launchpad-db.py @@ -0,0 +1,57 @@ +# Copyright 2022-2023 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +from charms.launchpad.base import get_service_config +from charms.launchpad.db import strip_dsn_authentication, update_pgpass +from charms.launchpad.payload import configure_lazr +from charms.reactive import ( + clear_flag, + endpoint_from_flag, + hook, + set_flag, + when, + when_not, + when_not_all, +) +from ols import postgres + + +@when("launchpad.base.configured", "db.master.available") +@when_not("launchpad.db.configured") +def configure(): + db = endpoint_from_flag("db.master.available") + 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 + ) + configure_lazr(config, "launchpad-db-lazr.conf", "launchpad-db-lazr.conf") + set_flag("launchpad.db.configured") + + +@when("launchpad.db.configured") +@when_not_all("launchpad.base.configured", "db.master.available") +def deconfigure(): + clear_flag("launchpad.db.configured") + + +@when("config.changed") +def config_changed(): + clear_flag("launchpad.db.configured") + + +@when("db.database.changed", "launchpad.db.configured") +def db_changed(): + clear_flag("launchpad.db.configured") + clear_flag("db.database.changed") + + +@hook("{requires:rabbitmq}-relation-changed") +def rabbitmq_relation_changed(*args): + clear_flag("launchpad.db.configured") diff --git a/launchpad-db/templates/launchpad-db-lazr.conf b/launchpad-db/templates/launchpad-db-lazr.conf new file mode 100644 index 0000000..e6f4f80 --- /dev/null +++ b/launchpad-db/templates/launchpad-db-lazr.conf @@ -0,0 +1,19 @@ +# Public configuration data. The contents of this file may be freely shared +# with developers if needed for debugging. + +# A schema's sections, keys, and values are automatically inherited, except +# for '.optional' sections. Update this config to override key values. +# Values are strings, except for numbers that look like ints. The tokens +# true, false, and none are treated as True, False, and None. + +{% from "macros.j2" import opt -%} + +[meta] +extends: launchpad-base-lazr.conf + +[database] +{{- opt("db_statement_timeout", db_statement_timeout) }} +rw_main_primary: {{ db_primary }} +rw_main_standby: {{ db_standby or db_primary }} +set_role_after_connecting: True +
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp