Validate default_kernel global property on set

[alimirjamali]

fixes: QubesOS/qubes-issues#8992

[alimirjamali]

Additional side notes:

I did not go to the extreme of doing it like how it is done in the QubesVM class. A simpler approach of just using the Kernel Pool is implemented. It is similar to how it is done in Qubes Manager.

[alimirjamali]

Humm. The pylint complain is very well understandable:

qubes/app.py:1598:4: C0103: Method name "on_property_pre_set_default_kernel" doesn't conform to '[a-z_][a-z0-9_]{2,30}$' pattern (invalid-name)

On the other hand, the failed test is confusing:

    return iter(self._pool.list_volumes())
                ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/gitlab-runner/builds/QubesOS/qubes-core-admin/qubes/storage/kernels.py", line 216, in list_volumes
    for kernel_version in os.listdir(self.dir_path)]
                          ^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/var/lib/qubes/vm-kernels'

p.s.: Oh. I see. TC_90_Qubes.setUp setting the default_kernel and everything else. This PR breaks all the unit tests.

It is not possible to cheat and reuse the test for qubesvm.py kernel setter. It is not implemented.

[alimirjamali]

The nice fact about the two last failed tests is that it shows that the code is actually working.

1st. test failed since /var/lib/qubes/vm-kernels directory did not exist.
2nd. test failed since /var/lib/qubes/vm-kernels/1.0 which was used by some unittest did not exist.

I guess there is no need for additional unit tests.

[alimirjamali]

Fixing few remaining unit tests.

[marmarek]

This won't work when running tests in actual dom0 (vm-kernels dir exists, but "1.0" kernel doesn't).
Some of the tests patch qubes_kernels_base_dir, see for example test_260_kernelopts in qubes/tests/vm/qubesvm.py which should be safer approach.

[alimirjamali]

Yes. Just recognized it. I will do the tests locally in order to avoid abusing the CI/CD servers and cluttering the logs. Will come back after I fix it properly

[alimirjamali]

@marmarek I finally managed to patch one of the major base classes (AdminAPITestCase). I believe that I can go forward and properly patch the rest.

I have a question regarding the CI/CD servers. Are they owned by Qubes project or they belong to Gitlab? I am trying to avoid using the online ones as much as possible and test the patches locally. But using the online ones are sometimes more convenient.

[marmarek]

I have a question regarding the CI/CD servers. Are they owned by Qubes project or they belong to Gitlab? I am trying to avoid using the online ones as much as possible and test the patches locally. But using the online ones are sometimes more convenient.

We use our own runners, you can use them as much as you like :)

[alimirjamali]

Nice. The tearDown function is purging /tmp/qubes-test-dir. So creating the /tmp/qubes-test-dir/vm-kernels/1.0/vmlinuz in the startup bash file is insufficient.

I am not going to abuse CI/CD server anymore. Reverting to local tests and will come back after I patched all unittests

[alimirjamali]

The approach of patching the existing untitests to allow default_kernel checkup is wrong. Some of them break in different scenarios. The proper approach would be mocking on_property_pre_set_default_kernel in relevant unittest classes to ignore it. Then write a single unittest for on_property_pre_set_default_kernel scenario. I will do it tomorrow.

[codecov]

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.35%. Comparing base (a1a023b) to head (151010d).
Report is 1 commits behind head on main.

Files	Patch %	Lines
qubes/app.py	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #613      +/-   ##
==========================================
+ Coverage   69.32%   69.35%   +0.02%     
==========================================
  Files          58       58              
  Lines       11953    11958       +5     
==========================================
+ Hits         8286     8293       +7     
+ Misses       3667     3665       -2     

Flag	Coverage Δ
unittests	69.35% <92.30%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alimirjamali]

Ok. We have passed CI checks and Codecov is advising on the new Unittests. I should use the helper function for QubesVM as well and add the necessary unittests. Hopefully tomorrow

[alimirjamali]

@marmarek This is ready for review