feat(ISV-7356): certProject check fallback to base branch on operator removal

[mantomas]

• moved the cert-project-check to separate Python module
• fallback to use base branch ci.yaml when operator is deleted in the PR
• fix invalid reference in check-permissions deleted_catalog_operators vs removed_catalog_operators
• cosmetic changes to tox.ini and docs as I was unable run it locally after the change to Poetry

Closes ISV-7356

Example ISV operator remove PR is here: redhat-openshift-ecosystem/operator-pipelines-test#3711 (comment)

AI-assisted-by: Cursor

Merge Request Checklists

• Development is done in feature branches
• Code changes are submitted as pull request into a primary branch [Provide reason for non-primary branch submissions]
• Code changes are covered with unit and integration tests.
• Code passes all automated code tests:
  • Linting
  • Code formatter - Black
  • Security scanners
  • Unit tests
  • Integration tests
• Code is reviewed by at least 1 team member
• Pull request is tagged with "risk/good-to-go" label for minor changes

[qodo-redhat-openshift-ecosystem]

PR Summary by Qodo

Fallback certProject validation to base branch when operator is removed

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

AI Description

• Add base-branch repo workspace to pipelines to support operator removal validation.
• Replace inline bash cert-project check with a Python CLI entrypoint and fallback logic.
• Update tox/Poetry workflow, tests, and docs to match new behavior.

Diagram

