diff --git a/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-hosted-pipeline.yml b/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-hosted-pipeline.yml index ce41ba578..a266edb04 100644 --- a/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-hosted-pipeline.yml +++ b/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-hosted-pipeline.yml @@ -536,6 +536,9 @@ spec: - name: source workspace: repository subPath: src + - name: base + workspace: repository + subPath: base - name: get-organization runAfter: diff --git a/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml b/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml index 1146ce021..94828344d 100644 --- a/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml +++ b/ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml @@ -293,6 +293,9 @@ spec: - name: source workspace: repository subPath: src + - name: base + workspace: repository + subPath: base - name: get-organization runAfter: diff --git a/ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml b/ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml index 4aa769271..4b5dadc79 100644 --- a/ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml +++ b/ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml @@ -20,6 +20,9 @@ spec: description: Identifier of certification project from Red Hat Connect workspaces: - name: source + - name: base + description: Clone of the repo at head of the base branch + optional: true steps: - name: certification-project-check image: "$(params.pipeline_image)" @@ -27,56 +30,17 @@ spec: script: | #! /usr/bin/env bash set -xe - echo "Checking availability of cert project identifier" - if [ "$(params.cert_project_required)" != "true" ]; then - echo "Cert project ID is not required." - echo -n "" | tee $(results.certification_project_id.path) - exit 0 + EXTRA_ARGS="" + if [[ "$(workspaces.base.bound)" == "true" ]]; then + EXTRA_ARGS+=" --repo-base-path $(workspaces.base.path)" fi - if [[ -z "$(params.affected_operators)" && -z "$(params.affected_catalog_operators)" ]]; then - echo "No operator is affected." - exit 1 - fi - - # Create dictionary of all affected operators to serve as a set - declare -A ALL_AFFECTED_OPERATORS - - # Add operators from affected_operators - if [ -n "$(params.affected_operators)" ]; then - IFS=',' read -ra affected_ops_array <<< "$(params.affected_operators)" - for op in "${affected_ops_array[@]}"; do - ALL_AFFECTED_OPERATORS["$op"]=1 - done - fi - - # Add operators from affected_catalog_operators - if [ -n "$(params.affected_catalog_operators)" ]; then - IFS=',' read -ra affected_catalog_ops_array <<< "$(params.affected_catalog_operators)" - for op in "${affected_catalog_ops_array[@]}"; do - operator_name=$(echo "$op" | cut -d'/' -f2) # Parse operator name from its catalog path - if [ -n "$operator_name" ]; then - ALL_AFFECTED_OPERATORS["$operator_name"]=1 - fi - done - fi - - - for operator_name in "${!ALL_AFFECTED_OPERATORS[@]}"; do - file_path="operators/$operator_name/ci.yaml" - - if [ ! -f "$file_path" ]; then - echo "File '$file_path' not found." - exit 1 - fi - - CERT_PROJECT_ID=$(cat "$file_path" | yq -r '.cert_project_id | select (.!=null)') - - if [ -z "$CERT_PROJECT_ID" ]; then - echo "Certification project ID is missing in '$file_path' file (cert_project_id)" - exit 1 - fi - done - - echo -n "$CERT_PROJECT_ID" | tee "$(results.certification_project_id.path)" + certification-project-check \ + --repo-head-path "$(workspaces.source.path)" \ + --affected-operators "$(params.affected_operators)" \ + --affected-catalog-operators "$(params.affected_catalog_operators)" \ + --cert-project-required "$(params.cert_project_required)" \ + --output-file "$(results.certification_project_id.path)" \ + --verbose \ + $EXTRA_ARGS diff --git a/docs/README.md b/docs/README.md index 6ddb4cd89..de90668ef 100644 --- a/docs/README.md +++ b/docs/README.md @@ -39,10 +39,12 @@ Depending on your use case, you may: To remove the operator completely from the catalog: -- Delete the your operator directory from the `operators/` folder. +- Delete your operator directory from the `operators/` folder. - Remove all catalog files related to your operator from the `catalogs/` directory. - Open a **single** pull request that includes these changes. Follow our contribution guidelines on how to [open a PR](users/contributing-via-pr.md). +Community and certified (ISV) operators follow the same removal steps above. You do not need to keep a `ci.yaml` stub in the pull request for certified operators. When the operator directory is removed in the PR, the ISV pipeline reads `cert_project_id` from the `ci.yaml` on the target branch to authorize and process the removal. + For reference, here’s an [example PR](https://github.com/redhat-openshift-ecosystem/community-operators-prod/pull/5955/files) demonstrating these steps. ### Remove the Operator from Specific Catalog Version(s) diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 6e6ff915e..eec41fc3f 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -346,7 +346,8 @@ python3 -m pip install ansible-lint To run unit tests and code style checkers: ```bash -tox +poetry run tox -p # all tests in paralel +poetry run tox -e bandit # single check ``` ### Local development diff --git a/operatorcert/entrypoints/certification_project_check.py b/operatorcert/entrypoints/certification_project_check.py new file mode 100644 index 000000000..bcccb3efa --- /dev/null +++ b/operatorcert/entrypoints/certification_project_check.py @@ -0,0 +1,194 @@ +"""Check certification project identifiers for affected operators.""" + +import argparse +import logging +import sys +from pathlib import Path + +from operatorcert.logger import setup_logger +from operatorcert.operator_repo.utils import load_yaml +from operatorcert.utils import SplitArgs + +LOGGER = logging.getLogger("operator-cert") + + +def setup_argparser() -> argparse.ArgumentParser: + """ + Setup argument parser + + Returns: + argparse.ArgumentParser: Argument parser + """ + parser = argparse.ArgumentParser( + description="Check certification project identifiers for affected operators." + ) + parser.add_argument( + "--repo-head-path", + required=True, + type=Path, + help="Path to the PR head repository clone", + ) + parser.add_argument( + "--repo-base-path", + type=Path, + help="Path to the base branch repository clone", + ) + parser.add_argument( + "--affected-operators", + default=[], + action=SplitArgs, + help="Comma separated list of affected operators", + ) + parser.add_argument( + "--affected-catalog-operators", + default=[], + action=SplitArgs, + help="Comma separated list of affected catalog operators", + ) + parser.add_argument( + "--cert-project-required", + default="true", + help="Whether a cert project ID must be present", + ) + parser.add_argument( + "--output-file", + help="Path to a file where the certification project ID will be stored", + ) + parser.add_argument("--verbose", action="store_true", help="Verbose output") + + return parser + + +def collect_affected_operators( + affected_operators: list[str], affected_catalog_operators: list[str] +) -> set[str]: + """ + Build a set of operator names affected by the pull request. + + Args: + affected_operators (list[str]): Operator names from detect-changes + affected_catalog_operators (list[str]): Catalog operators in catalog/version format + + Returns: + set[str]: Unique affected operator names + """ + operators = set(affected_operators) + for catalog_operator in affected_catalog_operators: + if not catalog_operator or "/" not in catalog_operator: + continue + _, operator_name = catalog_operator.split("/", 1) + if operator_name.strip(): + operators.add(operator_name.strip()) + return operators + + +def resolve_operator_ci_yaml_path( + source_root: Path, + base_root: Path | None, + operator_name: str, +) -> Path: + """ + Resolve the ci.yaml path for an operator, falling back to the base branch. + + Args: + source_root (Path): PR head repository root + base_root (Path | None): Base branch repository root + operator_name (str): Operator name + + Returns: + Path: Resolved ci.yaml path + + Raises: + FileNotFoundError: When ci.yaml cannot be found in head or base + """ + head_path = source_root / "operators" / operator_name / "ci.yaml" + if head_path.is_file(): + return head_path + + if base_root is not None: + base_path = base_root / "operators" / operator_name / "ci.yaml" + if base_path.is_file(): + LOGGER.info( + "Using ci.yaml from base branch for operator '%s' (removed in PR).", + operator_name, + ) + return base_path + + raise FileNotFoundError( + f"ci.yaml not found for operator '{operator_name}' in PR head or base branch" + ) + + +def check_certification_projects( + source_root: Path, + base_root: Path | None, + operator_names: set[str], +) -> str: + """ + Validate cert_project_id for all affected operators. + + Args: + source_root (Path): PR head repository root + base_root (Path | None): Base branch repository root + operator_names (set[str]): Operator names to check + + Returns: + str: Certification project ID from the last validated operator + + Raises: + ValueError: When no operators are affected or cert_project_id is missing + FileNotFoundError: When ci.yaml cannot be resolved + """ + if not operator_names: + raise ValueError("No operator is affected.") + + cert_project_id = "" + for operator_name in sorted(operator_names): + ci_path = resolve_operator_ci_yaml_path(source_root, base_root, operator_name) + config = load_yaml(ci_path) + project_id = (config or {}).get("cert_project_id") + if not project_id: + raise ValueError( + f"Certification project ID is missing in '{ci_path}' (cert_project_id)" + ) + cert_project_id = project_id + + return cert_project_id + + +def main() -> None: + """ + Main function of the script + """ + parser = setup_argparser() + args = parser.parse_args() + + log_level = "DEBUG" if args.verbose else "INFO" + setup_logger(level=log_level) + + if args.cert_project_required != "true": + LOGGER.info("Cert project ID is not required.") + if args.output_file: + Path(args.output_file).write_text("", encoding="utf-8") + return + + operator_names = collect_affected_operators( + args.affected_operators, args.affected_catalog_operators + ) + + try: + cert_project_id = check_certification_projects( + args.repo_head_path, + args.repo_base_path, + operator_names, + ) + except (FileNotFoundError, ValueError) as exc: + LOGGER.error("%s", exc) + sys.exit(1) + LOGGER.info("Certification project ID: %s", cert_project_id) + if args.output_file: + Path(args.output_file).write_text(cert_project_id, encoding="utf-8") + + +if __name__ == "__main__": # pragma: no cover + main() diff --git a/operatorcert/entrypoints/check_permissions.py b/operatorcert/entrypoints/check_permissions.py index 65868d2e9..6b5635189 100644 --- a/operatorcert/entrypoints/check_permissions.py +++ b/operatorcert/entrypoints/check_permissions.py @@ -562,9 +562,9 @@ def check_permissions( operators = operators.union( extract_operators_from_catalog(head_repo, affected_catalog_operators) ) - removed_catalog_operators = changes.get("removed_catalog_operators", []) + deleted_catalog_operators = changes.get("deleted_catalog_operators", []) operators = operators.union( - extract_operators_from_catalog(base_repo, removed_catalog_operators) + extract_operators_from_catalog(base_repo, deleted_catalog_operators) ) is_approved = [] diff --git a/operatorcert/utils.py b/operatorcert/utils.py index f2dd06c62..1babb5804 100644 --- a/operatorcert/utils.py +++ b/operatorcert/utils.py @@ -130,7 +130,14 @@ class SplitArgs(argparse.Action): def __call__( self, parser: Any, namespace: Any, values: Any, option_string: Any = None ) -> None: - setattr(namespace, self.dest, values.split(",")) + if not values or values.strip() == "": + setattr(namespace, self.dest, []) + else: + setattr( + namespace, + self.dest, + [v.strip() for v in values.split(",") if v.strip()], + ) def run_command( diff --git a/pyproject.toml b/pyproject.toml index 3dfe4373d..6e75499d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,6 +46,7 @@ build-scratch-catalog = "operatorcert.entrypoints.build_scratch_catalog:main" bulk-retrigger = "operatorcert.entrypoints.bulk_retrigger:main" bundle-dockerfile = "operatorcert.entrypoints.bundle_dockerfile:main" catalog-browser = "operatorcert.catalog.catalog_cli:main" +certification-project-check = "operatorcert.entrypoints.certification_project_check:main" check-permissions = "operatorcert.entrypoints.check_permissions:main" create-container-image = "operatorcert.entrypoints.create_container_image:main" create-github-gist = "operatorcert.entrypoints.create_github_gist:main" diff --git a/tests/entrypoints/test_certification_project_check.py b/tests/entrypoints/test_certification_project_check.py new file mode 100644 index 000000000..56fb913ad --- /dev/null +++ b/tests/entrypoints/test_certification_project_check.py @@ -0,0 +1,238 @@ +"""Tests for certification project check entrypoint.""" + +from pathlib import Path + +import pytest + +from operatorcert.entrypoints import certification_project_check + + +def test_collect_affected_operators() -> None: + result = certification_project_check.collect_affected_operators( + ["op-a", "op-b"], + ["v4.15/op-c", "v4.16/op-d"], + ) + assert result == {"op-a", "op-b", "op-c", "op-d"} + + +def test_collect_affected_operators_skips_invalid_catalog_paths() -> None: + result = certification_project_check.collect_affected_operators( + [], + ["invalid", "v4.15/op-c"], + ) + assert result == {"op-c"} + + +def test_main_ignores_empty_affected_operators_with_catalog_operators( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + operator_dir = tmp_path / "operators" / "catalog-op" + operator_dir.mkdir(parents=True) + (operator_dir / "ci.yaml").write_text( + "cert_project_id: cert-789\n", encoding="utf-8" + ) + + monkeypatch.setattr( + "sys.argv", + [ + "certification-project-check", + "--repo-head-path", + str(tmp_path), + "--affected-operators", + "", + "--affected-catalog-operators", + "v4.15/catalog-op", + "--cert-project-required", + "true", + ], + ) + + certification_project_check.main() + + assert "Certification project ID: cert-789" in capsys.readouterr().out + + +def test_resolve_operator_ci_yaml_path_from_head(tmp_path: Path) -> None: + operator_dir = tmp_path / "operators" / "my-operator" + operator_dir.mkdir(parents=True) + ci_file = operator_dir / "ci.yaml" + ci_file.write_text("cert_project_id: head-id\n", encoding="utf-8") + + result = certification_project_check.resolve_operator_ci_yaml_path( + tmp_path, None, "my-operator" + ) + + assert result == ci_file + + +def test_resolve_operator_ci_yaml_path_falls_back_to_base(tmp_path: Path) -> None: + base_root = tmp_path / "base" + head_root = tmp_path / "head" + base_ci = base_root / "operators" / "my-operator" / "ci.yaml" + base_ci.parent.mkdir(parents=True) + base_ci.write_text("cert_project_id: base-id\n", encoding="utf-8") + head_root.mkdir() + + result = certification_project_check.resolve_operator_ci_yaml_path( + head_root, base_root, "my-operator" + ) + + assert result == base_ci + + +def test_resolve_operator_ci_yaml_path_prefers_head_over_base(tmp_path: Path) -> None: + base_root = tmp_path / "base" + head_root = tmp_path / "head" + base_ci = base_root / "operators" / "my-operator" / "ci.yaml" + head_ci = head_root / "operators" / "my-operator" / "ci.yaml" + base_ci.parent.mkdir(parents=True) + head_ci.parent.mkdir(parents=True) + base_ci.write_text("cert_project_id: base-id\n", encoding="utf-8") + head_ci.write_text("cert_project_id: head-id\n", encoding="utf-8") + + result = certification_project_check.resolve_operator_ci_yaml_path( + head_root, base_root, "my-operator" + ) + + assert result == head_ci + + +def test_resolve_operator_ci_yaml_path_missing_in_head_and_base(tmp_path: Path) -> None: + base_root = tmp_path / "base" + head_root = tmp_path / "head" + base_root.mkdir() + head_root.mkdir() + + with pytest.raises(FileNotFoundError, match="ci.yaml not found"): + certification_project_check.resolve_operator_ci_yaml_path( + head_root, base_root, "my-operator" + ) + + +def test_check_certification_projects_uses_base_for_deleted_operator( + tmp_path: Path, +) -> None: + base_root = tmp_path / "base" + head_root = tmp_path / "head" + base_ci = base_root / "operators" / "removed-op" / "ci.yaml" + base_ci.parent.mkdir(parents=True) + base_ci.write_text("cert_project_id: cert-123\n", encoding="utf-8") + head_root.mkdir() + + result = certification_project_check.check_certification_projects( + head_root, + base_root, + {"removed-op"}, + ) + + assert result == "cert-123" + + +def test_check_certification_projects_missing_cert_project_id(tmp_path: Path) -> None: + operator_dir = tmp_path / "operators" / "my-operator" + operator_dir.mkdir(parents=True) + (operator_dir / "ci.yaml").write_text("fbc:\n enabled: true\n", encoding="utf-8") + + with pytest.raises(ValueError, match="Certification project ID is missing"): + certification_project_check.check_certification_projects( + tmp_path, + None, + {"my-operator"}, + ) + + +def test_check_certification_projects_no_operators(tmp_path: Path) -> None: + with pytest.raises(ValueError, match="No operator is affected"): + certification_project_check.check_certification_projects(tmp_path, None, set()) + + +def test_main_exits_on_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "sys.argv", + [ + "certification-project-check", + "--repo-head-path", + str(tmp_path), + "--affected-operators", + "missing-op", + "--cert-project-required", + "true", + ], + ) + + with pytest.raises(SystemExit) as exc_info: + certification_project_check.main() + + assert exc_info.value.code == 1 + + +def test_main_prints_cert_project_id_without_output_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + operator_dir = tmp_path / "operators" / "my-operator" + operator_dir.mkdir(parents=True) + (operator_dir / "ci.yaml").write_text( + "cert_project_id: cert-456\n", encoding="utf-8" + ) + + monkeypatch.setattr( + "sys.argv", + [ + "certification-project-check", + "--repo-head-path", + str(tmp_path), + "--affected-operators", + "my-operator", + "--cert-project-required", + "true", + ], + ) + + certification_project_check.main() + + assert "Certification project ID: cert-456" in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("cert_project_required", "expected_output"), + [ + ("false", ""), + ("true", "cert-123"), + ], +) +def test_main( + tmp_path: Path, + cert_project_required: str, + expected_output: str, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + operator_dir = tmp_path / "operators" / "my-operator" + operator_dir.mkdir(parents=True) + (operator_dir / "ci.yaml").write_text( + "cert_project_id: cert-123\n", encoding="utf-8" + ) + output_file = tmp_path / "cert_project_id.txt" + + monkeypatch.setattr( + "sys.argv", + [ + "certification-project-check", + "--repo-head-path", + str(tmp_path), + "--affected-operators", + "my-operator", + "--cert-project-required", + cert_project_required, + "--output-file", + str(output_file), + ], + ) + + certification_project_check.main() + + captured = capsys.readouterr() + if cert_project_required == "true": + assert f"Certification project ID: {expected_output}" in captured.out + + assert output_file.read_text(encoding="utf-8") == expected_output diff --git a/tests/entrypoints/test_check_permissions.py b/tests/entrypoints/test_check_permissions.py index 118d57cea..188127043 100644 --- a/tests/entrypoints/test_check_permissions.py +++ b/tests/entrypoints/test_check_permissions.py @@ -576,7 +576,7 @@ def test_check_permissions( "deleted_operators": ["operator3"], "added_catalog_operators": ["c1/operator4"], "modified_catalog_operators": ["c2/operator5"], - "removed_catalog_operators": ["c3/operator6"], + "deleted_catalog_operators": ["c3/operator6"], } head_repo.operator.side_effect = [ MagicMock(name="operator1"), diff --git a/tests/test_utils.py b/tests/test_utils.py index c88099741..ecf5766e3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,3 +1,4 @@ +import argparse import os import subprocess from pathlib import Path @@ -7,11 +8,28 @@ import pytest from operatorcert import utils -from operatorcert.utils import store_results +from operatorcert.utils import SplitArgs, store_results from requests import HTTPError, Session from requests.adapters import HTTPAdapter +@pytest.mark.parametrize( + ("values", "expected"), + [ + ("", []), + (" ", []), + ("op-a,op-b", ["op-a", "op-b"]), + (" op-a , op-b ", ["op-a", "op-b"]), + ("op-a,,op-b", ["op-a", "op-b"]), + ], +) +def test_split_args(values: str, expected: list[str]) -> None: + parser = argparse.ArgumentParser() + parser.add_argument("--items", action=SplitArgs) + namespace = parser.parse_args(["--items", values]) + assert namespace.items == expected + + def test_find_file(tmp_path: Path) -> None: tmp_path.joinpath("foo", "bar").mkdir(parents=True) tmp_path.joinpath("foo", "baz.txt").touch() diff --git a/tox.ini b/tox.ini index 4ccad7195..750e78907 100644 --- a/tox.ini +++ b/tox.ini @@ -18,16 +18,26 @@ envlist = test, skipsdist = True [testenv] +depends = .poetry-install deps = poetry poetry-plugin-export -commands_pre = - poetry install setenv = PYTHONPATH = {toxinidir} extras = dev +[testenv:.poetry-install] +description = install poetry dependencies once before parallel envs +deps = + poetry + poetry-plugin-export +skip_install = true +allowlist_externals = poetry +commands = poetry install +extras = dev + [testenv:test] +allowlist_externals = pytest commands = pytest -v \ --cov {[vars]OPERATOR_MODULE} \ --cov-report term-missing \ @@ -36,21 +46,26 @@ commands = pytest -v \ {posargs} [testenv:black] +allowlist_externals = black commands = black --check --diff . [testenv:black-format] +allowlist_externals = black commands = black . [testenv:mypy] +allowlist_externals = mypy commands = mypy {[vars]MYPY_SOURCE_ARGS} {[vars]OPERATOR_MODULE} mypy {[vars]MYPY_TEST_ARGS} {[vars]OPERATOR_TESTS} [testenv:pylint] +allowlist_externals = pylint commands = pylint {[vars]OPERATOR_MODULE} \ --min-similarity-lines 9 \ -d fixme [testenv:yamllint] +allowlist_externals = yamllint basepython = python3.13 files = . @@ -59,10 +74,11 @@ commands = [testenv:bandit] +allowlist_externals = bandit commands = bandit -r operatorcert -ll [testenv:pip-audit] -allowlist_externals = bash,python +allowlist_externals = bash,python,poetry commands = poetry export \ --without-hashes \ --format=requirements.txt -o /tmp/requirements.txt @@ -70,11 +86,12 @@ commands = poetry export \ # run pip audit in a way it can't exit with non-zero status bash ./local-dev/pip-audit.sh # pip-audit does not support not failing on unfixable vulnerabilities, this hacks around that - python ./local-dev/pip-audit-parse.py /tmp/audit-output.json + poetry run python ./local-dev/pip-audit-parse.py /tmp/audit-output.json [testenv:hadolint] allowlist_externals = hadolint +depends = deps = commands_pre = commands = hadolint --failure-threshold warning \ @@ -82,6 +99,7 @@ commands = hadolint --failure-threshold warning \ Dockerfile [testenv:pymarkdownlnt] +allowlist_externals = pymarkdown commands = pymarkdown scan . pymarkdown scan -r ansible docs