Repository navigation
Refresh documentation for current Quadlet, lifecycle, paths, and networking - #409
Merged
Merged
Conversation
Signed-off-by: Ryan Cook <rcook@redhat.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the methods documentation page to provide inline Quadlet usage and requirements while consolidating links and examples for recent deployment, lifecycle, rollback, and operational features; strict Sphinx and diff validation passed. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 27 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/methods.rst" line_range="457" />
<code_context>
+ - name: web
+ targetPath: examples/quadlet/units/
+ schedule: "*/1 * * * *"
+ root: true
+ start: true
+ restart: true
</code_context>
<issue_to_address>
**issue (review_instructions):** The runnable example enables rootful deployment without explaining the privilege risk or requiring it only when necessary.
Running the helper with host administrative access can allow a compromised bundle or image to affect the entire host. Prefer `root: false` and rootless Podman by default; if rootful deployment is required, explicitly document the minimum permissions and isolation requirements.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify potential security risks and provide a safer alternative.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=77cf81e0-aaed-48cb-bc7b-43cb0ecdb17b&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 2
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** The documentation warns that the helper has administrative access but does not provide an actionable image-integrity safeguard.
Using the mutable `quay.io/fetchit/fetchit:latest` default makes deployments vulnerable to tag changes or registry compromise. Recommend pinning `helperImage` to an immutable digest and verifying its signature/provenance, for example: `helperImage: quay.io/fetchit/fetchit@sha256:<approved-digest>`.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment explains the issue clearly and suggests a specific, actionable solution.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=77cf81e0-aaed-48cb-bc7b-43cb0ecdb17b&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 3
<location path="docs/methods.rst" line_range="139" />
<code_context>
+``cleanupOnRemoval`` defaults to false. Removing a method from configuration
+retains its resources unless cleanup was enabled beforehand. The method sections
+and :doc:`lifecycle` explain ownership, migration, and recovery requirements.
+Trailing slashes in ``targetPath`` are optional; the Git target paths section
+above explains normalization and FileTransfer's flat destination layout.
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Please add tests covering target-path normalization edge cases.
In particular, test equivalent paths with and without trailing slashes, root or empty paths, and paths that could cause identity collisions. This documentation introduces normalization and ownership behavior that should be protected by explicit coverage.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that suggest adding tests for edge cases or invalid inputs to improve code robustness.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=99de523b-cf4c-499c-848e-2830fb0292eb&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 4
<location path="docs/methods.rst" line_range="134" />
<code_context>
+ - Optional tracked-file cleanup
+ * - ``ansible``
+ - Host configuration through playbooks
+ - No automatic undo of playbook changes
+
+``cleanupOnRemoval`` defaults to false. Removing a method from configuration
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Add a minimal Ansible configuration example and explicit recovery guidance.
The method-selection table identifies Ansible's lack of automatic rollback, but gives users no actionable pattern for handling that limitation. For example, add a snippet such as `ansible:\n- name: configure-host\n targetPath: playbooks/site.yml\n schedule: "*/15 * * * *"` and explain that the playbook should be idempotent and that reverting requires a compensating playbook or restoring the host manually. This makes the operational behavior easier to understand and gives the documentation a testable, runnable starting point.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Suggest actionable refactoring or code improvements that enhance readability, maintainability, or testability, with clear examples.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5c09625f-903f-40dc-92f9-737652f634c6&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 5
<location path="docs/methods.rst" line_range="587" />
<code_context>
+cleanup. Enable tracking while a method is still configured, before removing it.
+FileTransfer and Systemd refuse to adopt pre-existing untracked files; follow the
+migration steps in :doc:`lifecycle` before enabling cleanup on existing deployments.
+Preserve the FetchIt volume and host journals across upgrades.
+
+Git targets containing only Raw, Kube, or Quadlet may also opt into apply rollback:
</code_context>
<issue_to_address>
**issue (review_instructions):** Explain why volumes and journals must be preserved across upgrades.
This is an important operational recommendation, but the documentation does not explain what functionality or recovery data would be lost if they were removed. Add a brief rationale or link it directly to the lifecycle/rollback behavior described below.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment addresses a clear and necessary improvement or correction in the code or documentation, and provides a clear rationale or explanation for the suggested change.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=fdcfaed2-0f15-4af5-9a7d-297ce8a56695&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 6
<location path="docs/methods.rst" line_range="503" />
<code_context>
+
+``cleanupOnRemoval: true`` enables persisted bundle cleanup when the method leaves
+configuration. Cleanup stops journal-owned services and removes its bundle;
+volumes and other resources follow the authored units' stop behavior. Defaults
+retain workloads. See :doc:`quadlet` for the full setup, supported unit types,
+inspection commands, and migration limits, and :doc:`lifecycle` for retry and
</code_context>
<issue_to_address>
**issue (review_instructions):** Explain precisely whether named volumes are removed during Quadlet cleanup.
“Follow the authored units' stop behavior” is ambiguous for destructive cleanup: readers may not know whether a named volume is preserved, stopped, or deleted. Please state the exact behavior and give a concrete example, such as whether ``cleanupOnRemoval: true`` removes a volume declared by a ``.volume`` unit or only removes the deployed unit files.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Verify that the comment addresses a potential source of confusion or ambiguity in the code or documentation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=15391db5-3e4d-44d0-a8f4-14b1499ae578&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 7
<location path="docs/methods.rst" line_range="503" />
<code_context>
+
+``cleanupOnRemoval: true`` enables persisted bundle cleanup when the method leaves
+configuration. Cleanup stops journal-owned services and removes its bundle;
+volumes and other resources follow the authored units' stop behavior. Defaults
+retain workloads. See :doc:`quadlet` for the full setup, supported unit types,
+inspection commands, and migration limits, and :doc:`lifecycle` for retry and
</code_context>
<issue_to_address>
**issue (review_instructions):** Cleanup impact on persistent volumes and other resources is too ambiguous.
A user enabling cleanup needs to know whether named volumes, networks, and generated services are stopped, deleted, or retained. This ambiguity creates a risk of either unexpected data loss or leaked resources during method removal. Specify the exact cleanup behavior for each resource type and point to an explicit preservation procedure for persistent data.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify and clearly describe the issue or risk in the code, such as a bug or potential problem.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=058edb80-ca38-412f-beb1-7f023822720b&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 8
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** Using a mutable `latest` helper image is a supply-chain and reproducibility risk.
The next paragraph says the helper uses host binaries and administrative filesystem access, but the default is `quay.io/fetchit/fetchit:latest`. A tag can change between deployments and introduce different privileged behavior; recommend pinning a digest (or at least an immutable version) by default or explicitly warn that operators must pin it for production.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify potential bugs or risks in the code.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=8a7aba75-59c7-4fb6-99cc-848bb5f45de2&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 9
<location path="docs/methods.rst" line_range="468" />
<code_context>
+unsupported because it could omit dependencies. Bundles are limited to 32 MiB
+and 4,096 files.
+
+``root`` defaults to false. ``start`` defaults to false and starts services after
+deployment. ``restart`` defaults to false and implies ``start``; changed supporting
+files also trigger workload restarts. Network and volume services are started
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Remove the ambiguity around the ``start`` default and behavior.
As written, “``start`` defaults to false and starts services” is contradictory. Consider: “``start`` defaults to false; set it to true to start services after deployment.”
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear and actionable suggestion for improvement.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=4ea295bd-cb2d-4d56-9aa4-6a934ebb653d&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 10
<location path="docs/methods.rst" line_range="106" />
<code_context>
=======
-Various methods are available to lifecycle and manage the container environment on a host. Funcionality also exists to
-allow for files or directories of files to be deployed to the container host to be used by containers.
+Methods manage containers, host services, and files. The examples below show the
+configuration keys used inside ``targetConfigs``. Use a FetchIt image containing
+the documented features; see :doc:`release_notes` for unreleased changes.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Specify the minimum FetchIt image version or a concrete compatible image tag.
“Containing the documented features” is not actionable because readers cannot determine which image satisfies that requirement. Naming the required version or tag would make the setup reproducible and align with the later Quadlet compatibility guidance.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear and actionable suggestion for improvement.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=65ae85eb-b6df-41fd-aa4d-9d78b08b8729&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 11
<location path="docs/methods.rst" line_range="439" />
<code_context>
--------------
-See :doc:`quadlet` for host-managed Podman Quadlet bundles, configuration, and lifecycle behavior.
+The ``quadlet`` method installs a complete Git bundle into the host's Quadlet
+search path. The host Podman generator validates the bundle and generates systemd
+services. Use host Podman **5.7 or newer within major version 5**, cgroup v2, and a
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Move the detailed Quadlet guidance into the dedicated ``quadlet`` documentation.
This section now contains extensive setup, lifecycle, and recovery details while also referring readers to :doc:`quadlet` for the full behavior. Keeping ``methods.rst`` focused on method selection and a concise configuration summary, with the detailed material consolidated in ``docs/quadlet.rst`` and linked here, would avoid duplication and make future updates more maintainable.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear suggestion for improving the organization or structure of the code or documentation, with rationale and references to relevant sections.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5823e292-3e9c-49a0-ad36-4eb208fb9cbd&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 12
<location path="docs/methods.rst" line_range="580" />
<code_context>
complete encrypted Secret/Pod example, configuration, key rotation, deletion,
limits, and recovery. Ordinary Kube methods retain their current behavior.
+
+Cleanup, rollback, and method identity
+--------------------------------------
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Organize the new cleanup and rollback material into the lifecycle documentation.
The cleanup, migration, rollback, and retained-resource rules in this new section overlap with the referenced :doc:`lifecycle` page. Moving the operational details there and retaining only a short overview and link in ``docs/methods.rst`` would give these related rules one authoritative location and reduce documentation drift.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear suggestion for improving the organization or structure of the code or documentation, with rationale and references to relevant sections.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5823e292-3e9c-49a0-ad36-4eb208fb9cbd&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 13
<location path="docs/methods.rst" line_range="468" />
<code_context>
+unsupported because it could omit dependencies. Bundles are limited to 32 MiB
+and 4,096 files.
+
+``root`` defaults to false. ``start`` defaults to false and starts services after
+deployment. ``restart`` defaults to false and implies ``start``; changed supporting
+files also trigger workload restarts. Network and volume services are started
</code_context>
<issue_to_address>
**issue (review_instructions):** issue: The documented default for `start` contradicts the stated behavior.
This says `start` defaults to false but also starts services after deployment. Please clarify that services start after deployment only when `start: true` (or correct the documented default).
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify and point out discrepancies or inaccuracies in documentation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=09d0061e-0fcb-4f4a-a6ad-7096cf4cf877&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 14
<location path="docs/methods.rst" line_range="589" />
<code_context>
+migration steps in :doc:`lifecycle` before enabling cleanup on existing deployments.
+Preserve the FetchIt volume and host journals across upgrades.
+
+Git targets containing only Raw, Kube, or Quadlet may also opt into apply rollback:
+
+.. code-block:: yaml
</code_context>
<issue_to_address>
**issue (review_instructions):** Specify how rollback behaves for targets containing other methods.
“Containing only” leaves the behavior of mixed targets ambiguous. Please explicitly state whether ``rollback`` is rejected, ignored, or applies only to the Raw/Kube/Quadlet methods when FileTransfer, Systemd, or Ansible entries are also present, with a small configuration example if supported.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Verify that the comment addresses potential ambiguities or misunderstandings in the code or documentation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=7e710e50-7c98-4c6c-84b4-935be462fec6&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 15
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** Document digest pinning and signature verification for the privileged helper image.
This helper uses host binaries and administrative filesystem access, so “use trusted repositories and images” is not sufficiently actionable. Recommend a pinned digest and fail-closed signature verification, for example: ``helperImage: quay.io/fetchit/fetchit@sha256:<approved-digest>`` plus verification with the deployment platform's cosign policy; abort the deployment if the digest or signature cannot be verified.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment addresses a potential security issue or improvement and provides a clear, actionable suggestion with implementation details.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=10c7919b-ce44-454b-aece-6a73c8875795&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 16
<location path="docs/methods.rst" line_range="623" />
<code_context>
+checks, and authentication. SSH setup is documented above; verify host keys
+rather than disabling verification.
+
+Set ``FETCHIT_STATUS_ADDR`` to expose optional ``/healthz`` and ``/status`` HTTP
+endpoints. See :doc:`status` for binding and monitoring examples. Neither endpoint
+is enabled by default.
</code_context>
<issue_to_address>
**issue (review_instructions):** Document safe binding and access controls for the status endpoints.
These endpoints may disclose service and deployment information if bound to a non-loopback address. Document a secure default and an explicit fail-closed configuration, such as ``FETCHIT_STATUS_ADDR=127.0.0.1:8080`` for local monitoring, or require TLS/authentication before allowing ``0.0.0.0:8080``; reject startup when a public bind lacks those controls.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment addresses a potential security issue or improvement and provides a clear, actionable suggestion with implementation details.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=10c7919b-ce44-454b-aece-6a73c8875795&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 17
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** Using the mutable ``latest`` helper image can introduce incompatible behavior without a configuration change.
Pin the default helper image to a version or digest known to be compatible with the documented Podman release, and document how to update it. For example:
.. code-block:: yaml
helperImage: quay.io/fetchit/fetchit:1.2.3@sha256:<digest>
This makes deployments reproducible and prevents an upstream image update from unexpectedly breaking Quadlet setup or cleanup.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment identifies a potential bug risk or issue in the code and suggests a specific, actionable improvement.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=68cd1c97-e0eb-41f3-92a8-e4ec616def7f&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 18
<location path="docs/methods.rst" line_range="439" />
<code_context>
--------------
-See :doc:`quadlet` for host-managed Podman Quadlet bundles, configuration, and lifecycle behavior.
+The ``quadlet`` method installs a complete Git bundle into the host's Quadlet
+search path. The host Podman generator validates the bundle and generates systemd
+services. Use host Podman **5.7 or newer within major version 5**, cgroup v2, and a
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Keep the Quadlet overview here and move detailed lifecycle/setup guidance to the dedicated Quadlet documentation.
This section now duplicates substantial material that is already referenced as ``:doc:`quadlet``` below, including supported versions, rootless paths, helper images, cleanup, and lifecycle behavior. Please retain a concise method summary and link to ``quadlet.rst`` for the operational details so the two documents do not drift.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
The comment should address consistency or organization issues within the project structure.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=43a22178-7557-4e59-abf4-35e3f76d883e&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 19
<location path="docs/methods.rst" line_range="580" />
<code_context>
complete encrypted Secret/Pod example, configuration, key rotation, deletion,
limits, and recovery. Ordinary Kube methods retain their current behavior.
+
+Cleanup, rollback, and method identity
+--------------------------------------
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Centralize the new cleanup and rollback rules in the lifecycle documentation rather than duplicating them here.
The added section repeats ownership, removal, rollback, and retained-resource guidance while also referring readers to ``:doc:`lifecycle```. Please keep only a short cross-reference in ``methods.rst`` and make ``lifecycle.rst`` the authoritative location, with specific links to the relevant subsections.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
The comment should address consistency or organization issues within the project structure.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=43a22178-7557-4e59-abf4-35e3f76d883e&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 20
<location path="docs/methods.rst" line_range="439" />
<code_context>
--------------
-See :doc:`quadlet` for host-managed Podman Quadlet bundles, configuration, and lifecycle behavior.
+The ``quadlet`` method installs a complete Git bundle into the host's Quadlet
+search path. The host Podman generator validates the bundle and generates systemd
+services. Use host Podman **5.7 or newer within major version 5**, cgroup v2, and a
</code_context>
<issue_to_address>
**suggestion (review_instructions):** This section duplicates the dedicated Quadlet documentation unnecessarily.
Most of the behavior described here—supported versions, rootless paths, defaults, helper images, cleanup, and migration—is already delegated to :doc:`quadlet` below. Consider reducing this page to a short overview and runnable configuration, for example: ``See :doc:`quadlet` for supported host versions, paths, lifecycle behavior, and troubleshooting.`` This avoids two sources of truth that can drift.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify redundant or unnecessary code, especially when a built-in feature or parameter already handles the situation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=7bbb5bc0-90a9-4be4-932e-7a13f3b564df&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 21
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** Using a mutable ``latest`` helper image creates a supply-chain and reproducibility risk.
The following line makes deployments implicitly depend on whatever image is currently published under ``latest``. A compromised or incompatible update could gain the documented host-binary and administrative filesystem access, or break deployment unexpectedly. Pin the default to a version or digest, and document an explicit upgrade process.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify and highlight potential bugs or risks in the code, such as missing syntax elements that could lead to unintended behavior.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5326d396-ab1b-4134-b556-367c52b2b055&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 22
<location path="docs/methods.rst" line_range="468" />
<code_context>
+unsupported because it could omit dependencies. Bundles are limited to 32 MiB
+and 4,096 files.
+
+``root`` defaults to false. ``start`` defaults to false and starts services after
+deployment. ``restart`` defaults to false and implies ``start``; changed supporting
+files also trigger workload restarts. Network and volume services are started
</code_context>
<issue_to_address>
**issue (review_instructions):** The `start` behavior is described contradictorily, which could lead to services not being started as intended.
As written, `start: false` appears to start services after deployment. Please clarify that services are started only when `start: true` (or correct the default/behavior), and state how this interacts with `restart: true`.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify potential bugs or risks in the code and provide a clear explanation of the issue; suggest specific improvements or revisions.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=f09cf967-42fb-43b3-896e-6b72e9cad5be&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 23
<location path="docs/methods.rst" line_range="136" />
<code_context>
+ - Host configuration through playbooks
+ - No automatic undo of playbook changes
+
+``cleanupOnRemoval`` defaults to false. Removing a method from configuration
+retains its resources unless cleanup was enabled beforehand. The method sections
+and :doc:`lifecycle` explain ownership, migration, and recovery requirements.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Clarify that cleanup must be enabled before removal and specify what “removing a method” means.
This is ambiguous about whether removal means deleting the method entry, removing the Git target, or disabling the whole target configuration. Consider: “If a method entry is removed from a target configuration, its resources are retained unless `cleanupOnRemoval: true` was enabled before that change.”
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear and actionable suggestion for improvement; verify that the suggestion enhances clarity and conciseness of the text; check that the comment includes a specific example or rephrasing to illustrate the suggestion.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=2bfb8466-4134-4442-a1f2-4c5bb6ccce1e&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 24
<location path="docs/methods.rst" line_range="496" />
<code_context>
+host configuration directory and enable lingering if services must survive logout.
+Rootful deployment requires ``/etc/containers`` on the host.
+
+``helperImage`` optionally selects a compatible FetchIt helper; its default is
+``quay.io/fetchit/fetchit:latest``. Engine and helper images must include Quadlet
+support. The helper uses host binaries and administrative filesystem access, so
</code_context>
<issue_to_address>
**issue (review_instructions):** Recommend pinning helper images instead of relying on the mutable ``latest`` tag.
Because the helper uses host binaries and administrative filesystem access, a compromised or replaced image could affect the host. Document digest pinning and, ideally, validation against an allowlist of trusted helper images; retain ``latest`` only as an explicitly acknowledged development default.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that suggest security improvements, such as input validation, especially when they prevent potential runtime failures or malicious use.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=d61aca01-181f-4b01-a1fb-8449fd0887e6&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 25
<location path="docs/methods.rst" line_range="484" />
<code_context>
+ targetPath: examples/quadlet/units
+ schedule: "*/1 * * * *"
+ root: false
+ hostHome: /home/operator
+ hostConfigHome: /home/operator/.config
+ hostRuntimeDir: /run/user/1234
</code_context>
<issue_to_address>
**issue (review_instructions):** Document validation and confinement requirements for user-supplied host paths.
These paths are used outside the FetchIt container and the surrounding text grants the helper host filesystem access. Specify that paths must be absolute, normalized, confined to approved directories, and owned by the intended user (reject traversal, symlinks, and sensitive locations) before deployment; otherwise a misconfiguration or malicious repository/configuration could expose or overwrite host files.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that suggest security improvements, such as input validation, especially when they prevent potential runtime failures or malicious use.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=d61aca01-181f-4b01-a1fb-8449fd0887e6&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 26
<location path="docs/methods.rst" line_range="143" />
<code_context>
+above explains normalization and FileTransfer's flat destination layout.
+
+See :doc:`samples` for current runnable applications, amd64/arm64 images, ports,
+and archive-loading instructions. HTTP examples use container port 8080 and the
+FetchIt sample image; ``APP_COLOR`` remains a configuration example and does not
+change the page color.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** This samples-specific detail is out of scope for the Methods overview and adds distracting, low-value information.
The surrounding paragraph already points readers to ``samples`` for application-specific details. Keeping the overview focused on method behavior would improve clarity; move the ``APP_COLOR`` and page-color clarification to the relevant sample documentation if it is needed.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Verify that the suggestion is necessary and adds value to the code or documentation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=62035705-16c9-4382-aa99-e167b9500af8&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 27
<location path="docs/methods.rst" line_range="468" />
<code_context>
+unsupported because it could omit dependencies. Bundles are limited to 32 MiB
+and 4,096 files.
+
+``root`` defaults to false. ``start`` defaults to false and starts services after
+deployment. ``restart`` defaults to false and implies ``start``; changed supporting
+files also trigger workload restarts. Network and volume services are started
</code_context>
<issue_to_address>
**issue (review_instructions):** The description of `start` is contradictory and could lead to services not starting as expected.
As written, it says `start` defaults to false and starts services, which conflicts with the apparent meaning of the option and the example using `start: true`. Clarify that `start: true` starts services after deployment, while the default `false` leaves them stopped.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify potential bugs or risks in the code, such as typos in critical values like IP addresses.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=59964304-38e3-4f4b-abff-d04f315cf630&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>Signed-off-by: Ryan Cook <rcook@redhat.com>
…havior Signed-off-by: Ryan Cook <rcook@redhat.com>
cooktheryan
changed the title
Signed-off-by: Ryan Cook <rcook@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refresh the Read the Docs guides to match current main-branch functionality. Define rootful/rootless Quadlet usage directly on the methods page and make cleanup, rollback, normalized Git paths, Podman networks, mirrors, status, and current multi-platform samples discoverable.
Correct stale launch instructions: configuration mounts at /opt/mount/config.yaml, consistent root/user service names and daemon reloads, explicit host identity for tracked rootless services, volume persistence, and reload behavior. Align Quadlet removal/rollback guidance and plaintext Kube API behavior with the implementation. Repair authentication/YAML examples and replace unsupported FETCHIT_CONFIG instructions with the supported file/URL setup.
Add a documentation publishing guide explaining Read the Docs version activation, configuration filename, historical-tag failures, and local versus hosted builds. Remove the hardcoded 0.1 docs label in favor of the selected Read the Docs version or development.
Validation: strict Sphinx build of every page, parsing all 27 YAML examples, and git diff checks pass. Hosted publication still requires merging and a Read the Docs build of the updated revision.
Sourcery feedback: favor rootless inline usage, clarify start/restart and exact resource retention, consolidate operational details, document approved helper digests and source compatibility, and explain persistence, Ansible recovery, mixed-target rollback rejection, and status access controls. See docs/design/documentation-review-resolution.md for decisions and existing regression coverage.
Summary by Sourcery
Align the documentation with current application behavior and provide accurate guidance for setup, operation, recovery, and Read the Docs publishing.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: