Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPE-1766] Juju 3 peer secrets #121

Merged
merged 48 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
40a74e8
Upgraded TLS lib
marceloneppel Jul 14, 2023
cb76a71
Initial implementation
dragomirp Aug 2, 2023
e17a98c
Test linting
dragomirp Aug 2, 2023
3612e76
Missed peer call
dragomirp Aug 2, 2023
16baa0f
Add overrides
dragomirp Aug 2, 2023
c6dc645
monitoring_password override
dragomirp Aug 2, 2023
777c667
Linting
dragomirp Aug 2, 2023
fc44e8d
Linting
dragomirp Aug 2, 2023
0778560
auth_file override
dragomirp Aug 2, 2023
654cbfb
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 3, 2023
d18808a
Normalise arbitrary keys to fit Juju secrets criteria
dragomirp Aug 3, 2023
ea0145c
Update on secrets changes
dragomirp Aug 3, 2023
425b22e
Remove maintainers metadata
dragomirp Aug 3, 2023
ee5ea81
Renovate includes
dragomirp Aug 7, 2023
ad24524
Bump libs
dragomirp Aug 7, 2023
ba5cbf6
Merge remote-tracking branch 'origin/dpe-2260-upgrade-tls-lib' into d…
dragomirp Aug 7, 2023
c882f60
Defer on non-existent peer databag
dragomirp Aug 7, 2023
deed19e
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 7, 2023
31a17bb
Handle model error while checking extensions
dragomirp Aug 8, 2023
46eb327
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 9, 2023
2399249
Merge lock
dragomirp Aug 9, 2023
142f15c
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 10, 2023
aaca491
Merge remote-tracking branch 'origin/main' into dpe-2260-upgrade-tls-lib
marceloneppel Aug 10, 2023
1f21653
Upgrade libs
marceloneppel Aug 10, 2023
3f8f6d3
Merge branch 'dpe-2260-upgrade-tls-lib' into dpe-1766-peer-secrets
dragomirp Aug 14, 2023
0f4ca87
Bruteforce getting secrets
dragomirp Aug 14, 2023
5967079
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 14, 2023
5e4e6ce
Lint
dragomirp Aug 14, 2023
539199c
Store config in peer databag
dragomirp Aug 14, 2023
239de24
Missed secrets call
dragomirp Aug 14, 2023
3e840b0
Force config update on TLS files push
dragomirp Aug 14, 2023
79a4610
Unit tests
dragomirp Aug 14, 2023
c9658bd
Force secret get
dragomirp Aug 15, 2023
4ac0228
Revert force rerender
dragomirp Aug 16, 2023
ed4e2cb
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 16, 2023
8008d1f
Unit tests
dragomirp Aug 16, 2023
c89d0c3
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 17, 2023
addacee
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 17, 2023
02c35cf
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 22, 2023
ea6b1a1
Revert db rel changes
dragomirp Aug 22, 2023
4594968
Bump libs
dragomirp Aug 23, 2023
6f2799c
Reduce the amount of mattermost units
dragomirp Aug 23, 2023
49b6ba4
Bump libs
dragomirp Aug 24, 2023
ab77cfc
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Aug 25, 2023
e4d4acb
Merge branch 'main' into dpe-1766-peer-secrets
dragomirp Sep 1, 2023
cba0ac0
Update lock
dragomirp Sep 1, 2023
fbd8f4a
Revert tls test
dragomirp Sep 1, 2023
dfb9880
Generate password only once
dragomirp Sep 4, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,3 @@ jobs:
run: tox run -e ${{ matrix.tox-environments }}-${{ env.libjuju }} -- -m '${{ steps.select-tests.outputs.mark_expression }}'
env:
CI_PACKED_CHARMS: ${{ needs.build.outputs.charms }}
- name: Dump logs
uses: canonical/charm-logdump-action@main
if: failure()
with:
app: pgbouncer-k8s
Comment on lines -127 to -131
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to be doing anything useful.

