Skip to content

Commit

Permalink
[DPE-5976] Add admin user (#9)
Browse files Browse the repository at this point in the history
This PR adds the following functionality to the etcd-operator:
- automatically create an internal admin user (`root` as per [etcd
standard](https://etcd.io/docs/v3.5/op-guide/authentication/rbac/#special-users-and-roles))
- always enable authentication in the etcd cluster on startup
- methods `add_user()`, `update_password()` and `enable_auth()` in the
`EtcdClient` class
- a configuration option `system-users` with which users can configure a
secret_id. The corresponding secret has to be granted to the etcd
application and has to include the password as key `root=...`.
- unit and integration test coverage for the above mentioned
  • Loading branch information
reneradoi authored Dec 10, 2024
1 parent 4ea0531 commit 6ff42c1
Show file tree
Hide file tree
Showing 13 changed files with 474 additions and 44 deletions.
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,
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:
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"
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

0 comments on commit 6ff42c1

Please sign in to comment.