fix(qubes-setup-dnat-to-ns): handle null dns dbus response

[3nprob]

resolve1.Get dbus call is not guaranteed to return a list. This handles the case of a None result as equal to [].

An alternative would be to fall back to get_dns_resolv_conf (as already done for handling known errors)

• Broken out from Add support for IPv6 Virtual DNS (#462) #592

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.10%. Comparing base (e5d626b) to head (3dbe9c3).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #593   +/-   ##
=======================================
  Coverage   71.10%   71.10%           
=======================================
  Files           3        3           
  Lines         481      481           
=======================================
  Hits          342      342           
  Misses        139      139           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[marmarek]

According to the documentation DNS is an array, not "array or nothing". Even if I unset all addresses, I still get an array. What is the case when the service does respond with something else? Note the service not responding at all is already handled via an exception.

[3nprob]

According to the documentation DNS is an array, not "array or nothing". Even if I unset all addresses, I still get an array. What is the case when the service does respond with something else? Note the service not responding at all is already handled via an exception.

While as you note the dbus interface itself always returns an arrary, dbus-python typings indicate Nothing being a possible return value for the (soft-deprecated) Python API. I didn't look into under what conditions that may happen or if the interface is just mistyped. Seems sane to cover the cases specified by API surface in any case?

[3nprob]

The ambiguity makes me think maybe the None case should be handled like known error (falling back to resolv.conf).

Updated accordingly.

[marmarek]

This line isn't needed - there is no code path that leave dns unset and gets to the place where it's used. And there would be - better get an exception that is easy to find, than silently break DNS.

[3nprob]

And there would be - better get an exception that is easy to find, than silently break DNS.

Hum, the fallback function get_resolv_conf does seem to use the strategy of "fail silently and return empty on error (which does seem sane as there is no global exception handler or hook to catch such errors right now?)

[marmarek]

That may want fixing too, at some point (if not propagate exception, then at least log a message)... But that's independent of this PR.