feat(RELEASE-1989): skopeo helper client

[midnightercz]

This commit implements skopeo client supporting copy and inspect operations. Also it provides fake skopeo client which can be injected instead of real skopeo client during tests

[qodo-code-review]

Review Summary by Qodo

(Agentic_describe updated until commit ccaf73b)

Implement skopeo client with fake testing support

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Implement SkopeoClient wrapper for container image operations
  - Supports inspect and copy operations with full parameter coverage
  - Handles credentials via Secret objects and TLS verification
• Create FakeSkopeoClient for testing with YAML-based mock configuration
  - Loads mock responses from environment variable config file
  - Supports exact matching and regex patterns for flexible test scenarios
  - Validates configuration at load time with detailed error messages
• Provide comprehensive test coverage for both real and fake implementations
  - 684 lines of tests for SkopeoClient covering all operations and edge cases
  - 763 lines of tests for FakeSkopeoClient covering validation and matching logic
• Include documentation and example configuration for mock setup

Diagram

flowchart LR
  A["SkopeoClient"] -->|wraps| B["skopeo CLI"]
  A -->|executes| C["subprocess.run"]
  C -->|returns| D["SkopeoClientError or result"]
  E["FakeSkopeoClient"] -->|loads config from| F["YAML file"]
  F -->|defines rules| G["inspect/copy operations"]
  G -->|matches against| H["actual parameters"]
  H -->|returns mock| I["response or error"]
  J["patch_skopeo_client"] -->|monkey-patches| A
  J -->|replaces with| E

Loading

File Changes

1. scripts/python/helpers/skopeo.py ✨ Enhancement +354/-0

Real skopeo client wrapper implementation

• Implements SkopeoClient class wrapping skopeo CLI commands
• Provides inspect() method returning dict or formatted string based on format parameter
• Provides copy() method for image copying with comprehensive parameter support
• Handles Secret objects for credentials with automatic unveiling
• Raises SkopeoClientError with detailed command and output information on failures
• Supports global flags like debug, insecure-policy, tmpdir, and architecture overrides

scripts/python/helpers/skopeo.py

---

2. scripts/python/helpers/fake/__init__.py ✨ Enhancement +17/-0

Fake client module initialization and exports

• Exports FakeSkopeoClient for test injection
• Provides patch_skopeo_client() function for monkey-patching
• Enables drop-in replacement of real SkopeoClient during testing

scripts/python/helpers/fake/init.py

---

3. scripts/python/helpers/fake/skopeo.py ✨ Enhancement +371/-0

Fake skopeo client with YAML-based mock configuration

• Implements FakeSkopeoClient with YAML configuration loading
• Supports inspect() and copy() operations with matching against mock rules
• Validates YAML structure at load time with comprehensive error checking
• Implements flexible matching logic supporting exact matches and regex patterns
• Handles Secret objects by ignoring them in matching logic
• Provides detailed error messages when no matching rule found
• Supports first-match-wins rule evaluation strategy

scripts/python/helpers/fake/skopeo.py

---

View more (4)

4. scripts/python/helpers/test_skopeo.py 🧪 Tests +684/-0

Unit tests for real skopeo client implementation

• Comprehensive unit tests for SkopeoClient covering initialization and global flags
• Tests for inspect() operation with various parameter combinations
• Tests for copy() operation with source/destination credentials and TLS options
• Tests for error handling and JSON parsing failures
• Tests for Secret object handling and command building
• Validates boolean flag handling and parameter conversion to CLI arguments

scripts/python/helpers/test_skopeo.py

---

5. scripts/python/helpers/fake/test_fake_skopeo.py 🧪 Tests +763/-0

Unit tests for fake skopeo client implementation

• Tests for FakeSkopeoClient initialization and environment variable requirements
• Tests for YAML configuration validation and error handling
• Tests for inspect() operation with format parameter and regex matching
• Tests for copy() operation with success and failure scenarios
• Tests for matching logic including Secret handling and regex patterns
• Tests for logging behavior and edge cases in command building

scripts/python/helpers/fake/test_fake_skopeo.py

---

6. scripts/python/helpers/fake/SKOPEO.md 📝 Documentation +146/-0

Documentation for fake skopeo client implementation

• Comprehensive documentation of fake client implementation
• Explains design decisions and activation mechanisms
• Provides usage patterns for test integration
• Documents YAML structure and matching logic
• Includes troubleshooting and next steps guidance

scripts/python/helpers/fake/SKOPEO.md

---

7. scripts/python/helpers/fake/example_config.yaml 📝 Documentation +84/-0

Example mock configuration for fake skopeo client

• Provides example YAML configuration for FakeSkopeoClient
• Demonstrates inspect operations with format and regex matching
• Demonstrates copy operations with success and failure scenarios
• Shows authentication failure and manifest unknown error examples
• Ready-to-use template for test configuration

scripts/python/helpers/fake/example_config.yaml

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (8) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 Convert publish-index-image internal task to standalone script

✅ Compliance rules (platform): 25 rules

1. Optional annotations in SkopeoClient 📘 Rule violation ⚙ Maintainability

Description

The new SkopeoClient and FakeSkopeoClient code uses typing.Optional[...] in type annotations
instead of PEP 604 T | None. This reintroduces legacy union syntax and unnecessary Optional
imports, violating the required modern union style.

Code

scripts/python/helpers/skopeo.py[R83-94]

+    def __init__(
+        self,
+        *,
+        debug: bool = False,
+        insecure_policy: bool = False,
+        tmpdir: Optional[str] = None,
+        command_timeout: Optional[str] = None,
+        override_arch: Optional[str] = None,
+        override_os: Optional[str] = None,
+        override_variant: Optional[str] = None,
+        logger: Optional[Logger] = None,
+    ):

Evidence

PR Compliance ID 865821 requires using PEP 604 union syntax (T | None) instead of Optional[...]
in changed Python code. The cited files show Optional being imported and then used in new/added
function signatures (multiple parameter and return annotations), demonstrating that legacy
Optional[...] was introduced in the updated skopeo.py and fake/skopeo.py implementations.

Rule 865821: Prefer | syntax over Union / Optional for type unions
scripts/python/helpers/skopeo.py[7-7]
scripts/python/helpers/skopeo.py[88-94]
scripts/python/helpers/skopeo.py[160-170]
scripts/python/helpers/fake/skopeo.py[9-9]
scripts/python/helpers/fake/skopeo.py[56-62]
scripts/python/helpers/fake/skopeo.py[210-224]

Agent prompt

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

