chore(code): fix code quality warnings (#1409)

* chore(code): fix code quality warnings

+ re-enable Qodana on PRs

---------

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>

.github/workflows/code-quality.yml

@@ -2,8 +2,10 @@ name: Qodana
 on:
   workflow_dispatch:
   push:
-    branches: # Specify your branches here
-      - main # The 'main' branch
+    branches:
+      - main
+  pull_request:
+    types: [opened, synchronize]
 
 jobs:
   qodana:
@@ -19,5 +21,7 @@ jobs:
           fetch-depth: 0  # a full history is required for pull request analysis
       - name: 'Qodana Scan'
         uses: JetBrains/qodana-action@v2024.1.5
+        with:
+          post-pr-comment: false
         env:
           QODANA_TOKEN: ${{ secrets.QODANA_TOKEN }}

.github/workflows/test.yml

@@ -87,7 +87,6 @@ jobs:
         with:
           name: test-log
           path: /tmp/gotest.log
-          if-no-files-found: error
 
       - name: Check for uncommitted changes in generated docs
         run: make docs && git diff --exit-code

.github/workflows/testacc.yml

@@ -49,6 +49,9 @@ jobs:
       - name: Get dependencies
         run: go mod download
 
+      - name: Set up gotestfmt
+        run: go install github.com/gotesttools/gotestfmt/v2/cmd/gotestfmt@latest
+
       - uses: hashicorp/setup-terraform@651471c36a6092792c552e8b1bef71e592b462d8 # v3.1.1
         with:
           terraform_version: ${{ matrix.terraform }}.*
@@ -78,4 +81,3 @@ jobs:
         with:
           name: test-log
           path: /tmp/gotest.log
-          if-no-files-found: error

docs/resources/virtual_environment_vm2.md

@@ -36,6 +36,7 @@ The attributes are also marked as optional to allow the practitioner to set (or
 - `description` (String) The description of the VM.
 - `id` (Number) The unique identifier of the VM in the Proxmox cluster.
 - `name` (String) The name of the VM. Doesn't have to be unique.
+- `stop_on_destroy` (Boolean) Set to true to stop (rather than shutdown) the VM on destroy (defaults to `false`).
 - `tags` (Set of String) The tags assigned to the VM.
 - `template` (Boolean) Set to true to create a VM template.
 - `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts))

fwprovider/ha/resource_hagroup.go

@@ -81,8 +81,8 @@ func (r *hagroupResource) Schema(
 				Optional:    true,
 				Validators: []validator.String{
 					stringvalidator.UTF8LengthAtLeast(1),
-					stringvalidator.RegexMatches(regexp.MustCompile(`^[^\s]|^$`), "must not start with whitespace"),
-					stringvalidator.RegexMatches(regexp.MustCompile(`[^\s]$|^$`), "must not end with whitespace"),
+					stringvalidator.RegexMatches(regexp.MustCompile(`^\S|^$`), "must not start with whitespace"),
+					stringvalidator.RegexMatches(regexp.MustCompile(`\S$|^$`), "must not end with whitespace"),
 				},
 			},
 			"nodes": schema.MapAttribute{

fwprovider/ha/resource_haresource.go

@@ -101,8 +101,8 @@ func (r *haResourceResource) Schema(
 				Optional:    true,
 				Validators: []validator.String{
 					stringvalidator.UTF8LengthAtLeast(1),
-					stringvalidator.RegexMatches(regexp.MustCompile(`^[^\s]|^$`), "must not start with whitespace"),
-					stringvalidator.RegexMatches(regexp.MustCompile(`[^\s]$|^$`), "must not end with whitespace"),
+					stringvalidator.RegexMatches(regexp.MustCompile(`^\S|^$`), "must not start with whitespace"),
+					stringvalidator.RegexMatches(regexp.MustCompile(`\S$|^$`), "must not end with whitespace"),
 				},
 			},
 			"group": schema.StringAttribute{

fwprovider/vm/model.go

@@ -35,13 +35,14 @@ type Model struct {
 		ID      types.Int64 `tfsdk:"id"`
 		Retries types.Int64 `tfsdk:"retries"`
 	} `tfsdk:"clone"`
-	ID       types.Int64     `tfsdk:"id"`
-	Name     types.String    `tfsdk:"name"`
-	NodeName types.String    `tfsdk:"node_name"`
-	Tags     stringset.Value `tfsdk:"tags"`
-	Template types.Bool      `tfsdk:"template"`
-	Timeouts timeouts.Value  `tfsdk:"timeouts"`
-	VGA      vga.Value       `tfsdk:"vga"`
+	ID            types.Int64     `tfsdk:"id"`
+	Name          types.String    `tfsdk:"name"`
+	NodeName      types.String    `tfsdk:"node_name"`
+	StopOnDestroy types.Bool      `tfsdk:"stop_on_destroy"`
+	Tags          stringset.Value `tfsdk:"tags"`
+	Template      types.Bool      `tfsdk:"template"`
+	Timeouts      timeouts.Value  `tfsdk:"timeouts"`
+	VGA           vga.Value       `tfsdk:"vga"`
 }
 
 // read retrieves the current state of the resource from the API and updates the state.

fwprovider/vm/resource.go

@@ -374,11 +374,8 @@ func (r *Resource) Delete(ctx context.Context, req resource.DeleteRequest, resp
 		return
 	}
 
-	// stop := d.Get(mkStopOnDestroy).(bool)
-	stop := false
-
 	if status.Status != "stopped" {
-		if stop {
+		if state.StopOnDestroy.ValueBool() {
 			if e := vmStop(ctx, vmAPI); e != nil {
 				resp.Diagnostics.AddWarning("Failed to stop VM", e.Error())
 			}
@@ -445,6 +442,9 @@ func (r *Resource) ImportState(
 		return
 	}
 
+	// not clear why this is needed, but ImportStateVerify fails without it
+	state.StopOnDestroy = types.BoolValue(false)
+
 	diags := resp.State.Set(ctx, state)
 	resp.Diagnostics.Append(diags...)
 }

fwprovider/vm/resource_schema.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"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/boolplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
@@ -88,6 +89,13 @@ func (r *Resource) Schema(
 				Description: "The name of the node where the VM is provisioned.",
 				Required:    true,
 			},
+			"stop_on_destroy": schema.BoolAttribute{
+				Description:         "Set to true to stop (rather than shutdown) the VM on destroy.",
+				MarkdownDescription: "Set to true to stop (rather than shutdown) the VM on destroy (defaults to `false`).",
+				Optional:            true,
+				Computed:            true,
+				Default:             booldefault.StaticBool(false),
+			},
 			"tags": stringset.ResourceAttribute("The tags assigned to the VM.", ""),
 			"template": schema.BoolAttribute{
 				Description: "Set to true to create a VM template.",

proxmox/nodes/tasks/tasks.go

@@ -45,7 +45,8 @@ func (c *Client) GetTaskStatus(ctx context.Context, upid string) (*GetTaskStatus
 // Reads first 50 lines by default.
 func (c *Client) GetTaskLog(ctx context.Context, upid string) ([]string, error) {
 	resBody := &GetTaskLogResponseBody{}
-	lines := []string{}
+
+	var lines []string //nolint: prealloc
 
 	path, err := c.BuildPath(upid, "log")
 	if err != nil {

proxmox/nodes/vms/custom_storage_device.go

@@ -112,7 +112,7 @@ func (d *CustomStorageDevice) IsCloudInitDrive(vmID int) bool {
 
 // EncodeOptions converts a CustomStorageDevice's common options a URL value.
 func (d *CustomStorageDevice) EncodeOptions() string {
-	values := []string{}
+	var values []string
 
 	if d.AIO != nil {
 		values = append(values, fmt.Sprintf("aio=%s", *d.AIO))

proxmox/nodes/vms/custom_usb_device.go

@@ -31,7 +31,7 @@ func (r *CustomUSBDevice) EncodeValues(key string, v *url.Values) error {
 		return fmt.Errorf("either device ID or resource mapping must be set")
 	}
 
-	values := []string{}
+	var values []string
 	if r.HostDevice != nil {
 		values = append(values, fmt.Sprintf("host=%s", *(r.HostDevice)))
 	}