diff --git a/docs/changelog.md b/docs/changelog.md index 58dfbf36..9d81c340 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-07-06 - docs: Add a how-to guide for configuring the backend protocol. diff --git a/docs/release-notes/artifacts/pr0609.yaml b/docs/release-notes/artifacts/pr0609.yaml new file mode 100644 index 00000000..781c96ad --- /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` 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 + related_issue: + visibility: public + highlight: false diff --git a/haproxy-operator/charmcraft.yaml b/haproxy-operator/charmcraft.yaml index 60869ac0..e461e56e 100644 --- a/haproxy-operator/charmcraft.yaml +++ b/haproxy-operator/charmcraft.yaml @@ -146,6 +146,9 @@ 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 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 01d24f6b..16947649 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( @@ -717,6 +718,63 @@ 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: + """Return the on-disk haproxy configuration for debugging. + + 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 + """ + if not file_exists(HAPROXY_CONFIG): + event.fail(f"HAProxy configuration file at {HAPROXY_CONFIG} not found. ") + return + configuration = read_file(HAPROXY_CONFIG) + + 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." + ) + 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." + ) + + event.set_results({"configuration": configuration, "source": "disk"}) + + def _configuration_is_default(self, configuration: str) -> bool: + """Return whether the given configuration matches the default configuration. + + 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. + """ + 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, + ) + ) + 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 1d92f0ea..a3949d96 100644 --- a/haproxy-operator/src/haproxy.py +++ b/haproxy-operator/src/haproxy.py @@ -218,14 +218,42 @@ 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 as a string. + + 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. + + 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 +271,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 +292,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 27f385aa..396bd8c7 100644 --- a/haproxy-operator/tests/integration/test_actions.py +++ b/haproxy-operator/tests/integration/test_actions.py @@ -11,14 +11,14 @@ @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 @@ -48,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", @@ -57,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", @@ -79,6 +78,12 @@ def test_get_proxied_endpoints_action( ) assert task.results == {"endpoints": "[]"}, task.results + # get-configuration returns exactly the configuration currently on disk. + on_disk = juju.ssh(f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg") + 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 + 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 4cada97e..a06fefba 100644 --- a/haproxy-operator/tests/unit/test_charm.py +++ b/haproxy-operator/tests/unit/test_charm.py @@ -15,7 +15,7 @@ import scenario 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 @@ -470,3 +470,111 @@ 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") +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) + + context.run(context.on.action("get-configuration"), state) + + assert context.action_results == {"configuration": content, "source": "disk"} + + +@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) + + assert "not found" in exc_info.value.message + + +@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), + ) + 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 + + +@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)