From bf4bd360b85180f359ad4ca0a20a411d8df89d95 Mon Sep 17 00:00:00 2001 From: Luiz Carvalho Date: Thu, 15 Oct 2020 17:06:08 -0400 Subject: [PATCH] Fix broken regeneration of YAML CSV ruamel will introduce a line break if the yaml line is longer than yaml.width. Unfortunately, this causes issues for JSON values nested within a YAML file, e.g. metadata.annotations."alm-examples" in a CSV file. The default value is 80. Set it to a more forgivinng higher number to avoid issues Signed-off-by: Luiz Carvalho --- omps/api/common.py | 18 +++++++++++++++++- omps/api/v1/push.py | 10 +++++++--- omps/constants.py | 6 ++++++ tests/api/test_common.py | 18 ++++++++++++++---- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/omps/api/common.py b/omps/api/common.py index 39662ec..2c7755b 100644 --- a/omps/api/common.py +++ b/omps/api/common.py @@ -10,6 +10,7 @@ from ruamel.yaml import YAML +from omps.constants import YAML_WIDTH from omps.errors import OMPSAuthorizationHeaderRequired @@ -64,7 +65,7 @@ def adjust_csv_annotations(quay_org, dir_path, context): if not quay_org.csv_annotations: return - yaml = YAML() + yaml = get_yaml_parser() # for filename in sorted(os.listdir(dir_path)): for filename in _yield_yaml_files(dir_path): with open(filename, 'r+') as f: @@ -90,3 +91,18 @@ def _yield_yaml_files(dir_path): fname_lower = fname.lower() if fname_lower.endswith('.yml') or fname_lower.endswith('.yaml'): yield os.path.join(root, fname) + + +def get_yaml_parser(): + """Returns an instance of the YAML parser with common settings + + :rtype: ruamel.yaml.YAML + :return: YAML parser + """ + yaml = YAML() + # IMPORTANT: ruamel will introduce a line break if the yaml line is longer than + # yaml.width. Unfortunately, this causes issues for JSON values nested within a + # YAML file, e.g. metadata.annotations."alm-examples" in a CSV file. The default + # value is 80. Set it to a more forgiving higher number to avoid issues + yaml.width = YAML_WIDTH + return yaml diff --git a/omps/api/v1/push.py b/omps/api/v1/push.py index 35f7e39..42b0f84 100644 --- a/omps/api/v1/push.py +++ b/omps/api/v1/push.py @@ -10,10 +10,14 @@ import zipfile from flask import jsonify, current_app, request -from ruamel.yaml import YAML from . import API -from omps.api.common import extract_auth_token, replace_registries, adjust_csv_annotations +from omps.api.common import ( + adjust_csv_annotations, + extract_auth_token, + get_yaml_parser, + replace_registries, +) from omps.constants import ( ALLOWED_EXTENSIONS, DEFAULT_ZIPFILE_MAX_UNCOMPRESSED_SIZE, @@ -173,7 +177,7 @@ def _process_package_name(quay_org, dir_path): If not found, or malformed, raise PackageValidationError. If package_name_suffix is configured, modify the packageName. """ - yaml = YAML() + yaml = get_yaml_parser() for filename in sorted(os.listdir(dir_path)): filename = os.path.join(dir_path, filename) if filename.endswith('.yaml') or filename.endswith('.yml'): diff --git a/omps/constants.py b/omps/constants.py index a656672..5ce40bc 100644 --- a/omps/constants.py +++ b/omps/constants.py @@ -15,3 +15,9 @@ ALLOWED_EXTENSIONS = {".zip", } KOJI_OPERATOR_MANIFESTS_ARCHIVE_KEY = 'operator_manifests_archive' + +# IMPORTANT: ruamel will introduce a line break if the yaml line is longer than +# yaml.width. Unfortunately, this causes issues for JSON values nested within a +# YAML file, e.g. metadata.annotations."alm-examples" in a CSV file. The default +# value is 80. Set it to a more forgiving higher number to avoid issues +YAML_WIDTH = 200 diff --git a/tests/api/test_common.py b/tests/api/test_common.py index 0aa422c..fb0bed2 100644 --- a/tests/api/test_common.py +++ b/tests/api/test_common.py @@ -5,9 +5,13 @@ import os -from ruamel.yaml import YAML - -from omps.api.common import replace_registries, adjust_csv_annotations, _yield_yaml_files +from omps.constants import YAML_WIDTH +from omps.api.common import ( + _yield_yaml_files, + adjust_csv_annotations, + get_yaml_parser, + replace_registries, +) from omps.quay import QuayOrganization @@ -61,9 +65,15 @@ def test_adjust_csv_annotations(datadir): adjust_csv_annotations(quay_org, dir_path, {'package_name': 'etcd'}) - yaml = YAML() + yaml = get_yaml_parser() for fpath in should_have_annotations: with open(fpath, 'r') as f: contents = yaml.load(f.read()) for name, value in expected_annotations.items(): assert contents['metadata']['annotations'][name] == value + + +def test_get_yaml_parser(): + """Test if yaml parser is configured correctly""" + yaml = get_yaml_parser() + assert yaml.width == YAML_WIDTH