282 changes: 136 additions & 146 deletions poetry.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions renovate.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
"regexManagers": [
{
"fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"],
"matchStrings": ["- \"(?<currentValue>.*?)\" +# renovate: latest juju 2"],
"matchStrings": ["(- | agent-versions: )\"(?<currentValue>.*?)\" +# renovate: latest juju 2"],
"depNameTemplate": "Juju 2",
"packageNameTemplate": "juju/juju",
"datasourceTemplate": "github-releases",
"versioningTemplate": "loose",
"extractVersionTemplate": "Juju release"
}, {
"fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"],
"matchStrings": ["- \"(?<currentValue>.*?)\" +# renovate: latest juju 3"],
"matchStrings": ["(- | agent-versions: )\"(?<currentValue>.*?)\" +# renovate: latest juju 3"],
"depNameTemplate": "Juju 3",
"packageNameTemplate": "juju/juju",
"datasourceTemplate": "github-releases",
Expand Down
188 changes: 182 additions & 6 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
from charms.postgresql_k8s.v0.postgresql_tls import PostgreSQLTLS
from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
from jinja2 import Template
from ops import JujuVersion
from ops.charm import CharmBase, ConfigChangedEvent, PebbleReadyEvent
from ops.framework import StoredState
from ops.main import main
from ops.model import ActiveStatus, BlockedStatus, WaitingStatus
from ops.model import ActiveStatus, BlockedStatus, SecretNotFoundError, WaitingStatus
from ops.pebble import ConnectionError, Layer, PathError, ServiceStatus
from tenacity import Retrying, stop_after_attempt, wait_fixed

from constants import (
APP_SCOPE,
AUTH_FILE_DATABAG_KEY,
AUTH_FILE_PATH,
CLIENT_RELATION_NAME,
Expand All @@ -37,9 +40,15 @@
PGB,
PGB_DIR,
PGB_LOG_DIR,
SECRET_CACHE_LABEL,
SECRET_DELETED_LABEL,
SECRET_INTERNAL_LABEL,
SECRET_KEY_OVERRIDES,
SECRET_LABEL,
TLS_CA_FILE,
TLS_CERT_FILE,
TLS_KEY_FILE,
UNIT_SCOPE,
)
from relations.backend_database import BackendDatabaseRequires
from relations.db import DbProvides
Expand All @@ -57,6 +66,8 @@ class PgBouncerK8sCharm(CharmBase):
def __init__(self, *args):
super().__init__(*args)

self.secrets = {APP_SCOPE: {}, UNIT_SCOPE: {}}

self.framework.observe(self.on.config_changed, self._on_config_changed)
self.framework.observe(self.on.pgbouncer_pebble_ready, self._on_pgbouncer_pebble_ready)
self.framework.observe(self.on.start, self._on_start)
Expand Down Expand Up @@ -288,7 +299,7 @@ def _on_start(self, _) -> None:

def render_auth_file_if_available(self):
"""Render the auth file if it's available in the secret store."""
if auth_file := self.get_secret("app", AUTH_FILE_DATABAG_KEY):
if auth_file := self.get_secret(APP_SCOPE, AUTH_FILE_DATABAG_KEY):
self.render_auth_file(auth_file)

def _on_update_status(self, _) -> None:
Expand Down Expand Up @@ -354,7 +365,7 @@ def reload_pgbouncer(self) -> None:

def _generate_monitoring_service(self, enabled: bool = True) -> Dict[str, str]:
if enabled:
stats_password = self.peers.get_secret("app", MONITORING_PASSWORD_KEY)
stats_password = self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY)
command = (
f'pgbouncer_exporter --web.listen-address=:{METRICS_PORT} --pgBouncer.connectionString="'
f'postgres://{self.backend.stats_user}:{stats_password}@localhost:{self.config["listen_port"]}/pgbouncer?sslmode=disable"'
Expand Down Expand Up @@ -426,13 +437,178 @@ def get_hostname_by_unit(self, unit_name: str) -> str:
unit_id = unit_name.split("/")[1]
return f"{self.app.name}-{unit_id}.{self.app.name}-endpoints"

def _normalize_secret_key(self, key: str) -> str:
new_key = key.replace("_", "-")
new_key = new_key.strip("-")

return new_key
Comment on lines +440 to +444
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PGB is storing relation based secrets with generated names, so we cannot simply use an override mapping.


def _scope_obj(self, scope: str):
if scope == APP_SCOPE:
return self.framework.model.app
if scope == UNIT_SCOPE:
return self.framework.model.unit

def _juju_secrets_get(self, scope: str) -> Optional[bool]:
"""Helper function to get Juju secret."""
if scope == UNIT_SCOPE:
peer_data = self.peers.unit_databag
else:
peer_data = self.peers.app_databag

if not peer_data.get(SECRET_INTERNAL_LABEL):
return

if SECRET_CACHE_LABEL not in self.secrets[scope]:
for attempt in Retrying(stop=stop_after_attempt(3), wait=wait_fixed(1), reraise=True):
with attempt:
try:
# NOTE: Secret contents are not yet available!
secret = self.model.get_secret(id=peer_data[SECRET_INTERNAL_LABEL])
except SecretNotFoundError as e:
logging.debug(
f"No secret found for ID {peer_data[SECRET_INTERNAL_LABEL]}, {e}"
)
return

logging.debug(f"Secret {peer_data[SECRET_INTERNAL_LABEL]} downloaded")

# We keep the secret object around -- needed when applying modifications
self.secrets[scope][SECRET_LABEL] = secret

# We retrieve and cache actual secret data for the lifetime of the event scope
self.secrets[scope][SECRET_CACHE_LABEL] = secret.get_content()

return bool(self.secrets[scope].get(SECRET_CACHE_LABEL))

def _juju_secret_get_key(self, scope: str, key: str) -> Optional[str]:
if not key:
return

key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key))

