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