Enable revive's defer rule in golangci-lint (#2070)

kolide · Jan 28, 2025 · 988d072 · 988d072
1 parent 293162a
commit 988d072
Show file tree

Hide file tree

Showing 13 changed files with 265 additions and 188 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -71,6 +71,8 @@ linters-settings:
         disabled: false
       - name: confusing-naming
         disabled: false
+      - name: defer
+        disabled: false
   staticcheck:
     checks: ["all"]
 

diff --git a/cmd/package-builder/package-builder.go b/cmd/package-builder/package-builder.go
@@ -290,22 +290,30 @@ func runMake(args []string) error {
 	}
 
 	for _, target := range targets {
-		outputFileName := fmt.Sprintf("launcher.%s.%s", target.String(), target.PkgExtension())
-		outputFile, err := os.Create(filepath.Join(outputDir, outputFileName))
-		if err != nil {
-			return fmt.Errorf("failed to make package output file: %w", err)
-		}
-		defer outputFile.Close()
-
-		if err := packageOptions.Build(ctx, outputFile, target); err != nil {
-			return fmt.Errorf("could not generate packages: %w", err)
+		if err := makeTarget(ctx, target, packageOptions, outputDir); err != nil {
+			return fmt.Errorf("making target %s: %w", target.String(), err)
 		}
 	}
 
 	fmt.Printf("Built packages in %s\n", outputDir)
 	return nil
 }
 