## Issue description
`scripts/python/helpers/skopeo.py` and `scripts/python/helpers/fake/skopeo.py` use `Optional[T]` for nullable/union type annotations in newly changed code. The guideline requires PEP 604 unions (`T | None`) and avoiding importing `Optional` solely for annotations.

## Issue Context
PR Compliance ID 865821 mandates modern `|` union syntax across changed Python files and prohibits introducing `Optional[...]` in updated code.

## Fix Focus Areas
- scripts/python/helpers/skopeo.py[7-7]
- scripts/python/helpers/skopeo.py[83-94]
- scripts/python/helpers/skopeo.py[156-170]
- scripts/python/helpers/skopeo.py[249-270]
- scripts/python/helpers/fake/skopeo.py[9-9]
- scripts/python/helpers/fake/skopeo.py[51-62]
- scripts/python/helpers/fake/skopeo.py[210-224]
- scripts/python/helpers/fake/skopeo.py[255-269]
- scripts/python/helpers/fake/skopeo.py[299-320]

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

---

2. Missing future import in tests 📘 Rule violation ⚙ Maintainability

Description

Several changed Python modules (scripts/python/helpers/fake/test_fake_skopeo.py,
scripts/python/helpers/test_skopeo.py, and scripts/python/helpers/fake/__init__.py) are missing
from __future__ import annotations at the top of the file. This violates the required module
header pattern and may undermine consistent forward-reference typing behavior across modules.

Code

scripts/python/helpers/fake/test_fake_skopeo.py[R1-8]

+"""Tests for FakeSkopeoClient."""
+
+import logging
+import os
+import tempfile
+import pytest
+
+from fake.skopeo import FakeSkopeoClient

Evidence

PR Compliance ID 865822 requires from __future__ import annotations to appear near the top of each
changed Python module (typically as the first import, after an optional module docstring). In each
cited file, the module begins with normal imports but omits the required future import,
demonstrating non-compliance with the rule.

Rule 865822: Always enable postponed evaluation of annotations in Python modules
scripts/python/helpers/fake/test_fake_skopeo.py[1-10]
scripts/python/helpers/test_skopeo.py[1-10]
scripts/python/helpers/fake/init.py[1-4]

Agent prompt

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

## Issue description
Changed Python modules are missing `from __future__ import annotations` near the top of the file (as the first import after an optional module docstring), violating PR Compliance ID 865822 and potentially causing inconsistent forward-reference typing behavior.

## Issue Context
Compliance requires postponed evaluation of annotations in all changed Python modules, including tests and package modules; the affected files currently start with standard imports without the required future import.

## Fix Focus Areas
- scripts/python/helpers/fake/test_fake_skopeo.py[1-10]
- scripts/python/helpers/test_skopeo.py[1-10]
- scripts/python/helpers/fake/__init__.py[1-4]

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

---

3. patch_skopeo_client missing type hints 📘 Rule violation ⚙ Maintainability

Description

patch_skopeo_client has no return type annotation (-> None). This violates the requirement to
type all function parameters and return values for new/modified functions.

Code

scripts/python/helpers/fake/init.py[R6-14]

+def patch_skopeo_client():
+    """Monkey-patch skopeo.SkopeoClient with FakeSkopeoClient.
+
+    This function replaces the real SkopeoClient with the fake implementation.
+    Must be called before importing any modules that use SkopeoClient.
+    """
+    import skopeo
+
+    skopeo.SkopeoClient = FakeSkopeoClient

Evidence

PR Compliance ID 865829 requires all new/modified function definitions to include type hints for
parameters and return types; patch_skopeo_client() is defined without -> None.

Rule 865829: Require type hints on all function parameters and return values
scripts/python/helpers/fake/init.py[6-14]

Agent prompt

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

## Issue description
`patch_skopeo_client` is a new public function and lacks an explicit return type annotation.

## Issue Context
The compliance rule requires explicit type hints on all function parameters and return values.

## Fix Focus Areas
- scripts/python/helpers/fake/__init__.py[6-14]

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

---

View more (6)

4. _build_command has untyped **kwargs 📘 Rule violation ⚙ Maintainability

Description

FakeSkopeoClient._build_command declares **kwargs without a type annotation. This violates the
requirement that varargs/kwargs must be typed.

Code

scripts/python/helpers/fake/skopeo.py[235]

+    def _build_command(self, operation: str, **kwargs) -> list[str]:

Evidence

PR Compliance ID 865829 requires typed varargs/kwargs; _build_command introduces an untyped
**kwargs in new code.

Rule 865829: Require type hints on all function parameters and return values
scripts/python/helpers/fake/skopeo.py[235-235]

Agent prompt

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

## Issue description
`**kwargs` must be explicitly typed (e.g., `**kwargs: Any` or a more specific mapping) per the type hint rule.

## Issue Context
The compliance rule requires typing for varargs and kwargs.

## Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[235-253]

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

---

5. SkopeoClient.__init__ missing -> None 📘 Rule violation ⚙ Maintainability

Description

SkopeoClient.__init__ and FakeSkopeoClient.__init__ are defined without an explicit -> None
return type annotation, violating the requirement to annotate special methods. This breaches PR
Compliance ID 865829’s mandate for explicit return type annotations on all methods, including
__init__.

Code

scripts/python/helpers/skopeo.py[R83-94]

+    def __init__(
+        self,
+        *,
+        debug: bool = False,
+        insecure_policy: bool = False,
+        tmpdir: Optional[str] = None,
+        command_timeout: Optional[str] = None,
+        override_arch: Optional[str] = None,
+        override_os: Optional[str] = None,
+        override_variant: Optional[str] = None,
+        logger: Optional[Logger] = None,
+    ):

Evidence

PR Compliance ID 865829 requires explicit return type annotations for all functions/methods and
explicitly includes special methods like __init__; the cited __init__ implementations for both
SkopeoClient and FakeSkopeoClient do not specify -> None, demonstrating non-compliance with
the rule.

Rule 865829: Require type hints on all function parameters and return values
scripts/python/helpers/skopeo.py[83-94]
scripts/python/helpers/fake/skopeo.py[51-62]

Agent prompt

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

## Issue description
Add explicit `-> None` return type annotations to the `__init__` methods for `SkopeoClient` and `FakeSkopeoClient`.

## Issue Context
PR Compliance ID 865829 requires explicit return type annotations for all functions/methods, including special methods like `__init__`; the current `__init__` definitions omit the required `-> None` annotation.

## Fix Focus Areas
- scripts/python/helpers/skopeo.py[83-104]
- scripts/python/helpers/fake/skopeo.py[51-62]

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

---

6. Pytest tests missing -> None 📘 Rule violation ▣ Testability

