Draft: Add executor abstraction for provision step

[thrix]

Summary

Relates to #4570

This implements an executor/execution_driver abstraction that separates command execution logic from guest connectivity, enabling cleaner bootc/image-mode support across different provision types.

Key changes:

• Add ExecutionDriver and DeferrableExecutor base classes in tmt/steps/provision/executor/
• Implement SSHExecutor, LocalExecutor, ContainerExecutor, MockExecutor
• Add BootcContainerfileExecutor and DeferrableSSHExecutor for bootc support
• Integrate executors with Guest classes via _create_executor() method
• Remove CommandCollector mixin (replaced by executor pattern)

Benefits:

• Separation of concerns: Transport (SSH/local/container) separate from execution mode
• Composability: Easy to add new execution strategies
• Extensibility: Bootc support can be added to other guest types
• Backward compatibility: External Guest.execute() API unchanged
• Testability: Executors can be unit tested independently

Architecture

ExecutionDriver (ABC)                    # Base abstraction
    |
    +-- SSHExecutor                      # Execute via SSH
    +-- LocalExecutor                    # Execute locally
    +-- ContainerExecutor                # Execute via podman exec
    +-- MockExecutor                     # Execute via mock shell

DeferrableExecutor(ExecutionDriver)      # Commands can be batched
    |
    +-- BootcContainerfileExecutor       # Collect into Containerfile, build, reboot

DeferrableSSHExecutor(DeferrableExecutor)  # Composes SSH + Bootc deferral

Test plan

• Run tests/prepare/bootc/test.sh - all 6 assertions pass
• Run full test suite
• Type checking with mypy

🤖 Generated with Claude Code

Assisted-by: Claude Code

[gemini-code-assist]

Summary of Changes

Hello @thrix, 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 introduces a new executor/execution_driver abstraction to cleanly separate command execution logic from guest connectivity. This refactoring enables more robust and flexible support for various provision types, particularly enhancing bootc/image-mode capabilities. The changes improve separation of concerns, increase composability and extensibility for future execution strategies, and facilitate independent unit testing of execution components, all while maintaining backward compatibility for the external Guest.execute() API.

Highlights

• Executor Abstraction: Introduced ExecutionDriver and DeferrableExecutor base classes in tmt/steps/provision/executor/ to separate command execution logic from guest connectivity.
• Executor Implementations: Implemented concrete executors including SSHExecutor, LocalExecutor, ContainerExecutor, and MockExecutor.
• Bootc Support: Added BootcContainerfileExecutor and DeferrableSSHExecutor to provide specialized support for bootc/image-mode guests, enabling deferred command execution via Containerfile builds.
• Guest Integration: Integrated the new executor pattern with existing Guest classes through a _create_executor() method, allowing guests to dynamically select their execution strategy.
• CommandCollector Removal: Replaced the CommandCollector mixin with the new executor pattern, streamlining command handling.

Changelog

• docs/releases/pending/4495.fmf
  • Added a release note detailing the prepare/shell plugin's new support for image mode guests.
• tests/prepare/bootc/data/.fmf/version
  • Added a version file for the bootc test data.
• tests/prepare/bootc/data/plans.fmf
  • Added a test plan to install the 'tree' package on a bootc guest using prepare/shell.
• tests/prepare/bootc/data/test.fmf
  • Added a test to verify the successful installation of the 'tree' package and bootc status after prepare/shell execution on bootc.
• tests/prepare/bootc/main.fmf
  • Added a main test plan to verify prepare/shell command collection and execution via Containerfile on bootc guests.
• tests/prepare/bootc/test.sh
  • Added a shell script to execute and validate the bootc prepare/shell test, asserting Containerfile collection, image build, bootc switch, and reboot.
• tests/recipe/data/recipe.yaml
  • Added the is-image-mode fact to the guest facts schema.
• tmt/package_managers/bootc.py
  • Implemented dry-run checks in _get_base_containerfile_directives and build_container.
  • Modified build_container to mount run_workdir with the SELinux private label (:Z) during podman build.
• tmt/steps/init.py
  • Introduced _save_deferred_run_outcome to record INFO results for commands collected for deferred execution.
• tmt/steps/prepare/init.py
  • Added a call to guest.on_step_complete for each guest after the prepare step to handle deferred commands.
• tmt/steps/prepare/shell.py
  • Modified _invoke_script to pass immediately=False to guest.execute for commands that can be deferred.
  • Updated error handling to use _save_deferred_run_outcome for commands that are deferred.
• tmt/steps/provision/init.py
  • Imported DeferrableExecutor and ExecutionDriver.
  • Added is_image_mode to GuestFacts and a _query_is_image_mode method to detect image mode systems.
  • Updated the sync method to include is_image_mode fact synchronization.
  • Introduced an abstract _create_executor method and an executor property to the Guest base class.
  • Added an on_step_complete method to Guest to notify the executor when a step finishes.
  • Modified the execute method signature to include an immediately parameter and return Optional[CommandOutput].
  • Updated GuestSsh docstring to reflect image mode support and deferred execution.
  • Added _create_executor and _deferrable_executor properties to GuestSsh.
  • Overloaded the execute method in GuestSsh to handle the immediately parameter, delegating to the appropriate executor.
