Jose/ansible_role_proxmox_provision
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor ♻️: Refactor legacy and minified proxmoxlib.js patching logic to use handlers #2

Merged
Jose merged 7 commits from dev into main 2026-02-08 08:06:27 +01:00
Conversation 3 Commits 7 Files Changed 4 +11 -11
2 changed files with 11 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1f2ff1d9ba - Show all commits

10
handlers/main.yml
View File

@@ -15,9 +15,9 @@
ansible.builtin.systemd_service:
daemon_reexec: true
- name: patch legacy proxmoxlib.js
- name: Ppatch legacy proxmoxlib.js
block:
- name: Patch legacy proxmoxlib.js
- name: Ppatch legacy proxmoxlib.js
gitea-actions commented 2026-02-08 07:13:32 +01:00
Outdated
Review
Copy Link

[Lines 18-20] [Score: 3] Consider adding variable names for clarify and easier maintenance. For example: patch_legacy_proxmoxlib, patch_minified_proxmoxlib_min.

[Lines 18-20] [Score: 3] Consider adding variable names for clarify and easier maintenance. For example: `patch_legacy_proxmoxlib`, `patch_minified_proxmoxlib_min`.
ansible.builtin.replace:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js
regexp: "if \\(data.status !== 'Active'\\)"
@@ -41,11 +41,11 @@
content: "{{ proxmoxlib_js_after.stat.checksum }}\n"
gitea-actions commented 2026-02-08 08:05:25 +01:00
Review
Copy Link

[Score: 3] Using a variable for the checksum directly in the file content could potentially expose sensitive information, especially if this playbook is run on an unsecured machine or shared environment. Consider hashing the checksum value before using it to avoid such risks.

[Score: 3] Using a variable for the checksum directly in the file content could potentially expose sensitive information, especially if this playbook is run on an unsecured machine or shared environment. Consider hashing the checksum value before using it to avoid such risks.
- name: Restart pveproxy
systemd:
ansible.builtin.systemd:
name: pveproxy
state: restarted
- name: patch minified proxmoxlib.js
- name: Patch minified proxmoxlib.js
block:
- name: Patch minified proxmoxlib.min.js
ansible.builtin.replace:
@@ -70,6 +70,6 @@
content: "{{ proxmoxlib_min_after.stat.checksum }}\n"
- name: Restart pveproxy
systemd:
ansible.builtin.systemd:
name: pveproxy
state: restarted
gitea-actions commented 2026-02-08 08:00:17 +01:00
Review
Copy Link

[Score: 4] Logic error in patching the proxmoxlib files that may affect VM management through Proxmox UI.

[Score: 4] Logic error in patching the proxmoxlib files that may affect VM management through Proxmox UI.

12
tasks/subscription.yml
View File

@@ -24,14 +24,14 @@
when: proxmoxlib_js.stat.exists
failed_when: false
- name: Trigger legacy nag patch if needed
meta: flush_handlers
- name: subscription | Trigger legacy nag patch if needed
ansible.builtin.meta: flush_handlers
when:
- proxmoxlib_js.stat.exists
- proxmoxlib_js_checksum_stored.content is not defined
or proxmoxlib_js.stat.checksum
!= (proxmoxlib_js_checksum_stored.content | b64decode | trim)
gitea-actions commented 2026-02-08 08:05:25 +01:00
Review
Copy Link

[Lines 31-33] [Score: 2] The condition checks if proxmoxlib_js_checksum_stored.content is not defined, but it's actually registered in the previous task. This means this check may never be true, and the "Patch legacy proxmoxlib.js" notification will always be triggered when the playbook runs. To fix this issue, you can change the condition to only check if proxmoxlib_js.stat.checksum is different from the stored checksum.

[Lines 31-33] [Score: 2] The condition checks if `proxmoxlib_js_checksum_stored.content` is not defined, but it's actually registered in the previous task. This means this check may never be true, and the "Patch legacy proxmoxlib.js" notification will always be triggered when the playbook runs. To fix this issue, you can change the condition to only check if `proxmoxlib_js.stat.checksum` is different from the stored checksum.
notify: patch legacy proxmoxlib.js
notify: Ppatch legacy proxmoxlib.js
gitea-actions commented 2026-02-08 07:13:32 +01:00
Review
Copy Link

[Lines 28-35] [Score: 2] This task could be simplified by combining the ansible.builtin.meta: flush_handlers and the subsequent notify call into a single line for better readability.

[Lines 28-35] [Score: 2] This task could be simplified by combining the `ansible.builtin.meta: flush_handlers` and the subsequent `notify` call into a single line for better readability.
############################
# Minified proxmoxlib.min.js (VE 8/9)
@@ -50,11 +50,11 @@
when: proxmoxlib_min_js.stat.exists
failed_when: false
- name: Trigger minified nag patch if needed
meta: flush_handlers
- name: subscription | Trigger minified nag patch if needed
ansible.builtin.meta: flush_handlers
when:
- proxmoxlib_min_js.stat.exists
- proxmoxlib_min_checksum_stored.content is not defined
or proxmoxlib_min_js.stat.checksum
!= (proxmoxlib_min_checksum_stored.content | b64decode | trim)
notify: patch minified proxmoxlib.js
notify: Patch minified proxmoxlib.js
gitea-actions commented 2026-02-08 07:13:32 +01:00
Review
Copy Link

[Lines 54-60] [Score: 2] Similar to the previous suggestion, this task could also be simplified by combining the ansible.builtin.meta: flush_handlers and the subsequent notify call into a single line for better readability.

[Lines 54-60] [Score: 2] Similar to the previous suggestion, this task could also be simplified by combining the `ansible.builtin.meta: flush_handlers` and the subsequent `notify` call into a single line for better readability.