Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-133616 / 25.04 / Add logic to validate app migrations #61

Merged
merged 1 commit into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
Loading