Added handling for minion return exceptions containing the word "return" #50184
Conversation
ghost
self-requested a review
salt/client/__init__.py
Outdated
| # step. Check if we have a string, log the issue and continue. | ||
| if isinstance(raw['data']['return'], six.string_types): | ||
| log.error("unexpected return from minion: %s", raw) | ||
| raw['data']['return'] = {} |
There was a problem hiding this comment.
i don't think we want to overwrite this, because we still want to print out the exceptions from the minion return so people can troubleshoot issues.
There was a problem hiding this comment.
Sounds good. I was being a bit paranoid to make sure that it was in a state which would be thrown out if it was used somewhere else that I wasn't seeing.
ethanculler
force-pushed
the
fix-minion-return-exception-with-return
branch
from
466c23e
to
a4e54d7
Compare
|
@ethanculler Any chance, you could provide a test for this? Otherwise those little details might break again in the future with code changes. |
|
@brejoc no problem. |
rallytime
added
the
Needs-Testcase
|
Thanks @ethanculler and welcome to the saltstack contributors club 🍾 |
|
bah, we probably do still need a test for this. sorry, that is on me. |
|
@gtmanfred no worries. This week has been busy, but i'll get a test in for it in another PR (will link to this one) hopefully this weekend. Cheers! |
What does this PR do?
This PR adds a check for the object type of the minion return to ensure we are dealing with a dict rather than a string containing 'return' (the key that the dict is looking for). This prevents a bad minion return from negatively impacting the master.
This bug was seen as far back as 2016.3 release
What issues does this PR fix or reference?
I didn't find any existing issues documenting this case, but there is certainly the chance I missed one.
Tests written?
No
Commits signed with GPG?
Yes