Merge pull request #549 from simondeziel/golangci-lint

Improve golangci-lint coverage

.golangci.yml

@@ -1,18 +1,106 @@
 linters:
-  # TODO: remove the `disable` section
-  disable:
-    - errcheck
-    - ineffassign
-    - staticcheck
-    - unused
   enable:
+    - gci
     - godot
     - gofmt
     - misspell
     - whitespace
-    - gci
+    - revive
+issues:
+  exclude-use-default: false
 linters-settings:
   gci:
     sections:
       - standard
       - default
+  errcheck:
+    exclude-functions:
+      - (*os.File).Close
+  revive:
+    rules:
+      ## https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#exported
+      #- name: exported
+      #  arguments:
+      #    - "checkPrivateReceivers"
+      #    - "disableStutteringCheck"
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#unchecked-type-assertion
+      - name: unchecked-type-assertion
+        arguments:
+          - { "acceptIgnoredAssertionResult": true }
+
+      ## https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#var-naming
+      #- name: var-naming
+      #  arguments: # The arguments here are quite odd looking. See the rule description.
+      #    - [ ]
+      #    - [ ]
+      #    - [ { "upperCaseConst": true } ]
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#early-return
+      - name: early-return
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#redundant-import-alias
+      - name: redundant-import-alias
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#redefines-builtin-id
+      - name: redefines-builtin-id
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#struct-tag
+      - name: struct-tag
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#receiver-naming
+      - name: receiver-naming
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#deep-exit
+      - name: deep-exit
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#bool-literal-in-expr
+      - name: bool-literal-in-expr
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#comment-spacings
+      - name: comment-spacings
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#confusing-results
+      - name: confusing-results
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#use-any
+      - name: use-any
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#bare-return
+      - name: bare-return
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#empty-block
+      - name: empty-block
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#range-val-address
+      - name: range-val-address
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#range-val-in-closure
+      - name: range-val-in-closure
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#var-declaration
+      - name: var-declaration
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#useless-break
+      - name: useless-break
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#error-naming
+      - name: error-naming
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#indent-error-flow
+      - name: indent-error-flow
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#datarace
+      - name: datarace
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#modifies-value-receiver
+      - name: modifies-value-receiver
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#empty-lines
+      - name: empty-lines
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#duplicated-imports
+      - name: duplicated-imports
+
+      # https://github.com/mgechev/revive/blob/2a1701aadbedfcc175cb92836a51407bec382652/RULES_DESCRIPTIONS.md#error-return
+      - name: error-return

Makefile

@@ -65,7 +65,7 @@ static-analysis:
 .PHONY: update-gomod
 update-gomod:
 	$(GO) get -t -v -u ./...
-	$(GO) mod tidy --go=1.23.3
+	$(GO) mod tidy
 	$(GO) get toolchain@none
 	@echo "Dependencies updated"
 

internal/acctest/checks.go

@@ -121,7 +121,7 @@ func PreCheckLocalServerHTTPS(t *testing.T) {
 		t.Skip(`Skipping remote provider test. LXD is not available on "https://127.0.0.1:8443"`)
 	}
 
-	conn.Close()
+	_ = conn.Close()
 }
 
 // ConfigureTrustPassword sets and returns the trust password. If the server

internal/common/lxd_exec.go

@@ -13,6 +13,7 @@ import (
 	"github.com/terraform-lxd/terraform-provider-lxd/internal/utils"
 )
 
