Merge pull request #9 from truenas/refactor-app-version-validation

Refactor app version validation

apps_validation/validation/app_version.py

@@ -1,75 +1,47 @@
 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
 
 
 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

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': {

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',

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}'
+                    )

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)