From 8baace9a7427533f935a5eb3358e96c7526d9f99 Mon Sep 17 00:00:00 2001 From: Peter Tselios <27899013+itcultus@users.noreply.github.com> Date: Fri, 28 Nov 2025 12:18:36 +0200 Subject: [PATCH 1/4] proxmox_node: improve certificate management - Use correct custom certificate API endpoints (upload/delete) - Fix overly generic error messages during certificate upload - Exclude default Proxmox certificates (pve-ssl.pem, pve-root-ca.pem) from comparisons - Fix state=absent implementation that previously used wrong endpoint - Make all return messages more descriptive and user-friendly - Add proper force handling and idempotency with leaf fingerprint check Closes #231 --- plugins/modules/proxmox_node.py | 76 ++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/plugins/modules/proxmox_node.py b/plugins/modules/proxmox_node.py index ca31e287..3f8b1a02 100644 --- a/plugins/modules/proxmox_node.py +++ b/plugins/modules/proxmox_node.py @@ -1,6 +1,7 @@ #!/usr/bin/python # -*- coding: utf-8 -*- # +# Copyright (c) 2025, Peter Tselios (@icultus) <27899013+itcultus@users.noreply.github.com> # Copyright (c) 2025, Florian Paul Azim Hoberg (@gyptazy) # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later @@ -14,7 +15,9 @@ short_description: Manage Proxmox VE nodes description: - Manage the Proxmox VE nodes itself. -author: Florian Paul Azim Hoberg (@gyptazy) +author: + - Florian Paul Azim Hoberg (@gyptazy) + - Peter Tselios (@itcultus) attributes: check_mode: support: full @@ -57,7 +60,7 @@ default: false force: description: - - Overwrite existing custom or ACME certificate files. + - Overwrite existing custom certificate files. type: bool default: false dns: @@ -259,11 +262,19 @@ def certificates(self): force = self.bool_to_int(self.module.params.get("certificates", {}).get("force", False)) changed = False result_certificates = "Unchanged" + upload_cert = False try: - current_cert = self.proxmox_api.nodes(node_name).certificates.custom.get() + all_certs = self.proxmox_api.nodes(node_name).certificates.info.get() + # Filter out default Proxmox certificates (pve-root-ca.pem, pve-ssl.pem) + # Keep only custom certificates + current_cert = [cert for cert in all_certs if cert.get('filename') not in ['pve-root-ca.pem', 'pve-ssl.pem']] + if not current_cert: + current_cert = [] + + has_custom_cert = len(current_cert) > 0 except Exception as e: - current_cert = self.proxmox_api.nodes(node_name).certificates.info.get() + self.module.fail_json(msg=f"Failed to get certificate information: {str(e)}") if node_certificate_state == "present": cert_path = self.module.params.get("certificates", {}).get("cert") @@ -271,37 +282,50 @@ def certificates(self): cert = self.read_file(cert_path) key = self.read_file(key_path) fingerprints_file = self.get_certificate_fingerprints_file(cert) - fingerprints_api = self.get_certificate_fingerprints_api(current_cert) - if all(fp in fingerprints_api for fp in fingerprints_file): - changed = False - result_certificates = f"Certificate for node '{node_name}' is already present." + # Check if custom certificate already exists + if has_custom_cert: + fingerprints_api = self.get_certificate_fingerprints_api(current_cert) + + # Compare only the leaf certificate (first in chain), not the certificates of the chain + if (fingerprints_file and fingerprints_file[0] in fingerprints_api) and not force: + result_certificates = f"Certificate for node '{node_name}' is already present and matches." + else: + # Certificate exists but is different, need to upload + upload_cert = True + changed = True + result_certificates = f"Certificate for node '{node_name}' updated." else: - if not self.module.check_mode: - try: - self.proxmox_api.nodes(node_name).certificates.custom.post(certificates=cert, key=key, force=force) - except Exception as e: - self.module.fail_json(msg="Failed to upload certificate. Certificate is already present. Please use 'force' to overwrite it.") - self.proxmox_api.nodes(node_name).services("pveproxy").restart.post() changed = True - result_certificates = f"Certificate for node '{node_name}' has been uploaded." + if not self.module.check_mode: + upload_cert = True - if node_certificate_state == "absent": - custom_cert = True - try: - custom_cert = self.proxmox_api.nodes(node_name).certificates.custom.get() - except Exception as e: - custom_cert = False + if (not self.module.check_mode) and (upload_cert): + try: + self.proxmox_api.nodes(node_name).certificates.custom.post(certificates=cert, key=key, force=force) + result_certificates = f"Certificate for node '{node_name}' has been uploaded." + except Exception as e: + error_msg = str(e) + self.module.fail_json(msg=f"Failed to upload certificate: {error_msg}") - if custom_cert: + if node_certificate_state == "absent": + if has_custom_cert: + changed = True if not self.module.check_mode: try: self.proxmox_api.nodes(node_name).certificates.custom.delete() + result_certificates = f"Certificate for node '{node_name}' has been removed." except Exception as e: - pass - self.proxmox_api.nodes(node_name).services("pveproxy").restart.post() - changed = True - result_certificates = f"Certificate for node '{node_name}' has been removed." + error_msg = str(e) + self.module.fail_json(msg=f"Failed to delete certificate: {error_msg}") + + if changed and (restart := self.module.params.get("certificates", {}).get("restart", False)): + if not self.module.check_mode: + try: + self.proxmox_api.nodes(node_name).services("pveproxy").restart.post() + result_certificates += " pveproxy service restarted." + except Exception as e: + self.module.warn(f"Failed to restart pveproxy: {str(e)}") return changed, result_certificates From f8759962a070584f8290ac632faba837d872cb7d Mon Sep 17 00:00:00 2001 From: Peter Tselios <27899013+itcultus@users.noreply.github.com> Date: Fri, 28 Nov 2025 12:39:50 +0200 Subject: [PATCH 2/4] Add changelog fragments --- changelogs/fragments/232-update-proxmox_node-certificates.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/232-update-proxmox_node-certificates.yml diff --git a/changelogs/fragments/232-update-proxmox_node-certificates.yml b/changelogs/fragments/232-update-proxmox_node-certificates.yml new file mode 100644 index 00000000..a3147e90 --- /dev/null +++ b/changelogs/fragments/232-update-proxmox_node-certificates.yml @@ -0,0 +1,2 @@ +bugfixes: + - "remove wrong api endpoints and error messages from proxmod_node certificate management(https://github.com/ansible-collections/community.proxmox/pull/232)." From f4fba1ed1585bf4b33d91774a0c7ba81590d9666 Mon Sep 17 00:00:00 2001 From: Peter Tselios <27899013+itcultus@users.noreply.github.com> Date: Wed, 3 Dec 2025 17:59:48 +0200 Subject: [PATCH 3/4] Fix indentation in some lines CHG: Fix indentation in some lines to pass nox --- plugins/modules/proxmox_node.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/modules/proxmox_node.py b/plugins/modules/proxmox_node.py index 3f8b1a02..1b07ac14 100644 --- a/plugins/modules/proxmox_node.py +++ b/plugins/modules/proxmox_node.py @@ -298,7 +298,7 @@ def certificates(self): else: changed = True if not self.module.check_mode: - upload_cert = True + upload_cert = True if (not self.module.check_mode) and (upload_cert): try: @@ -320,12 +320,12 @@ def certificates(self): self.module.fail_json(msg=f"Failed to delete certificate: {error_msg}") if changed and (restart := self.module.params.get("certificates", {}).get("restart", False)): - if not self.module.check_mode: - try: - self.proxmox_api.nodes(node_name).services("pveproxy").restart.post() - result_certificates += " pveproxy service restarted." - except Exception as e: - self.module.warn(f"Failed to restart pveproxy: {str(e)}") + if not self.module.check_mode: + try: + self.proxmox_api.nodes(node_name).services("pveproxy").restart.post() + result_certificates += " pveproxy service restarted." + except Exception as e: + self.module.warn(f"Failed to restart pveproxy: {str(e)}") return changed, result_certificates From 129b63731c1c31e57c252b11851da84b49d9e7d3 Mon Sep 17 00:00:00 2001 From: Peter Tselios <27899013+itcultus@users.noreply.github.com> Date: Thu, 4 Dec 2025 16:07:00 +0200 Subject: [PATCH 4/4] Make it compatible with Python 3.7 CHG: Remove walrus operator to make it compatible with Python 3.7 --- plugins/modules/proxmox_node.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/proxmox_node.py b/plugins/modules/proxmox_node.py index 1b07ac14..668174b7 100644 --- a/plugins/modules/proxmox_node.py +++ b/plugins/modules/proxmox_node.py @@ -319,7 +319,8 @@ def certificates(self): error_msg = str(e) self.module.fail_json(msg=f"Failed to delete certificate: {error_msg}") - if changed and (restart := self.module.params.get("certificates", {}).get("restart", False)): + restart = self.module.params.get("certificates", {}).get("restart", False) + if changed and restart: if not self.module.check_mode: try: self.proxmox_api.nodes(node_name).services("pveproxy").restart.post()