Description

Pytest-collected tests are missing explicit -> None return annotations on functions/methods named
test_*. This violates PR Compliance ID 865837’s rule that all test_* tests must declare `->
None`.

Code

scripts/python/helpers/fake/test_fake_skopeo.py[R13-19]

+def test_fake_client_requires_env_var():
+    """Test that FakeSkopeoClient requires RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP."""
+    # Ensure env var is not set
+    os.environ.pop("RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP", None)
+
+    with pytest.raises(ValueError, match="RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP.*not set"):
+        FakeSkopeoClient()

Evidence

PR Compliance ID 865837 requires every pytest test named test_* to include an explicit -> None
return annotation. In the cited files, there are test definitions such as the top-level
test_fake_client_requires_env_var() in scripts/python/helpers/fake/test_fake_skopeo.py and
class-based methods like def test_init_with_defaults(self): in
scripts/python/helpers/test_skopeo.py, each shown without a return annotation, with similar
occurrences throughout the referenced line ranges—demonstrating noncompliance with the rule.

Rule 865837: Test functions must explicitly return None
scripts/python/helpers/fake/test_fake_skopeo.py[13-19]
scripts/python/helpers/test_skopeo.py[15-17]

Agent prompt

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

## Issue description
Pytest tests (functions or methods) named `test_*` must explicitly declare a `None` return type using `-> None`; several test definitions are missing this annotation.

## Issue Context
PR Compliance ID 865837 enforces that all `test_*` tests include `-> None`, and this applies equally to top-level pytest test functions and class-based test methods.

## Fix Focus Areas
- scripts/python/helpers/fake/test_fake_skopeo.py[13-288]
- scripts/python/helpers/fake/test_fake_skopeo.py[677-763]
- scripts/python/helpers/test_skopeo.py[12-684]

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

---

7. os.environ used in tests 📘 Rule violation ▣ Testability

Description

The new pytest tests directly mutate os.environ instead of using pytest's monkeypatch fixture
for environment changes. This can cause cross-test contamination and violates the project's
standardized mocking approach.

Code

scripts/python/helpers/fake/test_fake_skopeo.py[R13-27]

+def test_fake_client_requires_env_var():
+    """Test that FakeSkopeoClient requires RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP."""
+    # Ensure env var is not set
+    os.environ.pop("RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP", None)
+
+    with pytest.raises(ValueError, match="RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP.*not set"):
+        FakeSkopeoClient()
+
+
+def test_fake_client_requires_valid_file():
+    """Test that FakeSkopeoClient requires config file to exist."""
+    os.environ["RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP"] = "/nonexistent/file.yaml"
+
+    with pytest.raises(FileNotFoundError, match="Config file not found"):
+        FakeSkopeoClient()

Evidence

PR Compliance ID 865838 requires environment changes in tests to use monkeypatch, but the added
test file repeatedly uses os.environ.pop(...) and direct assignments to os.environ[...].

Rule 865838: Consistent mocking tools for tests
scripts/python/helpers/fake/test_fake_skopeo.py[16-16]
scripts/python/helpers/fake/test_fake_skopeo.py[24-24]

Agent prompt

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

## Issue description
Tests directly modify `os.environ` (`pop`, assignment) instead of using pytest's `monkeypatch.setenv()` / `monkeypatch.delenv()`.

## Issue Context
The compliance rule standardizes mocking tools: use `unittest.mock` for general mocking and `monkeypatch` for environment variables.

## Fix Focus Areas
- scripts/python/helpers/fake/test_fake_skopeo.py[13-27]
- scripts/python/helpers/fake/test_fake_skopeo.py[30-289]
- scripts/python/helpers/fake/test_fake_skopeo.py[301-377]
- scripts/python/helpers/fake/test_fake_skopeo.py[476-534]
- scripts/python/helpers/fake/test_fake_skopeo.py[575-640]
- scripts/python/helpers/fake/test_fake_skopeo.py[663-697]
- scripts/python/helpers/fake/test_fake_skopeo.py[750-763]

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

---

8. Secret breaks exception __str__ 🐞 Bug ≡ Correctness

Description

SkopeoClientError.__str__ does ' '.join(self.command), but SkopeoClient builds cmd lists
containing Secret objects (e.g., creds/tokens) and stores that list on exceptions. When a skopeo
call fails while using credentials, converting/logging the exception will raise TypeError, masking
the original failure.

Code

scripts/python/helpers/skopeo.py[R39-46]

+    def __str__(self) -> str:
+        """Return a detailed error message including command and outputs."""
+        return (
+            f"{super().__str__()}\n"
+            f"    Command: {' '.join(self.command)}\n"
+            f"    Return code: {self.returncode}\n"
+            f"    Stderr: {self.stderr}"
+        )

Evidence

The real client appends Secret objects into cmd and later raises SkopeoClientError with
command=cmd, while SkopeoClientError.__str__ joins the list assuming strings—this will raise a
TypeError when any Secret is present.

scripts/python/helpers/skopeo.py[39-46]
scripts/python/helpers/skopeo.py[126-155]
scripts/python/helpers/skopeo.py[205-227]
scripts/python/helpers/skopeo.py[327-343]

Agent prompt

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

### Issue description
`SkopeoClientError.__str__()` assumes every element of `self.command` is a string, but `SkopeoClient` builds `cmd` with `Secret` objects (e.g., `--creds`, `--src-creds`, `--registry-token`) and passes that same list into `SkopeoClientError`. If the subprocess fails (or JSON parsing fails) on an authenticated call, `str(exc)` will raise `TypeError` and hide the real error.

### Issue Context
- `_run_command` unveils secrets only for execution, but stores the original `cmd` in the raised exception.
- This also affects any logging that implicitly stringifies the exception.

### Fix Focus Areas
- scripts/python/helpers/skopeo.py[39-46]
- scripts/python/helpers/skopeo.py[126-155]
- scripts/python/helpers/skopeo.py[205-247]
- scripts/python/helpers/skopeo.py[310-354]

### Suggested fix approach
- Ensure `SkopeoClientError.command` is always `list[str]` (never `Secret`).
 - Option A: create `cmd_exec` (unveiled) and `cmd_display` (sanitized) and store `cmd_display` on exceptions.
 - Option B: in `SkopeoClientError.__str__`, join `map(str, self.command)` (but prefer also normalizing stored command type).
- Ensure the copy-command logger uses the sanitized/display command, not the raw list containing `Secret` objects.

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

---

9. Inspect rules allow missing return 🐞 Bug ☼ Reliability

Description

