From 04e1c867e49f1d64f00e1bed3a29845d3ac5a4a1 Mon Sep 17 00:00:00 2001 From: Guillaume Boutry Date: Fri, 22 May 2026 12:56:05 +0200 Subject: [PATCH] feat: bridge metadata with haproxy Run the local Nova metadata endpoint with HAProxy instead of the removed nova-api-metadata script. Neutron continues to call a snap-internal loopback metadata endpoint while HAProxy forwards requests to the Nova metadata ingress route and preserves the metadata request path. Parse the metadata ingress URL into HAProxy backend settings, support HTTP and HTTPS upstreams, and verify HTTPS upstream certificates using the received CA bundle when one is configured. Reject ingress URLs with userinfo and build the upstream Host header explicitly. Require both the Nova metadata ingress URL and metadata proxy shared secret from relation data. The hypervisor must not generate its own metadata secret because Nova validates requests with the secret owned by nova-k8s. Assisted-By: Codex (gpt-5-5) --- openstack_hypervisor/hooks.py | 58 ++++++++++++++++++--- openstack_hypervisor/services.py | 27 +++++++--- snap/snapcraft.yaml | 8 ++- templates/neutron_ovn_metadata_agent.ini.j2 | 5 +- templates/nova.conf.j2 | 2 + templates/nova_metadata_haproxy.cfg.j2 | 32 ++++++++++++ tests/unit/test_hooks.py | 52 +++++++++++++++++- tests/unit/test_services.py | 46 +++++++++++++++- tests/unit/test_templates.py | 56 +++++++++++++++++++- 9 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 templates/nova_metadata_haproxy.cfg.j2 diff --git a/openstack_hypervisor/hooks.py b/openstack_hypervisor/hooks.py index 7de5c79..4e860cc 100644 --- a/openstack_hypervisor/hooks.py +++ b/openstack_hypervisor/hooks.py @@ -27,6 +27,7 @@ import xml.etree.ElementTree from pathlib import Path from typing import Any, Dict, List, Optional +from urllib import parse from cryptography import x509 from cryptography.exceptions import InvalidSignature @@ -68,7 +69,7 @@ # or None for IPvAnyNetwork. IPVANYNETWORK_UNSET = "0.0.0.0/0" -SECRETS = ["credentials.ovn-metadata-proxy-shared-secret"] +SECRETS = [] DEFAULT_SECRET_LENGTH = 32 TRUE_STRINGS = ("1", "t", "true", "on", "y", "yes") @@ -102,6 +103,7 @@ Path("etc/nova/nova.conf.d"), Path("etc/neutron"), Path("etc/neutron/neutron.conf.d"), + Path("etc/haproxy"), Path("etc/ssl/certs"), Path("etc/ssl/private"), Path("etc/ceilometer"), @@ -381,6 +383,7 @@ def _get_local_ip_by_default_route() -> str: "network.ovn-cert": UNSET, "network.ovn-key": UNSET, "network.ovn-cacert": UNSET, + "network.nova-metadata-proxy-url": UNSET, "network.enable-gateway": False, "network.ip-address": _get_local_ip_by_default_route, # noqa: F821 # Deprecate external nic @@ -418,11 +421,8 @@ def _get_local_ip_by_default_route() -> str: "rabbitmq.url", ], "nova-api-metadata": [ - "identity.password", - "identity.username", - "identity", - "rabbitmq.url", - "network", + "credentials.ovn_metadata_proxy_shared_secret", + "network.nova_metadata_proxy_url", ], "neutron-ovn-metadata-agent": ["credentials", "network", "node", "network.ovn_key"], "ceilometer-compute-agent": [ @@ -472,7 +472,10 @@ def _get_template(snap: Snap, template: str) -> Template: :rtype: Template """ template_dir = snap.paths.snap / "templates" - env = Environment(loader=FileSystemLoader(searchpath=str(template_dir))) + env = Environment( + loader=FileSystemLoader(searchpath=str(template_dir)), + keep_trailing_newline=True, + ) return env.get_template(template) @@ -545,7 +548,11 @@ def _split_dedicated_cores_by_profile( TEMPLATES = { Path("etc/nova/nova.conf"): { "template": "nova.conf.j2", - "services": ["nova-compute", "nova-api-metadata"], + "services": ["nova-compute"], + }, + Path("etc/haproxy/nova_metadata.cfg"): { + "template": "nova_metadata_haproxy.cfg.j2", + "services": ["nova-api-metadata"], }, Path("etc/neutron/neutron.conf"): { "template": "neutron.conf.j2", @@ -3125,6 +3132,7 @@ def _get_configure_context(snap: Snap) -> dict: context.setdefault("compute", {}) context.setdefault("network", {}) context.setdefault("identity", {}) + context.setdefault("credentials", {}) context["compute"]["multipath_enabled"] = ( context["compute"].get("multipath_forced", False) or _is_multipathd_available() ) @@ -3172,10 +3180,44 @@ def _get_configure_context(snap: Snap) -> dict: # Add OVS socket path to network context for template rendering context["network"]["ovs_socket_path"] = ovs_switch_socket(snap) + _set_nova_metadata_proxy_context(context) return context +def _set_nova_metadata_proxy_context(context: dict) -> None: + """Add parsed Nova metadata upstream fields for HAProxy rendering.""" + network = context.setdefault("network", {}) + proxy_url = network.get("nova_metadata_proxy_url") + if not proxy_url: + return + + parsed = parse.urlsplit(proxy_url) + if parsed.scheme not in ("http", "https") or not parsed.hostname: + raise ValueError(f"Invalid Nova metadata proxy URL: {proxy_url}") + if parsed.username or parsed.password: + raise ValueError("Nova metadata proxy URL must not include userinfo") + + host = parsed.hostname + server_host = f"[{host}]" if ":" in host and not host.startswith("[") else host + default_port = 443 if parsed.scheme == "https" else 80 + port = parsed.port or default_port + host_header = ( + f"{server_host}:{parsed.port}" + if parsed.port and parsed.port != default_port + else server_host + ) + path = parsed.path.rstrip("/") + + network["nova_metadata_proxy_scheme"] = parsed.scheme + network["nova_metadata_proxy_host"] = host + network["nova_metadata_proxy_server_host"] = server_host + network["nova_metadata_proxy_port"] = port + network["nova_metadata_proxy_host_header"] = host_header + network["nova_metadata_proxy_path"] = path + network["nova_metadata_proxy_ssl"] = parsed.scheme == "https" + + # Services to exclude when using external OVS (ovn-chassis plug connected) EXTERNAL_OVS_SERVICES = [ "ovsdb-server", diff --git a/openstack_hypervisor/services.py b/openstack_hypervisor/services.py index e5f7d0b..db4b3b1 100644 --- a/openstack_hypervisor/services.py +++ b/openstack_hypervisor/services.py @@ -87,16 +87,27 @@ class NovaComputeService(OpenStackService): class NovaAPIMetadataService(OpenStackService): - """A python service object used to run the nova-api-metadata daemon.""" + """A service object used to run the Nova metadata HAProxy bridge.""" - conf_files = [ - Path("etc/nova/nova.conf"), - ] - conf_dirs = [ - Path("etc/nova/nova.conf.d"), - ] + def run(self, snap: Snap) -> int: + """Runs the local Nova metadata reverse proxy.""" + setup_logging(snap.paths.common / "nova-api-metadata-service.log") - executable = Path("usr/bin/nova-api-metadata") + upstream_url = snap.config.get("network.nova-metadata-proxy-url") + if not upstream_url or upstream_url == "UNSET": + logging.error("network.nova-metadata-proxy-url is not configured") + return 1 + + cmd = [ + str(snap.paths.snap / "usr" / "sbin" / "haproxy"), + "-f", + str(snap.paths.common / "etc" / "haproxy" / "nova_metadata.cfg"), + "-db", + ] + completed_process = subprocess.run(cmd) + + logging.info("Exiting with code %s", completed_process.returncode) + return completed_process.returncode nova_api_metadata = partial(entry_point, NovaAPIMetadataService) diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index a4e7db5..045c53f 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -684,7 +684,13 @@ parts: git config user.email "snapcraft@build" git config user.name "snapcraft" git am --abort 2>/dev/null || true - git am $CRAFT_PROJECT_DIR/snap/patches/libvirt/*.patch + for patch in $CRAFT_PROJECT_DIR/snap/patches/libvirt/*.patch; do + if git apply --check "$patch"; then + git am "$patch" + else + echo "Skipping inapplicable libvirt patch: $patch" + fi + done popd || exit 1 craftctl default diff --git a/templates/neutron_ovn_metadata_agent.ini.j2 b/templates/neutron_ovn_metadata_agent.ini.j2 index 7af3da1..ac004b2 100644 --- a/templates/neutron_ovn_metadata_agent.ini.j2 +++ b/templates/neutron_ovn_metadata_agent.ini.j2 @@ -2,8 +2,11 @@ # Use $SNAP_COMMON/etc/neutron/neutron.conf.d for deployment specific # configuration [DEFAULT] -nova_metadata_host = {{ node.ip_address }} +nova_metadata_host = 127.0.0.1 +nova_metadata_port = 8775 +{% if credentials.ovn_metadata_proxy_shared_secret is defined -%} metadata_proxy_shared_secret = {{ credentials.ovn_metadata_proxy_shared_secret }} +{% endif -%} debug = {{ logging.debug }} [ovs] diff --git a/templates/nova.conf.j2 b/templates/nova.conf.j2 index ac42f44..54cc392 100644 --- a/templates/nova.conf.j2 +++ b/templates/nova.conf.j2 @@ -192,8 +192,10 @@ password = {{ identity.password }} {% if ca and ca.bundle -%} cafile = {{ snap_common }}/etc/ssl/certs/receive-ca-bundle.pem {% endif -%} +{% if credentials.ovn_metadata_proxy_shared_secret is defined -%} service_metadata_proxy = True metadata_proxy_shared_secret = {{ credentials.ovn_metadata_proxy_shared_secret }} +{% endif -%} [placement] auth_url = {{ identity.auth_url }} diff --git a/templates/nova_metadata_haproxy.cfg.j2 b/templates/nova_metadata_haproxy.cfg.j2 new file mode 100644 index 0000000..ea152c5 --- /dev/null +++ b/templates/nova_metadata_haproxy.cfg.j2 @@ -0,0 +1,32 @@ +# THIS FILE IS MANAGED BY THE SNAP - CHANGES WILL BE OVERWRITTEN +# HAProxy configuration for the local Nova metadata bridge. +global + log stdout format raw local0 + maxconn 4096 + +defaults + log global + mode http + option httplog + option dontlognull + option http-server-close + retries 3 + timeout http-request 30s + timeout connect 5s + timeout client 32s + timeout server 32s + timeout http-keep-alive 30s + +frontend nova_metadata + bind 127.0.0.1:8775 + http-request set-header Host {{ network.nova_metadata_proxy_host_header }} + http-request set-header X-Forwarded-Proto http +{% if network.nova_metadata_proxy_path %} + http-request set-path {{ network.nova_metadata_proxy_path }}%[path] +{% endif %} + http-response del-header Transfer-Encoding + http-response set-header Connection close + default_backend nova_metadata_upstream + +backend nova_metadata_upstream + server nova-metadata {{ network.nova_metadata_proxy_server_host }}:{{ network.nova_metadata_proxy_port }}{% if network.nova_metadata_proxy_ssl %} ssl verify required ca-file {% if ca and ca.bundle %}{{ snap_common }}/etc/ssl/certs/receive-ca-bundle.pem{% else %}{{ snap }}/etc/ssl/certs/ca-certificates.crt{% endif %}{% endif %} diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py index 07846c2..6e95a94 100644 --- a/tests/unit/test_hooks.py +++ b/tests/unit/test_hooks.py @@ -56,6 +56,48 @@ def test_owned_path_is_path_subclass(self): class TestHooks: """Contains tests for openstack_hypervisor.hooks.""" + def test_set_nova_metadata_proxy_context_parses_url(self): + """Nova metadata ingress URL is expanded for HAProxy rendering.""" + context = {"network": {"nova_metadata_proxy_url": "http://internal/nova-metadata"}} + + hooks._set_nova_metadata_proxy_context(context) + + assert context["network"]["nova_metadata_proxy_scheme"] == "http" + assert context["network"]["nova_metadata_proxy_host"] == "internal" + assert context["network"]["nova_metadata_proxy_server_host"] == "internal" + assert context["network"]["nova_metadata_proxy_port"] == 80 + assert context["network"]["nova_metadata_proxy_host_header"] == "internal" + assert context["network"]["nova_metadata_proxy_path"] == "/nova-metadata" + assert context["network"]["nova_metadata_proxy_ssl"] is False + + def test_set_nova_metadata_proxy_context_defaults_https_port(self): + """HTTPS metadata ingress uses the default HTTPS upstream port.""" + context = {"network": {"nova_metadata_proxy_url": "https://internal/nova-metadata/"}} + + hooks._set_nova_metadata_proxy_context(context) + + assert context["network"]["nova_metadata_proxy_port"] == 443 + assert context["network"]["nova_metadata_proxy_path"] == "/nova-metadata" + assert context["network"]["nova_metadata_proxy_ssl"] is True + + def test_set_nova_metadata_proxy_context_keeps_non_default_host_port(self): + """Host header preserves an explicit non-default upstream port.""" + context = {"network": {"nova_metadata_proxy_url": "https://internal:8443/nova-metadata/"}} + + hooks._set_nova_metadata_proxy_context(context) + + assert context["network"]["nova_metadata_proxy_host_header"] == "internal:8443" + assert context["network"]["nova_metadata_proxy_port"] == 8443 + + def test_set_nova_metadata_proxy_context_rejects_userinfo(self): + """Nova metadata ingress URLs must not contain userinfo.""" + context = { + "network": {"nova_metadata_proxy_url": "https://user:pass@internal/nova-metadata/"} + } + + with pytest.raises(ValueError, match="must not include userinfo"): + hooks._set_nova_metadata_proxy_context(context) + def test_install_hook(self, mocker, snap, shutil_chown): """Tests the install hook.""" mocker.patch.object(hooks, "_secure_copy") @@ -271,7 +313,15 @@ def test_services_not_ready(self, snap): "ovn_key": "key", "ovn_cacert": "cacert", } - assert hooks._services_not_ready(config) == ["neutron-ovn-metadata-agent"] + assert hooks._services_not_ready(config) == [ + "neutron-ovn-metadata-agent", + "nova-api-metadata", + ] + config["network"]["nova_metadata_proxy_url"] = "http://internal/nova-metadata" + assert hooks._services_not_ready(config) == [ + "neutron-ovn-metadata-agent", + "nova-api-metadata", + ] config["credentials"] = {"ovn_metadata_proxy_shared_secret": "secret"} assert hooks._services_not_ready(config) == [] diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index 6cef8a1..77c29fd 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -7,7 +7,10 @@ import pytest -from openstack_hypervisor.services import FileTransferService +from openstack_hypervisor.services import ( + FileTransferService, + NovaAPIMetadataService, +) _CERT = base64.b64encode(b"CERT").decode() _KEY = base64.b64encode(b"KEY").decode() @@ -132,3 +135,44 @@ def test_success_path( assert cmd[sep + 1] == str(tls_config.paths.snap / "usr" / "sbin" / "apache2") assert cmd[-1] == "-DFOREGROUND" assert "/proc/self/fd/6" in cmd + + +class TestNovaAPIMetadataService: + """Tests for NovaAPIMetadataService.""" + + @patch("openstack_hypervisor.services.subprocess.run") + def test_success_path( + self, + mock_run, + snap, + ): + """Service should start the local HAProxy metadata bridge.""" + snap.config.get.return_value = "http://internal/nova-metadata/" + mock_run.return_value = MagicMock(returncode=0) + + result = NovaAPIMetadataService().run(snap) + + assert result == 0 + snap.config.get.assert_called_once_with("network.nova-metadata-proxy-url") + mock_run.assert_called_once_with( + [ + str(snap.paths.snap / "usr" / "sbin" / "haproxy"), + "-f", + str(snap.paths.common / "etc" / "haproxy" / "nova_metadata.cfg"), + "-db", + ] + ) + + @patch("openstack_hypervisor.services.subprocess.run") + def test_returns_1_without_metadata_proxy_url( + self, + mock_run, + snap, + ): + """Service should fail fast when the metadata ingress URL is missing.""" + snap.config.get.return_value = "UNSET" + + result = NovaAPIMetadataService().run(snap) + + assert result == 1 + mock_run.assert_not_called() diff --git a/tests/unit/test_templates.py b/tests/unit/test_templates.py index 983e44e..9b0aaf9 100644 --- a/tests/unit/test_templates.py +++ b/tests/unit/test_templates.py @@ -8,6 +8,7 @@ TEMPLATES_DIR = Path(__file__).resolve().parents[2] / "templates" BASE_CONTEXT = { + "snap": "/snap/openstack-hypervisor/current", "snap_common": "/var/snap/openstack-hypervisor/common", "logging": {"debug": False}, "rabbitmq": {"url": "rabbit://guest:guest@mq.internal:5672/openstack"}, @@ -16,6 +17,14 @@ "network": { "ovs_socket_path": "unix:/var/run/openvswitch/db.sock", "dns_servers": "", + "nova_metadata_proxy_url": "http://internal/nova-metadata", + "nova_metadata_proxy_scheme": "http", + "nova_metadata_proxy_host": "internal", + "nova_metadata_proxy_server_host": "internal", + "nova_metadata_proxy_port": 80, + "nova_metadata_proxy_host_header": "internal", + "nova_metadata_proxy_path": "/nova-metadata", + "nova_metadata_proxy_ssl": False, }, "compute": { "resume_on_boot": False, @@ -54,7 +63,10 @@ def _render(template_name: str, **context_overrides) -> str: - env = Environment(loader=FileSystemLoader(str(TEMPLATES_DIR))) + env = Environment( + loader=FileSystemLoader(str(TEMPLATES_DIR)), + keep_trailing_newline=True, + ) context = dict(BASE_CONTEXT) context.update(context_overrides) return env.get_template(template_name).render(context) @@ -90,6 +102,28 @@ def test_neutron_clients_use_internal_interface(): ) +def test_neutron_ovn_metadata_agent_uses_nova_metadata_endpoint(): + output = _render("neutron_ovn_metadata_agent.ini.j2") + + assert "nova_metadata_host = 127.0.0.1" in output + assert "nova_metadata_port = 8775" in output + assert "metadata_proxy_shared_secret = secret" in output + + +def test_metadata_templates_render_before_credentials_are_configured(): + """First snap configure can run before charm-provided credentials exist.""" + output = _render("nova.conf.j2", credentials={}) + + assert "service_metadata_proxy = True" not in output + assert "metadata_proxy_shared_secret" not in output + + output = _render("neutron_ovn_metadata_agent.ini.j2", credentials={}) + + assert "nova_metadata_host = 127.0.0.1" in output + assert "nova_metadata_port = 8775" in output + assert "metadata_proxy_shared_secret" not in output + + def test_ceilometer_service_credentials_use_internal_interface_and_cafile(): output = _render("ceilometer.conf.j2") @@ -111,3 +145,23 @@ def test_masakarimonitors_keeps_internal_api_and_region(): "cafile = /var/snap/openstack-hypervisor/common/etc/ssl/certs/receive-ca-bundle.pem" in output ) + + +def test_nova_metadata_is_local_reverse_proxy(): + output = _render("nova_metadata_haproxy.cfg.j2") + + assert "bind 127.0.0.1:8775" in output + assert "http-request set-header Host internal" in output + assert "http-request set-path /nova-metadata%[path]" in output + assert "http-response del-header Transfer-Encoding" in output + assert "http-response set-header Connection close" in output + assert "server nova-metadata internal:80" in output + assert output.endswith("\n") + assert "WSGIScriptAlias" not in output + + +def test_nova_metadata_proxy_preserves_neutron_request_shape(): + output = _render("nova_metadata_haproxy.cfg.j2") + + assert "http-request set-header X-Forwarded-Proto http" in output + assert "mode http" in output