utils/vmimage: Add fallback mirrors for Fedora images

[harvey0100]

The Fedora download URLs and image naming conventions are subject to frequent changes as versions are released or archived. Recently, Fedora 40 introduced a new naming scheme that caused recurring CI failures with Provider not available errors.

This PR implements a more robust fetching logic to handle network instability and evolving directory structures.

Changes

1. Enhanced Error Handling
   URLError Support: Now catches DNS resolution failures and general network errors in addition to HTTPError.

Improved Observability: Added debug logging for network-level exceptions to simplify remote troubleshooting.

2. Robust Mirror & Pattern Logic
   Automatic Fallback: FedoraImageProvider now attempts to reach multiple mirrors in sequence if the primary fails (e.g., a 404 or timeout).

Primary: dl.fedoraproject.org

Secondary (Archive): archives.fedoraproject.org

Dynamic Pattern Selection: Implemented version-aware naming patterns to support Fedora's evolving conventions:

[gemini-code-assist]

Summary of Changes

Hello @harvey0100, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the reliability of Fedora image provisioning by introducing a comprehensive fallback system for download URLs. It addresses recurring CI failures caused by volatile Fedora mirror availability by allowing the image provider to cycle through multiple mirrors and handle a broader range of network errors. This change ensures more stable and consistent image acquisition, reducing maintenance overhead and improving the overall robustness of the system.

Highlights

• Robust Fallback Mechanism: Implemented a robust fallback mechanism for Fedora image downloads, allowing the system to try multiple mirrors sequentially if the primary URL fails due to issues like 404 errors or network problems.
• Enhanced Error Handling: Expanded network error handling to catch URLError (DNS failures) in addition to HTTPError, making the image provider more resilient to various connectivity issues.
• Configurable Mirrors: Introduced MIRRORS lists in FedoraImageProvider and FedoraSecondaryImageProvider to define a prioritized sequence of fallback URLs, including download.fedoraproject.org, dl.fedoraproject.org, and mirrors.kernel.org.
• Improved Initialization Logic: Refactored the __init__ methods of Fedora image providers to use the _version parameter for initial version checks, preventing premature network calls during object instantiation.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a fallback mechanism for Fedora image download URLs to prevent CI failures due to mirror unavailability or URL changes, by adding URLError handling and multiple mirror attempts. However, the implementation exposes two security vulnerabilities: a potential Argument Injection in the qemu-img command construction due to unquoted user-supplied parameters, and a Regular Expression Denial of Service (ReDoS) vulnerability where the version parameter is used directly as a regex pattern. It is recommended to sanitize user inputs and ensure proper quoting of command-line arguments to address these issues.

[gemini-code-assist]

The url_images template is constructed using user-supplied version and arch parameters. This template is eventually formatted and used to derive a file path (self.base_image) which is passed unquoted to a shell command in _take_snapshot (line 701). Because process.run() uses shlex.split() on the entire command string, any spaces in version or arch will result in additional arguments being injected into the qemu-img command. For example, a version string like 30 -o backing_file=/etc/shadow could allow an attacker to manipulate qemu-img options or potentially read sensitive files.

[gemini-code-assist]

The self.version property (which uses the user-supplied version parameter) is called in the new _get_image_url_from_mirror method. This property triggers a version discovery process that uses the version string as a regular expression pattern in VMImageHtmlParser. A malicious user can provide a crafted regex (e.g., (a+)+$) that causes catastrophic backtracking when matched against mirror HTML content, leading to a Denial of Service (ReDoS).

[gemini-code-assist]

The error message could be improved by including the specific Fedora version that failed, instead of a generic self._version. This would help in debugging.

Suggested change

	raise ImageProviderError(
	f"All mirrors failed for Fedora {self._version}. "
	f"Last error: {last_error}"
	)
	raise ImageProviderError(
	f"All mirrors failed for Fedora {self.version}. " # Use self.version to display the specific Fedora version
	f"Last error: {last_error}"
	)

[gemini-code-assist]

It's better to assign self.url_versions before self.url_mirrors to ensure self.url_versions is always initialized, even if self.MIRRORS is empty.

        super().__init__(version, build, arch)
        self.url_versions = self.MIRRORS[0]  # Default to first mirror
        self.url_mirrors = self.MIRRORS

[gemini-code-assist]

This change adds URLError to the exception handling, which is good for catching DNS failures. However, consider logging the exception message for debugging purposes, especially in production environments.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@avocado/utils/vmimage.py`:
- Around line 286-297: The code currently parses self._version to int (ver) and
selects image_pattern immediately, which misclassifies the default regex
"[0-9]+" as ver==0 and forces the legacy pattern; change this to defer selecting
or setting self.image_pattern until the concrete/resolved version is known (not
when self._version is a regex). Update the logic around the int conversion of
self._version and the pattern assignments (referencing ver, self._version, and
self.image_pattern) so that if self._version is non-numeric or a regex (e.g.,
contains non-digits or brackets) you skip pattern selection now and instead
choose the correct Fedora 40 vs 41+ vs legacy pattern after the actual resolved
version is obtained.
- Around line 233-247: The final ImageProviderError raised in get_image_url
currently only includes the last_error; change it to include the full list of
mirrors tried and (optionally) per-mirror errors: iterate self.url_mirrors
collecting each mirror and its exception message when
_get_image_url_from_mirror(mirror) raises, then raise ImageProviderError
containing a summary like "All mirrors failed for Fedora {self._version}: tried
mirrors [..]" and include the per-mirror error details (use last_error only if
needed). Update the raise in get_image_url to reference self.url_mirrors and the
collected error info so callers see all attempted mirrors and their errors.
- Around line 273-277: Annotate the class-level mutable sequences with
typing.ClassVar to satisfy Ruff RUF012: import ClassVar and the appropriate
collection type from typing (e.g., List[str]) and change the declarations such
as MIRRORS to use a ClassVar annotation (e.g., MIRRORS: ClassVar[List[str]] =
[...]); apply the same ClassVar/List[str] annotation to the other mutable class
attributes referenced around lines 310-313 so all mutable class attributes are
explicitly ClassVar-annotated.

[codecov]

Codecov Report

❌ Patch coverage is 89.39394% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.13%. Comparing base (fdf8aee) to head (e7c7e01).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
avocado/utils/vmimage.py	89.39%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6266      +/-   ##
==========================================
+ Coverage   75.02%   75.13%   +0.11%     
==========================================
  Files         206      206              
  Lines       22510    22617     +107     
==========================================
+ Hits        16888    16994     +106     
- Misses       5622     5623       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

avocado/utils/vmimage.py (1)

252-262: Potential ValueError if version property returns a non-numeric string.

When url_mirrors is empty/None, the code falls through to int(self.version). If version discovery fails or returns a non-numeric value, this will raise ValueError. Consider adding defensive handling similar to _get_image_url_from_mirror.

🛠️ Suggested fix

         # No mirrors configured, use default behavior
-        if int(self.version) >= 28:
+        try:
+            ver = int(self.version)
+        except ValueError:
+            ver = 99  # Assume recent version
+        if ver >= 28:
             cloud = "Cloud"
         else:
             cloud = "CloudImages"

-        if self.url_old_images and int(self.version) <= 40:
+        if self.url_old_images and ver <= 40:
             self.url_versions = self.url_old_images

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@avocado/utils/vmimage.py`:
- Around line 203-235: The code currently triggers version discovery via the
version property before trying mirrors, which can raise ImageProviderError and
short‑circuit fallback; modify the mirror-resolution flow so version parsing and
cloud/path selection happen inside each mirror attempt (e.g., move logic from
FedoraImageProvider.get_image_url()/FedoraSecondaryImageProvider.get_image_url()
into FedoraImageProviderBase._get_image_url_from_mirror) and ensure those
methods use the init value self._version (or a local parsed int) rather than
self.version, so a failing mirror only affects that iteration and remaining
mirrors are still tried; update similar blocks referenced around lines 321-329
and 370-377 to follow the same per‑mirror deferred discovery pattern.

[clebergnu]

Hi @harvey0100, thanks for this! Here are some comments for this round of review.