From e081499f72c1e5da2b868204a81852b24ce60ab9 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 00:12:11 +0100 Subject: [PATCH 01/15] feat: added acme certificate ordering Signed-off-by: lonelysadness --- .../virtual_environment_acme_certificate.md | 100 +++ .../import.sh | 3 + .../resource.tf | 71 ++ fwprovider/nodes/resource_acme_certificate.go | 680 ++++++++++++++++++ .../nodes/resource_acme_certificate_test.go | 149 ++++ fwprovider/provider.go | 1 + proxmox/nodes/certificate.go | 32 + proxmox/nodes/certificate_types.go | 15 + proxmox/nodes/config.go | 2 +- proxmox/nodes/config_types.go | 48 +- 10 files changed, 1088 insertions(+), 13 deletions(-) create mode 100644 docs/resources/virtual_environment_acme_certificate.md create mode 100644 examples/resources/proxmox_virtual_environment_acme_certificate/import.sh create mode 100644 examples/resources/proxmox_virtual_environment_acme_certificate/resource.tf create mode 100644 fwprovider/nodes/resource_acme_certificate.go create mode 100644 fwprovider/nodes/resource_acme_certificate_test.go diff --git a/docs/resources/virtual_environment_acme_certificate.md b/docs/resources/virtual_environment_acme_certificate.md new file mode 100644 index 000000000..87e929da0 --- /dev/null +++ b/docs/resources/virtual_environment_acme_certificate.md @@ -0,0 +1,100 @@ +--- +layout: page +title: proxmox_virtual_environment_acme_certificate +parent: Resources +subcategory: Virtual Environment +--- + +# Resource: proxmox_virtual_environment_acme_certificate + +Manages ACME SSL certificates for Proxmox VE nodes. This resource orders and renews certificates from an ACME Certificate Authority for a specific node. + +## Example Usage + +### Basic ACME Certificate with HTTP-01 Challenge + +```terraform +# First, create an ACME account +resource "proxmox_virtual_environment_acme_account" "example" { + name = "production" + contact = "admin@example.com" + directory = "https://acme-v02.api.letsencrypt.org/directory" + tos = "https://letsencrypt.org/documents/LE-SA-v1.3-September-21-2022.pdf" +} + +# Order a certificate for the node +resource "proxmox_virtual_environment_acme_certificate" "example" { + node_name = "pve" + + # Depends on the ACME account being created first + depends_on = [ + proxmox_virtual_environment_acme_account.example + ] +} +``` + +### ACME Certificate with DNS-01 Challenge + +```terraform +# Create an ACME account +resource "proxmox_virtual_environment_acme_account" "example" { + name = "production" + contact = "admin@example.com" + directory = "https://acme-v02.api.letsencrypt.org/directory" + tos = "https://letsencrypt.org/documents/LE-SA-v1.3-September-21-2022.pdf" +} + +# Configure a DNS plugin (Desec example) +resource "proxmox_virtual_environment_acme_dns_plugin" "desec" { + plugin = "desec" + api = "desec" + + data = { + DEDYN_TOKEN = var.dedyn_token + } +} + +# Order a certificate using the DNS plugin +resource "proxmox_virtual_environment_acme_certificate" "test" { + node_name = "pve" + account = proxmox_virtual_environment_acme_account.example.name + force = false + + domains = [ + { + domain = "pve.example.dedyn.io" + plugin = proxmox_virtual_environment_acme_dns_plugin.desec.plugin + } + ] + + depends_on = [ + proxmox_virtual_environment_acme_account.test, + proxmox_virtual_environment_acme_dns_plugin.desec + ] +} +``` + +### Force Certificate Renewal + +```terraform +resource "proxmox_virtual_environment_acme_certificate" "example_force" { + node_name = "pve" + force = true # This will trigger renewal on every apply +} +``` + +## Import + +ACME certificates can be imported using the node name: + +```shell +#!/usr/bin/env sh +# ACME certificates can be imported using the node name, e.g.: +terraform import proxmox_virtual_environment_acme_certificate.example pve +``` + +## Related Resources + +- [`proxmox_virtual_environment_acme_account`](virtual_environment_acme_account) - Manages ACME accounts +- [`proxmox_virtual_environment_acme_dns_plugin`](virtual_environment_acme_dns_plugin) - Manages ACME DNS plugins for DNS-01 challenges +- [`proxmox_virtual_environment_certificate`](virtual_environment_certificate) - Manages custom SSL/TLS certificates (non-ACME) diff --git a/examples/resources/proxmox_virtual_environment_acme_certificate/import.sh b/examples/resources/proxmox_virtual_environment_acme_certificate/import.sh new file mode 100644 index 000000000..525d0bd54 --- /dev/null +++ b/examples/resources/proxmox_virtual_environment_acme_certificate/import.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env sh +# ACME certificates can be imported using the node name, e.g.: +terraform import proxmox_virtual_environment_acme_certificate.example pve-node-01 diff --git a/examples/resources/proxmox_virtual_environment_acme_certificate/resource.tf b/examples/resources/proxmox_virtual_environment_acme_certificate/resource.tf new file mode 100644 index 000000000..e2ea6bac9 --- /dev/null +++ b/examples/resources/proxmox_virtual_environment_acme_certificate/resource.tf @@ -0,0 +1,71 @@ +# Example: Basic ACME certificate with HTTP-01 challenge (standalone) +resource "proxmox_virtual_environment_acme_account" "example" { + name = "production" + contact = "admin@example.com" + directory = "https://acme-v02.api.letsencrypt.org/directory" + tos = "https://letsencrypt.org/documents/LE-SA-v1.3-September-21-2022.pdf" +} + +resource "proxmox_virtual_environment_acme_certificate" "http_example" { + node_name = "pve-node-01" + account = proxmox_virtual_environment_acme_account.example.name + + domains = [ + { + domain = "pve.example.com" + # No plugin specified = HTTP-01 challenge + } + ] +} + +# Example: ACME certificate with DNS-01 challenge using Cloudflare +resource "proxmox_virtual_environment_acme_dns_plugin" "cloudflare" { + plugin = "cloudflare" + api = "cf" + + # Wait 2 minutes for DNS propagation + validation_delay = 120 + + data = { + CF_Account_ID = "your-cloudflare-account-id" + CF_Token = "your-cloudflare-api-token" + CF_Zone_ID = "your-cloudflare-zone-id" + } +} + +resource "proxmox_virtual_environment_acme_certificate" "dns_example" { + node_name = "pve-node-01" + account = proxmox_virtual_environment_acme_account.example.name + + domains = [ + { + domain = "pve.example.com" + plugin = proxmox_virtual_environment_acme_dns_plugin.cloudflare.plugin + } + ] + + depends_on = [ + proxmox_virtual_environment_acme_account.example, + proxmox_virtual_environment_acme_dns_plugin.cloudflare + ] +} + +# Example: Force certificate renewal +resource "proxmox_virtual_environment_acme_certificate" "force_renew" { + node_name = "pve-node-01" + account = proxmox_virtual_environment_acme_account.example.name + force = true + + domains = [ + { + domain = "pve.example.com" + plugin = proxmox_virtual_environment_acme_dns_plugin.cloudflare.plugin + } + ] + + depends_on = [ + proxmox_virtual_environment_acme_account.example, + proxmox_virtual_environment_acme_dns_plugin.cloudflare + ] +} + diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go new file mode 100644 index 000000000..3d28a5fdb --- /dev/null +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -0,0 +1,680 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package nodes + +import ( + "context" + "fmt" + "time" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" + + "github.com/bpg/terraform-provider-proxmox/fwprovider/attribute" + "github.com/bpg/terraform-provider-proxmox/fwprovider/config" + "github.com/bpg/terraform-provider-proxmox/proxmox" + "github.com/bpg/terraform-provider-proxmox/proxmox/nodes" + proxmoxtypes "github.com/bpg/terraform-provider-proxmox/proxmox/types" +) + +var ( + _ resource.Resource = &acmeCertificateResource{} + _ resource.ResourceWithConfigure = &acmeCertificateResource{} + _ resource.ResourceWithImportState = &acmeCertificateResource{} +) + +// NewACMECertificateResource creates a new resource for managing ACME certificates on nodes. +func NewACMECertificateResource() resource.Resource { + return &acmeCertificateResource{} +} + +// acmeCertificateResource contains the resource's internal data. +type acmeCertificateResource struct { + // The Proxmox client + client proxmox.Client +} + +// acmeCertificateModel maps the schema data for the ACME certificate resource. +type acmeCertificateModel struct { + // ID is the unique identifier for the resource (node_name) + ID types.String `tfsdk:"id"` + // NodeName is the name of the node for which to order the certificate + NodeName types.String `tfsdk:"node_name"` + // ACME account name to use + Account types.String `tfsdk:"account"` + // Domains to include in the certificate + Domains types.List `tfsdk:"domains"` + // Force certificate renewal even if not due yet + Force types.Bool `tfsdk:"force"` + // Certificate PEM data (computed after ordering) + Certificate types.String `tfsdk:"certificate"` + // Certificate fingerprint (computed) + Fingerprint types.String `tfsdk:"fingerprint"` + // Certificate issuer (computed) + Issuer types.String `tfsdk:"issuer"` + // Certificate subject (computed) + Subject types.String `tfsdk:"subject"` + // Certificate expiration date (computed) + NotAfter types.String `tfsdk:"not_after"` + // Certificate start date (computed) + NotBefore types.String `tfsdk:"not_before"` + // Certificate subject alternative names (computed) + SubjectAlternativeNames types.List `tfsdk:"subject_alternative_names"` +} + +// acmeDomainModel maps the schema data for an ACME domain configuration. +type acmeDomainModel struct { + // Domain name + Domain types.String `tfsdk:"domain"` + // DNS plugin to use for validation (optional, if not set uses standalone http-01) + Plugin types.String `tfsdk:"plugin"` + // Alias domain for DNS validation (optional) + Alias types.String `tfsdk:"alias"` +} + +// Metadata defines the name of the resource. +func (r *acmeCertificateResource) Metadata( + _ context.Context, + req resource.MetadataRequest, + resp *resource.MetadataResponse, +) { + resp.TypeName = req.ProviderTypeName + "_acme_certificate" +} + +// Schema defines the schema for the resource. +func (r *acmeCertificateResource) Schema( + _ context.Context, + _ resource.SchemaRequest, + resp *resource.SchemaResponse, +) { + resp.Schema = schema.Schema{ + Description: "Manages ACME SSL certificates for Proxmox VE nodes. " + + "This resource orders and renews certificates from an ACME Certificate Authority (like Let's Encrypt) " + + "for a specific node.", + MarkdownDescription: "Manages ACME SSL certificates for Proxmox VE nodes.\n\n" + + "This resource orders and renews certificates from an ACME Certificate Authority (like Let's Encrypt) " + + "for a specific node. Before using this resource, ensure that:\n" + + "- An ACME account is configured (using `proxmox_virtual_environment_acme_account`)\n" + + "- DNS plugins are configured if using DNS-01 challenge (using `proxmox_virtual_environment_acme_dns_plugin`)", + Attributes: map[string]schema.Attribute{ + "id": attribute.ResourceID(), + "node_name": schema.StringAttribute{ + Description: "The name of the Proxmox VE node for which to order/manage the ACME certificate.", + Required: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplace(), + }, + }, + "account": schema.StringAttribute{ + Description: "The ACME account name to use for ordering the certificate.", + Required: true, + }, + "domains": schema.ListNestedAttribute{ + Description: "The list of domains to include in the certificate. At least one domain is required.", + Required: true, + NestedObject: schema.NestedAttributeObject{ + Attributes: map[string]schema.Attribute{ + "domain": schema.StringAttribute{ + Description: "The domain name to include in the certificate.", + Required: true, + }, + "plugin": schema.StringAttribute{ + Description: "The DNS plugin to use for DNS-01 challenge validation. " + + "If not specified, the standalone HTTP-01 challenge will be used.", + Optional: true, + }, + "alias": schema.StringAttribute{ + Description: "An optional alias domain for DNS validation. " + + "This allows you to validate the domain using a different domain's DNS records.", + Optional: true, + }, + }, + }, + }, + "force": schema.BoolAttribute{ + Description: "Force certificate renewal even if the certificate is not due for renewal yet. " + + "Setting this to true will trigger a new certificate order on every apply.", + Optional: true, + Computed: true, + Default: booldefault.StaticBool(false), + }, + "certificate": schema.StringAttribute{ + Description: "The PEM-encoded certificate data.", + Computed: true, + }, + "fingerprint": schema.StringAttribute{ + Description: "The certificate fingerprint.", + Computed: true, + }, + "issuer": schema.StringAttribute{ + Description: "The certificate issuer.", + Computed: true, + }, + "subject": schema.StringAttribute{ + Description: "The certificate subject.", + Computed: true, + }, + "not_after": schema.StringAttribute{ + Description: "The certificate expiration timestamp.", + Computed: true, + }, + "not_before": schema.StringAttribute{ + Description: "The certificate start timestamp.", + Computed: true, + }, + "subject_alternative_names": schema.ListAttribute{ + Description: "The certificate subject alternative names (SANs).", + Computed: true, + ElementType: types.StringType, + }, + }, + } +} + +// Configure adds the provider-configured client to the resource. +func (r *acmeCertificateResource) Configure( + _ context.Context, + req resource.ConfigureRequest, + resp *resource.ConfigureResponse, +) { + if req.ProviderData == nil { + return + } + + cfg, ok := req.ProviderData.(config.Resource) + if !ok { + resp.Diagnostics.AddError( + "Unexpected Resource Configure Type", + fmt.Sprintf("Expected config.Resource, got: %T. Please report this issue to the provider developers.", req.ProviderData), + ) + + return + } + + r.client = cfg.Client +} + +// Create orders a new ACME certificate for the node. +func (r *acmeCertificateResource) Create( + ctx context.Context, + req resource.CreateRequest, + resp *resource.CreateResponse, +) { + var plan acmeCertificateModel + + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + + if resp.Diagnostics.HasError() { + return + } + + nodeName := plan.NodeName.ValueString() + nodeClient := r.client.Node(nodeName) + + // First, configure the node with ACME settings + if err := r.configureNodeACME(ctx, nodeClient, &plan); err != nil { + resp.Diagnostics.AddError( + "Unable to configure node ACME settings", + fmt.Sprintf("An error occurred while configuring ACME settings for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Order the certificate + force := proxmoxtypes.CustomBool(plan.Force.ValueBool()) + orderReq := &nodes.CertificateOrderRequestBody{ + Force: &force, + } + + taskID, err := nodeClient.OrderCertificate(ctx, orderReq) + if err != nil { + resp.Diagnostics.AddError( + "Unable to order ACME certificate", + fmt.Sprintf("An error occurred while ordering the ACME certificate for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Wait for the task to complete + if taskID != nil && *taskID != "" { + err = nodeClient.Tasks().WaitForTask(ctx, *taskID) + if err != nil { + resp.Diagnostics.AddError( + "Certificate order task failed", + fmt.Sprintf("The certificate order task for node %s failed: %s", nodeName, err.Error()), + ) + + return + } + } + + // Wait a bit for the certificate to be written + time.Sleep(2 * time.Second) + + // Read the certificate information + certificates, err := nodeClient.ListCertificates(ctx) + if err != nil { + resp.Diagnostics.AddError( + "Unable to read certificate information", + fmt.Sprintf("An error occurred while reading certificate information for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Update the state with certificate information + if err := r.updateModelFromCertificates(&plan, certificates); err != nil { + resp.Diagnostics.AddError( + "Unable to process certificate information", + fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), + ) + + return + } + + plan.ID = plan.NodeName + + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +// Read reads the current certificate information for the node. +func (r *acmeCertificateResource) Read( + ctx context.Context, + req resource.ReadRequest, + resp *resource.ReadResponse, +) { + var state acmeCertificateModel + + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + + if resp.Diagnostics.HasError() { + return + } + + nodeName := state.NodeName.ValueString() + nodeClient := r.client.Node(nodeName) + + // Read certificate information + certificates, err := nodeClient.ListCertificates(ctx) + if err != nil { + resp.Diagnostics.AddError( + "Unable to read certificate information", + fmt.Sprintf("An error occurred while reading certificate information for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Update the state with certificate information + if err := r.updateModelFromCertificates(&state, certificates); err != nil { + resp.Diagnostics.AddError( + "Unable to process certificate information", + fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), + ) + + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, &state)...) +} + +// Update renews the certificate for the node. +func (r *acmeCertificateResource) Update( + ctx context.Context, + req resource.UpdateRequest, + resp *resource.UpdateResponse, +) { + var plan, state acmeCertificateModel + + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + + if resp.Diagnostics.HasError() { + return + } + + nodeName := plan.NodeName.ValueString() + nodeClient := r.client.Node(nodeName) + + // Update node configuration if account or domains changed + if !plan.Account.Equal(state.Account) || !plan.Domains.Equal(state.Domains) { + if err := r.configureNodeACME(ctx, nodeClient, &plan); err != nil { + resp.Diagnostics.AddError( + "Unable to update node ACME settings", + fmt.Sprintf("An error occurred while updating ACME settings for node %s: %s", nodeName, err.Error()), + ) + + return + } + } + + // Renew the certificate if force is true or other changes are made + force := proxmoxtypes.CustomBool(plan.Force.ValueBool()) + renewReq := &nodes.CertificateRenewRequestBody{ + Force: &force, + } + + taskID, err := nodeClient.RenewCertificate(ctx, renewReq) + if err != nil { + resp.Diagnostics.AddError( + "Unable to renew ACME certificate", + fmt.Sprintf("An error occurred while renewing the ACME certificate for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Wait for the task to complete + if taskID != nil && *taskID != "" { + err = nodeClient.Tasks().WaitForTask(ctx, *taskID) + if err != nil { + resp.Diagnostics.AddError( + "Certificate renewal task failed", + fmt.Sprintf("The certificate renewal task for node %s failed: %s", nodeName, err.Error()), + ) + + return + } + } + + // Wait a bit for the certificate to be written + time.Sleep(2 * time.Second) + + // Read the updated certificate information + certificates, err := nodeClient.ListCertificates(ctx) + if err != nil { + resp.Diagnostics.AddError( + "Unable to read certificate information", + fmt.Sprintf("An error occurred while reading certificate information for node %s: %s", nodeName, err.Error()), + ) + + return + } + + // Update the state with certificate information + if err := r.updateModelFromCertificates(&plan, certificates); err != nil { + resp.Diagnostics.AddError( + "Unable to process certificate information", + fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), + ) + + return + } + + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) +} + +// Delete removes the certificate resource from Terraform state. +// Note: This doesn't actually delete the certificate from the node, +// as ACME certificates should remain on the node for the services to use. +func (r *acmeCertificateResource) Delete( + ctx context.Context, + req resource.DeleteRequest, + resp *resource.DeleteResponse, +) { + var state acmeCertificateModel + + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + + if resp.Diagnostics.HasError() { + return + } + + // Note: We don't actually delete the certificate from the node + // as it's needed for the node's services. We just remove it from Terraform state. + // If users want to remove the certificate from the node, they should use + // the Proxmox UI or manually delete it via the API. +} + +// ImportState imports an existing certificate by node name. +func (r *acmeCertificateResource) ImportState( + ctx context.Context, + req resource.ImportStateRequest, + resp *resource.ImportStateResponse, +) { + // The import ID is the node name + nodeName := req.ID + + nodeClient := r.client.Node(nodeName) + + // Read the node configuration to get ACME settings + config, err := nodeClient.GetConfig(ctx) + if err != nil { + resp.Diagnostics.AddError( + "Unable to read node configuration", + fmt.Sprintf("An error occurred while reading configuration for node %s: %s", nodeName, err.Error()), + ) + + return + } + + if config == nil || len(*config) == 0 { + resp.Diagnostics.AddError( + "Unable to read node configuration", + fmt.Sprintf("No configuration found for node %s", nodeName), + ) + + return + } + + nodeConfig := (*config)[0] + + // Extract ACME account from config + var accountName string + var domains []acmeDomainModel + + // Check for standalone ACME configuration + if nodeConfig.ACME != nil && nodeConfig.ACME.Account != nil { + accountName = *nodeConfig.ACME.Account + for _, domain := range nodeConfig.ACME.Domains { + domains = append(domains, acmeDomainModel{ + Domain: types.StringValue(domain), + Plugin: types.StringNull(), + Alias: types.StringNull(), + }) + } + } + + // Check for DNS challenge domain configurations + acmeDomainConfigs := []*nodes.ACMEDomainConfig{ + nodeConfig.ACMEDomain0, + nodeConfig.ACMEDomain1, + nodeConfig.ACMEDomain2, + nodeConfig.ACMEDomain3, + nodeConfig.ACMEDomain4, + } + + for _, domainConfig := range acmeDomainConfigs { + if domainConfig != nil { + domains = append(domains, acmeDomainModel{ + Domain: types.StringValue(domainConfig.Domain), + Plugin: stringPtrToValue(domainConfig.Plugin), + Alias: stringPtrToValue(domainConfig.Alias), + }) + } + } + + if accountName == "" { + resp.Diagnostics.AddWarning( + "ACME account not found in node configuration", + "Could not determine the ACME account name from the node configuration. "+ + "You may need to manually set the 'account' attribute after import.", + ) + } + + // Convert domains to Terraform list + domainsList, diag := types.ListValueFrom(ctx, types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "domain": types.StringType, + "plugin": types.StringType, + "alias": types.StringType, + }, + }, domains) + resp.Diagnostics.Append(diag...) + + if resp.Diagnostics.HasError() { + return + } + + // Set all attributes + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("node_name"), nodeName)...) + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), nodeName)...) + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("force"), false)...) + + if accountName != "" { + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("account"), accountName)...) + } + + if len(domains) > 0 { + resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("domains"), domainsList)...) + } +} + +// configureNodeACME configures the node with ACME settings before ordering a certificate. +func (r *acmeCertificateResource) configureNodeACME( + ctx context.Context, + nodeClient *nodes.Client, + model *acmeCertificateModel, +) error { + // Parse domains from the model + var domains []acmeDomainModel + diag := model.Domains.ElementsAs(ctx, &domains, false) + if diag.HasError() { + return fmt.Errorf("error parsing domains: %v", diag.Errors()) + } + + if len(domains) == 0 { + return fmt.Errorf("at least one domain is required") + } + + // Build the config update request + configUpdate := &nodes.ConfigUpdateRequestBody{} + accountName := model.Account.ValueString() + + // Separate domains into standalone (no plugin) and DNS challenge (with plugin) + var standaloneDomains []string + var dnsDomains []acmeDomainModel + + for _, domain := range domains { + if domain.Plugin.IsNull() || domain.Plugin.ValueString() == "" { + standaloneDomains = append(standaloneDomains, domain.Domain.ValueString()) + } else { + dnsDomains = append(dnsDomains, domain) + } + } + + // Always configure the ACME account + // If we have standalone domains, include them; otherwise just set the account + configUpdate.ACME = &nodes.ACMEConfig{ + Account: &accountName, + Domains: standaloneDomains, + } + + // Configure DNS challenge domains (up to 5 domains with DNS plugins) + if len(dnsDomains) > 0 { + if len(dnsDomains) > 5 { + return fmt.Errorf("Proxmox supports a maximum of 5 DNS challenge domains, got %d", len(dnsDomains)) + } + + for i, domain := range dnsDomains { + domainConfig := &nodes.ACMEDomainConfig{ + Domain: domain.Domain.ValueString(), + } + + if !domain.Plugin.IsNull() { + plugin := domain.Plugin.ValueString() + domainConfig.Plugin = &plugin + } + + if !domain.Alias.IsNull() { + alias := domain.Alias.ValueString() + domainConfig.Alias = &alias + } + + // Map to the appropriate acmedomain field + switch i { + case 0: + configUpdate.ACMEDomain0 = domainConfig + case 1: + configUpdate.ACMEDomain1 = domainConfig + case 2: + configUpdate.ACMEDomain2 = domainConfig + case 3: + configUpdate.ACMEDomain3 = domainConfig + case 4: + configUpdate.ACMEDomain4 = domainConfig + } + } + } + + // Update the node configuration + return nodeClient.UpdateConfig(ctx, configUpdate) +} + +// updateModelFromCertificates updates the model with certificate information. +func (r *acmeCertificateResource) updateModelFromCertificates( + model *acmeCertificateModel, + certificates *[]nodes.CertificateListResponseData, +) error { + if certificates == nil || len(*certificates) == 0 { + return fmt.Errorf("no certificates found") + } + + // Find the first certificate (usually the active one) + cert := (*certificates)[0] + + // Update basic certificate fields + model.Certificate = stringPtrToValue(cert.Certificates) + model.Fingerprint = stringPtrToValue(cert.Fingerprint) + model.Issuer = stringPtrToValue(cert.Issuer) + model.Subject = stringPtrToValue(cert.Subject) + + // Update timestamps + if cert.NotAfter != nil { + model.NotAfter = types.StringValue(time.Time(*cert.NotAfter).Format(time.RFC3339)) + } else { + model.NotAfter = types.StringNull() + } + + if cert.NotBefore != nil { + model.NotBefore = types.StringValue(time.Time(*cert.NotBefore).Format(time.RFC3339)) + } else { + model.NotBefore = types.StringNull() + } + + // Handle subject alternative names + if cert.SubjectAlternativeNames != nil { + sanList := make([]types.String, 0, len(*cert.SubjectAlternativeNames)) + for _, san := range *cert.SubjectAlternativeNames { + sanList = append(sanList, types.StringValue(san)) + } + + list, _ := types.ListValueFrom(context.Background(), types.StringType, sanList) + model.SubjectAlternativeNames = list + } else { + model.SubjectAlternativeNames = types.ListNull(types.StringType) + } + + return nil +} + +// stringPtrToValue converts a string pointer to a types.String value. +func stringPtrToValue(ptr *string) types.String { + if ptr != nil { + return types.StringValue(*ptr) + } + + return types.StringNull() +} diff --git a/fwprovider/nodes/resource_acme_certificate_test.go b/fwprovider/nodes/resource_acme_certificate_test.go new file mode 100644 index 000000000..b7ce69d83 --- /dev/null +++ b/fwprovider/nodes/resource_acme_certificate_test.go @@ -0,0 +1,149 @@ +//go:build acceptance || all + +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package nodes_test + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + + "github.com/bpg/terraform-provider-proxmox/fwprovider/test" + "github.com/bpg/terraform-provider-proxmox/utils" +) + +// TestAccResourceACMECertificate tests the ACME certificate resource. +// Note: This test requires a properly configured ACME environment: +// - Set PROXMOX_VE_ACC_ACME_ACCOUNT_NAME environment variable +// - Set PROXMOX_VE_ACC_ACME_DOMAIN environment variable +// - Set PROXMOX_VE_ACC_ACME_DNS_PLUGIN (optional, for DNS-01 challenge) +// The test will be skipped if these are not set. +func TestAccResourceACMECertificate(t *testing.T) { + te := test.InitEnvironment(t) + + acmeAccount := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_ACCOUNT_NAME") + acmeDomain := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_DOMAIN") + dnsPlugin := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_DNS_PLUGIN") + + if acmeAccount == "" || acmeDomain == "" { + t.Skip("Skipping ACME certificate test - set PROXMOX_VE_ACC_ACME_ACCOUNT_NAME and PROXMOX_VE_ACC_ACME_DOMAIN") + } + + nodeName := te.NodeName + if nodeName == "" { + nodeName = "pve" + } + + // Build domains config + domainsConfig := `domains = [{ + domain = "` + acmeDomain + `"` + + if dnsPlugin != "" { + domainsConfig += ` + plugin = "` + dnsPlugin + `"` + } + + domainsConfig += ` + }]` + + te.AddTemplateVars(map[string]interface{}{ + "NodeName": nodeName, + "Account": acmeAccount, + "DomainsConfig": domainsConfig, + }) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: te.AccProviders, + Steps: []resource.TestStep{ + { + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_acme_certificate" "test_cert" { + node_name = "{{.NodeName}}" + account = "{{.Account}}" + force = false + {{.DomainsConfig}} + }`, test.WithRootUser()), + Check: resource.ComposeTestCheckFunc( + test.ResourceAttributes("proxmox_virtual_environment_acme_certificate.test_cert", map[string]string{ + "node_name": nodeName, + "account": acmeAccount, + "force": "false", + }), + test.ResourceAttributesSet("proxmox_virtual_environment_acme_certificate.test_cert", []string{ + "certificate", + "fingerprint", + "issuer", + "subject", + "not_after", + "not_before", + }), + ), + }, + }, + }) +} + +// TestAccResourceACMECertificate_Import tests importing an ACME certificate resource. +func TestAccResourceACMECertificate_Import(t *testing.T) { + te := test.InitEnvironment(t) + + acmeAccount := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_ACCOUNT_NAME") + acmeDomain := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_DOMAIN") + dnsPlugin := utils.GetAnyStringEnv("PROXMOX_VE_ACC_ACME_DNS_PLUGIN") + + if acmeAccount == "" || acmeDomain == "" { + t.Skip("Skipping ACME certificate import test - set PROXMOX_VE_ACC_ACME_ACCOUNT_NAME and PROXMOX_VE_ACC_ACME_DOMAIN") + } + + nodeName := te.NodeName + if nodeName == "" { + nodeName = "pve" + } + + // Build domains config + domainsConfig := `domains = [{ + domain = "` + acmeDomain + `"` + + if dnsPlugin != "" { + domainsConfig += ` + plugin = "` + dnsPlugin + `"` + } + + domainsConfig += ` + }]` + + te.AddTemplateVars(map[string]interface{}{ + "NodeName": nodeName, + "Account": acmeAccount, + "DomainsConfig": domainsConfig, + }) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: te.AccProviders, + Steps: []resource.TestStep{ + { + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_acme_certificate" "test_cert_import" { + node_name = "{{.NodeName}}" + account = "{{.Account}}" + force = true + {{.DomainsConfig}} + }`, test.WithRootUser()), + }, + { + ResourceName: "proxmox_virtual_environment_acme_certificate.test_cert_import", + ImportState: true, + ImportStateId: nodeName, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "force", // force is not stored in state + }, + }, + }, + }) +} diff --git a/fwprovider/provider.go b/fwprovider/provider.go index 63e1ffb2e..d47cd20d9 100644 --- a/fwprovider/provider.go +++ b/fwprovider/provider.go @@ -528,6 +528,7 @@ func (p *proxmoxProvider) Resources(_ context.Context) []func() resource.Resourc metrics.NewMetricsServerResource, network.NewLinuxBridgeResource, network.NewLinuxVLANResource, + nodes.NewACMECertificateResource, nodes.NewDownloadFileResource, options.NewClusterOptionsResource, vm.NewResource, diff --git a/proxmox/nodes/certificate.go b/proxmox/nodes/certificate.go index 3e2c8dc0b..f005c2121 100644 --- a/proxmox/nodes/certificate.go +++ b/proxmox/nodes/certificate.go @@ -49,3 +49,35 @@ func (c *Client) UpdateCertificate(ctx context.Context, d *CertificateUpdateRequ return nil } + +// OrderCertificate orders a new certificate from ACME CA for a node. +func (c *Client) OrderCertificate(ctx context.Context, d *CertificateOrderRequestBody) (*string, error) { + resBody := &CertificateOrderResponseBody{} + + err := c.DoRequest(ctx, http.MethodPost, c.ExpandPath("certificates/acme/certificate"), d, resBody) + if err != nil { + return nil, fmt.Errorf("error ordering ACME certificate: %w", err) + } + + if resBody.Data == nil { + return nil, api.ErrNoDataObjectInResponse + } + + return resBody.Data, nil +} + +// RenewCertificate renews an existing ACME certificate for a node. +func (c *Client) RenewCertificate(ctx context.Context, d *CertificateRenewRequestBody) (*string, error) { + resBody := &CertificateOrderResponseBody{} + + err := c.DoRequest(ctx, http.MethodPut, c.ExpandPath("certificates/acme/certificate"), d, resBody) + if err != nil { + return nil, fmt.Errorf("error renewing ACME certificate: %w", err) + } + + if resBody.Data == nil { + return nil, api.ErrNoDataObjectInResponse + } + + return resBody.Data, nil +} diff --git a/proxmox/nodes/certificate_types.go b/proxmox/nodes/certificate_types.go index 844e583d5..9307715f8 100644 --- a/proxmox/nodes/certificate_types.go +++ b/proxmox/nodes/certificate_types.go @@ -41,3 +41,18 @@ type CertificateUpdateRequestBody struct { PrivateKey *string `json:"key,omitempty" url:"key,omitempty"` Restart *types.CustomBool `json:"restart,omitempty" url:"restart,omitempty,int"` } + +// CertificateOrderRequestBody contains the body for an ACME certificate order request. +type CertificateOrderRequestBody struct { + Force *types.CustomBool `json:"force,omitempty" url:"force,omitempty,int"` +} + +// CertificateOrderResponseBody contains the body from an ACME certificate order response. +type CertificateOrderResponseBody struct { + Data *string `json:"data,omitempty"` +} + +// CertificateRenewRequestBody contains the body for an ACME certificate renewal request. +type CertificateRenewRequestBody struct { + Force *types.CustomBool `json:"force,omitempty" url:"force,omitempty,int"` +} diff --git a/proxmox/nodes/config.go b/proxmox/nodes/config.go index e7826aa25..ad271a715 100644 --- a/proxmox/nodes/config.go +++ b/proxmox/nodes/config.go @@ -32,7 +32,7 @@ func (c *Client) GetConfig(ctx context.Context) (*[]ConfigGetResponseData, error // UpdateConfig updates the config for a node. func (c *Client) UpdateConfig(ctx context.Context, d *ConfigUpdateRequestBody) error { - err := c.DoRequest(ctx, http.MethodPost, c.ExpandPath("config"), d, nil) + err := c.DoRequest(ctx, http.MethodPut, c.ExpandPath("config"), d, nil) if err != nil { return fmt.Errorf("error updating node config: %w", err) } diff --git a/proxmox/nodes/config_types.go b/proxmox/nodes/config_types.go index 5e3d81ce6..1db7744a0 100644 --- a/proxmox/nodes/config_types.go +++ b/proxmox/nodes/config_types.go @@ -47,26 +47,26 @@ type ConfigGetResponseData struct { // ConfigUpdateRequestBody contains the body for a config update request. type ConfigUpdateRequestBody struct { // Node specific ACME settings. - ACME *ACMEConfig `json:"acme,omitempty"` + ACME *ACMEConfig `json:"acme,omitempty" url:"acme,omitempty"` // ACME domain and validation plugin - ACMEDomain0 *ACMEDomainConfig `json:"acmedomain0,omitempty"` + ACMEDomain0 *ACMEDomainConfig `json:"acmedomain0,omitempty" url:"acmedomain0,omitempty"` // ACME domain and validation plugin - ACMEDomain1 *ACMEDomainConfig `json:"acmedomain1,omitempty"` + ACMEDomain1 *ACMEDomainConfig `json:"acmedomain1,omitempty" url:"acmedomain1,omitempty"` // ACME domain and validation plugin - ACMEDomain2 *ACMEDomainConfig `json:"acmedomain2,omitempty"` + ACMEDomain2 *ACMEDomainConfig `json:"acmedomain2,omitempty" url:"acmedomain2,omitempty"` // ACME domain and validation plugin - ACMEDomain3 *ACMEDomainConfig `json:"acmedomain3,omitempty"` + ACMEDomain3 *ACMEDomainConfig `json:"acmedomain3,omitempty" url:"acmedomain3,omitempty"` // ACME domain and validation plugin - ACMEDomain4 *ACMEDomainConfig `json:"acmedomain4,omitempty"` - Delete *string `json:"delete,omitempty"` + ACMEDomain4 *ACMEDomainConfig `json:"acmedomain4,omitempty" url:"acmedomain4,omitempty"` + Delete *string `json:"delete,omitempty" url:"delete,omitempty"` // Description for the Node. Shown in the web-interface node notes panel. This is saved as comment inside the configuration file. - Description *string `json:"description,omitempty"` + Description *string `json:"description,omitempty" url:"description,omitempty"` // Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications. - Digest *string `json:"digest,omitempty"` + Digest *string `json:"digest,omitempty" url:"digest,omitempty"` // Initial delay in seconds, before starting all the Virtual Guests with on-boot enabled. - StartAllOnbootDelay *int `json:"startall-onboot-delay,omitempty"` + StartAllOnbootDelay *int `json:"startall-onboot-delay,omitempty" url:"startall-onboot-delay,omitempty"` // Node specific wake on LAN settings. - WakeOnLan *WakeOnLandConfig `json:"wakeonlan,omitempty"` + WakeOnLan *WakeOnLandConfig `json:"wakeonlan,omitempty" url:"wakeonlan,omitempty"` } // ACMEConfig contains the ACME account / domains configuration that use the "standalone" plugin (http challenge). @@ -110,12 +110,26 @@ func (a *ACMEConfig) UnmarshalJSON(b []byte) error { // EncodeValues encodes a ACMEConfig struct into a string. func (a *ACMEConfig) EncodeValues(key string, v *url.Values) error { + // Skip encoding if the config is nil + if a == nil { + return nil + } + value := "" if a.Account != nil { value = fmt.Sprintf("account=%s", *a.Account) } - value = fmt.Sprintf("%s,%s", value, strings.Join(a.Domains, ";")) + // Only add domains if there are any + if len(a.Domains) > 0 { + domainsStr := strings.Join(a.Domains, ";") + if value != "" { + value = fmt.Sprintf("%s,domains=%s", value, domainsStr) + } else { + value = fmt.Sprintf("domains=%s", domainsStr) + } + } + v.Add(key, value) return nil @@ -166,6 +180,11 @@ func (a *ACMEDomainConfig) UnmarshalJSON(b []byte) error { // EncodeValues encodes a ACMEDomainConfig struct into a string. func (a *ACMEDomainConfig) EncodeValues(key string, v *url.Values) error { + // Skip encoding if the config is nil (unused domain slot) + if a == nil { + return nil + } + value := a.Domain if a.Alias != nil { value = fmt.Sprintf("%s,alias=%s", value, *a.Alias) @@ -225,6 +244,11 @@ func (a *WakeOnLandConfig) UnmarshalJSON(b []byte) error { // EncodeValues encodes a WakeOnLandConfig struct into a string. func (a *WakeOnLandConfig) EncodeValues(key string, v *url.Values) error { + // Skip encoding if the config is nil + if a == nil { + return nil + } + value := a.MACAddress if a.BindInterface != nil { value = fmt.Sprintf("%s,bind-interface=%s", value, *a.BindInterface) From 11ab40fc1b64117d4942323b60cd038c5811ebfa Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:08:39 +0100 Subject: [PATCH 02/15] fix: time.Sleep to wait certificate Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 3d28a5fdb..97a42a0cd 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -11,6 +11,7 @@ import ( "fmt" "time" + "github.com/avast/retry-go/v4" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -204,6 +205,39 @@ func (r *acmeCertificateResource) Configure( r.client = cfg.Client } +// waitForCertificateAvailable polls ListCertificates until a certificate is available. +// This replaces the fixed time.Sleep with a more robust retry mechanism. +func (r *acmeCertificateResource) waitForCertificateAvailable( + ctx context.Context, + nodeClient *nodes.Client, +) (*[]nodes.CertificateListResponseData, error) { + var certificates *[]nodes.CertificateListResponseData + + err := retry.Do( + func() error { + certs, err := nodeClient.ListCertificates(ctx) + if err != nil { + return err + } + + // Check if any certificates are found + if certs == nil || len(*certs) == 0 { + return fmt.Errorf("no certificates found yet") + } + + certificates = certs + return nil + }, + retry.Attempts(30), // Maximum 30 attempts + retry.Delay(1 * time.Second), // Start with 1 second delay + retry.DelayType(retry.BackOffDelay), // Use exponential backoff + retry.MaxJitter(500 * time.Millisecond), // Add jitter to prevent thundering herd + retry.Context(ctx), // Respect context cancellation + ) + + return certificates, err +} + // Create orders a new ACME certificate for the node. func (r *acmeCertificateResource) Create( ctx context.Context, @@ -260,15 +294,12 @@ func (r *acmeCertificateResource) Create( } } - // Wait a bit for the certificate to be written - time.Sleep(2 * time.Second) - - // Read the certificate information - certificates, err := nodeClient.ListCertificates(ctx) + // Poll for the certificate to be available using retry mechanism + certificates, err := r.waitForCertificateAvailable(ctx, nodeClient) if err != nil { resp.Diagnostics.AddError( "Unable to read certificate information", - fmt.Sprintf("An error occurred while reading certificate information for node %s: %s", nodeName, err.Error()), + fmt.Sprintf("Failed to retrieve the ordered certificate for node %s after multiple attempts: %s", nodeName, err.Error()), ) return @@ -389,15 +420,12 @@ func (r *acmeCertificateResource) Update( } } - // Wait a bit for the certificate to be written - time.Sleep(2 * time.Second) - - // Read the updated certificate information - certificates, err := nodeClient.ListCertificates(ctx) + // Poll for the certificate to be available using retry mechanism + certificates, err := r.waitForCertificateAvailable(ctx, nodeClient) if err != nil { resp.Diagnostics.AddError( "Unable to read certificate information", - fmt.Sprintf("An error occurred while reading certificate information for node %s: %s", nodeName, err.Error()), + fmt.Sprintf("Failed to retrieve the renewed certificate for node %s after multiple attempts: %s", nodeName, err.Error()), ) return From 08e924ff845e27367fa89cdbcd58cd7ccd8dbec7 Mon Sep 17 00:00:00 2001 From: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Date: Wed, 29 Oct 2025 00:28:48 +0100 Subject: [PATCH 03/15] Update docs/resources/virtual_environment_acme_certificate.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Signed-off-by: lonelysadness --- docs/resources/virtual_environment_acme_certificate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/resources/virtual_environment_acme_certificate.md b/docs/resources/virtual_environment_acme_certificate.md index 87e929da0..f3df9a4af 100644 --- a/docs/resources/virtual_environment_acme_certificate.md +++ b/docs/resources/virtual_environment_acme_certificate.md @@ -68,7 +68,7 @@ resource "proxmox_virtual_environment_acme_certificate" "test" { ] depends_on = [ - proxmox_virtual_environment_acme_account.test, + proxmox_virtual_environment_acme_account.example, proxmox_virtual_environment_acme_dns_plugin.desec ] } From 7590a20da1ca09e1a99af6ac2a39b8fca95fd655 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:18:31 +0100 Subject: [PATCH 04/15] fix: return the first certificate Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 122 +++++++++++++++++- 1 file changed, 120 insertions(+), 2 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 97a42a0cd..8c034d37e 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -9,6 +9,7 @@ package nodes import ( "context" "fmt" + "strings" "time" "github.com/avast/retry-go/v4" @@ -651,6 +652,120 @@ func (r *acmeCertificateResource) configureNodeACME( return nodeClient.UpdateConfig(ctx, configUpdate) } +// isProxmoxGeneratedCertificate checks if a certificate is generated by Proxmox itself. +// This helps identify Proxmox's self-signed or auto-generated certificates that should be skipped +// when looking for ACME certificates. Proxmox-generated certificates have "Proxmox" in the issuer. +func isProxmoxGeneratedCertificate(cert *nodes.CertificateListResponseData) bool { + if cert.Issuer == nil { + return false + } + // Check if issuer contains "Proxmox" or "PVE" (Proxmox VE) + issuer := *cert.Issuer + return strings.Contains(issuer, "Proxmox") || strings.Contains(issuer, "PVE") +} + +// findMatchingCertificate finds the certificate that matches the domains in the model. +// It prioritizes ACME certificates (issued by certificate authorities like Let's Encrypt) +// over Proxmox-generated certificates. When multiple certificates match the configured domains, +// it returns the one with the most matching domains. +func (r *acmeCertificateResource) findMatchingCertificate( + model *acmeCertificateModel, + certificates *[]nodes.CertificateListResponseData, +) (*nodes.CertificateListResponseData, error) { + if certificates == nil || len(*certificates) == 0 { + return nil, fmt.Errorf("no certificates found") + } + + // Extract domains from the model + var configDomains []string + diag := model.Domains.ElementsAs(context.Background(), &configDomains, false) + if diag.HasError() { + // If we can't parse domains, try to find an ACME certificate (not Proxmox-generated) + for i := range *certificates { + if !isProxmoxGeneratedCertificate(&(*certificates)[i]) { + return &(*certificates)[i], nil + } + } + // Fall back to first certificate if all are Proxmox-generated + return &(*certificates)[0], nil + } + + // Convert to a map for faster lookup + domainMap := make(map[string]bool) + for _, domain := range configDomains { + domainMap[domain] = true + } + + // Find the certificate that matches the most domains, preferring ACME certificates + var bestMatch *nodes.CertificateListResponseData + bestMatchCount := 0 + bestMatchIsProxmoxGen := true + + for i := range *certificates { + cert := &(*certificates)[i] + isProxmoxGen := isProxmoxGeneratedCertificate(cert) + matchCount := 0 + + // Check Subject Alternative Names (primary matching criteria for ACME certs) + if cert.SubjectAlternativeNames != nil { + for _, san := range *cert.SubjectAlternativeNames { + if domainMap[san] { + matchCount++ + } + } + } + + // Check Subject field (CN) if SANs don't have matches + if cert.Subject != nil && matchCount == 0 { + // Extract CN from Subject string (format: CN=domain.com,...) + // Simple extraction: look for CN= and take until the next comma + subject := *cert.Subject + if cnIdx := strings.Index(subject, "CN="); cnIdx != -1 { + cnStart := cnIdx + 3 + cnEnd := strings.Index(subject[cnStart:], ",") + if cnEnd == -1 { + cnEnd = len(subject[cnStart:]) + } else { + cnEnd += cnStart + } + cn := subject[cnStart:cnEnd] + if domainMap[cn] { + matchCount++ + } + } + } + + // Update best match if: + // 1. This certificate matches more domains, OR + // 2. It matches the same domains but is ACME (not Proxmox-generated) + if matchCount > bestMatchCount || (matchCount > 0 && matchCount == bestMatchCount && !isProxmoxGen && bestMatchIsProxmoxGen) { + bestMatch = cert + bestMatchCount = matchCount + bestMatchIsProxmoxGen = isProxmoxGen + + // If we found an ACME certificate with all domains, we can stop searching + if bestMatchCount == len(domainMap) && !isProxmoxGen { + break + } + } + } + + // If we found a certificate with matching domains, return it + if bestMatch != nil && bestMatchCount > 0 { + return bestMatch, nil + } + + // If no domain matches found, prefer ACME certificates (not Proxmox-generated) + for i := range *certificates { + if !isProxmoxGeneratedCertificate(&(*certificates)[i]) { + return &(*certificates)[i], nil + } + } + + // Last resort: return first certificate (shouldn't reach here in normal cases) + return &(*certificates)[0], nil +} + // updateModelFromCertificates updates the model with certificate information. func (r *acmeCertificateResource) updateModelFromCertificates( model *acmeCertificateModel, @@ -660,8 +775,11 @@ func (r *acmeCertificateResource) updateModelFromCertificates( return fmt.Errorf("no certificates found") } - // Find the first certificate (usually the active one) - cert := (*certificates)[0] + // Find the certificate that matches the configured domains + cert, err := r.findMatchingCertificate(model, certificates) + if err != nil { + return err + } // Update basic certificate fields model.Certificate = stringPtrToValue(cert.Certificates) From 9aed62f904fe5228fef5edd9fa843bd765340b12 Mon Sep 17 00:00:00 2001 From: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Date: Wed, 29 Oct 2025 08:19:54 +0100 Subject: [PATCH 05/15] Update fwprovider/nodes/resource_acme_certificate.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 8c034d37e..bb3179bcd 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -392,21 +392,22 @@ func (r *acmeCertificateResource) Update( } } - // Renew the certificate if force is true or other changes are made + // Order a new certificate if force is true or other changes are made force := proxmoxtypes.CustomBool(plan.Force.ValueBool()) - renewReq := &nodes.CertificateRenewRequestBody{ + orderReq := &nodes.CertificateOrderRequestBody{ Force: &force, } - taskID, err := nodeClient.RenewCertificate(ctx, renewReq) + taskID, err := nodeClient.OrderCertificate(ctx, orderReq) if err != nil { resp.Diagnostics.AddError( - "Unable to renew ACME certificate", - fmt.Sprintf("An error occurred while renewing the ACME certificate for node %s: %s", nodeName, err.Error()), + "Unable to re-order ACME certificate", + fmt.Sprintf("An error occurred while re-ordering the ACME certificate for node %s: %s", nodeName, err.Error()), ) return } + } // Wait for the task to complete if taskID != nil && *taskID != "" { From 266b8453fc28dce4bdeb5b2b530b926bc08b00cf Mon Sep 17 00:00:00 2001 From: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Date: Wed, 29 Oct 2025 08:20:08 +0100 Subject: [PATCH 06/15] Update fwprovider/nodes/resource_acme_certificate.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: lonelysadness <145705478+lonelysadness@users.noreply.github.com> Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index bb3179bcd..4271acd6f 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -808,7 +808,7 @@ func (r *acmeCertificateResource) updateModelFromCertificates( sanList = append(sanList, types.StringValue(san)) } - list, _ := types.ListValueFrom(context.Background(), types.StringType, sanList) + list, _ := types.ListValueFrom(ctx, types.StringType, sanList) model.SubjectAlternativeNames = list } else { model.SubjectAlternativeNames = types.ListNull(types.StringType) From 7d691842019adbb5b6e7eb1364b386d0939b9d38 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:32:51 +0100 Subject: [PATCH 07/15] fix: added missing ctx Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 4271acd6f..e320f8076 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -307,7 +307,7 @@ func (r *acmeCertificateResource) Create( } // Update the state with certificate information - if err := r.updateModelFromCertificates(&plan, certificates); err != nil { + if err := r.updateModelFromCertificates(ctx, &plan, certificates); err != nil { resp.Diagnostics.AddError( "Unable to process certificate information", fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), @@ -350,7 +350,7 @@ func (r *acmeCertificateResource) Read( } // Update the state with certificate information - if err := r.updateModelFromCertificates(&state, certificates); err != nil { + if err := r.updateModelFromCertificates(ctx, &state, certificates); err != nil { resp.Diagnostics.AddError( "Unable to process certificate information", fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), @@ -407,7 +407,6 @@ func (r *acmeCertificateResource) Update( return } - } // Wait for the task to complete if taskID != nil && *taskID != "" { @@ -434,7 +433,7 @@ func (r *acmeCertificateResource) Update( } // Update the state with certificate information - if err := r.updateModelFromCertificates(&plan, certificates); err != nil { + if err := r.updateModelFromCertificates(ctx, &plan, certificates); err != nil { resp.Diagnostics.AddError( "Unable to process certificate information", fmt.Sprintf("An error occurred while processing certificate information: %s", err.Error()), @@ -670,6 +669,7 @@ func isProxmoxGeneratedCertificate(cert *nodes.CertificateListResponseData) bool // over Proxmox-generated certificates. When multiple certificates match the configured domains, // it returns the one with the most matching domains. func (r *acmeCertificateResource) findMatchingCertificate( + ctx context.Context, model *acmeCertificateModel, certificates *[]nodes.CertificateListResponseData, ) (*nodes.CertificateListResponseData, error) { @@ -679,7 +679,7 @@ func (r *acmeCertificateResource) findMatchingCertificate( // Extract domains from the model var configDomains []string - diag := model.Domains.ElementsAs(context.Background(), &configDomains, false) + diag := model.Domains.ElementsAs(ctx, &configDomains, false) if diag.HasError() { // If we can't parse domains, try to find an ACME certificate (not Proxmox-generated) for i := range *certificates { @@ -769,6 +769,7 @@ func (r *acmeCertificateResource) findMatchingCertificate( // updateModelFromCertificates updates the model with certificate information. func (r *acmeCertificateResource) updateModelFromCertificates( + ctx context.Context, model *acmeCertificateModel, certificates *[]nodes.CertificateListResponseData, ) error { @@ -777,7 +778,7 @@ func (r *acmeCertificateResource) updateModelFromCertificates( } // Find the certificate that matches the configured domains - cert, err := r.findMatchingCertificate(model, certificates) + cert, err := r.findMatchingCertificate(ctx, model, certificates) if err != nil { return err } From 0ee8e706302d40c4cd20485b5512703f54817523 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:35:47 +0100 Subject: [PATCH 08/15] fix: handle error when creating subject alternative names list Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index e320f8076..1424b93ac 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -809,7 +809,10 @@ func (r *acmeCertificateResource) updateModelFromCertificates( sanList = append(sanList, types.StringValue(san)) } - list, _ := types.ListValueFrom(ctx, types.StringType, sanList) + list, diag := types.ListValueFrom(ctx, types.StringType, sanList) + if diag.HasError() { + return fmt.Errorf("error creating subject_alternative_names list: %v", diag.Errors()) + } model.SubjectAlternativeNames = list } else { model.SubjectAlternativeNames = types.ListNull(types.StringType) From 6608698556f33fb4d84cc7cd9c405927005a5d2e Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:48:03 +0100 Subject: [PATCH 09/15] fix: correct domain parsing in certificate matching logic Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 1424b93ac..e743effae 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -678,8 +678,8 @@ func (r *acmeCertificateResource) findMatchingCertificate( } // Extract domains from the model - var configDomains []string - diag := model.Domains.ElementsAs(ctx, &configDomains, false) + var domainModels []acmeDomainModel + diag := model.Domains.ElementsAs(ctx, &domainModels, false) if diag.HasError() { // If we can't parse domains, try to find an ACME certificate (not Proxmox-generated) for i := range *certificates { @@ -691,6 +691,12 @@ func (r *acmeCertificateResource) findMatchingCertificate( return &(*certificates)[0], nil } + // Extract domain strings for matching + configDomains := make([]string, len(domainModels)) + for i, dm := range domainModels { + configDomains[i] = dm.Domain.ValueString() + } + // Convert to a map for faster lookup domainMap := make(map[string]bool) for _, domain := range configDomains { From f17e7499205eeafdd059583db0610efe3906c0a3 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:48:58 +0100 Subject: [PATCH 10/15] docs: complete HTTP-01 certificate example Signed-off-by: lonelysadness --- docs/resources/virtual_environment_acme_certificate.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/resources/virtual_environment_acme_certificate.md b/docs/resources/virtual_environment_acme_certificate.md index f3df9a4af..b83e59325 100644 --- a/docs/resources/virtual_environment_acme_certificate.md +++ b/docs/resources/virtual_environment_acme_certificate.md @@ -25,10 +25,13 @@ resource "proxmox_virtual_environment_acme_account" "example" { # Order a certificate for the node resource "proxmox_virtual_environment_acme_certificate" "example" { node_name = "pve" + account = proxmox_virtual_environment_acme_account.example.name - # Depends on the ACME account being created first - depends_on = [ - proxmox_virtual_environment_acme_account.example + domains = [ + { + domain = "pve.example.com" + # No plugin specified implies HTTP-01 challenge + } ] } ``` From 176b66621f0b17fbe85f9a3ee9e38a570ae8f378 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:49:18 +0100 Subject: [PATCH 11/15] docs: complete force renewal certificate example Signed-off-by: lonelysadness --- docs/resources/virtual_environment_acme_certificate.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/resources/virtual_environment_acme_certificate.md b/docs/resources/virtual_environment_acme_certificate.md index b83e59325..472fbfdce 100644 --- a/docs/resources/virtual_environment_acme_certificate.md +++ b/docs/resources/virtual_environment_acme_certificate.md @@ -82,7 +82,14 @@ resource "proxmox_virtual_environment_acme_certificate" "test" { ```terraform resource "proxmox_virtual_environment_acme_certificate" "example_force" { node_name = "pve" + account = proxmox_virtual_environment_acme_account.example.name force = true # This will trigger renewal on every apply + + domains = [ + { + domain = "pve.example.com" + } + ] } ``` From 54b4053a41e70b806fd7d51487a312dc41855409 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 08:50:39 +0100 Subject: [PATCH 12/15] chore: remove unused RenewCertificate function and type Signed-off-by: lonelysadness --- proxmox/nodes/certificate.go | 15 --------------- proxmox/nodes/certificate_types.go | 5 ----- 2 files changed, 20 deletions(-) diff --git a/proxmox/nodes/certificate.go b/proxmox/nodes/certificate.go index f005c2121..6a808a5f4 100644 --- a/proxmox/nodes/certificate.go +++ b/proxmox/nodes/certificate.go @@ -66,18 +66,3 @@ func (c *Client) OrderCertificate(ctx context.Context, d *CertificateOrderReques return resBody.Data, nil } -// RenewCertificate renews an existing ACME certificate for a node. -func (c *Client) RenewCertificate(ctx context.Context, d *CertificateRenewRequestBody) (*string, error) { - resBody := &CertificateOrderResponseBody{} - - err := c.DoRequest(ctx, http.MethodPut, c.ExpandPath("certificates/acme/certificate"), d, resBody) - if err != nil { - return nil, fmt.Errorf("error renewing ACME certificate: %w", err) - } - - if resBody.Data == nil { - return nil, api.ErrNoDataObjectInResponse - } - - return resBody.Data, nil -} diff --git a/proxmox/nodes/certificate_types.go b/proxmox/nodes/certificate_types.go index 9307715f8..e7244cdc6 100644 --- a/proxmox/nodes/certificate_types.go +++ b/proxmox/nodes/certificate_types.go @@ -51,8 +51,3 @@ type CertificateOrderRequestBody struct { type CertificateOrderResponseBody struct { Data *string `json:"data,omitempty"` } - -// CertificateRenewRequestBody contains the body for an ACME certificate renewal request. -type CertificateRenewRequestBody struct { - Force *types.CustomBool `json:"force,omitempty" url:"force,omitempty,int"` -} From 0887826f5aa92fc7d507caa669204d5010c9cf1d Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 09:12:42 +0100 Subject: [PATCH 13/15] fix: clean up ACME configuration Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 91 +++++++++++-------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index e743effae..24cae5517 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -445,9 +445,8 @@ func (r *acmeCertificateResource) Update( resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } -// Delete removes the certificate resource from Terraform state. -// Note: This doesn't actually delete the certificate from the node, -// as ACME certificates should remain on the node for the services to use. +// Delete removes the certificate resource from Terraform state and cleans up ACME configuration from the node. +// The certificate files are preserved on the Proxmox node, but the ACME configuration is removed. func (r *acmeCertificateResource) Delete( ctx context.Context, req resource.DeleteRequest, @@ -461,10 +460,22 @@ func (r *acmeCertificateResource) Delete( return } - // Note: We don't actually delete the certificate from the node - // as it's needed for the node's services. We just remove it from Terraform state. - // If users want to remove the certificate from the node, they should use - // the Proxmox UI or manually delete it via the API. + nodeName := state.NodeName.ValueString() + nodeClient := r.client.Node(nodeName) + + // Clean up the ACME configuration from the node. The certificate files will remain. + toDelete := "acme,acmedomain0,acmedomain1,acmedomain2,acmedomain3,acmedomain4" + configUpdate := &nodes.ConfigUpdateRequestBody{ + Delete: &toDelete, + } + + if err := nodeClient.UpdateConfig(ctx, configUpdate); err != nil { + // Log a warning as the resource is being deleted anyway, but the user should be notified. + resp.Diagnostics.AddWarning( + "Failed to clean up node ACME configuration", + fmt.Sprintf("An error occurred while cleaning up ACME settings for node %s on delete: %s. Manual cleanup of /etc/pve/nodes/%s/config may be required.", nodeName, err.Error(), nodeName), + ) + } } // ImportState imports an existing certificate by node name. @@ -612,42 +623,50 @@ func (r *acmeCertificateResource) configureNodeACME( } // Configure DNS challenge domains (up to 5 domains with DNS plugins) - if len(dnsDomains) > 0 { - if len(dnsDomains) > 5 { - return fmt.Errorf("Proxmox supports a maximum of 5 DNS challenge domains, got %d", len(dnsDomains)) - } + if len(dnsDomains) > 5 { + return fmt.Errorf("Proxmox supports a maximum of 5 DNS challenge domains, got %d", len(dnsDomains)) + } - for i, domain := range dnsDomains { - domainConfig := &nodes.ACMEDomainConfig{ - Domain: domain.Domain.ValueString(), - } + for i, domain := range dnsDomains { + domainConfig := &nodes.ACMEDomainConfig{ + Domain: domain.Domain.ValueString(), + } - if !domain.Plugin.IsNull() { - plugin := domain.Plugin.ValueString() - domainConfig.Plugin = &plugin - } + if !domain.Plugin.IsNull() { + plugin := domain.Plugin.ValueString() + domainConfig.Plugin = &plugin + } - if !domain.Alias.IsNull() { - alias := domain.Alias.ValueString() - domainConfig.Alias = &alias - } + if !domain.Alias.IsNull() { + alias := domain.Alias.ValueString() + domainConfig.Alias = &alias + } - // Map to the appropriate acmedomain field - switch i { - case 0: - configUpdate.ACMEDomain0 = domainConfig - case 1: - configUpdate.ACMEDomain1 = domainConfig - case 2: - configUpdate.ACMEDomain2 = domainConfig - case 3: - configUpdate.ACMEDomain3 = domainConfig - case 4: - configUpdate.ACMEDomain4 = domainConfig - } + // Map to the appropriate acmedomain field + switch i { + case 0: + configUpdate.ACMEDomain0 = domainConfig + case 1: + configUpdate.ACMEDomain1 = domainConfig + case 2: + configUpdate.ACMEDomain2 = domainConfig + case 3: + configUpdate.ACMEDomain3 = domainConfig + case 4: + configUpdate.ACMEDomain4 = domainConfig } } + // Clean up unused acmedomain slots + var toDelete []string + for i := len(dnsDomains); i < 5; i++ { + toDelete = append(toDelete, fmt.Sprintf("acmedomain%d", i)) + } + if len(toDelete) > 0 { + deleteValue := strings.Join(toDelete, ",") + configUpdate.Delete = &deleteValue + } + // Update the node configuration return nodeClient.UpdateConfig(ctx, configUpdate) } From fd0130c2d01c5fb7254b0e0b3da2926202548f39 Mon Sep 17 00:00:00 2001 From: lonelysadness Date: Wed, 29 Oct 2025 09:22:17 +0100 Subject: [PATCH 14/15] fix: delete certificate on resource destruction Signed-off-by: lonelysadness --- fwprovider/nodes/resource_acme_certificate.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index 24cae5517..b31df47f9 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -445,8 +445,7 @@ func (r *acmeCertificateResource) Update( resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } -// Delete removes the certificate resource from Terraform state and cleans up ACME configuration from the node. -// The certificate files are preserved on the Proxmox node, but the ACME configuration is removed. +// Delete removes the certificate resource from Terraform state and cleans up ACME configuration and certificate from the node. func (r *acmeCertificateResource) Delete( ctx context.Context, req resource.DeleteRequest, @@ -463,14 +462,26 @@ func (r *acmeCertificateResource) Delete( nodeName := state.NodeName.ValueString() nodeClient := r.client.Node(nodeName) - // Clean up the ACME configuration from the node. The certificate files will remain. + // Delete the custom certificate + restart := proxmoxtypes.CustomBool(true) + deleteReq := &nodes.CertificateDeleteRequestBody{ + Restart: &restart, + } + + if err := nodeClient.DeleteCertificate(ctx, deleteReq); err != nil { + resp.Diagnostics.AddWarning( + "Failed to delete certificate", + fmt.Sprintf("An error occurred while deleting the certificate for node %s: %s", nodeName, err.Error()), + ) + } + + // Clean up the ACME configuration from the node toDelete := "acme,acmedomain0,acmedomain1,acmedomain2,acmedomain3,acmedomain4" configUpdate := &nodes.ConfigUpdateRequestBody{ Delete: &toDelete, } if err := nodeClient.UpdateConfig(ctx, configUpdate); err != nil { - // Log a warning as the resource is being deleted anyway, but the user should be notified. resp.Diagnostics.AddWarning( "Failed to clean up node ACME configuration", fmt.Sprintf("An error occurred while cleaning up ACME settings for node %s on delete: %s. Manual cleanup of /etc/pve/nodes/%s/config may be required.", nodeName, err.Error(), nodeName), From 68f8f1ec72e288229fa9c8d1374f28bdac90232c Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Sat, 1 Nov 2025 12:09:56 -0400 Subject: [PATCH 15/15] fix linter error Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- fwprovider/nodes/resource_acme_certificate.go | 25 +++++++++++++++---- proxmox/nodes/certificate.go | 1 - proxmox/nodes/config_types.go | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/fwprovider/nodes/resource_acme_certificate.go b/fwprovider/nodes/resource_acme_certificate.go index b31df47f9..b8c391aa5 100644 --- a/fwprovider/nodes/resource_acme_certificate.go +++ b/fwprovider/nodes/resource_acme_certificate.go @@ -227,16 +227,20 @@ func (r *acmeCertificateResource) waitForCertificateAvailable( } certificates = certs + return nil }, retry.Attempts(30), // Maximum 30 attempts - retry.Delay(1 * time.Second), // Start with 1 second delay + retry.Delay(1*time.Second), // Start with 1 second delay retry.DelayType(retry.BackOffDelay), // Use exponential backoff - retry.MaxJitter(500 * time.Millisecond), // Add jitter to prevent thundering herd + retry.MaxJitter(500*time.Millisecond), // Add jitter to prevent thundering herd retry.Context(ctx), // Respect context cancellation ) + if err != nil { + return nil, fmt.Errorf("waiting for certificate availability: %w", err) + } - return certificates, err + return certificates, nil } // Create orders a new ACME certificate for the node. @@ -484,7 +488,11 @@ func (r *acmeCertificateResource) Delete( if err := nodeClient.UpdateConfig(ctx, configUpdate); err != nil { resp.Diagnostics.AddWarning( "Failed to clean up node ACME configuration", - fmt.Sprintf("An error occurred while cleaning up ACME settings for node %s on delete: %s. Manual cleanup of /etc/pve/nodes/%s/config may be required.", nodeName, err.Error(), nodeName), + fmt.Sprintf( + "An error occurred while cleaning up ACME settings for node %s on delete: %s. "+ + "Manual cleanup of /etc/pve/nodes/%s/config may be required.", + nodeName, err.Error(), nodeName, + ), ) } } @@ -601,6 +609,7 @@ func (r *acmeCertificateResource) configureNodeACME( ) error { // Parse domains from the model var domains []acmeDomainModel + diag := model.Domains.ElementsAs(ctx, &domains, false) if diag.HasError() { return fmt.Errorf("error parsing domains: %v", diag.Errors()) @@ -635,7 +644,7 @@ func (r *acmeCertificateResource) configureNodeACME( // Configure DNS challenge domains (up to 5 domains with DNS plugins) if len(dnsDomains) > 5 { - return fmt.Errorf("Proxmox supports a maximum of 5 DNS challenge domains, got %d", len(dnsDomains)) + return fmt.Errorf("proxmox supports a maximum of 5 DNS challenge domains, got %d", len(dnsDomains)) } for i, domain := range dnsDomains { @@ -673,6 +682,7 @@ func (r *acmeCertificateResource) configureNodeACME( for i := len(dnsDomains); i < 5; i++ { toDelete = append(toDelete, fmt.Sprintf("acmedomain%d", i)) } + if len(toDelete) > 0 { deleteValue := strings.Join(toDelete, ",") configUpdate.Delete = &deleteValue @@ -691,6 +701,7 @@ func isProxmoxGeneratedCertificate(cert *nodes.CertificateListResponseData) bool } // Check if issuer contains "Proxmox" or "PVE" (Proxmox VE) issuer := *cert.Issuer + return strings.Contains(issuer, "Proxmox") || strings.Contains(issuer, "PVE") } @@ -709,6 +720,7 @@ func (r *acmeCertificateResource) findMatchingCertificate( // Extract domains from the model var domainModels []acmeDomainModel + diag := model.Domains.ElementsAs(ctx, &domainModels, false) if diag.HasError() { // If we can't parse domains, try to find an ACME certificate (not Proxmox-generated) @@ -759,12 +771,14 @@ func (r *acmeCertificateResource) findMatchingCertificate( subject := *cert.Subject if cnIdx := strings.Index(subject, "CN="); cnIdx != -1 { cnStart := cnIdx + 3 + cnEnd := strings.Index(subject[cnStart:], ",") if cnEnd == -1 { cnEnd = len(subject[cnStart:]) } else { cnEnd += cnStart } + cn := subject[cnStart:cnEnd] if domainMap[cn] { matchCount++ @@ -849,6 +863,7 @@ func (r *acmeCertificateResource) updateModelFromCertificates( if diag.HasError() { return fmt.Errorf("error creating subject_alternative_names list: %v", diag.Errors()) } + model.SubjectAlternativeNames = list } else { model.SubjectAlternativeNames = types.ListNull(types.StringType) diff --git a/proxmox/nodes/certificate.go b/proxmox/nodes/certificate.go index 6a808a5f4..30cafdc71 100644 --- a/proxmox/nodes/certificate.go +++ b/proxmox/nodes/certificate.go @@ -65,4 +65,3 @@ func (c *Client) OrderCertificate(ctx context.Context, d *CertificateOrderReques return resBody.Data, nil } - diff --git a/proxmox/nodes/config_types.go b/proxmox/nodes/config_types.go index 1db7744a0..c3efd4901 100644 --- a/proxmox/nodes/config_types.go +++ b/proxmox/nodes/config_types.go @@ -58,7 +58,7 @@ type ConfigUpdateRequestBody struct { ACMEDomain3 *ACMEDomainConfig `json:"acmedomain3,omitempty" url:"acmedomain3,omitempty"` // ACME domain and validation plugin ACMEDomain4 *ACMEDomainConfig `json:"acmedomain4,omitempty" url:"acmedomain4,omitempty"` - Delete *string `json:"delete,omitempty" url:"delete,omitempty"` + Delete *string `json:"delete,omitempty" url:"delete,omitempty"` // Description for the Node. Shown in the web-interface node notes panel. This is saved as comment inside the configuration file. Description *string `json:"description,omitempty" url:"description,omitempty"` // Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications.