From f2071c12d1bba0d0a9f357f890d2b8f3985f91a3 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 23:40:45 +0500 Subject: [PATCH 1/6] Add initial file to refactor app version file validation mess --- .../validation/app_version_refactor.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 apps_validation/validation/app_version_refactor.py diff --git a/apps_validation/validation/app_version_refactor.py b/apps_validation/validation/app_version_refactor.py new file mode 100644 index 0000000..9649a5e --- /dev/null +++ b/apps_validation/validation/app_version_refactor.py @@ -0,0 +1,28 @@ +import os +import yaml + +from typing import Optional + +from apps_validation.exceptions import ValidationErrors + + +def validate_app_version_file( + verrors: ValidationErrors, app_version_path: str, schema: str, item_name: str, version_name: Optional[str] = None, + train_name: Optional[str] = None, +) -> ValidationErrors: + if not os.path.exists(app_version_path): + verrors.add( + schema, f'App version file {app_version_path!r} does not exist' + ) + return verrors + + with open(app_version_path, 'r') as f: + try: + app_config = yaml.safe_load(f.read()) + except yaml.YAMLError: + verrors.add(schema, 'Must be a valid yaml file') + return verrors + + + + From eb3bdb478ed95eb99dee70d486c9a6006a773ea9 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 23:58:29 +0500 Subject: [PATCH 2/6] Add json schema for app metadata file --- .../validation/app_version_refactor.py | 4 -- .../validation/json_schema_utils.py | 40 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/apps_validation/validation/app_version_refactor.py b/apps_validation/validation/app_version_refactor.py index 9649a5e..e026421 100644 --- a/apps_validation/validation/app_version_refactor.py +++ b/apps_validation/validation/app_version_refactor.py @@ -22,7 +22,3 @@ def validate_app_version_file( except yaml.YAMLError: verrors.add(schema, 'Must be a valid yaml file') return verrors - - - - diff --git a/apps_validation/validation/json_schema_utils.py b/apps_validation/validation/json_schema_utils.py index 0645c8a..0fd96dc 100644 --- a/apps_validation/validation/json_schema_utils.py +++ b/apps_validation/validation/json_schema_utils.py @@ -1,3 +1,43 @@ +APP_METADATA_JSON_SCHEMA = { + 'type': 'object', + 'properties': { + 'name': {'type': 'string'}, + 'train': {'type': 'string'}, + 'annotations': { + 'type': 'object', + 'properties': { + 'min_scale_version': {'type': 'string'}, + 'max_scale_version': {'type': 'string'}, + }, + }, + 'sources': { + 'type': 'array', + 'items': {'type': 'string'}, + }, + 'maintainers': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'name': {'type': 'string'}, + 'email': {'type': 'string'}, + }, + 'required': ['name', 'email'], + }, + }, + 'version': { + 'type': 'string', + 'pattern': '[0-9]+.[0-9]+.[0-9]+', + }, + 'lib_version': { + 'type': 'string', + 'pattern': '[0-9]+.[0-9]+.[0-9]+', + }, + }, + 'required': [ + 'name', 'train', 'version', + ], +} APP_MIGRATION_SCHEMA = { 'type': 'array', 'items': { From 1801cae949d6332bf0a8aeaf81eba403610906ae Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Tue, 7 May 2024 00:01:54 +0500 Subject: [PATCH 3/6] Validate app config via defined json schema --- apps_validation/validation/app_version_refactor.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apps_validation/validation/app_version_refactor.py b/apps_validation/validation/app_version_refactor.py index e026421..2831b57 100644 --- a/apps_validation/validation/app_version_refactor.py +++ b/apps_validation/validation/app_version_refactor.py @@ -3,8 +3,12 @@ from typing import Optional +from jsonschema import validate as json_schema_validate, ValidationError as JsonValidationError + from apps_validation.exceptions import ValidationErrors +from .json_schema_utils import APP_METADATA_JSON_SCHEMA + def validate_app_version_file( verrors: ValidationErrors, app_version_path: str, schema: str, item_name: str, version_name: Optional[str] = None, @@ -22,3 +26,9 @@ def validate_app_version_file( except yaml.YAMLError: verrors.add(schema, 'Must be a valid yaml file') return verrors + + try: + json_schema_validate(app_config, APP_METADATA_JSON_SCHEMA) + except JsonValidationError as e: + verrors.add(schema, f'Failed to validate app version file: {e.message}') + return verrors From f9eaff2561b332313d40d8303f95abe09a09c18f Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Tue, 7 May 2024 00:16:01 +0500 Subject: [PATCH 4/6] Refactor implementation of validating apps file --- apps_validation/validation/app_version.py | 78 ++++++------------- .../validation/app_version_refactor.py | 34 -------- 2 files changed, 25 insertions(+), 87 deletions(-) delete mode 100644 apps_validation/validation/app_version_refactor.py diff --git a/apps_validation/validation/app_version.py b/apps_validation/validation/app_version.py index 1799651..e218b52 100644 --- a/apps_validation/validation/app_version.py +++ b/apps_validation/validation/app_version.py @@ -1,11 +1,13 @@ import os import yaml + from typing import Optional -from semantic_version import Version +from jsonschema import validate as json_schema_validate, ValidationError as JsonValidationError from apps_validation.exceptions import ValidationErrors +from .json_schema_utils import APP_METADATA_JSON_SCHEMA from .scale_version import validate_min_max_version_values @@ -13,63 +15,33 @@ def validate_app_version_file( verrors: ValidationErrors, app_version_path: str, schema: str, item_name: str, version_name: Optional[str] = None, train_name: Optional[str] = None, ) -> ValidationErrors: - if os.path.exists(app_version_path): - with open(app_version_path, 'r') as f: - try: - app_config = yaml.safe_load(f.read()) - except yaml.YAMLError: - verrors.add(schema, 'Must be a valid yaml file') - else: - if not isinstance(app_config, dict): - verrors.add(schema, 'Must be a dictionary') - else: - if app_config.get('name') != item_name: - verrors.add(f'{schema}.item_name', 'Item name not correctly set in "app.yaml".') - - if not isinstance(app_config.get('annotations', {}), dict): - verrors.add(f'{schema}.annotations', 'Annotations must be a dictionary') - elif app_config.get('annotations'): - validate_min_max_version_values(app_config['annotations'], verrors, schema) + if not os.path.exists(app_version_path): + verrors.add(schema, 'Missing app version file') + return verrors - if not isinstance(app_config.get('sources', []), list): - verrors.add(f'{schema}.sources', 'Sources must be a list') - else: - for index, source in enumerate(app_config.get('sources', [])): - if not isinstance(source, str): - verrors.add(f'{schema}.sources.{index}', 'Source must be a string') + with open(app_version_path, 'r') as f: + try: + app_config = yaml.safe_load(f.read()) + except yaml.YAMLError: + verrors.add(schema, 'Must be a valid yaml file') + return verrors - if not isinstance(app_config.get('maintainers', []), list): - verrors.add(f'{schema}.maintainers', 'Maintainers must be a list') - else: - for index, maintainer in enumerate(app_config.get('maintainers', [])): - if not isinstance(maintainer, dict): - verrors.add(f'{schema}.maintainers.{index}', 'Maintainer must be a dictionary') - elif not all(k in maintainer and isinstance(maintainer[k], str) for k in ('name', 'email')): - verrors.add( - f'{schema}.maintainers.{index}', - 'Maintainer must have name and email attributes defined and be strings.' - ) + try: + json_schema_validate(app_config, APP_METADATA_JSON_SCHEMA) + except JsonValidationError as e: + verrors.add(schema, f'Failed to validate app version file: {e.message}') + return verrors - app_version = app_config.get('version') - if app_version is None: - verrors.add(f'{schema}.version', 'Version must be configured in "app.yaml"') - else: - try: - Version(app_version) - except ValueError: - verrors.add(f'{schema}.version', f'{app_version!r} is not a valid version name') + if app_config.get('name') != item_name: + verrors.add(f'{schema}.item_name', 'Item name not correctly set in "app.yaml"') - if version_name is not None and app_version != version_name: - verrors.add( - f'{schema}.version', - 'Configured version in "app.yaml" does not match version directory name.' - ) + if app_config.get('annotations'): + validate_min_max_version_values(app_config['annotations'], verrors, schema) - if train_name is not None: - if app_config.get('train') != train_name: - verrors.add(f'{schema}.train', 'Train name not correctly set in "app.yaml".') + if version_name is not None and app_config['version'] != version_name: + verrors.add(f'{schema}.version', 'Version name does not match with the version name in the app version file') - else: - verrors.add(schema, 'Missing app version file') + if train_name is not None and app_config['train'] != train_name: + verrors.add(f'{schema}.train', 'Train name does not match with the train name in the app version file') return verrors diff --git a/apps_validation/validation/app_version_refactor.py b/apps_validation/validation/app_version_refactor.py deleted file mode 100644 index 2831b57..0000000 --- a/apps_validation/validation/app_version_refactor.py +++ /dev/null @@ -1,34 +0,0 @@ -import os -import yaml - -from typing import Optional - -from jsonschema import validate as json_schema_validate, ValidationError as JsonValidationError - -from apps_validation.exceptions import ValidationErrors - -from .json_schema_utils import APP_METADATA_JSON_SCHEMA - - -def validate_app_version_file( - verrors: ValidationErrors, app_version_path: str, schema: str, item_name: str, version_name: Optional[str] = None, - train_name: Optional[str] = None, -) -> ValidationErrors: - if not os.path.exists(app_version_path): - verrors.add( - schema, f'App version file {app_version_path!r} does not exist' - ) - return verrors - - with open(app_version_path, 'r') as f: - try: - app_config = yaml.safe_load(f.read()) - except yaml.YAMLError: - verrors.add(schema, 'Must be a valid yaml file') - return verrors - - try: - json_schema_validate(app_config, APP_METADATA_JSON_SCHEMA) - except JsonValidationError as e: - verrors.add(schema, f'Failed to validate app version file: {e.message}') - return verrors From 21e3a4abf9f4ba65ef67d399d733c14f8b323393 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Wed, 8 May 2024 18:07:12 +0500 Subject: [PATCH 5/6] Fix minor bugs --- apps_validation/validation/validate_app_version.py | 4 +++- catalog_templating/render.py | 5 +---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apps_validation/validation/validate_app_version.py b/apps_validation/validation/validate_app_version.py index 4b06665..0a3b5f3 100644 --- a/apps_validation/validation/validate_app_version.py +++ b/apps_validation/validation/validate_app_version.py @@ -58,7 +58,9 @@ def validate_catalog_item_version( if app_basic_details.get('lib_version') is not None: # Now we just want to make sure that actual directory for this lib version exists if not pathlib.Path( - os.path.join(version_path, 'library', f'v{app_basic_details["lib_version"].replace(".", "_")}') + os.path.join( + version_path, 'templates/library', f'base_v{app_basic_details["lib_version"].replace(".", "_")}' + ) ).exists(): verrors.add( f'{schema}.lib_version', diff --git a/catalog_templating/render.py b/catalog_templating/render.py index a8f3152..204b429 100644 --- a/catalog_templating/render.py +++ b/catalog_templating/render.py @@ -85,7 +85,4 @@ def import_module_context(module_name, file_path): def remove_pycache(library_path: str): - for modules in filter( - lambda p: os.path.exists(os.path.join(library_path, p, '__pycache__')), os.listdir(library_path) - ): - shutil.rmtree(os.path.join(library_path, modules, '__pycache__')) + shutil.rmtree(os.path.join(library_path, '__pycache__'), ignore_errors=True) From 17a4dbd6b005ef360547d827714ac6464e716ed6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Thu, 9 May 2024 15:34:28 +0500 Subject: [PATCH 6/6] Validate that rendered template is a valid yaml file --- apps_validation/validation/validate_templates.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps_validation/validation/validate_templates.py b/apps_validation/validation/validate_templates.py index bffe455..167f239 100644 --- a/apps_validation/validation/validate_templates.py +++ b/apps_validation/validation/validate_templates.py @@ -1,6 +1,7 @@ import os import pathlib import re +import yaml from apps_validation.exceptions import ValidationErrors from catalog_reader.app_utils import get_app_basic_details, get_values @@ -76,3 +77,11 @@ def validate_library(app_path: str, schema: str, verrors: ValidationErrors) -> N else: if not rendered: verrors.add(schema, 'No templates were rendered') + else: + for file_name, rendered_template in rendered.items(): + try: + yaml.safe_load(rendered_template) + except yaml.YAMLError as e: + verrors.add( + f'{schema}.{file_name}', f'Failed to verify rendered template is a valid yaml file: {e}' + )