Don't translate dom0 to @adminvm#204
Conversation
|
As can been seen by the updates needed to the test cases, this change can have some side effects. I did some basic testing on a running system, but it definitively needs a full openqa run to make sure it doesn't break an assumption about dom0 vs. @adminvm elsewhere. For that reason I probably would not backport this, even though I think being unable to specify a |
94abb91 to
d9044ee
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
==========================================
+ Coverage 78.80% 78.96% +0.16%
==========================================
Files 55 55
Lines 10458 10502 +44
==========================================
+ Hits 8241 8293 +52
+ Misses 2217 2209 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Indeed there are problematic cases. With sys-gui, clipboard paste doesn't work: The policy is here: https://github.com/QubesOS/qubes-mgmt-salt-dom0-virtual-machines/blob/main/qvm/template-gui.jinja#L15 and it has |
And the VM requests |
|
I must admit it's a weird case. But it worked before, and we have no idea where is is that used (in various user scripts etc). So, I guess it needs to remain working. |
I assumed so, but thought worth asking given that it's not fully clear if this should still be the case, now that we kind of change
We need to be careful when such a change changes the meaning of a previously valid policies. But in that case that's probably ok, given that the remote |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025051904-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025031804-4.3&flavor=update
Failed tests11 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/132953#dependencies 14 fixed
Unstable testsDetails
Performance TestsPerformance degradation:13 performance degradations
Remaining performance tests:43 tests
|
d9044ee to
dedf332
Compare
The previous behavior of handling "dom0" and "@adminvm" in policies was inconsistent and let to the problem that you could not specify target=dom0 default_target=dom0 (or both @adminvm) since one was translated to @adminvm and the other was alsways expanded to dom0. Based on the discussion in QubesOS/qubes-issues#9870 we now stop to translate dom0 (or it's UUID) to @adminvm. The later works as keyword that is expanded to dom0. Note this also uncovered some wrong tests (like that "@type:AdminVM" should not match "dom0"). Closes QubesOS/qubes-issues#9870
dedf332 to
4d62b9e
Compare
|
That case should now work too. Tried to unify the handling of dom0 (and it's effective aliases) a bit and added further tests. |
|
PipelineRetry |
Apparently Qubes 4.2 and 4.3 work differently when it comes to translating @adminvm to dom0 and vice versa. I see some code explicitly doing this [1], so I am not sure it is a bug. But in the behavior is: - Qubes 4.2: dom0 ---translates-to--> @adminvm - Qubes 4.3: dom0 ---translates-to--> dom0 Therefore we have to add a switch case to disambiguate. Either that or wrap around our qrexec_policy() function. [1]: QubesOS/qubes-core-qrexec#204
Apparently Qubes 4.2 and 4.3 work differently when it comes to translating @adminvm to dom0 and vice versa. I see some code explicitly doing this [1], so I am not sure it is a bug. But in the behavior is: - Qubes 4.2: dom0 ---translates-to--> @adminvm - Qubes 4.3: dom0 ---translates-to--> dom0 Therefore we have to add a switch case to disambiguate. Either that or wrap around our qrexec_policy() function. [1]: QubesOS/qubes-core-qrexec#204
3 participants
The previous behavior of handling "dom0" and "@adminvm" in policies was
inconsistent and let to the problem that you could not specify
target=dom0 default_target=dom0 (or both @adminvm) since one was
translated to @adminvm and the other was alsways expanded to dom0.
Based on the discussion in QubesOS/qubes-issues#9870 we now stop to
translate dom0 (or it's UUID) to @adminvm. The later works as keyword
that is expanded to dom0.
Note this also uncovered some wrong tests (like that "@type:AdminVM"
should not match "dom0").
Closes QubesOS/qubes-issues#9870
Depends on #203