From a1a9badb7921fc6ade1623e91221c136fc2f2181 Mon Sep 17 00:00:00 2001 From: Minh Nguyen Date: Fri, 17 Jul 2026 02:37:59 -0400 Subject: [PATCH 01/26] feat: add get-configuration action Add a get-configuration Juju action so operators can inspect the effective haproxy configuration for debugging without shell access to the unit. - source=disk (default): return the applied /etc/haproxy/haproxy.cfg - source=relations: preview the configuration the next reconcile would generate from the current haproxy-route relation data (read-only; no file writes or service reload) - Log a warning when the effective configuration matches the default (no proxy backends configured) and, in relations mode, when a haproxy-route-policy relation makes the policy backend converge asynchronously (source=disk stays authoritative) - Extract shared template-context and render-to-string helpers so the reconcile and preview paths cannot drift AI-assisted. --- haproxy-operator/charmcraft.yaml | 24 +++ haproxy-operator/src/charm.py | 116 ++++++++++- haproxy-operator/src/haproxy.py | 138 +++++++++++-- .../tests/integration/test_actions.py | 85 ++++++++ haproxy-operator/tests/unit/test_charm.py | 182 ++++++++++++++++++ 5 files changed, 523 insertions(+), 22 deletions(-) diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 60869ac06..591cc9753 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -146,6 +146,30 @@ actions: description: | The name of the backend for which to get the endpoints. If no backend with this name is present, an empty list is returned. + get-configuration: + description: | + Return the haproxy configuration. By default (source=disk) returns the + rendered configuration currently on disk (/etc/haproxy/haproxy.cfg) that + haproxy is running. Set source=relations to instead preview the + haproxy-route configuration that would be generated from the current + relation data, without writing to disk or reloading the service. + Intended for debugging purposes. + params: + source: + type: string + description: | + Where to obtain the configuration from. "disk" (default) returns the + rendered configuration currently on disk. "relations" recomputes and + previews the configuration that would be generated on the next + reconcile from the current haproxy-route relation data, without + applying it. When a haproxy-route-policy relation is present, the + policy backend in the preview reflects the policy charm's current + output and converges asynchronously; use "disk" for the authoritative + applied configuration. + default: disk + enum: + - disk + - relations charm-libs: - lib: traefik_k8s.ingress_per_unit diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 01d24f6be..b79344a9a 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -47,7 +47,7 @@ from ops.charm import ActionEvent from ops.model import Port, SecretNotFoundError -from haproxy import HAPROXY_SERVICE, HAProxyService +from haproxy import HAPROXY_CONFIG, HAPROXY_SERVICE, HAProxyService, file_exists, read_file from http_interface import ( HTTPBackendAvailableEvent, HTTPBackendRemovedEvent, @@ -217,6 +217,7 @@ def __init__(self, *args: typing.Any): self.framework.observe( self.on.get_proxied_endpoints_action, self._on_get_proxied_endpoints_action ) + self.framework.observe(self.on.get_configuration_action, self._on_get_configuration_action) # Hook peer relation events so non-leader units reconcile when the leader # publishes certificate data to the peer relation app databag. self.framework.observe( @@ -284,10 +285,13 @@ def _on_http_backend_removed(self, _: HTTPBackendRemovedEvent) -> None: """Handle data_removed event for reverseproxy integration.""" self._reconcile() - def _reconcile(self) -> None: - """Render the haproxy config and restart the service.""" - self.unit.status = ops.MaintenanceStatus("Configuring haproxy.") - charm_state = CharmState.from_charm( + def _charm_state(self) -> CharmState: + """Build the charm state from the current charm and its providers. + + Returns: + The charm state component. + """ + return CharmState.from_charm( self, self._ingress_provider, self._ingress_per_unit_provider, @@ -296,6 +300,11 @@ def _reconcile(self) -> None: self.reverseproxy_requirer, self.haproxy_route_policy, ) + + def _reconcile(self) -> None: + """Render the haproxy config and restart the service.""" + self.unit.status = ops.MaintenanceStatus("Configuring haproxy.") + charm_state = self._charm_state() proxy_mode = charm_state.mode if proxy_mode == ProxyMode.INVALID: # We don't raise any exception/set status here as it should already be handled @@ -717,6 +726,103 @@ def _on_get_proxied_endpoints_action(self, event: ActionEvent) -> None: event.set_results({"endpoints": json.dumps(proxied_endpoints)}) + def _on_get_configuration_action(self, event: ActionEvent) -> None: + """Triggered when users run the `get-configuration` Juju action. + + By default (`source=disk`) reads the rendered haproxy configuration from + disk and returns it, strictly read-only: it never renders configuration, + reloads the haproxy service, or writes to a relation databag. + + When `source=relations`, it previews the haproxy-route configuration that + the current relation data would generate on the next reconcile, without + writing to disk or reloading the service. When a haproxy-route-policy + relation is present, the policy backend reflects the policy charm's + current output, which converges asynchronously; `source=disk` remains + authoritative for the applied configuration. + + Args: + event: Juju event + """ + source = event.params.get("source", "disk") + if source == "relations": + try: + configuration = self._recompute_haproxy_route_configuration() + except ( + CharmStateValidationBaseError, + HaproxyRouteIntegrationDataValidationError, + ) as exc: + event.fail(f"Failed to recompute configuration from relations: {exc}") + return + if self.haproxy_route_policy.relation is not None: + event.log( + "A haproxy-route-policy relation is present; the policy backend in this " + "preview reflects the policy charm's current output and converges " + "asynchronously. Use source=disk for the authoritative applied configuration." + ) + else: + if not file_exists(HAPROXY_CONFIG): + event.fail( + f"HAProxy configuration file {HAPROXY_CONFIG} does not exist yet. " + "Ensure the charm is configured and integrated before running this action." + ) + return + configuration = read_file(HAPROXY_CONFIG) + + if self._configuration_is_default(configuration): + event.log( + "The HAProxy configuration matches the default configuration. This usually " + "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " + "reverseproxy relations)." + ) + + event.set_results({"configuration": configuration, "source": source}) + + def _recompute_haproxy_route_configuration(self) -> str: + """Recompute the haproxy-route configuration from the current relation data. + + This is the read-only counterpart of `_configure_haproxy_route`: it + gathers the same state from the current relations but performs no side + effects (no port changes, no databag writes, no file writes, no reload). + + Returns: + The configuration that the current haproxy-route relations would generate. + """ + charm_state = self._charm_state() + haproxy_route_requirers_information = HaproxyRouteRequirersInformation.from_provider( + haproxy_route=self.haproxy_route_provider, + haproxy_route_tcp=self.haproxy_route_tcp_provider, + haproxy_route_policy=self.haproxy_route_policy, + external_hostname=typing.cast("str | None", self.config.get("external-hostname")), + peers=self._get_peer_units_address(), + ca_certs_configured=bool(self.recv_ca_certs.get_all_certificates()), + ) + ddos_protection_config = DDosProtection.from_charm(self.ddos_requirer) + spoe_oauth_info_list = SpoeAuthInformation.from_requirer(self.spoe_auth_requirer) + return self.haproxy_service.render_haproxy_route_config( + charm_state, + haproxy_route_requirers_information, + spoe_oauth_info_list, + ddos_protection_config, + ) + + def _configuration_is_default(self, configuration: str) -> bool: + """Return whether the given configuration matches the default configuration. + + Used to warn operators that the effective configuration is just the + default, which usually means no proxy backends are configured. + + Args: + configuration: The configuration to compare against the default. + + Returns: + True if the configuration is identical to the rendered default config. + """ + try: + default_configuration = self.haproxy_service.render_default_config(self._charm_state()) + except CharmStateValidationBaseError: + return False + return configuration == default_configuration + def _publish_haproxy_route_proxied_endpoints( self, haproxy_route_requirers_information: HaproxyRouteRequirersInformation ) -> None: diff --git a/haproxy-operator/src/haproxy.py b/haproxy-operator/src/haproxy.py index 1d92f0ea3..856cc272b 100644 --- a/haproxy-operator/src/haproxy.py +++ b/haproxy-operator/src/haproxy.py @@ -173,8 +173,79 @@ def reconcile_haproxy_route( store_config_to_file(ddos_protection_config.ip_allow_list, IP_ALLOW_LIST_FILE) store_config_to_file(ddos_protection_config.deny_paths, DENY_PATHS_FILE) + template_context = self._build_haproxy_route_template_context( + charm_state, + haproxy_route_requirers_information, + spoe_oauth_info_list, + ddos_protection_config, + ) + self._render_haproxy_config(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) + if spoe_oauth_info_list: + spoe_auth_template_context = { + "spoe_auth_info_list": spoe_oauth_info_list, + } + self._render_config_file( + SPOE_AUTH_CONFIG_TEMPLATE, spoe_auth_template_context, SPOE_AUTH_CONFIG + ) + self._validate_haproxy_config() + self._reload_haproxy_service() + + def render_haproxy_route_config( + self, + charm_state: CharmState, + haproxy_route_requirers_information: HaproxyRouteRequirersInformation, + spoe_oauth_info_list: list[SpoeAuthInformation], + ddos_protection_config: DDosProtection, + ) -> str: + """Render the haproxy-route configuration and return it without applying it. + + Read-only counterpart of `reconcile_haproxy_route`: both build their + context through `_build_haproxy_route_template_context`, but this method + returns the rendered configuration as a string with no side effects (no + file writes, validation, or reload). It is used to preview the + configuration that the current relation data would generate. + + Args: + charm_state: The charm state component. + haproxy_route_requirers_information: HaproxyRouteRequirersInformation state component. + spoe_oauth_info_list: Information about SPOE auth providers. + ddos_protection_config: DDoS protection configuration. + + Returns: + The rendered haproxy-route configuration. + """ + template_context = self._build_haproxy_route_template_context( + charm_state, + haproxy_route_requirers_information, + spoe_oauth_info_list, + ddos_protection_config, + ) + return self._render_to_string(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) + + def _build_haproxy_route_template_context( + self, + charm_state: CharmState, + haproxy_route_requirers_information: HaproxyRouteRequirersInformation, + spoe_oauth_info_list: list[SpoeAuthInformation], + ddos_protection_config: DDosProtection, + ) -> dict: + """Build the template context for the haproxy-route configuration. + + Shared by `reconcile_haproxy_route` (which applies the config) and + `render_haproxy_route_config` (which only previews it), so both always + produce identical output. + + Args: + charm_state: The charm state component. + haproxy_route_requirers_information: HaproxyRouteRequirersInformation state component. + spoe_oauth_info_list: Information about SPOE auth providers. + ddos_protection_config: DDoS protection configuration. + + Returns: + The template context for the haproxy-route template. + """ valid_backends = haproxy_route_requirers_information.valid_backends() - template_context = { + return { "config_global_max_connection": charm_state.global_max_connection, "enable_hsts": charm_state.enable_hsts, "ddos_protection": charm_state.ddos_protection, @@ -199,16 +270,6 @@ def reconcile_haproxy_route( "deny_paths_file": DENY_PATHS_FILE, "policy_provider_backend": haproxy_route_requirers_information.policy_provider_backend, } - self._render_haproxy_config(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) - if spoe_oauth_info_list: - spoe_auth_template_context = { - "spoe_auth_info_list": spoe_oauth_info_list, - } - self._render_config_file( - SPOE_AUTH_CONFIG_TEMPLATE, spoe_auth_template_context, SPOE_AUTH_CONFIG - ) - self._validate_haproxy_config() - self._reload_haproxy_service() def reconcile_default(self, charm_state: CharmState) -> None: """Render the default haproxy config and reload the service. @@ -218,14 +279,45 @@ def reconcile_default(self, charm_state: CharmState) -> None: """ self._render_haproxy_config( HAPROXY_DEFAULT_CONFIG_TEMPLATE, - { - "config_global_max_connection": charm_state.global_max_connection, - "ddos_protection": charm_state.ddos_protection, - }, + self._build_default_template_context(charm_state), ) self._validate_haproxy_config() self._reload_haproxy_service() + def render_default_config(self, charm_state: CharmState) -> str: + """Render the default haproxy configuration and return it without applying it. + + Read-only counterpart of `reconcile_default`: both build their context + through `_build_default_template_context`, but this method returns the + rendered configuration as a string with no side effects (no file writes, + validation, or reload). It is used to detect whether the effective + configuration is just the default. + + Args: + charm_state: The charm state component. + + Returns: + The rendered default configuration. + """ + return self._render_to_string( + HAPROXY_DEFAULT_CONFIG_TEMPLATE, + self._build_default_template_context(charm_state), + ) + + def _build_default_template_context(self, charm_state: CharmState) -> dict: + """Build the template context for the default haproxy configuration. + + Args: + charm_state: The charm state component. + + Returns: + The template context for the default template. + """ + return { + "config_global_max_connection": charm_state.global_max_connection, + "ddos_protection": charm_state.ddos_protection, + } + def _render_haproxy_config(self, template_file_path: str, context: dict) -> None: """Render the haproxy configuration file. @@ -243,6 +335,19 @@ def _render_config_file(self, template_file_path: str, context: dict, path: Path context: Context needed to render the template. path: Path of the file to render. """ + rendered = self._render_to_string(template_file_path, context) + render_file(path, rendered, 0o644) + + def _render_to_string(self, template_file_path: str, context: dict) -> str: + """Render a template to a string without writing it to disk. + + Args: + template_file_path: Path of the template to load. + context: Context needed to render the template. + + Returns: + The rendered template content. + """ env = Environment( loader=FileSystemLoader("templates"), autoescape=select_autoescape(), @@ -251,8 +356,7 @@ def _render_config_file(self, template_file_path: str, context: dict, path: Path lstrip_blocks=True, ) template = env.get_template(template_file_path) - rendered = template.render(context) - render_file(path, rendered, 0o644) + return template.render(context) def _reload_haproxy_service(self) -> None: """Reload the haproxy service. diff --git a/haproxy-operator/tests/integration/test_actions.py b/haproxy-operator/tests/integration/test_actions.py index 27f385aa6..f4f96ed63 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -5,11 +5,36 @@ """Integration tests for haproxy charm actions.""" import json +import time import jubilant import pytest +def _integrate_haproxy_route(juju: jubilant.Juju, provider_endpoint: str, requirer: str) -> None: + """Integrate haproxy-route, retrying while a prior relation is still being removed. + + Tests in this module add and remove the same haproxy-route relation. Because + relation removal is asynchronous, a subsequent integrate can race with it and + fail with "already exists"; retry until the previous relation has cleared. + + Args: + juju: Jubilant juju instance. + provider_endpoint: The haproxy-route provider endpoint (e.g. "haproxy:haproxy-route"). + requirer: The requirer application name. + """ + deadline = time.monotonic() + 120 + while True: + try: + juju.integrate(provider_endpoint, requirer) + return + except jubilant.CLIError as exc: + if "already exists" in str(exc) and time.monotonic() < deadline: + time.sleep(2) + continue + raise + + @pytest.mark.abort_on_fail def test_get_proxied_endpoints_action( configured_application_with_tls: str, @@ -82,3 +107,63 @@ def test_get_proxied_endpoints_action( juju.remove_relation( f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer ) + + +@pytest.mark.abort_on_fail +def test_get_configuration_action( + configured_application_with_tls: str, + any_charm_haproxy_route_requirer: str, + juju: jubilant.Juju, +): + """arrange: Deploy the charm integrated with any_charm haproxy-route. + act: Trigger the action 'get-configuration' in disk and relations modes. + assert: The returned configuration matches the on-disk haproxy.cfg, and the + relations-mode preview matches the applied configuration. + """ + _integrate_haproxy_route( + juju, f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer + ) + + service_name = "any_charm" + juju.run( + f"{any_charm_haproxy_route_requirer}/0", + "rpc", + { + "method": "update_relation", + "args": json.dumps( + [ + { + "service": service_name, + "ports": [80], + "hostname": "ok.haproxy.internal", + "paths": ["/v1"], + } + ] + ), + }, + ) + juju.wait( + lambda status: jubilant.all_active( + status, configured_application_with_tls, any_charm_haproxy_route_requirer + ) + ) + + on_disk = juju.ssh(f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg") + + # Full configuration must match what is on disk. + task = juju.run(f"{configured_application_with_tls}/0", "get-configuration") + assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results + + # Recomputing from relations (source=relations) must match the applied config + # when the deployment is settled, without touching disk. + task = juju.run( + f"{configured_application_with_tls}/0", + "get-configuration", + {"source": "relations"}, + ) + assert task.results["source"] == "relations", task.results + assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results + + juju.remove_relation( + f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer + ) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 4cada97ed..db17e3c33 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -14,6 +14,7 @@ import pytest import scenario +import charm as charm_module import tls_relation from charm import HAProxyCharm from tests.unit.conftest import TEST_EXTERNAL_HOSTNAME_CONFIG @@ -470,3 +471,184 @@ 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 TestGetConfigurationAction: + """Test "get-configuration" Action.""" + + def test_returns_full_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the config file on disk with known content. + act: trigger the get-configuration action without a filter. + assert: the full file content is returned unchanged. + """ + content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: content) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + assert context.action_results == {"configuration": content, "source": "disk"} + + def test_source_relations_previews_config(self) -> None: + """ + arrange: create state with a haproxy-route relation for a specific backend. + act: trigger the get-configuration action with source=relations. + assert: the recomputed configuration is returned (containing the backend) + without reading from disk. + """ + service_name = "haproxy-tutorial-ingress-configurator" + context = ops.testing.Context(HAProxyCharm) + haproxy_route_relation = ops.testing.Relation( + "haproxy-route", + remote_app_data={ + "hostname": f'"{TEST_EXTERNAL_HOSTNAME_CONFIG}"', + "paths": '["/v1"]', + "ports": "[443]", + "protocol": '"http"', + "service": f'"{service_name}"', + }, + remote_units_data={0: {"address": '"10.75.1.129"'}}, + ) + state = ops.testing.State( + relations=[haproxy_route_relation], + leader=True, + model=ops.testing.Model(name="haproxy-tutorial"), + app_status=ops.testing.ActiveStatus(), + unit_status=ops.testing.ActiveStatus(), + ) + + context.run(context.on.action("get-configuration", params={"source": "relations"}), state) + + out = context.action_results + assert out is not None + assert out["source"] == "relations" + assert service_name in out["configuration"] + + def test_warns_when_policy_relation_present(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: create state with a haproxy-route-policy relation. + act: trigger the get-configuration action with source=relations. + assert: a warning about the policy backend converging asynchronously is logged. + """ + monkeypatch.setattr( + charm_module.HAProxyCharm, + "_recompute_haproxy_route_configuration", + lambda self: "global\n", + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State( + relations=[ops.testing.Relation("haproxy-route-policy")], + leader=True, + ) + + context.run(context.on.action("get-configuration", params={"source": "relations"}), state) + + assert any("haproxy-route-policy" in log.lower() for log in context.action_logs) + + @pytest.mark.parametrize("num_relations", [20, 100]) + def test_source_relations_scales(self, num_relations: int) -> None: + """ + arrange: create state with many haproxy-route relations (one backend each). + act: trigger the get-configuration action with source=relations. + assert: every backend appears in the recomputed configuration (nothing is + truncated at the charm level). + """ + context = ops.testing.Context(HAProxyCharm) + relations = [] + service_names = [] + for i in range(num_relations): + service = f"backend-service-{i}" + service_names.append(service) + relations.append( + ops.testing.Relation( + "haproxy-route", + remote_app_data={ + "hostname": f'"svc{i}.{TEST_EXTERNAL_HOSTNAME_CONFIG}"', + "paths": '["/v1"]', + "ports": "[443]", + "protocol": '"http"', + "service": f'"{service}"', + }, + remote_units_data={0: {"address": '"10.75.1.129"'}}, + ) + ) + state = ops.testing.State( + relations=relations, + leader=True, + model=ops.testing.Model(name="haproxy-tutorial"), + app_status=ops.testing.ActiveStatus(), + unit_status=ops.testing.ActiveStatus(), + ) + + context.run(context.on.action("get-configuration", params={"source": "relations"}), state) + + out = context.action_results + assert out is not None + full_config = out["configuration"] + for service in service_names: + assert service in full_config, ( + f"{service} missing from a {num_relations}-relation config" + ) + + def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the config file as absent from disk. + act: trigger the get-configuration action. + assert: the action fails with a clear message rather than returning empty. + """ + monkeypatch.setattr(charm_module, "file_exists", lambda _: False) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + with pytest.raises(ops.testing.ActionFailed) as exc_info: + context.run(context.on.action("get-configuration"), state) + + assert "does not exist" in exc_info.value.message + + def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the on-disk config to equal the rendered default config. + act: trigger the get-configuration action. + assert: a warning is logged and the configuration is still returned. + """ + default_config = "global\n maxconn 4096\n" + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: default_config) + monkeypatch.setattr( + charm_module.HAProxyService, "render_default_config", lambda self, _: default_config + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + assert context.action_results == {"configuration": default_config, "source": "disk"} + assert any("default configuration" in log.lower() for log in context.action_logs) + + def test_no_warning_when_config_differs_from_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + arrange: mock the on-disk config to differ from the rendered default config. + act: trigger the get-configuration action. + assert: no default-configuration warning is logged. + """ + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr( + charm_module, "read_file", lambda _: "frontend haproxy\n bind :80\n" + ) + monkeypatch.setattr( + charm_module.HAProxyService, + "render_default_config", + lambda self, _: "global\n maxconn 4096\n", + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + assert not any("default configuration" in log.lower() for log in context.action_logs) From 96f34be94b16449bdf14da9cebf62ddb6d95b553 Mon Sep 17 00:00:00 2001 From: Minh Nguyen Date: Fri, 17 Jul 2026 03:29:39 -0400 Subject: [PATCH 02/26] edited docstrings so that they are more accurate --- haproxy-operator/src/charm.py | 30 +++++++++++------------------- haproxy-operator/src/haproxy.py | 23 ++++++++--------------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index b79344a9a..ff7bb5df9 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -729,16 +729,12 @@ def _on_get_proxied_endpoints_action(self, event: ActionEvent) -> None: def _on_get_configuration_action(self, event: ActionEvent) -> None: """Triggered when users run the `get-configuration` Juju action. - By default (`source=disk`) reads the rendered haproxy configuration from - disk and returns it, strictly read-only: it never renders configuration, - reloads the haproxy service, or writes to a relation databag. - - When `source=relations`, it previews the haproxy-route configuration that - the current relation data would generate on the next reconcile, without - writing to disk or reloading the service. When a haproxy-route-policy - relation is present, the policy backend reflects the policy charm's - current output, which converges asynchronously; `source=disk` remains - authoritative for the applied configuration. + `source=disk` (default) returns the on-disk configuration. + `source=relations` renders the haproxy-route configuration from the + current relation data. Neither writes to disk nor reloads the service. + When a haproxy-route-policy relation is present, the rendered policy + backend reflects the policy charm's current, asynchronously-converging + output. Args: event: Juju event @@ -778,14 +774,13 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: event.set_results({"configuration": configuration, "source": source}) def _recompute_haproxy_route_configuration(self) -> str: - """Recompute the haproxy-route configuration from the current relation data. + """Render the haproxy-route configuration from the current relation data. - This is the read-only counterpart of `_configure_haproxy_route`: it - gathers the same state from the current relations but performs no side - effects (no port changes, no databag writes, no file writes, no reload). + Unlike `_configure_haproxy_route`, performs no side effects (no port + changes, databag writes, file writes, or reload). Returns: - The configuration that the current haproxy-route relations would generate. + The rendered haproxy-route configuration. """ charm_state = self._charm_state() haproxy_route_requirers_information = HaproxyRouteRequirersInformation.from_provider( @@ -808,14 +803,11 @@ def _recompute_haproxy_route_configuration(self) -> str: def _configuration_is_default(self, configuration: str) -> bool: """Return whether the given configuration matches the default configuration. - Used to warn operators that the effective configuration is just the - default, which usually means no proxy backends are configured. - Args: configuration: The configuration to compare against the default. Returns: - True if the configuration is identical to the rendered default config. + True if it is identical to the rendered default configuration. """ try: default_configuration = self.haproxy_service.render_default_config(self._charm_state()) diff --git a/haproxy-operator/src/haproxy.py b/haproxy-operator/src/haproxy.py index 856cc272b..8c8c6373b 100644 --- a/haproxy-operator/src/haproxy.py +++ b/haproxy-operator/src/haproxy.py @@ -197,13 +197,10 @@ def render_haproxy_route_config( spoe_oauth_info_list: list[SpoeAuthInformation], ddos_protection_config: DDosProtection, ) -> str: - """Render the haproxy-route configuration and return it without applying it. + """Render the haproxy-route configuration and return it as a string. - Read-only counterpart of `reconcile_haproxy_route`: both build their - context through `_build_haproxy_route_template_context`, but this method - returns the rendered configuration as a string with no side effects (no - file writes, validation, or reload). It is used to preview the - configuration that the current relation data would generate. + Unlike `reconcile_haproxy_route`, performs no side effects (no file + writes, validation, or reload). Args: charm_state: The charm state component. @@ -231,9 +228,8 @@ def _build_haproxy_route_template_context( ) -> dict: """Build the template context for the haproxy-route configuration. - Shared by `reconcile_haproxy_route` (which applies the config) and - `render_haproxy_route_config` (which only previews it), so both always - produce identical output. + Shared by `reconcile_haproxy_route` and `render_haproxy_route_config` + so they cannot drift. Args: charm_state: The charm state component. @@ -285,13 +281,10 @@ def reconcile_default(self, charm_state: CharmState) -> None: self._reload_haproxy_service() def render_default_config(self, charm_state: CharmState) -> str: - """Render the default haproxy configuration and return it without applying it. + """Render the default haproxy configuration and return it as a string. - Read-only counterpart of `reconcile_default`: both build their context - through `_build_default_template_context`, but this method returns the - rendered configuration as a string with no side effects (no file writes, - validation, or reload). It is used to detect whether the effective - configuration is just the default. + Unlike `reconcile_default`, performs no side effects. Used to detect + whether the effective configuration is just the default. Args: charm_state: The charm state component. From 192f14ffd22298515241d5d71589d185e8d2d39f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:42:25 +0000 Subject: [PATCH 03/26] Add release notes artifact for PR #609 (get-configuration action) --- docs/release-notes/artifacts/pr0609.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 docs/release-notes/artifacts/pr0609.yaml diff --git a/docs/release-notes/artifacts/pr0609.yaml b/docs/release-notes/artifacts/pr0609.yaml new file mode 100644 index 000000000..a99cf44f8 --- /dev/null +++ b/docs/release-notes/artifacts/pr0609.yaml @@ -0,0 +1,20 @@ +version_schema: 2 + +changes: + - title: Add get-configuration Juju action for inspecting effective haproxy config + author: minulo + type: minor + description: > + Added a `get-configuration` Juju action that allows operators to inspect + the effective haproxy configuration without shell access to the unit. + Supports two modes: `source=disk` (default) returns the currently applied + `/etc/haproxy/haproxy.cfg`, and `source=relations` previews the + configuration that would be generated from the current relation data + without writing files or reloading the service. Logs a warning when the + effective configuration matches the default (no proxy backends configured). + urls: + pr: + - https://github.com/canonical/haproxy-operator/pull/609 + related_issue: + visibility: public + highlight: false From d1e620fb478ec184e6b18c5bc68e878404fb50e6 Mon Sep 17 00:00:00 2001 From: Minh Nguyen Date: Fri, 17 Jul 2026 12:27:06 -0400 Subject: [PATCH 04/26] edited release-notes and changelog file --- docs/changelog.md | 4 ++++ docs/release-notes/artifacts/pr0609.yaml | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index c8b8261e6..cf3bf1c0b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -8,6 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Each revision is versioned by the date of the revision. +## 2026-07-17 + +- Added a `get-configuration` action to inspect the effective HAProxy configuration. + ## 2026-06-26 - docs: Add docs on the Terraform module. diff --git a/docs/release-notes/artifacts/pr0609.yaml b/docs/release-notes/artifacts/pr0609.yaml index a99cf44f8..781c96ad4 100644 --- a/docs/release-notes/artifacts/pr0609.yaml +++ b/docs/release-notes/artifacts/pr0609.yaml @@ -8,10 +8,10 @@ changes: Added a `get-configuration` Juju action that allows operators to inspect the effective haproxy configuration without shell access to the unit. Supports two modes: `source=disk` (default) returns the currently applied - `/etc/haproxy/haproxy.cfg`, and `source=relations` previews the - configuration that would be generated from the current relation data - without writing files or reloading the service. Logs a warning when the - effective configuration matches the default (no proxy backends configured). + `/etc/haproxy/haproxy.cfg`, and `source=relations` renders the + configuration from the current relation data without writing files or + reloading the service. Logs a warning when the effective configuration + matches the default (no proxy backends configured). urls: pr: - https://github.com/canonical/haproxy-operator/pull/609 From 29261ea2fe9e33bf7dd3ee5422fa963d4481b243 Mon Sep 17 00:00:00 2001 From: Minh Nguyen Date: Tue, 21 Jul 2026 01:06:07 -0400 Subject: [PATCH 05/26] Updating the get-configuration juju action description for better accuracy --- haproxy-operator/charmcraft.yaml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 591cc9753..4a7aba01a 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -150,22 +150,14 @@ actions: description: | Return the haproxy configuration. By default (source=disk) returns the rendered configuration currently on disk (/etc/haproxy/haproxy.cfg) that - haproxy is running. Set source=relations to instead preview the - haproxy-route configuration that would be generated from the current + haproxy is running. Set source=relations to instead read the + haproxy-route configuration that is enroute to be generated from the current relation data, without writing to disk or reloading the service. Intended for debugging purposes. params: source: type: string description: | - Where to obtain the configuration from. "disk" (default) returns the - rendered configuration currently on disk. "relations" recomputes and - previews the configuration that would be generated on the next - reconcile from the current haproxy-route relation data, without - applying it. When a haproxy-route-policy relation is present, the - policy backend in the preview reflects the policy charm's current - output and converges asynchronously; use "disk" for the authoritative - applied configuration. default: disk enum: - disk From 3ca96c7a843bff7876389556a7a1387c710e1b48 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 22 Jul 2026 02:36:47 -0400 Subject: [PATCH 06/26] =?UTF-8?q?Add=20a=20=20boolean=20action=20parameter?= =?UTF-8?q?=20(default=20false)=20to=20the=20get-configuration=20action.?= =?UTF-8?q?=20When=20false,=20the=20constant=20scaffold=20that=20is=20iden?= =?UTF-8?q?tical=20across=20all=20deployments=20=E2=80=94=20the=20global,?= =?UTF-8?q?=20defaults,=20prometheus=20frontend=20and=20fallback=20backend?= =?UTF-8?q?=20sections=20=E2=80=94=20is=20hidden=20and=20the=20user=20is?= =?UTF-8?q?=20notified=20via=20event.log,=20leaving=20only=20the=20operato?= =?UTF-8?q?r-specific=20config.=20Pass=20full=3Dtrue=20to=20return=20the?= =?UTF-8?q?=20complete=20configuration.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hidden scaffold is derived from render_default_config (stripping the head/tail the config shares with the default render) rather than matching hard-coded section names, so it stays correct if the base template changes. --- haproxy-operator/charmcraft.yaml | 8 +++ haproxy-operator/src/charm.py | 74 +++++++++++++++++++++++ haproxy-operator/tests/unit/test_charm.py | 41 ++++++++++++- lxd.log | 8 +++ 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 lxd.log diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 4a7aba01a..671d4ea60 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -162,6 +162,14 @@ actions: enum: - disk - relations + full: + type: boolean + default: false + description: | + When false (the default), the constant sections that are identical + across all deployments (global, defaults, the prometheus frontend and + the fallback backend) are hidden for readability. Set to true to + return the complete configuration. charm-libs: - lib: traefik_k8s.ingress_per_unit diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index ff7bb5df9..be6257c19 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -108,6 +108,46 @@ def _validate_port(port: int) -> bool: return 0 <= port <= 65535 +def _strip_shared_boundaries(configuration: str, reference: str) -> str: + """Remove the leading and trailing lines ``configuration`` shares with ``reference``. + + Keeps everything between the first and last differing line, so + operator-specific content is never dropped. Returns the input verbatim when + nothing is shared at the boundaries. + + Args: + configuration: The configuration to trim. + reference: The reference configuration to compare against. + + Returns: + The trimmed configuration. + """ + config_lines = configuration.splitlines() + reference_lines = reference.splitlines() + + start = 0 + while ( + start < len(config_lines) + and start < len(reference_lines) + and config_lines[start] == reference_lines[start] + ): + start += 1 + + config_end = len(config_lines) + reference_end = len(reference_lines) + while ( + config_end > start + and reference_end > start + and config_lines[config_end - 1] == reference_lines[reference_end - 1] + ): + config_end -= 1 + reference_end -= 1 + + if start == 0 and config_end == len(config_lines): + return configuration + return "\n".join(config_lines[start:config_end]) + + # pylint: disable=too-many-instance-attributes class HAProxyCharm(ops.CharmBase): """Charm haproxy.""" @@ -771,6 +811,9 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: "reverseproxy relations)." ) + if not typing.cast(bool, event.params.get("full", False)): + configuration = self._hide_constant_configuration(configuration, event) + event.set_results({"configuration": configuration, "source": source}) def _recompute_haproxy_route_configuration(self) -> str: @@ -815,6 +858,37 @@ def _configuration_is_default(self, configuration: str) -> bool: return False return configuration == default_configuration + def _hide_constant_configuration(self, configuration: str, event: ActionEvent) -> str: + """Hide the constant scaffold the config shares with the default render. + + The shared head/tail (global, defaults, prometheus frontend, fallback + backend) is derived from ``render_default_config`` rather than hard-coded + section names, so it stays correct if the template changes. Notifies the + user via ``event.log`` when anything is hidden. + + Args: + configuration: The configuration to trim. + event: Juju event, used to notify the user when content is hidden. + + Returns: + The trimmed configuration, or the input unchanged if the default + configuration cannot be rendered. + """ + try: + default_configuration = self.haproxy_service.render_default_config(self._charm_state()) + except CharmStateValidationBaseError: + return configuration + + trimmed = _strip_shared_boundaries(configuration, default_configuration) + if trimmed != configuration: + event.log( + "Constant/default config sections (global, defaults, the prometheus frontend " + "and the fallback backend) that are identical across deployments have been " + "hidden for readability. Re-run this action with full=true to return the " + "complete configuration." + ) + return trimmed + def _publish_haproxy_route_proxied_endpoints( self, haproxy_route_requirers_information: HaproxyRouteRequirersInformation ) -> None: diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index db17e3c33..3bd41515f 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -480,7 +480,7 @@ class TestGetConfigurationAction: def test_returns_full_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: """ arrange: mock the config file on disk with known content. - act: trigger the get-configuration action without a filter. + act: trigger the get-configuration action with full=true. assert: the full file content is returned unchanged. """ content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" @@ -489,10 +489,45 @@ def test_returns_full_configuration(self, monkeypatch: pytest.MonkeyPatch) -> No context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration"), state) + context.run(context.on.action("get-configuration", params={"full": True}), state) assert context.action_results == {"configuration": content, "source": "disk"} + def test_hides_constant_configuration_by_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """ + arrange: mock the on-disk config as the default scaffold wrapping an + operator-specific frontend section. + act: trigger the get-configuration action without full=true. + assert: the constant head/tail shared with the default render is hidden, + the operator-specific section is kept, and the user is notified. + """ + default_config = "global\n maxconn 4096\n\nbackend default\n return 200\n" + on_disk = ( + "global\n maxconn 4096\n\n" + "frontend haproxy\n bind :443\n" + "backend default\n return 200\n" + ) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: on_disk) + monkeypatch.setattr( + charm_module.HAProxyService, "render_default_config", lambda self, _: default_config + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + out = context.action_results + assert out is not None + configuration = out["configuration"] + assert "global" not in configuration + assert "maxconn" not in configuration + assert "frontend haproxy" in configuration + assert "bind :443" in configuration + assert any("hidden for readability" in log.lower() for log in context.action_logs) + def test_source_relations_previews_config(self) -> None: """ arrange: create state with a haproxy-route relation for a specific backend. @@ -624,7 +659,7 @@ def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration"), state) + context.run(context.on.action("get-configuration", params={"full": True}), state) assert context.action_results == {"configuration": default_config, "source": "disk"} assert any("default configuration" in log.lower() for log in context.action_logs) diff --git a/lxd.log b/lxd.log new file mode 100644 index 000000000..5bfbc1c06 --- /dev/null +++ b/lxd.log @@ -0,0 +1,8 @@ +controller-0: 03:54:06 INFO juju.worker.apicaller [4d0cfe] "machine-0" successfully connected to "wss://localhost:17070" +controller-0: 03:54:06 INFO juju.worker.logforwarder config change - log forwarding not enabled +controller-0: 03:54:06 INFO juju.worker.machineundertaker setting up machine undertaker +controller-0: 03:54:06 INFO juju.worker.logger logger worker started +controller-0: 03:54:06 INFO juju.worker.pruner.action pruner config: max age: 336h0m0s, max collection size 5120M for jubilant-adf52682 (4d0cfe1e-2637-4244-8b76-b0739d80c1a6) +controller-0: 03:54:06 INFO juju.worker.pruner.statushistory pruner config: max age: 336h0m0s, max collection size 5120M for jubilant-adf52682 (4d0cfe1e-2637-4244-8b76-b0739d80c1a6) +controller-0: 03:54:06 INFO juju.worker.provisioner entering provisioner task loop; using provisioner pool with 16 workers +controller-0: 03:54:06 INFO juju.worker.provisioner provisioning in zones: [my-juju-vm] From 20be5278b3f68066a0f2fdc8c13a882fc6e5b6e3 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 22 Jul 2026 10:09:52 -0400 Subject: [PATCH 07/26] Update tests and licensing failures --- .gitignore | 3 +++ .../tests/integration/test_actions.py | 17 ++++++++++++----- lxd.log | 8 -------- 3 files changed, 15 insertions(+), 13 deletions(-) delete mode 100644 lxd.log diff --git a/.gitignore b/.gitignore index 50f72334c..0881633c0 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ haproxy-route-policy/.python-version artifacts.build.yaml .worktrees + +# local juju/lxd logs +*.log diff --git a/haproxy-operator/tests/integration/test_actions.py b/haproxy-operator/tests/integration/test_actions.py index f4f96ed63..5339cf40d 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -150,20 +150,27 @@ def test_get_configuration_action( on_disk = juju.ssh(f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg") - # Full configuration must match what is on disk. - task = juju.run(f"{configured_application_with_tls}/0", "get-configuration") + # full=true must return the complete configuration matching what is on disk. + task = juju.run(f"{configured_application_with_tls}/0", "get-configuration", {"full": True}) assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results - # Recomputing from relations (source=relations) must match the applied config - # when the deployment is settled, without touching disk. + # Recomputing from relations (source=relations) with full=true must match the + # applied config when the deployment is settled, without touching disk. task = juju.run( f"{configured_application_with_tls}/0", "get-configuration", - {"source": "relations"}, + {"source": "relations", "full": True}, ) assert task.results["source"] == "relations", task.results assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results + # Default (full=false) hides the constant scaffold shared with the default + # render (e.g. the prometheus frontend) but keeps the operator-specific backend. + task = juju.run(f"{configured_application_with_tls}/0", "get-configuration") + default_config = task.results["configuration"] + assert "frontend prometheus" not in default_config, task.results + assert service_name in default_config, task.results + juju.remove_relation( f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer ) diff --git a/lxd.log b/lxd.log deleted file mode 100644 index 5bfbc1c06..000000000 --- a/lxd.log +++ /dev/null @@ -1,8 +0,0 @@ -controller-0: 03:54:06 INFO juju.worker.apicaller [4d0cfe] "machine-0" successfully connected to "wss://localhost:17070" -controller-0: 03:54:06 INFO juju.worker.logforwarder config change - log forwarding not enabled -controller-0: 03:54:06 INFO juju.worker.machineundertaker setting up machine undertaker -controller-0: 03:54:06 INFO juju.worker.logger logger worker started -controller-0: 03:54:06 INFO juju.worker.pruner.action pruner config: max age: 336h0m0s, max collection size 5120M for jubilant-adf52682 (4d0cfe1e-2637-4244-8b76-b0739d80c1a6) -controller-0: 03:54:06 INFO juju.worker.pruner.statushistory pruner config: max age: 336h0m0s, max collection size 5120M for jubilant-adf52682 (4d0cfe1e-2637-4244-8b76-b0739d80c1a6) -controller-0: 03:54:06 INFO juju.worker.provisioner entering provisioner task loop; using provisioner pool with 16 workers -controller-0: 03:54:06 INFO juju.worker.provisioner provisioning in zones: [my-juju-vm] From 682b4fe0b6585cf9be0b207b12f909d859da87fd Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 22 Jul 2026 14:41:51 -0400 Subject: [PATCH 08/26] ci: trigger integration tests From 4b045dd5af8b279c30c2e063f3c1f7a7d8f3b9c5 Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 23 Jul 2026 01:30:01 -0400 Subject: [PATCH 09/26] feat(haproxy): add backend filter to get-configuration action --- .gitignore | 4 + haproxy-operator/charmcraft.yaml | 8 ++ haproxy-operator/src/charm.py | 121 ++++++++++++++++-- .../tests/integration/test_actions.py | 20 +++ haproxy-operator/tests/unit/test_charm.py | 100 +++++++++++++++ 5 files changed, 245 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 0881633c0..e34ff6d59 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,7 @@ artifacts.build.yaml # local juju/lxd logs *.log + +# local scratch documentation +PROGRESS.md +CHARM_EXPLAINER.md diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 671d4ea60..3982070d9 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -170,6 +170,14 @@ actions: across all deployments (global, defaults, the prometheus frontend and the fallback backend) are hidden for readability. Set to true to return the complete configuration. + backend: + type: string + description: | + If set, return only the `backend ` section for this backend + The shared frontend and the constant scaffold are not included; + use full=true to see those. Takes precedence over 'full'. If no + such backend exists, an empty configuration is returned and the + backends that are present are logged. charm-libs: - lib: traefik_k8s.ingress_per_unit diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index be6257c19..32490baa6 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -148,6 +148,81 @@ def _strip_shared_boundaries(configuration: str, reference: str) -> str: return "\n".join(config_lines[start:config_end]) +def _is_section_header(line: str) -> bool: + """Return whether a line starts a new haproxy config section. + + Args: + line: A single configuration line. + + Returns: + True if the line begins a section. + """ + return bool(line) and not line[0].isspace() and not line.lstrip().startswith("#") + + +def _split_config_sections(configuration: str) -> list[list[str]]: + """Split an haproxy configuration into its sections, preserving lines verbatim. + + Each section is a header line plus every line beneath it, up to (but not + including) the next header. + + Args: + configuration: The haproxy configuration text. + + Returns: + A list of sections, each the list of lines it contains. + """ + sections: list[list[str]] = [] + current: list[str] = [] + for line in configuration.splitlines(): + # A new header means the section we were accumulating is finished. + if _is_section_header(line) and current: + sections.append(current) + current = [] + current.append(line) + if current: + sections.append(current) + return sections + + +def _filter_config_by_backend(configuration: str, backend_name: str) -> str: + """Return the ``backend `` section(s) for ``backend_name``. + + A section-aware alternative to grepping the output: a section is matched only + by its header (``backend ``). + + Args: + configuration: The haproxy configuration text. + backend_name: The backend name to filter for. + + Returns: + The matching ``backend`` section(s), or "" if none match. + """ + matched = [] + for section in _split_config_sections(configuration): + tokens = section[0].split() + if len(tokens) >= 2 and tokens[0] == "backend" and tokens[1] == backend_name: + matched.append(section) + return "\n".join("\n".join(section) for section in matched) + + +def _config_backend_names(configuration: str) -> list[str]: + """Return the names of every ``backend`` section in the configuration. + + Args: + configuration: The haproxy configuration text. + + Returns: + The backend names, in the order they appear. + """ + names: list[str] = [] + for section in _split_config_sections(configuration): + tokens = section[0].split() + if len(tokens) >= 2 and tokens[0] == "backend": + names.append(tokens[1]) + return names + + # pylint: disable=too-many-instance-attributes class HAProxyCharm(ops.CharmBase): """Charm haproxy.""" @@ -804,15 +879,19 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: return configuration = read_file(HAPROXY_CONFIG) - if self._configuration_is_default(configuration): - event.log( - "The HAProxy configuration matches the default configuration. This usually " - "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " - "reverseproxy relations)." - ) + backend = typing.cast(str, event.params.get("backend", "")).strip() + if backend: + configuration = self._filter_configuration_for_backend(configuration, backend, event) + else: + if self._configuration_is_default(configuration): + event.log( + "The HAProxy configuration matches the default configuration. This usually " + "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " + "reverseproxy relations)." + ) - if not typing.cast(bool, event.params.get("full", False)): - configuration = self._hide_constant_configuration(configuration, event) + if not typing.cast(bool, event.params.get("full", False)): + configuration = self._hide_constant_configuration(configuration, event) event.set_results({"configuration": configuration, "source": source}) @@ -889,6 +968,32 @@ def _hide_constant_configuration(self, configuration: str, event: ActionEvent) - ) return trimmed + def _filter_configuration_for_backend( + self, configuration: str, backend_name: str, event: ActionEvent + ) -> str: + """Return only the ``backend `` section for ``backend_name``. + + Args: + configuration: The configuration to filter. + backend_name: The backend name to filter for. + event: Juju event, used to notify the caller when nothing matches. + + Returns: + The matching sections, or "" if the backend is not present. + """ + filtered = _filter_config_by_backend(configuration, backend_name) + if not filtered: + available = _config_backend_names(configuration) + event.log( + f"No configuration section involves a backend named '{backend_name}'. " + + ( + f"Backends present: {', '.join(available)}." + if available + else "No backends are configured." + ) + ) + return filtered + def _publish_haproxy_route_proxied_endpoints( self, haproxy_route_requirers_information: HaproxyRouteRequirersInformation ) -> None: diff --git a/haproxy-operator/tests/integration/test_actions.py b/haproxy-operator/tests/integration/test_actions.py index 5339cf40d..e9ef89018 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -171,6 +171,26 @@ def test_get_configuration_action( assert "frontend prometheus" not in default_config, task.results assert service_name in default_config, task.results + # Per-backend filter returns only that backend's own section, section-aware + # (not grep). Derive a real, non-default backend name from the full config. + full_config = juju.run( + f"{configured_application_with_tls}/0", "get-configuration", {"full": True} + ).results["configuration"] + backend_names = [ + line.split()[1] + for line in full_config.splitlines() + if line.startswith("backend ") and line.split()[1] != "default" + ] + assert backend_names, f"expected at least one non-default backend:\n{full_config}" + target = backend_names[0] + filtered = juju.run( + f"{configured_application_with_tls}/0", "get-configuration", {"backend": target} + ).results["configuration"] + assert f"backend {target}" in filtered, filtered + # only the backend section is returned: the frontend and scaffold are excluded + assert "frontend haproxy" not in filtered, filtered + assert "frontend prometheus" not in filtered, filtered + juju.remove_relation( f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer ) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 3bd41515f..3068df56e 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -528,6 +528,106 @@ def test_hides_constant_configuration_by_default( assert "bind :443" in configuration assert any("hidden for readability" in log.lower() for log in context.action_logs) + def test_backend_filter_returns_whole_section(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock an on-disk config with a frontend and two backends, where the + target backend's body has a line that does NOT contain the backend name. + act: trigger the get-configuration action with backend=foo. + assert: only the whole `backend foo` section is returned (including the body + line grep would miss); the shared frontend, the unrelated backend and + the global scaffold are all excluded. + """ + config = ( + "global\n maxconn 4096\n\n" + "frontend haproxy\n" + " use_backend foo if acl_host_foo\n" + " use_backend bar if acl_host_bar\n\n" + "backend foo\n" + " server srv1 10.0.0.1:80\n" + " timeout server 60s\n\n" + "backend bar\n" + " server srv2 10.0.0.2:80\n" + ) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: config) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration", params={"backend": "foo"}), state) + + results = context.action_results + assert results is not None + out = results["configuration"] + assert "backend foo" in out + # whole-section body preserved — grep for "foo" would have dropped this line + assert "server srv1 10.0.0.1:80" in out + assert "timeout server 60s" in out + # the shared frontend is NOT included (use full for that) + assert "frontend haproxy" not in out + # the unrelated backend's section is excluded (its unique server line is gone) + assert "server srv2 10.0.0.2:80" not in out + # the constant scaffold is excluded + assert "maxconn" not in out + + def test_backend_filter_unknown_logs_available(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock an on-disk config with a single backend. + act: trigger the get-configuration action with a backend name that is absent. + assert: an empty configuration is returned and the available backends are logged. + """ + config = "global\n maxconn 4096\n\nbackend foo\n server srv1 10.0.0.1:80\n" + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: config) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration", params={"backend": "nope"}), state) + + results = context.action_results + assert results is not None + assert results["configuration"] == "" + assert any("nope" in log and "foo" in log for log in context.action_logs), ( + context.action_logs + ) + + def test_backend_filter_matches_last_section(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock an on-disk config where the target backend is the LAST section, + with no section after it — the case the section splitter's post-loop flush + must handle (haproxy-route backends are appended at the end of the file). + act: trigger the get-configuration action with backend=foo. + assert: the whole trailing `backend foo` section is returned, body and all, + and nothing preceding it leaks in. + """ + config = ( + "global\n maxconn 4096\n\n" + "frontend haproxy\n" + " use_backend bar if acl_host_bar\n" + " use_backend foo if acl_host_foo\n\n" + "backend bar\n server srv2 10.0.0.2:80\n\n" + "backend foo\n" + " server srv1 10.0.0.1:80\n" + " timeout server 60s\n" + ) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: config) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration", params={"backend": "foo"}), state) + + results = context.action_results + assert results is not None + out = results["configuration"] + assert "backend foo" in out + assert "server srv1 10.0.0.1:80" in out + # the LAST line of the trailing section is not dropped + assert "timeout server 60s" in out + # earlier sections are excluded + assert "frontend haproxy" not in out + assert "server srv2 10.0.0.2:80" not in out + assert "maxconn" not in out + def test_source_relations_previews_config(self) -> None: """ arrange: create state with a haproxy-route relation for a specific backend. From f86e7bdf9e3b309b017dd9038196d873c1d596d8 Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 23 Jul 2026 02:07:39 -0400 Subject: [PATCH 10/26] ci: Rerun flaky integration test From f8093fd56903c6f408f39e24854f703930ceb4ef Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 23 Jul 2026 02:15:58 -0400 Subject: [PATCH 11/26] ci: Rerun flaky integration test From dfb9e6c331a542f55bbb737c546e313cd1171d93 Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 23 Jul 2026 02:38:08 -0400 Subject: [PATCH 12/26] ci: Rerun flaky integration test From 0e9d039bfe9b05d168fc2538538d3bc8359a340d Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 23 Jul 2026 02:39:59 -0400 Subject: [PATCH 13/26] ci: Rerun flaky integration test From 085eca9c8ab28a2986969e8eb052873a8c0f7955 Mon Sep 17 00:00:00 2001 From: tphan025 Date: Mon, 10 Aug 2026 19:11:45 +0200 Subject: [PATCH 14/26] set timeout for page --- tests/integration/test_oauth_spoe.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_oauth_spoe.py b/tests/integration/test_oauth_spoe.py index 8598d397b..213d93f78 100644 --- a/tests/integration/test_oauth_spoe.py +++ b/tests/integration/test_oauth_spoe.py @@ -119,7 +119,9 @@ def _assert_idp_login_success(haproxy_unit_ip, hostname, test_email, test_passwo ) context = browser.new_context(ignore_https_errors=True) page = context.new_page() + page.set_default_timeout(60000) page.goto(f"https://{hostname}") + page.wait_for_load_state("networkidle") logger.info("Page content: %s", page.content()) expect(page).not_to_have_title(re.compile("Sign in failed")) # This will timeout if there is no email field. From ddb1a26ed800f83b288993b8df6051a45fa9dccb Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 10:37:28 -0400 Subject: [PATCH 15/26] refactor(haproxy): scope get-configuration action to source=disk only --- haproxy-operator/charmcraft.yaml | 33 +-- haproxy-operator/src/charm.py | 262 ++---------------- haproxy-operator/src/haproxy.py | 79 +----- .../tests/integration/test_actions.py | 47 +--- haproxy-operator/tests/unit/test_charm.py | 246 +--------------- 5 files changed, 42 insertions(+), 625 deletions(-) diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 3982070d9..9130598fe 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -148,36 +148,9 @@ actions: If no backend with this name is present, an empty list is returned. get-configuration: description: | - Return the haproxy configuration. By default (source=disk) returns the - rendered configuration currently on disk (/etc/haproxy/haproxy.cfg) that - haproxy is running. Set source=relations to instead read the - haproxy-route configuration that is enroute to be generated from the current - relation data, without writing to disk or reloading the service. - Intended for debugging purposes. - params: - source: - type: string - description: | - default: disk - enum: - - disk - - relations - full: - type: boolean - default: false - description: | - When false (the default), the constant sections that are identical - across all deployments (global, defaults, the prometheus frontend and - the fallback backend) are hidden for readability. Set to true to - return the complete configuration. - backend: - type: string - description: | - If set, return only the `backend ` section for this backend - The shared frontend and the constant scaffold are not included; - use full=true to see those. Takes precedence over 'full'. If no - such backend exists, an empty configuration is returned and the - backends that are present are logged. + Return the rendered haproxy configuration currently on disk + (/etc/haproxy/haproxy.cfg) that haproxy is running. Intended for + debugging purposes. charm-libs: - lib: traefik_k8s.ingress_per_unit diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 32490baa6..78c7acb6c 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -108,121 +108,6 @@ def _validate_port(port: int) -> bool: return 0 <= port <= 65535 -def _strip_shared_boundaries(configuration: str, reference: str) -> str: - """Remove the leading and trailing lines ``configuration`` shares with ``reference``. - - Keeps everything between the first and last differing line, so - operator-specific content is never dropped. Returns the input verbatim when - nothing is shared at the boundaries. - - Args: - configuration: The configuration to trim. - reference: The reference configuration to compare against. - - Returns: - The trimmed configuration. - """ - config_lines = configuration.splitlines() - reference_lines = reference.splitlines() - - start = 0 - while ( - start < len(config_lines) - and start < len(reference_lines) - and config_lines[start] == reference_lines[start] - ): - start += 1 - - config_end = len(config_lines) - reference_end = len(reference_lines) - while ( - config_end > start - and reference_end > start - and config_lines[config_end - 1] == reference_lines[reference_end - 1] - ): - config_end -= 1 - reference_end -= 1 - - if start == 0 and config_end == len(config_lines): - return configuration - return "\n".join(config_lines[start:config_end]) - - -def _is_section_header(line: str) -> bool: - """Return whether a line starts a new haproxy config section. - - Args: - line: A single configuration line. - - Returns: - True if the line begins a section. - """ - return bool(line) and not line[0].isspace() and not line.lstrip().startswith("#") - - -def _split_config_sections(configuration: str) -> list[list[str]]: - """Split an haproxy configuration into its sections, preserving lines verbatim. - - Each section is a header line plus every line beneath it, up to (but not - including) the next header. - - Args: - configuration: The haproxy configuration text. - - Returns: - A list of sections, each the list of lines it contains. - """ - sections: list[list[str]] = [] - current: list[str] = [] - for line in configuration.splitlines(): - # A new header means the section we were accumulating is finished. - if _is_section_header(line) and current: - sections.append(current) - current = [] - current.append(line) - if current: - sections.append(current) - return sections - - -def _filter_config_by_backend(configuration: str, backend_name: str) -> str: - """Return the ``backend `` section(s) for ``backend_name``. - - A section-aware alternative to grepping the output: a section is matched only - by its header (``backend ``). - - Args: - configuration: The haproxy configuration text. - backend_name: The backend name to filter for. - - Returns: - The matching ``backend`` section(s), or "" if none match. - """ - matched = [] - for section in _split_config_sections(configuration): - tokens = section[0].split() - if len(tokens) >= 2 and tokens[0] == "backend" and tokens[1] == backend_name: - matched.append(section) - return "\n".join("\n".join(section) for section in matched) - - -def _config_backend_names(configuration: str) -> list[str]: - """Return the names of every ``backend`` section in the configuration. - - Args: - configuration: The haproxy configuration text. - - Returns: - The backend names, in the order they appear. - """ - names: list[str] = [] - for section in _split_config_sections(configuration): - tokens = section[0].split() - if len(tokens) >= 2 and tokens[0] == "backend": - names.append(tokens[1]) - return names - - # pylint: disable=too-many-instance-attributes class HAProxyCharm(ops.CharmBase): """Charm haproxy.""" @@ -842,85 +727,31 @@ def _on_get_proxied_endpoints_action(self, event: ActionEvent) -> None: event.set_results({"endpoints": json.dumps(proxied_endpoints)}) def _on_get_configuration_action(self, event: ActionEvent) -> None: - """Triggered when users run the `get-configuration` Juju action. + """Return the on-disk haproxy configuration for debugging. - `source=disk` (default) returns the on-disk configuration. - `source=relations` renders the haproxy-route configuration from the - current relation data. Neither writes to disk nor reloads the service. - When a haproxy-route-policy relation is present, the rendered policy - backend reflects the policy charm's current, asynchronously-converging - output. + Reads the rendered configuration currently on disk + (/etc/haproxy/haproxy.cfg) that haproxy is running. Does not write to + disk or reload the service. Args: event: Juju event """ - source = event.params.get("source", "disk") - if source == "relations": - try: - configuration = self._recompute_haproxy_route_configuration() - except ( - CharmStateValidationBaseError, - HaproxyRouteIntegrationDataValidationError, - ) as exc: - event.fail(f"Failed to recompute configuration from relations: {exc}") - return - if self.haproxy_route_policy.relation is not None: - event.log( - "A haproxy-route-policy relation is present; the policy backend in this " - "preview reflects the policy charm's current output and converges " - "asynchronously. Use source=disk for the authoritative applied configuration." - ) - else: - if not file_exists(HAPROXY_CONFIG): - event.fail( - f"HAProxy configuration file {HAPROXY_CONFIG} does not exist yet. " - "Ensure the charm is configured and integrated before running this action." - ) - return - configuration = read_file(HAPROXY_CONFIG) - - backend = typing.cast(str, event.params.get("backend", "")).strip() - if backend: - configuration = self._filter_configuration_for_backend(configuration, backend, event) - else: - if self._configuration_is_default(configuration): - event.log( - "The HAProxy configuration matches the default configuration. This usually " - "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " - "reverseproxy relations)." - ) - - if not typing.cast(bool, event.params.get("full", False)): - configuration = self._hide_constant_configuration(configuration, event) - - event.set_results({"configuration": configuration, "source": source}) - - def _recompute_haproxy_route_configuration(self) -> str: - """Render the haproxy-route configuration from the current relation data. + if not file_exists(HAPROXY_CONFIG): + event.fail( + f"HAProxy configuration file {HAPROXY_CONFIG} does not exist yet. " + "Ensure the charm is configured and integrated before running this action." + ) + return + configuration = read_file(HAPROXY_CONFIG) - Unlike `_configure_haproxy_route`, performs no side effects (no port - changes, databag writes, file writes, or reload). + if self._configuration_is_default(configuration): + event.log( + "The HAProxy configuration matches the default configuration. This usually " + "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " + "reverseproxy relations)." + ) - Returns: - The rendered haproxy-route configuration. - """ - charm_state = self._charm_state() - haproxy_route_requirers_information = HaproxyRouteRequirersInformation.from_provider( - haproxy_route=self.haproxy_route_provider, - haproxy_route_tcp=self.haproxy_route_tcp_provider, - haproxy_route_policy=self.haproxy_route_policy, - external_hostname=typing.cast("str | None", self.config.get("external-hostname")), - peers=self._get_peer_units_address(), - ca_certs_configured=bool(self.recv_ca_certs.get_all_certificates()), - ) - ddos_protection_config = DDosProtection.from_charm(self.ddos_requirer) - spoe_oauth_info_list = SpoeAuthInformation.from_requirer(self.spoe_auth_requirer) - return self.haproxy_service.render_haproxy_route_config( - charm_state, - haproxy_route_requirers_information, - spoe_oauth_info_list, - ddos_protection_config, - ) + event.set_results({"configuration": configuration, "source": "disk"}) def _configuration_is_default(self, configuration: str) -> bool: """Return whether the given configuration matches the default configuration. @@ -937,63 +768,6 @@ def _configuration_is_default(self, configuration: str) -> bool: return False return configuration == default_configuration - def _hide_constant_configuration(self, configuration: str, event: ActionEvent) -> str: - """Hide the constant scaffold the config shares with the default render. - - The shared head/tail (global, defaults, prometheus frontend, fallback - backend) is derived from ``render_default_config`` rather than hard-coded - section names, so it stays correct if the template changes. Notifies the - user via ``event.log`` when anything is hidden. - - Args: - configuration: The configuration to trim. - event: Juju event, used to notify the user when content is hidden. - - Returns: - The trimmed configuration, or the input unchanged if the default - configuration cannot be rendered. - """ - try: - default_configuration = self.haproxy_service.render_default_config(self._charm_state()) - except CharmStateValidationBaseError: - return configuration - - trimmed = _strip_shared_boundaries(configuration, default_configuration) - if trimmed != configuration: - event.log( - "Constant/default config sections (global, defaults, the prometheus frontend " - "and the fallback backend) that are identical across deployments have been " - "hidden for readability. Re-run this action with full=true to return the " - "complete configuration." - ) - return trimmed - - def _filter_configuration_for_backend( - self, configuration: str, backend_name: str, event: ActionEvent - ) -> str: - """Return only the ``backend `` section for ``backend_name``. - - Args: - configuration: The configuration to filter. - backend_name: The backend name to filter for. - event: Juju event, used to notify the caller when nothing matches. - - Returns: - The matching sections, or "" if the backend is not present. - """ - filtered = _filter_config_by_backend(configuration, backend_name) - if not filtered: - available = _config_backend_names(configuration) - event.log( - f"No configuration section involves a backend named '{backend_name}'. " - + ( - f"Backends present: {', '.join(available)}." - if available - else "No backends are configured." - ) - ) - return filtered - def _publish_haproxy_route_proxied_endpoints( self, haproxy_route_requirers_information: HaproxyRouteRequirersInformation ) -> None: diff --git a/haproxy-operator/src/haproxy.py b/haproxy-operator/src/haproxy.py index 8c8c6373b..a3949d96d 100644 --- a/haproxy-operator/src/haproxy.py +++ b/haproxy-operator/src/haproxy.py @@ -173,75 +173,8 @@ def reconcile_haproxy_route( store_config_to_file(ddos_protection_config.ip_allow_list, IP_ALLOW_LIST_FILE) store_config_to_file(ddos_protection_config.deny_paths, DENY_PATHS_FILE) - template_context = self._build_haproxy_route_template_context( - charm_state, - haproxy_route_requirers_information, - spoe_oauth_info_list, - ddos_protection_config, - ) - self._render_haproxy_config(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) - if spoe_oauth_info_list: - spoe_auth_template_context = { - "spoe_auth_info_list": spoe_oauth_info_list, - } - self._render_config_file( - SPOE_AUTH_CONFIG_TEMPLATE, spoe_auth_template_context, SPOE_AUTH_CONFIG - ) - self._validate_haproxy_config() - self._reload_haproxy_service() - - def render_haproxy_route_config( - self, - charm_state: CharmState, - haproxy_route_requirers_information: HaproxyRouteRequirersInformation, - spoe_oauth_info_list: list[SpoeAuthInformation], - ddos_protection_config: DDosProtection, - ) -> str: - """Render the haproxy-route configuration and return it as a string. - - Unlike `reconcile_haproxy_route`, performs no side effects (no file - writes, validation, or reload). - - Args: - charm_state: The charm state component. - haproxy_route_requirers_information: HaproxyRouteRequirersInformation state component. - spoe_oauth_info_list: Information about SPOE auth providers. - ddos_protection_config: DDoS protection configuration. - - Returns: - The rendered haproxy-route configuration. - """ - template_context = self._build_haproxy_route_template_context( - charm_state, - haproxy_route_requirers_information, - spoe_oauth_info_list, - ddos_protection_config, - ) - return self._render_to_string(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) - - def _build_haproxy_route_template_context( - self, - charm_state: CharmState, - haproxy_route_requirers_information: HaproxyRouteRequirersInformation, - spoe_oauth_info_list: list[SpoeAuthInformation], - ddos_protection_config: DDosProtection, - ) -> dict: - """Build the template context for the haproxy-route configuration. - - Shared by `reconcile_haproxy_route` and `render_haproxy_route_config` - so they cannot drift. - - Args: - charm_state: The charm state component. - haproxy_route_requirers_information: HaproxyRouteRequirersInformation state component. - spoe_oauth_info_list: Information about SPOE auth providers. - ddos_protection_config: DDoS protection configuration. - - Returns: - The template context for the haproxy-route template. - """ valid_backends = haproxy_route_requirers_information.valid_backends() - return { + template_context = { "config_global_max_connection": charm_state.global_max_connection, "enable_hsts": charm_state.enable_hsts, "ddos_protection": charm_state.ddos_protection, @@ -266,6 +199,16 @@ def _build_haproxy_route_template_context( "deny_paths_file": DENY_PATHS_FILE, "policy_provider_backend": haproxy_route_requirers_information.policy_provider_backend, } + self._render_haproxy_config(HAPROXY_ROUTE_CONFIG_TEMPLATE, template_context) + if spoe_oauth_info_list: + spoe_auth_template_context = { + "spoe_auth_info_list": spoe_oauth_info_list, + } + self._render_config_file( + SPOE_AUTH_CONFIG_TEMPLATE, spoe_auth_template_context, SPOE_AUTH_CONFIG + ) + self._validate_haproxy_config() + self._reload_haproxy_service() def reconcile_default(self, charm_state: CharmState) -> None: """Render the default haproxy config and reload the service. diff --git a/haproxy-operator/tests/integration/test_actions.py b/haproxy-operator/tests/integration/test_actions.py index e9ef89018..54534643d 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -116,9 +116,8 @@ def test_get_configuration_action( juju: jubilant.Juju, ): """arrange: Deploy the charm integrated with any_charm haproxy-route. - act: Trigger the action 'get-configuration' in disk and relations modes. - assert: The returned configuration matches the on-disk haproxy.cfg, and the - relations-mode preview matches the applied configuration. + act: Trigger the 'get-configuration' action. + assert: The returned configuration matches the on-disk haproxy.cfg. """ _integrate_haproxy_route( juju, f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer @@ -150,46 +149,10 @@ def test_get_configuration_action( on_disk = juju.ssh(f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg") - # full=true must return the complete configuration matching what is on disk. - task = juju.run(f"{configured_application_with_tls}/0", "get-configuration", {"full": True}) - assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results - - # Recomputing from relations (source=relations) with full=true must match the - # applied config when the deployment is settled, without touching disk. - task = juju.run( - f"{configured_application_with_tls}/0", - "get-configuration", - {"source": "relations", "full": True}, - ) - assert task.results["source"] == "relations", task.results - assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results - - # Default (full=false) hides the constant scaffold shared with the default - # render (e.g. the prometheus frontend) but keeps the operator-specific backend. + # The action returns exactly the configuration currently on disk. task = juju.run(f"{configured_application_with_tls}/0", "get-configuration") - default_config = task.results["configuration"] - assert "frontend prometheus" not in default_config, task.results - assert service_name in default_config, task.results - - # Per-backend filter returns only that backend's own section, section-aware - # (not grep). Derive a real, non-default backend name from the full config. - full_config = juju.run( - f"{configured_application_with_tls}/0", "get-configuration", {"full": True} - ).results["configuration"] - backend_names = [ - line.split()[1] - for line in full_config.splitlines() - if line.startswith("backend ") and line.split()[1] != "default" - ] - assert backend_names, f"expected at least one non-default backend:\n{full_config}" - target = backend_names[0] - filtered = juju.run( - f"{configured_application_with_tls}/0", "get-configuration", {"backend": target} - ).results["configuration"] - assert f"backend {target}" in filtered, filtered - # only the backend section is returned: the frontend and scaffold are excluded - assert "frontend haproxy" not in filtered, filtered - assert "frontend prometheus" not in filtered, filtered + assert task.results["source"] == "disk", task.results + assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results juju.remove_relation( f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 3068df56e..3d186058b 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -477,11 +477,11 @@ def test_spoe_auth_invalid_data(monkeypatch: pytest.MonkeyPatch, certificates_in class TestGetConfigurationAction: """Test "get-configuration" Action.""" - def test_returns_full_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_returns_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: """ arrange: mock the config file on disk with known content. - act: trigger the get-configuration action with full=true. - assert: the full file content is returned unchanged. + act: trigger the get-configuration action. + assert: the on-disk file content is returned unchanged. """ content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" monkeypatch.setattr(charm_module, "file_exists", lambda _: True) @@ -489,245 +489,9 @@ def test_returns_full_configuration(self, monkeypatch: pytest.MonkeyPatch) -> No context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration", params={"full": True}), state) - - assert context.action_results == {"configuration": content, "source": "disk"} - - def test_hides_constant_configuration_by_default( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: - """ - arrange: mock the on-disk config as the default scaffold wrapping an - operator-specific frontend section. - act: trigger the get-configuration action without full=true. - assert: the constant head/tail shared with the default render is hidden, - the operator-specific section is kept, and the user is notified. - """ - default_config = "global\n maxconn 4096\n\nbackend default\n return 200\n" - on_disk = ( - "global\n maxconn 4096\n\n" - "frontend haproxy\n bind :443\n" - "backend default\n return 200\n" - ) - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: on_disk) - monkeypatch.setattr( - charm_module.HAProxyService, "render_default_config", lambda self, _: default_config - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration"), state) - out = context.action_results - assert out is not None - configuration = out["configuration"] - assert "global" not in configuration - assert "maxconn" not in configuration - assert "frontend haproxy" in configuration - assert "bind :443" in configuration - assert any("hidden for readability" in log.lower() for log in context.action_logs) - - def test_backend_filter_returns_whole_section(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock an on-disk config with a frontend and two backends, where the - target backend's body has a line that does NOT contain the backend name. - act: trigger the get-configuration action with backend=foo. - assert: only the whole `backend foo` section is returned (including the body - line grep would miss); the shared frontend, the unrelated backend and - the global scaffold are all excluded. - """ - config = ( - "global\n maxconn 4096\n\n" - "frontend haproxy\n" - " use_backend foo if acl_host_foo\n" - " use_backend bar if acl_host_bar\n\n" - "backend foo\n" - " server srv1 10.0.0.1:80\n" - " timeout server 60s\n\n" - "backend bar\n" - " server srv2 10.0.0.2:80\n" - ) - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: config) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - - context.run(context.on.action("get-configuration", params={"backend": "foo"}), state) - - results = context.action_results - assert results is not None - out = results["configuration"] - assert "backend foo" in out - # whole-section body preserved — grep for "foo" would have dropped this line - assert "server srv1 10.0.0.1:80" in out - assert "timeout server 60s" in out - # the shared frontend is NOT included (use full for that) - assert "frontend haproxy" not in out - # the unrelated backend's section is excluded (its unique server line is gone) - assert "server srv2 10.0.0.2:80" not in out - # the constant scaffold is excluded - assert "maxconn" not in out - - def test_backend_filter_unknown_logs_available(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock an on-disk config with a single backend. - act: trigger the get-configuration action with a backend name that is absent. - assert: an empty configuration is returned and the available backends are logged. - """ - config = "global\n maxconn 4096\n\nbackend foo\n server srv1 10.0.0.1:80\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: config) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - - context.run(context.on.action("get-configuration", params={"backend": "nope"}), state) - - results = context.action_results - assert results is not None - assert results["configuration"] == "" - assert any("nope" in log and "foo" in log for log in context.action_logs), ( - context.action_logs - ) - - def test_backend_filter_matches_last_section(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock an on-disk config where the target backend is the LAST section, - with no section after it — the case the section splitter's post-loop flush - must handle (haproxy-route backends are appended at the end of the file). - act: trigger the get-configuration action with backend=foo. - assert: the whole trailing `backend foo` section is returned, body and all, - and nothing preceding it leaks in. - """ - config = ( - "global\n maxconn 4096\n\n" - "frontend haproxy\n" - " use_backend bar if acl_host_bar\n" - " use_backend foo if acl_host_foo\n\n" - "backend bar\n server srv2 10.0.0.2:80\n\n" - "backend foo\n" - " server srv1 10.0.0.1:80\n" - " timeout server 60s\n" - ) - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: config) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - - context.run(context.on.action("get-configuration", params={"backend": "foo"}), state) - - results = context.action_results - assert results is not None - out = results["configuration"] - assert "backend foo" in out - assert "server srv1 10.0.0.1:80" in out - # the LAST line of the trailing section is not dropped - assert "timeout server 60s" in out - # earlier sections are excluded - assert "frontend haproxy" not in out - assert "server srv2 10.0.0.2:80" not in out - assert "maxconn" not in out - - def test_source_relations_previews_config(self) -> None: - """ - arrange: create state with a haproxy-route relation for a specific backend. - act: trigger the get-configuration action with source=relations. - assert: the recomputed configuration is returned (containing the backend) - without reading from disk. - """ - service_name = "haproxy-tutorial-ingress-configurator" - context = ops.testing.Context(HAProxyCharm) - haproxy_route_relation = ops.testing.Relation( - "haproxy-route", - remote_app_data={ - "hostname": f'"{TEST_EXTERNAL_HOSTNAME_CONFIG}"', - "paths": '["/v1"]', - "ports": "[443]", - "protocol": '"http"', - "service": f'"{service_name}"', - }, - remote_units_data={0: {"address": '"10.75.1.129"'}}, - ) - state = ops.testing.State( - relations=[haproxy_route_relation], - leader=True, - model=ops.testing.Model(name="haproxy-tutorial"), - app_status=ops.testing.ActiveStatus(), - unit_status=ops.testing.ActiveStatus(), - ) - - context.run(context.on.action("get-configuration", params={"source": "relations"}), state) - - out = context.action_results - assert out is not None - assert out["source"] == "relations" - assert service_name in out["configuration"] - - def test_warns_when_policy_relation_present(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: create state with a haproxy-route-policy relation. - act: trigger the get-configuration action with source=relations. - assert: a warning about the policy backend converging asynchronously is logged. - """ - monkeypatch.setattr( - charm_module.HAProxyCharm, - "_recompute_haproxy_route_configuration", - lambda self: "global\n", - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State( - relations=[ops.testing.Relation("haproxy-route-policy")], - leader=True, - ) - - context.run(context.on.action("get-configuration", params={"source": "relations"}), state) - - assert any("haproxy-route-policy" in log.lower() for log in context.action_logs) - - @pytest.mark.parametrize("num_relations", [20, 100]) - def test_source_relations_scales(self, num_relations: int) -> None: - """ - arrange: create state with many haproxy-route relations (one backend each). - act: trigger the get-configuration action with source=relations. - assert: every backend appears in the recomputed configuration (nothing is - truncated at the charm level). - """ - context = ops.testing.Context(HAProxyCharm) - relations = [] - service_names = [] - for i in range(num_relations): - service = f"backend-service-{i}" - service_names.append(service) - relations.append( - ops.testing.Relation( - "haproxy-route", - remote_app_data={ - "hostname": f'"svc{i}.{TEST_EXTERNAL_HOSTNAME_CONFIG}"', - "paths": '["/v1"]', - "ports": "[443]", - "protocol": '"http"', - "service": f'"{service}"', - }, - remote_units_data={0: {"address": '"10.75.1.129"'}}, - ) - ) - state = ops.testing.State( - relations=relations, - leader=True, - model=ops.testing.Model(name="haproxy-tutorial"), - app_status=ops.testing.ActiveStatus(), - unit_status=ops.testing.ActiveStatus(), - ) - - context.run(context.on.action("get-configuration", params={"source": "relations"}), state) - - out = context.action_results - assert out is not None - full_config = out["configuration"] - for service in service_names: - assert service in full_config, ( - f"{service} missing from a {num_relations}-relation config" - ) + assert context.action_results == {"configuration": content, "source": "disk"} def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: """ @@ -759,7 +523,7 @@ def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration", params={"full": True}), state) + context.run(context.on.action("get-configuration"), state) assert context.action_results == {"configuration": default_config, "source": "disk"} assert any("default configuration" in log.lower() for log in context.action_logs) From a617a6c142409e73257f88361f01121883ece128 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 11:26:31 -0400 Subject: [PATCH 16/26] get-configuration action to source=disk only --- haproxy-operator/src/charm.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 78c7acb6c..dbb1b453e 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -285,13 +285,10 @@ def _on_http_backend_removed(self, _: HTTPBackendRemovedEvent) -> None: """Handle data_removed event for reverseproxy integration.""" self._reconcile() - def _charm_state(self) -> CharmState: - """Build the charm state from the current charm and its providers. - - Returns: - The charm state component. - """ - return CharmState.from_charm( + def _reconcile(self) -> None: + """Render the haproxy config and restart the service.""" + self.unit.status = ops.MaintenanceStatus("Configuring haproxy.") + charm_state = CharmState.from_charm( self, self._ingress_provider, self._ingress_per_unit_provider, @@ -300,11 +297,6 @@ def _charm_state(self) -> CharmState: self.reverseproxy_requirer, self.haproxy_route_policy, ) - - def _reconcile(self) -> None: - """Render the haproxy config and restart the service.""" - self.unit.status = ops.MaintenanceStatus("Configuring haproxy.") - charm_state = self._charm_state() proxy_mode = charm_state.mode if proxy_mode == ProxyMode.INVALID: # We don't raise any exception/set status here as it should already be handled @@ -763,7 +755,17 @@ def _configuration_is_default(self, configuration: str) -> bool: True if it is identical to the rendered default configuration. """ try: - default_configuration = self.haproxy_service.render_default_config(self._charm_state()) + default_configuration = self.haproxy_service.render_default_config( + CharmState.from_charm( + self, + self._ingress_provider, + self._ingress_per_unit_provider, + self.haproxy_route_provider, + self.haproxy_route_tcp_provider, + self.reverseproxy_requirer, + self.haproxy_route_policy, + ) + ) except CharmStateValidationBaseError: return False return configuration == default_configuration From f7c3ef43c6baaf72c08000a5062e79cc7f8b3028 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 12:08:45 -0400 Subject: [PATCH 17/26] chore: stop tracking local scratch ignores in .gitignore --- .gitignore | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.gitignore b/.gitignore index e34ff6d59..50f72334c 100644 --- a/.gitignore +++ b/.gitignore @@ -38,10 +38,3 @@ haproxy-route-policy/.python-version artifacts.build.yaml .worktrees - -# local juju/lxd logs -*.log - -# local scratch documentation -PROGRESS.md -CHARM_EXPLAINER.md From 21b76b6d1a4295c5f3061d644643a54a88b18494 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 12:15:28 -0400 Subject: [PATCH 18/26] Fixing linting error --- docs/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.md b/docs/changelog.md index 48c0fb383..9d81c3402 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,7 @@ Each revision is versioned by the date of the revision. ## 2026-07-17 - Added a `get-configuration` action to inspect the effective HAProxy configuration. + ## 2026-07-06 - docs: Add a how-to guide for configuring the backend protocol. From cfec9e1fa4f5b57a71073781a12c64ef9768a0a0 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 14:12:39 -0400 Subject: [PATCH 19/26] Apply suggestions from code review Co-authored-by: Phan Trung Thanh --- haproxy-operator/charmcraft.yaml | 4 +--- haproxy-operator/src/charm.py | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 9130598fe..e461e56ea 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -148,9 +148,7 @@ actions: If no backend with this name is present, an empty list is returned. get-configuration: description: | - Return the rendered haproxy configuration currently on disk - (/etc/haproxy/haproxy.cfg) that haproxy is running. Intended for - debugging purposes. + Return the rendered haproxy configuration currently on the haproxy unit. Intended for debugging purposes. charm-libs: - lib: traefik_k8s.ingress_per_unit diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index dbb1b453e..84248079a 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -730,8 +730,7 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: """ if not file_exists(HAPROXY_CONFIG): event.fail( - f"HAProxy configuration file {HAPROXY_CONFIG} does not exist yet. " - "Ensure the charm is configured and integrated before running this action." + f"HAProxy configuration file at {HAPROXY_CONFIG} not found. " ) return configuration = read_file(HAPROXY_CONFIG) @@ -739,8 +738,7 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: if self._configuration_is_default(configuration): event.log( "The HAProxy configuration matches the default configuration. This usually " - "means no proxy backends are configured (e.g. no haproxy-route, ingress, or " - "reverseproxy relations)." + "means no proxy backends are configured." ) event.set_results({"configuration": configuration, "source": "disk"}) From d57e6ae26e19e770de51a2feca71b36036455493 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 15:01:02 -0400 Subject: [PATCH 20/26] Update error catching so that it makes more sense when charm_state parsing in _configuration_is_default fails --- haproxy-operator/src/charm.py | 41 +++++++++++++---------- haproxy-operator/tests/unit/test_charm.py | 28 +++++++++++++++- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 84248079a..8144c9742 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -729,13 +729,19 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: event: Juju event """ if not file_exists(HAPROXY_CONFIG): - event.fail( - f"HAProxy configuration file at {HAPROXY_CONFIG} not found. " - ) + event.fail(f"HAProxy configuration file at {HAPROXY_CONFIG} not found. ") return configuration = read_file(HAPROXY_CONFIG) - if self._configuration_is_default(configuration): + try: + configuration_is_default = self._configuration_is_default(configuration) + except CharmStateValidationBaseError: + event.log( + "Could not determine whether this is the default configuration because the " + "charm state is invalid; see `juju status` for the blocking condition." + ) + configuration_is_default = False + if configuration_is_default: event.log( "The HAProxy configuration matches the default configuration. This usually " "means no proxy backends are configured." @@ -749,23 +755,24 @@ def _configuration_is_default(self, configuration: str) -> bool: Args: configuration: The configuration to compare against the default. + Raises: + CharmStateValidationBaseError: When the charm state needed to render + the default configuration cannot be built. + Returns: True if it is identical to the rendered default configuration. """ - try: - default_configuration = self.haproxy_service.render_default_config( - CharmState.from_charm( - self, - self._ingress_provider, - self._ingress_per_unit_provider, - self.haproxy_route_provider, - self.haproxy_route_tcp_provider, - self.reverseproxy_requirer, - self.haproxy_route_policy, - ) + default_configuration = self.haproxy_service.render_default_config( + CharmState.from_charm( + self, + self._ingress_provider, + self._ingress_per_unit_provider, + self.haproxy_route_provider, + self.haproxy_route_tcp_provider, + self.reverseproxy_requirer, + self.haproxy_route_policy, ) - except CharmStateValidationBaseError: - return False + ) return configuration == default_configuration def _publish_haproxy_route_proxied_endpoints( diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 3d186058b..4f3e78d3b 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -506,7 +506,7 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: with pytest.raises(ops.testing.ActionFailed) as exc_info: context.run(context.on.action("get-configuration"), state) - assert "does not exist" in exc_info.value.message + assert "not found" in exc_info.value.message def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> None: """ @@ -551,3 +551,29 @@ def test_no_warning_when_config_differs_from_default( context.run(context.on.action("get-configuration"), state) assert not any("default configuration" in log.lower() for log in context.action_logs) + + def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: the config file is present, but building the charm state raises. + act: trigger the get-configuration action. + assert: the configuration is still returned, and a "could not determine" note is + logged instead of silently claiming it is not the default. + """ + content = "frontend haproxy\n bind :80\n" + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: content) + + def _raise(*_args: object, **_kwargs: object) -> charm_module.CharmState: + raise charm_module.CharmStateValidationBaseError("invalid config") + + monkeypatch.setattr(charm_module.CharmState, "from_charm", _raise) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + results = context.action_results + assert results is not None + assert results == {"configuration": content, "source": "disk"} + assert any("could not determine" in log.lower() for log in context.action_logs) + assert not any("matches the default" in log.lower() for log in context.action_logs) From cda77c5dbd921b086f3b8fed631b94c8d35c1fe8 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 16:51:24 -0400 Subject: [PATCH 21/26] Implememnt test case changes following suggestions --- haproxy-operator/tests/unit/test_charm.py | 84 +++++++++++------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 4f3e78d3b..c067e117e 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -484,8 +484,8 @@ def test_returns_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: assert: the on-disk file content is returned unchanged. """ content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: content) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -499,7 +499,7 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: act: trigger the get-configuration action. assert: the action fails with a clear message rather than returning empty. """ - monkeypatch.setattr(charm_module, "file_exists", lambda _: False) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=False)) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -508,49 +508,50 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: assert "not found" in exc_info.value.message - def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock the on-disk config to equal the rendered default config. - act: trigger the get-configuration action. - assert: a warning is logged and the configuration is still returned. - """ - default_config = "global\n maxconn 4096\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: default_config) - monkeypatch.setattr( - charm_module.HAProxyService, "render_default_config", lambda self, _: default_config - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - - context.run(context.on.action("get-configuration"), state) - - assert context.action_results == {"configuration": default_config, "source": "disk"} - assert any("default configuration" in log.lower() for log in context.action_logs) - - def test_no_warning_when_config_differs_from_default( - self, monkeypatch: pytest.MonkeyPatch + @pytest.mark.parametrize( + "on_disk_config, rendered_default, expect_default_warning", + [ + pytest.param( + "global\n maxconn 4096\n", + "global\n maxconn 4096\n", + True, + id="matches-default", + ), + pytest.param( + "frontend haproxy\n bind :80\n", + "global\n maxconn 4096\n", + False, + id="differs-from-default", + ), + ], + ) + def test_default_configuration_warning( + self, + monkeypatch: pytest.MonkeyPatch, + on_disk_config: str, + rendered_default: str, + expect_default_warning: bool, ) -> None: """ - arrange: mock the on-disk config to differ from the rendered default config. + arrange: mock the on-disk config to either match or differ from the rendered default. act: trigger the get-configuration action. - assert: no default-configuration warning is logged. + assert: the configuration is returned, and the "matches default" warning is logged + only when the config is the default. """ - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr( - charm_module, "read_file", lambda _: "frontend haproxy\n bind :80\n" - ) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=on_disk_config)) monkeypatch.setattr( - charm_module.HAProxyService, - "render_default_config", - lambda self, _: "global\n maxconn 4096\n", + "charm.HAProxyService.render_default_config", + MagicMock(return_value=rendered_default), ) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) context.run(context.on.action("get-configuration"), state) - assert not any("default configuration" in log.lower() for log in context.action_logs) + assert context.action_results == {"configuration": on_disk_config, "source": "disk"} + warned = any("default configuration" in log.lower() for log in context.action_logs) + assert warned == expect_default_warning def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.MonkeyPatch) -> None: """ @@ -560,13 +561,12 @@ def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.Monkey logged instead of silently claiming it is not the default. """ content = "frontend haproxy\n bind :80\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: content) - - def _raise(*_args: object, **_kwargs: object) -> charm_module.CharmState: - raise charm_module.CharmStateValidationBaseError("invalid config") - - monkeypatch.setattr(charm_module.CharmState, "from_charm", _raise) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) + monkeypatch.setattr( + "charm.CharmState.from_charm", + MagicMock(side_effect=charm_module.CharmStateValidationBaseError("invalid config")), + ) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) From cbb455635643947be856ad0ef049af61ca8a3618 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 21:58:40 -0400 Subject: [PATCH 22/26] revert review-comment test changes --- haproxy-operator/tests/unit/test_charm.py | 84 +++++++++++------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index c067e117e..4f3e78d3b 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -484,8 +484,8 @@ def test_returns_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: assert: the on-disk file content is returned unchanged. """ content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: content) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -499,7 +499,7 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: act: trigger the get-configuration action. assert: the action fails with a clear message rather than returning empty. """ - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=False)) + monkeypatch.setattr(charm_module, "file_exists", lambda _: False) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -508,50 +508,49 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: assert "not found" in exc_info.value.message - @pytest.mark.parametrize( - "on_disk_config, rendered_default, expect_default_warning", - [ - pytest.param( - "global\n maxconn 4096\n", - "global\n maxconn 4096\n", - True, - id="matches-default", - ), - pytest.param( - "frontend haproxy\n bind :80\n", - "global\n maxconn 4096\n", - False, - id="differs-from-default", - ), - ], - ) - def test_default_configuration_warning( - self, - monkeypatch: pytest.MonkeyPatch, - on_disk_config: str, - rendered_default: str, - expect_default_warning: bool, + def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the on-disk config to equal the rendered default config. + act: trigger the get-configuration action. + assert: a warning is logged and the configuration is still returned. + """ + default_config = "global\n maxconn 4096\n" + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: default_config) + monkeypatch.setattr( + charm_module.HAProxyService, "render_default_config", lambda self, _: default_config + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) + + assert context.action_results == {"configuration": default_config, "source": "disk"} + assert any("default configuration" in log.lower() for log in context.action_logs) + + def test_no_warning_when_config_differs_from_default( + self, monkeypatch: pytest.MonkeyPatch ) -> None: """ - arrange: mock the on-disk config to either match or differ from the rendered default. + arrange: mock the on-disk config to differ from the rendered default config. act: trigger the get-configuration action. - assert: the configuration is returned, and the "matches default" warning is logged - only when the config is the default. + assert: no default-configuration warning is logged. """ - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=on_disk_config)) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr( + charm_module, "read_file", lambda _: "frontend haproxy\n bind :80\n" + ) monkeypatch.setattr( - "charm.HAProxyService.render_default_config", - MagicMock(return_value=rendered_default), + charm_module.HAProxyService, + "render_default_config", + lambda self, _: "global\n maxconn 4096\n", ) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) context.run(context.on.action("get-configuration"), state) - assert context.action_results == {"configuration": on_disk_config, "source": "disk"} - warned = any("default configuration" in log.lower() for log in context.action_logs) - assert warned == expect_default_warning + assert not any("default configuration" in log.lower() for log in context.action_logs) def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.MonkeyPatch) -> None: """ @@ -561,12 +560,13 @@ def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.Monkey logged instead of silently claiming it is not the default. """ content = "frontend haproxy\n bind :80\n" - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) - monkeypatch.setattr( - "charm.CharmState.from_charm", - MagicMock(side_effect=charm_module.CharmStateValidationBaseError("invalid config")), - ) + monkeypatch.setattr(charm_module, "file_exists", lambda _: True) + monkeypatch.setattr(charm_module, "read_file", lambda _: content) + + def _raise(*_args: object, **_kwargs: object) -> charm_module.CharmState: + raise charm_module.CharmStateValidationBaseError("invalid config") + + monkeypatch.setattr(charm_module.CharmState, "from_charm", _raise) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) From c53dcdfefb61dc13d642fdb7ca50c691690aa256 Mon Sep 17 00:00:00 2001 From: minulo Date: Wed, 12 Aug 2026 22:23:11 -0400 Subject: [PATCH 23/26] refactored the test_action file so that the whole file run one function that test both actions. While not the best use of parallelism, it is organized better as all action testing is under one file. --- .../tests/integration/test_actions.py | 86 ++----------------- 1 file changed, 8 insertions(+), 78 deletions(-) diff --git a/haproxy-operator/tests/integration/test_actions.py b/haproxy-operator/tests/integration/test_actions.py index 54534643d..396bd8c7a 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -5,45 +5,20 @@ """Integration tests for haproxy charm actions.""" import json -import time import jubilant import pytest -def _integrate_haproxy_route(juju: jubilant.Juju, provider_endpoint: str, requirer: str) -> None: - """Integrate haproxy-route, retrying while a prior relation is still being removed. - - Tests in this module add and remove the same haproxy-route relation. Because - relation removal is asynchronous, a subsequent integrate can race with it and - fail with "already exists"; retry until the previous relation has cleared. - - Args: - juju: Jubilant juju instance. - provider_endpoint: The haproxy-route provider endpoint (e.g. "haproxy:haproxy-route"). - requirer: The requirer application name. - """ - deadline = time.monotonic() + 120 - while True: - try: - juju.integrate(provider_endpoint, requirer) - return - except jubilant.CLIError as exc: - if "already exists" in str(exc) and time.monotonic() < deadline: - time.sleep(2) - continue - raise - - @pytest.mark.abort_on_fail -def test_get_proxied_endpoints_action( +def test_action( configured_application_with_tls: str, any_charm_haproxy_route_requirer: str, juju: jubilant.Juju, ): """arrange: Deploy the charm integrated with any_charm haproxy-route. - act: Trigger the action 'get-proxied-endpoints. - assert: The correct proxied endpoints are returned. + act: Trigger the charm's actions (get-proxied-endpoints and get-configuration). + assert: Each action returns the expected result. """ juju.integrate( f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer @@ -73,6 +48,7 @@ def test_get_proxied_endpoints_action( ) ) + # get-proxied-endpoints returns an endpoint for every hostname/path combination. expected_endpoints = { "https://ok.haproxy.internal/v1", "https://ok.haproxy.internal/v2", @@ -82,21 +58,19 @@ def test_get_proxied_endpoints_action( "https://ok3.haproxy.internal/v2", } - # Test without backend param + # Test without a backend param (filter) task = juju.run(f"{configured_application_with_tls}/0", "get-proxied-endpoints") - endpoints = set(json.loads(task.results["endpoints"])) assert endpoints == expected_endpoints, task.results - # Test with backend param + # Test with a backend param (filter) task = juju.run( f"{configured_application_with_tls}/0", "get-proxied-endpoints", {"backend": "any_charm"} ) - endpoints = set(json.loads(task.results["endpoints"])) assert endpoints == expected_endpoints, task.results - # Test with backend param with non existing backend + # Test with a non-existing backend task = juju.run( f"{configured_application_with_tls}/0", "get-proxied-endpoints", @@ -104,52 +78,8 @@ def test_get_proxied_endpoints_action( ) assert task.results == {"endpoints": "[]"}, task.results - juju.remove_relation( - f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer - ) - - -@pytest.mark.abort_on_fail -def test_get_configuration_action( - configured_application_with_tls: str, - any_charm_haproxy_route_requirer: str, - juju: jubilant.Juju, -): - """arrange: Deploy the charm integrated with any_charm haproxy-route. - act: Trigger the 'get-configuration' action. - assert: The returned configuration matches the on-disk haproxy.cfg. - """ - _integrate_haproxy_route( - juju, f"{configured_application_with_tls}:haproxy-route", any_charm_haproxy_route_requirer - ) - - service_name = "any_charm" - juju.run( - f"{any_charm_haproxy_route_requirer}/0", - "rpc", - { - "method": "update_relation", - "args": json.dumps( - [ - { - "service": service_name, - "ports": [80], - "hostname": "ok.haproxy.internal", - "paths": ["/v1"], - } - ] - ), - }, - ) - juju.wait( - lambda status: jubilant.all_active( - status, configured_application_with_tls, any_charm_haproxy_route_requirer - ) - ) - + # get-configuration returns exactly the configuration currently on disk. on_disk = juju.ssh(f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg") - - # The action returns exactly the configuration currently on disk. task = juju.run(f"{configured_application_with_tls}/0", "get-configuration") assert task.results["source"] == "disk", task.results assert task.results["configuration"].splitlines() == on_disk.splitlines(), task.results From e7494fe8fd00d74e7d50d02fdf20938e03ec2569 Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 13 Aug 2026 00:35:04 -0400 Subject: [PATCH 24/26] Imnplemented test cases so that they use MagicMock, monkeypatch and parametrize --- haproxy-operator/tests/unit/test_charm.py | 87 +++++++++++------------ 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index 4f3e78d3b..a0ed828d5 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -14,9 +14,8 @@ import pytest import scenario -import charm as charm_module import tls_relation -from charm import HAProxyCharm +from charm import CharmStateValidationBaseError, HAProxyCharm from tests.unit.conftest import TEST_EXTERNAL_HOSTNAME_CONFIG from .conftest import build_haproxy_route_relation, build_spoe_auth_relation @@ -484,8 +483,8 @@ def test_returns_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: assert: the on-disk file content is returned unchanged. """ content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: content) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -499,7 +498,7 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: act: trigger the get-configuration action. assert: the action fails with a clear message rather than returning empty. """ - monkeypatch.setattr(charm_module, "file_exists", lambda _: False) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=False)) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) @@ -508,49 +507,50 @@ def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: assert "not found" in exc_info.value.message - def test_warns_when_config_is_default(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock the on-disk config to equal the rendered default config. - act: trigger the get-configuration action. - assert: a warning is logged and the configuration is still returned. - """ - default_config = "global\n maxconn 4096\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: default_config) - monkeypatch.setattr( - charm_module.HAProxyService, "render_default_config", lambda self, _: default_config - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - - context.run(context.on.action("get-configuration"), state) - - assert context.action_results == {"configuration": default_config, "source": "disk"} - assert any("default configuration" in log.lower() for log in context.action_logs) - - def test_no_warning_when_config_differs_from_default( - self, monkeypatch: pytest.MonkeyPatch + @pytest.mark.parametrize( + "on_disk_config, rendered_default, expect_default_warning", + [ + pytest.param( + "global\n maxconn 4096\n", + "global\n maxconn 4096\n", + True, + id="matches-default", + ), + pytest.param( + "frontend haproxy\n bind :80\n", + "global\n maxconn 4096\n", + False, + id="differs-from-default", + ), + ], + ) + def test_default_configuration_warning( + self, + monkeypatch: pytest.MonkeyPatch, + on_disk_config: str, + rendered_default: str, + expect_default_warning: bool, ) -> None: """ - arrange: mock the on-disk config to differ from the rendered default config. + arrange: mock the on-disk config to either match or differ from the rendered default. act: trigger the get-configuration action. - assert: no default-configuration warning is logged. + assert: the configuration is returned, and the "matches default" warning is logged + only when the config is the default. """ - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr( - charm_module, "read_file", lambda _: "frontend haproxy\n bind :80\n" - ) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=on_disk_config)) monkeypatch.setattr( - charm_module.HAProxyService, - "render_default_config", - lambda self, _: "global\n maxconn 4096\n", + "charm.HAProxyService.render_default_config", + MagicMock(return_value=rendered_default), ) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) context.run(context.on.action("get-configuration"), state) - assert not any("default configuration" in log.lower() for log in context.action_logs) + assert context.action_results == {"configuration": on_disk_config, "source": "disk"} + warned = any("default configuration" in log.lower() for log in context.action_logs) + assert warned == expect_default_warning def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.MonkeyPatch) -> None: """ @@ -560,13 +560,12 @@ def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.Monkey logged instead of silently claiming it is not the default. """ content = "frontend haproxy\n bind :80\n" - monkeypatch.setattr(charm_module, "file_exists", lambda _: True) - monkeypatch.setattr(charm_module, "read_file", lambda _: content) - - def _raise(*_args: object, **_kwargs: object) -> charm_module.CharmState: - raise charm_module.CharmStateValidationBaseError("invalid config") - - monkeypatch.setattr(charm_module.CharmState, "from_charm", _raise) + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) + monkeypatch.setattr( + "charm.CharmState.from_charm", + MagicMock(side_effect=CharmStateValidationBaseError("invalid config")), + ) context = ops.testing.Context(HAProxyCharm) state = ops.testing.State(leader=True) From 23ebdec620d5db68017df3bd3c403ccc84332861 Mon Sep 17 00:00:00 2001 From: minulo Date: Thu, 13 Aug 2026 10:17:27 -0400 Subject: [PATCH 25/26] Apply suggestions from code review Co-authored-by: Phan Trung Thanh --- haproxy-operator/src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index 8144c9742..169476491 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -738,7 +738,7 @@ def _on_get_configuration_action(self, event: ActionEvent) -> None: except CharmStateValidationBaseError: event.log( "Could not determine whether this is the default configuration because the " - "charm state is invalid; see `juju status` for the blocking condition." + "charm state is invalid." ) configuration_is_default = False if configuration_is_default: From 534a69a50665507869e40877fd9340f29c9b4bd7 Mon Sep 17 00:00:00 2001 From: minulo Date: Tue, 18 Aug 2026 16:15:31 -0400 Subject: [PATCH 26/26] refactored tests --- haproxy-operator/tests/unit/test_charm.py | 182 +++++++++++----------- 1 file changed, 92 insertions(+), 90 deletions(-) diff --git a/haproxy-operator/tests/unit/test_charm.py b/haproxy-operator/tests/unit/test_charm.py index a0ed828d5..a06fefba8 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -473,106 +473,108 @@ def test_spoe_auth_invalid_data(monkeypatch: pytest.MonkeyPatch, certificates_in @pytest.mark.usefixtures("systemd_mock", "mocks_external_calls") -class TestGetConfigurationAction: - """Test "get-configuration" Action.""" +def test_get_configuration_returns_disk_config(monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the config file on disk with known content. + act: trigger the get-configuration action. + assert: the on-disk file content is returned unchanged. + """ + content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) - def test_returns_configuration(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock the config file on disk with known content. - act: trigger the get-configuration action. - assert: the on-disk file content is returned unchanged. - """ - content = "global\n maxconn 4096\n\nfrontend default\n bind :80\n" - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) + context.run(context.on.action("get-configuration"), state) - context.run(context.on.action("get-configuration"), state) + assert context.action_results == {"configuration": content, "source": "disk"} - assert context.action_results == {"configuration": content, "source": "disk"} - def test_missing_file_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: mock the config file as absent from disk. - act: trigger the get-configuration action. - assert: the action fails with a clear message rather than returning empty. - """ - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=False)) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) +@pytest.mark.usefixtures("systemd_mock", "mocks_external_calls") +def test_get_configuration_missing_file_fails(monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: mock the config file as absent from disk. + act: trigger the get-configuration action. + assert: the action fails with a clear message rather than returning empty. + """ + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=False)) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) - with pytest.raises(ops.testing.ActionFailed) as exc_info: - context.run(context.on.action("get-configuration"), state) + with pytest.raises(ops.testing.ActionFailed) as exc_info: + context.run(context.on.action("get-configuration"), state) - assert "not found" in exc_info.value.message + assert "not found" in exc_info.value.message - @pytest.mark.parametrize( - "on_disk_config, rendered_default, expect_default_warning", - [ - pytest.param( - "global\n maxconn 4096\n", - "global\n maxconn 4096\n", - True, - id="matches-default", - ), - pytest.param( - "frontend haproxy\n bind :80\n", - "global\n maxconn 4096\n", - False, - id="differs-from-default", - ), - ], + +@pytest.mark.usefixtures("systemd_mock", "mocks_external_calls") +@pytest.mark.parametrize( + "on_disk_config, rendered_default, expect_default_warning", + [ + pytest.param( + "global\n maxconn 4096\n", + "global\n maxconn 4096\n", + True, + id="matches-default", + ), + pytest.param( + "frontend haproxy\n bind :80\n", + "global\n maxconn 4096\n", + False, + id="differs-from-default", + ), + ], +) +def test_get_configuration_default_warning( + monkeypatch: pytest.MonkeyPatch, + on_disk_config: str, + rendered_default: str, + expect_default_warning: bool, +) -> None: + """ + arrange: mock the on-disk config to either match or differ from the rendered default. + act: trigger the get-configuration action. + assert: the configuration is returned, and the "matches default" warning is logged + only when the config is the default. + """ + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=on_disk_config)) + monkeypatch.setattr( + "charm.HAProxyService.render_default_config", + MagicMock(return_value=rendered_default), ) - def test_default_configuration_warning( - self, - monkeypatch: pytest.MonkeyPatch, - on_disk_config: str, - rendered_default: str, - expect_default_warning: bool, - ) -> None: - """ - arrange: mock the on-disk config to either match or differ from the rendered default. - act: trigger the get-configuration action. - assert: the configuration is returned, and the "matches default" warning is logged - only when the config is the default. - """ - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=on_disk_config)) - monkeypatch.setattr( - "charm.HAProxyService.render_default_config", - MagicMock(return_value=rendered_default), - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration"), state) + context.run(context.on.action("get-configuration"), state) - assert context.action_results == {"configuration": on_disk_config, "source": "disk"} - warned = any("default configuration" in log.lower() for log in context.action_logs) - assert warned == expect_default_warning + assert context.action_results == {"configuration": on_disk_config, "source": "disk"} + warned = any("default configuration" in log.lower() for log in context.action_logs) + assert warned == expect_default_warning - def test_notes_when_charm_state_cannot_be_built(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ - arrange: the config file is present, but building the charm state raises. - act: trigger the get-configuration action. - assert: the configuration is still returned, and a "could not determine" note is - logged instead of silently claiming it is not the default. - """ - content = "frontend haproxy\n bind :80\n" - monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) - monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) - monkeypatch.setattr( - "charm.CharmState.from_charm", - MagicMock(side_effect=CharmStateValidationBaseError("invalid config")), - ) - context = ops.testing.Context(HAProxyCharm) - state = ops.testing.State(leader=True) - context.run(context.on.action("get-configuration"), state) +@pytest.mark.usefixtures("systemd_mock", "mocks_external_calls") +def test_get_configuration_notes_invalid_charm_state(monkeypatch: pytest.MonkeyPatch) -> None: + """ + arrange: the config file is present, but building the charm state raises. + act: trigger the get-configuration action. + assert: the configuration is still returned, and a "could not determine" note is + logged instead of silently claiming it is not the default. + """ + content = "frontend haproxy\n bind :80\n" + monkeypatch.setattr("charm.file_exists", MagicMock(return_value=True)) + monkeypatch.setattr("charm.read_file", MagicMock(return_value=content)) + monkeypatch.setattr( + "charm.CharmState.from_charm", + MagicMock(side_effect=CharmStateValidationBaseError("invalid config")), + ) + context = ops.testing.Context(HAProxyCharm) + state = ops.testing.State(leader=True) + + context.run(context.on.action("get-configuration"), state) - results = context.action_results - assert results is not None - assert results == {"configuration": content, "source": "disk"} - assert any("could not determine" in log.lower() for log in context.action_logs) - assert not any("matches the default" in log.lower() for log in context.action_logs) + results = context.action_results + assert results is not None + assert results == {"configuration": content, "source": "disk"} + assert any("could not determine" in log.lower() for log in context.action_logs) + assert not any("matches the default" in log.lower() for log in context.action_logs)