FakeSkopeoClient validates return only if it exists, but inspect() always does `return
rule['return']. An inspect rule without return` will pass load-time validation and then crash
with KeyError at runtime instead of raising a controlled SkopeoClientError.

Code

scripts/python/helpers/fake/skopeo.py[R121-131]

+        # Check required fields
+        if "match" not in rule:
+            raise ValueError(f"Rule #{idx} in '{operation}' missing required 'match' field")
+
+        if not isinstance(rule["match"], dict):
+            raise ValueError(f"Rule #{idx} in '{operation}': 'match' must be a dict")
+
+        # Validate return structure based on operation
+        if "return" in rule:
+            self._validate_return(operation, idx, rule)
+

Evidence

Validation only checks return when present, yet inspect returns rule['return'] unconditionally, so
a rule without return will cause a KeyError at runtime.

scripts/python/helpers/fake/skopeo.py[114-131]
scripts/python/helpers/fake/skopeo.py[255-297]

Agent prompt

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

### Issue description
`FakeSkopeoClient` does not require `return` to be present for `inspect` rules at validation time, but `inspect()` unconditionally accesses `rule['return']`. This creates a config that loads successfully but fails later with `KeyError`.

### Issue Context
- For `copy`, omitting `return` is intentionally treated as success.
- For `inspect`, omitting `return` cannot be treated as success because a return value is required.

### Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[114-131]
- scripts/python/helpers/fake/skopeo.py[255-297]

### Suggested fix approach
- In `_validate_rule`, when `operation == 'inspect'`, enforce that `return` exists (and then validate its type as already implemented).
- Optionally add a defensive check in `inspect()` to raise a `SkopeoClientError` with a clear message if `return` is missing (in case validation is bypassed).

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

---

10. Non-imperative module docstring 📘 Rule violation ⚙ Maintainability

Description

The module docstring summary line is not written in imperative mood, violating the docstring mood
requirement for new/modified docstrings. The current summary is written as a descriptive phrase
rather than starting with an imperative verb phrase.

Code

scripts/python/helpers/skopeo.py[1]

+"""Python client for skopeo container image operations."""

Evidence

PR Compliance ID 865832 requires docstring summary lines to be in imperative mood; the cited module
docstrings use descriptive phrases (e.g., Python client ... and Fake SkopeoClient ...) instead
of an imperative verb phrase, demonstrating the noncompliance with the required docstring style.

Rule 865832: Write all docstrings in imperative mood
scripts/python/helpers/skopeo.py[1-1]
scripts/python/helpers/fake/skopeo.py[1-1]

Agent prompt

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

## Issue description
Docstring summary lines must start with an imperative verb (e.g., "Provide...", "Execute...", "Inspect...") rather than a descriptive noun phrase.

## Issue Context
This rule applies to module/class/function docstrings added or modified in the PR.

## Fix Focus Areas
- scripts/python/helpers/skopeo.py[1-1]
- scripts/python/helpers/fake/skopeo.py[1-1]

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

---

11. Fake inspect return type drift 🐞 Bug ≡ Correctness

Description

FakeSkopeoClient decides inspect return type based on whether format appears in the rule’s
*match* clause, not based on whether the caller actually passed format. This allows `inspect(...,
format=...) to match a rule without format` and return a dict, diverging from SkopeoClient which
always returns a string when format is provided.

Code

scripts/python/helpers/fake/skopeo.py[R274-297]

+        # Build actual parameters for matching (only non-Secret, non-constructor params)
+        actual_params = {"image": image}
+        if format is not None:
+            actual_params["format"] = format
+
+        # Find matching rule
+        rule = self._find_matching_rule("inspect", actual_params)
+
+        if rule is None:
+            # No match - raise error
+            error_msg = self._build_error_message("inspect", actual_params)
+            if self.logger:
+                self.logger.error(error_msg)
+
+            raise SkopeoClientError(
+                "No mock match found",
+                command=self._build_command("inspect", image=image, format=format),
+                returncode=1,
+                stdout="",
+                stderr=error_msg,
+            )
+
+        # Return the configured value
+        return rule["return"]

Evidence

The fake only includes format in actual_params when provided, but matching ignores extra call
params (so a rule that matches only image can still match a call that provides format), and the
fake returns rule['return'] without considering the call. The real client’s return type is driven
directly by the call’s format parameter.

scripts/python/helpers/fake/skopeo.py[198-209]
scripts/python/helpers/fake/skopeo.py[274-297]
scripts/python/helpers/skopeo.py[233-240]

Agent prompt

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

### Issue description
`SkopeoClient.inspect()` returns `str` when `format` is provided and `dict` when it is not. `FakeSkopeoClient.inspect()` returns whatever is in the matched rule’s `return`, and rules that omit `format` in `match` can still match calls that *do* pass `format` (because extra call params are ignored). This can break code under test (type mismatch) and violates drop-in behavior.

### Issue Context
- Current validation enforces return type based on `has_format = 'format' in match`, not based on the actual call.

### Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[132-155]
- scripts/python/helpers/fake/skopeo.py[198-209]
- scripts/python/helpers/fake/skopeo.py[255-297]
- scripts/python/helpers/skopeo.py[233-240]

### Suggested fix approach
- After selecting the matching rule in `FakeSkopeoClient.inspect()`, validate the matched `return` against the *actual* call:
 - if `format is not None`: require `return` is `str`
 - else: require `return` is `dict`
 - if mismatch: raise a clear `ValueError` (config error) or `SkopeoClientError` explaining the mismatch.
- Keep (or simplify) load-time validation, but don’t rely solely on `format` being present in the match clause for correctness.

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

---

12. Fake command rendering inaccurate 🐞 Bug ⚙ Maintainability

Description

FakeSkopeoClient._build_command formats positional arguments like image, source, and
destination as flags (--image, --source, --destination). This makes
SkopeoClientError.command inconsistent with the real client’s command layout, reducing the
usefulness of error diagnostics in tests.

Code

scripts/python/helpers/fake/skopeo.py[R235-253]

+    def _build_command(self, operation: str, **kwargs) -> list[str]:
+        """Reconstruct the skopeo command for error reporting."""
+        cmd = ["skopeo", operation]
+
+        # Add common parameters
+        for key, value in kwargs.items():
+            if value is None or isinstance(value, Secret):
+                continue
+
+            # Convert pythonic parameter names to CLI flags
+            flag_name = key.replace("_", "-")
+
+            if isinstance(value, bool):
+                if value:
+                    cmd.append(f"--{flag_name}")
+            else:
+                cmd.extend([f"--{flag_name}", str(value)])
+
+        return cmd

Evidence

