-
Notifications
You must be signed in to change notification settings - Fork 25
Add support for usbip needed for virtual passkey #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @spoore1, I'm Gemini Code Assist1! 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 infrastructure for virtual passkey testing by integrating Highlights
🧠 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. Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for usbip, which is needed for virtual passkey testing. The changes involve adding Ansible tasks to install usbip from packages on Fedora, and building it from source on CentOS Stream 10 and RHEL 10. My review focuses on improving the Ansible playbooks for better maintainability, security, and robustness. Key suggestions include simplifying task structures, avoiding GPG check disabling, ensuring temporary file cleanup, and making variable definitions more robust.
| - name: Create temporary build directory | ||
| tempfile: | ||
| state: directory | ||
| suffix: usbip_build | ||
| register: build_dir | ||
|
|
||
| - name: Ensure Koji config directory exists | ||
| file: | ||
| path: "~/.koji/config.d" | ||
| state: directory | ||
| mode: '0755' | ||
|
|
||
| - name: Configure CentOS Stream Koji profile | ||
| copy: | ||
| dest: "~/.koji/config.d/centos-stream.conf" | ||
| content: | | ||
| [centos-stream] | ||
| server = https://kojihub.stream.centos.org/kojihub | ||
| topurl = https://kojihub.stream.centos.org/kojifiles | ||
|
|
||
| - name: Download Kernel Source RPM via Koji | ||
| command: | ||
| cmd: "koji --profile centos-stream download-build --arch=src kernel-{{ kver }}" | ||
| chdir: "{{ build_dir.path }}" | ||
| args: | ||
| creates: "{{ build_dir.path }}/kernel-{{ kver }}.src.rpm" | ||
|
|
||
| - name: Extract Source RPM and Kernel Tarball | ||
| shell: | | ||
| set -ex | ||
| rpm2cpio kernel-{{ kver }}.src.rpm | cpio -id | ||
| xz -dc linux-{{ kver }}.tar.xz | tar xf - | ||
| args: | ||
| chdir: "{{ build_dir.path }}" | ||
| creates: "{{ build_dir.path }}/linux-{{ kver }}/tools/usb/usbip/configure" | ||
|
|
||
| - name: Build and Install USBIP tools | ||
| shell: | | ||
| set -ex | ||
| ./autogen.sh | ||
| ./configure | ||
| make install | ||
| args: | ||
| chdir: "{{ build_dir.path }}/linux-{{ kver }}/tools/usb/usbip/" | ||
| creates: "/usr/local/sbin/usbip" | ||
|
|
||
| - name: Create symlink for usbip | ||
| file: | ||
| src: /usr/local/sbin/usbip | ||
| dest: /usr/sbin/usbip | ||
| state: link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The temporary build directory created by tempfile is not cleaned up after the playbook runs, which can leave artifacts on the system. It's a good practice to use a block with an always section to ensure the temporary directory is removed, even if tasks fail.
- name: Build and install usbip from source
block:
- name: Create temporary build directory
tempfile:
state: directory
suffix: usbip_build
register: build_dir
- name: Ensure Koji config directory exists
file:
path: "~/.koji/config.d"
state: directory
mode: '0755'
- name: Configure CentOS Stream Koji profile
copy:
dest: "~/.koji/config.d/centos-stream.conf"
content: |
[centos-stream]
server = https://kojihub.stream.centos.org/kojihub
topurl = https://kojihub.stream.centos.org/kojifiles
- name: Download Kernel Source RPM via Koji
command:
cmd: "koji --profile centos-stream download-build --arch=src kernel-{{ kver }}"
chdir: "{{ build_dir.path }}"
args:
creates: "{{ build_dir.path }}/kernel-{{ kver }}.src.rpm"
- name: Extract Source RPM and Kernel Tarball
shell: |
set -ex
rpm2cpio kernel-{{ kver }}.src.rpm | cpio -id
xz -dc linux-{{ kver }}.tar.xz | tar xf -
args:
chdir: "{{ build_dir.path }}"
creates: "{{ build_dir.path }}/linux-{{ kver }}/tools/usb/usbip/configure"
- name: Build and Install USBIP tools
shell: |
set -ex
./autogen.sh
./configure
make install
args:
chdir: "{{ build_dir.path }}/linux-{{ kver }}/tools/usb/usbip/"
creates: "/usr/local/sbin/usbip"
- name: Create symlink for usbip
file:
src: /usr/local/sbin/usbip
dest: /usr/sbin/usbip
state: link
always:
- name: Clean up temporary build directory
file:
path: "{{ build_dir.path }}"
state: absent
when: build_dir.path is definedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it may be better to leave the directory behind for debugging in the event that something fails. I'll be looking for more input on this one though.
34af684 to
f7cee5a
Compare
- passkey usbip ansible code converted from shell script with gemini. - Also updating keycloak dep package to java-25-openjdk-headless Assisted-by: Gemini
No description provided.