refactor: copy files into role, copy containers into quay lsr#27
Merged
richm merged 1 commit intoApr 10, 2026
Merged
Conversation
copy quadlet files into role from external git repo copy container images into quay.io linux-system-roles Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Reviewer's GuideRefactors the trustee_quadlet role to stop cloning quadlet/config files from an external Git repo and instead ship static quadlet and config assets within the role, simplifying installation and KBS configuration handling. Flow diagram for updated trustee_quadlet.yml tasksflowchart TD
Start[Start trustee_quadlet tasks] --> CreateDirs[Ensure /etc/trustee-gc and systemd quadlet dir exist]
CreateDirs --> CopyQuadlet[Copy static quadlet files from role files/quadlet to __trustee_client_quadlet_install_dir]
CopyQuadlet --> CopyAAConfigs[Copy AA configs from files/configs/aa to /etc/trustee-gc/]
CopyAAConfigs --> CopyCDHConfigs[Copy CDH configs from files/configs/cdh to /etc/trustee-gc/]
CopyCDHConfigs --> SetKBSCert[Set __trustee_client_kbs_cert_content from path or explicit content]
SetKBSCert --> ReplaceKBSUrl[Replace KBS_URL in aa and cdh config.toml]
ReplaceKBSUrl -->|when trustee_client_kbs_url length > 0| ReplaceKBSCert[Replace KBS_CERT in aa and cdh config.toml]
ReplaceKBSCert -->|when __trustee_client_kbs_cert_content length > 0| WriteCert[Write KBS certificate to /etc/trustee-gc/server.crt]
WriteCert --> SevStat[Stat /dev/sev-guest device]
SevStat --> PodmanStat[Stat trustee-gc pod quadlet file]
PodmanStat --> EnableService[Enable and start trustee-gc pod with systemd]
EnableService --> End[End trustee_quadlet tasks]
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 1 issue, and left some high level feedback:
- In the
copytasks inside the role, you can simplify and make the role more conventional by using role-relative paths likesrc: quadlet/andsrc: configs/aainstead offiles/quadlet/andfiles/configs/aa, since Ansible automatically looks under the role’sfiles/directory. - By removing the final
meta: flush_handlers, any handlers (e.g. for systemd units) will now run only at the end of the play; if service startup is expected immediately after quadlet deployment, consider reintroducing a targetedflush_handlersat the appropriate point instead of dropping it entirely.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `copy` tasks inside the role, you can simplify and make the role more conventional by using role-relative paths like `src: quadlet/` and `src: configs/aa` instead of `files/quadlet/` and `files/configs/aa`, since Ansible automatically looks under the role’s `files/` directory.
- By removing the final `meta: flush_handlers`, any handlers (e.g. for systemd units) will now run only at the end of the play; if service startup is expected immediately after quadlet deployment, consider reintroducing a targeted `flush_handlers` at the appropriate point instead of dropping it entirely.
## Individual Comments
### Comment 1
<location path="tasks/trustee_quadlet.yml" line_range="16-19" />
<code_context>
- when: quadlet_files_found.files | length == 0
-
- name: Copy Trustee Guest Components quadlet files to install directory
ansible.builtin.copy:
- src: "{{ item.path }}"
- dest: "{{ __trustee_client_quadlet_install_dir }}/{{ item.path | basename }}"
+ src: files/quadlet/
+ dest: "{{ __trustee_client_quadlet_install_dir }}/"
mode: "0644"
- remote_src: true
- force: true
</code_context>
<issue_to_address>
**suggestion:** Consider specifying `directory_mode` for the quadlet copy to ensure directory permissions are consistent and explicit.
Since `src` is now the `files/quadlet/` directory, `mode` will only apply to files and directory permissions will use Ansible’s defaults. To match the config copy tasks and avoid relying on distribution defaults, please set an explicit `directory_mode` (e.g. `"0755"`) here as well.
Suggested implementation:
```
- name: Copy Trustee Guest Components quadlet files to install directory
ansible.builtin.copy:
src: files/quadlet/
dest: "{{ __trustee_client_quadlet_install_dir }}/"
mode: "0644"
directory_mode: "0755"
```
If there are other `ansible.builtin.copy` tasks in this role that copy directories (e.g. the “config copy tasks” you mentioned), consider aligning their `directory_mode` values as well to keep permissions consistent across the role.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
16
to
19
| ansible.builtin.copy: | ||
| src: "{{ item.path }}" | ||
| dest: "{{ __trustee_client_quadlet_install_dir }}/{{ item.path | basename }}" | ||
| src: files/quadlet/ | ||
| dest: "{{ __trustee_client_quadlet_install_dir }}/" | ||
| mode: "0644" |
There was a problem hiding this comment.
suggestion: Consider specifying directory_mode for the quadlet copy to ensure directory permissions are consistent and explicit.
Since src is now the files/quadlet/ directory, mode will only apply to files and directory permissions will use Ansible’s defaults. To match the config copy tasks and avoid relying on distribution defaults, please set an explicit directory_mode (e.g. "0755") here as well.
Suggested implementation:
- name: Copy Trustee Guest Components quadlet files to install directory
ansible.builtin.copy:
src: files/quadlet/
dest: "{{ __trustee_client_quadlet_install_dir }}/"
mode: "0644"
directory_mode: "0755"
If there are other ansible.builtin.copy tasks in this role that copy directories (e.g. the “config copy tasks” you mentioned), consider aligning their directory_mode values as well to keep permissions consistent across the role.
Collaborator
Author
|
[citest] |
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
1 participant
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.
copy quadlet files into role from external git repo
copy container images into quay.io linux-system-roles
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Inline trustee quadlet and config files into the role instead of downloading them from an external repository, and simplify related configuration handling.
New Features:
Enhancements:
Tests: