Modernize linter

.golangci.yaml

@@ -0,0 +1,56 @@
+run:
+  skip-files:
+    - "/zz_generated_"
+  deadline: 5m
+linters:
+  disable-all: true
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - containedctx
+    - dogsled
+    - dupword
+    - durationcheck
+    - errcheck
+    - errchkjson
+    - exportloopref
+    - gci
+    - ginkgolinter
+    - goconst
+    - gocritic
+    - godot
+    - gofmt
+    - goimports
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - importas
+    - ineffassign
+    - misspell
+    - nakedret
+    - nilerr
+    - noctx
+    - nolintlint
+    - nosprintfhostport
+    - prealloc
+    - predeclared
+    - revive
+    - rowserrcheck
+    - staticcheck
+    - stylecheck
+    - thelper
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - whitespace
+linters-settings:
+  gosec:
+    excludes:
+      - G101 # Potential hardcoded credentials
+      - G110 # Potential DoS vulnerability via decompression bomb
+      - G402 # TLS MinVersion too low.

Dockerfile.dapper

@@ -1,14 +1,14 @@
-FROM golang:1.19.13-alpine3.18
+FROM registry.suse.com/bci/golang:1.20
 
 ARG DAPPER_HOST_ARCH
 ENV ARCH $DAPPER_HOST_ARCH
 
-RUN apk -U add bash git gcc musl-dev docker vim less file curl wget ca-certificates
-RUN go install golang.org/x/lint/golint@latest && \
-    go install golang.org/x/tools/cmd/goimports@latest && \
-    rm -rf /go/src /go/pkg
+ENV GOLANGCI_LINT_VERSION v1.53.3
+
+RUN zypper -n install docker
+
 RUN if [ "${ARCH}" == "amd64" ]; then \
-        curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.50.1; \
+        curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s ${GOLANGCI_LINT_VERSION}; \
     fi
 
 ENV DAPPER_ENV REPO TAG DRONE_TAG CROSS

go.mod

@@ -1,6 +1,6 @@
 module github.com/rancher/system-agent
 
-go 1.19
+go 1.20
 
 replace (
 	k8s.io/api => k8s.io/api v0.24.2

main.go

@@ -56,7 +56,7 @@ func main() {
 	}
 }
 