The fake’s builder emits --{key} flags for all kwargs, while the real client appends
image/source/destination as positional arguments; therefore error command layouts diverge.

scripts/python/helpers/fake/skopeo.py[235-253]
scripts/python/helpers/fake/skopeo.py[288-294]
scripts/python/helpers/fake/skopeo.py[341-347]
scripts/python/helpers/skopeo.py[205-229]
scripts/python/helpers/skopeo.py[351-354]

Agent prompt

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

### Issue description
`FakeSkopeoClient._build_command()` converts all kwargs into CLI flags, but real skopeo usage (and the real `SkopeoClient`) uses positional args for `inspect <image>` and `copy <source> <destination>`. The fake’s error command is therefore misleading.

### Issue Context
This only affects error reporting, not matching/execution, but it can slow down debugging test failures and makes `SkopeoClientError.command` inconsistent between real and fake clients.

### Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[235-253]
- scripts/python/helpers/fake/skopeo.py[282-294]
- scripts/python/helpers/fake/skopeo.py[335-347]
- scripts/python/helpers/skopeo.py[205-229]
- scripts/python/helpers/skopeo.py[351-354]

### Suggested fix approach
- Special-case positional parameters:
 - For `inspect`: append `image` as the final arg (not `--image`).
 - For `copy`: append `source` and `destination` as final args (not flags).
- Keep other kwargs rendered as flags as today.

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

---

[qodo-app-for-konflux-ci]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (5) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Tests lack -> None annotations ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

New test functions/methods omit the required -> None return type annotation. This reduces
type-checking consistency across the test suite.

Code

scripts/python/helpers/test_skopeo.py[R15-17]

+    def test_init_with_defaults(self):
+        """Test creating client with default parameters."""
+        client = SkopeoClient()

Relevance

⭐⭐⭐ High

Existing tests include explicit -> None annotations, implying team expects this in new tests too.

PR-#738

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 907 requires every test function named test_... to have an explicit -> None
return annotation. The added tests include multiple def test_... definitions without return
annotations.

Rule 907: Type-hint test functions with -> None return type
scripts/python/helpers/fake/test_fake_skopeo.py[13-20]
scripts/python/helpers/test_skopeo.py[15-17]

Agent prompt

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

## Issue description
Test functions (and test methods) must be explicitly annotated with `-> None`.

## Issue Context
The checklist requires all `test_...` functions to declare `-> None` to standardize typing in tests.

## Fix Focus Areas
- scripts/python/helpers/fake/test_fake_skopeo.py[13-31]
- scripts/python/helpers/test_skopeo.py[15-17]
- scripts/python/helpers/test_skopeo.py[46-54]
- scripts/python/helpers/test_skopeo.py[82-84]
- scripts/python/helpers/test_skopeo.py[654-655]

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

---

2. __future__ annotations missing/late 📘 Rule violation ⚙ Maintainability

Description

Several new/modified Python files either omit from __future__ import annotations or do not place
it as the first non-comment line. This breaks the repository’s required module header convention.

Code

scripts/python/helpers/fake/init.py[R1-4]

+"""Fake implementations for testing."""
+
+from fake.skopeo import FakeSkopeoClient
+

Relevance

⭐⭐⭐ High

Repo Python modules add from __future__ import annotations right after docstring (e.g.,
helpers/image_ref.py).

PR-#765

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 902 requires from __future__ import annotations as the first non-comment line in
every changed .py module. scripts/python/helpers/fake/__init__.py has no such import, and other
changed modules place the import after the module docstring or omit it entirely.

Rule 902: Always import postponed annotations in Python modules
scripts/python/helpers/fake/init.py[1-4]
scripts/python/helpers/fake/skopeo.py[1-5]
scripts/python/helpers/skopeo.py[1-5]
scripts/python/helpers/fake/test_fake_skopeo.py[1-8]
scripts/python/helpers/test_skopeo.py[1-7]

Agent prompt

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

## Issue description
Files are missing `from __future__ import annotations` or place it after other top-level content, violating the required import ordering.

## Issue Context
The checklist requires the first non-shebang, non-encoding, non-comment, non-empty line to be exactly `from __future__ import annotations`.

## Fix Focus Areas
- scripts/python/helpers/fake/__init__.py[1-4]
- scripts/python/helpers/fake/skopeo.py[1-5]
- scripts/python/helpers/skopeo.py[1-5]
- scripts/python/helpers/fake/test_fake_skopeo.py[1-8]
- scripts/python/helpers/test_skopeo.py[1-7]

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

---

3. Missing function type annotations 📘 Rule violation ⚙ Maintainability

Description

New/modified functions and methods are missing required parameter/return type annotations (notably
**kwargs and several __init__-like entrypoints). This violates the project-wide requirement for
fully annotated function signatures.

Code

scripts/python/helpers/fake/skopeo.py[R235-236]

+    def _build_command(self, operation: str, **kwargs) -> list[str]:
+        """Reconstruct the skopeo command for error reporting."""

Relevance

⭐⭐⭐ High

Recent helpers/tests use fully typed signatures and -> None returns, suggesting strict annotation
culture.

PR-#738

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 913 requires explicit type hints for all parameters and return types. The PR adds
patch_skopeo_client() without a return annotation, adds _build_command(..., **kwargs) without
typing **kwargs, and introduces multiple __init__ methods without explicit -> None return
types.

Rule 913: Require type hints for all function parameters and return types
scripts/python/helpers/fake/init.py[6-14]
scripts/python/helpers/fake/skopeo.py[51-62]
scripts/python/helpers/fake/skopeo.py[235-236]
scripts/python/helpers/skopeo.py[24-31]
scripts/python/helpers/skopeo.py[83-94]

Agent prompt

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

## Issue description
Some new/modified functions/methods are missing required type hints (e.g., untyped `**kwargs`, missing `-> None` return annotations).

## Issue Context
The compliance checklist requires explicit typing for all parameters (including `*args`/`**kwargs`) and explicit return types.

## Fix Focus Areas
- scripts/python/helpers/fake/__init__.py[6-15]
- scripts/python/helpers/fake/skopeo.py[51-62]
- scripts/python/helpers/fake/skopeo.py[235-253]
- scripts/python/helpers/skopeo.py[24-38]
- scripts/python/helpers/skopeo.py[83-104]

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

---

View more (4)

4. Tests mutate os.environ directly 📘 Rule violation ▣ Testability

Description

New tests directly set/unset os.environ instead of using pytest’s monkeypatch fixture. This can
cause test order dependence and leaked environment state.

Code

scripts/python/helpers/fake/test_fake_skopeo.py[R13-26]

