From d56641e247868b4a9b46cb9601afff0d66e21d48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:18:59 +0000 Subject: [PATCH 1/6] Initial plan From 7e160523dbe6b1d43f209f3287884e298a5b4f41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:31:39 +0000 Subject: [PATCH 2/6] Switch TLS certificate mode from UNIT to APP and share via peer relation - Changed TLSCertificatesRequiresV4 mode from Mode.UNIT to Mode.APP - Added certificate sharing via haproxy-peer relation app databag - Leader unit shares certificate data (cert, chain, private key) to peers - Non-leader units read certificate data from peer relation - Added _get_tls_information() helper for leader/non-leader branching - Added _reconcile_certificates() to handle sharing after writing to disk - Hooked peer relation-changed event to reconcile loop - Updated test fixtures for APP mode (local_app_data for CSR data, leader=True) Agent-Logs-Url: https://github.com/canonical/haproxy-operator/sessions/984ade73-92cb-4e20-89e1-2f2c22483668 Co-authored-by: Thanhphan1147 <42444001+Thanhphan1147@users.noreply.github.com> --- haproxy-operator/src/charm.py | 77 +++++++++++++++---- haproxy-operator/src/tls_relation.py | 63 ++++++++++++++- haproxy-operator/tests/unit/conftest.py | 5 +- haproxy-operator/tests/unit/test_charm.py | 9 ++- .../tests/unit/test_ddos_protection.py | 2 + haproxy-operator/tests/unit/test_hsts.py | 3 + 6 files changed, 138 insertions(+), 21 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 4142529f0..d548bffea 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -135,7 +135,7 @@ def __init__(self, *args: typing.Any): self.haproxy_route_provider.on.data_available, self.haproxy_route_provider.on.data_removed, ], - mode=Mode.UNIT, + mode=Mode.APP, ) self._tls = TLSRelationService(self.model, self.certificates, self.recv_ca_certs) @@ -210,6 +210,9 @@ def __init__(self, *args: typing.Any): self.framework.observe( self.on[DDOS_PROTECTION_RELATION_NAME].relation_broken, self._on_config_changed ) + self.framework.observe( + self.on[HAPROXY_PEER_INTEGRATION].relation_changed, self._on_config_changed + ) @validate_config_and_tls(defer=False) def _on_install(self, _: typing.Any) -> None: @@ -292,22 +295,60 @@ def _reconcile(self) -> None: self._configure_haproxy_route(charm_state, ha_information) case _: if self.model.get_relation(TLS_CERT_RELATION): - # Reconcile certificates in case the certificates relation is present - tls_information = TLSInformation.from_charm(self, self.certificates) - self._tls.certificate_available(tls_information) + tls_information = self._get_tls_information() + if tls_information: + self._reconcile_certificates(tls_information) self.unit.set_ports(80) self.haproxy_service.reconcile_default(charm_state) self.unit.status = ops.ActiveStatus() + def _get_tls_information( + self, allow_no_certificates: bool = False + ) -> typing.Optional[TLSInformation]: + """Get TLS information from the TLS library (leader) or peer relation (non-leader). + + Args: + allow_no_certificates: If the charm can proceed without requesting any certificates. + + Returns: + TLSInformation if available, None otherwise. + """ + if self.unit.is_leader(): + return TLSInformation.from_charm(self, self.certificates, allow_no_certificates) + + peer_relation = self.model.get_relation(HAPROXY_PEER_INTEGRATION) + if not peer_relation: + return None + return TLSRelationService.get_tls_information_from_peer_relation( + peer_relation, self.app + ) + + def _reconcile_certificates(self, tls_information: TLSInformation) -> None: + """Reconcile certificates: write to disk and share via peer relation if leader. + + Args: + tls_information: TLSInformation charm state component. + """ + self._tls.certificate_available(tls_information) + if self.unit.is_leader(): + peer_relation = self.model.get_relation(HAPROXY_PEER_INTEGRATION) + if peer_relation: + self._tls.share_certificates_via_peer_relation( + peer_relation, tls_information + ) + def _configure_ingress( self, charm_state: CharmState, requirer_class: type[IngressRequirersInformation | IngressPerUnitRequirersInformation], ) -> None: """Configure the ingress or ingress-per-unit relation.""" - tls_information = TLSInformation.from_charm(self, self.certificates) - self._tls.certificate_available(tls_information) + tls_information = self._get_tls_information() + if not tls_information: + logger.info("TLS information not available yet, skipping ingress configuration.") + return + self._reconcile_certificates(tls_information) ingress_provider = ( self._ingress_provider @@ -329,9 +370,9 @@ def _configure_ingress( def _configure_legacy(self, charm_state: CharmState) -> None: """Configure the legacy mode.""" if self.model.get_relation(TLS_CERT_RELATION): - # Reconcile certificates in case the certificates relation is present - tls_information = TLSInformation.from_charm(self, self.certificates) - self._tls.certificate_available(tls_information) + tls_information = self._get_tls_information() + if tls_information: + self._reconcile_certificates(tls_information) legacy_invalid_requested_port: list[str] = [] required_ports: set[Port] = set() @@ -377,8 +418,14 @@ def _configure_haproxy_route( for frontend in haproxy_route_requirers_information.tcp_frontends ) ) - tls_information = TLSInformation.from_charm(self, self.certificates, allow_no_certificates) - self._tls.certificate_available(tls_information) + tls_information = self._get_tls_information(allow_no_certificates) + if not tls_information and not allow_no_certificates: + logger.info( + "TLS information not available yet, skipping haproxy-route configuration." + ) + return + if tls_information: + self._reconcile_certificates(tls_information) ddos_protection_config = DDosProtection.from_charm(self.ddos_requirer) spoe_oauth_info_list = SpoeAuthInformation.from_requirer(self.spoe_auth_requirer) @@ -490,7 +537,9 @@ def _on_ingress_per_unit_data_provided(self, _: IngressDataReadyEvent) -> None: """Handle the data-provided event for ingress-per-unit.""" self._reconcile() if self.unit.is_leader(): - tls_information = TLSInformation.from_charm(self, self.certificates) + tls_information = self._get_tls_information() + if not tls_information: + return for relation in self._ingress_per_unit_provider.relations: for unit in relation.units: if not self._ingress_per_unit_provider.is_unit_ready(relation, unit): @@ -516,7 +565,9 @@ def _on_ingress_data_provided(self, event: IngressPerAppDataProvidedEvent) -> No """ self._reconcile() if self.unit.is_leader(): - tls_information = TLSInformation.from_charm(self, self.certificates) + tls_information = self._get_tls_information() + if not tls_information: + return integration_data = self._ingress_provider.get_data(event.relation) path_prefix = f"{integration_data.app.model}-{integration_data.app.name}" self._ingress_provider.publish_url( diff --git a/haproxy-operator/src/tls_relation.py b/haproxy-operator/src/tls_relation.py index b9f5b1f30..314aec90d 100644 --- a/haproxy-operator/src/tls_relation.py +++ b/haproxy-operator/src/tls_relation.py @@ -4,6 +4,7 @@ # mypy guesses the relations might be None about all of them. """Haproxy TLS relation business logic.""" +import json import logging import typing from pathlib import Path @@ -17,13 +18,14 @@ ProviderCertificate, TLSCertificatesRequiresV4, ) -from ops.model import Model +from ops.model import Model, Relation from haproxy import file_exists, read_file, render_file from state.haproxy_route import HAPROXY_CAS_DIR, HAPROXY_CAS_FILE from state.tls import TLSInformation TLS_CERT = "certificates" +PEER_TLS_KEY = "tls_certificate_data" HAPROXY_CERTS_DIR = Path("/var/lib/haproxy/certs") logger = logging.getLogger() @@ -80,9 +82,6 @@ def certificate_available(self, tls_information: TLSInformation) -> None: Args: tls_information: TLSInformation charm state component. """ - if len(self.certificates.certificate_requests) == 0: - logger.warning("No certificate was requested") - return for certificate, chain in tls_information.tls_cert_and_ca_chain.values(): if not self._certificate_matches_stored_content( certificate=certificate, @@ -95,6 +94,62 @@ def certificate_available(self, tls_information: TLSInformation) -> None: private_key=tls_information.private_key, ) + def share_certificates_via_peer_relation( + self, + peer_relation: Relation, + tls_information: TLSInformation, + ) -> None: + """Share TLS certificate data with peer units via the peer relation app databag. + + Args: + peer_relation: The haproxy-peers relation. + tls_information: TLSInformation charm state component. + """ + certificates_data: dict[str, dict[str, typing.Any]] = {} + for hostname, (certificate, chain) in tls_information.tls_cert_and_ca_chain.items(): + certificates_data[hostname] = { + "certificate": str(certificate), + "chain": [str(cert) for cert in chain], + } + data = { + "hostnames": tls_information.hostnames, + "certificates": certificates_data, + "private_key": str(tls_information.private_key), + } + peer_relation.data[self.application][PEER_TLS_KEY] = json.dumps(data) + logger.info("Shared TLS certificate data via peer relation.") + + @staticmethod + def get_tls_information_from_peer_relation( + peer_relation: Relation, + app: typing.Any, + ) -> typing.Optional[TLSInformation]: + """Read TLS certificate data from the peer relation app databag. + + Args: + peer_relation: The haproxy-peers relation. + app: The application object to read from the relation databag. + + Returns: + TLSInformation if available, None otherwise. + """ + raw = peer_relation.data[app].get(PEER_TLS_KEY) + if not raw: + return None + data = json.loads(raw) + hostnames = data["hostnames"] + private_key = data["private_key"] + tls_cert_and_ca_chain: dict[str, tuple[Certificate, list[Certificate]]] = {} + for hostname, cert_data in data["certificates"].items(): + certificate = Certificate.from_string(cert_data["certificate"]) + chain = [Certificate.from_string(c) for c in cert_data["chain"]] + tls_cert_and_ca_chain[hostname] = (certificate, chain) + return TLSInformation( + hostnames=hostnames, + tls_cert_and_ca_chain=tls_cert_and_ca_chain, + private_key=private_key, + ) + def update_trusted_cas(self) -> None: """Handle the change in the set of CAs to trust.""" ca_certificates = self.recv_ca_cert.get_all_certificates() diff --git a/haproxy-operator/tests/unit/conftest.py b/haproxy-operator/tests/unit/conftest.py index 5164a919d..cc6d40149 100644 --- a/haproxy-operator/tests/unit/conftest.py +++ b/haproxy-operator/tests/unit/conftest.py @@ -251,7 +251,7 @@ def certificates_integration_fixture(certificates_relation_data, csr_certificate return scenario.Relation( endpoint="certificates", remote_app_data=certificates_relation_data, - local_unit_data={ + local_app_data={ "certificate_signing_requests": json.dumps( [ { @@ -312,6 +312,7 @@ def base_state_with_ingress_fixture(peer_relation, ingress_integration, certific Yield: The modeled haproxy-peers relation. """ input_state = { + "leader": True, "relations": [peer_relation, ingress_integration, certificates_integration], "config": { "external-hostname": "ingress.local", @@ -345,6 +346,7 @@ def base_state_haproxy_route_fixture( Yield: The modeled haproxy-peers relation. """ input_state = { + "leader": True, "relations": [ peer_relation, certificates_integration, @@ -413,6 +415,7 @@ def base_state_with_ingress_per_unit_fixture( Yield: The modeled haproxy-peers relation. """ input_state = { + "leader": True, "relations": [peer_relation, ingress_per_unit_integration, certificates_integration], "config": { "external-hostname": "ingress.local", diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 4575d0ca6..9bb440ab0 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -338,7 +338,8 @@ def test_spoe_auth(monkeypatch: pytest.MonkeyPatch, certificates_integration): ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( - relations=[certificates_integration, spoe_auth_relation, haproxy_route_relation] + leader=True, + relations=[certificates_integration, spoe_auth_relation, haproxy_route_relation], ) out = ctx.run( ctx.on.relation_changed(spoe_auth_relation), @@ -387,13 +388,14 @@ def test_two_spoe_auth(monkeypatch: pytest.MonkeyPatch, certificates_integration ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[ certificates_integration, spoe_auth_relation_1, spoe_auth_relation_2, haproxy_route_relation_1, haproxy_route_relation_2, - ] + ], ) out = ctx.run( ctx.on.relation_changed(spoe_auth_relation_1), @@ -441,7 +443,8 @@ def test_spoe_auth_invalid_data(monkeypatch: pytest.MonkeyPatch, certificates_in ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( - relations=[certificates_integration, spoe_auth_relation, haproxy_route_relation] + leader=True, + relations=[certificates_integration, spoe_auth_relation, haproxy_route_relation], ) out = ctx.run( ctx.on.relation_changed(spoe_auth_relation), diff --git a/haproxy-operator/tests/unit/test_ddos_protection.py b/haproxy-operator/tests/unit/test_ddos_protection.py index ab8a98b59..a4180fd5e 100644 --- a/haproxy-operator/tests/unit/test_ddos_protection.py +++ b/haproxy-operator/tests/unit/test_ddos_protection.py @@ -26,6 +26,7 @@ def test_ddos_protection_enabled(monkeypatch: pytest.MonkeyPatch, certificates_i ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[certificates_integration, haproxy_route_relation], ) out = ctx.run( @@ -64,6 +65,7 @@ def test_ddos_protection_disabled(monkeypatch: pytest.MonkeyPatch, certificates_ ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[certificates_integration, haproxy_route_relation], config={"ddos-protection": False}, ) diff --git a/haproxy-operator/tests/unit/test_hsts.py b/haproxy-operator/tests/unit/test_hsts.py index 5098404e6..e72149688 100644 --- a/haproxy-operator/tests/unit/test_hsts.py +++ b/haproxy-operator/tests/unit/test_hsts.py @@ -29,6 +29,7 @@ def test_hsts_disabled(monkeypatch: pytest.MonkeyPatch, certificates_integration ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[certificates_integration, haproxy_route_relation], ) out = ctx.run( @@ -56,6 +57,7 @@ def test_hsts_enabled(monkeypatch: pytest.MonkeyPatch, certificates_integration) ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[certificates_integration, haproxy_route_relation], config={"enable-hsts": True}, ) @@ -88,6 +90,7 @@ def test_hsts_disabled_allow_http(monkeypatch: pytest.MonkeyPatch, certificates_ ctx = ops.testing.Context(HAProxyCharm) state = ops.testing.State( + leader=True, relations=[certificates_integration, haproxy_route_relation], config={"enable-hsts": True}, ) From fa535df48b580df4a95af878504df868251db157 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:36:10 +0000 Subject: [PATCH 3/6] Add unit tests for certificate sharing via peer relation - Test leader shares certificates to peer relation app databag - Test non-leader reads certificates from peer relation - Test non-leader without peer data skips TLS gracefully - Test non-leader haproxy-route reads from peer relation - Test share_certificates_via_peer_relation method directly - Test get_tls_information_from_peer_relation with data - Test get_tls_information_from_peer_relation without data returns None Agent-Logs-Url: https://github.com/canonical/haproxy-operator/sessions/984ade73-92cb-4e20-89e1-2f2c22483668 Co-authored-by: Thanhphan1147 <42444001+Thanhphan1147@users.noreply.github.com> --- .../tests/unit/legacy/test_tls_relation.py | 102 +++++++++++ haproxy-operator/tests/unit/test_charm.py | 167 ++++++++++++++++++ 2 files changed, 269 insertions(+) diff --git a/haproxy-operator/tests/unit/legacy/test_tls_relation.py b/haproxy-operator/tests/unit/legacy/test_tls_relation.py index d641e94b5..240edbbd3 100644 --- a/haproxy-operator/tests/unit/legacy/test_tls_relation.py +++ b/haproxy-operator/tests/unit/legacy/test_tls_relation.py @@ -133,3 +133,105 @@ def test_write_certificate_to_unit( pem_file_content = f"{mock_certificate!s}\n{chain_string}\n{mock_private_key!s}" write_text_mock.assert_called_once_with(pem_file_content, encoding="utf-8") + + +def test_share_certificates_via_peer_relation( + harness: Harness, + mock_certificate_and_key: typing.Tuple[Certificate, PrivateKey], +): + """arrange: Given a TLSRelationService and TLS information. + act: Run share_certificates_via_peer_relation. + assert: Peer relation app databag contains serialized certificate data. + """ + mock_certificate, mock_private_key = mock_certificate_and_key + harness.begin() + harness.set_leader(True) + tls_relation_service = TLSRelationService( + harness.model, harness.charm.certificates, harness.charm.recv_ca_certs + ) + hostname = "haproxy.internal" + tls_information = TLSInformation( + hostnames=[hostname], + tls_cert_and_ca_chain={hostname: (mock_certificate, [mock_certificate])}, + private_key=str(mock_private_key), + ) + + peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") + peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) + + tls_relation_service.share_certificates_via_peer_relation(peer_relation, tls_information) + + import json + + from tls_relation import PEER_TLS_KEY + + raw_data = peer_relation.data[harness.model.app].get(PEER_TLS_KEY) + assert raw_data is not None + data = json.loads(raw_data) + assert data["hostnames"] == [hostname] + assert hostname in data["certificates"] + assert data["certificates"][hostname]["certificate"] == str(mock_certificate) + assert data["private_key"] == str(mock_private_key) + + +def test_get_tls_information_from_peer_relation( + harness: Harness, + mock_certificate_and_key: typing.Tuple[Certificate, PrivateKey], +): + """arrange: Given a peer relation with certificate data in app databag. + act: Run get_tls_information_from_peer_relation. + assert: TLSInformation is correctly deserialized. + """ + import json + + from tls_relation import PEER_TLS_KEY + + mock_certificate, mock_private_key = mock_certificate_and_key + harness.begin() + harness.set_leader(True) + hostname = "haproxy.internal" + peer_data = json.dumps( + { + "hostnames": [hostname], + "certificates": { + hostname: { + "certificate": str(mock_certificate), + "chain": [str(mock_certificate)], + } + }, + "private_key": str(mock_private_key), + } + ) + + peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") + harness.update_relation_data(peer_relation_id, harness.model.app.name, {PEER_TLS_KEY: peer_data}) + peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) + + result = TLSRelationService.get_tls_information_from_peer_relation( + peer_relation, harness.model.app + ) + + assert result is not None + assert result.hostnames == [hostname] + assert hostname in result.tls_cert_and_ca_chain + certificate, chain = result.tls_cert_and_ca_chain[hostname] + assert str(certificate) == str(mock_certificate) + assert len(chain) == 1 + assert result.private_key == str(mock_private_key) + + +def test_get_tls_information_from_peer_relation_empty( + harness: Harness, +): + """arrange: Given a peer relation with no certificate data. + act: Run get_tls_information_from_peer_relation. + assert: None is returned. + """ + harness.begin() + peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") + peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) + + result = TLSRelationService.get_tls_information_from_peer_relation( + peer_relation, harness.model.app + ) + assert result is None diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 9bb440ab0..29f020e75 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -453,3 +453,170 @@ def test_spoe_auth_invalid_data(monkeypatch: pytest.MonkeyPatch, certificates_in assert render_file_mock.call_count == 0 assert out.unit_status.name == ops.testing.BlockedStatus.name assert spoe_auth_relation.remote_app_name in out.unit_status.message + + +@pytest.mark.usefixtures("systemd_mock", "mocks_external_calls") +class TestCertificateSharingViaPeerRelation: + """Tests for sharing certificates between units via peer relation in APP mode.""" + + def test_leader_shares_certificates_to_peer_relation( + self, + monkeypatch: pytest.MonkeyPatch, + mock_certificate_and_key, + certificates_integration, + peer_relation, + ): + """ + arrange: Prepare a leader unit with haproxy-route and certificates. + act: Trigger config_changed. + assert: Certificate data is written to the peer relation app databag. + """ + mock_certificate, mock_private_key = mock_certificate_and_key + monkeypatch.setattr("haproxy.render_file", MagicMock()) + monkeypatch.setattr("haproxy.HAProxyService.reconcile_haproxy_route", MagicMock()) + monkeypatch.setattr( + "tls_relation.TLSRelationService.write_certificate_to_unit", MagicMock() + ) + monkeypatch.setattr( + "charm.HAProxyCharm._get_unit_address", MagicMock(return_value="10.0.0.1") + ) + + haproxy_route_relation = build_haproxy_route_relation() + + ctx = ops.testing.Context(HAProxyCharm) + state = ops.testing.State( + leader=True, + relations=[peer_relation, certificates_integration, haproxy_route_relation], + config={"external-hostname": TEST_EXTERNAL_HOSTNAME_CONFIG}, + ) + out = ctx.run(ctx.on.config_changed(), state) + + out_peer_relation = [r for r in out.relations if r.endpoint == "haproxy-peers"][0] + peer_app_data = out_peer_relation.local_app_data + assert tls_relation.PEER_TLS_KEY in peer_app_data + + shared_data = json.loads(peer_app_data[tls_relation.PEER_TLS_KEY]) + assert "hostnames" in shared_data + assert "certificates" in shared_data + assert "private_key" in shared_data + assert shared_data["private_key"] == str(mock_private_key) + + def test_non_leader_reads_certificates_from_peer_relation( + self, + monkeypatch: pytest.MonkeyPatch, + csr_certificate_and_key, + ): + """ + arrange: Prepare a non-leader unit with certificate data in the peer relation. + act: Trigger config_changed event. + assert: Unit reaches active status with TLS data from peer relation. + """ + _, certificate, private_key = csr_certificate_and_key + hostname = TEST_EXTERNAL_HOSTNAME_CONFIG + + write_cert_mock = MagicMock() + monkeypatch.setattr( + "tls_relation.TLSRelationService.write_certificate_to_unit", write_cert_mock + ) + monkeypatch.setattr("haproxy.render_file", MagicMock()) + + peer_tls_data = json.dumps( + { + "hostnames": [hostname], + "certificates": { + hostname: { + "certificate": str(certificate), + "chain": [str(certificate)], + } + }, + "private_key": str(private_key), + } + ) + peer_relation_with_tls = scenario.PeerRelation( + endpoint="haproxy-peers", + local_app_data={tls_relation.PEER_TLS_KEY: peer_tls_data}, + ) + + ctx = ops.testing.Context(HAProxyCharm) + state = ops.testing.State( + leader=False, + relations=[peer_relation_with_tls], + ) + out = ctx.run(ctx.on.config_changed(), state) + assert out.unit_status == ops.testing.ActiveStatus("") + + def test_non_leader_without_peer_data_skips_tls( + self, + monkeypatch: pytest.MonkeyPatch, + peer_relation, + ): + """ + arrange: Prepare a non-leader unit with no TLS data in the peer relation. + act: Trigger config_changed. + assert: Unit reaches active status without error (default mode, no TLS needed). + """ + monkeypatch.setattr("haproxy.render_file", MagicMock()) + + ctx = ops.testing.Context(HAProxyCharm) + state = ops.testing.State( + leader=False, + relations=[peer_relation], + ) + out = ctx.run(ctx.on.config_changed(), state) + assert out.unit_status == ops.testing.ActiveStatus("") + + def test_non_leader_haproxy_route_reads_from_peer_relation( + self, + monkeypatch: pytest.MonkeyPatch, + csr_certificate_and_key, + certificates_integration, + ): + """ + arrange: Prepare a non-leader unit with haproxy-route and certificate data in peer. + act: Trigger config_changed. + assert: The haproxy config is rendered with data from peer relation certificates. + """ + _, certificate, private_key = csr_certificate_and_key + hostname = TEST_EXTERNAL_HOSTNAME_CONFIG + reconcile_mock = MagicMock() + monkeypatch.setattr("haproxy.HAProxyService.reconcile_haproxy_route", reconcile_mock) + monkeypatch.setattr( + "tls_relation.TLSRelationService.write_certificate_to_unit", MagicMock() + ) + monkeypatch.setattr("haproxy.HAProxyService.install", MagicMock()) + monkeypatch.setattr( + "charm.HAProxyCharm._get_unit_address", MagicMock(return_value="10.0.0.2") + ) + + peer_tls_data = json.dumps( + { + "hostnames": [hostname], + "certificates": { + hostname: { + "certificate": str(certificate), + "chain": [str(certificate)], + } + }, + "private_key": str(private_key), + } + ) + peer_relation_with_tls = scenario.PeerRelation( + endpoint="haproxy-peers", + local_app_data={tls_relation.PEER_TLS_KEY: peer_tls_data}, + ) + + haproxy_route_relation = build_haproxy_route_relation() + + ctx = ops.testing.Context(HAProxyCharm) + state = ops.testing.State( + leader=False, + relations=[ + peer_relation_with_tls, + certificates_integration, + haproxy_route_relation, + ], + config={"external-hostname": TEST_EXTERNAL_HOSTNAME_CONFIG}, + ) + out = ctx.run(ctx.on.config_changed(), state) + reconcile_mock.assert_called_once() + assert out.unit_status == ops.testing.ActiveStatus("") From 5b10cce3c3d534ddd04df23609746f97c60c5de7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:40:36 +0000 Subject: [PATCH 4/6] Fix lint and mypy issues in tests and improve type annotation - Fix ruff RUF059: prefix unused variable with underscore - Fix ruff RUF015: use next() instead of list comprehension slice - Fix mypy union-attr: add assertions for optional relation types - Improve type annotation: use Application instead of typing.Any Agent-Logs-Url: https://github.com/canonical/haproxy-operator/sessions/984ade73-92cb-4e20-89e1-2f2c22483668 Co-authored-by: Thanhphan1147 <42444001+Thanhphan1147@users.noreply.github.com> --- haproxy-operator/src/charm.py | 12 +++--------- haproxy-operator/src/tls_relation.py | 4 ++-- .../tests/unit/legacy/test_tls_relation.py | 5 ++++- haproxy-operator/tests/unit/test_charm.py | 5 +++-- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index d548bffea..82f1d5a9a 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -320,9 +320,7 @@ def _get_tls_information( peer_relation = self.model.get_relation(HAPROXY_PEER_INTEGRATION) if not peer_relation: return None - return TLSRelationService.get_tls_information_from_peer_relation( - peer_relation, self.app - ) + return TLSRelationService.get_tls_information_from_peer_relation(peer_relation, self.app) def _reconcile_certificates(self, tls_information: TLSInformation) -> None: """Reconcile certificates: write to disk and share via peer relation if leader. @@ -334,9 +332,7 @@ def _reconcile_certificates(self, tls_information: TLSInformation) -> None: if self.unit.is_leader(): peer_relation = self.model.get_relation(HAPROXY_PEER_INTEGRATION) if peer_relation: - self._tls.share_certificates_via_peer_relation( - peer_relation, tls_information - ) + self._tls.share_certificates_via_peer_relation(peer_relation, tls_information) def _configure_ingress( self, @@ -420,9 +416,7 @@ def _configure_haproxy_route( ) tls_information = self._get_tls_information(allow_no_certificates) if not tls_information and not allow_no_certificates: - logger.info( - "TLS information not available yet, skipping haproxy-route configuration." - ) + logger.info("TLS information not available yet, skipping haproxy-route configuration.") return if tls_information: self._reconcile_certificates(tls_information) diff --git a/haproxy-operator/src/tls_relation.py b/haproxy-operator/src/tls_relation.py index 314aec90d..b8fc7f0fc 100644 --- a/haproxy-operator/src/tls_relation.py +++ b/haproxy-operator/src/tls_relation.py @@ -18,7 +18,7 @@ ProviderCertificate, TLSCertificatesRequiresV4, ) -from ops.model import Model, Relation +from ops.model import Application, Model, Relation from haproxy import file_exists, read_file, render_file from state.haproxy_route import HAPROXY_CAS_DIR, HAPROXY_CAS_FILE @@ -122,7 +122,7 @@ def share_certificates_via_peer_relation( @staticmethod def get_tls_information_from_peer_relation( peer_relation: Relation, - app: typing.Any, + app: Application, ) -> typing.Optional[TLSInformation]: """Read TLS certificate data from the peer relation app databag. diff --git a/haproxy-operator/tests/unit/legacy/test_tls_relation.py b/haproxy-operator/tests/unit/legacy/test_tls_relation.py index 240edbbd3..8a1e16317 100644 --- a/haproxy-operator/tests/unit/legacy/test_tls_relation.py +++ b/haproxy-operator/tests/unit/legacy/test_tls_relation.py @@ -158,6 +158,7 @@ def test_share_certificates_via_peer_relation( peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) + assert peer_relation is not None tls_relation_service.share_certificates_via_peer_relation(peer_relation, tls_information) @@ -204,7 +205,9 @@ def test_get_tls_information_from_peer_relation( ) peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") - harness.update_relation_data(peer_relation_id, harness.model.app.name, {PEER_TLS_KEY: peer_data}) + harness.update_relation_data( + peer_relation_id, harness.model.app.name, {PEER_TLS_KEY: peer_data} + ) peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) result = TLSRelationService.get_tls_information_from_peer_relation( diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 29f020e75..cacb9eb2b 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -471,7 +471,7 @@ def test_leader_shares_certificates_to_peer_relation( act: Trigger config_changed. assert: Certificate data is written to the peer relation app databag. """ - mock_certificate, mock_private_key = mock_certificate_and_key + _mock_certificate, mock_private_key = mock_certificate_and_key monkeypatch.setattr("haproxy.render_file", MagicMock()) monkeypatch.setattr("haproxy.HAProxyService.reconcile_haproxy_route", MagicMock()) monkeypatch.setattr( @@ -491,8 +491,9 @@ def test_leader_shares_certificates_to_peer_relation( ) out = ctx.run(ctx.on.config_changed(), state) - out_peer_relation = [r for r in out.relations if r.endpoint == "haproxy-peers"][0] + out_peer_relation = next(r for r in out.relations if r.endpoint == "haproxy-peers") peer_app_data = out_peer_relation.local_app_data + assert peer_app_data is not None assert tls_relation.PEER_TLS_KEY in peer_app_data shared_data = json.loads(peer_app_data[tls_relation.PEER_TLS_KEY]) From a3d6e5ade895a62efa9a74bcd89507ebba619c9e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:34:35 +0000 Subject: [PATCH 5/6] Move leader/non-leader TLS branching logic into TLSInformation.from_charm - Moved _get_tls_information() logic from charm.py into TLSInformation.from_charm() in state/tls.py per reviewer feedback - from_charm() now returns Optional[TLSInformation]: leader reads from TLS library, non-leader reads from peer relation via new _from_peer_relation() classmethod - Removed _get_tls_information() method from charm.py - Moved PEER_TLS_KEY constant to state/tls.py (canonical location for TLS state) - Removed get_tls_information_from_peer_relation() from TLSRelationService (logic now in TLSInformation._from_peer_relation) - Updated tests to test through TLSInformation.from_charm() Agent-Logs-Url: https://github.com/canonical/haproxy-operator/sessions/96fecf67-9c28-47d1-8590-78ac4718d7c2 Co-authored-by: Thanhphan1147 <42444001+Thanhphan1147@users.noreply.github.com> --- haproxy-operator/src/charm.py | 31 +++---------- haproxy-operator/src/state/tls.py | 46 ++++++++++++++++++- haproxy-operator/src/tls_relation.py | 36 +-------------- .../tests/unit/legacy/test_tls_relation.py | 37 +++++++-------- 4 files changed, 68 insertions(+), 82 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 82f1d5a9a..45472193d 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -295,7 +295,7 @@ def _reconcile(self) -> None: self._configure_haproxy_route(charm_state, ha_information) case _: if self.model.get_relation(TLS_CERT_RELATION): - tls_information = self._get_tls_information() + tls_information = TLSInformation.from_charm(self, self.certificates) if tls_information: self._reconcile_certificates(tls_information) @@ -303,25 +303,6 @@ def _reconcile(self) -> None: self.haproxy_service.reconcile_default(charm_state) self.unit.status = ops.ActiveStatus() - def _get_tls_information( - self, allow_no_certificates: bool = False - ) -> typing.Optional[TLSInformation]: - """Get TLS information from the TLS library (leader) or peer relation (non-leader). - - Args: - allow_no_certificates: If the charm can proceed without requesting any certificates. - - Returns: - TLSInformation if available, None otherwise. - """ - if self.unit.is_leader(): - return TLSInformation.from_charm(self, self.certificates, allow_no_certificates) - - peer_relation = self.model.get_relation(HAPROXY_PEER_INTEGRATION) - if not peer_relation: - return None - return TLSRelationService.get_tls_information_from_peer_relation(peer_relation, self.app) - def _reconcile_certificates(self, tls_information: TLSInformation) -> None: """Reconcile certificates: write to disk and share via peer relation if leader. @@ -340,7 +321,7 @@ def _configure_ingress( requirer_class: type[IngressRequirersInformation | IngressPerUnitRequirersInformation], ) -> None: """Configure the ingress or ingress-per-unit relation.""" - tls_information = self._get_tls_information() + tls_information = TLSInformation.from_charm(self, self.certificates) if not tls_information: logger.info("TLS information not available yet, skipping ingress configuration.") return @@ -366,7 +347,7 @@ def _configure_ingress( def _configure_legacy(self, charm_state: CharmState) -> None: """Configure the legacy mode.""" if self.model.get_relation(TLS_CERT_RELATION): - tls_information = self._get_tls_information() + tls_information = TLSInformation.from_charm(self, self.certificates) if tls_information: self._reconcile_certificates(tls_information) @@ -414,7 +395,7 @@ def _configure_haproxy_route( for frontend in haproxy_route_requirers_information.tcp_frontends ) ) - tls_information = self._get_tls_information(allow_no_certificates) + tls_information = TLSInformation.from_charm(self, self.certificates, allow_no_certificates) if not tls_information and not allow_no_certificates: logger.info("TLS information not available yet, skipping haproxy-route configuration.") return @@ -531,7 +512,7 @@ def _on_ingress_per_unit_data_provided(self, _: IngressDataReadyEvent) -> None: """Handle the data-provided event for ingress-per-unit.""" self._reconcile() if self.unit.is_leader(): - tls_information = self._get_tls_information() + tls_information = TLSInformation.from_charm(self, self.certificates) if not tls_information: return for relation in self._ingress_per_unit_provider.relations: @@ -559,7 +540,7 @@ def _on_ingress_data_provided(self, event: IngressPerAppDataProvidedEvent) -> No """ self._reconcile() if self.unit.is_leader(): - tls_information = self._get_tls_information() + tls_information = TLSInformation.from_charm(self, self.certificates) if not tls_information: return integration_data = self._ingress_provider.get_data(event.relation) diff --git a/haproxy-operator/src/state/tls.py b/haproxy-operator/src/state/tls.py index a7fd67d8b..0e27bdedd 100644 --- a/haproxy-operator/src/state/tls.py +++ b/haproxy-operator/src/state/tls.py @@ -3,7 +3,9 @@ """haproxy-operator charm tls information.""" +import json import logging +import typing from dataclasses import dataclass import ops @@ -13,6 +15,10 @@ TLSCertificatesRequiresV4, ) +from .ha import HAPROXY_PEER_INTEGRATION + +PEER_TLS_KEY = "tls_certificate_data" + logger = logging.getLogger() @@ -47,9 +53,12 @@ def from_charm( charm: ops.CharmBase, certificates: TLSCertificatesRequiresV4, allow_no_certificates: bool = False, - ) -> "TLSInformation": + ) -> typing.Optional["TLSInformation"]: """Get TLS information from a charm instance. + On leader units, reads certificates from the TLS library. + On non-leader units, reads certificates shared via the peer relation. + Args: charm: The haproxy charm. certificates: TLS certificates requirer class. @@ -59,8 +68,11 @@ def from_charm( PrivateKeyNotGeneratedError: When waiting for the private key to be generated. Returns: - TLSInformation: Information about configured TLS certs. + TLSInformation if available, None if non-leader and peer data not yet available. """ + if not charm.unit.is_leader(): + return cls._from_peer_relation(charm) + cls.validate(charm, certificates, allow_no_certificates) hostnames = [ @@ -86,6 +98,36 @@ def from_charm( private_key=str(private_key), ) + @classmethod + def _from_peer_relation(cls, charm: ops.CharmBase) -> typing.Optional["TLSInformation"]: + """Read TLS certificate data from the peer relation app databag. + + Args: + charm: The haproxy charm. + + Returns: + TLSInformation if available, None otherwise. + """ + peer_relation = charm.model.get_relation(HAPROXY_PEER_INTEGRATION) + if not peer_relation: + return None + raw = peer_relation.data[charm.app].get(PEER_TLS_KEY) + if not raw: + return None + data = json.loads(raw) + hostnames = data["hostnames"] + private_key = data["private_key"] + tls_cert_and_ca_chain: dict[str, tuple[Certificate, list[Certificate]]] = {} + for hostname, cert_data in data["certificates"].items(): + certificate = Certificate.from_string(cert_data["certificate"]) + chain = [Certificate.from_string(c) for c in cert_data["chain"]] + tls_cert_and_ca_chain[hostname] = (certificate, chain) + return cls( + hostnames=hostnames, + tls_cert_and_ca_chain=tls_cert_and_ca_chain, + private_key=private_key, + ) + # Validation is done in this method instead of using a pydantic model because # there are cases where we need to validate the state but we don't need the state instance. @classmethod diff --git a/haproxy-operator/src/tls_relation.py b/haproxy-operator/src/tls_relation.py index b8fc7f0fc..7d981cb42 100644 --- a/haproxy-operator/src/tls_relation.py +++ b/haproxy-operator/src/tls_relation.py @@ -18,14 +18,13 @@ ProviderCertificate, TLSCertificatesRequiresV4, ) -from ops.model import Application, Model, Relation +from ops.model import Model, Relation from haproxy import file_exists, read_file, render_file from state.haproxy_route import HAPROXY_CAS_DIR, HAPROXY_CAS_FILE -from state.tls import TLSInformation +from state.tls import PEER_TLS_KEY, TLSInformation TLS_CERT = "certificates" -PEER_TLS_KEY = "tls_certificate_data" HAPROXY_CERTS_DIR = Path("/var/lib/haproxy/certs") logger = logging.getLogger() @@ -119,37 +118,6 @@ def share_certificates_via_peer_relation( peer_relation.data[self.application][PEER_TLS_KEY] = json.dumps(data) logger.info("Shared TLS certificate data via peer relation.") - @staticmethod - def get_tls_information_from_peer_relation( - peer_relation: Relation, - app: Application, - ) -> typing.Optional[TLSInformation]: - """Read TLS certificate data from the peer relation app databag. - - Args: - peer_relation: The haproxy-peers relation. - app: The application object to read from the relation databag. - - Returns: - TLSInformation if available, None otherwise. - """ - raw = peer_relation.data[app].get(PEER_TLS_KEY) - if not raw: - return None - data = json.loads(raw) - hostnames = data["hostnames"] - private_key = data["private_key"] - tls_cert_and_ca_chain: dict[str, tuple[Certificate, list[Certificate]]] = {} - for hostname, cert_data in data["certificates"].items(): - certificate = Certificate.from_string(cert_data["certificate"]) - chain = [Certificate.from_string(c) for c in cert_data["chain"]] - tls_cert_and_ca_chain[hostname] = (certificate, chain) - return TLSInformation( - hostnames=hostnames, - tls_cert_and_ca_chain=tls_cert_and_ca_chain, - private_key=private_key, - ) - def update_trusted_cas(self) -> None: """Handle the change in the set of CAs to trust.""" ca_certificates = self.recv_ca_cert.get_all_certificates() diff --git a/haproxy-operator/tests/unit/legacy/test_tls_relation.py b/haproxy-operator/tests/unit/legacy/test_tls_relation.py index 8a1e16317..e8d59dcb7 100644 --- a/haproxy-operator/tests/unit/legacy/test_tls_relation.py +++ b/haproxy-operator/tests/unit/legacy/test_tls_relation.py @@ -21,11 +21,12 @@ def test_tls_information_integration_missing(harness: Harness): - """arrange: Given a charm with tls integration missing. + """arrange: Given a leader charm with tls integration missing. act: Initialize TLSInformation state component. assert: TLSNotReadyError is raised. """ harness.begin() + harness.set_leader(True) with pytest.raises(TLSNotReadyError): TLSInformation.from_charm(harness.charm, harness.charm.certificates) @@ -175,21 +176,20 @@ def test_share_certificates_via_peer_relation( assert data["private_key"] == str(mock_private_key) -def test_get_tls_information_from_peer_relation( +def test_non_leader_from_charm_reads_peer_relation( harness: Harness, mock_certificate_and_key: typing.Tuple[Certificate, PrivateKey], ): - """arrange: Given a peer relation with certificate data in app databag. - act: Run get_tls_information_from_peer_relation. - assert: TLSInformation is correctly deserialized. + """arrange: Given a non-leader unit with certificate data in the peer relation app databag. + act: Run TLSInformation.from_charm. + assert: TLSInformation is correctly deserialized from the peer relation. """ import json - from tls_relation import PEER_TLS_KEY + from state.tls import PEER_TLS_KEY mock_certificate, mock_private_key = mock_certificate_and_key - harness.begin() - harness.set_leader(True) + harness.set_leader(False) hostname = "haproxy.internal" peer_data = json.dumps( { @@ -203,16 +203,13 @@ def test_get_tls_information_from_peer_relation( "private_key": str(mock_private_key), } ) - peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") harness.update_relation_data( peer_relation_id, harness.model.app.name, {PEER_TLS_KEY: peer_data} ) - peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) + harness.begin() - result = TLSRelationService.get_tls_information_from_peer_relation( - peer_relation, harness.model.app - ) + result = TLSInformation.from_charm(harness.charm, harness.charm.certificates) assert result is not None assert result.hostnames == [hostname] @@ -223,18 +220,16 @@ def test_get_tls_information_from_peer_relation( assert result.private_key == str(mock_private_key) -def test_get_tls_information_from_peer_relation_empty( +def test_non_leader_from_charm_returns_none_without_peer_data( harness: Harness, ): - """arrange: Given a peer relation with no certificate data. - act: Run get_tls_information_from_peer_relation. + """arrange: Given a non-leader unit with no certificate data in the peer relation. + act: Run TLSInformation.from_charm. assert: None is returned. """ + harness.set_leader(False) + harness.add_relation("haproxy-peers", "haproxy") harness.begin() - peer_relation_id = harness.add_relation("haproxy-peers", "haproxy") - peer_relation = harness.model.get_relation("haproxy-peers", peer_relation_id) - result = TLSRelationService.get_tls_information_from_peer_relation( - peer_relation, harness.model.app - ) + result = TLSInformation.from_charm(harness.charm, harness.charm.certificates) assert result is None From 0fa1b222b1d843cf758dcace7c75dbfc54fb683b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:36:39 +0000 Subject: [PATCH 6/6] Add logging and improve docstring in _from_peer_relation Agent-Logs-Url: https://github.com/canonical/haproxy-operator/sessions/96fecf67-9c28-47d1-8590-78ac4718d7c2 Co-authored-by: Thanhphan1147 <42444001+Thanhphan1147@users.noreply.github.com> --- haproxy-operator/src/state/tls.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/haproxy-operator/src/state/tls.py b/haproxy-operator/src/state/tls.py index 0e27bdedd..a28d90f90 100644 --- a/haproxy-operator/src/state/tls.py +++ b/haproxy-operator/src/state/tls.py @@ -106,13 +106,16 @@ def _from_peer_relation(cls, charm: ops.CharmBase) -> typing.Optional["TLSInform charm: The haproxy charm. Returns: - TLSInformation if available, None otherwise. + TLSInformation if available, None if the peer relation does not exist + or the certificate data has not yet been shared by the leader. """ peer_relation = charm.model.get_relation(HAPROXY_PEER_INTEGRATION) if not peer_relation: + logger.info("Peer relation not available, cannot read TLS data.") return None raw = peer_relation.data[charm.app].get(PEER_TLS_KEY) if not raw: + logger.info("No TLS certificate data in peer relation yet.") return None data = json.loads(raw) hostnames = data["hostnames"]