-func run(c *cli.Context) error {
+func run(_ *cli.Context) error {
 	topContext := signals.SetupSignalContext()
 
 	logrus.Infof("Rancher System Agent version %s is starting", version.FriendlyVersion())

pkg/applyinator/applyinator.go

@@ -9,7 +9,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -33,7 +32,7 @@ type Applyinator struct {
 	imageUtil       *image.Utility
 }
 
-// CalculatedPlan is passed into Applyinator and is a Plan with checksum calculated
+// CalculatedPlan is passed into Applyinator and is a Plan with checksum calculated.
 type CalculatedPlan struct {
 	Plan     Plan
 	Checksum string
@@ -75,8 +74,9 @@ type OneTimeInstruction struct {
 	SaveOutput bool `json:"saveOutput,omitempty"`
 }
 
+// File represents a file as it would appear on disk
 // Path would be `/etc/kubernetes/ssl/ca.pem`, Content is base64 encoded.
-// If Directory is true, then we are creating a directory, not a file
+// If Directory is true, then we are creating a directory, not a file.
 type File struct {
 	Content     string `json:"content,omitempty"`
 	Directory   bool   `json:"directory,omitempty"`
@@ -140,7 +140,7 @@ type ApplyInput struct {
 }
 
 // Apply accepts a context, calculated plan, a bool to indicate whether to run the onetime instructions, the existing onetimeinstruction output, and an input byte slice which is a base64+gzip json-marshalled map of PeriodicInstructionOutput
-// entries where the key is the PeriodicInstructionOutput.Name. It outputs a revised versions of the existing outputs, and if specified, runs the one time instructions. Notably, ApplyOutput.OneTimeApplySucceeded will be false if ApplyInput.RunOneTimeInstructions is false
+// entries where the key is the PeriodicInstructionOutput.Name. It outputs a revised versions of the existing outputs, and if specified, runs the one time instructions. Notably, ApplyOutput.OneTimeApplySucceeded will be false if ApplyInput.RunOneTimeInstructions is false.
 func (a *Applyinator) Apply(ctx context.Context, input ApplyInput) (ApplyOutput, error) {
 	logrus.Debugf("[Applyinator] Applying plan with checksum %s", input.CalculatedPlan.Checksum)
 	logrus.Tracef("[Applyinator] Applying plan - attempting to get lock")
@@ -353,7 +353,9 @@ func gzipByteSlice(input []byte) ([]byte, error) {
 
 	gzWriter := gzip.NewWriter(&gzOutput)
 
-	gzWriter.Write(input)
+	if _, err := gzWriter.Write(input); err != nil {
+		return []byte{}, err
+	}
 	if err := gzWriter.Close(); err != nil {
 		return []byte{}, err
 	}
@@ -431,7 +433,7 @@ func (a *Applyinator) writePlanToDisk(now time.Time, plan *CalculatedPlan) error
 		sort.Slice(planFiles, func(i, j int) bool {
 			return planFiles[i].Name() > planFiles[j].Name()
 		})
-		existingFileContent, err := ioutil.ReadFile(filepath.Join(a.appliedPlanDir, planFiles[0].Name()))
+		existingFileContent, err := os.ReadFile(filepath.Join(a.appliedPlanDir, planFiles[0].Name()))
 		if err != nil {
 			return err
 		}

pkg/applyinator/file.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/base64"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -39,15 +38,15 @@ func writeContentToFile(path string, uid int, gid int, perm os.FileMode, content
 		return fmt.Errorf("path was empty")
 	}
 
-	existing, err := ioutil.ReadFile(path)
+	existing, err := os.ReadFile(path)
 	if err == nil && bytes.Equal(existing, content) {
 		logrus.Debugf("[Applyinator] File %s does not need to be written", path)
 	} else {
 		dir := filepath.Dir(path)
 		if err := os.MkdirAll(dir, defaultDirectoryPermissions); err != nil {
 			return err
 		}
-		if err := ioutil.WriteFile(path, content, perm); err != nil {
+		if err := os.WriteFile(path, content, perm); err != nil {
 			return err
 		}
 	}

pkg/config/config.go

@@ -3,7 +3,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -60,7 +60,7 @@ func Parse(path string, result interface{}) error {
 	case strings.Contains(file, ".json"):
 		return json.NewDecoder(f).Decode(result)
 	case strings.Contains(file, ".yaml"):
-		b, err := ioutil.ReadAll(f)
+		b, err := io.ReadAll(f)
 		if err != nil {
 			return err
 		}

pkg/image/util.go

@@ -101,13 +101,11 @@ func (u *Utility) Stage(destDir string, imgString string) error {
 				return err
 			}
 			registry.DefaultKeychain = plugins
-		} else {
+		} else if os.Getenv("HOME") != "" {
 			// The kubelet image credential provider plugin also falls back to checking legacy Docker credentials, so only
 			// explicitly set up the go-containerregistry DefaultKeychain if plugins are not configured.
 			// DefaultKeychain tries to read config from the home dir, and will error if HOME isn't set, so also gate on that.
-			if os.Getenv("HOME") != "" {
-				registry.DefaultKeychain = authn.DefaultKeychain
-			}
+			registry.DefaultKeychain = authn.DefaultKeychain
 		}
 
 		logrus.Infof("Pulling image %s", image.Name())

pkg/k8splan/watcher.go

@@ -244,11 +244,9 @@ func (w *watcher) start(ctx context.Context) {
 							if failureCount >= maxFailureThreshold && maxFailureThreshold != -1 {
 								logrus.Errorf("[K8s] Maximum failure threshold exceeded for plan with checksum value of %s, (failures: %d, threshold: %d)", cp.Checksum, failureCount, maxFailureThreshold)
 								needsApplied = false
-							} else {
-								if !currentTime.Equal(lastApplyTime) && !currentTime.After(lastApplyTime.Add(cooldownPeriod)) {
-									logrus.Debugf("[K8s] %f second cooldown timer for failed plan application has not passed yet.", cooldownPeriod.Seconds())
-									needsApplied = false
-								}
+							} else if !currentTime.Equal(lastApplyTime) && !currentTime.After(lastApplyTime.Add(cooldownPeriod)) {
+								logrus.Debugf("[K8s] %f second cooldown timer for failed plan application has not passed yet.", cooldownPeriod.Seconds())
+								needsApplied = false
 							}
 						} else {
 							logrus.Errorf("[K8s] Received plan checksum (%s) did not match failed plan checksum (%s) and failure count was greater than zero. Cancelling failure cooldown.", cp.Checksum, string(rFC))

pkg/localplan/watcher.go

@@ -4,7 +4,8 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"io/ioutil"
+	"errors"
+	"io"
 	"os"
 	"path/filepath"
 	"sort"
@@ -26,7 +27,7 @@ func WatchFiles(ctx context.Context, applyinator applyinator.Applyinator, bases
 	go w.start(ctx)
 }
 
-// stdout and stderr are both base64, gzipped
+// stdout and stderr are both base64, gzipped.
 type NodePlanPosition struct {
 	AppliedChecksum string                        `json:"appliedChecksum,omitempty"`
 	Output          []byte                        `json:"output,omitempty"`
@@ -45,11 +46,8 @@ const (
 )
 
 func (w *watcher) start(ctx context.Context) {
-	force := true
 	for {
-		if err := w.listFiles(ctx, force); err == nil {
-			force = false
-		} else {
+		if err := w.listFiles(ctx); err != nil {
 			logrus.Errorf("Failed to process config: %v", err)
 		}
 		select {
@@ -60,17 +58,17 @@ func (w *watcher) start(ctx context.Context) {
 	}
 }
 
-func (w *watcher) listFiles(ctx context.Context, force bool) error {
+func (w *watcher) listFiles(ctx context.Context) error {
 	var errs []error
 	for _, base := range w.bases {
-		if err := w.listFilesIn(ctx, base, force); err != nil {
+		if err := w.listFilesIn(ctx, base); err != nil {
 			errs = append(errs, err)
 		}
 	}
-	return nil
+	return errors.Join(errs...)
 }
 
-func (w *watcher) listFilesIn(ctx context.Context, base string, force bool) error {
+func (w *watcher) listFilesIn(ctx context.Context, base string) error {
 	files := map[string]os.FileInfo{}
 	if err := filepath.Walk(base, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
@@ -120,13 +118,7 @@ func (w *watcher) listFilesIn(ctx context.Context, base string, force bool) erro
 			logrus.Errorf("error parsing position data: %v", err)
 		}
 
-		needsApplied, probeStatuses, err := w.needsApplication(planPosition, cp)
-
-		if err != nil {
-			logrus.Errorf("[local] Error while determining if node plan needed application: %v", err)
-			continue
-		}
-
+		needsApplied, probeStatuses := w.needsApplication(planPosition, cp)
 		if probeStatuses == nil {
 			probeStatuses = make(map[string]prober.ProbeStatus)
 		}
@@ -184,7 +176,7 @@ func (w *watcher) listFilesIn(ctx context.Context, base string, force bool) erro
 			logrus.Errorf("error marshalling new plan position data: %v", err)
 		}
 
-		if bytes.Compare(newPPData, posData) != 0 {
+		if !bytes.Equal(newPPData, posData) {
 			logrus.Debugf("[local] Writing position data")
 			if err := os.WriteFile(posFile, newPPData, 0600); err != nil {
 				logrus.Errorf("[local] Error encountered when writing position file for %s: %v", path, err)
@@ -202,7 +194,7 @@ func (w *watcher) parsePlan(file string) (applyinator.CalculatedPlan, error) {
 	}
 	defer f.Close()
 
-	b, err := ioutil.ReadAll(f)
+	b, err := io.ReadAll(f)
 	if err != nil {
 		return applyinator.CalculatedPlan{}, err
 	}
@@ -245,18 +237,17 @@ func parsePositionData(positionData []byte) (NodePlanPosition, error) {
 }
 
 // Returns true if the plan needs to be applied, false if not
-// needsApplication, probeStatus, error
-func (w *watcher) needsApplication(planPosition NodePlanPosition, cp applyinator.CalculatedPlan) (bool, map[string]prober.ProbeStatus, error) {
+// needsApplication, probeStatus.
+func (w *watcher) needsApplication(planPosition NodePlanPosition, cp applyinator.CalculatedPlan) (bool, map[string]prober.ProbeStatus) {
 	computedChecksum := cp.Checksum
 	if planPosition.AppliedChecksum == computedChecksum {
 		logrus.Debugf("[local] Plan checksum (%s) matched", computedChecksum)
-		return false, planPosition.ProbeStatus, nil
+		return false, planPosition.ProbeStatus
 	}
 	logrus.Infof("[local] Plan checksums differed (%s:%s)", computedChecksum, planPosition.AppliedChecksum)
 
 	// Default to needing application.
-	return true, planPosition.ProbeStatus, nil
-
+	return true, planPosition.ProbeStatus
 }
 
 func skipFile(fileName string, skips map[string]bool) bool {