if self._juju_secrets_get(scope):
secret_cache = self.secrets[scope].get(SECRET_CACHE_LABEL)
if secret_cache:
secret_data = secret_cache.get(key)
if secret_data and secret_data != SECRET_DELETED_LABEL:
logging.debug(f"Getting secret {scope}:{key}")
return secret_data
logging.debug(f"No value found for secret {scope}:{key}")

def get_secret(self, scope: str, key: str) -> Optional[str]:
"""Get secret from the secret storage."""
return self.peers.get_secret(scope, key)
if scope not in [APP_SCOPE, UNIT_SCOPE]:
raise RuntimeError("Unknown secret scope.")

if scope == UNIT_SCOPE:
result = self.peers.unit_databag.get(key, None)
else:
result = self.peers.app_databag.get(key, None)

# TODO change upgrade to switch to secrets once minor version upgrades is done
if result:
return result

juju_version = JujuVersion.from_environ()
if juju_version.has_secrets:
return self._juju_secret_get_key(scope, key)

def _juju_secret_set(self, scope: str, key: str, value: str) -> str:
"""Helper function setting Juju secret."""
if scope == UNIT_SCOPE:
peer_data = self.peers.unit_databag
else:
peer_data = self.peers.app_databag
self._juju_secrets_get(scope)

key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key))

def set_secret(self, scope: str, key: str, value: Optional[str]) -> None:
secret = self.secrets[scope].get(SECRET_LABEL)

# It's not the first secret for the scope, we can re-use the existing one
# that was fetched in the previous call
if secret:
secret_cache = self.secrets[scope][SECRET_CACHE_LABEL]

if secret_cache.get(key) == value:
logging.debug(f"Key {scope}:{key} has this value defined already")
else:
secret_cache[key] = value
try:
secret.set_content(secret_cache)
except OSError as error:
logging.error(
f"Error in attempt to set {scope}:{key}. "
f"Existing keys were: {list(secret_cache.keys())}. {error}"
)
logging.debug(f"Secret {scope}:{key} was {key} set")

# We need to create a brand-new secret for this scope
else:
scope_obj = self._scope_obj(scope)

secret = scope_obj.add_secret({key: value})
if not secret:
raise RuntimeError(f"Couldn't set secret {scope}:{key}")

self.secrets[scope][SECRET_LABEL] = secret
self.secrets[scope][SECRET_CACHE_LABEL] = {key: value}
logging.debug(f"Secret {scope}:{key} published (as first). ID: {secret.id}")
peer_data.update({SECRET_INTERNAL_LABEL: secret.id})

return self.secrets[scope][SECRET_LABEL].id

def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]:
"""Set secret from the secret storage."""
self.peers.set_secret(scope, key, value)
if scope not in [APP_SCOPE, UNIT_SCOPE]:
raise RuntimeError("Unknown secret scope.")

if not value:
return self.remove_secret(scope, key)

juju_version = JujuVersion.from_environ()

if juju_version.has_secrets:
self._juju_secret_set(scope, key, value)
return
if scope == UNIT_SCOPE:
self.peers.unit_databag.update({key: value})
else:
self.peers.app_databag.update({key: value})

def _juju_secret_remove(self, scope: str, key: str) -> None:
"""Remove a Juju 3.x secret."""
self._juju_secrets_get(scope)

key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key))

secret = self.secrets[scope].get(SECRET_LABEL)
if not secret:
logging.error(f"Secret {scope}:{key} wasn't deleted: no secrets are available")
return

secret_cache = self.secrets[scope].get(SECRET_CACHE_LABEL)
if not secret_cache or key not in secret_cache:
logging.error(f"No secret {scope}:{key}")
return

