From 1fca8b1e386aba4adbb8775998a4b37df18ebaa9 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Wed, 15 Nov 2023 16:42:57 -0500 Subject: [PATCH] fix: sort order for Last restarted (#14878) # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Documented any API changes (docs/REST API/rest-api.md or docs/Contributing/API-for-contributors.md) - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Rachael Shaw --- changes/13160-sort-order | 2 + .../expectedHostDetailResponseJson.json | 3 +- .../expectedHostDetailResponseYaml.yml | 1 + .../testdata/expectedListHostsJson.json | 4 +- .../testdata/expectedListHostsMDM.json | 4 +- .../testdata/expectedListHostsYaml.yml | 2 + docs/REST API/rest-api.md | 3 + frontend/__mocks__/hostMock.ts | 1 + frontend/interfaces/host.ts | 2 + .../hosts/ManageHostsPage/HostTableConfig.tsx | 7 +- .../hosts/ManageHostsPage/ManageHostsPage.tsx | 10 ++- .../HostDetailsPage/HostDetailsPage.tsx | 1 + .../pages/hosts/details/cards/About/About.tsx | 6 +- frontend/utilities/helpers.tsx | 34 --------- server/datastore/mysql/hosts.go | 4 +- server/datastore/mysql/hosts_test.go | 70 +++++++++++++++++++ server/fleet/hosts.go | 3 + server/service/integration_core_test.go | 6 +- 18 files changed, 112 insertions(+), 51 deletions(-) create mode 100644 changes/13160-sort-order diff --git a/changes/13160-sort-order b/changes/13160-sort-order new file mode 100644 index 000000000..0470db245 --- /dev/null +++ b/changes/13160-sort-order @@ -0,0 +1,2 @@ +- Fixed an edge case sorting bug by consolidating the logic for generating the `last_restarted` + value for hosts into the backend. diff --git a/cmd/fleetctl/testdata/expectedHostDetailResponseJson.json b/cmd/fleetctl/testdata/expectedHostDetailResponseJson.json index 2d16a9670..0c96f0f9d 100644 --- a/cmd/fleetctl/testdata/expectedHostDetailResponseJson.json +++ b/cmd/fleetctl/testdata/expectedHostDetailResponseJson.json @@ -9,6 +9,7 @@ "label_updated_at": "0001-01-01T00:00:00Z", "policy_updated_at": "0001-01-01T00:00:00Z", "last_enrolled_at": "0001-01-01T00:00:00Z", + "last_restarted_at": "0001-01-01T00:00:00Z", "seen_time": "0001-01-01T00:00:00Z", "software_updated_at": "0001-01-01T00:00:00Z", "refetch_requested": false, @@ -93,4 +94,4 @@ "display_text": "test_host", "display_name": "test_host" } -} +} \ No newline at end of file diff --git a/cmd/fleetctl/testdata/expectedHostDetailResponseYaml.yml b/cmd/fleetctl/testdata/expectedHostDetailResponseYaml.yml index a862a283b..af0fd5000 100644 --- a/cmd/fleetctl/testdata/expectedHostDetailResponseYaml.yml +++ b/cmd/fleetctl/testdata/expectedHostDetailResponseYaml.yml @@ -28,6 +28,7 @@ spec: label_updated_at: "0001-01-01T00:00:00Z" labels: [] last_enrolled_at: "0001-01-01T00:00:00Z" + last_restarted_at: "0001-01-01T00:00:00Z" logger_tls_period: 0 mdm: encryption_key_available: false diff --git a/cmd/fleetctl/testdata/expectedListHostsJson.json b/cmd/fleetctl/testdata/expectedListHostsJson.json index b09381bb5..f24053db2 100644 --- a/cmd/fleetctl/testdata/expectedListHostsJson.json +++ b/cmd/fleetctl/testdata/expectedListHostsJson.json @@ -8,6 +8,7 @@ "detail_updated_at": "0001-01-01T00:00:00Z", "label_updated_at": "0001-01-01T00:00:00Z", "last_enrolled_at": "0001-01-01T00:00:00Z", + "last_restarted_at": "0001-01-01T00:00:00Z", "seen_time": "0001-01-01T00:00:00Z", "software_updated_at": "0001-01-01T00:00:00Z", "refetch_requested": false, @@ -77,6 +78,7 @@ "detail_updated_at": "0001-01-01T00:00:00Z", "label_updated_at": "0001-01-01T00:00:00Z", "last_enrolled_at": "0001-01-01T00:00:00Z", + "last_restarted_at": "0001-01-01T00:00:00Z", "seen_time": "0001-01-01T00:00:00Z", "software_updated_at": "0001-01-01T00:00:00Z", "refetch_requested": false, @@ -127,4 +129,4 @@ "status": "offline", "display_text": "test_host2" } -} +} \ No newline at end of file diff --git a/cmd/fleetctl/testdata/expectedListHostsMDM.json b/cmd/fleetctl/testdata/expectedListHostsMDM.json index 2598cb357..e729ace58 100644 --- a/cmd/fleetctl/testdata/expectedListHostsMDM.json +++ b/cmd/fleetctl/testdata/expectedListHostsMDM.json @@ -9,6 +9,7 @@ "detail_updated_at": "0001-01-01T00:00:00Z", "label_updated_at": "0001-01-01T00:00:00Z", "last_enrolled_at": "0001-01-01T00:00:00Z", + "last_restarted_at": "0001-01-01T00:00:00Z", "seen_time": "0001-01-01T00:00:00Z", "software_updated_at": "0001-01-01T00:00:00Z", "refetch_requested": false, @@ -78,6 +79,7 @@ "detail_updated_at": "0001-01-01T00:00:00Z", "label_updated_at": "0001-01-01T00:00:00Z", "last_enrolled_at": "0001-01-01T00:00:00Z", + "last_restarted_at": "0001-01-01T00:00:00Z", "seen_time": "0001-01-01T00:00:00Z", "software_updated_at": "0001-01-01T00:00:00Z", "refetch_requested": false, @@ -129,4 +131,4 @@ "display_text": "test_host2" } } -] +] \ No newline at end of file diff --git a/cmd/fleetctl/testdata/expectedListHostsYaml.yml b/cmd/fleetctl/testdata/expectedListHostsYaml.yml index cb924c635..c26945a16 100644 --- a/cmd/fleetctl/testdata/expectedListHostsYaml.yml +++ b/cmd/fleetctl/testdata/expectedListHostsYaml.yml @@ -32,6 +32,7 @@ spec: total_issues_count: 0 label_updated_at: "0001-01-01T00:00:00Z" last_enrolled_at: "0001-01-01T00:00:00Z" + last_restarted_at: "0001-01-01T00:00:00Z" logger_tls_period: 0 mdm: encryption_key_available: false @@ -88,6 +89,7 @@ spec: total_issues_count: 0 label_updated_at: "0001-01-01T00:00:00Z" last_enrolled_at: "0001-01-01T00:00:00Z" + last_restarted_at: "0001-01-01T00:00:00Z" logger_tls_period: 0 mdm: encryption_key_available: false diff --git a/docs/REST API/rest-api.md b/docs/REST API/rest-api.md index c52c44da9..6cd0fbfee 100644 --- a/docs/REST API/rest-api.md +++ b/docs/REST API/rest-api.md @@ -1836,6 +1836,7 @@ the `software` table. - `policy_updated_at`: the last time we updated the policy results for the host based on the queries ran. - `seen_time`: the last time the host contacted the fleet server, regardless of what operation it was for. - `software_updated_at`: the last time software changed for the host in any way. +- `last_restarted_at`: the last time that the host was restarted. ### List hosts @@ -1912,6 +1913,7 @@ If `after` is being used with `created_at` or `updated_at`, the table must be sp "updated_at": "2020-11-05T06:03:39Z", "id": 1, "detail_updated_at": "2020-11-05T05:09:45Z", + "last_restarted_at": "2020-11-01T03:01:45Z", "software_updated_at": "2020-11-05T05:09:44Z", "label_updated_at": "2020-11-05T05:14:51Z", "policy_updated_at": "2023-06-26T18:33:15Z", @@ -2221,6 +2223,7 @@ Returns the information of the specified host. ], "id": 1, "detail_updated_at": "2021-08-19T21:07:53Z", + "last_restarted_at": "2020-11-01T03:01:45Z", "software_updated_at": "2020-11-05T05:09:44Z", "label_updated_at": "2021-08-19T21:07:53Z", "policy_updated_at": "2023-06-26T18:33:15Z", diff --git a/frontend/__mocks__/hostMock.ts b/frontend/__mocks__/hostMock.ts index e415c944e..36c969987 100644 --- a/frontend/__mocks__/hostMock.ts +++ b/frontend/__mocks__/hostMock.ts @@ -20,6 +20,7 @@ const DEFAULT_HOST_MOCK: IHost = { created_at: "2022-01-01T12:00:00Z", updated_at: "2022-01-02T12:00:00Z", detail_updated_at: "2022-01-02T12:00:00Z", + last_restarted_at: "2022-01-02T12:00:00Z", label_updated_at: "2022-01-02T12:00:00Z", policy_updated_at: "2022-01-02T12:00:00Z", last_enrolled_at: "2022-01-02T12:00:00Z", diff --git a/frontend/interfaces/host.ts b/frontend/interfaces/host.ts index bdfa84be1..fe741fcf7 100644 --- a/frontend/interfaces/host.ts +++ b/frontend/interfaces/host.ts @@ -19,6 +19,7 @@ export default PropTypes.shape({ updated_at: PropTypes.string, id: PropTypes.number, detail_updated_at: PropTypes.string, + last_restarted_at: PropTypes.string, label_updated_at: PropTypes.string, policy_updated_at: PropTypes.string, last_enrolled_at: PropTypes.string, @@ -199,6 +200,7 @@ export interface IHost { updated_at: string; id: number; detail_updated_at: string; + last_restarted_at: string; label_updated_at: string; policy_updated_at: string; last_enrolled_at: string; diff --git a/frontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx b/frontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx index d2c5f511b..dfaecd155 100644 --- a/frontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx +++ b/frontend/pages/hosts/ManageHostsPage/HostTableConfig.tsx @@ -21,7 +21,6 @@ import NotSupported from "components/NotSupported"; import { humanHostMemory, - humanHostLastRestart, humanHostLastSeen, hostTeamName, } from "utilities/helpers"; @@ -568,9 +567,9 @@ const allHostTableHeaders: IDataColumn[] = [ isSortedDesc={cellProps.column.isSortedDesc} /> ), - accessor: "uptime", + accessor: "last_restarted_at", Cell: (cellProps: ICellProps) => { - const { uptime, detail_updated_at, platform } = cellProps.row.original; + const { platform, last_restarted_at } = cellProps.row.original; if (platform === "chrome") { return NotSupported; @@ -578,7 +577,7 @@ const allHostTableHeaders: IDataColumn[] = [ return ( diff --git a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx index 677536b62..385138ad1 100644 --- a/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx +++ b/frontend/pages/hosts/ManageHostsPage/ManageHostsPage.tsx @@ -729,10 +729,18 @@ const ManageHostsPage = ({ let sort = sortBy; if (sortHeader) { + let direction = sortDirection; + if (sortHeader === "last_restarted_at") { + if (sortDirection === "asc") { + direction = "desc"; + } else { + direction = "asc"; + } + } sort = [ { key: sortHeader, - direction: sortDirection || DEFAULT_SORT_DIRECTION, + direction: direction || DEFAULT_SORT_DIRECTION, }, ]; } else if (!sortBy.length) { diff --git a/frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx b/frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx index 84041e073..3ecaa2f3a 100644 --- a/frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx +++ b/frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx @@ -399,6 +399,7 @@ const HostDetailsPage = ({ "geolocation", "batteries", "detail_updated_at", + "last_restarted_at", ]) ); diff --git a/frontend/pages/hosts/details/cards/About/About.tsx b/frontend/pages/hosts/details/cards/About/About.tsx index f9de0d032..f1c30de2d 100644 --- a/frontend/pages/hosts/details/cards/About/About.tsx +++ b/frontend/pages/hosts/details/cards/About/About.tsx @@ -6,7 +6,6 @@ import TooltipWrapper from "components/TooltipWrapper"; import CustomLink from "components/CustomLink"; import { IHostMdmData, IMunkiData, IDeviceUser } from "interfaces/host"; -import { humanHostLastRestart } from "utilities/helpers"; import { DEFAULT_EMPTY_CELL_VALUE, MDM_STATUS_TOOLTIP, @@ -209,10 +208,7 @@ const About = ({ Last restarted diff --git a/frontend/utilities/helpers.tsx b/frontend/utilities/helpers.tsx index 11243d7e8..c255d4eed 100644 --- a/frontend/utilities/helpers.tsx +++ b/frontend/utilities/helpers.tsx @@ -547,40 +547,6 @@ export const inMilliseconds = (nanoseconds: number): number => { return nanoseconds / NANOSECONDS_PER_MILLISECOND; }; -export const humanHostLastRestart = ( - detailUpdatedAt: string, - uptime: number | string -): string => { - if ( - !detailUpdatedAt || - !uptime || - detailUpdatedAt === DEFAULT_EMPTY_CELL_VALUE || - detailUpdatedAt < INITIAL_FLEET_DATE || - typeof uptime !== "number" - ) { - return "Unavailable"; - } - try { - const currentDate = new Date(); - const updatedDate = new Date(detailUpdatedAt); - const millisecondsLastUpdated = - currentDate.getTime() - updatedDate.getTime(); - - // Sum of calculated milliseconds since last updated with uptime - const millisecondsLastRestart = - millisecondsLastUpdated + uptime / NANOSECONDS_PER_MILLISECOND; - - const restartDate = new Date(); - restartDate.setMilliseconds( - restartDate.getMilliseconds() - millisecondsLastRestart - ); - - return restartDate.toISOString(); - } catch { - return "Unavailable"; - } -}; - export const humanHostLastSeen = (lastSeen: string): string => { if (!lastSeen || lastSeen < INITIAL_FLEET_DATE) { return "Never"; diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 03638c993..434599b91 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -579,6 +579,7 @@ SELECT COALESCE(hst.seen_time, h.created_at) AS seen_time, t.name AS team_name, COALESCE(hu.software_updated_at, h.created_at) AS software_updated_at, + (CASE WHEN uptime = 0 THEN DATE('0001-01-01') ELSE DATE_SUB(h.detail_updated_at, INTERVAL uptime/1000 MICROSECOND) END) as last_restarted_at, ( SELECT additional @@ -848,7 +849,8 @@ func (ds *Datastore) ListHosts(ctx context.Context, filter fleet.TeamFilter, opt COALESCE(hd.percent_disk_space_available, 0) as percent_disk_space_available, COALESCE(hst.seen_time, h.created_at) AS seen_time, t.name AS team_name, - COALESCE(hu.software_updated_at, h.created_at) AS software_updated_at + COALESCE(hu.software_updated_at, h.created_at) AS software_updated_at, + (CASE WHEN uptime = 0 THEN DATE('0001-01-01') ELSE DATE_SUB(h.detail_updated_at, INTERVAL uptime/1000 MICROSECOND) END) as last_restarted_at ` sql += hostMDMSelect diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index 1c9f1a047..22d76e62b 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -154,6 +154,7 @@ func TestHosts(t *testing.T) { {"GetMatchingHostSerials", testGetMatchingHostSerials}, {"ListHostsLiteByIDs", testHostsListHostsLiteByIDs}, {"ListHostsWithPagination", testListHostsWithPagination}, + {"LastRestarted", testLastRestarted}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -7530,3 +7531,72 @@ func testListHostsWithPagination(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Equal(t, hostCount, count) } + +func testLastRestarted(t *testing.T, ds *Datastore) { + ctx := context.Background() + + // Arbitrary value + const uptimeVal = 16691000000000 + now := time.Now() + newHostFunc := func(name string, uptimeZero bool) (*fleet.Host, time.Time) { + newHost := &fleet.Host{ + DetailUpdatedAt: now, + LabelUpdatedAt: now, + PolicyUpdatedAt: now, + SeenTime: now, + NodeKey: ptr.String(name), + UUID: name, + Hostname: "foo.local." + name, + } + + var expectedLastRestartedAt time.Time + + if uptimeZero { + newHost.Uptime = 0 + } else { + newHost.Uptime = uptimeVal + // Rounding to nearest second because the SQL query does integer division. + expectedLastRestartedAt = newHost.DetailUpdatedAt.Add(-newHost.Uptime).Round(time.Second).UTC() + } + + host, err := ds.NewHost(ctx, newHost) + require.NoError(t, err) + require.NotNil(t, host) + return host, expectedLastRestartedAt + } + + hostCount := 10 + hosts := make([]*fleet.Host, 0, hostCount) + hostsToVals := make(map[uint]time.Time, 0) + for i := 0; i < hostCount; i++ { + nh, expectedVal := newHostFunc(fmt.Sprintf("h%d", i), i%2 == 0) + hosts = append(hosts, nh) + hostsToVals[nh.ID] = expectedVal + } + + opts := fleet.HostListOptions{} + + userFilter := fleet.TeamFilter{User: test.UserAdmin} + + returnedHosts := listHostsCheckCount(t, ds, userFilter, opts, len(hosts)) + + for i, h := range returnedHosts { + require.Equal(t, hosts[i].Uptime, h.Uptime) + require.Equal(t, hostsToVals[h.ID], h.LastRestartedAt) + } + + h1 := hosts[0] // has Uptime == 0 + h2 := hosts[1] // has Uptime == uptimeVal + + host, err := ds.Host(ctx, h1.ID) + require.NoError(t, err) + require.Equal(t, h1.ID, host.ID) + require.Equal(t, time.Duration(0), host.Uptime) + require.Equal(t, hostsToVals[host.ID], host.LastRestartedAt) + + host, err = ds.Host(ctx, h2.ID) + require.NoError(t, err) + require.Equal(t, h2.ID, host.ID) + require.Equal(t, time.Duration(uptimeVal), host.Uptime) + require.Equal(t, hostsToVals[host.ID], host.LastRestartedAt) +} diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index a981d577b..db4f4bda7 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -316,6 +316,9 @@ type Host struct { // The boolean is based on information ingested from the Apple DEP API that is stored in the // host_dep_assignments table. DEPAssignedToFleet *bool `json:"dep_assigned_to_fleet,omitempty" db:"dep_assigned_to_fleet" csv:"-"` + + // LastRestartedAt is a UNIX timestamp that indicates when the Host was last restarted. + LastRestartedAt time.Time `json:"last_restarted_at" db:"last_restarted_at"` } type MDMHostData struct { diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 8e7bbef51..4f974ab02 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -6326,7 +6326,7 @@ func (s *integrationTestSuite) TestHostsReportDownload() { res.Body.Close() require.NoError(t, err) require.Len(t, rows, len(hosts)+1) // all hosts + header row - require.Len(t, rows[0], 48) // total number of cols + require.Len(t, rows[0], 49) // total number of cols const ( idCol = 3 @@ -8029,7 +8029,7 @@ func (s *integrationTestSuite) TestHostsReportWithPolicyResults() { res.Body.Close() require.NoError(t, err) require.Len(t, rows1, len(hosts)+1) // all hosts + header row - require.Len(t, rows1[0], 48) // total number of cols + require.Len(t, rows1[0], 49) // total number of cols var ( idIdx int @@ -8056,7 +8056,7 @@ func (s *integrationTestSuite) TestHostsReportWithPolicyResults() { res.Body.Close() require.NoError(t, err) require.Len(t, rows2, len(hosts)+1) // all hosts + header row - require.Len(t, rows2[0], 48) // total number of cols + require.Len(t, rows2[0], 49) // total number of cols // Check that all hosts have 0 issues and that they match the previous call to `/hosts/report`. for i := 1; i < len(hosts)+1; i++ {