• tmt/steps/provision/executor/init.py
  • Added ExecutionDriver as an abstract base class for command execution.
  • Added DeferrableExecutor as an abstract base class for executors supporting deferred/batched command execution.
• tmt/steps/provision/executor/bootc.py
  • Added BootcContainerfileExecutor to collect commands into a Containerfile, build a new image, and reboot.
  • Added DeferrableSSHExecutor to compose SSH execution with bootc-specific deferred execution for image mode guests.
• tmt/steps/provision/executor/container.py
  • Added ContainerExecutor to encapsulate command execution within Podman containers using podman exec.
• tmt/steps/provision/executor/local.py
  • Added LocalExecutor to encapsulate local command execution logic.
• tmt/steps/provision/executor/mock.py
  • Added MockExecutor to encapsulate command execution within Mock shell environments.
• tmt/steps/provision/executor/ssh.py
  • Added SSHExecutor to encapsulate SSH command execution logic, including connection management and command building.
• tmt/steps/provision/local.py
  • Integrated LocalExecutor by adding a _create_executor method.
  • Updated the execute method signature to include the immediately parameter.
• tmt/steps/provision/mock.py
  • Integrated MockExecutor by adding a _create_executor method.
  • Updated the execute method signature to include the immediately parameter.
• tmt/steps/provision/podman.py
  • Integrated ContainerExecutor by adding a _create_executor method.
  • Updated the execute method signature to include the immediately parameter.

Activity

• No human activity (comments, reviews) has been recorded for this pull request yet.

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 significant refactoring by abstracting command execution into an ExecutionDriver, separating guest connectivity from execution logic to better support bootc and image-mode guests, and utilizes DeferrableExecutor for command batching. However, the implementation of the bootc executor and package manager contains critical security issues, including unquoted paths in tmt/package_managers/bootc.py and a lack of newline sanitization in tmt/steps/provision/executor/bootc.py, which could lead to command and directive injection vulnerabilities. Furthermore, the refactoring is incomplete, as GuestLocal, GuestContainer, and GuestMock have not been fully updated to use the new executor pattern, resulting in code duplication and inconsistencies in their execute methods.

I am having trouble creating individual review comments. Click here to see my feedback.

tmt/steps/provision/local.py (123)

The immediately parameter is added, but the method body is not updated to delegate to self.executor. The current implementation duplicates the logic from LocalExecutor and does not use the new executor pattern. This should be refactored to call self.executor.execute(...) to avoid code duplication and make the refactoring complete.

tmt/steps/provision/mock.py (543)

GuestMock inherits from GuestSsh, but its execute method overrides the parent's implementation without adopting the new executor pattern. The immediately parameter is added but unused.

This method should be refactored to delegate to self.executor for immediate execution and handle deferred execution (e.g., by calling super().execute(...) when immediately=False), similar to GuestSsh.execute. The current implementation duplicates logic from MockExecutor and prevents deferred execution from working on mock guests in image mode.

tmt/steps/provision/podman.py (437)

The immediately parameter is added, but the execute method body is not updated to delegate to self.executor. The current implementation duplicates the logic from ContainerExecutor and does not use the new executor pattern. This should be refactored to call self.executor.execute(...) to avoid code duplication and make the refactoring complete.

tmt/package_managers/bootc.py (280)

The podman build command constructed on line 280 uses self.guest.run_workdir multiple times without shell quoting. If the workdir path (which can be derived from the plan name in the test metadata) contains shell metacharacters such as spaces, semicolons, or backticks, it could lead to command injection or command failure on the guest. Given that tmt is often used to run tests from untrusted sources (e.g., pull quotes), this poses a security risk.

Recommendation: Use shlex.quote() for all path variables inserted into shell command strings, or preferably, use the Command class to build the command as a list of arguments to avoid shell interpretation issues.

tmt/steps/provision/executor/bootc.py (144)

In BootcContainerfileExecutor.defer, the collected_command is appended to a RUN directive in the Containerfile without sanitizing for newlines. If collected_command contains newlines (which is common for multi-line shell scripts provided in test metadata), it will terminate the RUN directive and the subsequent lines will be interpreted as new Containerfile directives. A malicious metadata author could use this to inject arbitrary directives such as COPY, USER, or ENV, potentially manipulating the image build process, accessing unauthorized files from the build context, or escalating privileges within the container.

Recommendation: Sanitize collected_command to ensure it does not contain unescaped newlines before appending it to the RUN directive, or use backslashes to escape newlines so the entire script remains part of the RUN directive.