+func makeTarget(ctx context.Context, target packaging.Target, packageOptions packaging.PackageOptions, outputDir string) error {
+	outputFileName := fmt.Sprintf("launcher.%s.%s", target.String(), target.PkgExtension())
+	outputFile, err := os.Create(filepath.Join(outputDir, outputFileName))
+	if err != nil {
+		return fmt.Errorf("failed to make package output file: %w", err)
+	}
+	defer outputFile.Close()
+
+	if err := packageOptions.Build(ctx, outputFile, target); err != nil {
+		return fmt.Errorf("could not generate packages: %w", err)
+	}
+
+	return nil
+}
+
 func usageFor(fs *flag.FlagSet, short string) func() {
 	return func() {
 		fmt.Fprintf(os.Stderr, "USAGE\n")

diff --git a/ee/debug/checkups/bboltdb.go b/ee/debug/checkups/bboltdb.go
@@ -71,18 +71,9 @@ func (c *bboltdbCheckup) backupStats() (map[string]map[string]any, error) {
 
 		backupStatsMap[backupDbLocation] = make(map[string]any)
 
-		// Open a connection to the backup, since we don't have one available yet
-		boltOptions := &bbolt.Options{Timeout: time.Duration(30) * time.Second}
-		backupDb, err := bbolt.Open(backupDbLocation, 0600, boltOptions)
+		backupStats, err := backupStatsFromDb(backupDbLocation)
 		if err != nil {
-			return nil, fmt.Errorf("could not open backup db at %s: %w", backupDbLocation, err)
-		}
-		defer backupDb.Close()
-
-		// Gather stats
-		backupStats, err := agent.GetStats(backupDb)
-		if err != nil {
-			return nil, fmt.Errorf("could not get backup db stats: %w", err)
+			return nil, fmt.Errorf("could not get backup db stats from %s: %w", backupDbLocation, err)
 		}
 
 		for k, v := range backupStats.Buckets {
@@ -93,6 +84,24 @@ func (c *bboltdbCheckup) backupStats() (map[string]map[string]any, error) {
 	return backupStatsMap, nil
 }
 
+func backupStatsFromDb(backupDbLocation string) (*agent.Stats, error) {
+	// Open a connection to the backup, since we don't have one available yet
+	boltOptions := &bbolt.Options{Timeout: time.Duration(30) * time.Second}
+	backupDb, err := bbolt.Open(backupDbLocation, 0600, boltOptions)
+	if err != nil {
+		return nil, fmt.Errorf("could not open backup db at %s: %w", backupDbLocation, err)
+	}
+	defer backupDb.Close()
+
+	// Gather stats
+	backupStats, err := agent.GetStats(backupDb)
+	if err != nil {
+		return nil, fmt.Errorf("could not get backup db stats: %w", err)
+	}
+
+	return backupStats, nil
+}
+
 func (c *bboltdbCheckup) ExtraFileName() string {
 	return "backup.json"
 }

diff --git a/ee/debug/checkups/checkups.go b/ee/debug/checkups/checkups.go
@@ -216,12 +216,7 @@ func RunDoctor(ctx context.Context, k types.Knapsack, w io.Writer) {
 	warningCheckups := []string{}
 
 	for _, c := range checkupsFor(k, doctorSupported) {
-		ctx, cancel := context.WithTimeout(context.TODO(), 20*time.Second)
-		defer cancel()
-
-		doctorCheckup(ctx, c, w)
-
-		switch c.Status() {
+		switch runDoctorCheckup(ctx, c, w) {
 		case Warning:
 			warningCheckups = append(warningCheckups, c.Name())
 		case Failing, Erroring:
@@ -248,6 +243,15 @@ func RunDoctor(ctx context.Context, k types.Knapsack, w io.Writer) {
 	}
 }
 
+func runDoctorCheckup(ctx context.Context, c checkupInt, w io.Writer) Status {
+	ctx, cancel := context.WithTimeout(ctx, 20*time.Second)
+	defer cancel()
+
+	doctorCheckup(ctx, c, w)
+
+	return c.Status()
+}
+
 type runtimeEnvironmentType string
 
 const (

diff --git a/ee/debug/checkups/network.go b/ee/debug/checkups/network.go
@@ -48,14 +48,7 @@ func (n *networkCheckup) Run(ctx context.Context, extraWriter io.Writer) error {
 	}
 
 	for _, c := range listCommands() {
-		ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
-		defer cancel()
-
-		cmd, err := c.cmd(ctx, c.args...)
-		if err != nil {
-			continue
-		}
-		_ = runCmdMarkdownLogged(cmd.Cmd, commandOutput)
+		runCommand(ctx, c, commandOutput)
 	}
 
 	for _, fileLocation := range listFiles() {
@@ -65,6 +58,17 @@ func (n *networkCheckup) Run(ctx context.Context, extraWriter io.Writer) error {
 	return nil
 }
 
+func runCommand(ctx context.Context, c networkCommand, commandOutput io.Writer) {
+	ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+
+	cmd, err := c.cmd(ctx, c.args...)
+	if err != nil {
+		return
+	}
+	_ = runCmdMarkdownLogged(cmd.Cmd, commandOutput)
+}
+
 func (n *networkCheckup) ExtraFileName() string {
 	return "network.zip"
 }

diff --git a/ee/desktop/user/notify/notify_linux.go b/ee/desktop/user/notify/notify_linux.go
@@ -10,7 +10,6 @@ import (
 	"log/slog"
 	"sync"
 	"sync/atomic"
-	"time"
 
 	"github.com/godbus/dbus/v5"
 	"github.com/kolide/launcher/ee/allowedcmd"
@@ -101,9 +100,7 @@ func (d *dbusNotifier) Execute() error {
 			actionUri := signal.Body[1].(string)
 
 			for _, browserLauncher := range browserLaunchers {
-				ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
-				defer cancel()
-				cmd, err := browserLauncher(ctx, actionUri)
+				cmd, err := browserLauncher(context.TODO(), actionUri)
 				if err != nil {
 					d.slogger.Log(context.TODO(), slog.LevelWarn,
 						"couldn't create command to start process",

diff --git a/ee/katc/table_test.go b/ee/katc/table_test.go
@@ -61,23 +61,7 @@ func TestQueryFirefoxIndexedDB(t *testing.T) {
 			require.NoError(t, err, "opening reader to zip file")
 			defer zipReader.Close()
 			for _, fileInZip := range zipReader.File {
-				fileInZipReader, err := fileInZip.Open()
-				require.NoError(t, err, "opening file in zip")
-				defer fileInZipReader.Close()
-
-				idbFilePath := filepath.Join(tempDir, fileInZip.Name)
-
-				if fileInZip.FileInfo().IsDir() {
-					require.NoError(t, os.MkdirAll(idbFilePath, fileInZip.Mode()), "creating dir")
-					continue
-				}
-
-				outFile, err := os.OpenFile(idbFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileInZip.Mode())
-				require.NoError(t, err, "opening output file")
-				defer outFile.Close()
-
-				_, err = io.Copy(outFile, fileInZipReader)
-				require.NoError(t, err, "copying from zip to temp dir")
+				unzipFile(t, fileInZip, tempDir)
 			}
 
 			// Construct table
@@ -148,6 +132,26 @@ func TestQueryFirefoxIndexedDB(t *testing.T) {
 	}
 }
 
+func unzipFile(t *testing.T, fileInZip *zip.File, tempDir string) {
+	fileInZipReader, err := fileInZip.Open()
+	require.NoError(t, err, "opening file in zip")
+	defer fileInZipReader.Close()
+
+	idbFilePath := filepath.Join(tempDir, fileInZip.Name)
+
+	if fileInZip.FileInfo().IsDir() {
+		require.NoError(t, os.MkdirAll(idbFilePath, fileInZip.Mode()), "creating dir")
+		return
+	}
+
+	outFile, err := os.OpenFile(idbFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileInZip.Mode())
+	require.NoError(t, err, "opening output file")
+	defer outFile.Close()
+
+	_, err = io.Copy(outFile, fileInZipReader)
+	require.NoError(t, err, "copying from zip to temp dir")
+}
+
 func TestQueryChromeIndexedDB(t *testing.T) {
 	t.Parallel()
 
@@ -206,23 +210,7 @@ func TestQueryChromeIndexedDB(t *testing.T) {
 			require.NoError(t, err, "opening reader to zip file")
 			defer zipReader.Close()
 			for _, fileInZip := range zipReader.File {
-				fileInZipReader, err := fileInZip.Open()
-				require.NoError(t, err, "opening file in zip")
-				defer fileInZipReader.Close()
-
-				idbFilePath := filepath.Join(tempDir, fileInZip.Name)
-
-				if fileInZip.FileInfo().IsDir() {
-					require.NoError(t, os.MkdirAll(idbFilePath, fileInZip.Mode()), "creating dir")
-					continue
-				}
-
-				outFile, err := os.OpenFile(idbFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileInZip.Mode())
-				require.NoError(t, err, "opening output file")
-				defer outFile.Close()
-
-				_, err = io.Copy(outFile, fileInZipReader)
-				require.NoError(t, err, "copying from zip to temp dir")
+				unzipFile(t, fileInZip, tempDir)
 			}
 
 			// Construct table