Tests support for vm boot options

[rvykydal]

The use of the test (and results of a run) can be seen in scope of network tests for another PR: #1087 (comment).

• The test/run change (definition of bootopts-net1 scenario) should be probably moved from this PR to the networking PR as well. It illustrates the usage.

[gemini-code-assist]

Summary of Changes

Hello @rvykydal, 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 testing framework by providing more granular control over virtual machine test environments. It introduces a robust mechanism to specify and apply custom boot options to VMs during testing and allows tests to be conditionally executed based on the VM setup. This improvement increases the flexibility and coverage of VM-related tests, particularly for scenarios requiring specific boot configurations like network setup via kernel parameters.

Highlights

• New Test Filtering Mechanism: Introduced a decorator @run_on_vm_setups and associated logic to allow tests to specify which VM setups they are compatible with, automatically skipping them otherwise. This enhances test execution control based on environment variables.
• Support for Extra VM Boot Arguments: Added functionality to pass arbitrary extra boot arguments to the virt-install command via the TEST_EXTRA_BOOT_ARGS environment variable, enabling more flexible VM configuration during tests.
• New bootopts-net1 Test Scenario: Implemented a new test scenario that leverages the new boot argument functionality to test VM installations with network configuration provided directly through boot options, demonstrating the new capabilities.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 support for running tests in different VM setups, allowing extra boot options to be passed to the VM for testing various installation scenarios. While the overall implementation for adding a new decorator and logic to skip tests based on VM setup is solid, a critical command injection vulnerability has been identified in test/machine_install.py. This vulnerability arises from insecurely handling the TEST_EXTRA_BOOT_ARGS environment variable when constructing shell commands and must be addressed by properly escaping the input using shlex.quote().

[sourcery-ai]

Hey - I've left some high level feedback:

• TEST_EXTRA_BOOT_ARGS is interpolated directly into the virt-install shell command; consider using proper shell-escaping (e.g., shlex.quote or passing arguments as a list) to avoid breakage or injection issues when the value contains spaces or quotes.
• The skip message for unsupported VM setups currently prints the raw list representation (e.g., ['','net1']); consider formatting this as a comma-separated string to make the required setups clearer in test output.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- TEST_EXTRA_BOOT_ARGS is interpolated directly into the virt-install shell command; consider using proper shell-escaping (e.g., shlex.quote or passing arguments as a list) to avoid breakage or injection issues when the value contains spaces or quotes.
- The skip message for unsupported VM setups currently prints the raw list representation (e.g., ['','net1']); consider formatting this as a comma-separated string to make the required setups clearer in test output.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[KKoukiou]

This creates a new test scenario here. We need to also define it here if we want it to run:
https://github.com/cockpit-project/bots/blob/main/lib/testmap.py#L25

[KKoukiou]

We don't want these options by default for all tests?

[rvykydal]

This creates a new test scenario here. We need to also define it here if we want it to run: https://github.com/cockpit-project/bots/blob/main/lib/testmap.py#L25

Yes, thanks for the pointer, that would be the final step.
In fact, I think the test/run change should go in only with the use of the scenario, ie with 50ed010 (updated)

[rvykydal]

We don't want these options by default for all tests?

No, we want to set these specific options for special network tests as in 50ed010 of #1087. For example this test's decorator contains also "" which means the tests should run both on default setup (I think others scenario) and the special bootopts-net1 setup.

In the future we might to want add a vm_setup for two network devices configured by boot options (which means added to the vm when creating it) (There is a separate issue for it).

[rvykydal]

Updated for Gemini review.

The feature is used in #1087 with Add network configuration tests for boot options configured device

See the fedora-rawhide-boot/bootopts-net1 check result (triggered by bots/tests-trigger --repo rhinstaller/anaconda-webui --force --allow 1087 fedora-rawhide-boot/bootopts-net1)

For example

   @run_on_vm_setups("", "bootopts-net1")
    def testConfigurationPreInstall(self):

was run also in fedora-rawhide-boot/other as expected ("" argument of run_on_vm_setups)