+def test_fake_client_requires_env_var():
+    """Test that FakeSkopeoClient requires RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP."""
+    # Ensure env var is not set
+    os.environ.pop("RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP", None)
+
+    with pytest.raises(ValueError, match="RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP.*not set"):
+        FakeSkopeoClient()
+
+
+def test_fake_client_requires_valid_file():
+    """Test that FakeSkopeoClient requires config file to exist."""
+    os.environ["RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP"] = "/nonexistent/file.yaml"
+
+    with pytest.raises(FileNotFoundError, match="Config file not found"):

Relevance

⭐⭐⭐ High

Tests in repo commonly use pytest monkeypatch for env vars; indicates direct os.environ mutation
discouraged.

PR-#783

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 905 requires using pytest monkeypatch for environment variable overrides in
tests. The added tests repeatedly modify os.environ directly and manually clean up, instead of
using monkeypatch.

Rule 905: Standardize mocking libraries in tests
scripts/python/helpers/fake/test_fake_skopeo.py[13-19]
scripts/python/helpers/fake/test_fake_skopeo.py[24-27]
scripts/python/helpers/fake/test_fake_skopeo.py[43-52]

Agent prompt

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

## Issue description
Tests directly mutate `os.environ` (e.g., `os.environ[...] = ...`, `os.environ.pop(...)`) rather than using pytest's `monkeypatch` fixture.

## Issue Context
The compliance standard requires `monkeypatch.setenv()` / `monkeypatch.delenv()` for environment overrides in tests to ensure isolation.

## Fix Focus Areas
- scripts/python/helpers/fake/test_fake_skopeo.py[13-26]
- scripts/python/helpers/fake/test_fake_skopeo.py[43-52]
- scripts/python/helpers/fake/test_fake_skopeo.py[68-78]
- scripts/python/helpers/fake/test_fake_skopeo.py[119-127]
- scripts/python/helpers/fake/test_fake_skopeo.py[241-248]

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

---

5. Optional used in type hints 📘 Rule violation ⚙ Maintainability

Description

New Python modules import and use typing.Optional in annotations even though the project targets
Python >=3.12. This violates the required | None union style and keeps outdated typing imports in
the codebase.

Code

scripts/python/helpers/skopeo.py[7]

+from typing import Any, Optional

Relevance

⭐⭐ Medium

No clear historical enforcement for | None vs Optional; insufficient evidence team would change
it now.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 901 requires using | unions instead of Optional[]. The touched modules import
Optional and use it in multiple function signatures (e.g., tmpdir: Optional[str], `logger:
Optional[Logger]`).

Rule 901: Prefer | union syntax over Union[] and Optional[]
scripts/python/helpers/skopeo.py[7-7]
scripts/python/helpers/skopeo.py[88-94]
scripts/python/helpers/fake/skopeo.py[9-9]
scripts/python/helpers/fake/skopeo.py[56-62]

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 code uses `typing.Optional[...]` in type hints. With Python >=3.12, the project standard requires `T | None` (and removing unused `Optional` imports).

## Issue Context
`pyproject.toml` sets `requires-python = ">=3.12"`, so modern union syntax is supported.

## Fix Focus Areas
- scripts/python/helpers/skopeo.py[7-94]
- scripts/python/helpers/fake/skopeo.py[9-62]
- scripts/python/helpers/fake/skopeo.py[210-224]

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

---

6. Secret breaks error formatting ✓ Resolved 🐞 Bug ⛨ Security

Description

SkopeoClientError.__str__() does ' '.join(self.command) but SkopeoClient builds command lists
containing Secret objects (e.g., --creds, --src-creds), so formatting/logging errors can raise
TypeError and/or expose secret values. copy() also logs the raw command list (including secrets)
at info level.

Code

scripts/python/helpers/skopeo.py[R39-46]

+    def __str__(self) -> str:
+        """Return a detailed error message including command and outputs."""
+        return (
+            f"{super().__str__()}\n"
+            f"    Command: {' '.join(self.command)}\n"
+            f"    Return code: {self.returncode}\n"
+            f"    Stderr: {self.stderr}"
+        )

Relevance

⭐⭐ Medium

No prior repo evidence on Secret-safe command/error formatting or log redaction expectations.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The exception string formatter joins self.command as strings, but the command list is constructed
with Secret objects (credentials and tokens) and is also logged directly in copy(). This makes
error reporting fragile and risks leaking sensitive values.

scripts/python/helpers/skopeo.py[39-46]
scripts/python/helpers/skopeo.py[139-154]
scripts/python/helpers/skopeo.py[211-226]
scripts/python/helpers/skopeo.py[328-343]
scripts/python/helpers/skopeo.py[351-354]

Agent prompt

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

### Issue description
`SkopeoClientError.__str__()` assumes `self.command` is a list of strings and joins it, but the client stores `Secret` objects in `cmd`. This can crash error formatting and/or leak credentials when the command is logged or printed.

### Issue Context
- `inspect()` and `copy()` append `Secret` objects directly into `cmd`.
- `_run_command()` unveils secrets only for execution, but still stores the original unsanitized `cmd` on exceptions.
- `copy()` logs the full `cmd` list.

### Fix approach
- Build two command representations:
 - `exec_cmd`: all strings with secrets unveiled (used only for `subprocess.run`).
 - `display_cmd`: all strings with secrets **redacted** (e.g., `"<secret>"`) used for logging and attached to `SkopeoClientError`.
- Update `SkopeoClientError` to accept `command: list[str]` that is already sanitized OR make `__str__()` robust (convert elements to strings) and explicitly redact `Secret` values.
- Ensure logger messages use `display_cmd`, not the raw list.

### Fix Focus Areas
- scripts/python/helpers/skopeo.py[39-46]
- scripts/python/helpers/skopeo.py[126-155]
- scripts/python/helpers/skopeo.py[205-227]
- scripts/python/helpers/skopeo.py[249-354]

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

---

7. Inspect return not validated ✓ Resolved 🐞 Bug ≡ Correctness

Description

FakeSkopeoClient allows inspect rules without a return field during config validation, but
inspect() always does return rule["return"], causing a runtime KeyError instead of a clear
load-time validation failure.

Code

scripts/python/helpers/fake/skopeo.py[R114-131]

