Update os version aggregated stats for all teams (#5083)

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)

frontend/pages/Homepage/cards/OperatingSystems/OperatingSystems.tsx

@@ -36,7 +36,7 @@ const EmptyOperatingSystems = (platform: IOsqueryPlatform): JSX.Element => (
     } operating systems detected`}</h1>
     <p>
       {`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.`}
     </p>
   </div>
 );
@@ -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 (
     <div className={baseClass}>
       {!showOperatingSystemsUI && (

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")
 	}

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) {

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
 	}
 

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))
+}