Fix metadata grain KeyError on http.query error responses

Since 3006.3 salt.utils.http.query (tornado backend) returns ``body`` on
HTTPError but does not populate ``headers``. salt.grains.metadata._search()
indexed ``linedata["headers"]`` unconditionally, so any recursive lookup that
hit a 4xx/5xx (e.g. IMDS rejecting a sub-path produced by the legacy
``=``-splitter on an EC2 user-data line) crashed the whole grain load with::

    [CRITICAL] Failed to load grains defined in grain file metadata.metadata
    ...
    KeyError: 'headers'

Guard the headers indexing the same way ``body`` is already guarded: when
``headers`` is absent, treat it as "no Content-Type information" and fall
through to the existing parsing path. Add three pytest regressions covering
the missing-headers case (both on the initial query and on recursion) and a
sanity guard for the existing ``application/octet-stream`` short-circuit.

Fixes #65184

Author: dwoz

changelog/65184.fixed.md

@@ -0,0 +1 @@
+Fixed the EC2/cloud metadata grain crashing with ``KeyError: 'headers'`` when ``salt.utils.http.query`` returns an error response (4xx/5xx with a body, e.g. when the IMDS rejects a recursive sub-path lookup). Since 3006.3 the tornado backend has populated ``body`` on HTTPError without also populating ``headers``; the grain now treats the missing ``headers`` key as "no Content-Type information" instead of letting the lookup blow up the whole grain load.

salt/grains/metadata.py

@@ -49,10 +49,12 @@ def _search(prefix="latest/"):
     if "body" not in linedata:
         return ret
     body = salt.utils.stringutils.to_unicode(linedata["body"])
-    if (
-        linedata["headers"].get("Content-Type", "text/plain")
-        == "application/octet-stream"
-    ):
+    # Since 3006.3, salt.utils.http.query (tornado backend) returns ``body``
+    # on HTTPError but does not populate ``headers``. Treat a missing
+    # ``headers`` key as "no Content-Type information" rather than letting
+    # KeyError propagate and break the whole grain load (#65184).
+    response_headers = linedata.get("headers") or {}
+    if response_headers.get("Content-Type", "text/plain") == "application/octet-stream":
         return body
     for line in body.split("\n"):
         if line.endswith("/"):

tests/pytests/unit/grains/test_metadata.py

@@ -0,0 +1,144 @@
+"""
+Unit tests for salt.grains.metadata
+
+Regression coverage for #65184: when ``salt.utils.http.query`` returns an
+error response (4xx/5xx with a body, e.g. AWS IMDS returning HTTP 400 for a
+bogus path produced by the legacy ``=``-splitter), the tornado backend
+populates ``body`` and ``status`` but does NOT set ``headers``.
+``salt.grains.metadata._search()`` previously indexed ``linedata["headers"]``
+unconditionally and crashed with ``KeyError: 'headers'``, causing the entire
+metadata grain to fail to load with::
+
+    [CRITICAL] Failed to load grains defined in grain file metadata.metadata
+    ...
+    KeyError: 'headers'
+"""
+
+import logging
+
+import pytest
+
+import salt.grains.metadata as metadata
+import salt.utils.http as http
+from tests.support.mock import create_autospec, patch
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def configure_loader_modules():
+    return {metadata: {"__opts__": {"metadata_server_grains": "True"}}}
+
+
+def _make_mock_http(responses):
+    """
+    Build a mock for salt.utils.http.query that returns canned responses
+    keyed by URL. Unknown URLs return an empty body dict so the recursion
+    terminates cleanly.
+    """
+
+    def mock_http(url="", headers=False, header_list=None, **_kwargs):
+        if url in responses:
+            return responses[url]
+        # Default: empty body, plain text. Mirrors what the metadata
+        # service does for absent leaves and lets recursion bottom out.
+        return {"body": "", "headers": {"Content-Type": "text/plain"}}
+
+    return mock_http
+
+
+def test_search_handles_error_response_without_headers_65184():
+    """
+    Regression for #65184: a recursive ``http.query`` call that returns an
+    error-shaped response (``body`` present, ``headers`` absent — the shape
+    produced by the tornado backend on HTTPError since 3006.3) must not
+    crash ``_search()`` with ``KeyError: 'headers'``.
+
+    The reporter's traceback shows the crash happens on the recursive call
+    triggered by a top-level metadata listing entry (the ``prefix == "latest/"``
+    branch), where the recursive ``_search`` then calls ``http.query`` for
+    ``latest/dynamic/`` (or similar) and gets back an error response without a
+    ``headers`` key. Before the fix the indexing ``linedata["headers"]`` raised.
+    After the fix the missing-headers case is treated like "no Content-Type
+    information" and parsing proceeds.
+    """
+    responses = {
+        "http://169.254.169.254/latest/": {
+            "body": "dynamic",
+            "headers": {"Content-Type": "text/plain"},
+        },
+        # Recursive call: error-shape response. Body + status + error, NO
+        # headers key. This is exactly what salt.utils.http.query returns on
+        # tornado HTTPError since commit 43b7fb52842 (3006.3).
+        "http://169.254.169.254/latest/dynamic/": {
+            "body": "<html><body><h1>400 Bad request</h1></body></html>\n",
+            "status": 400,
+            "error": "HTTP 400: Bad request",
+        },
+    }
+
+    with patch(
+        "salt.utils.http.query",
+        create_autospec(
+            http.query, autospec=True, side_effect=_make_mock_http(responses)
+        ),
+    ):
+        # Must not raise KeyError. Whatever it returns for the bad leaf is
+        # secondary; the contract is "do not crash the whole grain load".
+        result = metadata.metadata()
+
+    assert isinstance(result, dict)
+    assert "dynamic" in result
+
+
+def test_search_handles_missing_headers_on_initial_query_65184():
+    """
+    Companion to the above: the very first call inside ``_search()`` can also
+    produce a no-headers response (e.g. the metadata service returns 4xx for
+    the top-level listing). The function must still return a dict instead of
+    raising.
+    """
+    responses = {
+        "http://169.254.169.254/latest/": {
+            "body": "some-error-body",
+            "status": 400,
+            "error": "HTTP 400: Bad request",
+        },
+    }
+
+    with patch(
+        "salt.utils.http.query",
+        create_autospec(
+            http.query, autospec=True, side_effect=_make_mock_http(responses)
+        ),
+    ):
+        result = metadata.metadata()
+
+    # Either an empty dict or a parsed body is acceptable; the contract is
+    # "no KeyError".
+    assert isinstance(result, (dict, str))
+
+
+def test_search_octet_stream_still_returns_body_verbatim():
+    """
+    Sanity guard: the existing ``application/octet-stream`` short-circuit
+    (return body verbatim) must keep working. The fix for #65184 must not
+    regress that path.
+    """
+    responses = {
+        "http://169.254.169.254/latest/": {
+            "body": "raw-octet-stream-payload",
+            "headers": {"Content-Type": "application/octet-stream"},
+        },
+    }
+
+    with patch(
+        "salt.utils.http.query",
+        create_autospec(
+            http.query, autospec=True, side_effect=_make_mock_http(responses)
+        ),
+    ):
+        result = metadata.metadata()
+
+    # Body returned verbatim, not wrapped in a dict.
+    assert result == "raw-octet-stream-payload"