cephadm: Avoid crash on AppArmor profile names with space

[smarsching]

Fixes: http://tracker.ceph.com/issues/66073

cephadm shell (and possibly other sub-commands) would crash when
there was an AppArmor profile that contained a space character in its
name. This happened due to the code parsing
/sys/kernel/security/apparmor/profiles only expecting a single space
character per line.

This commit fixes this issue by splitting the line on the last space
character instead of splitting it on each one.

Contribution Guidelines

• To sign and title your commits, please refer to Submitting Patches to Ceph.

• If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

• When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

• Tracker (select at least one)
  • References tracker ticket
  • Very recent bug; references commit where it was introduced
  • New feature (ticket optional)
  • Doc update (no ticket needed)
  • Code cleanup (no ticket needed)
• Component impact
  • Affects Dashboard, opened tracker ticket
  • Affects Orchestrator, opened tracker ticket
  • No impact that needs to be tracked
• Documentation (select at least one)
  • Updates relevant documentation
  • No doc update is appropriate
• Tests (select at least one)
  • Includes unit test(s)
  • Includes integration test(s)
  • Includes bug reproducer
  • No tests

Show available Jenkins commands

• jenkins retest this please
• jenkins test classic perf
• jenkins test crimson perf
• jenkins test signed
• jenkins test make check
• jenkins test make check arm64
• jenkins test submodules
• jenkins test dashboard
• jenkins test dashboard cephadm
• jenkins test api
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox
• jenkins test windows
• jenkins test rook e2e

[phlogistonjohn]

Change looks OK but ideally we'd have some sort of test to avoid regressions, probably a unit test. Unfortunately, the host_facts stuff is a bit monolithic so I don't know how difficult it would be. Have you looked into adding a unit test? If not, could I bother you to do so but spend no more than a week at most on it. If you respond that you looked into it and it was too hard to write I'd happily approve anyway. I just don't want to miss the opporunity to have a test if it's not a big burden. Thanks!

[smarsching]

@phlogistonjohn I considered writing a unit test, but due to there being no unit tests for HostFacts at all, this would be a major task:

Testing _fetch_apparmor on its own would be reasonably simple: I would just have to mock-patch os.path.exists and read_file. However, there is no way to test _fetch_apparmor directly, because it is defined inside the kernel_security method, so the lowest level which the unit test could target is that method.

This is already a lot harder, because read_file and now has to be patched in way so that it returns different results for different arguments (as it is used multiple times in the functions called by kernel_security). In addition to that call would have to be mocked in order to make the _fetch_selinux function work.

But the work does not stop there: As read_file is a method, we need an instance of HostFacts to test it, so we have to mock-patch even more code in order to make the constructor work correctly regardless of the environment.

It certainly is possible to do this, but quite frankly I do not have the time to do all that work.

The best thing that I can offer is making _fetch_apparmor a module-level function, so that it can be tested on its own. Do you think that this makes sense?

I discarded this idea because I wanted to keep the change for the bugfix minimal, in particular considering that I think it should be backported to Reef, and for a simple one-line fix this is much more reasonable than for a refactoring of the HostFacts logic.

[smarsching]

A quick update: In the meantime, I found that this problem also affects Ubuntu 24.04 Server, not just the Desktop version.

And unfortunately, it cannot simply be fixed by replacing the cephadm executable in the host system with a patched version, because cephadm is also used internally, and in this case the version from the Docker image is used.

For example, this happens when running ceph health detail:

[WRN] CEPHADM_REFRESH_FAILED: failed to probe daemons or devices
    host XXXXXXXX `cephadm gather-facts` failed: cephadm exited with an error code: 1, stderr: Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 10700, in <module>
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 10688, in main
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 9772, in command_gather_facts
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 9762, in dump
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 9677, in kernel_security
  File "/var/lib/ceph/ea2b2af0-1879-11ef-a6b1-0cc47a841bc8/cephadm.2b9d7d139a9cb40289f2358faf49a109fc297c0a258bde893227c262c30bca8d/__main__.py", line 9658, in _fetch_apparmor
ValueError: too many values to unpack (expected 2)