Skip to content

Commit

Permalink
[DPE-5710] Enable update_certs to throw KeyError (#494)
Browse files Browse the repository at this point in the history
There is a potential race condition within `
opensearch_relation_provider.py`, where we may ask for an user to be set
(and hence, eventually call `update_certs`) before the certificate is
ready. That is more often when we deploy a complete bundle at once.

This PR fixes the issue by allowing `update_certs` to detect the missing
`chain` config in the TLS secrets and throws a `KeyError`.

Closes: #493
  • Loading branch information
phvalguima authored Oct 29, 2024
1 parent cd1c034 commit 6e9b48c
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
20 changes: 16 additions & 4 deletions lib/charms/opensearch/v0/opensearch_relation_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,12 @@ def _on_index_requested(self, event: IndexRequestedEvent) -> None: # noqa
self.opensearch_provides.set_version(rel_id, self.opensearch.version)
self.opensearch_provides.set_credentials(rel_id, username, pwd)
self.opensearch_provides.set_index(rel_id, event.index)
self.update_certs(rel_id)
try:
self.update_certs(rel_id)
except KeyError:
logger.warning("unable to get ca_chain from secret")
event.defer()
return
self.update_endpoints(event.relation)

logger.info(f"new index {event.index} available")
Expand Down Expand Up @@ -359,20 +364,27 @@ def update_certs(self, relation_id, ca_chain=None):
"""Update TLS certs passed into this relation.
If ca_chain is not provided, it'll get the app-admin CA generated by the TLS charm.
Raises:
KeyError: if the CA chain can't be found in the secret.
"""
if not self.unit.is_leader():
# We're updating app databags in this function, so it can't be called on follower
# units. This is not checked in `set_tls_ca`, in data-interfaces.
return
try:
# If the ca_chain=None, then we try to load the CA from the app-admin secret.
_ch_chain = ca_chain or self.charm.secrets.get_object(
Scope.APP, CertType.APP_ADMIN.val
).get("chain")
# If neither ca_chain nor the secret exists, we'll raise a KeyError.
_ch_chain = (
ca_chain
or self.charm.secrets.get_object(Scope.APP, CertType.APP_ADMIN.val)["chain"]
)
except AttributeError:
# cert doesn't exist - presumably we don't yet have a TLS relation.
logger.warning("unable to get ca_chain")
return
except TypeError:
raise KeyError("unable to get ca_chain")
self.opensearch_provides.set_tls_ca(relation_id, _ch_chain)

def _on_relation_changed(self, event: RelationChangedEvent) -> None:
Expand Down
9 changes: 8 additions & 1 deletion lib/charms/opensearch/v0/opensearch_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,14 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
return

for relation in self.charm.opensearch_provider.relations:
self.charm.opensearch_provider.update_certs(relation.id, ca_chain)
try:
self.charm.opensearch_provider.update_certs(relation.id, ca_chain)
except KeyError:
# As we are setting the ca_chain, it should not be likely to happen a KeyError at
# update_certs. This logic is left for a very corner case.
logger.error("Error updating certificates in the relation: ca_chain not set.")
event.defer()
return

# broadcast secret updates for certs and CA to related sub-clusters
if self.charm.unit.is_leader() and self.charm.opensearch_peer_cm.is_provider(typ="main"):
Expand Down
19 changes: 19 additions & 0 deletions tests/unit/lib/test_opensearch_relation_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
NodeLockRelationName,
PeerRelationName,
)
from charms.opensearch.v0.constants_tls import CertType
from charms.opensearch.v0.helper_security import generate_password
from charms.opensearch.v0.models import (
App,
Expand Down Expand Up @@ -521,3 +522,21 @@ def test_avoid_removing_non_charmed_users_and_roles(
mock_remove_role.assert_called_once_with(
f"{ClientRelationName}_{self.client_second_rel_id}"
)

@patch("ops.model.Unit.is_leader")
@patch("charms.opensearch.v0.opensearch_secrets.OpenSearchSecrets.get_object")
def test_update_certs(
self,
mock_get_object,
mock_is_leader,
):
event = MagicMock()
mock_get_object.return_value = {"some_expected_key": "some_value"}
mock_is_leader.return_value = True
try:
self.opensearch_provider.update_certs(event)
except KeyError as e:
self.assertEqual(str(e), "'chain'")
else:
self.fail("KeyError not raised")
mock_get_object.assert_called_once_with(Scope.APP, CertType.APP_ADMIN.val)

0 comments on commit 6e9b48c

Please sign in to comment.