fix: Make _should_wait_via_user_data() handle non-dict data #5976
Conversation
The function "_should_wait_via_user_data()" needs to be able to handle non-dict data (i.e., #cloud-config-archive). Fixes canonicalGH-5975
| @@ -348,6 +348,9 @@ def _should_wait_via_user_data( | |||
| ) | |||
| return True, "failed to parse user data as yaml" | |||
|
|
|||
| if not isinstance(parsed_yaml, dict): | |||
There was a problem hiding this comment.
This works, but I think that this works around an existing bug rather than fixing it.
handlers.type_from_starts_with() only returned "text/cloud-config" because of a bug in the slicing on line 334. I'm guessing that slicing by the length of the longest value in INCLUSION_TYPES_MAP should do it.
As it is, either of the following types will incorrectly return text/cloud-config:
#cloud-config-archive
#cloud-config-jsonp
If I revert your change to main.py and then update the slice from 13 to 42, the test passes.
Aside: this logic feels out of place in main.py
There was a problem hiding this comment.
As an alternative, you could also do .split(maxsplit=1)[0] to just get the first word
There was a problem hiding this comment.
@kageurufu , thanks for the suggestion, but the goal of the slice was to avoid parsing processing the entire blob.
There was a problem hiding this comment.
the goal of the slice was to avoid parsing processing the entire blob.
@kageurufu's suggestion would accomplish that: split on the first whitespace and return only the first value
There was a problem hiding this comment.
@holmanb I was more thinking about non-text binary data like some big gzipped blob.
|
@holmanb , I applied your suggestion but also kept my original change as I believe that it is also necessary. |
When would it be necessary? This wouldn't be valid cloud-config, so cloud-init just spits warnings and doesn't configure the system at all - what is there to wait for? Also, I think that @kageurufu's suggestion with |
Spits tracebacks which we shouldn't allow intentionally. |
I was referring to the warnings due to it being an invalid configuration, but yes I see now that this would cause a traceback - makes sense to me. |
…anonical#5976) The function "_should_wait_via_user_data()" wasn't properly handling user data that has a header that starts with #cloud-config, but isn't cloud-config, like #cloud-config-archive Fixes canonicalGH-5975
Proposed Commit Message
Additional Context
Test Steps
Merge type