+// ExecTriggerType represents an exec trigger.
 type ExecTriggerType string
 
 const (
@@ -25,6 +26,7 @@ func (t ExecTriggerType) String() string {
 	return string(t)
 }
 
+// ExecModel represents exec command to be executed on LXD instance.
 type ExecModel struct {
 	Command      types.List   `tfsdk:"command"`
 	Environment  types.Map    `tfsdk:"environment"`

internal/common/lxd_import.go

@@ -37,25 +37,25 @@ type ImportMetadata struct {
 //	  "image":   "jammy" // option
 //	  "optKey2": "value"  // option
 //	}
-func (m ImportMetadata) ParseImportID(importId string) (map[string]string, diag.Diagnostic) {
-	if strings.TrimSpace(importId) == "" {
-		return nil, newImportIDError(m, importId, fmt.Errorf("Import ID cannot be empty"))
+func (m ImportMetadata) ParseImportID(importID string) (map[string]string, diag.Diagnostic) {
+	if strings.TrimSpace(importID) == "" {
+		return nil, newImportIDError(m, importID, fmt.Errorf("Import ID cannot be empty"))
 	}
 
 	// First split by comma to determine mandatory and optional parts.
-	parts := strings.Split(importId, ",")
+	parts := strings.Split(importID, ",")
 
 	// Extract fields (including project and remote) from first part.
 	result, err := processFields(parts[0], m.RequiredFields)
 	if err != nil {
-		return nil, newImportIDError(m, importId, err)
+		return nil, newImportIDError(m, importID, err)
 	}
 
 	// Extract options.
 	if len(parts) > 1 {
 		options, err := processOptions(parts[1:], m.AllowedOptions)
 		if err != nil {
-			return nil, newImportIDError(m, importId, err)
+			return nil, newImportIDError(m, importID, err)
 		}
 
 		for k, v := range options {
@@ -142,7 +142,7 @@ func processOptions(options []string, allowedOptions []string) (map[string]strin
 }
 
 // newImportIDError converts an error into terraform diagnostic.
-func newImportIDError(m ImportMetadata, importId string, err error) diag.Diagnostic {
+func newImportIDError(m ImportMetadata, importID string, err error) diag.Diagnostic {
 	remote := "[<remote>:]"
 	project := "[<project>/]"
 	if len(m.RequiredFields) > 1 {
@@ -157,7 +157,7 @@ func newImportIDError(m ImportMetadata, importId string, err error) diag.Diagnos
 	fields := fmt.Sprintf("<%s>", strings.Join(m.RequiredFields, ">/<"))
 
 	return diag.NewErrorDiagnostic(
-		fmt.Sprintf("Invalid import ID: %q", importId),
+		fmt.Sprintf("Invalid import ID: %q", importID),
 		fmt.Sprintf(
 			"%v.\n\nValid import format:\nimport lxd_%s.<resource> %s%s%s%s",
 			err, m.ResourceName, remote, project, fields, options,

internal/common/lxd_interface.go

@@ -10,7 +10,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
 
-// Interface represents LXD instance network interface.
+// InterfaceModel represents LXD instance network interface.
 type InterfaceModel struct {
 	// Real name of the interface within the instance. If config interface is
 	// defined as "eth0", the real interface within a container will have the
@@ -27,7 +27,7 @@ type InterfaceModel struct {
 	Addresses types.List `tfsdk:"ips"`
 }
 
-// Interface IP is a wrapper of the interface IP address.
+// IPModel is a wrapper of the interface IP address.
 type IPModel struct {
 	// IP address.
 	Address types.String `tfsdk:"address"`

internal/image/resource_publish_image.go

@@ -267,7 +267,12 @@ func (r PublishImageResource) Create(ctx context.Context, req resource.CreateReq
 
 	// Extract fingerprint from operation response.
 	opResp := op.Get()
-	imageFingerprint := opResp.Metadata["fingerprint"].(string)
+	imageFingerprint, ok := opResp.Metadata["fingerprint"].(string)
+	if !ok {
+		resp.Diagnostics.AddError("Failed to extract fingerprint from operation response", "")
+		return
+	}
+
 	plan.Fingerprint = types.StringValue(imageFingerprint)
 
 	// Update Terraform state.
@@ -418,7 +423,7 @@ func (r PublishImageResource) Delete(ctx context.Context, req resource.DeleteReq
 // SyncState fetches the server's current state for a published image and
 // updates the provided model. It then applies this updated model as the
 // new state in Terraform.
-func (_ PublishImageResource) SyncState(ctx context.Context, tfState *tfsdk.State, server lxd.InstanceServer, m PublishImageModel) diag.Diagnostics {
+func (r PublishImageResource) SyncState(ctx context.Context, tfState *tfsdk.State, server lxd.InstanceServer, m PublishImageModel) diag.Diagnostics {
 	var respDiags diag.Diagnostics
 
 	imageFingerprint := m.Fingerprint.ValueString()

internal/instance/resource_instance.go

@@ -959,16 +959,16 @@ func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest
 
 	// Execute commands.
 	for _, k := range utils.SortMapKeys(newExecs) {
-		new := newExecs[k]
-		old := oldExecs[k]
+		newExec := newExecs[k]
+		oldExec := oldExecs[k]
 
 		// Copy run count from state (if exists).
-		if old != nil {
-			new.RunCount = old.RunCount
+		if oldExec != nil {
+			newExec.RunCount = oldExec.RunCount
 		}
 
-		if instanceRunning && new.IsTriggered(instanceStarted) {
-			diags := new.Execute(ctx, server, instance.Name)
+		if instanceRunning && newExec.IsTriggered(instanceStarted) {
+			diags := newExec.Execute(ctx, server, instance.Name)
 			if diags.HasError() {
 				resp.Diagnostics.Append(diags...)
 				return
@@ -1195,7 +1195,7 @@ func (r InstanceResource) SyncState(ctx context.Context, tfState *tfsdk.State, s
 }
 
 // ComputedKeys returns list of computed config keys.
-func (_ InstanceModel) ComputedKeys() []string {
+func (m InstanceModel) ComputedKeys() []string {
 	return []string{
 		"image.",
 		"volatile.",

internal/instance/resource_instance_file.go

@@ -335,12 +335,16 @@ func (r InstanceFileResource) Delete(ctx context.Context, req resource.DeleteReq
 // createFileResourceID creates new file ID by concatenating remote,
 // instnaceName, and targetPath using colon.
 func createFileResourceID(remote string, instanceName string, targetPath string) string {
-	return fmt.Sprintf("%s:%s:%s", remote, instanceName, targetPath)
+	return remote + ":" + instanceName + ":" + targetPath
 }
 
 // splitFileResourceID splits file ID into remote, intanceName, and
 // targetPath strings.
-func splitFileResourceID(id string) (string, string, string) {
+func splitFileResourceID(id string) (remote string, instanceName string, targetPath string) {
 	pieces := strings.SplitN(id, ":", 3)
+	if len(pieces) != 3 {
+		return "", "", ""
+	}
+
 	return pieces[0], pieces[1], pieces[2]
 }

internal/instance/resource_instance_snapshot.go

@@ -152,16 +152,16 @@ func (r InstanceSnapshotResource) Create(ctx context.Context, req resource.Creat
 
 		// Wait for snapshot operation to complete.
 		serr = op.Wait()
-		if serr != nil {
-			if snapshotReq.Stateful && strings.Contains(serr.Error(), "Dumping FAILED") {
-				log.Printf("[DEBUG] Error creating stateful snapshot [retry %d]: %v", i, serr)
-				time.Sleep(3 * time.Second)
-			} else if strings.Contains(serr.Error(), "file has vanished") {
-				// Ignore, try again.
-				time.Sleep(3 * time.Second)
-			} else {
-				break
-			}
+		if serr == nil {
+			break
+		}
+
+		if snapshotReq.Stateful && strings.Contains(serr.Error(), "Dumping FAILED") {
+			log.Printf("[DEBUG] Error creating stateful snapshot [retry %d]: %v", i, serr)
+			time.Sleep(3 * time.Second)
+		} else if strings.Contains(serr.Error(), "file has vanished") {
+			// Ignore, try again.
+			time.Sleep(3 * time.Second)
 		} else {
 			break
 		}

internal/network/resource_network.go

@@ -343,7 +343,7 @@ func (r NetworkResource) SyncState(ctx context.Context, tfState *tfsdk.State, se
 }
 
 // ComputedKeys returns list of computed LXD config keys.
-func (_ NetworkModel) ComputedKeys() []string {
+func (m NetworkModel) ComputedKeys() []string {
 	return []string{
 		"bridge.mtu",
 		"ipv4.address",

internal/network/resource_network_forward.go

@@ -35,7 +35,7 @@ type NetworkForwardModel struct {
 	Config        types.Map    `tfsdk:"config"`
 }
 
-// NetworkForwardModel resource data model that matches the schema.
+// NetworkForwardPortModel resource data model that matches the schema.
 type NetworkForwardPortModel struct {
 	Description   types.String `tfsdk:"description"`
 	Protocol      types.String `tfsdk:"protocol"`

internal/profile/resource_profile.go

@@ -26,6 +26,7 @@ import (
 	provider_config "github.com/terraform-lxd/terraform-provider-lxd/internal/provider-config"
 )
 
+// ProfileModel represents a LXD profile.
 type ProfileModel struct {
 	Name        types.String `tfsdk:"name"`
 	Description types.String `tfsdk:"description"`

internal/project/resource_project.go

@@ -295,7 +295,7 @@ func (r ProjectResource) SyncState(ctx context.Context, tfState *tfsdk.State, se
 }
 
 // ComputedKeys returns list of computed config keys.
-func (_ ProjectModel) ComputedKeys() []string {
+func (m ProjectModel) ComputedKeys() []string {
 	return []string{
 		"features.images",
 		"features.profiles",

internal/provider-config/config.go

@@ -224,7 +224,11 @@ func (p *LxdProviderConfig) InstanceServer(remoteName string, project string, ta
 		return nil, fmt.Errorf("Remote %q (%s) is not an InstanceServer", remoteName, connInfo.Protocol)
 	}
 
-	instServer := server.(lxd.InstanceServer)
+	instServer, ok := server.(lxd.InstanceServer)
+	if !ok {
+		return nil, fmt.Errorf("Remote %q is not an InstanceServer", remoteName)
+	}
+
 	instServer = instServer.UseProject(project)
 	instServer = instServer.UseTarget(target)
 
@@ -248,7 +252,12 @@ func (p *LxdProviderConfig) ImageServer(remoteName string) (lxd.ImageServer, err
 		return nil, fmt.Errorf("Remote %q (%s / %s) is not an ImageServer", remoteName, connInfo.Protocol, connInfo.Addresses[0])
 	}
 
-	return server.(lxd.ImageServer), nil
+	imageServer, ok := server.(lxd.ImageServer)
+	if !ok {
+		return nil, fmt.Errorf("Remote %q is not an ImageServer", remoteName)
+	}
+
+	return imageServer, nil
 }
 
 // server returns a server for the named remote. The returned server