Fix metadata grain KeyError on http.query error responses (#65184)#69471
Open
dwoz wants to merge 1 commit into
Open
Fix metadata grain KeyError on http.query error responses (#65184)#69471dwoz wants to merge 1 commit into
dwoz wants to merge 1 commit into
Conversation
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 saltstack#65184
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Guards
salt.grains.metadata._search()againstKeyError: 'headers'whensalt.utils.http.query()returns an error-shaped response (4xx/5xx with abody but no
headerskey — the shape the tornado backend has produced onHTTPError since 3006.3). The grain now treats the missing
headerskey as"no Content-Type information" and falls through to the existing parsing path
instead of letting the whole grain load fail.
What issues does this PR fix or reference?
Fixes #65184
Related: #62061 (open against 3006.x) fixes the contributing user-data
=-splitter bug that triggers the bogus IMDS sub-path lookup in thereporter's environment. The two fixes are complementary, not competing: even
with #62061 merged, any other 4xx/5xx response from IMDS (bad token, sporadic
server error, etc.) would still produce the same
KeyError. Merging bothgives users a complete fix.
Previous Behavior
With
metadata_server_grains: True, the grain crashed on load with:Root cause: between 3006.2 and 3006.3, commit
43b7fb52842("Return body whenerror with tornado backend salt.utils.http") made
http.querypopulatebodyon HTTPError without also populating
headers. The metadata grain indexedlinedata["headers"]unconditionally and crashed on any recursive sub-pathlookup that the IMDS rejected.
New Behavior
_search()returns a dict on the same recursive path; the missing-headersleaf is parsed without Content-Type info and recursion continues. The whole
grain no longer fails to load.
Merge requirements satisfied?
changelog/65184.fixed.md)tests/pytests/unit/grains/test_metadata.py,three regression cases: missing-headers on recursion, missing-headers on
the initial query, and a sanity guard for the existing
application/octet-streamshort-circuit)Commits signed with GPG?
No (matches base-branch policy on 3006.x).