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-5976] Add admin user #9

Merged
merged 53 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
27c9541
disable allure reports for now (will be added later)
reneradoi Nov 11, 2024
7e8b0a7
implement `on_install` hook
reneradoi Nov 11, 2024
262bacd
fix linting, add snap lib
reneradoi Nov 11, 2024
28c34ef
implement peer relation handler
reneradoi Nov 12, 2024
97d2cde
fix linting
reneradoi Nov 12, 2024
bf1c3a2
move base events from charm.py to `EtcdEvents` class
reneradoi Nov 12, 2024
723d2c0
temporarily move population peer relation unit data to start-hook
reneradoi Nov 13, 2024
a1d9d75
add peer relation to unit test
reneradoi Nov 13, 2024
0396f3e
add alive-check for snap, add handler for `update_status` event
reneradoi Nov 13, 2024
6531227
fix unit testing
reneradoi Nov 14, 2024
a4939b8
decouple cluster manager from charm.py, should be instantiated by eve…
reneradoi Nov 14, 2024
9c0e820
implement basic ConfigManager
reneradoi Nov 14, 2024
77d2395
add basic configuration required for initial startup of etcd cluster
reneradoi Nov 15, 2024
ccae158
use `pathlib` instead of `os` to create filepath for config file
reneradoi Nov 18, 2024
84c378f
use `Path` object to create directory path and write file
reneradoi Nov 19, 2024
0142dc4
rename peer-relation to `etcd-peers` for consistency within data plat…
reneradoi Nov 20, 2024
eb6bf16
use `pop` instead of `del` for deleting keys from relation data in or…
reneradoi Nov 20, 2024
d023cab
use `app.name` instead of hardcoded `etcd` for cluster-member-name
reneradoi Nov 20, 2024
d89d7d0
use `socket.gethostbyname` for ip address
reneradoi Nov 20, 2024
e39a322
add retry-mechanism to workload-init and installation
reneradoi Nov 20, 2024
81fe2d9
WIP: add `EtcdClient` class for executing `etcdctl` commands, add `ge…
reneradoi Nov 22, 2024
38800a9
WIP: refactor - managers should be instantiated from the charm, not f…
reneradoi Nov 25, 2024
233d0de
WIP: add generic method to execute `etcdctl` commands
reneradoi Nov 25, 2024
834efd9
add unit test coverage for EtcdClient and ClusterManager
reneradoi Nov 25, 2024
ecc1da7
Merge branch 'main' into add_cluster_manager
reneradoi Nov 25, 2024
6e3879b
doc string comment
reneradoi Nov 25, 2024
e45b742
minor changes on comments, logging and naming
reneradoi Nov 25, 2024
fc7f80a
return `None` instead of empty strings to enforce type checking
reneradoi Nov 26, 2024
b8ea77f
Move `EtcdClient` to a separate file
reneradoi Nov 26, 2024
01fe2ab
create internal admin user `on_install`
reneradoi Nov 28, 2024
a6abdf0
adjust unit test
reneradoi Nov 29, 2024
8bc9e13
add username/password to `EtcdClient`, add methods for adding users a…
reneradoi Nov 29, 2024
4987a13
Merge branch 'main' into add_admin_user
reneradoi Nov 29, 2024
3025864
use `typing.Optional` for optional arguments
reneradoi Nov 29, 2024
bc351ff
adjust handling of admin user and password
reneradoi Nov 29, 2024
924e99d
add authentication to EtcdClient
reneradoi Dec 2, 2024
355b6b9
adjust logging in case of subprocess errors
reneradoi Dec 3, 2024
c633b15
Merge branch 'main' into add_admin_user
reneradoi Dec 3, 2024
d1dc685
fix merging main into branch
reneradoi Dec 3, 2024
a7d0459
implement `get-password` and `set-password` actions
reneradoi Dec 3, 2024
e8e677f
fix integration test by adding authentication
reneradoi Dec 3, 2024
31f7864
add unit test coverage
reneradoi Dec 4, 2024
fb37c44
add integration test coverage
reneradoi Dec 4, 2024
7404ac7
only enable authentication if not already done
reneradoi Dec 5, 2024
9886c5f
address PR review feedback
reneradoi Dec 6, 2024
124d5fd
fix replacing `Optional` with `str | None`
reneradoi Dec 6, 2024
2c5ebe6
fix type hint
reneradoi Dec 6, 2024
ec03f83
remove implementation for `get-password` and `set-password` user actions
reneradoi Dec 6, 2024
8ee63e5
WIP: configure admin's user password via juju user secret
reneradoi Dec 9, 2024
ea8b4c9
add test coverage
reneradoi Dec 9, 2024
0af4d26
fix linting
reneradoi Dec 9, 2024
fa8cee1
add test for password to be still valid if config option was removed
reneradoi Dec 9, 2024
66bee61
rename config option to `system-users`, allow for storing multiple in…
reneradoi Dec 10, 2024
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
11 changes: 11 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.

options:
system-users:
type: secret
description: |
Configure the internal system user and it's password. The password will
be auto-generated if this option is not set. It is for internal use only
and SHOULD NOT be used by applications. This needs to be a Juju Secret URI pointing
to a secret that contains the following content: `root: <password>`.
113 changes: 97 additions & 16 deletions src/common/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import logging
import subprocess

from literals import SNAP_NAME
from common.exceptions import EtcdAuthNotEnabledError, EtcdUserManagementError
from literals import INTERNAL_USER, SNAP_NAME

logger = logging.getLogger(__name__)

Expand All @@ -18,9 +19,13 @@ class EtcdClient:

def __init__(
self,
username: str,
skourta marked this conversation as resolved.
Show resolved Hide resolved
password: str,
client_url: str,
):
self.client_url = client_url
self.user = username
self.password = password

def get_endpoint_status(self) -> dict:
"""Run the `endpoint status` command and return the result as dict."""
Expand All @@ -31,29 +36,85 @@ def get_endpoint_status(self) -> dict:
endpoints=self.client_url,
output_format="json",
):
endpoint_status = json.loads(result)[0]
try:
endpoint_status = json.loads(result)[0]
except json.JSONDecodeError:
pass

return endpoint_status

def _run_etcdctl(
def add_user(self, username: str) -> None:
"""Add a user to etcd."""
if result := self._run_etcdctl(
command="user",
subcommand="add",
endpoints=self.client_url,
user=username,
# only admin user is added with password, all others require `CommonName` based auth
user_password=self.password if username == INTERNAL_USER else "",
):
logger.debug(result)
else:
raise EtcdUserManagementError(f"Failed to add user {self.user}.")

def update_password(self, username: str, new_password: str) -> None:
"""Run the `user passwd` command in etcd."""
if result := self._run_etcdctl(
command="user",
subcommand="passwd",
endpoints=self.client_url,
auth_username=self.user,
auth_password=self.password,
user=username,
use_input=new_password,
):
logger.debug(f"{result} for user {username}.")
else:
raise EtcdUserManagementError(f"Failed to update user {username}.")

def enable_auth(self) -> None:
"""Enable authentication in etcd."""
if result := self._run_etcdctl(
command="auth",
subcommand="enable",
endpoints=self.client_url,
):
logger.debug(result)
else:
raise EtcdAuthNotEnabledError("Failed to enable authentication in etcd.")

def _run_etcdctl( # noqa: C901
self,
command: str,
subcommand: str | None,
endpoints: str,
output_format: str | None = "simple",
subcommand: str | None = None,
# We need to be able to run `etcdctl` without user/pw
# otherwise it will error if auth is not yet enabled
# this is relevant for `user add` and `auth enable` commands
auth_username: str | None = None,
auth_password: str | None = None,
user: str | None = None,
user_password: str | None = None,
output_format: str = "simple",
use_input: str | None = None,
) -> str | None:
"""Execute `etcdctl` command via subprocess.

The list of arguments will be extended once authentication/encryption is implemented.
This method aims to provide a very clear interface for executing `etcdctl` and minimize
the margin of error on cluster operations.
the margin of error on cluster operations. The following arguments can be passed to the
`etcdctl` command as parameters.

Args:
command: command to execute with etcdctl, e.g. `elect`, `member` or `endpoint`
subcommand: subcommand to add to the previous command, e.g. `add` or `status`
endpoints: str-formatted list of endpoints to run the command against
auth_username: username used for authentication
auth_password: password used for authentication
user: username to be added or updated in etcd
user_password: password to be set for the user that is added to etcd
output_format: set the output format (fields, json, protobuf, simple, table)
...
use_input: supply text input to be passed to the `etcdctl` command (e.g. for
non-interactive password change)

Returns:
The output of the subprocess-command as a string. In case of error, this will
Expand All @@ -62,20 +123,40 @@ def _run_etcdctl(
might differ.
"""
try:
args = [f"{SNAP_NAME}.etcdctl", command]
if subcommand:
args.append(subcommand)
if user:
args.append(user)
if user_password == "":
args.append("--no-password=True")
elif user_password:
args.append(f"--new-user-password={user_password}")
if endpoints:
args.append(f"--endpoints={endpoints}")
if auth_username:
args.append(f"--user={auth_username}")
if auth_password:
args.append(f"--password={auth_password}")
if output_format:
args.append(f"-w={output_format}")
if use_input:
args.append("--interactive=False")

result = subprocess.run(
args=[
f"{SNAP_NAME}.etcdctl",
command,
subcommand,
f"--endpoints={endpoints}",
f"-w={output_format}",
],
args=args,
check=True,
capture_output=True,
text=True,
input=use_input if use_input else "",
).stdout.strip()
except subprocess.CalledProcessError as e:
logger.warning(e)
logger.error(
f"etcdctl {command} command failed: returncode: {e.returncode}, error: {e.stderr}"
)
return None
except subprocess.TimeoutExpired as e:
logger.error(f"Timed out running etcdctl: {e.stderr}")
return None

return result
17 changes: 17 additions & 0 deletions src/common/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env python3
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.

"""Charm-specific exceptions."""


class RaftLeaderNotFoundError(Exception):
"""Custom Exception if there is no current Raft leader."""


class EtcdUserManagementError(Exception):
"""Custom Exception if user could not be added or updated in etcd cluster."""


class EtcdAuthNotEnabledError(Exception):
"""Custom Exception if authentication could not be enabled in the etcd cluster."""
22 changes: 22 additions & 0 deletions src/common/secrets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.

"""Utility functions related to secrets."""

import logging

from ops.model import ModelError, SecretNotFoundError

logger = logging.getLogger(__name__)


def get_secret_from_id(model, secret_id: str) -> dict[str, str] | None:
"""Resolve the given id of a Juju secret and return the content as a dict."""
try:
secret_content = model.get_secret(id=secret_id).get_content(refresh=True)
except SecretNotFoundError:
raise SecretNotFoundError(f"The secret '{secret_id}' does not exist.")
except ModelError:
skourta marked this conversation as resolved.
Show resolved Hide resolved
raise

return secret_content
6 changes: 4 additions & 2 deletions src/core/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from ops import Object, Relation, Unit

from core.models import EtcdCluster, EtcdServer
from literals import PEER_RELATION, SUBSTRATES
from literals import PEER_RELATION, SECRETS_APP, SUBSTRATES

if TYPE_CHECKING:
from charm import EtcdOperatorCharm
Expand All @@ -29,7 +29,9 @@ class ClusterState(Object):
def __init__(self, charm: "EtcdOperatorCharm", substrate: SUBSTRATES):
super().__init__(parent=charm, key="charm_state")
self.substrate: SUBSTRATES = substrate
self.peer_app_interface = DataPeerData(self.model, relation_name=PEER_RELATION)
self.peer_app_interface = DataPeerData(
self.model, relation_name=PEER_RELATION, additional_secret_fields=SECRETS_APP
)
self.peer_unit_interface = DataPeerUnitData(self.model, relation_name=PEER_RELATION)

@property
Expand Down
21 changes: 14 additions & 7 deletions src/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
"""Collection of state objects for the Etcd relations, apps and units."""

import logging
from collections.abc import MutableMapping

from charms.data_platform_libs.v0.data_interfaces import Data, DataPeerData, DataPeerUnitData
from ops.model import Application, Relation, Unit

from literals import CLIENT_PORT, PEER_PORT, SUBSTRATES
from literals import CLIENT_PORT, INTERNAL_USER, PEER_PORT, SUBSTRATES

logger = logging.getLogger(__name__)

Expand All @@ -31,11 +30,6 @@ def __init__(
self.substrate = substrate
self.relation_data = self.data_interface.as_dict(self.relation.id) if self.relation else {}

@property
def data(self) -> MutableMapping:
"""Data representing the state."""
return self.relation_data

def update(self, items: dict[str, str]) -> None:
"""Write to relation data."""
if not self.relation:
Expand Down Expand Up @@ -119,3 +113,16 @@ def __init__(
def initial_cluster_state(self) -> str:
"""The initial cluster state ('new' or 'existing') of the etcd cluster."""
return self.relation_data.get("initial_cluster_state", "")

@property
def internal_user_credentials(self) -> dict[str, str]:
"""Retrieve the credentials for the internal admin user."""
if password := self.relation_data.get(f"{INTERNAL_USER}-password"):
return {INTERNAL_USER: password}

return {}

@property
def auth_enabled(self) -> bool:
"""Flag to check if authentication is already enabled in the Cluster."""
return self.relation_data.get("authentication", "") == "enabled"
reneradoi marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 11 additions & 0 deletions src/core/workload.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

"""Base objects for workload operations across different substrates."""

import secrets
import string
from abc import ABC, abstractmethod


Expand All @@ -24,3 +26,12 @@ def alive(self) -> bool:
def write_file(self, content: str, file: str) -> None:
"""Write content to a file."""
pass

@staticmethod
def generate_password() -> str:
"""Create randomized string for use as app passwords.

Returns:
String of 32 randomized letter+digit characters
"""
return "".join([secrets.choice(string.ascii_letters + string.digits) for _ in range(32)])
Loading
Loading