From 575f265daf973428ed1b910f91fb124615fbebd4 Mon Sep 17 00:00:00 2001 From: Nataliia Solomko Date: Fri, 25 Sep 2026 16:51:50 +0300 Subject: config-management: T6304: Rewrite commit-archive location into a structured CLI Replace the free-form 'commit-archive location ' leafNode with a structured, per-destination tagNode: `set system config-management commit-archive location \ authentication { username | password } | server | port | path` Each location selects exactly one transport protocol (scp, sftp, ssh, ftp, ftps, http, https, git, tftp). 'git' additionally accepts a 'transport' (https/http/ssh, default https). 'source-address' and 'vrf' stay under 'commit-archive' and apply to every location, as before. --- python/vyos/config_mgmt.py | 115 +++++++++++++++++++++++++++++++++++++++++---- python/vyos/remote.py | 32 +++++++++---- 2 files changed, 128 insertions(+), 19 deletions(-) (limited to 'python') diff --git a/python/vyos/config_mgmt.py b/python/vyos/config_mgmt.py index d75122ddb..63dd11552 100644 --- a/python/vyos/config_mgmt.py +++ b/python/vyos/config_mgmt.py @@ -29,6 +29,7 @@ from shutil import copy, chown from subprocess import Popen from subprocess import PIPE from subprocess import DEVNULL +from urllib.parse import quote from urllib.parse import urlsplit from urllib.parse import urlunsplit from tabulate import tabulate @@ -152,13 +153,14 @@ class ConfigMgmt: d = config.get_config_dict( ['system', 'config-management'], key_mangling=('-', '_'), + no_tag_node_value_mangle=True, get_first_key=True, with_recursive_defaults=True, ) self.max_revisions = int(d.get('commit_revisions', 0)) self.num_revisions = 0 - self.locations = d.get('commit_archive', {}).get('location', []) + self.locations = d.get('commit_archive', {}).get('location', {}) self.source_address = d.get('commit_archive', {}).get('source_address', '') self.reboot_unconfirmed = bool( d.get('commit_confirm', {}).get('action') == 'reboot' @@ -178,9 +180,15 @@ class ConfigMgmt: # post-commit hook in conf_mode script base_path = ['system', 'config-management', 'commit-archive'] location_path = base_path + ['location'] - self.effective_locations = None + self.effective_locations = {} if config.exists_effective(location_path): - self.effective_locations = config.return_effective_values(location_path) + self.effective_locations = config.get_config_dict( + location_path, + effective=True, + key_mangling=('-', '_'), + no_tag_node_value_mangle=True, + get_first_key=True, + ) vrf_path = base_path + ['vrf'] self.effective_vrf = None @@ -496,6 +504,71 @@ Proceed ?""" self._add_log_entry() self._update_archive() + @staticmethod + def _build_archive_url(proto: str, conf: dict) -> str: + """Reconstruct an upload URL from a structured commit-archive entry. + + Credentials are stored as plain text and percent-encoded here so that + special characters do not corrupt the URL. + """ + from vyos.template import is_ipv6 + + # git over file:// is a local repository: just a path, no host, port or + # credentials. Return early and leave the remote path below untouched. + if proto == 'git' and conf.get('transport') == 'file': + path = conf.get('path', '/') + if not path.startswith('/'): + path = f'/{path}' + path = quote(path, safe='/') + return f'git+file://{path}' + + userinfo = '' + auth = conf.get('authentication', {}) + username = auth.get('username', '') + password = auth.get('password', '') + # a lone password is emitted as ':password@' (empty user) so ftp/ftps + # keep their REMOTE_USERNAME/anonymous fallback for the user + if username or password: + userinfo = quote(username, safe='') + if password: + userinfo += f":{quote(password, safe='')}" + userinfo += '@' + + # bracket IPv6 literals so the ':' are not read as host/port separators + server = conf['server'] + if is_ipv6(server): + server = f'[{server}]' + netloc = server + if conf.get('port'): + netloc += f":{conf['port']}" + + path = conf.get('path', '/') + if not path.startswith('/'): + path = f'/{path}' + if proto in ('scp', 'sftp', 'ssh', 'ftp', 'ftps'): + # these clients unquote the path once before use, so encode '%' as + # well: that single decode then restores the configured path, + # including a literal %2F + path = quote(path, safe='/') + else: + # http(s)/git keep the path encoded on the wire (the server/remote + # decodes it), so leave an existing %2F intact instead of + # double-encoding it + path = ''.join( + part if part.lower() == '%2f' else quote(part, safe='/') + for part in re.split('(%2[fF])', path) + ) + + if proto == 'git': + # default the transport to https here. Bare 'git' is the read-only + # native protocol and cannot push, so https is the + # sensible working default when none is configured. + transport = conf.get('transport', 'https') + scheme = f'git+{transport}' + else: + scheme = proto + return urlunsplit((scheme, f'{userinfo}{netloc}', path, '', '')) + def commit_archive(self): """Upload config to remote archive.""" from vyos.remote import upload @@ -508,13 +581,35 @@ Proceed ?""" if self.effective_locations: print('Archiving config...') - for location in self.effective_locations: - url = urlsplit(location) - _, _, netloc = url.netloc.rpartition('@') - redacted_location = urlunsplit(url._replace(netloc=netloc)) - print(f' {redacted_location}', end=' ', flush=True) - upload(archive_config_file, f'{location}/{remote_file}', - source_host=source_address, vrf=self.effective_vrf) + archive_failed = False + for name, archive in self.effective_locations.items(): + for proto, conf in archive.items(): + location = self._build_archive_url(proto, conf) + url = urlsplit(location) + _, _, netloc = url.netloc.rpartition('@') + redacted_location = urlunsplit(url._replace(netloc=netloc)) + print(f' {name}: {redacted_location}', end=' ', flush=True) + try: + upload( + archive_config_file, + f'{location.rstrip("/")}/{remote_file}', + source_host=source_address, + vrf=self.effective_vrf, + ) + print('OK', flush=True) + except SystemExit: + # upload() exits on failure and already prints the reason; + # just end the status line and keep archiving the rest + # instead of aborting the whole hook + print(flush=True) + archive_failed = True + except KeyboardInterrupt: + # a user interrupt aborts the whole archive run cleanly + # rather than moving on to the next destination + sys.exit(1) + + if archive_failed: + sys.exit(1) # op-mode functions # diff --git a/python/vyos/remote.py b/python/vyos/remote.py index a3cbc1abb..6a475eb13 100644 --- a/python/vyos/remote.py +++ b/python/vyos/remote.py @@ -43,6 +43,7 @@ from ftplib import error_reply from ftplib import parse150 from requests import Session from requests.adapters import HTTPAdapter +from urllib.parse import unquote from urllib.parse import urlsplit from urllib.parse import urlunsplit from urllib3 import PoolManager @@ -61,6 +62,15 @@ from vyos.defaults import directories CHUNK_SIZE = 8192 + +def _decode(value): + """Percent-decode a URL userinfo component so the real credentials reach + the server (they are stored plain and encoded into the URL, which may turn + characters like '@' into '%40'). Returns None when it is unset so the + caller falls back to the environment.""" + return unquote(value) if value else None + + class InteractivePolicy(MissingHostKeyPolicy): """ Paramiko policy for interactively querying the user on whether to proceed @@ -243,9 +253,11 @@ class FtpC: vrf=None): self.secure = url.scheme == 'ftps' self.hostname = url.hostname - self.path = url.path - self.username = url.username or os.getenv('REMOTE_USERNAME', 'anonymous') - self.password = url.password or os.getenv('REMOTE_PASSWORD', '') + self.path = unquote(url.path) + self.username = _decode(url.username) or os.getenv( + 'REMOTE_USERNAME', 'anonymous' + ) + self.password = _decode(url.password) or os.getenv('REMOTE_PASSWORD', '') self.port = url.port or 21 self.source = (source_host, source_port) self.progressbar = progressbar @@ -308,9 +320,9 @@ class SshC: timeout=10.0, vrf=None): self.hostname = url.hostname - self.path = url.path - self.username = url.username or os.getenv('REMOTE_USERNAME') - self.password = url.password or os.getenv('REMOTE_PASSWORD') + self.path = unquote(url.path) + self.username = _decode(url.username) or os.getenv('REMOTE_USERNAME') + self.password = _decode(url.password) or os.getenv('REMOTE_PASSWORD') self.port = url.port or 22 self.source = (source_host, source_port) self.progressbar = progressbar @@ -393,8 +405,8 @@ class HttpC: self.progressbar = progressbar self.check_space = check_space self.source_pair = (source_host, source_port) - self.username = url.username or os.getenv('REMOTE_USERNAME') - self.password = url.password or os.getenv('REMOTE_PASSWORD') + self.username = _decode(url.username) or os.getenv('REMOTE_USERNAME') + self.password = _decode(url.password) or os.getenv('REMOTE_PASSWORD') self.timeout = timeout self.vrf = vrf @@ -636,8 +648,10 @@ def upload(local_path, urlstring, progressbar=False, print_error(f'Unable to upload "{redacted_location}": {err}') sys.exit(1) except KeyboardInterrupt: + # a user interrupt is not an upload failure; re-raise instead of exiting + # so callers can tell the two apart and stop rather than carry on print_error('\nUpload aborted by user.') - sys.exit(1) + raise def get_remote_config(urlstring, source_host='', source_port=0): """ -- cgit v1.2.3