[QUESTION/HELP] Benign "Bug" in pkg/client/tools.go and Linter Configuration Interest
Question / Where do you need Help?
Hello! Long time user, first time source code explorer here.
While spending some time learning about the codebase through the lens of investigating https://github.com/k3d-io/k3d/issues/1538, I found a benign "bug" here https://github.com/k3d-io/k3d/blob/115c8ac31715e99b2a9bca9278277ac6bfbdc8e2/pkg/client/tools.go#L65-L67 with an impossible error condition. I then used the Makefile to install the tools and run make lint. I believe issue was not caught due to two reasons. The first reason is that it seems like the linter is not running on anything in pkg/ and the second reason is that nilness is not enabled for govet.
If you all are open to modifying linter settings, I would like to assist with this effort to cover more code, tune the linter, and fix whatever issues pop up.
Here is some output with minimal changes. It certainly would need more tuning and I acknowledge that much of this linting can be considered the opinion of the linter author. Some of the deprecations are also of interest.
golangci-lint run
pkg/client/host.go:138:13: nilness: tautological condition: non-nil != nil (govet)
if scanner != nil && execErr != nil {
^
pkg/client/tools.go:65:10: nilness: impossible condition: nil != nil (govet)
if err != nil {
^
pkg/config/config_test.go:290:13: unusedwrite: unused write to field HostIP (govet)
exposedAPI.HostIP = "0.0.0.0"
^
pkg/config/config_test.go:291:13: unusedwrite: unused write to field HostPort (govet)
exposedAPI.HostPort = "6443"
^
pkg/client/cluster.go:185:17: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
clusterConfig.Cluster.Nodes = append(clusterConfig.Cluster.Nodes, registryNode)
^
pkg/client/cluster.go:202:84: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
if err := RegistryConnectNetworks(ctx, runtime, regNode, []string{clusterConfig.Cluster.Network.Name}); err != nil {
^
pkg/client/cluster.go:457:3: QF1003: could use tagged switch on node.Role (staticcheck)
if node.Role == k3d.ServerRole {
^
pkg/client/cluster.go:574:30: QF1008: could remove embedded field "Node" from selector (staticcheck)
cluster.ServerLoadBalancer.Node.FillRuntimeLabels()
^
pkg/client/cluster.go:576:31: QF1008: could remove embedded field "Node" from selector (staticcheck)
cluster.ServerLoadBalancer.Node.RuntimeLabels[k] = v
^
pkg/client/cluster.go:596:30: QF1008: could remove embedded field "Node" from selector (staticcheck)
cluster.ServerLoadBalancer.Node.HookActions = append(cluster.ServerLoadBalancer.Node.HookActions, writeLbConfigAction)
^
pkg/client/node.go:538:13: ST1005: error strings should not end with punctuation or newlines (staticcheck)
return fmt.Errorf("[Node %s] Cannot activate DNS fix (K3D_FIX_DNS) when there's a file mounted at /etc/resolv.conf!", node.Name)
^
pkg/client/node.go:543:12: ST1005: error strings should not end with punctuation or newlines (staticcheck)
return fmt.Errorf("Cannot enable DNS fix, as Host Gateway IP is missing!")
^
pkg/client/node.go:623:2: QF1003: could use tagged switch on node.Role (staticcheck)
if node.Role == k3d.AgentRole { // TODO: check here AND in CLI or only here?
^
pkg/client/registry_test.go:64:5: QF1001: could apply De Morgan's law (staticcheck)
if !(strings.TrimSpace(string(cm)) == strings.TrimSpace(expectedYAMLString)) {
^
pkg/config/process.go:64:19: QF1008: could remove embedded field "PortMapping" from selector (staticcheck)
cluster.KubeAPI.PortMapping.Binding.HostPort = k3sPort
^
pkg/config/process.go:71:37: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
for _, node := range clusterConfig.Cluster.Nodes {
^
pkg/config/transform.go:121:6: S1009: should omit nil check; len() for nil slices is defined as zero (staticcheck)
if simpleConfig.Options.K3dOptions.Loadbalancer.ConfigOverrides != nil && len(simpleConfig.Options.K3dOptions.Loadbalancer.ConfigOverrides) > 0 {
^
pkg/runtimes/docker/container.go:56:7: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
if client.IsErrNotFound(err) {
^
pkg/runtimes/docker/container.go:118:13: ST1023: should omit type io.Writer from declaration; it will be inferred from the right-hand side (staticcheck)
var writer io.Writer = io.Discard
^
pkg/runtimes/docker/container.go:130:62: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func getNodeContainer(ctx context.Context, node *k3d.Node) (*types.Container, error) {
^
pkg/runtimes/docker/container.go:155:15: ST1005: error strings should not be capitalized (staticcheck)
return nil, fmt.Errorf("Failed to list containers: %+v", err)
^
pkg/runtimes/docker/container.go:159:15: ST1005: error strings should not be capitalized (staticcheck)
return nil, fmt.Errorf("Failed to get a single container for name '%s'. Found: %d", node.Name, len(containers))
^
pkg/runtimes/docker/container.go:163:15: ST1005: error strings should not be capitalized (staticcheck)
return nil, fmt.Errorf("Didn't find container for node '%s'", node.Name)
^
pkg/runtimes/docker/container.go:188:7: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
if client.IsErrNotFound(err) {
^
pkg/runtimes/docker/node.go:167:77: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func getContainersByLabel(ctx context.Context, labels map[string]string) ([]types.Container, error) {
^
pkg/runtimes/docker/node.go:196:68: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
func getContainerDetails(ctx context.Context, containerID string) (types.ContainerJSON, error) {
^
pkg/runtimes/docker/node.go:200:10: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
return types.ContainerJSON{}, fmt.Errorf("failed to create docker client. %w", err)
^
pkg/runtimes/docker/node.go:206:10: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
return types.ContainerJSON{}, fmt.Errorf("failed to get details for container '%s': %w", containerID, err)
^
pkg/runtimes/docker/node.go:255:37: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
running = containerInspectResponse.ContainerJSONBase.State.Running
^
pkg/runtimes/docker/node.go:256:41: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
stateString = containerInspectResponse.ContainerJSONBase.State.Status
^
pkg/runtimes/docker/node.go:290:31: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
if !containerInspectResponse.ContainerJSONBase.State.Running {
^
pkg/runtimes/docker/node.go:380:2: QF1007: could merge conditional assignment into variable declaration (staticcheck)
attachStdin := false
^
pkg/runtimes/docker/translate.go:185:37: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func TranslateContainerToNode(cont *types.Container) (*k3d.Node, error) {
^
pkg/runtimes/docker/translate.go:215:2: QF1007: could merge conditional assignment into variable declaration (staticcheck)
restart := false
^
pkg/runtimes/docker/translate.go:258:3: QF1003: could use tagged switch on k (staticcheck)
if k == k3d.LabelServerAPIHostIP {
^
pkg/runtimes/docker/util.go:89:18: SA1019: archive.CopyInfoSourcePath is deprecated: use [archive.CopyInfoSourcePath] instead. (staticcheck)
srcInfo, err := archive.CopyInfoSourcePath(src, false)
^
pkg/runtimes/docker/util.go:94:21: SA1019: archive.TarResource is deprecated: use [archive.TarResource] instead. (staticcheck)
srcArchive, err := archive.TarResource(srcInfo)
^
pkg/runtimes/docker/util.go:100:14: SA1019: archive.CopyInfo is deprecated: use [archive.CopyInfo] instead. (staticcheck)
destInfo := archive.CopyInfo{Path: dest}
^
pkg/runtimes/docker/util.go:106:35: SA1019: archive.PrepareArchiveCopy is deprecated: use [archive.PrepareArchiveCopy] instead. (staticcheck)
destDir, preparedArchive, err := archive.PrepareArchiveCopy(srcArchive, srcInfo, destInfo)
^
pkg/runtimes/docker/util.go:174:6: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
if client.IsErrNotFound(err) {
^
pkg/util/filter.go:118:3: QF1003: could use tagged switch on node.Role (staticcheck)
if node.Role == k3d.ServerRole {
^
41 issues:
* govet: 4
* staticcheck: 37
Scope of your Question
- Is your question related to a specific version of k3d (or k3s)?
This question is related to the main branch and general code for this project standards.