+    def _validate_rule(self, operation: str, idx: int, rule: dict) -> None:
+        """Validate a single rule structure."""
+        if not isinstance(rule, dict):
+            raise ValueError(
+                f"Rule #{idx} in '{operation}' must be a dict, got {type(rule).__name__}"
+            )
+
+        # Check required fields
+        if "match" not in rule:
+            raise ValueError(f"Rule #{idx} in '{operation}' missing required 'match' field")
+
+        if not isinstance(rule["match"], dict):
+            raise ValueError(f"Rule #{idx} in '{operation}': 'match' must be a dict")
+
+        # Validate return structure based on operation
+        if "return" in rule:
+            self._validate_return(operation, idx, rule)
+

Relevance

⭐⭐ Medium

No historical evidence found about enforcing load-time validation vs runtime KeyError for mock rule
configs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Validation currently treats return as optional, but inspect() assumes it exists and will crash
if it doesn’t. The included SKOPEO.md also describes return as optional at the YAML structure
level, increasing the likelihood of misconfiguration.

scripts/python/helpers/fake/skopeo.py[114-131]
scripts/python/helpers/fake/skopeo.py[255-297]
scripts/python/helpers/fake/SKOPEO.md[48-52]

Agent prompt

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

### Issue description
For the fake client, `inspect()` rules effectively require a `return` value, but the config validator does not enforce it. This leads to runtime `KeyError` when a rule matches but lacks `return`.

### Issue Context
- `_validate_rule()` only validates `return` if present.
- `inspect()` unconditionally accesses `rule["return"]`.
- The bundled documentation also says `return` is optional for rules, which is incompatible with `inspect()` behavior.

### Fix approach
- Enforce that `inspect` rules must include `return` during `_validate_rule()` (or inside `_validate_return()` with an explicit presence check).
- Optionally update docs (`SKOPEO.md`) to clarify: `copy` can omit `return` (success), but `inspect` must provide `return`.

### Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[114-131]
- scripts/python/helpers/fake/skopeo.py[255-297]
- scripts/python/helpers/fake/SKOPEO.md[48-52]

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

---

8. Fake match keys ignored 🐞 Bug ⚙ Maintainability

Description

FakeSkopeoClient matching only considers image/format for inspect and source/destination
for copy, so YAML rules that try to match on other supported parameters (e.g., tls_verify,
authfile, no_tags) can never match. This contradicts the documented rule semantics (“only fields
specified in match must match; extra parameters ignored”) and makes tests/configs brittle.

Code

scripts/python/helpers/fake/skopeo.py[R274-333]

+        # Build actual parameters for matching (only non-Secret, non-constructor params)
+        actual_params = {"image": image}
+        if format is not None:
+            actual_params["format"] = format
+
+        # Find matching rule
+        rule = self._find_matching_rule("inspect", actual_params)
+
+        if rule is None:
+            # No match - raise error
+            error_msg = self._build_error_message("inspect", actual_params)
+            if self.logger:
+                self.logger.error(error_msg)
+
+            raise SkopeoClientError(
+                "No mock match found",
+                command=self._build_command("inspect", image=image, format=format),
+                returncode=1,
+                stdout="",
+                stderr=error_msg,
+            )
+
+        # Return the configured value
+        return rule["return"]
+
+    def copy(
+        self,
+        source: str,
+        destination: str,
+        *,
+        all: Optional[bool] = None,
+        preserve_digests: Optional[bool] = None,
+        format: Optional[str] = None,
+        retry_times: Optional[int] = None,
+        quiet: bool = False,
+        src_creds: Optional[Secret] = None,
+        src_tls_verify: Optional[bool] = None,
+        src_no_creds: bool = False,
+        src_cert_dir: Optional[str] = None,
+        src_authfile: Optional[str] = None,
+        dest_creds: Optional[Secret] = None,
+        dest_tls_verify: Optional[bool] = None,
+        dest_no_creds: bool = False,
+        dest_cert_dir: Optional[str] = None,
+        dest_authfile: Optional[str] = None,
+        remove_signatures: bool = False,
+    ) -> None:
+        """Fake implementation of copy operation.
+
+        Matches against rules in config and either returns None (success)
+        or raises SkopeoClientError (failure).
+        """
+        # Build actual parameters for matching
+        actual_params = {
+            "source": source,
+            "destination": destination,
+        }
+
+        # Find matching rule
+        rule = self._find_matching_rule("copy", actual_params)

Relevance

⭐⭐ Medium

No historical evidence on FakeSkopeo matching semantics; unclear if team expects matching all
non-secret args.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The documentation describes a generic matching model based on fields present in match, but the
implementation only passes a limited subset of arguments into matching, so any rule that includes
other keys will fail. _match_rule() confirms missing keys become None and therefore won’t match
specified patterns.

scripts/python/helpers/fake/SKOPEO.md[53-58]
scripts/python/helpers/fake/skopeo.py[198-208]
scripts/python/helpers/fake/skopeo.py[274-333]

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 fake client’s rule matcher is effectively hardcoded to a subset of parameters, even though the docs describe a general “match only specified fields” behavior. If a config author includes other keys in `match`, those rules will never match because the actual call parameters are not included in `actual_params`.

### Issue Context
- `_match_rule()` expects that any key present in `rule['match']` is also present in `actual_params` (otherwise it sees `None` and fails).
- `inspect()` and `copy()` currently populate `actual_params` with only a few fields.

### Fix approach
- Populate `actual_params` with **all** method arguments (including booleans) except:
 - Any `Secret` values (keep ignoring secrets as intended).
 - Optionally exclude constructor-level settings, as documented.
- Make `FakeSkopeoClient.inspect()` signature consistent with `SkopeoClient.inspect()` (`retry_times: Optional[int]` instead of `Optional[str]`).
- If the limitation is intentional, update `SKOPEO.md` to explicitly state which keys are matchable (but current docs imply broader support).

### Fix Focus Areas
- scripts/python/helpers/fake/skopeo.py[198-208]
- scripts/python/helpers/fake/skopeo.py[255-333]
- scripts/python/helpers/fake/SKOPEO.md[53-58]

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

---

9. Module docstrings not imperative 📘 Rule violation ⚙ Maintainability

Description

Several new modules have module-level docstrings that do not begin with an imperative verb (e.g.,
they start with nouns like Python client... or Tests...). This violates the required docstring
style.

Code

scripts/python/helpers/skopeo.py[1]

+"""Python client for skopeo container image operations."""

Relevance

⭐ Low

Repo already uses non-imperative module docstrings like “Helpers for …”; style not enforced
historically.

PR-#765
PR-#738

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 914 requires module docstrings to start with an imperative verb. The added module
docstrings begin with noun phrases (e.g., Python client..., Fake ..., Unit tests ...).

