From 70a61b86f1c9b6b0f7c492228d35b13a0ee298f6 Mon Sep 17 00:00:00 2001 From: gillespi314 <73313222+gillespi314@users.noreply.github.com> Date: Mon, 18 Apr 2022 16:19:58 -0500 Subject: [PATCH] Update os version aggregated stats for all teams (#5083) --- cmd/fleet/cron.go | 5 +- .../OperatingSystems/OperatingSystems.tsx | 6 +-- server/datastore/mysql/hosts.go | 28 ++++++++-- server/datastore/mysql/hosts_test.go | 15 ++++++ server/service/hosts.go | 17 ++++-- server/service/hosts_test.go | 54 +++++++++++++++++++ 6 files changed, 111 insertions(+), 14 deletions(-) diff --git a/cmd/fleet/cron.go b/cmd/fleet/cron.go index a69a7bbfb9..9b53a7ff8c 100644 --- a/cmd/fleet/cron.go +++ b/cmd/fleet/cron.go @@ -208,7 +208,6 @@ func cronVulnerabilities( recentVulns := checkVulnerabilities(ctx, ds, logger, vulnPath, config, vulnAutomationEnabled) if vulnAutomationEnabled && len(recentVulns) > 0 { if appConfig.WebhookSettings.VulnerabilitiesWebhook.Enable { - // send recent vulnerabilities via webhook if err := webhooks.TriggerVulnerabilitiesWebhook(ctx, ds, kitlog.With(logger, "webhook", "vulnerabilities"), recentVulns, appConfig, time.Now()); err != nil { @@ -217,7 +216,6 @@ func cronVulnerabilities( sentry.CaptureException(err) } } else { - // queue job to create jira issues if err := worker.QueueJiraJobs( ctx, @@ -252,7 +250,8 @@ func cronVulnerabilities( } func checkVulnerabilities(ctx context.Context, ds fleet.Datastore, logger kitlog.Logger, - vulnPath string, config config.FleetConfig, collectRecentVulns bool) map[string][]string { + vulnPath string, config config.FleetConfig, collectRecentVulns bool, +) map[string][]string { err := vulnerabilities.TranslateSoftwareToCPE(ctx, ds, vulnPath, logger, config) if err != nil { level.Error(logger).Log("msg", "analyzing vulnerable software: Software->CPE", "err", err) diff --git a/frontend/pages/Homepage/cards/OperatingSystems/OperatingSystems.tsx b/frontend/pages/Homepage/cards/OperatingSystems/OperatingSystems.tsx index 90e9e04ac3..7890efe9e7 100644 --- a/frontend/pages/Homepage/cards/OperatingSystems/OperatingSystems.tsx +++ b/frontend/pages/Homepage/cards/OperatingSystems/OperatingSystems.tsx @@ -36,7 +36,7 @@ const EmptyOperatingSystems = (platform: IOsqueryPlatform): JSX.Element => ( } operating systems detected`}

{`Did you add ${`${PLATFORM_DISPLAY_NAMES[platform]} ` || ""}hosts to - Fleet? Try again in a few seconds as the system catches up.`} + Fleet? Try again in about an hour as the system catches up.`}

); @@ -89,10 +89,6 @@ const OperatingSystems = ({ // Renders opaque information as host information is loading const opacity = showOperatingSystemsUI ? { opacity: 1 } : { opacity: 0 }; - // TODO: Error states? Product says if any card on homepage fails the whole page should 500. Is - // that really what we want to happen? Do we want that to happen always? What if just one card - // fails? What if platform or team filter is applied? - // Currenly none of the homepage cards behave this way AFAICT. return (
{!showOperatingSystemsUI && ( diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 258cf9b082..bde61e5000 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -1626,7 +1626,11 @@ WHERE args = append(args, *teamID) } - if err := sqlx.GetContext(ctx, ds.reader, &row, query, args...); err != nil { + err := sqlx.GetContext(ctx, ds.reader, &row, query, args...) + if err != nil { + if err == sql.ErrNoRows { + return nil, ctxerr.Wrap(ctx, notFound("OSVersions")) + } return nil, err } @@ -1659,8 +1663,10 @@ WHERE return osVersions, nil } +// Aggregated stats for os versions are stored by team id with 0 representing the global case +// If existing team has no hosts, we explicity set the json value as an empty array func (ds *Datastore) UpdateOSVersions(ctx context.Context) error { - query := ` + sql := ` INSERT INTO aggregated_stats (id, type, json_value) SELECT team_id id, @@ -1703,11 +1709,27 @@ FROM ) as team_os_versions GROUP BY team_id +UNION +SELECT + t.id, + 'os_versions' type, + JSON_ARRAY() json_value +FROM + teams t +WHERE NOT EXISTS ( + SELECT + id + FROM + hosts h + WHERE + t.id = h.team_id +) ON DUPLICATE KEY UPDATE json_value = VALUES(json_value), updated_at = CURRENT_TIMESTAMP ` - _, err := ds.writer.ExecContext(ctx, query) + + _, err := ds.writer.ExecContext(ctx, sql) if err != nil { return ctxerr.Wrapf(ctx, err, "update aggregated stats for os versions") } diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index 6a67ca5f81..dd3c675b17 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -3793,6 +3793,11 @@ func testOSVersions(t *testing.T, ds *Datastore) { }) require.NoError(t, err) + team3, err := ds.NewTeam(context.Background(), &fleet.Team{ + Name: "team3", + }) + require.NoError(t, err) + // create some hosts for testing hosts := []*fleet.Host{ { @@ -3909,6 +3914,16 @@ func testOSVersions(t *testing.T, ds *Datastore) { {HostsCount: 3, Name: "macOS 12.3.0", Platform: "darwin"}, } require.Equal(t, expected, osVersions.OSVersions) + + // team 3 (no hosts assigned to team) + osVersions, err = ds.OSVersions(ctx, &team3.ID, nil) + require.NoError(t, err) + expected = []fleet.OSVersion{} + require.Equal(t, expected, osVersions.OSVersions) + + // non-existent team + osVersions, err = ds.OSVersions(ctx, ptr.Uint(404), nil) + require.Error(t, err) } func testHostsDeleteHosts(t *testing.T, ds *Datastore) { diff --git a/server/service/hosts.go b/server/service/hosts.go index 664dd00628..283e72bc71 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -899,8 +899,8 @@ type osVersionsRequest struct { } type osVersionsResponse struct { - CountsUpdatedAt *time.Time `json:"counts_updated_at,omitempty"` - OSVersions []fleet.OSVersion `json:"os_versions,omitempty"` + CountsUpdatedAt *time.Time `json:"counts_updated_at"` + OSVersions []fleet.OSVersion `json:"os_versions"` Err error `json:"error,omitempty"` } @@ -926,7 +926,18 @@ func (svc *Service) OSVersions(ctx context.Context, teamID *uint, platform *stri } osVersions, err := svc.ds.OSVersions(ctx, teamID, platform) - if err != nil { + if err != nil && fleet.IsNotFound(err) { + // differentiate case where team was added after UpdateOSVersions last ran + if teamID != nil { + // most of the time, team should exist so checking here saves unnecessary db calls + _, err := svc.ds.Team(ctx, *teamID) + if err != nil { + return nil, err + } + } + // if team exists but stats have not yet been gathered, return empty JSON array + osVersions = &fleet.OSVersions{} + } else if err != nil { return nil, err } diff --git a/server/service/hosts_test.go b/server/service/hosts_test.go index af633a5e9f..1d7b226561 100644 --- a/server/service/hosts_test.go +++ b/server/service/hosts_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "fmt" "testing" "time" @@ -428,3 +429,56 @@ func TestRefetchHostUserInTeams(t *testing.T) { assert.True(t, ds.HostLiteFuncInvoked) assert.True(t, ds.UpdateHostRefetchRequestedFuncInvoked) } + +func TestEmptyTeamOSVersions(t *testing.T) { + ds := new(mock.Store) + svc := newTestService(t, ds, nil, nil) + + testVersions := []fleet.OSVersion{{HostsCount: 1, Name: "macOS 12.1", Platform: "darwin"}} + + ds.TeamFunc = func(ctx context.Context, teamID uint) (*fleet.Team, error) { + if teamID == 1 { + return &fleet.Team{ + Name: "team1", + }, nil + } + if teamID == 2 { + return &fleet.Team{ + Name: "team2", + }, nil + } + + return nil, notFoundError{} + } + + ds.OSVersionsFunc = func(ctx context.Context, teamID *uint, platform *string) (*fleet.OSVersions, error) { + if *teamID == 1 { + return &fleet.OSVersions{CountsUpdatedAt: time.Now(), OSVersions: testVersions}, nil + } + if *teamID == 4 { + return nil, fmt.Errorf("some unknown error") + } + + return nil, notFoundError{} + } + + // team exists with stats + vers, err := svc.OSVersions(test.UserContext(test.UserAdmin), ptr.Uint(1), ptr.String("darwin")) + require.NoError(t, err) + assert.Len(t, vers.OSVersions, 1) + + // team exists but no stats + vers, err = svc.OSVersions(test.UserContext(test.UserAdmin), ptr.Uint(2), ptr.String("darwin")) + require.NoError(t, err) + assert.Empty(t, vers.OSVersions) + + // team does not exist + vers, err = svc.OSVersions(test.UserContext(test.UserAdmin), ptr.Uint(3), ptr.String("darwin")) + require.Error(t, err) + require.Equal(t, "not found", fmt.Sprint(err)) + + // some unknown error + vers, err = svc.OSVersions(test.UserContext(test.UserAdmin), ptr.Uint(4), ptr.String("darwin")) + require.Error(t, err) + require.Equal(t, "some unknown error", fmt.Sprint(err)) +}