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.

Sign up for GitHub

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

GO code staticcheck cleanup #1038

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

GO code staticcheck cleanup #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ else
COWSQL_PATH=$(GOPATH)/deps/cowsql
endif

# raft
.PHONY: default
default: build

Expand Down Expand Up @@ -56,6 +55,7 @@ incus-migrate:

.PHONY: deps
deps:
# raft
@if [ ! -e "$(RAFT_PATH)" ]; then \
git clone --depth=1 "https://github.com/cowsql/raft" "$(RAFT_PATH)"; \
elif [ -e "$(RAFT_PATH)/.git" ]; then \
Expand Down
5 changes: 3 additions & 2 deletions cmd/incus/admin_recover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"golang.org/x/text/cases"
"golang.org/x/text/language"

"github.com/lxc/incus/v6/client"
incus "github.com/lxc/incus/v6/client"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you ran goimports or something like that which has a tendency to go mess with our import names, can you remove all of those from your commit?

cli "github.com/lxc/incus/v6/internal/cmd"
"github.com/lxc/incus/v6/internal/i18n"
"github.com/lxc/incus/v6/internal/recover"
Expand Down Expand Up @@ -241,7 +241,8 @@ func (c *cmdAdminRecover) Run(cmd *cobra.Command, args []string) error {
// Send /internal/recover/import request to the daemon.
// Don't lint next line with gosimple. It says we should convert reqValidate directly to an RecoverImportPost
// because their types are identical. This is less clear and will not work if either type changes in the future.
reqImport := recover.ImportPost{ //nolint:gosimple
//nolint:all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why turn off all linting?

reqImport := recover.ImportPost{ //lint:ignore S1016 for reason above
Pools: reqValidate.Pools,
}

Expand Down
4 changes: 0 additions & 4 deletions cmd/incus/completion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ func (g *cmdGlobal) cmpNetworkForwardConfigs(networkName string, listenAddress s
}

func (g *cmdGlobal) cmpNetworkForwards(networkName string) ([]string, cobra.ShellCompDirective) {
results := []string{}
cmpDirectives := cobra.ShellCompDirectiveNoFileComp

resources, _ := g.ParseServers(networkName)
Expand All @@ -516,7 +515,6 @@ func (g *cmdGlobal) cmpNetworkForwards(networkName string) ([]string, cobra.Shel
}

func (g *cmdGlobal) cmpNetworkLoadBalancers(networkName string) ([]string, cobra.ShellCompDirective) {
results := []string{}
cmpDirectives := cobra.ShellCompDirectiveNoFileComp

resources, _ := g.ParseServers(networkName)
Expand Down Expand Up @@ -560,7 +558,6 @@ func (g *cmdGlobal) cmpNetworkPeerConfigs(networkName string, peerName string) (
}

func (g *cmdGlobal) cmpNetworkPeers(networkName string) ([]string, cobra.ShellCompDirective) {
results := []string{}
cmpDirectives := cobra.ShellCompDirectiveNoFileComp

resources, _ := g.ParseServers(networkName)
Expand Down Expand Up @@ -742,7 +739,6 @@ func (g *cmdGlobal) cmpNetworkZoneRecordConfigs(zoneName string, recordName stri
}

func (g *cmdGlobal) cmpNetworkZoneRecords(zoneName string) ([]string, cobra.ShellCompDirective) {
results := []string{}
cmpDirectives := cobra.ShellCompDirectiveNoFileComp

resources, _ := g.ParseServers(zoneName)
Expand Down
4 changes: 1 addition & 3 deletions cmd/incus/snapshot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/spf13/cobra"
"gopkg.in/yaml.v2"

"github.com/lxc/incus/v6/client"
incus "github.com/lxc/incus/v6/client"
cli "github.com/lxc/incus/v6/internal/cmd"
"github.com/lxc/incus/v6/internal/i18n"
"github.com/lxc/incus/v6/internal/instance"
Expand Down Expand Up @@ -514,8 +514,6 @@ func (c *cmdSnapshotRestore) Run(cmd *cobra.Command, args []string) error {
type cmdSnapshotShow struct {
global *cmdGlobal
snapshot *cmdSnapshot

flagExpanded bool
}

func (c *cmdSnapshotShow) Command() *cobra.Command {
Expand Down
6 changes: 3 additions & 3 deletions cmd/incusd/dev_incus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func hoistReq(f func(*Daemon, instance.Instance, http.ResponseWriter, *http.Requ
conn := ucred.GetConnFromContext(r.Context())
cred, ok := pidMapper.m[conn.(*net.UnixConn)]
if !ok {
http.Error(w, pidNotInContainerErr.Error(), http.StatusInternalServerError)
http.Error(w, errPidNotInContainer.Error(), http.StatusInternalServerError)
return
}

Expand Down Expand Up @@ -390,7 +390,7 @@ func (m *ConnPidMapper) ConnStateHandler(conn net.Conn, state http.ConnState) {
}
}

var pidNotInContainerErr = fmt.Errorf("pid not in container?")
var errPidNotInContainer = fmt.Errorf("pid not in container?")

func findContainerForPid(pid int32, s *state.State) (instance.Container, error) {
/*
Expand Down Expand Up @@ -496,5 +496,5 @@ func findContainerForPid(pid int32, s *state.State) (instance.Container, error)
}
}

return nil, pidNotInContainerErr
return nil, errPidNotInContainer
}
2 changes: 1 addition & 1 deletion cmd/incusd/dev_incus_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func TestHttpRequest(t *testing.T) {
t.Fatal(err)
}

if !strings.Contains(string(resp), pidNotInContainerErr.Error()) {
if !strings.Contains(string(resp), errPidNotInContainer.Error()) {
t.Fatal("resp error not expected: ", string(resp))
}
}
4 changes: 3 additions & 1 deletion cmd/incusd/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,9 @@ func instanceCreateAsCopy(s *state.State, opts instanceCreateAsCopyOpts, op *ope
"path": "/",
"pool": instRootDiskDevice["pool"],
}
} else { //nolint:staticcheck // (keep the empty branch for the comment)
//nolint:all
//lint:ignore SA9003 keep the empty branch for the comment
} else { //nolint:all
Comment on lines -430 to +432
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what's going on here, that change looks off.

// Snapshot has multiple root disk devices, we can't automatically fix this so
// leave alone so we don't prevent copy.
}
Expand Down
8 changes: 5 additions & 3 deletions internal/server/cluster/membership.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import (
"github.com/lxc/incus/v6/shared/util"
)

// clusterBusyError is returned by dqlite if attempting attempting to join a cluster at the same time as a role-change.
// errClusterBusy is returned by dqlite if attempting attempting to join a cluster at the same time as a role-change.
// This error tells us we can retry and probably join the cluster or fail due to something else.
// The error code here is SQLITE_BUSY.
var clusterBusyError = fmt.Errorf("A configuration change is already in progress (5)")
var errClusterBusy = fmt.Errorf("A configuration change is already in progress (5)")

// Bootstrap turns a non-clustered server into the first (and leader)
// member of a new cluster.
Expand Down Expand Up @@ -447,7 +447,7 @@ func Join(state *state.State, gateway *Gateway, networkCert *localtls.CertInfo,
return fmt.Errorf("Failed to join cluster: %w", ctx.Err())
default:
err = client.Add(ctx, info.NodeInfo)
if err != nil && err.Error() == clusterBusyError.Error() {
if err != nil && err.Error() == errClusterBusy.Error() {
// If the cluster is busy with a role change, sleep a second and then keep trying to join.
time.Sleep(1 * time.Second)
continue
Expand Down Expand Up @@ -593,6 +593,8 @@ func NotifyHeartbeat(state *state.State, gateway *Gateway) {
// Wait for heartbeat to finish and then release.
// Ignore staticcheck "SA2001: empty critical section" because we want to wait for the lock.
gateway.HeartbeatLock.Lock()
//nolint:all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why turn off all linting?

//lint:ignore SA2001 we want to wait for the lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above (// Ignore staticcheck ...) can be removed given this one just repeats it

gateway.HeartbeatLock.Unlock() //nolint:staticcheck
}

Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/certificate_projects.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/certificates.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/cluster_groups.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/config.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/devices.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/images.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/instance_profiles.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/instances.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/networks_integrations.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/nodes.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/nodes_cluster_groups.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/operations.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/profiles.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/projects.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/snapshots.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/cluster/warnings.mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cluster

// The code below was generated by incus-generate - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck

import (
"context"
Expand Down
1 change: 1 addition & 0 deletions internal/server/db/generate/file/write.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func Reset(path string, imports []string, buildComment string, iface bool) error
content := fmt.Sprintf(`%spackage %s

// The code below was generated by %s - DO NOT EDIT!
//lint:file-ignore U1000,SA4006 Ignore staticcheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just because the files are auto-generated doesn't mean that issues/lints aren't worth fixing.

It just means that rather than fixing the issues directly in the .mapper.go files, those issues need to instead be fixed in the generator code.


import (
`, buildComment, os.Getenv("GOPACKAGE"), os.Args[0])
Expand Down
16 changes: 0 additions & 16 deletions internal/server/network/ovs/utils.go
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion internal/server/response/swagger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Package response contains helpers for rendering HTTP responses.
//
//nolint:deadcode,unused
//nolint:all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ignoring all lints?

//lint:file-ignore U1000 Ignore unused

package response

import (
Expand Down
3 changes: 2 additions & 1 deletion internal/server/scriptlet/load/load.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sync"

"go.starlark.net/starlark"
"go.starlark.net/syntax"
)

// nameInstancePlacement is the name used in Starlark for the instance placement scriptlet.
Expand All @@ -30,7 +31,7 @@ func InstancePlacementCompile(src string) (*starlark.Program, error) {
}

// Parse, resolve, and compile a Starlark source file.
_, mod, err := starlark.SourceProgram(nameInstancePlacement, src, isPreDeclared)
_, mod, err := starlark.SourceProgramOptions(syntax.LegacyFileOptions(), nameInstancePlacement, src, isPreDeclared)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/server/util/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func CheckTrustState(cert x509.Certificate, trustedCerts map[string]x509.Certifi
return false, "" // CRL not signed by CA
}

for _, revoked := range crl.RevokedCertificates {
for _, revoked := range crl.RevokedCertificateEntries {
if cert.SerialNumber.Cmp(revoked.SerialNumber) == 0 {
return false, "" // Certificate is revoked, so not trusted anymore.
}
Expand Down
6 changes: 5 additions & 1 deletion shared/cliconfig/remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/zitadel/oidc/v3/pkg/oidc"
"golang.org/x/crypto/ssh"

"github.com/lxc/incus/v6/client"
incus "github.com/lxc/incus/v6/client"
"github.com/lxc/incus/v6/shared/api"
"github.com/lxc/incus/v6/shared/util"
)
Expand Down Expand Up @@ -332,6 +332,8 @@ func (c *Config) getConnectionArgs(name string) (*incus.ConnectionArgs, error) {
// Golang has deprecated all methods relating to PEM encryption due to a vulnerability.
// However, the weakness does not make PEM unsafe for our purposes as it pertains to password protection on the
// key file (client.key is only readable to the user in any case), so we'll ignore deprecation.
//nolint:all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why turn off all linting?

//lint:ignore SA1019 see above for reason
isEncrypted := x509.IsEncryptedPEMBlock(pemKey) //nolint:staticcheck
isSSH := pemKey.Type == "OPENSSH PRIVATE KEY"
if isEncrypted || isSSH {
Expand Down Expand Up @@ -366,6 +368,8 @@ func (c *Config) getConnectionArgs(name string) (*incus.ConnectionArgs, error) {
return nil, fmt.Errorf("Unsupported key type: %T", sshKey)
}
} else {
//nolint:all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why turn off all linting?

//lint:ignore SA1019 see above for reason
derKey, err := x509.DecryptPEMBlock(pemKey, []byte(password)) //nolint:staticcheck
if err != nil {
return nil, err
Expand Down
4 changes: 1 addition & 3 deletions test/mini-oidc/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func main() {
},
}

provider, err := op.NewOpenIDProvider(issuer, config, storage, op.WithAllowInsecure())
provider, err := op.NewProvider(config, storage, op.StaticIssuer(issuer), op.WithAllowInsecure())
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
Expand Down Expand Up @@ -93,8 +93,6 @@ func userCodeHandler(storage *storage.Storage, w http.ResponseWriter, r *http.Re
}

fmt.Printf("%s => %s\n", userCode, name)

return
}

func username() string {
Expand Down
Loading
Loading