From 891361c184362d35db6776afb1226e7af25799d8 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 04:09:53 +0500 Subject: [PATCH 1/9] Add validation to make sure templates exist in an app version --- .../validation/validate_app_version.py | 4 +-- .../validation/validate_templates.py | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 apps_validation/validation/validate_templates.py diff --git a/apps_validation/validation/validate_app_version.py b/apps_validation/validation/validate_app_version.py index ebade9b..12b2f0e 100644 --- a/apps_validation/validation/validate_app_version.py +++ b/apps_validation/validation/validate_app_version.py @@ -12,6 +12,7 @@ from .ix_values import validate_ix_values_schema from .json_schema_utils import METADATA_JSON_SCHEMA, VERSION_VALIDATION_SCHEMA from .validate_questions import validate_questions_yaml +from .validate_templates import validate_templates WANTED_FILES_IN_ITEM_VERSION = { @@ -57,8 +58,7 @@ def validate_catalog_item_version( except ValidationErrors as v: verrors.extend(v) - # FIXME: We should be validating templates as well - # FIXME: We should be validating specified functions as well + validate_templates(version_path, f'{schema}.templates') # FIXME: values.yaml is probably not needed here for values_file in ['ix_values.yaml'] + (['values.yaml'] if validate_values else []): diff --git a/apps_validation/validation/validate_templates.py b/apps_validation/validation/validate_templates.py new file mode 100644 index 0000000..918f648 --- /dev/null +++ b/apps_validation/validation/validate_templates.py @@ -0,0 +1,27 @@ +import os +import pathlib + +from apps_validation.exceptions import ValidationErrors + + +def validate_templates(app_path: str, schema: str) -> None: + verrors = ValidationErrors() + templates_dir = pathlib.Path(os.path.join(app_path, 'templates')) + if not templates_dir.exists(): + verrors.add(schema, 'Templates directory does not exist') + elif not templates_dir.is_dir(): + verrors.add(schema, 'Templates is not a directory') + else: + # TODO: Validate library directory + found_compose_file = False + for entry in templates_dir.iterdir(): + if entry.name.endswith('.yaml'): + if not entry.is_file(): + verrors.add(schema, f'{entry.name!r} template file is not a file') + else: + found_compose_file = True + + if not found_compose_file: + verrors.add(schema, 'No template files found in templates directory') + + verrors.check() From d3799050d021650793fb10ff3296f6508bd7b614 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 05:40:37 +0500 Subject: [PATCH 2/9] Make sure library modules are validated --- apps_validation/validation/names.py | 1 + .../validation/validate_app_version.py | 2 + .../validation/validate_templates.py | 54 ++++++++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/apps_validation/validation/names.py b/apps_validation/validation/names.py index 859a7f6..4bb1b1a 100644 --- a/apps_validation/validation/names.py +++ b/apps_validation/validation/names.py @@ -3,3 +3,4 @@ RE_SCALE_VERSION = re.compile(r'^(\d{2}\.\d{2}(?:\.\d)*(?:-?(?:RC|BETA)\.?\d?)?)$') # 24.04 / 24.04.1 / 24.04-RC.1 RE_TRAIN_NAME = re.compile(r'^\w+[\w.-]*$') +TEST_VALUES_FILENAME = 'test_values.yaml' diff --git a/apps_validation/validation/validate_app_version.py b/apps_validation/validation/validate_app_version.py index 12b2f0e..95b939f 100644 --- a/apps_validation/validation/validate_app_version.py +++ b/apps_validation/validation/validate_app_version.py @@ -11,6 +11,7 @@ from .app_version import validate_app_version_file from .ix_values import validate_ix_values_schema from .json_schema_utils import METADATA_JSON_SCHEMA, VERSION_VALIDATION_SCHEMA +from .names import TEST_VALUES_FILENAME from .validate_questions import validate_questions_yaml from .validate_templates import validate_templates @@ -19,6 +20,7 @@ 'app.yaml', 'questions.yaml', 'README.md', + TEST_VALUES_FILENAME, } diff --git a/apps_validation/validation/validate_templates.py b/apps_validation/validation/validate_templates.py index 918f648..c3c6c55 100644 --- a/apps_validation/validation/validate_templates.py +++ b/apps_validation/validation/validate_templates.py @@ -1,7 +1,16 @@ import os import pathlib +import re from apps_validation.exceptions import ValidationErrors +from catalog_reader.app_utils import get_app_basic_details, get_values +from catalog_templating.render import render_templates + +from .names import TEST_VALUES_FILENAME + + +RE_APP_VERSION = re.compile(r'^v\d+_\d+_\d+$') +RE_BASE_LIB_VERSION = re.compile(r'^base_v\d+_\d+_\d+$') def validate_templates(app_path: str, schema: str) -> None: @@ -12,7 +21,13 @@ def validate_templates(app_path: str, schema: str) -> None: elif not templates_dir.is_dir(): verrors.add(schema, 'Templates is not a directory') else: - # TODO: Validate library directory + if (library_path := templates_dir / 'library').exists(): + if library_path.exists(): + if not library_path.is_dir(): + verrors.add(f'{schema}.library', 'Library is not a directory') + else: + validate_library(app_path, f'{schema}.library', verrors) + found_compose_file = False for entry in templates_dir.iterdir(): if entry.name.endswith('.yaml'): @@ -25,3 +40,40 @@ def validate_templates(app_path: str, schema: str) -> None: verrors.add(schema, 'No template files found in templates directory') verrors.check() + + +def validate_library(app_path: str, schema: str, verrors: ValidationErrors) -> None: + library_dir = pathlib.Path(os.path.join(app_path, 'templates/library')) + library_contents = list(library_dir.iterdir()) + if not library_contents: + return + # TODO: Validate lib version in app.yaml is correct and reflects reality on filesystem + elif len(library_contents) > 2: + verrors.add(schema, 'Library directory should only contain library version from the catalog or the app') + + app_config = get_app_basic_details(app_path) + app_library = pathlib.Path(os.path.join( + library_dir.name, app_config['train'], app_config['name'], f'v{app_config["version"].replace(".", "_")}' + )) + # We expect 2 paths here, one for the base library and one for the app library + for entry in library_contents: + if RE_BASE_LIB_VERSION.findall(entry.name): + if not entry.is_dir(): + verrors.add(schema, f'Base library {entry.name!r} is not a directory') + elif entry.name == app_config['train']: + if app_library.exists() and not app_library.is_dir(): + verrors.add(schema, f'App library {app_library.name!r} is not a directory') + else: + verrors.add(schema, f'Unexpected library found: {entry.name!r}') + + if verrors: + # If we have issues, no point in continuing further + return + + try: + rendered = render_templates(app_path, get_values(os.path.join(app_path, TEST_VALUES_FILENAME))) + except Exception as e: + verrors.add(schema, f'Failed to render templates: {e}') + else: + if not rendered: + verrors.add(schema, 'No templates were rendered') From 4668be2dc130c9f31de1ccbc2a86a9f44d60326e Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 05:43:17 +0500 Subject: [PATCH 3/9] Add a todo statement --- apps_validation/validation/app_version.py | 2 ++ apps_validation/validation/validate_templates.py | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps_validation/validation/app_version.py b/apps_validation/validation/app_version.py index 93a275d..2fab7cd 100644 --- a/apps_validation/validation/app_version.py +++ b/apps_validation/validation/app_version.py @@ -64,6 +64,8 @@ def validate_app_version_file( 'Configured version in "app.yaml" does not match version directory name.' ) + # TODO: Validate train and lib version please + else: verrors.add(schema, 'Missing app version file') diff --git a/apps_validation/validation/validate_templates.py b/apps_validation/validation/validate_templates.py index c3c6c55..bffe455 100644 --- a/apps_validation/validation/validate_templates.py +++ b/apps_validation/validation/validate_templates.py @@ -47,7 +47,6 @@ def validate_library(app_path: str, schema: str, verrors: ValidationErrors) -> N library_contents = list(library_dir.iterdir()) if not library_contents: return - # TODO: Validate lib version in app.yaml is correct and reflects reality on filesystem elif len(library_contents) > 2: verrors.add(schema, 'Library directory should only contain library version from the catalog or the app') From 36a07617eebea79921a84cff46c66da47e85b67d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 21:55:35 +0500 Subject: [PATCH 4/9] Add changes to bring in train name as well when trying to validate app.yaml --- apps_validation/validation/app_version.py | 1 + apps_validation/validation/validate_app.py | 6 ++++-- apps_validation/validation/validate_app_version.py | 4 ++-- apps_validation/validation/validate_catalog.py | 4 +--- apps_validation/validation/validate_train.py | 2 +- catalog_reader/app.py | 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/apps_validation/validation/app_version.py b/apps_validation/validation/app_version.py index 2fab7cd..941e76b 100644 --- a/apps_validation/validation/app_version.py +++ b/apps_validation/validation/app_version.py @@ -11,6 +11,7 @@ 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: diff --git a/apps_validation/validation/validate_app.py b/apps_validation/validation/validate_app.py index 6dae6f3..3bbaf2e 100644 --- a/apps_validation/validation/validate_app.py +++ b/apps_validation/validation/validate_app.py @@ -9,7 +9,7 @@ from .utils import validate_key_value_types -def validate_catalog_item(catalog_item_path: str, schema: str, validate_versions: bool = True): +def validate_catalog_item(catalog_item_path: str, schema: str, train_name: str, validate_versions: bool = True): # We should ensure that each catalog item has at least 1 version available # Also that we have item.yaml present verrors = ValidationErrors() @@ -57,7 +57,9 @@ def validate_catalog_item(catalog_item_path: str, schema: str, validate_versions for version_path in (versions if validate_versions else []): try: - validate_catalog_item_version(version_path, f'{schema}.versions.{os.path.basename(version_path)}') + validate_catalog_item_version( + version_path, f'{schema}.versions.{os.path.basename(version_path)}', train_name=train_name, + ) except ValidationErrors as e: verrors.extend(e) diff --git a/apps_validation/validation/validate_app_version.py b/apps_validation/validation/validate_app_version.py index 95b939f..7ba5f94 100644 --- a/apps_validation/validation/validate_app_version.py +++ b/apps_validation/validation/validate_app_version.py @@ -34,7 +34,7 @@ def validate_catalog_item_version_data(version_data: dict, schema: str, verrors: def validate_catalog_item_version( version_path: str, schema: str, version_name: typing.Optional[str] = None, - item_name: typing.Optional[str] = None, validate_values: bool = False, + item_name: typing.Optional[str] = None, validate_values: bool = False, train_name: typing.Optional[str] = None, ): verrors = ValidationErrors() version_name = version_name or os.path.basename(version_path) @@ -51,7 +51,7 @@ def validate_catalog_item_version( verrors.add(f'{schema}.required_files', f'Missing {", ".join(files_diff)} required configuration files.') app_version_path = os.path.join(version_path, 'app.yaml') - validate_app_version_file(verrors, app_version_path, schema, item_name, version_name) + validate_app_version_file(verrors, app_version_path, schema, item_name, version_name, train_name=train_name) questions_path = os.path.join(version_path, 'questions.yaml') if os.path.exists(questions_path): diff --git a/apps_validation/validation/validate_catalog.py b/apps_validation/validation/validate_catalog.py index f74eb19..56a13b9 100644 --- a/apps_validation/validation/validate_catalog.py +++ b/apps_validation/validation/validate_catalog.py @@ -50,8 +50,6 @@ def validate_catalog(catalog_path: str): except ValidationErrors as e: verrors.extend(e) - # FIXME: Validate library structure and files - # FIXME: Validate ix-dev trains_dir = get_train_path(catalog_path) if not os.path.exists(trains_dir): verrors.add('trains', 'Trains directory is missing') @@ -71,7 +69,7 @@ def validate_catalog(catalog_path: str): with concurrent.futures.ProcessPoolExecutor(max_workers=5 if len(items) > 10 else 2) as exc: for item in items: - item_futures.append(exc.submit(validate_catalog_item, item[0], item[1])) + item_futures.append(exc.submit(validate_catalog_item, item[0], item[1], item[2], True)) for future in item_futures: try: diff --git a/apps_validation/validation/validate_train.py b/apps_validation/validation/validate_train.py index de91dbf..6f6aff9 100644 --- a/apps_validation/validation/validate_train.py +++ b/apps_validation/validation/validate_train.py @@ -22,5 +22,5 @@ def get_train_items(train_path: str) -> typing.List[typing.Tuple[str, str]]: item_path = os.path.join(train_path, catalog_item) if not os.path.isdir(item_path): continue - items.append((item_path, f'trains.{train}.{catalog_item}')) + items.append((item_path, f'trains.{train}.{catalog_item}', train)) return items diff --git a/catalog_reader/app.py b/catalog_reader/app.py index d6de873..0c6383e 100644 --- a/catalog_reader/app.py +++ b/catalog_reader/app.py @@ -37,7 +37,7 @@ def get_app_details( schema = f'{train}.{item}' try: - validate_catalog_item(item_location, schema, False) + validate_catalog_item(item_location, schema, train, False) except ValidationErrors as verrors: item_data['healthy_error'] = f'Following error(s) were found with {item!r}:\n' for verror in verrors: From f8726192339a1b9c76e9501183643c312649e859 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 21:56:23 +0500 Subject: [PATCH 5/9] Validate correct train name is specified in app.yaml --- apps_validation/validation/app_version.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps_validation/validation/app_version.py b/apps_validation/validation/app_version.py index 941e76b..dcf0523 100644 --- a/apps_validation/validation/app_version.py +++ b/apps_validation/validation/app_version.py @@ -65,7 +65,10 @@ def validate_app_version_file( 'Configured version in "app.yaml" does not match version directory name.' ) - # TODO: Validate train and lib version please + # TODO: Validate lib version please + 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".') else: verrors.add(schema, 'Missing app version file') From 8b864efbb048dbb9c79fed011cd979244eeaff3d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 22:06:39 +0500 Subject: [PATCH 6/9] Make sure we validate train in app.yaml for ix-dev as well --- apps_validation/validation/validate_dev_directory.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps_validation/validation/validate_dev_directory.py b/apps_validation/validation/validate_dev_directory.py index 95b2f8d..aa18056 100644 --- a/apps_validation/validation/validate_dev_directory.py +++ b/apps_validation/validation/validate_dev_directory.py @@ -41,7 +41,7 @@ def validate_train(catalog_path: str, train_path: str, schema: str, to_check_app app_path = os.path.join(train_path, app_name) try: - validate_app(app_path, f'{schema}.{app_name}') + validate_app(app_path, f'{schema}.{app_name}', train_name) except ValidationErrors as ve: verrors.extend(ve) else: @@ -65,14 +65,16 @@ def validate_upgrade_strategy(app_path: str, schema: str, verrors: ValidationErr verrors.add(schema, f'{upgrade_strategy_path!r} is not executable') -def validate_app(app_dir_path: str, schema: str) -> None: +def validate_app(app_dir_path: str, schema: str, train_name: str) -> None: app_name = os.path.basename(app_dir_path) chart_version_path = os.path.join(app_dir_path, 'app.yaml') verrors = validate_app_version_file(ValidationErrors(), chart_version_path, schema, app_name) validate_keep_versions(app_dir_path, app_name, verrors) verrors.check() - validate_catalog_item_version(app_dir_path, schema, get_app_version(app_dir_path), app_name, True) + validate_catalog_item_version( + app_dir_path, schema, get_app_version(app_dir_path), app_name, True, train_name=train_name, + ) required_files = set(REQUIRED_METADATA_FILES) available_files = set( From 0f11022a9f3c5607de96ce81513addb6a6df3879 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 22:14:16 +0500 Subject: [PATCH 7/9] Make sure specified lib version actually exists --- apps_validation/validation/validate_app_version.py | 12 ++++++++++++ catalog_reader/app_utils.py | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/apps_validation/validation/validate_app_version.py b/apps_validation/validation/validate_app_version.py index 7ba5f94..4b06665 100644 --- a/apps_validation/validation/validate_app_version.py +++ b/apps_validation/validation/validate_app_version.py @@ -1,4 +1,5 @@ import os +import pathlib import typing import yaml @@ -6,6 +7,7 @@ from semantic_version import Version from apps_validation.exceptions import ValidationErrors +from catalog_reader.app_utils import get_app_basic_details from catalog_reader.questions_util import CUSTOM_PORTALS_KEY from .app_version import validate_app_version_file @@ -52,6 +54,16 @@ def validate_catalog_item_version( app_version_path = os.path.join(version_path, 'app.yaml') validate_app_version_file(verrors, app_version_path, schema, item_name, version_name, train_name=train_name) + app_basic_details = get_app_basic_details(version_path) + 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(".", "_")}') + ).exists(): + verrors.add( + f'{schema}.lib_version', + f'Specified {app_basic_details["lib_version"]!r} library version does not exist' + ) questions_path = os.path.join(version_path, 'questions.yaml') if os.path.exists(questions_path): diff --git a/catalog_reader/app_utils.py b/catalog_reader/app_utils.py index be0cfb2..d29d856 100644 --- a/catalog_reader/app_utils.py +++ b/catalog_reader/app_utils.py @@ -47,7 +47,9 @@ def get_app_basic_details(app_path: str) -> dict: with contextlib.suppress(FileNotFoundError, yaml.YAMLError, KeyError): with open(os.path.join(app_path, 'app.yaml'), 'r') as f: app_config = yaml.safe_load(f.read()) - return {k: app_config[k] for k in ('name', 'train', 'version', 'lib_version')} + return {'lib_version': app_config.get('lib_version')} | { + k: app_config[k] for k in ('name', 'train', 'version') + } return {} From 25a5f9aadc25930872f9b2225e8341b6d1cddf4a Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 22:17:01 +0500 Subject: [PATCH 8/9] Make sure we don't error out if no global lib is being used by app --- catalog_templating/render.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/catalog_templating/render.py b/catalog_templating/render.py index 5417b29..a8f3152 100644 --- a/catalog_templating/render.py +++ b/catalog_templating/render.py @@ -38,11 +38,11 @@ def render_templates(app_version_path: str, test_values: dict) -> dict: def import_library(library_path: str, app_config) -> dict: modules_context = collections.defaultdict(dict) # 2 dirs which we want to import from - global_base_lib = os.path.join(library_path, f'base_v{app_config["lib_version"].replace(".", "_")}') + global_base_lib = os.path.join(library_path, f'base_v{(app_config["lib_version"] or "").replace(".", "_")}') app_lib = os.path.join( library_path, app_config['train'], app_config['name'], f'v{app_config["version"].replace(".", "_")}' ) - if pathlib.Path(global_base_lib).is_dir(): + if app_config['lib_version'] and pathlib.Path(global_base_lib).is_dir(): modules_context['base'] = import_app_modules(global_base_lib, os.path.basename(global_base_lib)) # base_v1_0_0 if pathlib.Path(app_lib).is_dir(): modules_context[app_config['train']] = { From ba04e5a2599cca01924be6db38ad6bc31b8ffe44 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Date: Mon, 6 May 2024 22:18:00 +0500 Subject: [PATCH 9/9] Remove todo as it has been fixed --- apps_validation/validation/app_version.py | 1 - 1 file changed, 1 deletion(-) diff --git a/apps_validation/validation/app_version.py b/apps_validation/validation/app_version.py index dcf0523..1799651 100644 --- a/apps_validation/validation/app_version.py +++ b/apps_validation/validation/app_version.py @@ -65,7 +65,6 @@ def validate_app_version_file( 'Configured version in "app.yaml" does not match version directory name.' ) - # TODO: Validate lib version please 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".')