Skip to content

Commit

Permalink
Add logic to validate app migrations
Browse files Browse the repository at this point in the history
  • Loading branch information
Qubad786 committed Jan 20, 2025
1 parent 5b0c2f0 commit 1473c0c
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 1 deletion.
33 changes: 33 additions & 0 deletions apps_validation/json_schema_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,36 @@
},
'additionalProperties': False
} # FIXME: See if all keys port
APP_CONFIG_MIGRATIONS_SCHEMA = {
'type': 'object',
'properties': {
'migrations': {
'type': 'array',
'items': {
'type': 'object',
'properties': {
'file': {'type': 'string'},
'from': {
'type': 'object',
'properties': {
'min_version': {'type': 'string', 'pattern': r'^\d+\.\d+\.\d+$'},
'max_version': {'type': 'string', 'pattern': r'^\d+\.\d+\.\d+$'}
},
'additionalProperties': True,
},
'target': {
'type': 'object',
'properties': {
'min_version': {'type': 'string', 'pattern': r'^\d+\.\d+\.\d+$'},
'max_version': {'type': 'string', 'pattern': r'^\d+\.\d+\.\d+$'}
},
'additionalProperties': True,
}
},
'required': ['file'],
'additionalProperties': True,
}
}
},
'required': ['migrations'],
}
96 changes: 95 additions & 1 deletion apps_validation/pytest/unit/test_app_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from apps_exceptions import ValidationErrors
from apps_validation.validate_app_version import WANTED_FILES_IN_ITEM_VERSION, validate_catalog_item_version
from apps_validation.validate_app import validate_catalog_item
from apps_validation.validate_migrations import validate_migration_config
from apps_validation.validate_questions import validate_questions_yaml, validate_variable_uniqueness
from apps_validation.validate_train import validate_train_structure

