feat(ISV-7180): Fail static check when bundle attempts to replace itself

[JakubDurkac]

The rest of the problems defined by ISV-7180 was already fixed previously in #928 and #930.
Tested the check in my fork: JakubDurkac/community-operators-pipeline-preprod#2

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-code-review]

Review Summary by Qodo

Prevent bundles from replacing themselves in static checks

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add validation to prevent bundles from replacing themselves
• Fail static check when bundle's replaces version matches current version
• Add test case for self-replacement scenario

Diagram

flowchart LR
  A["Bundle replaces check"] --> B["Extract replaces version"]
  B --> C["Compare with current version"]
  C --> D{Versions match?}
  D -->|Yes| E["Fail with error message"]
  D -->|No| F["Continue validation"]

Loading

File Changes

1. operatorcert/static_tests/common/bundle.py ✨ Enhancement +7/-0

Add self-replacement detection in bundle validation

• Added validation logic to detect when a bundle attempts to replace itself
• Compares extracted replaces version with bundle's current CSV operator version
• Yields a Fail result with descriptive error message if versions match
• Returns early to prevent further validation when self-replacement is detected

operatorcert/static_tests/common/bundle.py

---

2. tests/static_tests/common/test_bundle.py 🧪 Tests +13/-0

Add test for bundle self-replacement validation

• Added new test case for bundle self-replacement scenario
• Tests case where bundle's replaces version equals its current version
• Verifies that appropriate Fail result is returned with correct error message
• Uses pytest.param with id "Bundle attempts to replace itself"

tests/static_tests/common/test_bundle.py

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 Hosted pipeline check_replaces_availability should handle errors gracefuly

1. Replaces split can IndexError 🐞 Bug ☼ Reliability

Description

check_replaces_availability blindly does replaces.split(...)[1], so a malformed spec.replaces
that lacks the expected delimiter will raise IndexError. The check runner converts uncaught
exceptions into a Fail whose message is the full traceback, leaking noisy internals instead of a
clear validation error.

Code

operatorcert/static_tests/common/bundle.py[R248-249]

    delimiter = ".v" if ".v" in replaces else "."
    replaces_version = replaces.split(delimiter, 1)[1]

Evidence

The function indexes [1] from the split result without verifying the delimiter exists, which can
raise IndexError; the runner wraps any exception and emits Fail(traceback.format_exc()), so the
traceback becomes the user-visible error message.

operatorcert/static_tests/common/bundle.py[245-256]
operatorcert/operator_repo/checks/init.py[157-175]

Agent prompt

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

### Issue description
`check_replaces_availability()` assumes `spec.replaces` always contains the expected delimiter and unconditionally does `split(...)[1]`. If the value is malformed, this raises `IndexError` and the framework surfaces a full traceback as the failure message.

### Issue Context
Static checks should emit actionable `Fail(...)` messages, not Python tracebacks.

### Fix Focus Areas
- operatorcert/static_tests/common/bundle.py[245-257]
- operatorcert/operator_repo/checks/__init__.py[157-175]

### Suggested fix
- Ensure `replaces` is a non-empty string.
- Parse it safely (e.g., via `partition`/`rpartition`) and validate you actually got a version component.
- On invalid format, `yield Fail("Invalid spec.replaces value ... expected <name>.v<version>")` and `return` (instead of throwing).

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

---

2. Replaces name not validated 🐞 Bug ≡ Correctness

Description

check_replaces_availability discards the operator name from spec.replaces and resolves the
replaced bundle using only the extracted version, which can select the wrong bundle if the CSV
references a different operator name. This can produce misleading failures (including the new
“replace itself” message) or miss the real configuration error (wrong replaces target name).

Code

operatorcert/static_tests/common/bundle.py[R248-260]

    delimiter = ".v" if ".v" in replaces else "."
    replaces_version = replaces.split(delimiter, 1)[1]

+    if replaces_version == bundle.csv_operator_version:
+        yield Fail(
+            f"{bundle} attempts to replace itself. Please set "
+            "'spec.replaces' field to a different version in the CSV."
+        )
+        return
+
    ver_to_dir = {
        x.csv_operator_version: x.operator_version
        for x in bundle.operator.all_bundles()

Evidence

The code parses only the version suffix from spec.replaces and then looks up a bundle solely by
that version within the current operator’s bundle set; the operator name component is available
(bundle.csv_operator_name) but never compared to the replaces prefix.

operatorcert/static_tests/common/bundle.py[245-271]
operatorcert/operator_repo/core.py[93-123]

Agent prompt

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

### Issue description
`check_replaces_availability()` ignores the operator name portion of `spec.replaces` and matches the replaced bundle only by version. If `spec.replaces` mistakenly points to a different operator CSV name, this check can validate against the wrong bundle (or emit a misleading error).

### Issue Context
`spec.replaces` is a CSV *name*; both name and version parts are meaningful. The code already has access to `bundle.csv_operator_name` and `bundle.csv_operator_version`.

### Fix Focus Areas
- operatorcert/static_tests/common/bundle.py[245-271]

### Suggested fix
- Parse `spec.replaces` into `(replaces_name, replaces_version)`.
- If `replaces_name != bundle.csv_operator_name`, `yield Fail(...)` explaining the mismatch and `return`.
- Keep using `replaces_version` for `ver_to_dir` lookup after name validation.

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

---

[qodo-code-review]

1. Replaces split can indexerror 🐞 Bug ☼ Reliability

check_replaces_availability blindly does replaces.split(...)[1], so a malformed spec.replaces
that lacks the expected delimiter will raise IndexError. The check runner converts uncaught
exceptions into a Fail whose message is the full traceback, leaking noisy internals instead of a
clear validation error.

Agent Prompt

### Issue description
`check_replaces_availability()` assumes `spec.replaces` always contains the expected delimiter and unconditionally does `split(...)[1]`. If the value is malformed, this raises `IndexError` and the framework surfaces a full traceback as the failure message.

### Issue Context
Static checks should emit actionable `Fail(...)` messages, not Python tracebacks.

### Fix Focus Areas
- operatorcert/static_tests/common/bundle.py[245-257]
- operatorcert/operator_repo/checks/__init__.py[157-175]

### Suggested fix
- Ensure `replaces` is a non-empty string.
- Parse it safely (e.g., via `partition`/`rpartition`) and validate you actually got a version component.
- On invalid format, `yield Fail("Invalid spec.replaces value ... expected <name>.v<version>")` and `return` (instead of throwing).

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

[qodo-code-review]

2. Replaces name not validated 🐞 Bug ≡ Correctness

check_replaces_availability discards the operator name from spec.replaces and resolves the
replaced bundle using only the extracted version, which can select the wrong bundle if the CSV
references a different operator name. This can produce misleading failures (including the new
“replace itself” message) or miss the real configuration error (wrong replaces target name).

Agent Prompt

### Issue description
`check_replaces_availability()` ignores the operator name portion of `spec.replaces` and matches the replaced bundle only by version. If `spec.replaces` mistakenly points to a different operator CSV name, this check can validate against the wrong bundle (or emit a misleading error).

### Issue Context
`spec.replaces` is a CSV *name*; both name and version parts are meaningful. The code already has access to `bundle.csv_operator_name` and `bundle.csv_operator_version`.

### Fix Focus Areas
- operatorcert/static_tests/common/bundle.py[245-271]

### Suggested fix
- Parse `spec.replaces` into `(replaces_name, replaces_version)`.
- If `replaces_name != bundle.csv_operator_name`, `yield Fail(...)` explaining the mismatch and `return`.
- Keep using `replaces_version` for `ver_to_dir` lookup after name validation.

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