diff --git a/setup.py b/setup.py index 196777d0e3..a989acb876 100644 --- a/setup.py +++ b/setup.py @@ -257,6 +257,7 @@ 'xmltodict==0.12.0', 'lazy-object-proxy', 'six==1.16.0', + 'scp==0.14.5', ] + sonic_dependencies, setup_requires= [ 'pytest-runner', diff --git a/sonic_installer/main.py b/sonic_installer/main.py index 341111f265..c5d3a256f2 100644 --- a/sonic_installer/main.py +++ b/sonic_installer/main.py @@ -337,6 +337,8 @@ def migrate_sonic_packages(bootloader, binary_image_version): new_image_docker_mount = os.path.join(new_image_mount, "var", "lib", "docker") docker_default_config = os.path.join(new_image_mount, "etc", "default", "docker") docker_default_config_backup = os.path.join(new_image_mount, TMP_DIR, "docker_config_backup") + custom_manifests_path = os.path.join(PACKAGE_MANAGER_DIR, "manifests") + new_image_package_directory_path = os.path.join(new_image_mount, "var", "lib", "sonic-package-manager") if not os.path.isdir(new_image_docker_dir): # NOTE: This codepath can be reached if the installation process did not @@ -372,6 +374,8 @@ def migrate_sonic_packages(bootloader, binary_image_version): run_command_or_raise(["chroot", new_image_mount, DOCKER_CTL_SCRIPT, "start"]) docker_started = True run_command_or_raise(["cp", packages_path, os.path.join(new_image_mount, TMP_DIR, packages_file)]) + run_command_or_raise(["mkdir", "-p", custom_manifests_path]) + run_command_or_raise(["cp", "-arf", custom_manifests_path, new_image_package_directory_path]) run_command_or_raise(["touch", os.path.join(new_image_mount, "tmp", DOCKERD_SOCK)]) run_command_or_raise(["mount", "--bind", os.path.join(VAR_RUN_PATH, DOCKERD_SOCK), diff --git a/sonic_package_manager/main.py b/sonic_package_manager/main.py index 8a0aabb901..d305e3c911 100644 --- a/sonic_package_manager/main.py +++ b/sonic_package_manager/main.py @@ -15,6 +15,7 @@ from sonic_package_manager.errors import PackageManagerError from sonic_package_manager.logger import log from sonic_package_manager.manager import PackageManager +from sonic_package_manager.manifest import MANIFESTS_LOCATION BULLET_UC = '\u2022' @@ -157,6 +158,13 @@ def repository(ctx): pass +@cli.group() +@click.pass_context +def manifests(ctx): + """ Custom local Manifest management commands. """ + + pass + @cli.group() @click.pass_context def show(ctx): @@ -280,6 +288,73 @@ def changelog(ctx, exit_cli(f'Failed to print package changelog: {err}', fg='red') +@manifests.command('create') +@click.pass_context +@click.argument('name', type=click.Path()) +@click.option('--from-json', type=str, help='specify manifest json file') +@root_privileges_required +def create_manifest(ctx, name, from_json): + """Create a new custom local manifest file.""" + + manager: PackageManager = ctx.obj + try: + manager.create_package_manifest(name, from_json) + except Exception as e: + click.echo("Error: Manifest {} creation failed - {}".format(name, str(e))) + return + + +@manifests.command('update') +@click.pass_context +@click.argument('name', type=click.Path()) +@click.option('--from-json', type=str, required=True) +@root_privileges_required +def update_manifest(ctx, name, from_json): + """Update an existing custom local manifest file with new one.""" + + manager: PackageManager = ctx.obj + try: + manager.update_package_manifest(name, from_json) + except Exception as e: + click.echo(f"Error occurred while updating manifest '{name}': {e}") + return + + +@manifests.command('delete') +@click.pass_context +@click.argument('name', type=click.Path()) +@root_privileges_required +def delete_manifest(ctx, name): + """Delete a custom local manifest file.""" + manager: PackageManager = ctx.obj + try: + manager.delete_package_manifest(name) + except Exception as e: + click.echo("Error: Failed to delete manifest file '{}'. {}".format(name, e)) + + +@manifests.command('show') +@click.pass_context +@click.argument('name', type=click.Path()) +@root_privileges_required +def show_manifest(ctx, name): + """Show the contents of custom local manifest file.""" + manager: PackageManager = ctx.obj + try: + manager.show_package_manifest(name) + except FileNotFoundError: + click.echo("Manifest file '{}' not found.".format(name)) + + +@manifests.command('list') +@click.pass_context +@root_privileges_required +def list_manifests(ctx): + """List all custom local manifest files.""" + manager: PackageManager = ctx.obj + manager.list_package_manifest() + + @repository.command() @click.argument('name', type=str) @click.argument('repository', type=str) @@ -334,6 +409,14 @@ def remove(ctx, name): help='Allow package downgrade. By default an attempt to downgrade the package ' 'will result in a failure since downgrade might not be supported by the package, ' 'thus requires explicit request from the user.') +@click.option('--use-local-manifest', + is_flag=True, + default=None, + help='Use locally created custom manifest file. ', + hidden=True) +@click.option('--name', + type=str, + help='custom name for the package') @add_options(PACKAGE_SOURCE_OPTIONS) @add_options(PACKAGE_COMMON_OPERATION_OPTIONS) @add_options(PACKAGE_COMMON_INSTALL_OPTIONS) @@ -348,7 +431,9 @@ def install(ctx, enable, set_owner, skip_host_plugins, - allow_downgrade): + allow_downgrade, + use_local_manifest, + name): """ Install/Upgrade package using [PACKAGE_EXPR] in format "[=|@]". The repository to pull the package from is resolved by lookup in package database, @@ -378,16 +463,58 @@ def install(ctx, if allow_downgrade is not None: install_opts['allow_downgrade'] = allow_downgrade + if use_local_manifest: + if not name: + click.echo('name argument is not provided to use local manifest') + return + original_file = os.path.join(MANIFESTS_LOCATION, name) + if not os.path.exists(original_file): + click.echo(f'Local Manifest file for {name} does not exists to install') + return + try: manager.install(package_expr, from_repository, from_tarball, + use_local_manifest, + name, **install_opts) except Exception as err: exit_cli(f'Failed to install {package_source}: {err}', fg='red') except KeyboardInterrupt: exit_cli('Operation canceled by user', fg='red') +# At the end of sonic-package-manager install, a new manifest file is created with the name. +# At the end of sonic-package-manager uninstall name, +# this manifest file name and name.edit will be deleted. +# At the end of sonic-package-manager update, +# we need to mv maniests name.edit to name in case of success, else keep it as such. +# So during sonic-package-manager update, +# we could take old package from name and new package from edit and at the end, follow 3rd point + + +@cli.command() +@add_options(PACKAGE_COMMON_OPERATION_OPTIONS) +@add_options(PACKAGE_COMMON_INSTALL_OPTIONS) +@click.argument('name') +@click.pass_context +@root_privileges_required +def update(ctx, name, force, yes, skip_host_plugins): + """ Update package to the updated manifest file. """ + + manager: PackageManager = ctx.obj + + update_opts = { + 'force': force, + 'skip_host_plugins': skip_host_plugins, + 'update_only': True, + } + try: + manager.update(name, **update_opts) + except Exception as err: + exit_cli(f'Failed to update package {name}: {err}', fg='red') + except KeyboardInterrupt: + exit_cli('Operation canceled by user', fg='red') @cli.command() @add_options(PACKAGE_COMMON_OPERATION_OPTIONS) diff --git a/sonic_package_manager/manager.py b/sonic_package_manager/manager.py index e41bb00e8f..a052479607 100644 --- a/sonic_package_manager/manager.py +++ b/sonic_package_manager/manager.py @@ -65,7 +65,15 @@ version_to_tag, tag_to_version ) - +import click +import json +import requests +import getpass +import paramiko +import urllib.parse +from scp import SCPClient +from sonic_package_manager.manifest import Manifest, MANIFESTS_LOCATION, DEFAULT_MANIFEST_FILE +LOCAL_JSON = "/tmp/local_json" @contextlib.contextmanager def failure_ignore(ignore: bool): @@ -344,6 +352,8 @@ def install(self, expression: Optional[str] = None, repotag: Optional[str] = None, tarball: Optional[str] = None, + use_local_manifest: bool = False, + name: Optional[str] = None, **kwargs): """ Install/Upgrade SONiC Package from either an expression representing the package and its version, repository and tag or @@ -358,7 +368,7 @@ def install(self, PackageManagerError """ - source = self.get_package_source(expression, repotag, tarball) + source = self.get_package_source(expression, repotag, tarball, use_local_manifest=use_local_manifest, name=name) package = source.get_package() if self.is_installed(package.name): @@ -446,6 +456,37 @@ def install_from_source(self, self.database.update_package(package.entry) self.database.commit() + @under_lock + def update(self, + name: str, + **kwargs): + """ Update SONiC Package referenced by name. The update + can be forced if force argument is True. + + Args: + name: SONiC Package name. + Raises: + PackageManagerError + """ + if self.is_installed(name): + edit_name = name + '.edit' + edit_file = os.path.join(MANIFESTS_LOCATION, edit_name) + if os.path.exists(edit_file): + self.upgrade_from_source(None, name=name, **kwargs) + else: + click.echo("Package manifest {}.edit file does not exists to update".format(name)) + return + else: + click.echo("Package {} is not installed".format(name)) + return + + def remove_unused_docker_image(self, package): + image_id_used = any(entry.image_id == package.image_id for entry in self.database if entry.name != package.name) + if not image_id_used: + self.docker.rmi(package.image_id, force=True) + else: + log.info(f'Image with ID {package.image_id} is in use by other package(s). Skipping deletion') + @under_lock @opt_check def uninstall(self, name: str, @@ -493,7 +534,8 @@ def uninstall(self, name: str, self._get_installed_packages_except(package) ) self.docker.rm_by_ancestor(package.image_id, force=True) - self.docker.rmi(package.image_id, force=True) + # Delete image if it is not in use, otherwise skip deletion + self.remove_unused_docker_image(package) package.entry.image_id = None except Exception as err: raise PackageUninstallationError( @@ -504,6 +546,13 @@ def uninstall(self, name: str, package.entry.version = None self.database.update_package(package.entry) self.database.commit() + manifest_path = os.path.join(MANIFESTS_LOCATION, name) + edit_path = os.path.join(MANIFESTS_LOCATION, name + ".edit") + if os.path.exists(manifest_path): + os.remove(manifest_path) + if os.path.exists(edit_path): + os.remove(edit_path) + @under_lock @opt_check @@ -511,7 +560,9 @@ def upgrade_from_source(self, source: PackageSource, force=False, skip_host_plugins=False, - allow_downgrade=False): + allow_downgrade=False, + update_only: Optional[bool] = False, + name: Optional[str] = None): """ Upgrade SONiC Package to a version the package reference expression specifies. Can force the upgrade if force parameter is True. Force can allow a package downgrade. @@ -521,12 +572,17 @@ def upgrade_from_source(self, force: Force the upgrade. skip_host_plugins: Skip host OS plugins installation. allow_downgrade: Flag to allow package downgrade. + update_only: Perform package update with new manifest. + name: name of package. Raises: PackageManagerError """ - new_package = source.get_package() - name = new_package.name + if update_only: + new_package = self.get_installed_package(name, use_edit=True) + else: + new_package = source.get_package() + name = new_package.name with failure_ignore(force): if not self.is_installed(name): @@ -543,19 +599,20 @@ def upgrade_from_source(self, old_version = old_package.manifest['package']['version'] new_version = new_package.manifest['package']['version'] - with failure_ignore(force): - if old_version == new_version: - raise PackageUpgradeError(f'{new_version} is already installed') - - # TODO: Not all packages might support downgrade. - # We put a check here but we understand that for some packages - # the downgrade might be safe to do. There can be a variable in manifest - # describing package downgrade ability or downgrade-able versions. - if new_version < old_version and not allow_downgrade: - raise PackageUpgradeError( - f'Request to downgrade from {old_version} to {new_version}. ' - f'Downgrade might be not supported by the package' - ) + if not update_only: + with failure_ignore(force): + if old_version == new_version: + raise PackageUpgradeError(f'{new_version} is already installed') + + # TODO: Not all packages might support downgrade. + # We put a check here but we understand that for some packages + # the downgrade might be safe to do. There can be a variable in manifest + # describing package downgrade ability or downgrade-able versions. + if new_version < old_version and not allow_downgrade: + raise PackageUpgradeError( + f'Request to downgrade from {old_version} to {new_version}. ' + f'Downgrade might be not supported by the package' + ) # remove currently installed package from the list installed_packages = self._get_installed_packages_and(new_package) @@ -579,8 +636,9 @@ def upgrade_from_source(self, self._uninstall_cli_plugins(old_package) exits.callback(rollback(self._install_cli_plugins, old_package)) - source.install(new_package) - exits.callback(rollback(source.uninstall, new_package)) + if not update_only: + source.install(new_package) + exits.callback(rollback(source.uninstall, new_package)) feature_enabled = self.feature_registry.is_feature_enabled(old_feature) @@ -620,7 +678,8 @@ def upgrade_from_source(self, self._install_cli_plugins(new_package) exits.callback(rollback(self._uninstall_cli_plugin, new_package)) - self.docker.rmi(old_package.image_id, force=True) + if old_package.image_id != new_package.image_id: + self.remove_unused_docker_image(old_package) exits.pop_all() except Exception as err: @@ -633,6 +692,10 @@ def upgrade_from_source(self, new_package_entry.version = new_version self.database.update_package(new_package_entry) self.database.commit() + if update_only: + manifest_path = os.path.join(MANIFESTS_LOCATION, name) + edit_path = os.path.join(MANIFESTS_LOCATION, name + ".edit") + os.rename(edit_path, manifest_path) @under_lock @opt_check @@ -718,7 +781,7 @@ def migrate_package(old_package_entry, file.write(chunk) file.flush() - self.install(tarball=file.name) + self.install(tarball=file.name, name=name) else: log.info(f'installing {name} version {version}') @@ -755,7 +818,9 @@ def migrate_package(old_package_entry, new_package.version = old_package.version migrate_package(old_package, new_package) else: - self.install(f'{new_package.name}={new_package_default_version}') + # self.install(f'{new_package.name}={new_package_default_version}') + repo_tag_formed = "{}:{}".format(new_package.repository, new_package.default_reference) + self.install(None, repo_tag_formed, name=new_package.name) else: # No default version and package is not installed. # Migrate old package same version. @@ -764,7 +829,7 @@ def migrate_package(old_package_entry, self.database.commit() - def get_installed_package(self, name: str) -> Package: + def get_installed_package(self, name: str, use_local_manifest: bool = False, use_edit: bool = False) -> Package: """ Get installed package by name. Args: @@ -777,14 +842,19 @@ def get_installed_package(self, name: str) -> Package: source = LocalSource(package_entry, self.database, self.docker, - self.metadata_resolver) + self.metadata_resolver, + use_local_manifest=use_local_manifest, + name=name, + use_edit=use_edit) return source.get_package() def get_package_source(self, package_expression: Optional[str] = None, repository_reference: Optional[str] = None, tarboll_path: Optional[str] = None, - package_ref: Optional[PackageReference] = None): + package_ref: Optional[PackageReference] = None, + use_local_manifest: bool = False, + name: Optional[str] = None): """ Returns PackageSource object based on input source. Args: @@ -800,7 +870,7 @@ def get_package_source(self, if package_expression: ref = parse_reference_expression(package_expression) - return self.get_package_source(package_ref=ref) + return self.get_package_source(package_ref=ref, name=name) elif repository_reference: repo_ref = utils.DockerReference.parse(repository_reference) repository = repo_ref['name'] @@ -810,15 +880,19 @@ def get_package_source(self, reference, self.database, self.docker, - self.metadata_resolver) + self.metadata_resolver, + use_local_manifest, + name) elif tarboll_path: return TarballSource(tarboll_path, self.database, self.docker, - self.metadata_resolver) + self.metadata_resolver, + use_local_manifest, + name) elif package_ref: package_entry = self.database.get_package(package_ref.name) - + name = package_ref.name # Determine the reference if not specified. # If package is installed assume the installed # one is requested, otherwise look for default @@ -829,7 +903,9 @@ def get_package_source(self, return LocalSource(package_entry, self.database, self.docker, - self.metadata_resolver) + self.metadata_resolver, + use_local_manifest, + name) if package_entry.default_reference is not None: package_ref.reference = package_entry.default_reference else: @@ -840,7 +916,9 @@ def get_package_source(self, package_ref.reference, self.database, self.docker, - self.metadata_resolver) + self.metadata_resolver, + use_local_manifest, + name) else: raise ValueError('No package source provided') @@ -1018,6 +1096,196 @@ def _uninstall_cli_plugin(self, package: Package, command: str): if os.path.exists(host_plugin_path): os.remove(host_plugin_path) + def download_file(self, url, local_path): + # Parse information from the URL + parsed_url = urllib.parse.urlparse(url) + protocol = parsed_url.scheme + username = parsed_url.username + password = parsed_url.password + hostname = parsed_url.hostname + remote_path = parsed_url.path + supported_protocols = ['http', 'https', 'scp', 'sftp'] + + # clear the temporary local file + if os.path.exists(local_path): + os.remove(local_path) + + if not protocol: + # check for local file + if os.path.exists(url): + os.rename(url, local_path) + return True + else: + click.echo("Local file not present") + return False + if protocol not in supported_protocols: + click.echo("Protocol not supported") + return False + + # If the protocol is HTTP and no username or password is provided, proceed with the download using requests + if (protocol == 'http' or protocol == 'https') and not username and not password: + try: + with requests.get(url, stream=True) as response: + response.raise_for_status() + with open(local_path, 'wb') as f: + for chunk in response.iter_content(chunk_size=8192): + if chunk: + f.write(chunk) + except requests.exceptions.RequestException as e: + click.echo("Download error", e) + return False + else: + # If password is not provided, prompt the user for it securely + if password is None: + password = getpass.getpass(prompt=f"Enter password for {username}@{hostname}: ") + + # Create an SSH client + client = paramiko.SSHClient() + # Automatically add the server's host key (this is insecure and should be handled differently in production) + client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + + try: + # Connect to the SSH server + client.connect(hostname, username=username, password=password) + + if protocol == 'scp': + # Create an SCP client + scp = SCPClient(client.get_transport()) + # Download the file + scp.get(remote_path, local_path) + elif protocol == 'sftp': + # Open an SFTP channel + with client.open_sftp() as sftp: + # Download the file + sftp.get(remote_path, local_path) + elif protocol == 'http' or protocol == 'https': + # Download using HTTP for URLs without credentials + try: + with requests.get(url, auth=(username, password), stream=True) as response: + response.raise_for_status() # Raise an exception if the request was not successful + with open(local_path, 'wb') as f: + for chunk in response.iter_content(chunk_size=8192): + if chunk: + f.write(chunk) + except requests.exceptions.RequestException as e: + click.echo("Download error", e) + return False + else: + click.echo(f"Error: Source file '{remote_path}' does not exist.") + + finally: + # Close the SSH connection + client.close() + + def create_package_manifest(self, name, from_json): + if name == "default_manifest": + click.echo("Default Manifest creation is not allowed by user") + return + if self.is_installed(name): + click.echo("Error: A package with the same name {} is already installed".format(name)) + return + mfile_name = os.path.join(MANIFESTS_LOCATION, name) + if os.path.exists(mfile_name): + click.echo("Error: Manifest file '{}' already exists.".format(name)) + return + + if from_json: + ret = self.download_file(from_json, LOCAL_JSON) + if ret is False: + return + from_json = LOCAL_JSON + else: + from_json = DEFAULT_MANIFEST_FILE + data = {} + with open(from_json, 'r') as file: + data = json.load(file) + # Validate with manifest scheme + Manifest.marshal(data) + + # Make sure the 'name' is overwritten into the dict + data['package']['name'] = name + data['service']['name'] = name + + with open(mfile_name, 'w') as file: + json.dump(data, file, indent=4) + click.echo(f"Manifest '{name}' created successfully.") + + def update_package_manifest(self, name, from_json): + if name == "default_manifest": + click.echo("Default Manifest updation is not allowed") + return + + original_file = os.path.join(MANIFESTS_LOCATION, name) + if not os.path.exists(original_file): + click.echo(f'Local Manifest file for {name} does not exists to update') + return + # download json file from remote/local path + ret = self.download_file(from_json, LOCAL_JSON) + if ret is False: + return + from_json = LOCAL_JSON + + with open(from_json, 'r') as file: + data = json.load(file) + + # Validate with manifest scheme + Manifest.marshal(data) + + # Make sure the 'name' is overwritten into the dict + data['package']['name'] = name + data['service']['name'] = name + + if self.is_installed(name): + edit_name = name + '.edit' + edit_file = os.path.join(MANIFESTS_LOCATION, edit_name) + with open(edit_file, 'w') as edit_file: + json.dump(data, edit_file, indent=4) + click.echo(f"Manifest '{name}' updated successfully.") + else: + # If package is not installed, + # update the name file directly + with open(original_file, 'w') as orig_file: + json.dump(data, orig_file, indent=4) + click.echo(f"Manifest '{name}' updated successfully.") + + def delete_package_manifest(self, name): + if name == "default_manifest": + click.echo("Default Manifest deletion is not allowed") + return + # Check if the manifest file exists + mfile_name = "{}/{}".format(MANIFESTS_LOCATION, name) + if not os.path.exists(mfile_name): + click.echo("Error: Manifest file '{}' not found.".format(name)) + return + # Confirm deletion with user input + confirm = click.prompt("Are you sure you want to delete the manifest file '{}'? (y/n)".format(name), type=str) + if confirm.lower() == 'y': + os.remove(mfile_name) + click.echo("Manifest '{}' deleted successfully.".format(name)) + else: + click.echo("Deletion cancelled.") + return + + def show_package_manifest(self, name): + mfile_name = "{}/{}".format(MANIFESTS_LOCATION, name) + edit_file_name = "{}.edit".format(mfile_name) + if os.path.exists(edit_file_name): + mfile_name = edit_file_name + with open(mfile_name, 'r') as file: + data = json.load(file) + click.echo("Manifest file: {}".format(name)) + click.echo(json.dumps(data, indent=4)) + + def list_package_manifest(self): + # Get all files in the manifest location + manifest_files = os.listdir(MANIFESTS_LOCATION) + if not manifest_files: + click.echo("No custom local manifest files found.") + else: + click.echo("Custom Local Manifest files:") + for file in manifest_files: + click.echo("- {}".format(file)) + @staticmethod def get_manager() -> 'PackageManager': """ Creates and returns PackageManager instance. diff --git a/sonic_package_manager/manifest.py b/sonic_package_manager/manifest.py index 865db7ef5c..bc156f102c 100644 --- a/sonic_package_manager/manifest.py +++ b/sonic_package_manager/manifest.py @@ -10,7 +10,12 @@ ) from sonic_package_manager.errors import ManifestError from sonic_package_manager.version import Version +from sonic_package_manager.database import BASE_LIBRARY_PATH +import os +import json +MANIFESTS_LOCATION = os.path.join(BASE_LIBRARY_PATH, "manifests") +DEFAULT_MANIFEST_FILE = os.path.join(BASE_LIBRARY_PATH, "default_manifest") class ManifestSchema: """ ManifestSchema class describes and provides marshalling @@ -249,3 +254,38 @@ def marshal(cls, input_dict: dict): def unmarshal(self) -> Dict: return self.SCHEMA.unmarshal(self) + + def get_manifest_from_local_file(name): + + if '.edit' in name: + actual_name = name.split('.edit')[0] + else: + actual_name = name + + manifest_path = os.path.join(MANIFESTS_LOCATION, name) + if os.path.exists(manifest_path): + with open(manifest_path, 'r') as file: + manifest_dict = json.load(file) + manifest_dict["package"]["name"] = actual_name + manifest_dict["service"]["name"] = actual_name + else: + with open(DEFAULT_MANIFEST_FILE, 'r') as file: + manifest_dict = json.load(file) + manifest_dict["package"]["name"] = actual_name + manifest_dict["service"]["name"] = actual_name + new_manifest_path = os.path.join(MANIFESTS_LOCATION, name) + with open(new_manifest_path, 'w') as file: + json.dump(manifest_dict, file, indent=4) + + json_str = json.dumps(manifest_dict, indent=4) + desired_dict = { + 'Tag': 'master', + 'com': { + 'azure': { + 'sonic': { + 'manifest': json_str + } + } + } + } + return desired_dict diff --git a/sonic_package_manager/metadata.py b/sonic_package_manager/metadata.py index b44b658a74..6485a10782 100644 --- a/sonic_package_manager/metadata.py +++ b/sonic_package_manager/metadata.py @@ -4,15 +4,13 @@ import json import tarfile -from typing import Dict, List - +from typing import Dict, List, Optional from sonic_package_manager import utils from sonic_package_manager.errors import MetadataError from sonic_package_manager.logger import log from sonic_package_manager.manifest import Manifest from sonic_package_manager.version import Version - def translate_plain_to_tree(plain: Dict[str, str], sep='.') -> Dict: """ Convert plain key/value dictionary into a tree by spliting the key with '.' @@ -65,7 +63,8 @@ def __init__(self, docker, registry_resolver): self.docker = docker self.registry_resolver = registry_resolver - def from_local(self, image: str) -> Metadata: + def from_local(self, image: str, use_local_manifest: bool = False, + name: Optional[str] = None, use_edit: bool = False) -> Metadata: """ Reads manifest from locally installed docker image. Args: @@ -75,16 +74,31 @@ def from_local(self, image: str) -> Metadata: Raises: MetadataError """ + if name and (use_local_manifest or use_edit): + edit_file_name = name + '.edit' + if use_edit: + labels = Manifest.get_manifest_from_local_file(edit_file_name) + return self.from_labels(labels) + elif use_local_manifest: + labels = Manifest.get_manifest_from_local_file(name) + return self.from_labels(labels) labels = self.docker.labels(image) - if labels is None: - raise MetadataError('No manifest found in image labels') + if labels is None or len(labels) == 0 or 'com.azure.sonic.manifest' not in labels: + if name: + labels = Manifest.get_manifest_from_local_file(name) + if labels is None: + raise MetadataError('No manifest found in image labels') + else: + raise MetadataError('No manifest found in image labels') return self.from_labels(labels) def from_registry(self, repository: str, - reference: str) -> Metadata: + reference: str, + use_local_manifest: bool = False, + name: Optional[str] = None) -> Metadata: """ Reads manifest from remote registry. Args: @@ -96,19 +110,25 @@ def from_registry(self, MetadataError """ - registry = self.registry_resolver.get_registry_for(repository) + if use_local_manifest: + labels = Manifest.get_manifest_from_local_file(name) + return self.from_labels(labels) + registry = self.registry_resolver.get_registry_for(repository) manifest = registry.manifest(repository, reference) digest = manifest['config']['digest'] blob = registry.blobs(repository, digest) - labels = blob['config']['Labels'] + labels = blob['config'].get('Labels') + if labels is None or len(labels) == 0 or 'com.azure.sonic.manifest' not in labels: + if name is None: + raise MetadataError('The name(custom) option is required as there is no metadata found in image labels') + labels = Manifest.get_manifest_from_local_file(name) if labels is None: raise MetadataError('No manifest found in image labels') - return self.from_labels(labels) - def from_tarball(self, image_path: str) -> Metadata: + def from_tarball(self, image_path: str, use_local_manifest: bool = False, name: Optional[str] = None) -> Metadata: """ Reads manifest image tarball. Args: image_path: Path to image tarball. @@ -117,16 +137,23 @@ def from_tarball(self, image_path: str) -> Metadata: Raises: MetadataError """ + if use_local_manifest: + labels = Manifest.get_manifest_from_local_file(name) + return self.from_labels(labels) with tarfile.open(image_path) as image: manifest = json.loads(image.extractfile('manifest.json').read()) blob = manifest[0]['Config'] image_config = json.loads(image.extractfile(blob).read()) - labels = image_config['config']['Labels'] - if labels is None: - raise MetadataError('No manifest found in image labels') - + labels = image_config['config'].get('Labels') + if labels is None or len(labels) == 0 or 'com.azure.sonic.manifest' not in labels: + if name is None: + raise MetadataError('The name(custom) option is \ + required as there is no metadata found in image labels') + labels = Manifest.get_manifest_from_local_file(name) + if labels is None: + raise MetadataError('No manifest found in image labels') return self.from_labels(labels) @classmethod diff --git a/sonic_package_manager/source.py b/sonic_package_manager/source.py index 7a13dccbac..2a0f07b0f1 100644 --- a/sonic_package_manager/source.py +++ b/sonic_package_manager/source.py @@ -4,7 +4,7 @@ from sonic_package_manager.dockerapi import DockerApi, get_repository_from_image from sonic_package_manager.metadata import Metadata, MetadataResolver from sonic_package_manager.package import Package - +from typing import Optional class PackageSource(object): """ PackageSource abstracts the way manifest is read @@ -105,20 +105,24 @@ def __init__(self, tarball_path: str, database: PackageDatabase, docker: DockerApi, - metadata_resolver: MetadataResolver): + metadata_resolver: MetadataResolver, + use_local_manifest: bool = False, + name: Optional[str] = None): super().__init__(database, docker, metadata_resolver) self.tarball_path = tarball_path + self.use_local_manifest = use_local_manifest + self.name = name def get_metadata(self) -> Metadata: """ Returns manifest read from tarball. """ - - return self.metadata_resolver.from_tarball(self.tarball_path) + return self.metadata_resolver.from_tarball(self.tarball_path, + use_local_manifest=self.use_local_manifest, + name=self.name) def install_image(self, package: Package): """ Installs image from local tarball source. """ - return self.docker.load(self.tarball_path) @@ -131,18 +135,24 @@ def __init__(self, reference: str, database: PackageDatabase, docker: DockerApi, - metadata_resolver: MetadataResolver): + metadata_resolver: MetadataResolver, + use_local_manifest: bool = False, + name: Optional[str] = None): super().__init__(database, docker, metadata_resolver) self.repository = repository self.reference = reference + self.use_local_manifest = use_local_manifest + self.name = name def get_metadata(self) -> Metadata: """ Returns manifest read from registry. """ return self.metadata_resolver.from_registry(self.repository, - self.reference) + self.reference, + self.use_local_manifest, + self.name) def install_image(self, package: Package): """ Installs image from registry. """ @@ -161,11 +171,17 @@ def __init__(self, entry: PackageEntry, database: PackageDatabase, docker: DockerApi, - metadata_resolver: MetadataResolver): + metadata_resolver: MetadataResolver, + use_local_manifest: bool = False, + name: Optional[str] = None, + use_edit: bool = False): super().__init__(database, docker, metadata_resolver) self.entry = entry + self.use_local_manifest = use_local_manifest + self.name = name + self.use_edit = use_edit def get_metadata(self) -> Metadata: """ Returns manifest read from locally installed Docker. """ @@ -177,8 +193,7 @@ def get_metadata(self) -> Metadata: # won't have image_id in database. Using their # repository name as image. image = f'{self.entry.repository}:latest' - - return self.metadata_resolver.from_local(image) + return self.metadata_resolver.from_local(image, self.use_local_manifest, self.name, self.use_edit) def get_package(self) -> Package: return Package(self.entry, self.get_metadata()) diff --git a/tests/sonic_package_manager/conftest.py b/tests/sonic_package_manager/conftest.py index 10fe72cac1..ccfc2f4929 100644 --- a/tests/sonic_package_manager/conftest.py +++ b/tests/sonic_package_manager/conftest.py @@ -133,20 +133,20 @@ def __init__(self): self.add('Azure/docker-test-6', '2.0.0', 'test-package-6', '2.0.0') self.add('Azure/docker-test-6', 'latest', 'test-package-6', '1.5.0') - def from_registry(self, repository: str, reference: str): + def from_registry(self, repository: str, reference: str, use_local_manifest=None, name=None): manifest = Manifest.marshal(self.metadata_store[repository][reference]['manifest']) components = self.metadata_store[repository][reference]['components'] yang = self.metadata_store[repository][reference]['yang'] return Metadata(manifest, components, yang) - def from_local(self, image: str): + def from_local(self, image: str, use_local_manfiest=None, name=None, use_edit=None): ref = Reference.parse(image) manifest = Manifest.marshal(self.metadata_store[ref['name']][ref['tag']]['manifest']) components = self.metadata_store[ref['name']][ref['tag']]['components'] yang = self.metadata_store[ref['name']][ref['tag']]['yang'] return Metadata(manifest, components, yang) - def from_tarball(self, filepath: str) -> Manifest: + def from_tarball(self, filepath: str, use_local_manifest=None, name=None) -> Manifest: path, ref = filepath.split(':') manifest = Manifest.marshal(self.metadata_store[path][ref]['manifest']) components = self.metadata_store[path][ref]['components'] diff --git a/tests/sonic_package_manager/test_cli.py b/tests/sonic_package_manager/test_cli.py index 695d8cba58..1b7556ae68 100644 --- a/tests/sonic_package_manager/test_cli.py +++ b/tests/sonic_package_manager/test_cli.py @@ -4,6 +4,15 @@ from sonic_package_manager import main +from unittest.mock import patch, mock_open, MagicMock + +MANIFEST_LOCATION = 'fake_manifest_location' +DMFILE_NAME = 'fake_dmfile_name' +DEFAUT_MANIFEST_NAME = 'fake_default_manifest_name' +LOCAL_JSON = 'fake_local_json' +sample_manifest_json = '{"package": {"name": "test", "version": "1.0.0"}, "service": {"name": "test"}}' +fake_manifest_name = 'test-manifest' +MANIFEST_CONTENT = '{"package": {"name": "test", "version": "1.0.0"}, "service": {"name": "test"}}' def test_show_changelog(package_manager, fake_metadata_resolver): """ Test case for "sonic-package-manager package show changelog [NAME]" """ @@ -61,3 +70,217 @@ def test_show_changelog_no_changelog(package_manager): assert result.exit_code == 1 assert result.output == 'Failed to print package changelog: No changelog for package test-package\n' + + +def test_manifests_create_command_existing_manifest(package_manager): + """ Test case for "sonic-package-manager manifests create" with an existing manifest file """ + + runner = CliRunner() + + with patch('os.path.exists', side_effect=[True, False]), \ + patch('sonic_package_manager.main.PackageManager.is_installed', return_value=False), \ + patch('builtins.open', new_callable=mock_open()), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['create'], + ['test-manifest'], + input=sample_manifest_json, + obj=package_manager) + + assert 'Error: Manifest file \'test-manifest\' already exists.' in result.output + assert result.exit_code == 0 + + +def test_manifests_create_command_existing_package(package_manager): + """ Test case for "sonic-package-manager manifests create" with an existing installed package """ + + runner = CliRunner() + + with patch('os.path.exists', return_value=False), \ + patch('sonic_package_manager.main.PackageManager.is_installed', return_value=True), \ + patch('builtins.open', new_callable=mock_open()), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['create'], + ['test-manifest'], + input=sample_manifest_json, + obj=package_manager) + + assert 'Error: A package with the same name test-manifest is already installed' in result.output + assert result.exit_code == 0 + + +def test_manifests_update_command_error_handling(package_manager): + + runner = CliRunner() + + with patch('os.path.exists', return_value=False), \ + patch('builtins.open', new_callable=mock_open(read_data='{"key": "value"}')), \ + patch('json.load', side_effect=lambda x: MagicMock(return_value='{"key": "value"}')), \ + patch('json.dump'), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['update'], + ['non-existent-manifest', '--from-json', 'fake_json_path'], + obj=package_manager) + assert 'Local Manifest file for non-existent-manifest does not exists to update\n' in result.output + assert result.exit_code == 0 + + +def test_manifests_delete_command_deletion_cancelled(package_manager): + runner = CliRunner() + + with patch('os.path.exists', return_value=True), \ + patch('click.prompt', return_value='n'), \ + patch('os.remove') as mock_remove, \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['delete'], ['test-manifest'], obj=package_manager) + + # Check if the cancellation message is present in the result output + assert 'Deletion cancelled.' in result.output + # Check if os.remove is not called when the deletion is cancelled + assert not mock_remove.called + + +def test_manifests_list_command_no_manifests(package_manager): + runner = CliRunner() + + with patch('os.listdir', return_value=[]), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['list'], [], obj=package_manager) + + # Check if the appropriate message is present in the result output + assert 'No custom local manifest files found.\n' in result.output + + +def test_manifests_command(): + """ Test case for "sonic-package-manager manifests" """ + + runner = CliRunner() + result = runner.invoke(main.manifests) + assert result.exit_code == 0 + + +def test_manifests_create_command_exception(package_manager): + """Test case for "sonic-package-manager manifests create" with an exception during manifest creation""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.create_package_manifest', + side_effect=Exception("Custom error")), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['create'], ['test-manifest'], obj=package_manager) + + assert 'Error: Manifest test-manifest creation failed - Custom error' in result.output + assert result.exit_code == 0 + + +def test_manifests_update_command_exception(package_manager): + """Test case for 'sonic-package-manager manifests update' with an exception during manifest update""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.update_package_manifest', + side_effect=Exception("Custom error")), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['update'], + ['test-manifest', '--from-json', 'new_manifest.json'], + obj=package_manager) + + assert 'Error occurred while updating manifest \'test-manifest\': Custom error' in result.output + assert result.exit_code == 0 + + +def test_manifests_delete_command_exception(package_manager): + """Test case for 'sonic-package-manager manifests delete' with an exception during manifest deletion""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.delete_package_manifest', + side_effect=Exception("Custom error")), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['delete'], + ['test-manifest'], obj=package_manager) + + assert "Error: Failed to delete manifest file 'test-manifest'. Custom error" in result.output + assert result.exit_code == 0 + + +def test_manifests_show_command_file_not_found(package_manager): + """Test case for 'sonic-package-manager manifests show' with a non-existent manifest file""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.show_package_manifest', + side_effect=FileNotFoundError()), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.manifests.commands['show'], + ['nonexistent_manifest.json'], obj=package_manager) + + assert "Manifest file 'nonexistent_manifest.json' not found." in result.output + assert result.exit_code == 0 + + +def test_install_with_local_manifest(package_manager): + """Test case for 'install' command with use_local_manifest=True and name provided""" + + runner = CliRunner() + + with patch('os.path.exists', return_value=True), \ + patch('os.geteuid', return_value=0): + result = runner.invoke(main.install, + ['package_name', '--use-local-manifest', '-y'], + obj=package_manager) + + assert 'name argument is not provided to use local manifest' in result.output + assert result.exit_code == 0 + + +def test_install_with_nonexistent_manifest(package_manager): + """Test case for 'install' command with use_local_manifest=True and non-existent name provided""" + + runner = CliRunner() + + with patch('os.path.exists', return_value=False), \ + patch('os.geteuid', return_value=0): + result = runner.invoke( + main.install, + ['package_name', '--use-local-manifest', '--name', 'nonexistent_manifest', '-y'], + obj=package_manager) + + assert 'Local Manifest file for nonexistent_manifest does not exists to install' in result.output + assert result.exit_code == 0 + + +def test_update_command_exception(package_manager): + """Test case for 'update' command with an exception during package update""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.update', + side_effect=Exception("Custom error")), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.update, ['package_name'], obj=package_manager) + + assert 'Failed to update package package_name: Custom error' in result.output + + +def test_update_command_keyboard_interrupt(package_manager): + """Test case for 'update' command with a keyboard interrupt""" + + runner = CliRunner() + + with patch('sonic_package_manager.main.PackageManager.update', + side_effect=KeyboardInterrupt()), \ + patch('os.geteuid', return_value=0): + + result = runner.invoke(main.update, ['package_name'], obj=package_manager) + + assert 'Operation canceled by user' in result.output diff --git a/tests/sonic_package_manager/test_manager.py b/tests/sonic_package_manager/test_manager.py index 46ea3f6acb..a3a311ebb2 100644 --- a/tests/sonic_package_manager/test_manager.py +++ b/tests/sonic_package_manager/test_manager.py @@ -1,13 +1,14 @@ #!/usr/bin/env python import re -from unittest.mock import Mock, call, patch - +import unittest +from unittest.mock import Mock, call, patch, mock_open import pytest import sonic_package_manager from sonic_package_manager.errors import * from sonic_package_manager.version import Version +import json @pytest.fixture(autouse=True) def mock_run_command(): @@ -352,10 +353,10 @@ def test_manager_migration(package_manager, fake_db_for_migration): call('test-package-3=1.6.0'), # test-package-4 was not present in DB at all, but it is present and installed in # fake_db_for_migration, thus asserting that it is going to be installed. - call('test-package-4=1.5.0'), + call(None, 'Azure/docker-test-4:1.5.0', name='test-package-4'), # test-package-5 1.5.0 was installed in fake_db_for_migration but the default # in current db is 1.9.0, assert that migration will install the newer version. - call('test-package-5=1.9.0'), + call(None, 'Azure/docker-test-5:1.9.0', name='test-package-5'), # test-package-6 2.0.0 was installed in fake_db_for_migration but the default # in current db is 1.5.0, assert that migration will install the newer version. call('test-package-6=2.0.0')], @@ -389,3 +390,204 @@ def test_manager_migration_dockerd(package_manager, fake_db_for_migration, mock_ package_manager.migrate_packages(fake_db_for_migration, '/var/run/docker.sock') package_manager.get_docker_client.assert_has_calls([ call('/var/run/docker.sock')], any_order=True) + + +def test_create_package_manifest_default_manifest(package_manager): + """Test case for creating a default manifest.""" + + with patch('os.path.exists', return_value=False), \ + patch('os.mkdir'), \ + patch('builtins.open', new_callable=mock_open()), \ + patch('click.echo') as mock_echo: + + package_manager.create_package_manifest("default_manifest", from_json=None) + + mock_echo.assert_called_once_with("Default Manifest creation is not allowed by user") + + +def test_create_package_manifest_existing_package(package_manager): + """Test case for creating a manifest with an existing package.""" + + with patch('os.path.exists', side_effect=[False, True]), \ + patch('sonic_package_manager.main.PackageManager.is_installed', return_value=True), \ + patch('click.echo') as mock_echo: + + package_manager.create_package_manifest("test-package", from_json=None) + + mock_echo.assert_called_once_with("Error: A package with the same name test-package is already installed") + + +def test_create_package_manifest_existing_manifest(package_manager): + """Test case for creating a manifest with an existing manifest file.""" + + with patch('os.path.exists', return_value=True), \ + patch('click.echo') as mock_echo: + + package_manager.create_package_manifest("test-manifest", from_json=None) + + mock_echo.assert_called_once_with("Error: Manifest file 'test-manifest' already exists.") + + +def test_manifests_create_command(package_manager): + with patch('click.echo') as mock_echo, \ + patch('os.path.exists') as mock_exists, \ + patch('os.mkdir'), \ + patch('builtins.open', new_callable=mock_open()), \ + patch('json.dump'), \ + patch('json.load') as mock_json_load, \ + patch('sonic_package_manager.manifest.Manifest.marshal') as mock_marshal, \ + patch('sonic_package_manager.manager.PackageManager.is_installed') as mock_is_installed, \ + patch('sonic_package_manager.manager.PackageManager.download_file') as mock_download_file: + + dummy_json = {"package": {"name": "test", "version": "1.0.0"}, "service": {"name": "test"}} + # Setup mocks + mock_exists.return_value = False + mock_is_installed.return_value = False + mock_download_file.return_value = True + mock_marshal.return_value = None + mock_json_load.return_value = dummy_json + + # Run the function + package_manager.create_package_manifest("test_manifest", dummy_json) + + # Assertions + mock_echo.assert_called_with("Manifest 'test_manifest' created successfully.") + + +def test_manifests_update_command(package_manager): + with patch('click.echo') as mock_echo, \ + patch('os.path.exists') as mock_exists, \ + patch('os.mkdir'), \ + patch('builtins.open', new_callable=unittest.mock.mock_open), \ + patch('json.dump'), \ + patch('json.load') as mock_json_load, \ + patch('sonic_package_manager.manifest.Manifest.marshal') as mock_marshal, \ + patch('sonic_package_manager.manager.PackageManager.is_installed') as mock_is_installed, \ + patch('sonic_package_manager.manager.PackageManager.download_file') as mock_download_file: + + dummy_json = {"package": {"name": "test", "version": "2.0.0"}, "service": {"name": "test"}} + # Setup mocks + mock_exists.return_value = True + mock_is_installed.return_value = True + mock_download_file.return_value = True + mock_marshal.return_value = None + mock_json_load.return_value = dummy_json + + # Run the function + package_manager.update_package_manifest("test_manifest", "dummy_json") + + # Assertions + mock_echo.assert_called_with("Manifest 'test_manifest' updated successfully.") + + +def test_delete_package_manifest(package_manager): + with patch('click.echo') as mock_echo, \ + patch('click.prompt') as mock_prompt, \ + patch('os.path.exists') as mock_exists, \ + patch('os.remove'): + + # Test case 1: deleting default manifest + package_manager.delete_package_manifest("default_manifest") + mock_echo.assert_called_with("Default Manifest deletion is not allowed") + mock_echo.reset_mock() # Reset the mock for the next test case + + # Test case 2: manifest file doesn't exist + mock_exists.return_value = True + mock_exists.side_effect = lambda x: False if x.endswith("test_manifest") else True + package_manager.delete_package_manifest("test_manifest") + mock_echo.assert_called_with("Error: Manifest file 'test_manifest' not found.") + mock_echo.reset_mock() + + # Test case 3: user confirms deletion + mock_exists.side_effect = lambda x: True if x.endswith("test_manifest") else False + mock_prompt.return_value = "y" + package_manager.delete_package_manifest("test_manifest") + mock_echo.assert_called_with("Manifest 'test_manifest' deleted successfully.") + mock_echo.reset_mock() + + # Test case 4: user cancels deletion + mock_prompt.return_value = "n" + package_manager.delete_package_manifest("test_manifest") + mock_echo.assert_called_with("Deletion cancelled.") + mock_echo.reset_mock() + + +def test_show_package_manifest(package_manager): + with patch('click.echo') as mock_echo, \ + patch('os.path.exists') as mock_exists, \ + patch('builtins.open', unittest.mock.mock_open()), \ + patch('json.load') as mock_json_load: + + mock_exists.return_value = True + mock_exists.side_effect = lambda x: True if x.endswith("test_manifest") else False + + dummy_json = {"package": {"name": "test", "version": "2.0.0"}, "service": {"name": "test"}} + mock_json_load.return_value = dummy_json + + package_manager.show_package_manifest("test_manifest") + mock_echo.assert_called_with(json.dumps(dummy_json, indent=4)) + + +def test_list_package_manifest(package_manager): + with patch('click.echo') as mock_echo, \ + patch('os.path.exists') as mock_exists, \ + patch('os.listdir') as mock_listdir: + + # Test case 1: no custom local manifest files found + mock_exists.return_value = True + mock_listdir.return_value = [] + package_manager.list_package_manifest() + mock_echo.assert_called_with("No custom local manifest files found.") + + # Test case 2: custom local manifest files found + mock_listdir.return_value = ["manifest1.json", "manifest2.json"] + package_manager.list_package_manifest() + mock_echo.assert_any_call("Custom Local Manifest files:") + mock_echo.assert_any_call("- manifest1.json") + mock_echo.assert_any_call("- manifest2.json") + + +def test_download_file_http(package_manager): + fake_remote_url = "http://www.example.com/index.html" + fake_local_path = "local_path" + with patch("requests.get") as mock_requests_get: + with patch("builtins.open", mock_open()) as mock_file: + package_manager.download_file(fake_remote_url, fake_local_path) + mock_requests_get.assert_called_once_with(fake_remote_url, stream=True) + mock_file.assert_called_once_with("local_path", "wb") + + +def test_download_file_scp(package_manager): + fake_remote_url = "scp://admin@10.x.x.x:/home/admin/sec_update.json" + fake_local_path = "local_path" + + with patch("paramiko.SSHClient") as mock_ssh_client: + with patch("scp.SCPClient"): + with patch("getpass.getpass", return_value="test_password"): + package_manager.download_file(fake_remote_url, fake_local_path) + + mock_ssh_client.assert_called_once() + mock_ssh_client.return_value.set_missing_host_key_policy.assert_called_once() + mock_ssh_client.return_value.connect.assert_called_once_with( + "10.x.x.x", + username="admin", + password="test_password" + ) + + +def test_download_file_sftp(package_manager): + fake_remote_url = "sftp://admin@10.x.x.x:/home/admin/sec_update.json" + fake_local_path = "local_path" + + with patch("paramiko.SSHClient") as mock_ssh_client: + with patch("paramiko.SFTPClient.from_transport"): + with patch("getpass.getpass", return_value="test_password"): + package_manager.download_file(fake_remote_url, fake_local_path) + + mock_ssh_client.assert_called_once() + mock_ssh_client.return_value.set_missing_host_key_policy.assert_called_once() + mock_ssh_client.return_value.connect.assert_called_once_with( + "10.x.x.x", + username="admin", + password="test_password" + ) diff --git a/tests/sonic_package_manager/test_manifest.py b/tests/sonic_package_manager/test_manifest.py index 009895991a..5eaa2f6053 100644 --- a/tests/sonic_package_manager/test_manifest.py +++ b/tests/sonic_package_manager/test_manifest.py @@ -1,9 +1,11 @@ #!/usr/bin/env python import pytest +import json +from unittest.mock import patch, mock_open from sonic_package_manager.constraint import ComponentConstraints -from sonic_package_manager.manifest import Manifest, ManifestError +from sonic_package_manager.manifest import Manifest, ManifestError, MANIFESTS_LOCATION def test_manifest_v1_defaults(): @@ -85,3 +87,33 @@ def test_manifest_v1_unmarshal(): for key, section in manifest_json_input.items(): for field, value in section.items(): assert manifest_json[key][field] == value + + +@patch("sonic_package_manager.manifest.open", new_callable=mock_open) +def test_get_manifest_from_local_file_existing_manifest(mock_open, sonic_fs): + # Create a mock manifest file + manifest_name = "test_manifest.json" + manifest_content = {"package": {"name": "test_package", "version": "1.0.0"}, + "service": {"name": "test_service"}} + mock_open.return_value.__enter__.return_value.read.return_value = json.dumps(manifest_content) + sonic_fs.create_dir(MANIFESTS_LOCATION) + + # Call the function + desired_dict = Manifest.get_manifest_from_local_file(manifest_name) + + exp_manifest_content = {"package": {"name": "test_manifest.json", "version": "1.0.0"}, + "service": {"name": "test_manifest.json"}} + manifest_string = json.dumps(exp_manifest_content, indent=4) + desired_output = { + 'Tag': 'master', + 'com': { + 'azure': { + 'sonic': { + 'manifest': manifest_string + } + } + } + } + + # Check if the returned dictionary matches the expected structure + assert desired_dict == desired_output diff --git a/tests/sonic_package_manager/test_metadata.py b/tests/sonic_package_manager/test_metadata.py index 96f9bbc38d..f386836a83 100644 --- a/tests/sonic_package_manager/test_metadata.py +++ b/tests/sonic_package_manager/test_metadata.py @@ -2,13 +2,14 @@ import json import contextlib -from unittest.mock import Mock, MagicMock - +from unittest.mock import Mock, MagicMock, patch +import tempfile +import os import pytest from sonic_package_manager.database import PackageEntry from sonic_package_manager.errors import MetadataError -from sonic_package_manager.manifest import Manifest +from sonic_package_manager.manifest import MANIFESTS_LOCATION, DEFAULT_MANIFEST_FILE from sonic_package_manager.metadata import MetadataResolver from sonic_package_manager.version import Version @@ -87,3 +88,125 @@ def test_metadata_construction(manifest_str): }) assert metadata.yang_modules == ['TEST', 'TEST 2'] + +@pytest.fixture +def temp_manifest_dir(): + with tempfile.TemporaryDirectory() as temp_dir: + yield temp_dir + + +@pytest.fixture +def temp_tarball(temp_manifest_dir): + tarball_path = os.path.join(temp_manifest_dir, 'image.tar') + # Create an empty tarball file for testing + open(tarball_path, 'w').close() + yield tarball_path + + +def test_metadata_resolver_local_with_name_and_use_local_manifest(mock_registry_resolver, + mock_docker_api, + temp_manifest_dir): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + # Patching the get_manifest_from_local_file method to avoid FileNotFoundError + with patch('sonic_package_manager.manifest.Manifest.get_manifest_from_local_file') as mock_get_manifest: + # Setting the side_effect to None to simulate the absence of a manifest file + mock_get_manifest.side_effect = None + with contextlib.suppress(MetadataError): + metadata_resolver.from_local('image', use_local_manifest=True, name='test_manifest', use_edit=False) + + +def test_metadata_resolver_local_manifest_file_not_exist(mock_registry_resolver, mock_docker_api, temp_manifest_dir): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + # Patching the get_manifest_from_local_file method to avoid FileNotFoundError + with patch('sonic_package_manager.manifest.Manifest.get_manifest_from_local_file') as mock_get_manifest: + # Setting the side_effect to None to simulate the absence of a manifest file + mock_get_manifest.side_effect = None + with pytest.raises(MetadataError): + metadata_resolver.from_local('image', use_local_manifest=True, name='test_manifest', use_edit=False) + + +def test_metadata_resolver_tarball_with_use_local_manifest(mock_registry_resolver, + mock_docker_api, + temp_manifest_dir): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + # Patching the get_manifest_from_local_file method to avoid FileNotFoundError + with patch('sonic_package_manager.manifest.Manifest.get_manifest_from_local_file') as mock_get_manifest: + # Setting the side_effect to None to simulate the absence of a manifest file + mock_get_manifest.side_effect = None + with pytest.raises(MetadataError): + metadata_resolver.from_tarball('image.tar', use_local_manifest=True, name='test_manifest') + + +def test_metadata_resolver_no_name_and_no_metadata_in_labels_for_remote(mock_registry_resolver, mock_docker_api): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + # Mocking the registry resolver's get_registry_for method to return a MagicMock + mock_registry_resolver.get_registry_for = MagicMock(return_value=Mock()) + with pytest.raises(TypeError): + metadata_resolver.from_registry('test-repository', '1.2.0') + + +def test_metadata_resolver_tarball_with_use_local_manifest_true(mock_registry_resolver, + mock_docker_api, + temp_manifest_dir): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + # Patching the get_manifest_from_local_file method to avoid FileNotFoundError + with patch('sonic_package_manager.manifest.Manifest.get_manifest_from_local_file') as mock_get_manifest: + # Setting the side_effect to None to simulate the absence of a manifest file + mock_get_manifest.side_effect = None + with pytest.raises(MetadataError): + metadata_resolver.from_tarball('image.tar', use_local_manifest=True) + + +def test_metadata_resolver_no_metadata_in_labels_for_tarball(mock_registry_resolver, mock_docker_api): + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + with pytest.raises(FileNotFoundError): + metadata_resolver.from_tarball('image.tar') + + +def test_metadata_resolver_local_with_name_and_use_edit(mock_registry_resolver, + mock_docker_api, + temp_manifest_dir, + sonic_fs): + with patch('builtins.open') as mock_open, \ + patch('json.loads') as mock_json_loads: + sonic_fs.create_dir(MANIFESTS_LOCATION) # Create the directory using sonic_fs fixture + mock_open.side_effect = FileNotFoundError # Simulate FileNotFoundError when opening the manifest file + mock_json_loads.side_effect = ValueError # Simulate ValueError when parsing JSON + + # Create the default manifest file + sonic_fs.create_file(DEFAULT_MANIFEST_FILE) + sonic_fs.create_file(os.path.join(MANIFESTS_LOCATION, "test_manifest.edit")) + + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + with pytest.raises(FileNotFoundError): + metadata_resolver.from_local('image', + use_local_manifest=True, + name='test_manifest', + use_edit=True) + + mock_open.assert_called_with(os.path.join(MANIFESTS_LOCATION, 'test_manifest.edit'), 'r') + mock_json_loads.assert_not_called() # Ensure json.loads is not called + + +def test_metadata_resolver_local_with_name_and_default_manifest(mock_registry_resolver, + mock_docker_api, + temp_manifest_dir, + sonic_fs): + with patch('builtins.open') as mock_open, \ + patch('json.loads') as mock_json_loads: + sonic_fs.create_dir(MANIFESTS_LOCATION) # Create the directory using sonic_fs fixture + mock_open.side_effect = FileNotFoundError # Simulate FileNotFoundError when opening the manifest file + mock_json_loads.side_effect = ValueError # Simulate ValueError when parsing JSON + + # Create the default manifest file + sonic_fs.create_file(DEFAULT_MANIFEST_FILE) + + metadata_resolver = MetadataResolver(mock_docker_api, mock_registry_resolver) + with pytest.raises(FileNotFoundError): + metadata_resolver.from_local('image', + use_local_manifest=False, + name='test_manifest', + use_edit=True) + + mock_open.assert_called_with(DEFAULT_MANIFEST_FILE, 'r') + mock_json_loads.assert_not_called() # Ensure json.loads is not called diff --git a/tests/test_sonic_installer.py b/tests/test_sonic_installer.py index 9e8438a7fc..66eb972fdf 100644 --- a/tests/test_sonic_installer.py +++ b/tests/test_sonic_installer.py @@ -86,6 +86,9 @@ def rootfs_path_mock(path): call(["sh", "-c", f"echo 'DOCKER_OPTS=\"$DOCKER_OPTS {' '.join(dockerd_opts)}\"' >> {mounted_image_folder}/etc/default/docker"]), # dockerd started with added options as host dockerd call(["chroot", mounted_image_folder, "/usr/lib/docker/docker.sh", "start"]), call(["cp", "/var/lib/sonic-package-manager/packages.json", f"{mounted_image_folder}/tmp/packages.json"]), + call(["mkdir", "-p", "/var/lib/sonic-package-manager/manifests"]), + call(["cp", "-arf", "/var/lib/sonic-package-manager/manifests", + f"{mounted_image_folder}/var/lib/sonic-package-manager"]), call(["touch", f"{mounted_image_folder}/tmp/docker.sock"]), call(["mount", "--bind", "/var/run/docker.sock", f"{mounted_image_folder}/tmp/docker.sock"]), call(["cp", f"{mounted_image_folder}/etc/resolv.conf", "/tmp/resolv.conf.backup"]),