diff --git a/.github/workflows/orbit-golangci-lint.yml b/.github/workflows/golangci-lint.yml similarity index 88% rename from .github/workflows/orbit-golangci-lint.yml rename to .github/workflows/golangci-lint.yml index c62a16c286..d1072848fc 100644 --- a/.github/workflows/orbit-golangci-lint.yml +++ b/.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 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..ce8220bca6 --- /dev/null +++ b/.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 diff --git a/cmd/fleetctl/config.go b/cmd/fleetctl/config.go index 30a68e4941..250b3a4c71 100644 --- a/cmd/fleetctl/config.go +++ b/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: diff --git a/cmd/fleetctl/debug.go b/cmd/fleetctl/debug.go index 299477ff5c..1c82528223 100644 --- a/cmd/fleetctl/debug.go +++ b/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, ", ")) diff --git a/cmd/fleetctl/user.go b/cmd/fleetctl/user.go index d5663ac1de..853fc437a6 100644 --- a/cmd/fleetctl/user.go +++ b/cmd/fleetctl/user.go @@ -15,7 +15,6 @@ import ( ) const ( - adminFlagName = "admin" globalRoleFlagName = "global-role" teamFlagName = "team" passwordFlagName = "password" diff --git a/ee/fleetctl/updates.go b/ee/fleetctl/updates.go index 806f8f1f8c..43249336dc 100644 --- a/ee/fleetctl/updates.go +++ b/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") diff --git a/ee/server/licensing/licensing.go b/ee/server/licensing/licensing.go index c8a39d3998..cdac5af8f6 100644 --- a/ee/server/licensing/licensing.go +++ b/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. diff --git a/pkg/secure/secure.go b/pkg/secure/secure.go index f202ccebc6..9ec4d993ef 100644 --- a/pkg/secure/secure.go +++ b/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 diff --git a/server/datastore/mysql/carves_test.go b/server/datastore/mysql/carves_test.go index d7237291eb..6bc53776b6 100644 --- a/server/datastore/mysql/carves_test.go +++ b/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) diff --git a/server/datastore/mysql/email_changes_test.go b/server/datastore/mysql/email_changes_test.go index 65388e64e1..582851e393 100644 --- a/server/datastore/mysql/email_changes_test.go +++ b/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 diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index a3eebf8179..653c435652 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/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() diff --git a/server/datastore/mysql/invites_test.go b/server/datastore/mysql/invites_test.go index 2bff3a4ec7..9a04f4a86c 100644 --- a/server/datastore/mysql/invites_test.go +++ b/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) }) diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index a4bcdd1878..633ef078cb 100644 --- a/server/datastore/mysql/labels.go +++ b/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] }) diff --git a/server/datastore/mysql/migrations/tables/20171116163618_CreateTableOsqueryOptions.go b/server/datastore/mysql/migrations/tables/20171116163618_CreateTableOsqueryOptions.go index db9c60f43c..2babcf0750 100644 --- a/server/datastore/mysql/migrations/tables/20171116163618_CreateTableOsqueryOptions.go +++ b/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") } diff --git a/server/datastore/mysql/migrations/tables/20210601000008_TeamsEnrollSecrets.go b/server/datastore/mysql/migrations/tables/20210601000008_TeamsEnrollSecrets.go index d431819914..6dc4464119 100644 --- a/server/datastore/mysql/migrations/tables/20210601000008_TeamsEnrollSecrets.go +++ b/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") diff --git a/server/datastore/mysql/migrations/tables/20210816141251_AddJSONKeyValueTable.go b/server/datastore/mysql/migrations/tables/20210816141251_AddJSONKeyValueTable.go index f7f56bf1da..a429cc9f3a 100644 --- a/server/datastore/mysql/migrations/tables/20210816141251_AddJSONKeyValueTable.go +++ b/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, diff --git a/server/datastore/mysql/migrations/tables/20210818151827_RemoveForeignKeysSchedQStats.go b/server/datastore/mysql/migrations/tables/20210818151827_RemoveForeignKeysSchedQStats.go index ba5b496596..cb88c10d35 100644 --- a/server/datastore/mysql/migrations/tables/20210818151827_RemoveForeignKeysSchedQStats.go +++ b/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") } diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go index f29439ff89..b8915ce7c1 100644 --- a/server/datastore/mysql/mysql.go +++ b/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 diff --git a/server/datastore/mysql/scheduled_queries.go b/server/datastore/mysql/scheduled_queries.go index b4d2d5386b..15b600ebaf 100644 --- a/server/datastore/mysql/scheduled_queries.go +++ b/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") } diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 1934611211..b9e3d0f19b 100644 --- a/server/datastore/mysql/software.go +++ b/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") } diff --git a/server/datastore/mysql/statistics.go b/server/datastore/mysql/statistics.go index 8624a0ae3d..2e75fe235d 100644 --- a/server/datastore/mysql/statistics.go +++ b/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) { diff --git a/server/fleet/app.go b/server/fleet/app.go index c25ddb0bc9..70d1538944 100644 --- a/server/fleet/app.go +++ b/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. diff --git a/server/fleet/carves.go b/server/fleet/carves.go index 1a371a60ea..bd04c47d72 100644 --- a/server/fleet/carves.go +++ b/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 { diff --git a/server/fleet/import_config_unmarshaler.go b/server/fleet/import_config_unmarshaler.go index e9a4d20642..09027f0a60 100644 --- a/server/fleet/import_config_unmarshaler.go +++ b/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, diff --git a/server/fleet/packs_test.go b/server/fleet/packs_test.go index 6d943981fe..d141cddcfa 100644 --- a/server/fleet/packs_test.go +++ b/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 { diff --git a/server/fleet/users.go b/server/fleet/users.go index 12846dbe05..316fc2645e 100644 --- a/server/fleet/users.go +++ b/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 -} diff --git a/server/logging/filesystem.go b/server/logging/filesystem.go index f9aff5f53e..ac82ade98b 100644 --- a/server/logging/filesystem.go +++ 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 { diff --git a/server/logging/firehose.go b/server/logging/firehose.go index d8550d7e55..af13727d32 100644 --- a/server/logging/firehose.go +++ b/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: ®ion, + Region: ®ion, Endpoint: &endpointURL, // empty string or nil will use default values } diff --git a/server/logging/firehose_test.go b/server/logging/firehose_test.go index 4b8381e8c3..229b29e755 100644 --- a/server/logging/firehose_test.go +++ b/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") diff --git a/server/logging/kinesis.go b/server/logging/kinesis.go index 2f2b9e20e7..c67c5fc485 100644 --- a/server/logging/kinesis.go +++ b/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: ®ion, + Region: ®ion, Endpoint: &endpointURL, // empty string or nil will use default values } diff --git a/server/logging/kinesis_test.go b/server/logging/kinesis_test.go index 1b998a1edc..eddb2a9475 100644 --- a/server/logging/kinesis_test.go +++ b/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") diff --git a/server/pubsub/redis_query_results.go b/server/pubsub/redis_query_results.go index 74804d110a..387e848f13 100644 --- a/server/pubsub/redis_query_results.go +++ b/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 diff --git a/server/service/client_errors.go b/server/service/client_errors.go index 81543153d8..b792f8c4fe 100644 --- a/server/service/client_errors.go +++ b/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 } diff --git a/server/service/endpoint_teams.go b/server/service/endpoint_teams.go index 8698ed3148..f7a418effd 100644 --- a/server/service/endpoint_teams.go +++ b/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 } diff --git a/server/service/service_campaigns.go b/server/service/service_campaigns.go index a9b50c1574..1c9780fc6d 100644 --- a/server/service/service_campaigns.go +++ b/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 } diff --git a/server/service/service_osquery.go b/server/service/service_osquery.go index eacf6e9ae2..22f437d350 100644 --- a/server/service/service_osquery.go +++ b/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()} } diff --git a/server/service/service_sessions.go b/server/service/service_sessions.go index 232bb4d274..1e18fdc153 100644 --- a/server/service/service_sessions.go +++ b/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") } diff --git a/server/service/service_sessions_test.go b/server/service/service_sessions_test.go index 4868c8d859..be31ef550d 100644 --- a/server/service/service_sessions_test.go +++ b/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 -} diff --git a/server/service/service_users_test.go b/server/service/service_users_test.go index c1bcbda42c..da5407210b 100644 --- a/server/service/service_users_test.go +++ b/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() diff --git a/server/service/testing_utils.go b/server/service/testing_utils.go index 9ba014b0e1..e9b75cc538 100644 --- a/server/service/testing_utils.go +++ b/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 { diff --git a/server/service/transport.go b/server/service/transport.go index 38f07f61db..469f0bd615 100644 --- a/server/service/transport.go +++ b/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 -} diff --git a/server/service/transport_error.go b/server/service/transport_error.go index 212f8e8f1d..4c0ca37d9a 100644 --- a/server/service/transport_error.go +++ b/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) diff --git a/server/service/validation_app_config.go b/server/service/validation_app_config.go index 45382d8dc6..b430033734 100644 --- a/server/service/validation_app_config.go +++ b/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 == "" {