graph TD
P["Tekton pipeline"] --> T["cert-project-check task"] --> C["certification-project-check (Python CLI)"] --> R["cert_project_id result"]
H["repo head workspace (src)"] --> C
B["repo base workspace (base)"] --> C
C --> Y["operators/*/ci.yaml (head or base)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep bash/yq logic inside Tekton task

• ➕ No new Python entrypoint / packaging changes
• ➕ All logic remains visible in the task YAML
• ➖ Harder to unit test and evolve safely
• ➖ More complex shell logic for head/base fallback and parsing
• ➖ Continues duplication risk across tasks/pipelines

2. Require a ci.yaml stub to remain in PRs that delete an operator

• ➕ No need for base-branch checkout in pipeline workspaces
• ➕ Simplifies the check to only read from PR head
• ➖ Worsens contributor UX for removals (extra required artifact)
• ➖ Easy to get wrong; policy becomes harder to explain/enforce
• ➖ Doesn't generalize to other checks that may need base-state context

Recommendation: Current approach is the best tradeoff: moving the logic into a Python entrypoint makes the behavior testable and maintainable, and the optional base-workspace enables correct authorization checks during operator deletion without imposing extra contributor requirements.

Files changed (11) +454 / -59

Enhancement (2) +208 / -50

cert-project-check.ymlReplace inline bash cert-project check with Python CLI + base fallback +14/-50

Replace inline bash cert-project check with Python CLI + base fallback

• Introduces an optional 'base' workspace and forwards both head and base repo paths to a new certification-project-check command. Removes the previous inlined bash/yq implementation and delegates validation/output handling to the Python entrypoint.

ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml

certification_project_check.pyNew certification-project-check entrypoint with head→base ci.yaml resolution +194/-0

New certification-project-check entrypoint with head→base ci.yaml resolution

• Adds a new Python CLI that collects affected operators, resolves each operator's ci.yaml from PR head with optional fallback to base branch, validates cert_project_id presence, and writes the result to stdout and/or an output file. Includes verbose logging and consistent non-zero exit behavior on validation failures.

operatorcert/entrypoints/certification_project_check.py

Bug fix (1) +2 / -2

check_permissions.pyAlign catalog-operator deletion key name with changes payload +2/-2

Align catalog-operator deletion key name with changes payload

• Renames usage from 'removed_catalog_operators' to 'deleted_catalog_operators' when extracting operators from the base repo. Keeps permission checking consistent with the updated change-detection contract used elsewhere.

operatorcert/entrypoints/check_permissions.py

Tests (2) +210 / -1

test_certification_project_check.pyAdd unit coverage for certification-project-check entrypoint and fallback logic +209/-0

Add unit coverage for certification-project-check entrypoint and fallback logic

• Introduces tests for affected-operator collection, ci.yaml path resolution (head preferred, base fallback), missing cert_project_id handling, and main() behavior (stdout, output file writing, and exit-on-error). Ensures operator deletion scenarios remain authorized via base-branch ci.yaml.

tests/entrypoints/test_certification_project_check.py

test_check_permissions.pyUpdate test fixture to use deleted_catalog_operators key +1/-1

Update test fixture to use deleted_catalog_operators key

• Adjusts the check_permissions test input payload to match the renamed 'deleted_catalog_operators' field. Keeps tests aligned with the entrypoint behavior.

tests/entrypoints/test_check_permissions.py

Documentation (2) +5 / -2

README.mdDocument certified operator removal behavior with base-branch ci.yaml fallback +3/-1

Document certified operator removal behavior with base-branch ci.yaml fallback

• Clarifies removal instructions and explicitly documents that the ISV pipeline can read cert_project_id from the target branch when the operator directory is deleted in the PR. Fixes minor wording in the removal checklist.

docs/README.md

developer-guide.mdUpdate local tox invocation examples for Poetry-based workflow +2/-1

Update local tox invocation examples for Poetry-based workflow

• Replaces the plain 'tox' example with 'poetry run tox' commands and adds a single-env example. This aligns docs with the updated tox/Poetry setup.

docs/developer-guide.md

Other (4) +29 / -4

operator-hosted-pipeline.ymlMount optional base repo workspace for downstream tasks +3/-0

Mount optional base repo workspace for downstream tasks

• Adds an additional workspace mount (subPath: base) alongside the existing src mount. This enables tasks to access a base-branch checkout when needed (e.g., operator removal scenarios).

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-hosted-pipeline.yml

operator-release-pipeline.ymlMount optional base repo workspace for downstream tasks +3/-0

Mount optional base repo workspace for downstream tasks

• Adds the base workspace mount (subPath: base) to the release pipeline in addition to the src mount. This supports checks that need base-branch state when files are removed in the PR.

ansible/roles/operator-pipeline/templates/openshift/pipelines/operator-release-pipeline.yml

pyproject.tomlExpose certification-project-check as an installable CLI script +1/-0

Expose certification-project-check as an installable CLI script

• Registers the new entrypoint under [project.scripts] so Tekton tasks (and developers) can invoke it as 'certification-project-check'.

pyproject.toml

tox.iniMake tox compatible with Poetry installs and parallel env execution +22/-4

Make tox compatible with Poetry installs and parallel env execution

• Adds a dedicated .poetry-install env and makes other envs depend on it, avoiding repeated poetry installs under parallel runs. Updates allowlist_externals entries and runs pip-audit parsing via poetry to ensure dependencies and interpreter context are consistent.

tox.ini

[qodo-redhat-openshift-ecosystem]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 15 rules

✅ Skills: running-tests-and-quality-gates

1. Empty operator name included ✓ Resolved 🐞 Bug ≡ Correctness

Description

collect_affected_operators() turns the parsed --affected-operators list into a set without
filtering empty strings, so when Tekton passes an empty string it can try to resolve
operators//ci.yaml and fail even if valid catalog operators exist. This can break the
hosted/release pipelines for PRs where affected_operators is empty but
affected_catalog_operators is non-empty.

Code

operatorcert/entrypoints/certification_project_check.py[R62-82]

+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 in catalog_operator:
+            continue
+        _, operator_name = catalog_operator.split("/", 1)
+        if operator_name:
+            operators.add(operator_name)
+    return operators

Relevance

⭐⭐⭐ High

Team often accepts guard/robustness fixes preventing pipeline crashes (e.g., missing key/arg
formatting fixes in PRs #979, #859).

PR-#979
PR-#859

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Tekton serializes an empty affected_operators list to an empty string and always passes it to the
new entrypoint; the entrypoint’s SplitArgs/collect logic does not filter empty values, which can
produce an operator_name=="" and an invalid ci.yaml lookup.

ansible/roles/operator-pipeline/templates/openshift/tasks/parse-repo-changes.yml[91-96]
ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml[39-46]
operatorcert/utils.py[125-134]
operatorcert/entrypoints/certification_project_check.py[62-82]
operatorcert/entrypoints/certification_project_check.py[104-119]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `certification-project-check` entrypoint can incorrectly include an empty operator name (`""`) in the affected operator set when Tekton passes `--affected-operators ""`. This happens because `SplitArgs` naively splits the empty string into `[""]`, and `collect_affected_operators()` does not filter falsy/whitespace-only entries.

Impact: for catalog-only changes (or any PR where `.affected_operators` is an empty list), the pipeline can fail with a misleading “ci.yaml not found for operator ''” even though the real affected operators are present in `affected_catalog_operators`.

## Issue Context
Tekton’s `parse-repo-changes` task writes `affected_operators` using `jq ... | join(",")`, which produces an empty string when the list is empty, and `cert-project-check` always passes that value as `--affected-operators`.

## Fix Focus Areas
- operatorcert/utils.py[125-134]
- operatorcert/entrypoints/certification_project_check.py[62-82]
- ansible/roles/operator-pipeline/templates/openshift/tasks/parse-repo-changes.yml[91-96]
- ansible/roles/operator-pipeline/templates/openshift/tasks/cert-project-check.yml[39-46]

## How to fix
Implement one of (or both) of the following:
1) Make `SplitArgs` treat empty/whitespace input as an empty list and strip/filter items:
  - if `values.strip() == ""`: set `[]`
  - else: set `[v.strip() for v in values.split(',') if v.strip()]`
2) Defensive filtering in `collect_affected_operators()`:
  - build `operators = {op.strip() for op in affected_operators if op and op.strip()}`

Add/extend unit tests to cover the case where `--affected-operators ""` is provided while `--affected-catalog-operators` contains valid values, ensuring the empty entry is ignored and the check succeeds/fails based only on real operator names.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[kosciCZ]

LGTM, thanks for addressing my comments!