Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve golangci-lint coverage #549

Merged
merged 31 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
8e447ff
Makefile: drop unneeded Go version in `go mod tidy`
simondeziel Dec 10, 2024
9852d08
Align golangci-lint config with that of LXD
simondeziel Dec 10, 2024
9a7ad50
golangci: disable exported and var-naming revive rules for now
simondeziel Dec 10, 2024
29e4e06
internal/instance/resource_instance_snapshot: simplify if/else (revive)
simondeziel Dec 10, 2024
da816d1
internal/common/lxd_interface: fix comment of exported structs
simondeziel Dec 10, 2024
64b705f
internal/utils: fix comment for DiffSlices
simondeziel Dec 10, 2024
fa4aa91
internal/common/lxd_exec: add comments for exported type/struct
simondeziel Dec 10, 2024
23782bb
internal/common/lxd_import: `s/importId/importID/g`
simondeziel Dec 10, 2024
ea3a117
internal/network/resource_network_forward: add comment to exported st…
simondeziel Dec 10, 2024
606fae6
internal/profile/resource_profile: add comment to exported struct
simondeziel Dec 10, 2024
b2bbe76
internal/storage/resource_storage_bucket_key: add comment to exported…
simondeziel Dec 10, 2024
b62e173
internal/storage/resource_storage_pool: add comment to exported struct
simondeziel Dec 10, 2024
c7a8176
internal/storage/resource_storage_volume_copy: add comment to exporte…
simondeziel Dec 10, 2024
52845a2
internal/utils/buffer: add comments to exported funcs
simondeziel Dec 10, 2024
d56e40d
internal/acctest/checks: ignore conn.Close() error
simondeziel Dec 10, 2024
4eb2269
internal/common/lxd_import: `s/importId/importID/g`
simondeziel Dec 10, 2024
8c66fd7
internal/provider/provider: remove dup'ed import
simondeziel Dec 10, 2024
71f1304
internal/instance/resource_instance_file: use named returns in splitF…
simondeziel Dec 10, 2024
6722c57
internal/instance/resource_instance_file: don't access slice out of b…
simondeziel Dec 10, 2024
4f16d02
internal/instance/resource_instance_file: no need for fmt.Sprintf() i…
simondeziel Dec 10, 2024
44b8869
internal/instance/resource_instance: avoid redefining `new`
simondeziel Dec 10, 2024
0933074
internal/image/resource_publish_image: check if type cast was successful
simondeziel Dec 10, 2024
ee3e210
internal/provider-config/config: check if type casts were successful
simondeziel Dec 10, 2024
5cb1658
internal/provider/provider_test: ignore errors from os.Setenv/Unsetenv
simondeziel Dec 10, 2024
e9a1bd2
internal/image/resource_publish_image: name receiver
simondeziel Dec 10, 2024
d6fd8fb
internal/instance/resource_instance: name receiver
simondeziel Dec 10, 2024
e4740b9
internal/network/resource_network: name receiver
simondeziel Dec 10, 2024
6811934
internal/project/resource_project: name receiver
simondeziel Dec 10, 2024
b6f5b7b
internal/storage/resource_storage_bucket: name receiver
simondeziel Dec 10, 2024
194e210
internal/storage/resource_storage_pool: name receiver
simondeziel Dec 10, 2024
e121dad
internal/storage/resource_storage_volume: name receivers
simondeziel Dec 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 95 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion internal/acctest/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions internal/common/lxd_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/terraform-lxd/terraform-provider-lxd/internal/utils"
)

// ExecTriggerType represents an exec trigger.
type ExecTriggerType string

const (
Expand All @@ -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"`
Expand Down
16 changes: 8 additions & 8 deletions internal/common/lxd_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions internal/common/lxd_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"`
Expand Down
9 changes: 7 additions & 2 deletions internal/image/resource_publish_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
14 changes: 7 additions & 7 deletions internal/instance/resource_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.",
Expand Down
8 changes: 6 additions & 2 deletions internal/instance/resource_instance_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
20 changes: 10 additions & 10 deletions internal/instance/resource_instance_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/network/resource_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion internal/network/resource_network_forward.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
1 change: 1 addition & 0 deletions internal/profile/resource_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion internal/project/resource_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 11 additions & 2 deletions internal/provider-config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
Loading