Configure golangci-lint for the whole repository

Add a relatively minimal set of linters that raise safe and 
mostly un-opinionated issues with the code. It runs 
automatically on CI via a github action.

.github/workflows/golangci-lint.yml

@@ -16,6 +16,5 @@ jobs:
           # Required: the version of golangci-lint is required and must be
           # specified without patch version: we always use the latest patch
           # version.
-          version: v1.33
-          working-directory: orbit/
+          version: v1.42
           args: --timeout 3m

.golangci.yml

@@ -0,0 +1,41 @@
+linters:
+  disable-all: true
+  enable:
+    - deadcode
+    - gofmt
+    - govet
+    - ineffassign
+    - revive
+    - rowserrcheck
+    - sqlclosecheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unused
+
+linters-settings:
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: false
+
+  revive:
+    ignoreGeneratedHeader: false
+    severity: "warning"
+    confidence: 0.8
+    errorCode: 0
+    warningCode: 0
+
+    rules:
+      - name: dot-imports
+      - name: error-return
+      - name: var-declaration
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: time-naming
+      - name: indent-error-flow
+      - name: errorf
+      - name: empty-block
+      - name: superfluous-else
+      - name: unreachable-code
+      - name: redefines-builtin-id

cmd/fleetctl/config.go

@@ -123,9 +123,8 @@ func getConfigValue(configPath, context, key string) (interface{}, error) {
 	case "tls-skip-verify":
 		if currentContext.TLSSkipVerify {
 			return true, nil
-		} else {
-			return false, nil
 		}
+		return false, nil
 	case "url-prefix":
 		return currentContext.URLPrefix, nil
 	default:

cmd/fleetctl/debug.go

@@ -94,7 +94,7 @@ func debugProfileCommand() *cli.Command {
 
 func joinCmdline(cmdline string) string {
 	var tokens []string
-	for _, token := range strings.Split(string(cmdline), "\x00") {
+	for _, token := range strings.Split(cmdline, "\x00") {
 		tokens = append(tokens, fmt.Sprintf("'%s'", token))
 	}
 	return fmt.Sprintf("[%s]", strings.Join(tokens, ", "))

cmd/fleetctl/user.go

@@ -15,7 +15,6 @@ import (
 )
 
 const (
-	adminFlagName      = "admin"
 	globalRoleFlagName = "global-role"
 	teamFlagName       = "team"
 	passwordFlagName   = "password"

ee/fleetctl/updates.go

@@ -29,7 +29,7 @@ const (
 	// Expirations from
 	// https://github.com/theupdateframework/notary/blob/e87b31f46cdc5041403c64b7536df236d5e35860/docs/best_practices.md#expiration-prevention
 	// ~10 years
-	rootExpirationDuration = 10 * 365 * 24 * time.Hour
+	rootExpirationDuration = 10 * 365 * 24 * time.Hour //nolint:unused,deadcode
 	// ~3 years
 	targetsExpirationDuration = 3 * 365 * 24 * time.Hour
 	// ~3 years
@@ -93,9 +93,8 @@ func updatesInitFunc(c *cli.Context) error {
 	if _, err := os.Stat(filepath.Join(path, "keys")); !errors.Is(err, os.ErrNotExist) {
 		if err == nil {
 			return errors.Errorf("keys directory already exists: %s", filepath.Join(path, "keys"))
-		} else {
-			return errors.Wrap(err, "failed to check existence of keys directory")
 		}
+		return errors.Wrap(err, "failed to check existence of keys directory")
 	}
 
 	repo, err := tuf.NewRepo(store)
@@ -444,7 +443,7 @@ func (p *passphraseHandler) readPassphrase(role string, confirm bool) ([]byte, e
 		}
 
 		fmt.Printf("Enter %s key passphrase: ", role)
-		passphrase, err := terminal.ReadPassword(int(syscall.Stdin))
+		passphrase, err := terminal.ReadPassword(syscall.Stdin)
 		fmt.Println()
 		if err != nil {
 			return nil, errors.Wrap(err, "read password")
@@ -455,7 +454,7 @@ func (p *passphraseHandler) readPassphrase(role string, confirm bool) ([]byte, e
 		}
 
 		fmt.Printf("Repeat %s key passphrase: ", role)
-		confirmation, err := terminal.ReadPassword(int(syscall.Stdin))
+		confirmation, err := terminal.ReadPassword(syscall.Stdin)
 		fmt.Println()
 		if err != nil {
 			return nil, errors.Wrap(err, "read password confirmation")

ee/server/licensing/licensing.go

@@ -34,9 +34,8 @@ func loadPublicKey() (*ecdsa.PublicKey, error) {
 
 	if pub, ok := pub.(*ecdsa.PublicKey); ok {
 		return pub, nil
-	} else {
-		return nil, errors.Errorf("%T is not *ecdsa.PublicKey", pub)
 	}
+	return nil, errors.Errorf("%T is not *ecdsa.PublicKey", pub)
 }
 
 // LoadLicense loads and validates the license key.

pkg/secure/secure.go

@@ -30,9 +30,8 @@ func checkPermPath(path string, perm os.FileMode) error {
 					"Path %s already exists with mode %o instead of the expected %o", path, dir.Mode(), perm)
 			}
 			return nil
-		} else {
-			return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR}
 		}
+		return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR}
 	}
 
 	// Look for the parent directory in the path and then check the permissions in that

server/datastore/mysql/carves_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var mockCreatedAt time.Time = time.Now().UTC().Truncate(time.Second)
+var mockCreatedAt = time.Now().UTC().Truncate(time.Second)
 
 func TestCarveMetadata(t *testing.T) {
 	ds := CreateMySQLDS(t)

server/datastore/mysql/email_changes_test.go

@@ -33,7 +33,7 @@ func TestChangeEmail(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, "xxxx@yyy.com", user.Email)
 	// this should fail because it doesn't exist
-	newMail, err = ds.ConfirmPendingEmailChange(user.ID, "abcd12345")
+	_, err = ds.ConfirmPendingEmailChange(user.ID, "abcd12345")
 	assert.NotNil(t, err)
 
 	// test that wrong user can't confirm e-mail change

server/datastore/mysql/hosts_test.go

@@ -351,59 +351,6 @@ func TestDeleteHost(t *testing.T) {
 	assert.NotNil(t, err)
 }
 
-func testListHosts(t *testing.T, ds fleet.Datastore) {
-	hosts := []*fleet.Host{}
-	for i := 0; i < 10; i++ {
-		host, err := ds.NewHost(&fleet.Host{
-			DetailUpdatedAt: time.Now(),
-			LabelUpdatedAt:  time.Now(),
-			SeenTime:        time.Now(),
-			OsqueryHostID:   strconv.Itoa(i),
-			NodeKey:         fmt.Sprintf("%d", i),
-			UUID:            fmt.Sprintf("%d", i),
-			Hostname:        fmt.Sprintf("foo.local%d", i),
-		})
-		assert.Nil(t, err)
-		if err != nil {
-			return
-		}
-		hosts = append(hosts, host)
-	}
-
-	filter := fleet.TeamFilter{User: test.UserAdmin}
-	hosts2, err := ds.ListHosts(filter, fleet.HostListOptions{})
-	require.Nil(t, err)
-	assert.Equal(t, len(hosts), len(hosts2))
-
-	for _, host := range hosts2 {
-		i, err := strconv.Atoi(host.UUID)
-		assert.Nil(t, err)
-		assert.True(t, i >= 0)
-		assert.True(t, strings.HasPrefix(host.Hostname, "foo.local"))
-	}
-
-	// Test with logic for only a few hosts
-	hosts2, err = ds.ListHosts(filter, fleet.HostListOptions{ListOptions: fleet.ListOptions{PerPage: 4, Page: 0}})
-	require.Nil(t, err)
-	assert.Equal(t, 4, len(hosts2))
-
-	err = ds.DeleteHost(hosts[0].ID)
-	require.Nil(t, err)
-	hosts2, err = ds.ListHosts(filter, fleet.HostListOptions{})
-	require.Nil(t, err)
-	assert.Equal(t, len(hosts)-1, len(hosts2))
-
-	hosts, err = ds.ListHosts(filter, fleet.HostListOptions{})
-	require.Nil(t, err)
-	require.Equal(t, len(hosts2), len(hosts))
-
-	err = ds.SaveHost(hosts[0])
-	require.Nil(t, err)
-	hosts2, err = ds.ListHosts(filter, fleet.HostListOptions{})
-	require.Nil(t, err)
-	require.Equal(t, hosts[0].ID, hosts2[0].ID)
-}
-
 func TestListHostsFilterAdditional(t *testing.T) {
 	ds := CreateMySQLDS(t)
 	defer ds.Close()

server/datastore/mysql/invites_test.go

@@ -143,9 +143,8 @@ func TestInviteByToken(t *testing.T) {
 				require.NotNil(t, err)
 				assert.Equal(t, tt.wantErr.Error(), err.Error())
 				return
-			} else {
-				require.Nil(t, err)
 			}
+			require.Nil(t, err)
 			assert.NotEqual(t, invite.ID, 0)
 
 		})
@@ -178,9 +177,8 @@ func TestInviteByEmail(t *testing.T) {
 				require.NotNil(t, err)
 				assert.Equal(t, tt.wantErr.Error(), err.Error())
 				return
-			} else {
-				require.Nil(t, err)
 			}
+			require.Nil(t, err)
 			assert.NotEqual(t, invite.ID, 0)
 
 		})

server/datastore/mysql/labels.go

@@ -336,7 +336,7 @@ func (d *Datastore) RecordLabelQueryExecutions(host *fleet.Host, results map[uin
 	// Sort the results to have generated SQL queries ordered to minimize
 	// deadlocks. See https://github.com/fleetdm/fleet/v4/issues/1146.
 	orderedIDs := make([]uint, 0, len(results))
-	for labelID, _ := range results {
+	for labelID := range results {
 		orderedIDs = append(orderedIDs, labelID)
 	}
 	sort.Slice(orderedIDs, func(i, j int) bool { return orderedIDs[i] < orderedIDs[j] })

server/datastore/mysql/migrations/tables/20171116163618_CreateTableOsqueryOptions.go

@@ -93,7 +93,7 @@ func migrateOptions(tx *sql.Tx) error {
 		INNER JOIN file_integrity_monitoring_files AS mf
 		ON (fim.id = mf.file_integrity_monitoring_id)
 	`
-	rows, err := txx.Query(query)
+	rows, err := txx.Query(query) //nolint
 	if err != nil && err != sql.ErrNoRows {
 		return errors.Wrap(err, "retrieving fim paths")
 	}

server/datastore/mysql/migrations/tables/20210601000008_TeamsEnrollSecrets.go

@@ -38,6 +38,7 @@ func Up_20210601000008(tx *sql.Tx) error {
 	//}
 	// ********* TEST ONLY ENDS  *********
 
+	//nolint
 	rows, err := tx.Query(`SELECT secret, count(secret) FROM enroll_secrets GROUP BY secret HAVING count(secret) > 1`)
 	if err != nil {
 		return errors.Wrap(err, "remove duplicate secrets")

server/datastore/mysql/migrations/tables/20210816141251_AddJSONKeyValueTable.go

@@ -26,7 +26,7 @@ func Up_20210816141251(tx *sql.Tx) error {
 		return errors.Wrap(err, "create app_config_json")
 	}
 	row := tx.QueryRow(
-		`SELECT 
+		`SELECT
        			org_name,
 				org_logo_url,
 				server_url,
@@ -108,6 +108,7 @@ func Up_20210816141251(tx *sql.Tx) error {
 	if err != nil {
 		return errors.Wrap(err, "marshaling config")
 	}
+	//nolint
 	_, err = tx.Exec(
 		`INSERT INTO app_config_json(json_value) VALUES(?) ON DUPLICATE KEY UPDATE json_value = VALUES(json_value)`,
 		configBytes,

server/datastore/mysql/migrations/tables/20210818151827_RemoveForeignKeysSchedQStats.go

@@ -36,7 +36,7 @@ func Up_20210818151827(tx *sql.Tx) error {
 func constraintsForTable(tx *sql.Tx, table string, referencedTables map[string]struct{}) ([]string, error) {
 	var constraints []string
 	query := `SELECT DISTINCT CONSTRAINT_NAME, REFERENCED_TABLE_NAME FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE TABLE_NAME = ? AND CONSTRAINT_NAME <> 'PRIMARY'`
-	rows, err := tx.Query(query, table)
+	rows, err := tx.Query(query, table) //nolint
 	if err != nil {
 		return nil, errors.Wrap(err, "getting fk for scheduled_query_stats")
 	}

server/datastore/mysql/mysql.go

@@ -376,9 +376,8 @@ func (d *Datastore) whereFilterHostsByTeams(filter fleet.TeamFilter, hostKey str
 		case fleet.RoleObserver:
 			if filter.IncludeObserver {
 				return "TRUE"
-			} else {
-				return "FALSE"
 			}
+			return "FALSE"
 
 		default:
 			// Fall through to specific teams
@@ -425,9 +424,8 @@ func (d *Datastore) whereFilterTeams(filter fleet.TeamFilter, teamKey string) st
 		case fleet.RoleObserver:
 			if filter.IncludeObserver {
 				return "TRUE"
-			} else {
-				return "FALSE"
 			}
+			return "FALSE"
 
 		default:
 			// Fall through to specific teams

server/datastore/mysql/scheduled_queries.go

@@ -167,12 +167,12 @@ func (d *Datastore) ScheduledQuery(id uint) (*fleet.ScheduledQuery, error) {
 	return sq, nil
 }
 
-func (ds *Datastore) CleanupOrphanScheduledQueryStats() error {
-	_, err := ds.db.Exec(`DELETE FROM scheduled_query_stats where scheduled_query_id not in (select id from scheduled_queries where id=scheduled_query_id)`)
+func (d *Datastore) CleanupOrphanScheduledQueryStats() error {
+	_, err := d.db.Exec(`DELETE FROM scheduled_query_stats where scheduled_query_id not in (select id from scheduled_queries where id=scheduled_query_id)`)
 	if err != nil {
 		return errors.Wrap(err, "cleaning orphan scheduled_query_stats by scheduled_query")
 	}
-	_, err = ds.db.Exec(`DELETE FROM scheduled_query_stats where host_id not in (select id from hosts where id=host_id)`)
+	_, err = d.db.Exec(`DELETE FROM scheduled_query_stats where host_id not in (select id from hosts where id=host_id)`)
 	if err != nil {
 		return errors.Wrap(err, "cleaning orphan scheduled_query_stats by host")
 	}

server/datastore/mysql/software.go

@@ -211,13 +211,13 @@ func (d *Datastore) hostSoftwareFromHostID(tx *sqlx.Tx, id uint) ([]fleet.Softwa
 		selectFunc = tx.Select
 	}
 	sql := `
-		SELECT s.id, s.name, s.version, s.source, coalesce(scp.cpe, "") as generated_cpe, 
+		SELECT s.id, s.name, s.version, s.source, coalesce(scp.cpe, "") as generated_cpe,
 			IF(
-				COUNT(scv.cve) = 0, 
-				null, 
+				COUNT(scv.cve) = 0,
+				null,
 				GROUP_CONCAT(
 					JSON_OBJECT(
-						"cve", scv.cve, 
+						"cve", scv.cve,
 						"details_link", CONCAT('https://nvd.nist.gov/vuln/detail/', scv.cve)
 					)
 				)
@@ -273,7 +273,9 @@ func (si *softwareIterator) Next() bool {
 
 func (d *Datastore) AllSoftwareWithoutCPEIterator() (fleet.SoftwareIterator, error) {
 	sql := `SELECT s.* FROM software s LEFT JOIN software_cpe sc on (s.id=sc.software_id) WHERE sc.id is null`
-	rows, err := d.db.Queryx(sql)
+	// The rows.Close call is done by the caller once iteration using the
+	// returned fleet.SoftwareIterator is done.
+	rows, err := d.db.Queryx(sql) //nolint:sqlclosecheck
 	if err != nil {
 		return nil, errors.Wrap(err, "load host software")
 	}

server/datastore/mysql/statistics.go

@@ -37,9 +37,8 @@ func (d *Datastore) ShouldSendStatistics(frequency time.Duration) (fleet.Statist
 				FleetVersion:        version.Version().Version,
 				NumHostsEnrolled:    amountEnrolledHosts,
 			}, true, nil
-		} else {
-			return fleet.StatisticsPayload{}, false, err
 		}
+		return fleet.StatisticsPayload{}, false, err
 	}
 	lastUpdated := dest.UpdatedAt
 	if dest.CreatedAt.After(dest.UpdatedAt) {

server/fleet/app.go

@@ -168,15 +168,15 @@ type AppConfig struct {
 	VulnerabilitySettings VulnerabilitySettings `json:"vulnerability_settings"`
 }
 
-func (ac *AppConfig) ApplyDefaultsForNewInstalls() {
-	ac.ServerSettings.EnableAnalytics = true
-	ac.HostSettings.EnableHostUsers = true
-	ac.SMTPSettings.SMTPPort = 587
-	ac.SMTPSettings.SMTPEnableStartTLS = true
-	ac.SMTPSettings.SMTPAuthenticationType = AuthTypeNameUserNamePassword
-	ac.SMTPSettings.SMTPAuthenticationMethod = AuthMethodNamePlain
-	ac.SMTPSettings.SMTPVerifySSLCerts = true
-	ac.SMTPSettings.SMTPEnableTLS = true
+func (c *AppConfig) ApplyDefaultsForNewInstalls() {
+	c.ServerSettings.EnableAnalytics = true
+	c.HostSettings.EnableHostUsers = true
+	c.SMTPSettings.SMTPPort = 587
+	c.SMTPSettings.SMTPEnableStartTLS = true
+	c.SMTPSettings.SMTPAuthenticationType = AuthTypeNameUserNamePassword
+	c.SMTPSettings.SMTPAuthenticationMethod = AuthMethodNamePlain
+	c.SMTPSettings.SMTPVerifySSLCerts = true
+	c.SMTPSettings.SMTPEnableTLS = true
 }
 
 // OrgInfo contains general info about the organization using Fleet.

server/fleet/carves.go

@@ -62,8 +62,8 @@ func (c CarveMetadata) AuthzType() string {
 	return "carve"
 }
 
-func (m *CarveMetadata) BlocksComplete() bool {
-	return m.MaxBlock == m.BlockCount-1
+func (c *CarveMetadata) BlocksComplete() bool {
+	return c.MaxBlock == c.BlockCount-1
 }
 
 type CarveListOptions struct {

server/fleet/import_config_unmarshaler.go

@@ -170,7 +170,7 @@ func unmarshalQueryDetail(val interface{}) (QueryDetails, error) {
 	}
 	result = QueryDetails{
 		Query:    query,
-		Interval: OsQueryConfigInt(interval),
+		Interval: interval,
 		Removed:  removed,
 		Platform: platform,
 		Version:  version,

server/fleet/packs_test.go

@@ -1,8 +1,9 @@
 package fleet
 
 import (
-	"github.com/fleetdm/fleet/v4/server/ptr"
 	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/ptr"
 )
 
 func TestPack_EditablePackType(t *testing.T) {
@@ -24,7 +25,7 @@ func TestPack_EditablePackType(t *testing.T) {
 		want   bool
 	}{
 		{
-			name:   "default",
+			name: "default",
 			fields: fields{
 				ID:          0,
 				Name:        "",
@@ -33,10 +34,10 @@ func TestPack_EditablePackType(t *testing.T) {
 				Disabled:    false,
 				Type:        nil,
 			},
-			want:   true,
+			want: true,
 		},
 		{
-			name:   "type is empty string",
+			name: "type is empty string",
 			fields: fields{
 				ID:          0,
 				Name:        "",
@@ -45,10 +46,10 @@ func TestPack_EditablePackType(t *testing.T) {
 				Disabled:    false,
 				Type:        ptr.String(""),
 			},
-			want:   true,
+			want: true,
 		},
 		{
-			name:   "type is not empty",
+			name: "type is not empty",
 			fields: fields{
 				ID:          0,
 				Name:        "Global",
@@ -57,10 +58,10 @@ func TestPack_EditablePackType(t *testing.T) {
 				Disabled:    false,
 				Type:        ptr.String("global"),
 			},
-			want:   false,
+			want: false,
 		},
 		{
-			name:   "type is not empty",
+			name: "type is not empty",
 			fields: fields{
 				ID:          0,
 				Name:        "team-1",
@@ -69,7 +70,7 @@ func TestPack_EditablePackType(t *testing.T) {
 				Disabled:    false,
 				Type:        ptr.String("team-1"),
 			},
-			want:   false,
+			want: false,
 		},
 	}
 	for _, tt := range tests {

server/fleet/users.go

@@ -3,6 +3,7 @@ package fleet
 import (
 	"context"
 	"fmt"
+
 	"github.com/fleetdm/fleet/v4/server"
 	"golang.org/x/crypto/bcrypt"
 )
@@ -207,11 +208,3 @@ func (u *User) SetPassword(plaintext string, keySize, cost int) error {
 	u.Password = hashed
 	return nil
 }
-
-// helper to convert a bool pointer false
-func falseIfNil(b *bool) bool {
-	if b == nil {
-		return false
-	}
-	return *b
-}

server/logging/filesystem.go

@@ -35,7 +35,7 @@ func NewFilesystemLogWriter(path string, appLogger log.Logger, enableRotation bo
 			Compress:   enableCompression,
 		}
 		appLogger = log.With(appLogger, "component", "osqueryd-logger")
-		sig := make(chan os.Signal)
+		sig := make(chan os.Signal, 1)
 		signal.Notify(sig, syscall.SIGHUP)
 		go func() {
 			for {

server/logging/firehose.go

@@ -37,7 +37,7 @@ type firehoseLogWriter struct {
 
 func NewFirehoseLogWriter(region, endpointURL, id, secret, stsAssumeRoleArn, stream string, logger log.Logger) (*firehoseLogWriter, error) {
 	conf := &aws.Config{
-		Region: &region,
+		Region:   &region,
 		Endpoint: &endpointURL, // empty string or nil will use default values
 	}
 

server/logging/firehose_test.go

@@ -67,11 +67,10 @@ func TestFirehoseRetryableFailure(t *testing.T) {
 		assert.Equal(t, "foobar", *input.DeliveryStreamName)
 		if callCount < 3 {
 			return nil, awserr.New(firehose.ErrCodeServiceUnavailableException, "", nil)
-		} else {
-			// Returning a non-retryable error earlier helps keep
-			// this test faster
-			return nil, errors.New("generic error")
 		}
+		// Returning a non-retryable error earlier helps keep
+		// this test faster
+		return nil, errors.New("generic error")
 	}
 	f := &mock.FirehoseMock{PutRecordBatchFunc: putFunc}
 	writer := makeFirehoseWriterWithMock(f, "foobar")
@@ -167,22 +166,15 @@ func TestFirehoseFailAllRecords(t *testing.T) {
 			return &firehose.PutRecordBatchOutput{
 				FailedPutCount: aws.Int64(1),
 				RequestResponses: []*firehose.PutRecordBatchResponseEntry{
-					&firehose.PutRecordBatchResponseEntry{
-						ErrorCode: aws.String("error"),
-					},
-					&firehose.PutRecordBatchResponseEntry{
-						ErrorCode: aws.String("error"),
-					},
-					&firehose.PutRecordBatchResponseEntry{
-						ErrorCode: aws.String("error"),
-					},
+					{ErrorCode: aws.String("error")},
+					{ErrorCode: aws.String("error")},
+					{ErrorCode: aws.String("error")},
 				},
 			}, nil
-		} else {
-			// Make test quicker by returning non-retryable error
-			// before all retries are exhausted.
-			return nil, errors.New("generic error")
 		}
+		// Make test quicker by returning non-retryable error
+		// before all retries are exhausted.
+		return nil, errors.New("generic error")
 	}
 
 	writer := makeFirehoseWriterWithMock(f, "foobar")

server/logging/kinesis.go

@@ -40,7 +40,7 @@ type kinesisLogWriter struct {
 
 func NewKinesisLogWriter(region, endpointURL, id, secret, stsAssumeRoleArn, stream string, logger log.Logger) (*kinesisLogWriter, error) {
 	conf := &aws.Config{
-		Region: &region,
+		Region:   &region,
 		Endpoint: &endpointURL, // empty string or nil will use default values
 	}
 

server/logging/kinesis_test.go

@@ -43,10 +43,9 @@ func TestKinesisRetryableFailure(t *testing.T) {
 		assert.Equal(t, "foobar", *input.StreamName)
 		if callCount < 3 {
 			return nil, awserr.New(kinesis.ErrCodeProvisionedThroughputExceededException, "", nil)
-		} else {
-			// Returning a non-retryable error earlier helps keep this test faster
-			return nil, errors.New("generic error")
 		}
+		// Returning a non-retryable error earlier helps keep this test faster
+		return nil, errors.New("generic error")
 	}
 	k := &mock.KinesisMock{PutRecordsFunc: putFunc}
 	writer := makeKinesisWriterWithMock(k, "foobar")
@@ -142,22 +141,15 @@ func TestKinesisFailAllRecords(t *testing.T) {
 			return &kinesis.PutRecordsOutput{
 				FailedRecordCount: aws.Int64(1),
 				Records: []*kinesis.PutRecordsResultEntry{
-					&kinesis.PutRecordsResultEntry{
-						ErrorCode: aws.String("error"),
-					},
-					&kinesis.PutRecordsResultEntry{
-						ErrorCode: aws.String("error"),
-					},
-					&kinesis.PutRecordsResultEntry{
-						ErrorCode: aws.String("error"),
-					},
+					{ErrorCode: aws.String("error")},
+					{ErrorCode: aws.String("error")},
+					{ErrorCode: aws.String("error")},
 				},
 			}, nil
-		} else {
-			// Make test quicker by returning non-retryable error
-			// before all retries are exhausted.
-			return nil, errors.New("generic error")
 		}
+		// Make test quicker by returning non-retryable error
+		// before all retries are exhausted.
+		return nil, errors.New("generic error")
 	}
 
 	writer := makeKinesisWriterWithMock(k, "foobar")

server/pubsub/redis_query_results.go

@@ -167,10 +167,9 @@ func receiveMessages(ctx context.Context, pool *redisc.Cluster, query fleet.Dist
 				if err, ok := msg.(net.Error); ok && err.Timeout() {
 					// We ignore timeouts, we just want them there to make sure we don't hang on Receiving
 					continue
-				} else {
-					// If an error occurred (i.e. connection was closed), then we should exit
-					return
 				}
+				// If an error occurred (i.e. connection was closed), then we should exit
+				return
 			case redis.Subscription:
 				// If the subscription count is 0, the ReadChannel call that invoked this goroutine has unsubscribed,
 				// and we can exit

server/service/client_errors.go

@@ -46,7 +46,7 @@ func (e notFoundErr) Error() string {
 	return "The resource was not found"
 }
 
-func (n notFoundErr) NotFound() bool {
+func (e notFoundErr) NotFound() bool {
 	return true
 }
 

server/service/endpoint_teams.go

@@ -18,7 +18,7 @@ type createTeamRequest struct {
 
 type teamResponse struct {
 	Team *fleet.Team `json:"team,omitempty"`
-	Err  error        `json:"error,omitempty"`
+	Err  error       `json:"error,omitempty"`
 }
 
 func (r teamResponse) error() error { return r.Err }
@@ -88,7 +88,7 @@ type listTeamsRequest struct {
 
 type listTeamsResponse struct {
 	Teams []fleet.Team `json:"teams"`
-	Err   error         `json:"error,omitempty"`
+	Err   error        `json:"error,omitempty"`
 }
 
 func (r listTeamsResponse) error() error { return r.Err }
@@ -204,7 +204,7 @@ type teamEnrollSecretsRequest struct {
 
 type teamEnrollSecretsResponse struct {
 	Secrets []*fleet.EnrollSecret `json:"secrets"`
-	Err     error                  `json:"error,omitempty"`
+	Err     error                 `json:"error,omitempty"`
 }
 
 func (r teamEnrollSecretsResponse) error() error { return r.Err }

server/service/service_campaigns.go

@@ -96,7 +96,7 @@ func (svc Service) NewDistributedQueryCampaign(ctx context.Context, queryString
 	}
 
 	defer func() {
-		var numHosts uint = 0
+		var numHosts uint
 		if campaign != nil {
 			numHosts = campaign.Metrics.TotalHosts
 		}

server/service/service_osquery.go

@@ -532,7 +532,7 @@ func (svc *Service) ingestDistributedQuery(host fleet.Host, name string, rows []
 		// execute that query when we can't write to any subscriber
 		campaign, err := svc.ds.DistributedQueryCampaign(uint(campaignID))
 		if err != nil {
-			if err := svc.liveQueryStore.StopQuery(strconv.Itoa(int(campaignID))); err != nil {
+			if err := svc.liveQueryStore.StopQuery(strconv.Itoa(campaignID)); err != nil {
 				return osqueryError{message: "stop orphaned campaign after load failure: " + err.Error()}
 			}
 			return osqueryError{message: "loading orphaned campaign: " + err.Error()}
@@ -551,7 +551,7 @@ func (svc *Service) ingestDistributedQuery(host fleet.Host, name string, rows []
 			}
 		}
 
-		if err := svc.liveQueryStore.StopQuery(strconv.Itoa(int(campaignID))); err != nil {
+		if err := svc.liveQueryStore.StopQuery(strconv.Itoa(campaignID)); err != nil {
 			return osqueryError{message: "stopping orphaned campaign: " + err.Error()}
 		}
 
@@ -559,7 +559,7 @@ func (svc *Service) ingestDistributedQuery(host fleet.Host, name string, rows []
 		return nil
 	}
 
-	err = svc.liveQueryStore.QueryCompletedByHost(strconv.Itoa(int(campaignID)), host.ID)
+	err = svc.liveQueryStore.QueryCompletedByHost(strconv.Itoa(campaignID), host.ID)
 	if err != nil {
 		return osqueryError{message: "record query completion: " + err.Error()}
 	}

server/service/service_sessions.go

@@ -236,7 +236,7 @@ func (svc *Service) makeSession(id uint) (string, error) {
 		AccessedAt: time.Now().UTC(),
 	}
 
-	session, err = svc.ds.NewSession(session)
+	_, err = svc.ds.NewSession(session)
 	if err != nil {
 		return "", errors.Wrap(err, "creating new session")
 	}

server/service/service_sessions_test.go

@@ -1,12 +1,10 @@
 package service
 
 import (
-	"context"
 	"testing"
 	"time"
 
 	"github.com/fleetdm/fleet/v4/server/datastore/mysql"
-	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/fleetdm/fleet/v4/server/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -54,15 +52,3 @@ func TestAuthenticate(t *testing.T) {
 		})
 	}
 }
-
-type authViewerService struct {
-	fleet.Service
-}
-
-func (authViewerService) GetSessionByKey(ctx context.Context, key string) (*fleet.Session, error) {
-	return &fleet.Session{}, nil
-}
-
-func (authViewerService) UserUnauthorized(ctx context.Context, uid uint) (*fleet.User, error) {
-	return &fleet.User{}, nil
-}

server/service/service_users_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/fleetdm/fleet/v4/server/ptr"
 	"github.com/fleetdm/fleet/v4/server/test"
 
-	"github.com/WatchBeam/clock"
 	"github.com/fleetdm/fleet/v4/server/mock"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -385,40 +384,6 @@ func TestModifyAdminUserEmailPassword(t *testing.T) {
 // 	}
 // }
 
-func setupInvites(t *testing.T, ds fleet.Datastore, emails []string) map[string]*fleet.Invite {
-	invites := make(map[string]*fleet.Invite)
-	users := createTestUsers(t, ds)
-	mockClock := clock.NewMockClock()
-	for _, e := range emails {
-		invite, err := ds.NewInvite(&fleet.Invite{
-			InvitedBy: users["admin1"].ID,
-			Token:     e,
-			Email:     e,
-			UpdateCreateTimestamps: fleet.UpdateCreateTimestamps{
-				CreateTimestamp: fleet.CreateTimestamp{
-					CreatedAt: mockClock.Now(),
-				},
-			},
-		})
-		require.Nil(t, err)
-		invites[e] = invite
-	}
-	// add an expired invitation
-	invite, err := ds.NewInvite(&fleet.Invite{
-		InvitedBy: users["admin1"].ID,
-		Token:     "expired",
-		Email:     "expiredinvite@gmail.com",
-		UpdateCreateTimestamps: fleet.UpdateCreateTimestamps{
-			CreateTimestamp: fleet.CreateTimestamp{
-				CreatedAt: mockClock.Now().AddDate(-1, 0, 0),
-			},
-		},
-	})
-	require.Nil(t, err)
-	invites["expired"] = invite
-	return invites
-}
-
 func TestChangePassword(t *testing.T) {
 	ds := mysql.CreateMySQLDS(t)
 	defer ds.Close()

server/service/testing_utils.go

@@ -25,12 +25,16 @@ func newTestService(ds fleet.Datastore, rs fleet.QueryResultStore, lq fleet.Live
 func newTestServiceWithConfig(ds fleet.Datastore, fleetConfig config.FleetConfig, rs fleet.QueryResultStore, lq fleet.LiveQueryStore, opts ...TestServerOpts) fleet.Service {
 	mailer := &mockMailService{SendEmailFn: func(e fleet.Email) error { return nil }}
 	license := fleet.LicenseInfo{Tier: "core"}
-	writer, err := logging.NewFilesystemLogWriter(
+	writer, _ := logging.NewFilesystemLogWriter(
 		fleetConfig.Filesystem.StatusLogFile,
 		kitlog.NewNopLogger(),
 		fleetConfig.Filesystem.EnableLogRotation,
 		fleetConfig.Filesystem.EnableLogCompression,
 	)
+	// See #1776
+	//if err != nil {
+	//	panic(err)
+	//}
 	osqlogger := &logging.OsqueryLogger{Status: writer, Result: writer}
 	logger := kitlog.NewNopLogger()
 	if len(opts) > 0 && opts[0].Logger != nil {
@@ -53,6 +57,9 @@ func newTestBasicService(ds fleet.Datastore, rs fleet.QueryResultStore, lq fleet
 		testConfig.Filesystem.EnableLogRotation,
 		testConfig.Filesystem.EnableLogCompression,
 	)
+	if err != nil {
+		panic(err)
+	}
 	osqlogger := &logging.OsqueryLogger{Status: writer, Result: writer}
 	svc, err := NewService(ds, rs, kitlog.NewNopLogger(), osqlogger, testConfig, mailer, clock.C, nil, lq, ds, license)
 	if err != nil {
@@ -75,6 +82,9 @@ func newTestServiceWithClock(ds fleet.Datastore, rs fleet.QueryResultStore, lq f
 		testConfig.Filesystem.EnableLogRotation,
 		testConfig.Filesystem.EnableLogCompression,
 	)
+	if err != nil {
+		panic(err)
+	}
 	osqlogger := &logging.OsqueryLogger{Status: writer, Result: writer}
 	svc, err := NewService(ds, rs, kitlog.NewNopLogger(), osqlogger, testConfig, mailer, c, nil, lq, ds, license)
 	if err != nil {

server/service/transport.go

@@ -95,7 +95,7 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
 	orderKey := r.URL.Query().Get("order_key")
 	orderDirectionString := r.URL.Query().Get("order_direction")
 
-	var page int = 0
+	var page int
 	if pageString != "" {
 		page, err = strconv.Atoi(pageString)
 		if err != nil {
@@ -108,7 +108,7 @@ func listOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) {
 
 	// We default to 0 for per_page so that not specifying any paging
 	// information gets all results
-	var perPage int = 0
+	var perPage int
 	if perPageString != "" {
 		perPage, err = strconv.Atoi(perPageString)
 		if err != nil {
@@ -232,15 +232,3 @@ func decodeGetGenericSpecRequest(ctx context.Context, r *http.Request) (interfac
 	req.Name = name
 	return req, nil
 }
-
-type genericIDListRequest struct {
-	IDs []uint `json:"ids"`
-}
-
-func decodeGenericIDListRequest(ctx context.Context, r *http.Request) (interface{}, error) {
-	var req genericIDListRequest
-	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		return nil, err
-	}
-	return req, nil
-}

server/service/transport_error.go

@@ -3,12 +3,13 @@ package service
 import (
 	"context"
 	"encoding/json"
+	"net/http"
+	"strconv"
+
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	kithttp "github.com/go-kit/kit/transport/http"
 	"github.com/go-sql-driver/mysql"
 	"github.com/pkg/errors"
-	"net/http"
-	"strconv"
 )
 
 // erroer interface is implemented by response structs to encode business logic errors
@@ -54,10 +55,6 @@ type existsErrorInterface interface {
 	IsExists() bool
 }
 
-type causerInterface interface {
-	Cause() error
-}
-
 // encode error and status header to the client
 func encodeError(ctx context.Context, err error, w http.ResponseWriter) {
 	enc := json.NewEncoder(w)

server/service/validation_app_config.go

@@ -26,13 +26,6 @@ func (mw validationMiddleware) ModifyAppConfig(ctx context.Context, p []byte) (*
 	return mw.Service.ModifyAppConfig(ctx, p)
 }
 
-func isSet(val *string) bool {
-	if val != nil {
-		return len(*val) > 0
-	}
-	return false
-}
-
 func validateSSOSettings(p fleet.AppConfig, existing *fleet.AppConfig, invalid *fleet.InvalidArgumentError) {
 	if p.SSOSettings.EnableSSO {
 		if p.SSOSettings.Metadata == "" && p.SSOSettings.MetadataURL == "" {