Skip to content

Commit

Permalink
Changes in response to PR revie IV.
Browse files Browse the repository at this point in the history
  • Loading branch information
juditnovak committed Apr 5, 2024
1 parent ff71cd9 commit 9fce645
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 60 deletions.
2 changes: 1 addition & 1 deletion charm_version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.0
1.0+058c0ac-dirty+058c0ac-dirty+058c0ac-dirty+ff71cd9-dirty
29 changes: 4 additions & 25 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

# from events.provider import ProviderEvents
from events.tls import TLSEvents
from literals import CHARM_KEY, CHARM_USERS, PEER, SUBSTRATE
from literals import CHARM_KEY, CHARM_USERS, PEER, RESTART_TIMEOUT, SUBSTRATE
from managers.config import ConfigManager
from managers.tls import TLSManager
from workload import ODWorkload
Expand Down Expand Up @@ -182,13 +182,12 @@ def _restart(self, event: EventBase) -> None:
logger.info(f"{self.unit.name} (re)starting...")
self.workload.restart()

# TODO: Here we may wanna put a healthcheck?
time.sleep(5)
start_time = time.time()
while not self.workload.alive and time.time() - start_time < RESTART_TIMEOUT:
time.sleep(5)

self.unit.status = ActiveStatus()

# self.update_client_data()

# --- CONVENIENCE METHODS ---

def init_server(self):
Expand Down Expand Up @@ -218,26 +217,6 @@ def init_server(self):
}
)

# def update_client_data(self) -> None:
# """Writes necessary relation data to all related applications."""
# if not self.state.ready or not self.unit.is_leader():
# return
#
# for client in self.state.clients:
# if not client.password:
# continue # skip as ACLs have not yet been added
#
# client.update(
# {
# "uris": client.uris,
# "endpoints": client.endpoints,
# "tls": client.tls,
# "username": client.username,
# "password": client.password,
# "chroot": client.chroot,
# }
# )


if __name__ == "__main__":
main(OpensearchDasboardsCharm)
20 changes: 5 additions & 15 deletions src/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import logging
import socket
import subprocess
from typing import List, Literal, MutableMapping, Optional, Tuple
from typing import List, Literal, MutableMapping, Optional

from charms.data_platform_libs.v0.data_interfaces import Data
from ops.model import Application, Relation, Unit
Expand Down Expand Up @@ -78,14 +78,12 @@ def __init__(
password: str = "",
endpoints: str = "",
tls: bool = False,
uris: str = "",
):
super().__init__(relation, data_interface, component, substrate)
self.app = component
self._password = password
self._endpoints = endpoints
self._tls = tls
self._uris = uris
self._local_app = local_app

@override
Expand Down Expand Up @@ -114,11 +112,6 @@ def endpoints(self) -> List[str]:
endpoints_str = self.relation_data.get("endpoints")
return endpoints_str.split(",") if endpoints_str else []

@property
def uris(self) -> List[str]:
"""Connection uris for the client application to connect with."""
return self.endpoints

@property
def tls(self) -> bool:
"""Flag to confirm whether or not is TLS enabled.
Expand Down Expand Up @@ -152,21 +145,18 @@ def __init__(
self.app = component

@property
def internal_user_credentials(self) -> dict[str, Tuple[str, str]]:
def internal_user_credentials(self) -> dict[str, str]:
"""The passwords for the internal quorum and super users.
Returns:
Dict of key username, value password
"""
credentials = {
user: (username, password)
return {
user: password
for user in CHARM_USERS
if (password := self.relation_data.get(f"{user}-password"))
and (username := self.relation_data.get(f"{user}-username", user))
}

return credentials

# -- TLS --

@property
Expand All @@ -192,7 +182,7 @@ def __init__(
def unit_id(self) -> int:
"""The id of the unit from the unit name.
e.g zookeeper/2 --> 2
e.g opensearch-dashboards/2 --> 2
"""
return int(self.component.name.split("/")[1])

Expand Down
2 changes: 1 addition & 1 deletion src/events/password_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def __init__(self, charm):

def _get_password_action(self, event: ActionEvent) -> None:
"""Handler for get-super-password action event."""
username = event.params.get("username", "admin")
username = event.params.get("username", CHARM_USERS[0])
event.set_results(
{f"{username}-password": self.charm.state.cluster.internal_user_credentials[username]}
)
Expand Down
2 changes: 1 addition & 1 deletion src/events/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
logger.error("Can't use certificate, found unknown CSR")
return

# if certificate already exists, this event must be new, flag manual restart
# if certificate already exists, this event must be new, flag restart
if self.charm.state.unit_server.certificate:
self.charm.on[f"{self.charm.restart.name}"].acquire_lock.emit()

Expand Down
4 changes: 3 additions & 1 deletion src/literals.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# DASHBOARD_INDEX = ".opensearch-dashboards"
DASHBOARD_INDEX = "admin-index"
CONTAINER = "opensearch-dashboards"
CHARM_USERS = ["admin", "kibanaserver"]
CHARM_USERS = ["monitor"]
CERTS_REL_NAME = "certificates"
SERVER_PORT = 5601

Expand All @@ -32,3 +32,5 @@
"kibanaserver-password",
]
PEER_UNIT_SECRETS = ["ca-cert", "csr", "certificate", "private-key"]

RESTART_TIMEOUT = 30
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async def recreate_opensearch_kibanaserver(ops_test: OpsTest):

@pytest.mark.group(1)
@pytest.mark.abort_on_fail
@pytest.mark.password_rotation
@pytest.mark.charm
async def test_deploy_active(ops_test: OpsTest):

charm = await ops_test.build_charm(".")
Expand Down Expand Up @@ -163,8 +163,9 @@ async def test_dashboard_access_https(ops_test: OpsTest):
@pytest.mark.abort_on_fail
async def test_local_password_rotation(ops_test: OpsTest):
"""Test password rotation for local users -- in case we decide to have any."""
user = "kibanaserver"
user = "monitor"
password = await get_user_password(ops_test, user)
assert len(password) == 32

leader = None
for unit in ops_test.model.applications[APP_NAME].units:
Expand All @@ -186,6 +187,7 @@ async def test_local_password_rotation(ops_test: OpsTest):
new_password = await get_user_password(ops_test, user)

assert password != new_password
assert len(password) == 32


@pytest.mark.group(1)
Expand Down
14 changes: 0 additions & 14 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,3 @@ commands =
poetry run pytest -vv --tb native --log-cli-level=INFO --ignore={[vars]tests_path}/unit/ {posargs}
commands_post =
{[testenv:build-wrapper]commands_post}


[testenv:integration-{provider,password-rotation,tls,upgrade,ha,replication}]
description = Run integration tests
set_env =
{[testenv]set_env}
# Workaround for https://github.com/python-poetry/poetry/issues/6958
POETRY_INSTALLER_PARALLEL = false
pass_env =
{[testenv]pass_env}
CI
commands =
poetry --no-root install --with integration
poetry run pytest -vv --tb native --log-cli-level=INFO -s {posargs} {[vars]tests_path}/integration/{env:TEST_FILE}

0 comments on commit 9fce645

Please sign in to comment.