From 8d416b56eb645cd0a10897f9301db1231840d2a7 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Thu, 16 Jan 2025 17:24:17 -0500 Subject: [PATCH 1/9] Begin work to fix upcoming activities for ABM-deleted hosts --- server/datastore/mysql/activities.go | 16 +++++++++----- server/datastore/mysql/scripts.go | 3 ++- server/datastore/mysql/software_installers.go | 22 ++++++++++--------- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/server/datastore/mysql/activities.go b/server/datastore/mysql/activities.go index dfb2bb57b141..bb2f38104513 100644 --- a/server/datastore/mysql/activities.go +++ b/server/datastore/mysql/activities.go @@ -255,6 +255,7 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint LEFT OUTER JOIN host_software_installs hsi ON hsi.execution_id = hsr.execution_id WHERE hsr.host_id = :host_id AND + hsr.host_deleted_at IS NULL AND exit_code IS NULL AND hsi.execution_id IS NULL AND (sync_request = 0 OR hsr.created_at >= DATE_SUB(NOW(), INTERVAL :max_wait_time SECOND))`, @@ -262,11 +263,13 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint COUNT(*) c FROM host_software_installs hsi WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND + hsi.host_deleted_at IS NULL AND hsi.status = :software_status_install_pending`, `SELECT COUNT(*) c FROM host_software_installs hsi WHERE hsi.host_id = :host_id AND hsi.software_installer_id IS NOT NULL AND + hsi.host_deleted_at IS NULL AND hsi.status = :software_status_uninstall_pending`, ` SELECT @@ -334,6 +337,7 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint setup_experience_scripts ses ON ses.id = hsr.setup_experience_script_id WHERE hsr.host_id = :host_id AND + hsr.host_deleted_at IS NULL AND hsr.exit_code IS NULL AND ( hsr.sync_request = 0 OR @@ -377,6 +381,7 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint host_display_names hdn ON hdn.host_id = hsi.host_id WHERE hsi.host_id = :host_id AND + hsi.host_deleted_at IS NULL AND hsi.status = :software_status_install_pending `, // list pending software uninstalls @@ -413,6 +418,7 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint host_display_names hdn ON hdn.host_id = hsi.host_id WHERE hsi.host_id = :host_id AND + hsi.host_deleted_at IS NULL AND hsi.status = :software_status_uninstall_pending `, // list pending VPP installs @@ -439,15 +445,15 @@ SELECT ) AS details FROM host_vpp_software_installs hvsi -INNER JOIN +INNER JOIN nano_view_queue nvq ON nvq.command_uuid = hvsi.command_uuid -LEFT OUTER JOIN +LEFT OUTER JOIN users u ON hvsi.user_id = u.id -LEFT OUTER JOIN +LEFT OUTER JOIN host_display_names hdn ON hdn.host_id = hvsi.host_id -LEFT OUTER JOIN +LEFT OUTER JOIN vpp_apps vpa ON hvsi.adam_id = vpa.adam_id AND hvsi.platform = vpa.platform -LEFT OUTER JOIN +LEFT OUTER JOIN software_titles st ON st.id = vpa.title_id WHERE nvq.status IS NULL diff --git a/server/datastore/mysql/scripts.go b/server/datastore/mysql/scripts.go index 53d162d524cf..c0c5280823b2 100644 --- a/server/datastore/mysql/scripts.go +++ b/server/datastore/mysql/scripts.go @@ -241,6 +241,7 @@ func (ds *Datastore) ListPendingHostScriptExecutions(ctx context.Context, hostID host_script_results WHERE host_id = ? AND + host_deleted_at IS NULL AND %s %s ORDER BY @@ -504,7 +505,7 @@ func (ds *Datastore) deletePendingHostScriptExecutionsForPolicy(ctx context.Cont deleteStmt := fmt.Sprintf(` DELETE FROM host_script_results - WHERE + WHERE policy_id = ? AND script_id IN ( SELECT id FROM scripts WHERE scripts.global_or_team_id = ? diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index dd7a0aa091dc..3b71bee2d5cc 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -25,6 +25,8 @@ func (ds *Datastore) ListPendingSoftwareInstalls(ctx context.Context, hostID uin host_software_installs WHERE host_id = ? + AND + host_deleted_at IS NULL AND status = ? ORDER BY @@ -1709,24 +1711,24 @@ func (ds *Datastore) IsSoftwareInstallerLabelScoped(ctx context.Context, install UNION - -- exclude any, ignore software that depends on labels created - -- _after_ the label_updated_at timestamp of the host (because - -- we don't have results for that label yet, the host may or may + -- exclude any, ignore software that depends on labels created + -- _after_ the label_updated_at timestamp of the host (because + -- we don't have results for that label yet, the host may or may -- not be a member). SELECT COUNT(*) AS count_installer_labels, COUNT(lm.label_id) AS count_host_labels, - SUM(CASE - WHEN - lbl.created_at IS NOT NULL AND (SELECT label_updated_at FROM hosts WHERE id = :host_id) >= lbl.created_at THEN 1 - ELSE - 0 + SUM(CASE + WHEN + lbl.created_at IS NOT NULL AND (SELECT label_updated_at FROM hosts WHERE id = :host_id) >= lbl.created_at THEN 1 + ELSE + 0 END) as count_host_updated_after_labels FROM software_installer_labels sil LEFT OUTER JOIN labels lbl ON lbl.id = sil.label_id - LEFT OUTER JOIN label_membership lm + LEFT OUTER JOIN label_membership lm ON lm.label_id = sil.label_id AND lm.host_id = :host_id WHERE sil.software_installer_id = :installer_id @@ -1756,7 +1758,7 @@ func (ds *Datastore) IsSoftwareInstallerLabelScoped(ctx context.Context, install return res, nil } -const labelScopedFilter = ` +const labelScopedFilter = ` SELECT 1 FROM ( From b95d003ecdbcbf001b37aa48bc2d28d5788233d5 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Fri, 17 Jan 2025 11:26:59 -0500 Subject: [PATCH 2/9] Filter host_software_installs list using host_deleted_at --- server/datastore/mysql/software.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 9d929c2289ec..0775ee07e192 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -2259,7 +2259,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id -- get the status of the latest attempt; 27-1 is the length of the timestamp SUBSTRING(MAX(CONCAT(created_at, COALESCE(status, ''))), 27) AS hsi_status FROM host_software_installs - WHERE host_id = :host_id AND removed = 0 + WHERE host_id = :host_id AND removed = 0 AND host_deleted_at IS NULL GROUP BY host_id, software_installer_id UNION -- get latest install attempt @@ -2271,7 +2271,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id NULL as hsi_uninstalled_at, NULL as hsi_uninstall_execution_id, NULL as hsi_status FROM host_software_installs - WHERE host_id = :host_id AND removed = 0 AND uninstall = 0 + WHERE host_id = :host_id AND removed = 0 AND uninstall = 0 AND host_deleted_at IS NULL GROUP BY host_id, software_installer_id UNION -- get latest uninstall attempt @@ -2283,7 +2283,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id SUBSTRING(MAX(CONCAT(created_at, execution_id)), 27) AS hsi_uninstall_execution_id, NULL as hsi_status FROM host_software_installs - WHERE host_id = :host_id AND removed = 0 AND uninstall = 1 + WHERE host_id = :host_id AND removed = 0 AND uninstall = 1 AND host_deleted_at IS NULL GROUP BY host_id, software_installer_id ) as hsi_group GROUP BY hsi_group.host_id, hsi_group.software_installer_id @@ -2311,7 +2311,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id -- on host (via installer or VPP app). If only available for install is -- requested, then the software installed on host clause is empty. ( %s hsi.host_id IS NOT NULL OR hvsi.host_id IS NOT NULL ) - AND + AND -- label membership check ( -- do the label membership check only for software installers @@ -2346,9 +2346,9 @@ INNER JOIN software_cve scve ON scve.software_id = s.id UNION - -- exclude any, ignore software that depends on labels created - -- _after_ the label_updated_at timestamp of the host (because - -- we don't have results for that label yet, the host may or may + -- exclude any, ignore software that depends on labels created + -- _after_ the label_updated_at timestamp of the host (because + -- we don't have results for that label yet, the host may or may -- not be a member). SELECT COUNT(*) AS count_installer_labels, @@ -2358,7 +2358,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id software_installer_labels sil LEFT OUTER JOIN labels lbl ON lbl.id = sil.label_id - LEFT OUTER JOIN label_membership lm + LEFT OUTER JOIN label_membership lm ON lm.label_id = sil.label_id AND lm.host_id = :host_id WHERE sil.software_installer_id = si.id @@ -2473,9 +2473,9 @@ INNER JOIN software_cve scve ON scve.software_id = s.id UNION - -- exclude any, ignore software that depends on labels created - -- _after_ the label_updated_at timestamp of the host (because - -- we don't have results for that label yet, the host may or may + -- exclude any, ignore software that depends on labels created + -- _after_ the label_updated_at timestamp of the host (because + -- we don't have results for that label yet, the host may or may -- not be a member). SELECT COUNT(*) AS count_installer_labels, @@ -2485,7 +2485,7 @@ INNER JOIN software_cve scve ON scve.software_id = s.id software_installer_labels sil LEFT OUTER JOIN labels lbl ON lbl.id = sil.label_id - LEFT OUTER JOIN label_membership lm + LEFT OUTER JOIN label_membership lm ON lm.label_id = sil.label_id AND lm.host_id = :host_id WHERE sil.software_installer_id = si.id From c9a55373891c47da84a16fb6e7f1e1b732d3b1dd Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Fri, 17 Jan 2025 13:42:23 -0500 Subject: [PATCH 3/9] Only check against still-valid pending (un)-install --- server/datastore/mysql/software_installers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 3b71bee2d5cc..2ad8aa37f5cb 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -1022,7 +1022,7 @@ func (ds *Datastore) GetHostLastInstallData(ctx context.Context, hostID, install MAX(id) FROM host_software_installs WHERE - software_installer_id = :installer_id AND host_id = :host_id + software_installer_id = :installer_id AND host_id = :host_id AND host_deleted_at IS NULL GROUP BY host_id, software_installer_id) ` From 1af982086f390bad5cfef1471d1525fa673ede44 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Fri, 17 Jan 2025 16:00:51 -0500 Subject: [PATCH 4/9] Add changes/ --- changes/22353-abm-hosts-upcoming-activities | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/22353-abm-hosts-upcoming-activities diff --git a/changes/22353-abm-hosts-upcoming-activities b/changes/22353-abm-hosts-upcoming-activities new file mode 100644 index 000000000000..ee74914b6d44 --- /dev/null +++ b/changes/22353-abm-hosts-upcoming-activities @@ -0,0 +1 @@ +- Hosts that are restored from ABM no longer have old activities in their feed From e40283888edc5ea3606125f6f561aa3748361a19 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Mon, 20 Jan 2025 11:16:15 -0500 Subject: [PATCH 5/9] Add upcoming activities test --- server/datastore/mysql/activities_test.go | 26 ++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/server/datastore/mysql/activities_test.go b/server/datastore/mysql/activities_test.go index 378c884915c6..567519abcdc1 100644 --- a/server/datastore/mysql/activities_test.go +++ b/server/datastore/mysql/activities_test.go @@ -373,13 +373,15 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { test.CreateInsertGlobalVPPToken(t, ds) - // create three hosts + // create four hosts h1 := test.NewHost(t, ds, "h1.local", "10.10.10.1", "1", "1", time.Now()) nanoEnrollAndSetHostMDMData(t, ds, h1, false) h2 := test.NewHost(t, ds, "h2.local", "10.10.10.2", "2", "2", time.Now()) nanoEnrollAndSetHostMDMData(t, ds, h2, false) h3 := test.NewHost(t, ds, "h3.local", "10.10.10.3", "3", "3", time.Now()) nanoEnrollAndSetHostMDMData(t, ds, h3, false) + h4 := test.NewHost(t, ds, "h4.local", "10.10.10.4", "4", "4", time.Now()) + nanoEnrollAndSetHostMDMData(t, ds, h4, false) // create a couple of named scripts scr1, err := ds.NewScript(ctx, &fleet.Script{ @@ -563,6 +565,22 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { err = ds.DeleteSoftwareInstaller(ctx, sw3Meta.InstallerID) require.NoError(t, err) + // Setup host 4. We will create upcoming activities, then + // delete and "restore" the host, similar to what would happen + // if you delete an ABM DEP host. + hsr, err = ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{HostID: h4.ID, ScriptID: &scr1.ID, ScriptContents: scr1.ScriptContents, UserID: &u.ID}) + require.NoError(t, err) + // h4A := hsr.ExecutionID + // h4Bar, err := ds.InsertSoftwareInstallRequest(ctx, h4.ID, sw2Meta.InstallerID, false, nil) + _, err = ds.InsertSoftwareInstallRequest(ctx, h4.ID, sw2Meta.InstallerID, false, nil) + require.NoError(t, err) + // Delete the host + err = ds.DeleteHost(ctx, h4.ID) + require.NoError(t, err) + // DEP "restore" the host + err = ds.RestoreMDMApplePendingDEPHost(ctx, h4) + require.NoError(t, err) + // force-set the order of the created_at timestamps endTime := SetOrderedCreatedAtTimestamps(t, ds, time.Now(), "host_script_results", "execution_id", h1A, h1B) endTime = SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_software_installs", "execution_id", h1FooFailed, h1Bar) @@ -667,6 +685,12 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { wantExecs: []string{}, wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: false, TotalResults: 0}, }, + { + opts: fleet.ListOptions{}, + hostID: h4.ID, + wantExecs: []string{}, + wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: false, TotalResults: 0}, + }, } for _, c := range cases { t.Run(fmt.Sprintf("%v: %#v", c.hostID, c.opts), func(t *testing.T) { From 3e1c3a5ef7a7884b6237206cc9c4c872f8203cef Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Mon, 20 Jan 2025 11:25:07 -0500 Subject: [PATCH 6/9] Add test for pending software installs --- .../datastore/mysql/software_installers_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/server/datastore/mysql/software_installers_test.go b/server/datastore/mysql/software_installers_test.go index 56d996ebe59e..38d7cd81ea38 100644 --- a/server/datastore/mysql/software_installers_test.go +++ b/server/datastore/mysql/software_installers_test.go @@ -55,6 +55,7 @@ func testListPendingSoftwareInstalls(t *testing.T, ds *Datastore) { host1 := test.NewHost(t, ds, "host1", "1", "host1key", "host1uuid", time.Now()) host2 := test.NewHost(t, ds, "host2", "2", "host2key", "host2uuid", time.Now()) + host3 := test.NewHost(t, ds, "host3", "3", "host3key", "host3uuid", time.Now()) user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true) err := ds.UpsertSecretVariables(ctx, []fleet.SecretVariable{ @@ -200,6 +201,21 @@ func testListPendingSoftwareInstalls(t *testing.T, ds *Datastore) { require.Equal(t, installerID3, exec2.InstallerID) require.Equal(t, "SELECT 3", exec2.PreInstallCondition) require.True(t, exec2.SelfService) + + // Create install request, don't fulfil it, delete and restore host. + // Should not appear in list of pending installs for that host. + _, err = ds.InsertSoftwareInstallRequest(ctx, host3.ID, installerID1, false, nil) + require.NoError(t, err) + + err = ds.DeleteHost(ctx, host3.ID) + require.NoError(t, err) + + err = ds.RestoreMDMApplePendingDEPHost(ctx, host3) + require.NoError(t, err) + + hostInstalls4, err := ds.ListPendingSoftwareInstalls(ctx, host3.ID) + require.NoError(t, err) + require.Empty(t, hostInstalls4) } func testSoftwareInstallRequests(t *testing.T, ds *Datastore) { From b9283a39d33d92663ff0c56a1635ddd847276ce6 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Mon, 20 Jan 2025 11:52:50 -0500 Subject: [PATCH 7/9] Add script execution list test for dep restored hosts --- server/datastore/mysql/scripts_test.go | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/server/datastore/mysql/scripts_test.go b/server/datastore/mysql/scripts_test.go index 1a449e7ce6cc..7468347e50ff 100644 --- a/server/datastore/mysql/scripts_test.go +++ b/server/datastore/mysql/scripts_test.go @@ -26,6 +26,7 @@ func TestScripts(t *testing.T) { fn func(t *testing.T, ds *Datastore) }{ {"HostScriptResult", testHostScriptResult}, + {"DEPRestoredHost", testListPensingScriptDEPRestoration}, {"Scripts", testScripts}, {"ListScripts", testListScripts}, {"GetHostScriptDetails", testGetHostScriptDetails}, @@ -259,6 +260,54 @@ func testHostScriptResult(t *testing.T, ds *Datastore) { require.EqualValues(t, -1, *unsignedScriptResult.ExitCode) } +func testListPensingScriptDEPRestoration(t *testing.T, ds *Datastore) { + ctx := context.Background() + + host := test.NewHost(t, ds, "host", "10.0.0.1", "1", "uuid1", time.Now()) + + // no script saved yet + pending, err := ds.ListPendingHostScriptExecutions(ctx, host.ID, false) + require.NoError(t, err) + require.Empty(t, pending) + + // create a createdScript execution request (with a user) + u := test.NewUser(t, ds, "Bob", "bob@example.com", true) + createdScript, err := ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{ + HostID: host.ID, + ScriptContents: "echo", + UserID: &u.ID, + SyncRequest: true, + }) + require.NoError(t, err) + require.NotZero(t, createdScript.ID) + require.NotEmpty(t, createdScript.ExecutionID) + require.Equal(t, uint(1), createdScript.HostID) + require.NotEmpty(t, createdScript.ExecutionID) + require.Equal(t, "echo", createdScript.ScriptContents) + require.Nil(t, createdScript.ExitCode) + require.Empty(t, createdScript.Output) + require.NotNil(t, createdScript.UserID) + require.Equal(t, u.ID, *createdScript.UserID) + require.True(t, createdScript.SyncRequest) + + // the script execution is now listed as pending for this host + pending, err = ds.ListPendingHostScriptExecutions(ctx, host.ID, false) + require.NoError(t, err) + require.Len(t, pending, 1) + require.Equal(t, createdScript.ID, pending[0].ID) + + err = ds.DeleteHost(ctx, host.ID) + require.NoError(t, err) + + err = ds.RestoreMDMApplePendingDEPHost(ctx, host) + require.NoError(t, err) + + // the script execution is no longer listed as pending for this host + pending, err = ds.ListPendingHostScriptExecutions(ctx, host.ID, false) + require.NoError(t, err) + require.Len(t, pending, 0) +} + func testScripts(t *testing.T, ds *Datastore) { ctx := context.Background() From fd7064dc3d910ee0ce3df8ed86483eee88bd0d16 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Mon, 20 Jan 2025 12:13:41 -0500 Subject: [PATCH 8/9] Go lint --- server/datastore/mysql/activities_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/datastore/mysql/activities_test.go b/server/datastore/mysql/activities_test.go index 567519abcdc1..d12e9e718c2c 100644 --- a/server/datastore/mysql/activities_test.go +++ b/server/datastore/mysql/activities_test.go @@ -568,7 +568,7 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { // Setup host 4. We will create upcoming activities, then // delete and "restore" the host, similar to what would happen // if you delete an ABM DEP host. - hsr, err = ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{HostID: h4.ID, ScriptID: &scr1.ID, ScriptContents: scr1.ScriptContents, UserID: &u.ID}) + _, err = ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{HostID: h4.ID, ScriptID: &scr1.ID, ScriptContents: scr1.ScriptContents, UserID: &u.ID}) require.NoError(t, err) // h4A := hsr.ExecutionID // h4Bar, err := ds.InsertSoftwareInstallRequest(ctx, h4.ID, sw2Meta.InstallerID, false, nil) From fedec4cf707343bc15a88b7e0121954ad57989c9 Mon Sep 17 00:00:00 2001 From: dantecatalfamo Date: Tue, 21 Jan 2025 15:10:39 -0500 Subject: [PATCH 9/9] Typo in function title --- server/datastore/mysql/scripts_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/datastore/mysql/scripts_test.go b/server/datastore/mysql/scripts_test.go index 7468347e50ff..197732d04125 100644 --- a/server/datastore/mysql/scripts_test.go +++ b/server/datastore/mysql/scripts_test.go @@ -26,7 +26,7 @@ func TestScripts(t *testing.T) { fn func(t *testing.T, ds *Datastore) }{ {"HostScriptResult", testHostScriptResult}, - {"DEPRestoredHost", testListPensingScriptDEPRestoration}, + {"DEPRestoredHost", testListPendingScriptDEPRestoration}, {"Scripts", testScripts}, {"ListScripts", testListScripts}, {"GetHostScriptDetails", testGetHostScriptDetails}, @@ -260,7 +260,7 @@ func testHostScriptResult(t *testing.T, ds *Datastore) { require.EqualValues(t, -1, *unsignedScriptResult.ExitCode) } -func testListPensingScriptDEPRestoration(t *testing.T, ds *Datastore) { +func testListPendingScriptDEPRestoration(t *testing.T, ds *Datastore) { ctx := context.Background() host := test.NewHost(t, ds, "host", "10.0.0.1", "1", "uuid1", time.Now())