Don't crash if dmesg contains non-utf8 characters

[asomers]

On FreeBSD Salt scrapes /var/run/dmesg.boot to set the "cpu_flags" grain. But it's possible for that file to contain non-UTF-8 characters. Skipping over such characters is better than crashing.

Signed-off-by: Alan Somers [email protected]

What does this PR do?

Fixes a crash on startup on FreeBSD systems if non-UTF8 data is present in /var/run/dmesg.boot . That can happen, for example, if a connected SAS device has a serial number containing non-UTF8 characters.

Previous Behavior

On such systems, Salt would crash on startup with an error like this:

2022-06-30 13:54:03,038 [salt.log.setup   :911 ][ERROR   ][25928] An un-handled exception was caught by salt's global exception handler:
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 470: invalid start byte
Traceback (most recent call last):
  File "/usr/local/bin/salt-call", line 33, in <module>
    sys.exit(load_entry_point('salt==3004', 'console_scripts', 'salt-call')())
  File "/usr/local/lib/python3.8/site-packages/salt/scripts.py", line 432, in salt_call
    client.run()
  File "/usr/local/lib/python3.8/site-packages/salt/cli/call.py", line 45, in run
    caller = salt.cli.caller.Caller.factory(self.config)
  File "/usr/local/lib/python3.8/site-packages/salt/cli/caller.py", line 55, in factory
    return ZeroMQCaller(opts, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/salt/cli/caller.py", line 319, in _init_
    super()._init_(opts)
  File "/usr/local/lib/python3.8/site-packages/salt/cli/caller.py", line 79, in _init_
    self.minion = salt.minion.SMinion(opts)
  File "/usr/local/lib/python3.8/site-packages/salt/minion.py", line 926, in _init_
    opts["grains"] = salt.loader.grains(opts)
  File "/usr/local/lib/python3.8/site-packages/salt/loader/_init_.py", line 909, in grains
    ret = funcs[key]()
  File "/usr/local/lib/python3.8/site-packages/salt/loader/lazy.py", line 149, in _call_
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/salt/loader/lazy.py", line 1201, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/salt/loader/lazy.py", line 1216, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/salt/grains/core.py", line 2251, in os_data
    grains.update(_bsd_cpudata(grains))
  File "/usr/local/lib/python3.8/site-packages/salt/grains/core.py", line 412, in _bsd_cpudata
    for line in _fp:
  File "/usr/local/lib/python3.8/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 470: invalid start byte

New Behavior

Salt will skip over the illegal bytes in /var/run/dmesg.boot .

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• [ ] Docs
• [x] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• [ ] Tests written/updated

Commits signed with GPG?

Yes

[asomers]

The "Pre-Commit / Run Pre-Commit Against Salt" failure looks unrelated. It failed while building docs with this error: unknown document: /topics/releases/2019

[asomers]

I think this warrants a test case.

I would've written one if were easy to do. But there isn't currently any test coverage for that function, _bsd_cpudata. It's a complicated function to test because of its many different inputs, and the cpu_flags grain isn't very useful IMHO, so I think it's not worth my effort to test it. All that matters to me is that it doesn't crash.

[asomers]

@dwoz can we please merge this without a test case? It's a crash bug, and there are no existing tests for that module.

[whytewolf]

the current pre-commit failure is black running and changing things because of formatting. so that does need to be covered. and yes this is going to need a test case.

If there are no test cases already then at least get this covered by a test and we can come back latter and add into your test case the other stuff.

[asomers]

Then how about just rip that function out? It isn't any use, IMHO. But it's causing Salt to crash.

[whytewolf]

Then how about just rip that function out? It isn't any use, IMHO. But it's causing Salt to crash.

so, even if we were to merge this today it will not go in until the 3006 release. so there is no rush even if this is crashing. if it is crashing you can make the changes locally to keep the crash from happening.

as for no need. this is used for cpu_flags grains. just because you don't target minion based on the cpu flags doesn't mean someone out there doesn't. ripping out means under freebsd we would not have these grains.

next the tests for this should go in https://github.com/saltstack/salt/blob/master/tests/pytests/unit/grains/test_core.py you don't have to test every operating system. there are plenty of tests in there to use as examples.