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

Fix bug with FBC YAML Operators #655

Merged
merged 3 commits into from
Apr 17, 2024
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
32 changes: 31 additions & 1 deletion iib/workers/tasks/fbc_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
import os
import logging
import shutil

import json
from pathlib import Path
from typing import Tuple, List

import ruamel.yaml

from iib.exceptions import IIBError
from iib.workers.config import get_worker_config
from iib.common.tracing import instrument_tracing

log = logging.getLogger(__name__)
yaml = ruamel.yaml.YAML()


def is_image_fbc(image: str) -> bool:
Expand Down Expand Up @@ -81,6 +86,8 @@ def merge_catalogs_dirs(src_config: str, dest_config: str):
:param str src_config: source config directory
:param str dest_config: destination config directory
"""
from iib.workers.tasks.opm_operations import opm_validate

for conf_dir in (src_config, dest_config):
if not os.path.isdir(conf_dir):
msg = f"config directory does not exist: {conf_dir}"
Expand All @@ -89,6 +96,8 @@ def merge_catalogs_dirs(src_config: str, dest_config: str):

log.info("Merging config folders: %s to %s", src_config, dest_config)
shutil.copytree(src_config, dest_config, dirs_exist_ok=True)
enforce_json_config_dir(conf_dir)
opm_validate(conf_dir)


def extract_fbc_fragment(temp_dir: str, fbc_fragment: str) -> Tuple[str, List[str]]:
Expand Down Expand Up @@ -116,3 +125,24 @@ def extract_fbc_fragment(temp_dir: str, fbc_fragment: str) -> Tuple[str, List[st
raise IIBError("No operator packages in fbc_fragment %s", fbc_fragment)

return fbc_fragment_path, operator_packages


def enforce_json_config_dir(config_dir: str) -> None:
"""
Ensure the files from config dir are in JSON format.

It will walk recursively and convert any YAML files to the JSON format.

:param str config_dir: The config dir to walk recursively converting any YAML to JSON.
"""
log.info("Enforcing JSON content on config_dir: %s", config_dir)
for dirpath, _, filenames in os.walk(config_dir):
for file in filenames:
in_file = os.path.join(dirpath, file)
if in_file.lower().endswith(".yaml"):
out_file = os.path.join(dirpath, f"{Path(in_file).stem}.json")
log.debug(f"Converting {in_file} to {out_file}.")
with open(in_file, 'r') as yaml_in, open(out_file, 'w') as json_out:
data = yaml.load(yaml_in)
json.dump(data, json_out)
os.remove(in_file)
15 changes: 15 additions & 0 deletions iib/workers/tasks/opm_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ def opm_migrate(

run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to migrate index.db to file-based catalog')
log.info("Migration to file-based catalog was completed.")
opm_validate(fbc_dir_path)

if generate_cache:
# Remove outdated cache before generating new one
Expand Down Expand Up @@ -1118,6 +1119,20 @@ def deprecate_bundles(
run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to deprecate the bundles')


def opm_validate(config_dir: str) -> None:
"""
Validate the declarative config files in a given directory.

:param str config_dir: directory containing the declarative config files.
:raises IIBError: if the validation fails
"""
from iib.workers.tasks.utils import run_cmd

log.info("Validating files under %s", config_dir)
cmd = [Opm.opm_version, 'validate', config_dir]
run_cmd(cmd, exc_msg=f'Failed to validate the content from config_dir {config_dir}')


class Opm:
"""A class to store the opm version for the IIB operation."""

Expand Down
39 changes: 37 additions & 2 deletions tests/test_workers/test_tasks/test_fbc_utils.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
# SPDX-License-Identifier: GPL-3.0-or-later
import json
import os
import tempfile
from unittest import mock

import pytest
import ruamel.yaml

from iib.exceptions import IIBError
from iib.workers.config import get_worker_config
from iib.workers.tasks.fbc_utils import is_image_fbc, merge_catalogs_dirs, extract_fbc_fragment
from iib.workers.tasks.fbc_utils import (
is_image_fbc,
merge_catalogs_dirs,
enforce_json_config_dir,
extract_fbc_fragment,
)


yaml = ruamel.yaml.YAML()


@pytest.mark.parametrize(
Expand Down Expand Up @@ -92,7 +102,10 @@ def test_is_image_fbc(mock_si, skopeo_output, is_fbc):
assert is_image_fbc(image) is is_fbc


def test_merge_catalogs_dirs(tmpdir):
@mock.patch('iib.workers.tasks.opm_operations.Opm')
@mock.patch('iib.workers.tasks.utils.run_cmd')
@mock.patch("iib.workers.tasks.fbc_utils.enforce_json_config_dir")
def test_merge_catalogs_dirs(mock_enforce_json, mock_rc, mock_opm, tmpdir):
source_dir = os.path.join(tmpdir, 'src')
destination_dir = os.path.join(tmpdir, 'dst')
os.makedirs(destination_dir, exist_ok=True)
Expand All @@ -104,6 +117,11 @@ def test_merge_catalogs_dirs(tmpdir):
tempfile.NamedTemporaryFile(dir=operator_dir, delete=False)

merge_catalogs_dirs(src_config=source_dir, dest_config=destination_dir)
mock_enforce_json.assert_called_once_with(destination_dir)
mock_rc.called_once_with(
[mock_opm.opm_version, 'validate', destination_dir],
exc_msg=f'Failed to validate the content from config_dir {destination_dir}',
)

for r, d, f in os.walk(source_dir):

Expand Down Expand Up @@ -141,6 +159,23 @@ def test_merge_catalogs_dirs_raise(mock_isdir, mock_cpt, tmpdir):
mock_cpt.not_called()


def test_enforce_json_config_dir(tmpdir):
file_prefix = "test_file"
data = {"foo": "bar"}
test_file = os.path.join(tmpdir, f"{file_prefix}.yaml")
expected_file = os.path.join(tmpdir, f"{file_prefix}.json")
with open(test_file, 'w') as w:
yaml.dump(data, w)

enforce_json_config_dir(tmpdir)

assert os.path.isfile(expected_file)
assert not os.path.isfile(test_file)

with open(expected_file, 'r') as f:
assert json.load(f) == data


@pytest.mark.parametrize('ldr_output', [['testoperator'], ['test1', 'test2'], []])
@mock.patch('os.listdir')
@mock.patch('iib.workers.tasks.build._copy_files_from_image')
Expand Down
3 changes: 3 additions & 0 deletions tests/test_workers/test_tasks/test_opm_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ def test_serve_cmd_at_port_delayed_initialize(
assert mock_run_cmd.call_count == 7


@mock.patch('iib.workers.tasks.opm_operations.opm_validate')
@mock.patch('iib.workers.tasks.opm_operations.shutil.rmtree')
@mock.patch('iib.workers.tasks.opm_operations.generate_cache_locally')
@mock.patch('iib.workers.tasks.utils.run_cmd')
Expand All @@ -160,6 +161,7 @@ def test_opm_migrate(
mock_run_cmd,
mock_gcl,
moch_srmtree,
mock_opmvalidate,
tmpdir,
):
index_db_file = os.path.join(tmpdir, 'database/index.db')
Expand All @@ -175,6 +177,7 @@ def test_opm_migrate(
exc_msg='Failed to migrate index.db to file-based catalog',
)

mock_opmvalidate.assert_called_once_with(fbc_dir)
mock_gcl.assert_called_once_with(tmpdir, fbc_dir, mock.ANY)


Expand Down