Allow core-admin extension to extend libvirt xml templates

[marmarek]

The problem you're addressing (if any)
Ability for an extension to add/remove elements of libvirt xml for some (or all) VMs

Describe the solution you'd like
An extension could define extra directory where jinja template loader could search. Some more design work is needed to allow multiple templates to use this mechanism and to allow override only parts of the template, and load original template (extends jinja directive by default doesn't allow to load same-named template from a different directory).
This mechanism should also allow multiple extensions to use it, possibly to modify different parts of the libvirt xml.

Where is the value to a user, and who might that user be?
Write extensions that depends on libvirt xml modifications. Example use cases include #5051 or #4688

Describe alternatives you've considered

1. Use /etc/qubes/templates/libvirt/xen/by-name/..., but it doesn't allow multiple extensions to use it for the same VM.
2. Add a hook that allows post-process the libvirt xml at python level - this looks very unfriendly (need to parse xml and format it back), especially when we already have template engine in place.

Additional context
Right now it's mostly for #5051, to allow extension to replace network device xml.

Relevant documentation you've consulted
http://jinja.pocoo.org/docs/2.10/api/#loaders
http://jinja.pocoo.org/docs/2.10/templates/#template-inheritance
https://dev.qubes-os.org/projects/core-admin/en/latest/libvirt.html

Related, non-duplicate issues

cc @fepitre

[marmarek]

@woju any idea?

[woju]

Is it needed in extensions in particular? Current mechanism for customising libvirt.xml was really meant for quick, targeted changes by user. For that reason jinja2 was chosen, because it is easy to write. Can some other configuration mechanism be used, like salt formulas with rewriting logic? If rewriting config in extensions is really needed, I'd suggest to parse it ourselves to lxml's ElementTree and pass it in some form to extensions so etree can be modified to their heart's content. Then we (core) would put it back to str/bytes and eventually pass to libvirt. The API can be organised in two ways, either we pass ElementTree directly to extensions's hook, or we fire hook and the hook yields a visitor callable, and only the callable gets passed the etree. The direct one is probably simpler and good enough. This approach (lxml and hooks after jinja2) has downside that extensions would have final word and not user (jinja2 extends written directly by local user will be overridden by that extension), but jinja2 is not well suited for hooking up our extensions.

…

-- pozdrawiam / best regards _.-._ Wojtek Porczyk .-^' '^-. Invisible Things Lab |'-.-^-.-'| | | | | I do not fear computers, | '-.-' | I fear lack of them. '-._ : ,-' -- Isaac Asimov `^-^-_>

[marmarek]

Is it needed in extensions in particular?

Not necessary extensions, but some way allowing cleanly applying multiple modifications to the libvirt xml. The current solution allows only one entity to do so, directed for use by the user. Applying multiple modifications of the same XML file with salt seems fragile at least. Do you know some reliable method for xml modification in salt? That would be useful in other places too (like configuring xfce).

If sticking with extensions, this is also what I and @fepitre have considered, but since we already have template engine in place we hoped there is a better way.

BTW this is example modification: https://github.com/fepitre/qubes-core-admin-addon-bridged-netvm/blob/master/bridge.xml

[woju]

On Wed, Jun 19, 2019 at 06:52:14AM -0700, Marek Marczykowski-Górecki wrote: > Is it needed in extensions in particular? Not necessary extensions, but some way allowing cleanly applying multiple modifications to the libvirt xml. The current solution allows only one entity to do so, directed for use by the user. Applying multiple modifications of the same XML file with salt seems fragile at least.

Right.

Do you know some reliable method for xml modification in salt? That would be useful in other places too (like configuring xfce).

No, I don't. The modules that need it, like salt.cloud.libvirt, use xml or lxml directly: https://github.com/saltstack/salt/blob/develop/salt/cloud/clouds/libvirt.py

If sticking with extensions, this is also what I and @fepitre have considered, but since we already have template engine in place we hoped there is a better way.

I'd advise against repurposing jinja. Jinja is template language, a DSL if you like. I don't have any idea how that could be done correctly. In principle, jinja2 `{% extends %}` are constrained to whatever was forseen by original template writer, and any templates are constrained to whatever was forseen in python code (which variables were passed for render). For extension that is limitation.

…

BTW this is example modification: https://github.com/fepitre/qubes-core-admin-addon-bridged-netvm/blob/master/bridge.xml

-- pozdrawiam / best regards _.-._ Wojtek Porczyk .-^' '^-. Invisible Things Lab |'-.-^-.-'| | | | | I do not fear computers, | '-.-' | I fear lack of them. '-._ : ,-' -- Isaac Asimov `^-^-_>

[marmarek]

In principle, jinja2 {% extends %} are constrained to whatever was forseen by original template writer,

In theory this should be fine, as we're template writers and we can add blocks for all kind of nodes we have there (or are supported in libvirt). as for variables, we expose vm object, so this is also quite flexible. The problem is I don't see a way to stack such templates, like extends / super referencing "parent" template whatever it is (i.e. without explicitly naming it in the template itself). Maybe it could be worked around with a call to python "I'm template shipped by extension XYZ, what is my parent template?", like:

{% extends get_parent_template("extension-xyz") %}
...

but that doesn't sound like an elegant solution...
If you don't have any better idea, then indeed modifying libvirt xml directly from extension may be the way to go. With the problem you've mentioned that user no longer have the final word in what ends up in that xml.

[woju]

On Wed, Jun 19, 2019 at 07:56:16AM -0700, Marek Marczykowski-Górecki wrote: > In principle, jinja2 `{% extends %}` are constrained to whatever was forseen by original template writer, In theory this should be fine, as we're template writers and we can add blocks for all kind of nodes we have there (or are supported in libvirt). as for variables, we expose `vm` object, so this is also quite flexible. The problem is I don't see a way to stack such templates, like `extends` / `super` referencing "parent" template whatever it is (i.e. without explicitly naming it in the template itself). Maybe it could be worked around with a call to python "I'm template shipped by extension XYZ, what is my parent template?", like: ```jinja2 {% extends get_parent_template("extension-xyz") %} ... ``` but that doesn't sound like an elegant solution...

This would end up to be a footgun for the user, see below.

If you don't have any better idea, then indeed modifying libvirt xml directly from extension may be the way to go.

For this snippet:

BTW this is example modification: https://github.com/fepitre/qubes-core-admin-addon-bridged-netvm/blob/master/bridge.xml

It would be something like this: ```python class XMLExtension(qubes.ext.Extension): @qubes.ext.handler('domain-libvirt-xml') # ? def on_libvirt_xml(self, vm, event, libvirt_xml): # ? if vm.features.get(...): bridge = lxml.etree.XML(''' <interface type="bridge"> <source bridge="{vm.features[bridge_name]}" /> <mac address="{vm.features[bridge_mac]}" /> <backenddomain name="{vm.features[bridge_backenddomain]}" /> <script path="vif-bridge" /> </interface> '''.format(vm=vm)) devices, = libvirt_xml.xpath('//devices') devices.append(bridge) ``` I don't think this is much worse from the snippet you linked.

With the problem you've mentioned that user no longer have the final word in what ends up in that xml.

That would be also the problem with your `get_parent_template()` jinja2-only solution. Or that could be worked around by instructing user to also use that function, but then if user wouldn't actually use it, then all modifications from extensions would be dropped. Extensions would probably be failing for no apparent reason... In practice, I don't think this is a real problem. Most of what extensions can do is add-only. You can't really undo an effect from other extensions. This XML won't be an exception. To counteract possible user confusion, may I suggest to also supply some debugging tool, `qvm-debug-show-me-what-libvirt-xml-looks-like-and-why`.

…

-- pozdrawiam / best regards _.-._ Wojtek Porczyk .-^' '^-. Invisible Things Lab |'-.-^-.-'| | | | | I do not fear computers, | '-.-' | I fear lack of them. '-._ : ,-' -- Isaac Asimov `^-^-_>

[fepitre]

It would be something like this: python class XMLExtension(qubes.ext.Extension): @qubes.ext.handler('domain-libvirt-xml') # ? def on_libvirt_xml(self, vm, event, libvirt_xml): # ? if vm.features.get(...): bridge = lxml.etree.XML(''' <interface type="bridge"> <source bridge="{vm.features[bridge_name]}" /> <mac address="{vm.features[bridge_mac]}" /> <backenddomain name="{vm.features[bridge_backenddomain]}" /> <script path="vif-bridge" /> </interface> '''.format(vm=vm)) devices, = libvirt_xml.xpath('//devices') devices.append(bridge) I don't think this is much worse from the snippet you linked.
@woju : thank you I will use it and adapt it for multiple interfaces.