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 +76 -66
2 changed files with 76 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 12b1b87af1 - Show all commits

59
handlers/main.yml
View File

@@ -14,3 +14,62 @@
- name: Reload systemd
ansible.builtin.systemd_service:
daemon_reexec: true
- name: patch legacy proxmoxlib.js
block:
- name: Patch 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'\\)"
replace: "if (false)"
gitea-actions commented 2026-02-08 08:05:25 +01:00
Review
Copy Link

[Score: 3] Replacing the condition with a constant value could lead to unexpected behavior or confusion when debugging. It's better to keep the original check if possible, or at least document the purpose of this change.

[Score: 3] Replacing the condition with a constant value could lead to unexpected behavior or confusion when debugging. It's better to keep the original check if possible, or at least document the purpose of this change.
register: patch_legacy
failed_when: patch_legacy.matched == 0
notify: Restart pveproxy
- name: Re-stat proxmoxlib.js
ansible.builtin.stat:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js
checksum_algorithm: sha256
register: proxmoxlib_js_after
- name: Store patched checksum (legacy)
ansible.builtin.copy:
dest: /var/lib/proxmox-nag-patch/proxmoxlib.js.sha256
owner: root
group: root
mode: "0644"
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:
name: pveproxy
state: restarted
- name: patch minified proxmoxlib.js
block:
- name: Patch minified proxmoxlib.min.js
ansible.builtin.replace:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.min.js
regexp: "data.status!=='Active'"
replace: "false"
register: patch_minified
failed_when: patch_minified.matched == 0
- name: Re-stat proxmoxlib.min.js
ansible.builtin.stat:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.min.js
gitea-actions commented 2026-02-08 08:05:25 +01:00
Review
Copy Link

[Score: 3] It's good practice to document the purpose and usage of variables used throughout a playbook, especially when they contain sensitive information like file paths or checksums. Adding comments explaining what proxmoxlib_min_after is for would improve maintainability.

[Score: 3] It's good practice to document the purpose and usage of variables used throughout a playbook, especially when they contain sensitive information like file paths or checksums. Adding comments explaining what `proxmoxlib_min_after` is for would improve maintainability.
checksum_algorithm: sha256
register: proxmoxlib_min_after
- name: Store patched checksum (minified)
ansible.builtin.copy:
dest: /var/lib/proxmox-nag-patch/proxmoxlib.min.js.sha256
owner: root
group: root
mode: "0644"
content: "{{ proxmoxlib_min_after.stat.checksum }}\n"
- name: Restart pveproxy
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.

83
tasks/subscription.yml
View File

@@ -10,12 +10,6 @@
############################
# Legacy proxmoxlib.js
############################
- name: subscription | Read stored checksum (legacy)
ansible.builtin.slurp:
src: /var/lib/proxmox-nag-patch/proxmoxlib.js.sha256
register: proxmoxlib_js_checksum_stored
when: proxmoxlib_js.stat.exists
failed_when: false
- name: subscription | Check for legacy proxmoxlib.js
ansible.builtin.stat:
@@ -23,39 +17,21 @@
checksum_algorithm: sha256
register: proxmoxlib_js
- name: subscription | Remove subscription nag (legacy proxmoxlib.js)
ansible.builtin.replace:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js
regexp: "if \\(data.status !== 'Active'\\)"
replace: "if (false)"
- name: subscription | Read stored checksum (legacy)
ansible.builtin.slurp:
src: /var/lib/proxmox-nag-patch/proxmoxlib.js.sha256
register: proxmoxlib_js_checksum_stored
when: proxmoxlib_js.stat.exists
failed_when: false
- name: Trigger legacy nag patch if needed
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))
register: patch_legacy
failed_when:
- proxmoxlib_js.stat.exists
- patch_legacy.matched == 0
notify: restart pveproxy
- name: subscription | Read checksum after patch (legacy proxmoxlib.js)
ansible.builtin.stat:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js
checksum_algorithm: sha256
register: proxmoxlib_js_after
when: patch_legacy is changed
- name: subscription | Store patched checksum (legacy)
ansible.builtin.copy:
dest: /var/lib/proxmox-nag-patch/proxmoxlib.js.sha256
owner: root
group: root
mode: "0644"
content: "{{ proxmoxlib_js_after.stat.checksum }}\n"
when:
- proxmoxlib_js.stat.exists
- patch_legacy is changed
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
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)
@@ -74,36 +50,11 @@
when: proxmoxlib_min_js.stat.exists
failed_when: false
- name: subscription | Remove subscription nag (minified bundle for VE 8/9)
ansible.builtin.replace:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.min.js
regexp: "data.status!=='Active'"
replace: "false"
- name: Trigger minified nag patch if needed
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))
register: patch_minified
failed_when:
- proxmoxlib_min_js.stat.exists
- patch_minified.matched == 0
notify: restart pveproxy
- name: subscription | Re-stat proxmoxlib.min.js after patch
ansible.builtin.stat:
path: /usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.min.js
checksum_algorithm: sha256
register: proxmoxlib_min_js_after
when: patch_legacy is changed
- name: subscription | Store patched checksum (minified)
ansible.builtin.copy:
dest: /var/lib/proxmox-nag-patch/proxmoxlib.min.js.sha256
owner: root
group: root
mode: "0644"
content: "{{ proxmoxlib_min_js.stat.checksum }}\n"
when:
- proxmoxlib_min_js_after.stat.exists
- patch_minified is changed
or proxmoxlib_min_js.stat.checksum
!= (proxmoxlib_min_checksum_stored.content | b64decode | trim)
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.