Rule 914: Require docstrings for modules and public functions in triple-double-quote imperative format
scripts/python/helpers/skopeo.py[1-1]
scripts/python/helpers/fake/skopeo.py[1-1]
scripts/python/helpers/fake/init.py[1-1]
scripts/python/helpers/fake/test_fake_skopeo.py[1-1]
scripts/python/helpers/test_skopeo.py[1-1]

Agent prompt

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

## Issue description
Module-level docstrings must start with a verb in imperative mood.

## Issue Context
The checklist requires module docstrings in triple-double-quote format and starting with an imperative verb (e.g., "Provide", "Implement", "Test").

## Fix Focus Areas
- scripts/python/helpers/skopeo.py[1-1]
- scripts/python/helpers/fake/skopeo.py[1-1]
- scripts/python/helpers/fake/__init__.py[1-1]
- scripts/python/helpers/fake/test_fake_skopeo.py[1-1]
- scripts/python/helpers/test_skopeo.py[1-1]

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

---

[Copilot]

Pull request overview

Adds a Python SkopeoClient helper (inspect/copy) plus a YAML-configurable FakeSkopeoClient for tests, aligning with the repo’s pattern of small CLI wrappers under scripts/python/helpers/.

Changes:

• Introduces SkopeoClient and SkopeoClientError wrapping skopeo inspect / skopeo copy.
• Adds FakeSkopeoClient with rule-based matching from RELEASE_SERVICE_UTILS_FAKE_SKOPEO_SETUP and a patch_skopeo_client() helper for injection.
• Adds unit tests and documentation/examples for both implementations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
scripts/python/helpers/skopeo.py	New real skopeo CLI wrapper client + error type
scripts/python/helpers/test_skopeo.py	Unit tests for SkopeoClient
scripts/python/helpers/fake/skopeo.py	New YAML-driven fake client for testing
scripts/python/helpers/fake/test_fake_skopeo.py	Unit tests for FakeSkopeoClient
scripts/python/helpers/fake/init.py	Exposes FakeSkopeoClient and monkey-patch helper
scripts/python/helpers/fake/SKOPEO.md	Design/usage summary documentation
scripts/python/helpers/fake/example_config.yaml	Example YAML config for fake behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.01408% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.59%. Comparing base (f9b1b50) to head (6930a6d).

Files with missing lines	Patch %	Lines
scripts/python/helpers/rsmodels/secret.py	58.33%	10 Missing ⚠️
scripts/python/helpers/skopeo.py	97.43%	3 Missing ⚠️
scripts/python/helpers/fake/__init__.py	60.00%	2 Missing ⚠️
scripts/python/helpers/fake/skopeo.py	98.55%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #812      +/-   ##
==========================================
- Coverage   95.65%   95.59%   -0.07%     
==========================================
  Files          72       75       +3     
  Lines        7111     7395     +284     
==========================================
+ Hits         6802     7069     +267     
- Misses        309      326      +17     

Flag	Coverage Δ
unit-tests	95.59% <94.01%> (-0.07%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
scripts/python/helpers/fake/__init__.py	60.00% <60.00%> (ø)
scripts/python/helpers/fake/skopeo.py	98.55% <98.55%> (ø)
scripts/python/helpers/skopeo.py	97.69% <97.43%> (-2.31%)	⬇️
scripts/python/helpers/rsmodels/secret.py	58.33% <58.33%> (ø)

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9b1b50...6930a6d. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[theflockers]

IMHO this secret module could be at the same level as skopeo. It seems it can be reused in many other places. One example is the git authentication, which also relies on adding the token to github URL. That could be useful, for instance: secret.skopeo or secret.git to prevent leaking these tokens/passowords.

[midnightercz]

What prevents devs to import it from rsmodels? Secret doesn't have any functionality so it should belong to models

[qodo-app-for-konflux-ci]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: python

Failed stage: Black Lint [❌]

Failed test name: ""

Failure summary: The action failed during the psf/black formatting check (black --check --diff) because two Python files contain unresolved Git merge conflict markers, causing Black to fail parsing them: - scripts/python/helpers/skopeo.py shows <<<<<<< HEAD at line 1, which triggers ParseError: bad input. - scripts/python/helpers/test_skopeo.py shows >>>>>>> f27e288 (feat(RELEASE-1989): skopeo helper client) around line 879, also triggering a parse error. Because Black could not parse these files, it exited with code 123, failing the GitHub Action step.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 273: Turn off this advice by setting config variable advice.detachedHead to false 274: HEAD is now at f9f639e Merge 333ac6556d3e26f939305935d2149472dc35b7aa into a0c68a70db1cce60cd2f886d8b1044c100edd89a 275: ##[endgroup] 276: [command]/usr/bin/git log -1 --format=%H 277: f9f639e272cdcebb229fd0c0255d44c59de63671 278: ##[group]Run psf/black@b74b23013fe6d0f7e0cee430a472ac70c5ba1334 279: with: 280: options: --check --diff 281: src: . 282: jupyter: false 283: use_pyproject: false 284: summary: true 285: env: 286: GH_TOKEN: *** 287: ##[endgroup] 288: ##[group]Run # Even when black fails, do not close the shell 289: �[36;1m# Even when black fails, do not close the shell�[0m 290: �[36;1mset +e�[0m ... 312: �[36;1mexit ${exit_code}�[0m 313: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 314: env: 315: GH_TOKEN: *** 316: INPUT_OPTIONS: --check --diff 317: INPUT_SRC: . 318: INPUT_JUPYTER: false 319: INPUT_BLACK_ARGS: 320: INPUT_VERSION: 321: INPUT_USE_PYPROJECT: false 322: INPUT_SUMMARY: true 323: OUTPUT_FILE: 324: pythonioencoding: utf-8 325: ##[endgroup] 326: Installing .[colorama]... 327: error: cannot format /home/runner/work/release-service-utils/release-service-utils/scripts/python/helpers/skopeo.py: Cannot parse for target version Python 3.15: 1:0 328: <<<<<<< HEAD 329: ^ 330: ParseError: bad input 331: error: cannot format /home/runner/work/release-service-utils/release-service-utils/scripts/python/helpers/test_skopeo.py: Cannot parse for target version Python 3.15: 879:0 332: >>>>>>> f27e288 (feat(RELEASE-1989): skopeo helper client) 333: ^ 334: ParseError: bad input 335: Oh no! 💥 💔 💥 336: 146 files would be left unchanged, 2 files would fail to reformat. 337: ##[error]Process completed with exit code 123. 338: Post job cleanup.

[midnightercz]

/retest

[midnightercz]

/retest

[midnightercz]

/retest

[midnightercz]

/retest

[midnightercz]

/retest

[midnightercz]

/retest