Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5083: extend xml schema definitions to support child requirements #3575

Open
wants to merge 1 commit into
base: current
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,6 @@ python/vyos/xml_ref/pkg_cache/*_cache.py
# We do not use pip
Pipfile
Pipfile.lock

# KDE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change might be sensible for KDE users (not for me — I use MATE, btw; but @jestabro does use KDE :).
But it's out of scope of this PR, could you make it separate please?

.directory
26 changes: 0 additions & 26 deletions .vscode/settings.json

This file was deleted.

1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ interface_definitions: $(config_xml_obj)
find $(BUILD_DIR)/interface-definitions -type f -name "*.xml" | xargs -I {} $(CURDIR)/scripts/build-command-templates {} $(CURDIR)/schema/interface_definition.rng $(TMPL_DIR) || exit 1

$(CURDIR)/python/vyos/xml_ref/generate_cache.py --xml-dir $(BUILD_DIR)/interface-definitions || exit 1
$(CURDIR)/python/vyos/xml_ref/update_cache.py || exit 1

# XXX: delete top level node.def's that now live in other packages
# IPSec VPN EAP-RADIUS does not support source-address
Expand Down
64 changes: 64 additions & 0 deletions interface-definitions/container.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@
<regex>[-a-zA-Z0-9]+</regex>
</constraint>
<constraintErrorMessage>Container name must be alphanumeric and can contain hyphens</constraintErrorMessage>
<childSpecification>
<requiredChildren>
<child>image</child>
</requiredChildren>
<mutuallyExclusiveChildren>
<child>allow-host-networks</child>
<child>network</child>
</mutuallyExclusiveChildren>
<atLeastOneOf>
<child>allow-host-networks</child>
<child>network</child>
</atLeastOneOf>
<oneWayDependantChildren>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The word "dependent" is misspelled here (see https://en.wiktionary.org/wiki/dependent)

I also wonder if it should be just "dependentChildren" rather than "oneWayDependentChildren".

<dependants><child>gid</child></dependants>
<dependees><child>uid</child></dependees>
</oneWayDependantChildren>
</childSpecification>
</properties>
<children>
<leafNode name="allow-host-pid">
Expand Down Expand Up @@ -89,6 +106,11 @@
<constraint>
<validator name="sysctl"/>
</constraint>
<childSpecification>
<requiredChildren>
<child>value</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
<leafNode name="value">
Expand All @@ -104,6 +126,12 @@
<tagNode name="device">
<properties>
<help>Add a host device to the container</help>
<childSpecification>
<requiredChildren>
<child>source</child>
<child>destination</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
<leafNode name="source">
Expand Down Expand Up @@ -134,6 +162,11 @@
<regex>[-_a-zA-Z0-9]+</regex>
</constraint>
<constraintErrorMessage>Environment variable name must be alphanumeric and can contain hyphen and underscores</constraintErrorMessage>
<childSpecification>
<requiredChildren>
<child>value</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
<leafNode name="value">
Expand Down Expand Up @@ -205,6 +238,11 @@
<regex>[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?</regex>
</constraint>
<constraintErrorMessage>Label variable name must be alphanumeric and can contain hyphen, dots and underscores</constraintErrorMessage>
<childSpecification>
<requiredChildren>
<child>value</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
<leafNode name="value">
Expand Down Expand Up @@ -306,6 +344,12 @@
<tagNode name="port">
<properties>
<help>Publish port to the container</help>
<childSpecification>
<requiredChildren>
<child>source</child>
<child>destination</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
#include <include/listen-address.xml.i>
Expand Down Expand Up @@ -414,6 +458,12 @@
<tagNode name="volume">
<properties>
<help>Mount a volume into the container</help>
<childSpecification>
<requiredChildren>
<child>source</child>
<child>destination</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
<leafNode name="source">
Expand Down Expand Up @@ -498,6 +548,11 @@
<properties>
<help>Network name</help>
#include <include/constraint/container-network.xml.i>
<childSpecification>
<requiredChildren>
<child>prefix</child>
</requiredChildren>
</childSpecification>
</properties>
<children>
#include <include/generic-description.xml.i>
Expand Down Expand Up @@ -525,6 +580,15 @@
<tagNode name="registry">
<properties>
<help>Registry Name</help>
<childSpecification>
<oneWayDependantChildren>
<dependants><child>authentication</child></dependants>
<dependees>
<child>username</child>
<child>password</child>
</dependees>
</oneWayDependantChildren>
</childSpecification>
</properties>
<defaultValue>docker.io quay.io</defaultValue>
<children>
Expand Down
127 changes: 127 additions & 0 deletions interface-definitions/interfaces_openvpn.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,133 @@
<format>vtunN</format>
<description>OpenVPN interface name</description>
</valueHelp>
<childSpecification>
<requiredChildren>
<child>mode</child>
</requiredChildren>
<!-- OpenVPN client mode -->
<oneWayDependantChildren>
<dependants><descendant name="mode"><value>client</value></descendant></dependants>
<dependees>
<child>remote_host</child>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Underscore, not a hyphen?

</dependees>
</oneWayDependantChildren>
<mutuallyExclusiveChildren>
<descendant name="mode"><value>client</value></descendant>
<child>local-port</child>
<child>local-host</child>
<descendant name="tls"><child>dh-params</child></descendant>
</mutuallyExclusiveChildren>
<!-- / OpenVPN client mode -->
<!-- OpenVPN site-to-site -->
<oneWayDependantChildren>
<dependants><descendant name="mode"><value>site-to-site</value></descendant></dependants>
<dependees>
<child>remote-host</child>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Site-to-site mode doesn't actually require remote-host. One side of a site-to-site tunnel is a listener.

</dependees>
</oneWayDependantChildren>
<mutuallyExclusiveChildren>
<descendant name="mode"><value>site-to-site</value></descendant>
<descendant name="encryption"><child>ncp-ciphers</child></descendant>
</mutuallyExclusiveChildren>
<!-- / OpenVPN site-to-site -->
<!-- OpenVPN server mode -->
<mutuallyExclusiveChildren>
<descendant name="mode"><value>server</value></descendant>
<descendant name="protocol"><value>tcp-active</value></descendant>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="mode"><value>server</value></descendant>
<child>authentication</child>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sequence of <mutuallyExclusiveChildren> tags is very tedious to read, to be honest.

I'm also thinking that <mutuallyExclusiveChildren> should only be for children that simply cannot appear together under any circumstances, like server and remote-host.

In a situation when a node value is incompatible with a bunch of children, we may want to call it something else.

At the very least I'd like a more compact syntax, like:

<conflict>
  <source> <descendant name="mode" value="server"> </source>
  <excludes>remote-host</excludes>
  <excludes>remote-port</excludes>
  ...
</conflict>

<descendant name="mode"><value>server</value></descendant>
<child>remote-host</child>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="mode"><value>server</value></descendant>
<child>remote-port</child>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<child>server</child>
<descendant name="mode">
<value>site-to-site</value>
<value>client</value>
</descendant>
</mutuallyExclusiveChildren>
<mutuallyDependantChildren>
<descendant name="authentication">
<child>username</child>
<child>password</child>
</descendant>
</mutuallyDependantChildren>
<mutuallyDependantChildren>
<descendant name="server"><descendant name="client-ip-pool">
<child>start</child>
<child>stop</child>
</descendant></descendant>
</mutuallyDependantChildren>
<!-- / OpenVPN server mode -->
<!-- checks for both client and site-to-site go here -->
<mutuallyExclusiveChildren>
<descendant name="mode">
<value>client</value>
<value>site-to-site</value>
</descendant>
<descendant name="server"><child>reject-unconfigured-clients</child></descendant>
</mutuallyExclusiveChildren>
<!-- / checks for both client and site-to-site go here -->
<!-- OpenVPN common verification section (not depending on any operation mode) -->
<!-- TCP active -->
<mutuallyExclusiveChildren>
<descendant name="protocol"><value>tcp-active</value></descendant>
<child>local-port</child>
</mutuallyExclusiveChildren>
<oneWayDependantChildren>
<dependants><descendant name="protocol"><value>tcp-active</value></descendant></dependants>
<dependees><child>remote-host</child></dependees>
</oneWayDependantChildren>
<!-- / TCP active -->
<!-- TLS/encryption -->
<mutuallyExclusiveChildren>
<child>shared_secret_key</child>
<descendant name="encryption"><descendant name="cipher">
<value>aes128gcm</value>
<value>aes192gcm</value>
<value>aes256gcm</value>
</descendant></descendant>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="tls"><child>auth-key</child></descendant>
<descendant name="tls"><child>crypt-key</child></descendant>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="tls">
<descendant name="role">
<value>active</value>
</descendant>
</descendant>
<descendant name="protocol"><value>tcp-passive </value></descendant>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="tls">
<descendant name="role">
<value>active</value>
</descendant>
</descendant>
<child>dh-params</child>
</mutuallyExclusiveChildren>
<mutuallyExclusiveChildren>
<descendant name="tls">
<descendant name="role">
<value>passive</value>
</descendant>
</descendant>
<descendant name="protocol"><value>tcp-active</value></descendant>
</mutuallyExclusiveChildren>
<!-- / TLS/encryption -->
<!-- / OpenVPN common verification section (not depending on any operation mode) -->
</childSpecification>
</properties>
<children>
#include <include/interface/authentication.xml.i>
Expand Down
24 changes: 22 additions & 2 deletions python/vyos/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@

import re
import json
from typing import Union
from typing import Union, Any
from copy import deepcopy

import vyos.configtree
from vyos.xml_ref import multi_to_list
Expand All @@ -79,12 +80,24 @@
class ConfigDict(dict):
_from_defaults = {}
_dict_kwargs = {}
_raw_conf_dict: dict[str, Any]
_base: list[str]

def from_defaults(self, path: list[str]) -> bool:
return from_source(self._from_defaults, path)

@property
def kwargs(self) -> dict:
return self._dict_kwargs

@property
def raw_conf_dict(self):
return self._raw_conf_dict

@property
def base(self):
return self._base

def config_dict_merge(src: dict, dest: Union[dict, ConfigDict]) -> ConfigDict:
if not isinstance(dest, ConfigDict):
dest = ConfigDict(dest)
Expand Down Expand Up @@ -290,7 +303,7 @@ def get_config_dict(self, path=[], effective=False, key_mangling=None,
no_tag_node_value_mangle=False,
with_defaults=False,
with_recursive_defaults=False,
with_pki=False):
with_pki=False) -> ConfigDict:
"""
Args:
path (str list): Configuration tree path, can be empty
Expand All @@ -312,6 +325,8 @@ def get_config_dict(self, path=[], effective=False, key_mangling=None,
root_dict = self.get_cached_root_dict(effective)
conf_dict = get_sub_dict(root_dict, lpath, get_first_key=get_first_key)

raw_conf_dict = deepcopy(conf_dict)

rpath = lpath if get_first_key else lpath[:-1]

if not no_multi_convert:
Expand Down Expand Up @@ -346,6 +361,11 @@ def get_config_dict(self, path=[], effective=False, key_mangling=None,
# save optional args for a call to get_config_defaults
setattr(conf_dict, '_dict_kwargs', kwargs)

# save args that are reused during verification
setattr(conf_dict, '_raw_conf_dict', raw_conf_dict)
setattr(conf_dict, '_base', rpath)


return conf_dict

def get_config_defaults(self, path=[], effective=False, key_mangling=None,
Expand Down
3 changes: 2 additions & 1 deletion python/vyos/configdict.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from vyos.utils.dict import dict_search
from vyos.utils.process import cmd
from vyos.config import Config, ConfigDict

def retrieve_config(path_hash, base_path, config):
"""
Expand Down Expand Up @@ -425,7 +426,7 @@ def get_pppoe_interfaces(conf, vrf=None):

return pppoe_interfaces

def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pki=False):
def get_interface_dict(config: Config, base, ifname='', recursive_defaults=True, with_pki=False) -> tuple[str, ConfigDict]:
"""
Common utility function to retrieve and mangle the interfaces configuration
from the CLI input nodes. All interfaces have a common base where value
Expand Down
Loading
Loading