secret_cache[key] = SECRET_DELETED_LABEL
secret.set_content(secret_cache)
logging.debug(f"Secret {scope}:{key}")

def remove_secret(self, scope: str, key: str) -> None:
"""Removing a secret."""
if scope not in [APP_SCOPE, UNIT_SCOPE]:
raise RuntimeError("Unknown secret scope.")

juju_version = JujuVersion.from_environ()
if juju_version.has_secrets:
return self._juju_secret_remove(scope, key)
if scope == UNIT_SCOPE:
del self.peers.unit_databag[key]
else:
del self.peers.app_databag[key]

def push_tls_files_to_workload(self, update_config: bool = True) -> bool:
"""Uploads TLS files to the workload container."""
Expand Down
15 changes: 15 additions & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,18 @@
AUTH_FILE_DATABAG_KEY = "auth_file"

EXTENSIONS_BLOCKING_MESSAGE = "bad relation request - remote app requested extensions, which are unsupported. Please remove this relation."

SECRET_LABEL = "secret"
SECRET_CACHE_LABEL = "cache"
SECRET_INTERNAL_LABEL = "internal-secret"
SECRET_DELETED_LABEL = "None"

APP_SCOPE = "app"
UNIT_SCOPE = "unit"

SECRET_KEY_OVERRIDES = {
"cfg_file": "cfg-file",
"ca": "cauth",
"monitoring_password": "monitoring-password",
"auth_file": "auth-file",
}
9 changes: 4 additions & 5 deletions src/relations/backend_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
from ops.pebble import ConnectionError

from constants import (
APP_SCOPE,
AUTH_FILE_DATABAG_KEY,
AUTH_FILE_PATH,
BACKEND_RELATION_NAME,
Expand Down Expand Up @@ -247,15 +248,13 @@ def _on_database_created(self, event: DatabaseCreatedEvent) -> None:
self.initialise_auth_function([self.database.database, PG])

# Add the monitoring user.
if not (
monitoring_password := self.charm.peers.get_secret("app", MONITORING_PASSWORD_KEY)
):
if not (monitoring_password := self.charm.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY)):
monitoring_password = pgb.generate_password()
self.charm.peers.set_secret("app", MONITORING_PASSWORD_KEY, monitoring_password)
self.charm.set_secret(APP_SCOPE, MONITORING_PASSWORD_KEY, monitoring_password)
hashed_monitoring_password = pgb.get_hashed_password(self.stats_user, monitoring_password)

auth_file = f'"{self.auth_user}" "{hashed_password}"\n"{self.stats_user}" "{hashed_monitoring_password}"'
self.charm.peers.set_secret("app", AUTH_FILE_DATABAG_KEY, auth_file)
self.charm.set_secret(APP_SCOPE, AUTH_FILE_DATABAG_KEY, auth_file)
self.charm.render_auth_file(auth_file)

cfg = self.charm.read_pgb_config()
Expand Down
19 changes: 12 additions & 7 deletions src/relations/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,13 @@
Application,
BlockedStatus,
MaintenanceStatus,
ModelError,
Relation,
Unit,
WaitingStatus,
)

from constants import EXTENSIONS_BLOCKING_MESSAGE
from constants import APP_SCOPE, EXTENSIONS_BLOCKING_MESSAGE

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -168,9 +169,12 @@ def _block_on_extensions(self, relation, remote_app_databag: Dict) -> bool:

def _get_relation_extensions(self, relation: Relation) -> List[str]:
"""Get enabled extensions for a relation."""
for data in relation.data.values():
if "extensions" in data:
return data["extensions"].split(",")
try:
for data in relation.data.values():
if "extensions" in data:
return data["extensions"].split(",")
except ModelError:
logger.warning(f"Unable to access relation data for relation {relation.id}")
return []

def _check_for_blocking_relations(self, relation_id: int) -> bool:
Expand Down Expand Up @@ -233,10 +237,11 @@ def _on_relation_joined(self, join_event: RelationJoinedEvent):
user = self._generate_username(join_event)

if self.charm.unit.is_leader():
password = pgb.generate_password()
self.charm.peers.add_user(user, password)
if not (password := self.charm.get_secret(APP_SCOPE, user)):
password = pgb.generate_password()
self.charm.peers.add_user(user, password)
Comment on lines +240 to +242
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If password is already generated and set, don't regenerate it. This was causing the consistent mattermost failures.

else:
password = self.charm.peers.app_databag.get(user)
password = self.charm.get_secret(APP_SCOPE, user)

if None in [database, password]:
# If database isn't available, defer
Expand Down
Loading