Expand Down Expand Up @@ -306,7 +307,7 @@ def test_validate_catalog_item(mocker, catalog_path, test_yaml, train, item_yaml
validate_catalog_item(catalog_path, 'test_schema', train)


@pytest.mark.parametrize('version_path ,app_yaml, schema, required_files, should_work', [
@pytest.mark.parametrize('version_path, app_yaml, schema, required_files, should_work', [
(
'/mnt/mypool/ix-applications/catalogs/github_com_truenas_charts_git_master/charts/storj/1.0.4',
'''
Expand Down Expand Up @@ -385,6 +386,7 @@ def test_validate_catalog_item_version(mocker, version_path, app_yaml, schema, r
mocker.patch('apps_validation.validate_app_version.validate_questions_yaml', return_value=None)
mocker.patch('apps_validation.validate_app_version.validate_ix_values_yaml', return_value=None)
mocker.patch('apps_validation.validate_app_version.validate_templates', return_value=None)
mocker.patch('apps_validation.validate_app_version.validate_migration_config', return_value=None)
mocker.patch('apps_validation.validate_app_version.validate_k8s_to_docker_migrations', return_value=None)

if should_work:
Expand Down Expand Up @@ -451,3 +453,95 @@ def test_validate_variable_uniqueness(data, schema, should_work):
else:
with pytest.raises(ValidationErrors):
validate_variable_uniqueness(data, schema, verrors)


@pytest.mark.parametrize('yaml_data, should_work', [
(
'''
migrations:
- file: always.py
# Should run for any current/target combination
- file: only_min_version_from.py
from:
min_version: 1.0.0
''',
True
),
(
'''
migrations:
- file: always.py
# Should run for any current/target combination
- file: only_min_version_from.py
target:
min_version: 1.0.0
max_version: 2.0.0
''',
True
),
(
'''
migrations:
- file: always.py
# Should run for any current/target combination
- file: only_min_version_from.py
from:
min_version: 1.0.0
''',
True
),
(
'''
migrations:
- file: always.py
# Should run for any current/target combination
''',
True
),
(
'''
migrations
- file: always.py
# Should run for any current/target combination
- file: only_min_version_from.py
from:
min_version: 1.0.0
''',
False
),
(
'''
migrations:
''',
False
),
(
'''
''',
False
),
(
'''
migrations
- file: only_min_version_from.py
from:
''',
False
),
])
def test_validate_migrations_yaml(mocker, yaml_data, should_work):
mock_file = mocker.mock_open(read_data=yaml_data)
mocker.patch('builtins.open', mock_file)
if should_work:
assert validate_migration_config('/path/to/app_migrations.yaml', 'app_migration_config') is None
else:
with pytest.raises(ValidationErrors):
validate_migration_config('/path/to/app_migrations.yaml', 'app_migration_config')
19 changes: 19 additions & 0 deletions apps_validation/validate_app_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from .ix_values import validate_ix_values_schema
from .json_schema_utils import VERSION_VALIDATION_SCHEMA
from .validate_k8s_to_docker_migration import validate_k8s_to_docker_migrations
from .validate_migrations import validate_migration_config, validate_migration_file, get_migration_file_names
from .validate_questions import validate_questions_yaml
from .validate_templates import validate_templates

Expand Down Expand Up @@ -73,12 +74,30 @@ def validate_catalog_item_version(
verrors.add(f'{schema}.lib_version', 'Library version hash does not match with the actual library version')

questions_path = os.path.join(version_path, 'questions.yaml')
migrations_yaml_path = os.path.join(version_path, 'app_migrations.yaml')
app_migrations_dir = os.path.join(version_path, 'migrations')
if os.path.exists(questions_path):
try:
validate_questions_yaml(questions_path, f'{schema}.questions_configuration')
except ValidationErrors as v:
verrors.extend(v)

# Validating structure of app_migrations YAML
if os.path.exists(migrations_yaml_path):
try:
validate_migration_config(migrations_yaml_path, f'{schema}.migrations_configuration')
except ValidationErrors as v:
verrors.extend(v)

# Validating actual migration file
if os.path.isdir(app_migrations_dir) and os.path.exists(migrations_yaml_path):
try:
for filename in get_migration_file_names(migrations_yaml_path, f'{schema}.migrations_configuration'):
migration_file_path = os.path.join(app_migrations_dir, filename)
validate_migration_file(migration_file_path, f'{schema}.migration_file.{filename}')
except ValidationErrors as v:
verrors.extend(v)

validate_templates(version_path, f'{schema}.templates', verrors)

# FIXME: values.yaml is probably not needed here
Expand Down
57 changes: 57 additions & 0 deletions apps_validation/validate_migrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import os
import yaml

from jsonschema import validate, ValidationError as JsonValidationError

from apps_exceptions import ValidationErrors

from .json_schema_utils import APP_CONFIG_MIGRATIONS_SCHEMA


def get_migration_file_names(migration_yaml_path: str, schema: str) -> list[str]:
verrors = ValidationErrors()
try:
with open(migration_yaml_path, 'r') as f:
yaml_data = yaml.safe_load(f)
# Collect file names from the migrations
files = [migration['file'] for migration in yaml_data['migrations']]
return files
except FileNotFoundError:
return []
except yaml.YAMLError:
verrors.add(f'{schema}.yaml_file', 'Must be a valid YAML file')

verrors.check()


def validate_migration_config(migration_yaml_path: str, schema: str):
verrors = ValidationErrors()
with open(migration_yaml_path, 'r') as f:
try:
data = yaml.safe_load(f)
except yaml.YAMLError:
verrors.add(f'{schema}.yaml_file', 'Must be a valid YAML file')

verrors.check()

try:
validate(instance=data, schema=APP_CONFIG_MIGRATIONS_SCHEMA)
except JsonValidationError as e:
verrors.add(f'{schema}', f'Invalid format specified for app migrations: {e.message}')

verrors.check()


def validate_migration_file(migration_file_path: str, schema: str):
verrors = ValidationErrors()
if not os.path.isfile(migration_file_path):
verrors.add(schema, f"{migration_file_path} is not a valid file")
else:
if not os.access(migration_file_path, os.X_OK):
verrors.add(schema, f"{migration_file_path} is not executable")

with open(migration_file_path, 'r') as r:
if not r.readline().startswith('#!'):
verrors.add(schema, 'Migration file should start with shebang line')

verrors.check()

0 comments on commit 1473c0c

Please sign in to comment.