diff --git a/interface-definitions/container.xml.in b/interface-definitions/container.xml.in index e7dacea367b..902dd74d5ff 100644 --- a/interface-definitions/container.xml.in +++ b/interface-definitions/container.xml.in @@ -13,6 +13,21 @@ [-a-zA-Z0-9]+ Container name must be alphanumeric and can contain hyphens + + + + + + + + + + + + + + + @@ -69,6 +84,12 @@ Add a host device to the container + + + + + + @@ -99,6 +120,11 @@ [-_a-zA-Z0-9]+ Environment variable name must be alphanumeric and can contain hyphen and underscores + + + + + @@ -170,6 +196,11 @@ [a-z0-9](?:[a-z0-9.-]*[a-z0-9])? Label variable name must be alphanumeric and can contain hyphen, dots and underscores + + + + + @@ -253,6 +284,12 @@ Publish port to the container + + + + + + #include @@ -361,6 +398,12 @@ Mount a volume into the container + + + + + + diff --git a/python/vyos/config.py b/python/vyos/config.py index cca65f0eb96..671ae32a865 100644 --- a/python/vyos/config.py +++ b/python/vyos/config.py @@ -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 @@ -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) @@ -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: @@ -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, diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index 4cb84194aa0..021c1c799a5 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -23,9 +23,55 @@ from vyos import ConfigError from vyos.utils.dict import dict_search +from vyos.xml_ref import is_leaf, is_tag, is_tag_value # pattern re-used in ipsec migration script dynamic_interface_pattern = r'(ppp|pppoe|sstpc|l2tp|ipoe)[0-9]+' +def verify_children(rpath: list[str], config: dict, key_mangling=None): + from vyos.xml_ref import child_requirements + + if is_tag(rpath) and not is_tag_value(rpath): + for k, v in config.items(): + verify_children(rpath + [k], v) + return + + if is_leaf(rpath): + return + + schema_requirements = child_requirements(rpath) + + for requirement in schema_requirements: + match requirement[0]: + case "require": + for req in requirement[1]: + if req not in config: + raise ConfigError(f'[{" ".join(rpath)}] Requires "{req}" to be configured') + + case "atLeastOneOf": + for req in requirement[1]: + if req in config: + break + else: + children = '" or "'.join(requirement[1]) + raise ConfigError(f'[{" ".join(rpath)}] Requires at least one of "{children}" to be configured') + + case "depend": + if (requirement[1][0] in config) and not (requirement[1][1] in config): + raise ConfigError(f'[{" ".join(rpath)}] Can not configure "{requirement[1][0]}" without "{requirement[1][1]}"') + + case "conflict": + if (requirement[1][0] in config) and (requirement[1][1] in config): + raise ConfigError(f'[{" ".join(rpath)}] "{requirement[1][0]}" and "{requirement[1][1]}" can not be configured at the same time') + + case _: + raise ValueError("Unsupported child requirement type") + + for k, requirement in config.items(): + path = rpath.copy() + path.append(k) + verify_children(path, requirement) + + def verify_mtu(config): """ Common helper function used by interface implementations to perform diff --git a/python/vyos/xml_ref/__init__.py b/python/vyos/xml_ref/__init__.py index 2ba3da4e8ae..0ec988ab4e5 100644 --- a/python/vyos/xml_ref/__init__.py +++ b/python/vyos/xml_ref/__init__.py @@ -59,6 +59,9 @@ def owner(path: list) -> str: def priority(path: list) -> str: return load_reference().priority(path) +def child_requirements(path: list[str]) -> list: + return load_reference().child_requirements(path) + def cli_defined(path: list, node: str, non_local=False) -> bool: return load_reference().cli_defined(path, node, non_local=non_local) diff --git a/python/vyos/xml_ref/definition.py b/python/vyos/xml_ref/definition.py index c85835ffd2b..6030b661e27 100644 --- a/python/vyos/xml_ref/definition.py +++ b/python/vyos/xml_ref/definition.py @@ -162,6 +162,10 @@ def owner(self, path: list) -> str: def priority(self, path: list) -> str: return self._least_upper_data(path, 'priority') + def child_requirements(self, path: list[str]) -> list: + d = self._get_ref_path(path) + return self._get_ref_node_data(d, 'child_requirements') + @staticmethod def _dict_get(d: dict, path: list) -> dict: for i in path: diff --git a/python/vyos/xml_ref/generate_cache.py b/python/vyos/xml_ref/generate_cache.py index 5f3f84deee0..697279cf666 100755 --- a/python/vyos/xml_ref/generate_cache.py +++ b/python/vyos/xml_ref/generate_cache.py @@ -34,7 +34,7 @@ ref_cache = abspath(join(_here, 'cache.py')) node_data_fields = ("node_type", "multi", "valueless", "default_value", - "owner", "priority") + "owner", "priority", "child_requirements") def trim_node_data(cache: dict): for k in list(cache): diff --git a/schema/interface_definition.rnc b/schema/interface_definition.rnc index 758d9ce1ca6..61d7e75f4ce 100644 --- a/schema/interface_definition.rnc +++ b/schema/interface_definition.rnc @@ -18,7 +18,7 @@ # USA # The language of this file is compact form RELAX-NG -# http://relaxng.org/compact-tutorial-20030326.htm +# https://relaxng.org/compact-tutorial-20030326.html # (unless converted to XML, then just RELAX-NG :) # Interface definition starts with interfaceDefinition tag that may contain node tags @@ -105,6 +105,9 @@ properties = element properties (element secret { empty })? & (element priority { text })? & + # These are meaningful only for tag and node nodes + childRequirements? & + # These are meaningful only for tag nodes (element keepChildOrder { empty })? } @@ -184,3 +187,15 @@ completionHelp = element completionHelp (element path { text })* & (element script { text })* } + + +# childRequirements tags is a declarative way to configure basic +# requirements of node or tagnode children. +childRequirements = element childRequirements { + (element require { child+ } )? & + (element conflict { nodeNameAttr, child+ })* & + (element atLeastOneOf { child+ } )* & + (element depend { nodeNameAttr, child+ })* +} + +child = element child {nodeNameAttr,empty} diff --git a/schema/interface_definition.rng b/schema/interface_definition.rng index 94a828c3bbb..895852ee0f1 100644 --- a/schema/interface_definition.rng +++ b/schema/interface_definition.rng @@ -2,19 +2,19 @@ @@ -142,7 +142,7 @@ Nodes may have properties For simplicity, any property is allowed in any node, but whether they are used or not is implementation-defined - + Leaf nodes may differ in number of values that can be associated with them. By default, a leaf node can have only one value. @@ -150,7 +150,7 @@ "valueless" means it can have no values at all. "hidden" means node visibility can be toggled, eg 'dangerous' commands, "secret" allows a node to hide its value from unprivileged users. - + "priority" is used to influence node processing order for nodes with exact same dependencies and in compatibility modes. --> @@ -205,6 +205,10 @@ + + + + @@ -328,4 +332,50 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/schema/op-mode-definition.rng b/schema/op-mode-definition.rng index a255aeb7302..e6185cb25d5 100644 --- a/schema/op-mode-definition.rng +++ b/schema/op-mode-definition.rng @@ -139,10 +139,11 @@ diff --git a/smoketest/scripts/cli/test_container.py b/smoketest/scripts/cli/test_container.py index 3201883b8ca..ed2da2cfc2f 100755 --- a/smoketest/scripts/cli/test_container.py +++ b/smoketest/scripts/cli/test_container.py @@ -209,5 +209,70 @@ def test_uid_gid(self): tmp = cmd(f'sudo podman exec -it {cont_name} id -g') self.assertEqual(tmp, gid) + def test_image(self): + cont_name = 'image-test' + + self.cli_set(base_path + ['name', cont_name, 'allow-host-networks']) + + # verify() - image is required + with self.assertRaises(ConfigSessionError): + self.cli_commit() + self.cli_set(base_path + ['name', cont_name, 'image', cont_image]) + + self.cli_commit() + + # verify + pid = 0 + with open(PROCESS_PIDFILE.format(cont_name), 'r') as f: + pid = int(f.read()) + + # Check for running process + self.assertEqual(process_named_running(PROCESS_NAME), pid) + + def test_network_required(self): + cont_name = 'image-test' + + self.cli_set(base_path + ['name', cont_name, 'image', cont_image]) + + # verify() - image is required + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # at least one of "network" or "allow-host-networks" must be configured + self.cli_set(base_path + ['name', cont_name, 'allow-host-networks']) + self.cli_commit() + + # verify + pid = 0 + with open(PROCESS_PIDFILE.format(cont_name), 'r') as f: + pid = int(f.read()) + + # Check for running process + self.assertEqual(process_named_running(PROCESS_NAME), pid) + + def test_network_or_host_networks(self): + cont_name = 'image-test' + prefix = '192.0.2.0/24' + net_name = 'NET01' + + self.cli_set(base_path + ['name', cont_name, 'image', cont_image]) + self.cli_set(base_path + ['name', cont_name, 'allow-host-networks']) + self.cli_set(base_path + ['name', cont_name, 'network', net_name, 'address', str(ip_interface(prefix).ip)]) + + # verify() - image is required + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(base_path + ['name', cont_name, 'network', net_name]) + self.cli_commit() + + # verify + pid = 0 + with open(PROCESS_PIDFILE.format(cont_name), 'r') as f: + pid = int(f.read()) + + # Check for running process + self.assertEqual(process_named_running(PROCESS_NAME), pid) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/container.py b/src/conf_mode/container.py index a73a18ffa22..877570741ad 100755 --- a/src/conf_mode/container.py +++ b/src/conf_mode/container.py @@ -26,7 +26,7 @@ from vyos.configdict import dict_merge from vyos.configdict import node_changed from vyos.configdict import is_node_changed -from vyos.configverify import verify_vrf +from vyos.configverify import verify_vrf, verify_children from vyos.ifconfig import Interface from vyos.utils.file import write_file from vyos.utils.process import call @@ -65,8 +65,7 @@ def get_config(config=None): else: conf = Config() - base = ['container'] - container = conf.get_config_dict(base, key_mangling=('-', '_'), + container = conf.get_config_dict(['container'], key_mangling=('-', '_'), no_tag_node_value_mangle=True, get_first_key=True, with_recursive_defaults=True) @@ -74,7 +73,7 @@ def get_config(config=None): for name in container.get('name', []): # T5047: Any container related configuration changed? We only # wan't to restart the required containers and not all of them ... - tmp = is_node_changed(conf, base + ['name', name]) + tmp = is_node_changed(conf, container.base + ['name', name]) if tmp: if 'container_restart' not in container: container['container_restart'] = [name] @@ -85,16 +84,16 @@ def get_config(config=None): # default_values['registry'] into the tagNode variables if 'registry' not in container: container.update({'registry' : {}}) - default_values = default_value(base + ['registry']) + default_values = default_value(container.base + ['registry']) for registry in default_values: tmp = {registry : {}} container['registry'] = dict_merge(tmp, container['registry']) # Delete container network, delete containers - tmp = node_changed(conf, base + ['network']) + tmp = node_changed(conf, container.base + ['network']) if tmp: container.update({'network_remove' : tmp}) - tmp = node_changed(conf, base + ['name']) + tmp = node_changed(conf, container.base + ['name']) if tmp: container.update({'container_remove' : tmp}) return container @@ -104,12 +103,12 @@ def verify(container): if not container: return None + # Validate child config against schema definitions + verify_children(container.base, container.raw_conf_dict) + # Add new container if 'name' in container: for name, container_config in container['name'].items(): - # Container image is a mandatory option - if 'image' not in container_config: - raise ConfigError(f'Container image for "{name}" is mandatory!') # Check if requested container image exists locally. If it does not # exist locally - inform the user. This is required as there is a @@ -167,57 +166,12 @@ def verify(container): raise ConfigError(f'Only one IP address per address family can be used for '\ f'container "{name}". {cnt_ipv4} IPv4 and {cnt_ipv6} IPv6 address(es)!') - if 'device' in container_config: - for dev, dev_config in container_config['device'].items(): - if 'source' not in dev_config: - raise ConfigError(f'Device "{dev}" has no source path configured!') - - if 'destination' not in dev_config: - raise ConfigError(f'Device "{dev}" has no destination path configured!') - - source = dev_config['source'] - if not os.path.exists(source): - raise ConfigError(f'Device "{dev}" source path "{source}" does not exist!') - - if 'environment' in container_config: - for var, cfg in container_config['environment'].items(): - if 'value' not in cfg: - raise ConfigError(f'Environment variable {var} has no value assigned!') - - if 'label' in container_config: - for var, cfg in container_config['label'].items(): - if 'value' not in cfg: - raise ConfigError(f'Label variable {var} has no value assigned!') - if 'volume' in container_config: for volume, volume_config in container_config['volume'].items(): - if 'source' not in volume_config: - raise ConfigError(f'Volume "{volume}" has no source path configured!') - - if 'destination' not in volume_config: - raise ConfigError(f'Volume "{volume}" has no destination path configured!') - source = volume_config['source'] if not os.path.exists(source): raise ConfigError(f'Volume "{volume}" source path "{source}" does not exist!') - if 'port' in container_config: - for tmp in container_config['port']: - if not {'source', 'destination'} <= set(container_config['port'][tmp]): - raise ConfigError(f'Both "source" and "destination" must be specified for a port mapping!') - - # If 'allow-host-networks' or 'network' not set. - if 'allow_host_networks' not in container_config and 'network' not in container_config: - raise ConfigError(f'Must either set "network" or "allow-host-networks" for container "{name}"!') - - # Can not set both allow-host-networks and network at the same time - if {'allow_host_networks', 'network'} <= set(container_config): - raise ConfigError(f'"allow-host-networks" and "network" for "{name}" cannot be both configured at the same time!') - - # gid cannot be set without uid - if 'gid' in container_config and 'uid' not in container_config: - raise ConfigError(f'Cannot set "gid" without "uid" for container') - # Add new network if 'network' in container: for network, network_config in container['network'].items():