Skip to content

Commit

Permalink
fix: feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jahzielv committed Jan 6, 2025
1 parent b54e0c0 commit 0a34d34
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 62 deletions.
6 changes: 3 additions & 3 deletions ee/server/service/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.
// Get the hosts that are NOT in label scope currently (before the update happens)
var hostsNotInScope map[uint]struct{}
if dirty["Labels"] {
hostsNotInScope, err = svc.ds.GetHostsNotInScopeForSoftwareInstaller(ctx, payload.InstallerID)
hostsNotInScope, err = svc.ds.GetExcludedHostIDMapForSoftwareInstaller(ctx, payload.InstallerID)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "getting hosts not in scope for installer")
}
Expand All @@ -453,7 +453,7 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.

if dirty["Labels"] {
// Get the hosts that are now IN label scope (after the update)
hostsInScope, err := svc.ds.GetHostsInScopeForSoftwareInstaller(ctx, payload.InstallerID)
hostsInScope, err := svc.ds.GetIncludedHostIDMapForSoftwareInstaller(ctx, payload.InstallerID)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "getting hosts in scope for installer")
}
Expand All @@ -468,7 +468,7 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet.

// We clear the policy status here because otherwise the policy automation machinery
// won't pick this up and the software won't install.
if err := svc.ds.ClearAutoInstallPolicyStatusForHost(ctx, payload.InstallerID, hostsToClear); err != nil {
if err := svc.ds.ClearAutoInstallPolicyStatusForHosts(ctx, payload.InstallerID, hostsToClear); err != nil {
return nil, ctxerr.Wrap(ctx, err, "failed to clear auto install policy status for host")
}
}
Expand Down
28 changes: 28 additions & 0 deletions server/datastore/mysql/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,34 @@ func (ds *Datastore) RecordPolicyQueryExecutions(ctx context.Context, host *flee
return nil
}

func (ds *Datastore) ClearAutoInstallPolicyStatusForHosts(ctx context.Context, installerID uint, hostIDs []uint) error {
if len(hostIDs) == 0 {
return nil
}

stmt := `
UPDATE
policies p
JOIN policy_membership pm ON pm.policy_id = p.id
SET
passes = NULL
WHERE
p.software_installer_id = ?
AND pm.host_id IN (?)
`

stmt, args, err := sqlx.In(stmt, installerID, hostIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "building in statement for clearing auto install policy status")
}

if _, err := ds.writer(ctx).ExecContext(ctx, stmt, args...); err != nil {
return ctxerr.Wrap(ctx, err, "clearing auto install policy status")
}

return nil
}

func (ds *Datastore) ListGlobalPolicies(ctx context.Context, opts fleet.ListOptions) ([]*fleet.Policy, error) {
return listPoliciesDB(ctx, ds.reader(ctx), nil, opts)
}
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/mysql/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5439,7 +5439,7 @@ func testClearAutoInstallPolicyStatusForHost(t *testing.T, ds *Datastore) {
require.Equal(t, hostPolicies[1].Response, "fail")

// clear status for automatic install policy
err = ds.ClearAutoInstallPolicyStatusForHost(ctx, installer1ID, []uint{host.ID})
err = ds.ClearAutoInstallPolicyStatusForHosts(ctx, installer1ID, []uint{host.ID})
require.NoError(t, err)

// the status should be NULL for the automatic install policy but not the "regular" one
Expand Down
36 changes: 4 additions & 32 deletions server/datastore/mysql/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,7 @@ HAVING
AND count_installer_labels = count_host_updated_after_labels
AND count_host_labels = 0) t`

func (ds *Datastore) GetHostsInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
func (ds *Datastore) GetIncludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
stmt := fmt.Sprintf(`SELECT
h.id
FROM
Expand All @@ -1829,7 +1829,7 @@ WHERE

var hostIDs []uint
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hostIDs, stmt, installerID, installerID, installerID); err != nil {
return nil, ctxerr.Wrap(ctx, err, "listing hosts in scope for software")
return nil, ctxerr.Wrap(ctx, err, "listing hosts included in software installer scope")
}

res := make(map[uint]struct{}, len(hostIDs))
Expand All @@ -1840,7 +1840,7 @@ WHERE
return res, nil
}

func (ds *Datastore) GetHostsNotInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
func (ds *Datastore) GetExcludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
stmt := fmt.Sprintf(`SELECT
h.id
FROM
Expand All @@ -1851,7 +1851,7 @@ WHERE

var hostIDs []uint
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hostIDs, stmt, installerID, installerID, installerID); err != nil {
return nil, ctxerr.Wrap(ctx, err, "listing hosts in scope for software")
return nil, ctxerr.Wrap(ctx, err, "listing hosts excluded from software installer scope")
}

res := make(map[uint]struct{}, len(hostIDs))
Expand All @@ -1861,31 +1861,3 @@ WHERE

return res, nil
}

func (ds *Datastore) ClearAutoInstallPolicyStatusForHost(ctx context.Context, installerID uint, hostIDs []uint) error {
if len(hostIDs) == 0 {
return nil
}

stmt := `
UPDATE
policies p
JOIN policy_membership pm ON pm.policy_id = p.id
SET
passes = NULL
WHERE
p.software_installer_id = ?
AND pm.host_id IN (?)
`

stmt, args, err := sqlx.In(stmt, installerID, hostIDs)
if err != nil {
return ctxerr.Wrap(ctx, err, "building in statement for clearing auto install policy status")
}

if _, err := ds.writer(ctx).ExecContext(ctx, stmt, args...); err != nil {
return ctxerr.Wrap(ctx, err, "clearing auto install policy status")
}

return nil
}
4 changes: 2 additions & 2 deletions server/datastore/mysql/software_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5321,7 +5321,7 @@ func testListHostSoftwareWithLabelScoping(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.True(t, scoped)

hostsInScope, err := ds.GetHostsInScopeForSoftwareInstaller(ctx, installerID1)
hostsInScope, err := ds.GetIncludedHostIDMapForSoftwareInstaller(ctx, installerID1)
require.NoError(t, err)
require.Contains(t, hostsInScope, host.ID)

Expand All @@ -5347,7 +5347,7 @@ func testListHostSoftwareWithLabelScoping(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Empty(t, software)

hostsNotInScope, err := ds.GetHostsNotInScopeForSoftwareInstaller(ctx, installerID1)
hostsNotInScope, err := ds.GetExcludedHostIDMapForSoftwareInstaller(ctx, installerID1)
require.NoError(t, err)
require.Contains(t, hostsNotInScope, host.ID)

Expand Down
12 changes: 6 additions & 6 deletions server/fleet/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -1706,17 +1706,17 @@ type Datastore interface {
// Software installers
//

// GetHostsInScopeForSoftwareInstaller gets the set of hosts that are targeted/in scope for the
// GetIncludedHostIDMapForSoftwareInstaller gets the set of hosts that are targeted/in scope for the
// given software installer, based label membership.
GetHostsInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error)
GetIncludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error)

// GetHostsNotInScopeForSoftwareInstaller gets the set of hosts that are NOT targeted/in scope for the
// GetExcludedHostIDMapForSoftwareInstaller gets the set of hosts that are NOT targeted/in scope for the
// given software installer, based label membership.
GetHostsNotInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error)
GetExcludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error)

// ClearAutoInstallPolicyStatusForHost clears out the status of the policy related to the given
// ClearAutoInstallPolicyStatusForHosts clears out the status of the policy related to the given
// software installer for all the given hosts.
ClearAutoInstallPolicyStatusForHost(ctx context.Context, installerID uint, hostIDs []uint) error
ClearAutoInstallPolicyStatusForHosts(ctx context.Context, installerID uint, hostIDs []uint) error

// GetSoftwareInstallDetails returns details required to fetch and
// run software installers
Expand Down
36 changes: 18 additions & 18 deletions server/mock/datastore_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -1077,11 +1077,11 @@ type WipeHostViaWindowsMDMFunc func(ctx context.Context, host *fleet.Host, cmd *

type UpdateHostLockWipeStatusFromAppleMDMResultFunc func(ctx context.Context, hostUUID string, cmdUUID string, requestType string, succeeded bool) error

type GetHostsInScopeForSoftwareInstallerFunc func(ctx context.Context, installerID uint) (map[uint]struct{}, error)
type GetIncludedHostIDMapForSoftwareInstallerFunc func(ctx context.Context, installerID uint) (map[uint]struct{}, error)

type GetHostsNotInScopeForSoftwareInstallerFunc func(ctx context.Context, installerID uint) (map[uint]struct{}, error)
type GetExcludedHostIDMapForSoftwareInstallerFunc func(ctx context.Context, installerID uint) (map[uint]struct{}, error)

type ClearAutoInstallPolicyStatusForHostFunc func(ctx context.Context, installerID uint, hostIDs []uint) error
type ClearAutoInstallPolicyStatusForHostsFunc func(ctx context.Context, installerID uint, hostIDs []uint) error

type GetSoftwareInstallDetailsFunc func(ctx context.Context, executionId string) (*fleet.SoftwareInstallDetails, error)

Expand Down Expand Up @@ -2778,14 +2778,14 @@ type DataStore struct {
UpdateHostLockWipeStatusFromAppleMDMResultFunc UpdateHostLockWipeStatusFromAppleMDMResultFunc
UpdateHostLockWipeStatusFromAppleMDMResultFuncInvoked bool

GetHostsInScopeForSoftwareInstallerFunc GetHostsInScopeForSoftwareInstallerFunc
GetHostsInScopeForSoftwareInstallerFuncInvoked bool
GetIncludedHostIDMapForSoftwareInstallerFunc GetIncludedHostIDMapForSoftwareInstallerFunc
GetIncludedHostIDMapForSoftwareInstallerFuncInvoked bool

GetHostsNotInScopeForSoftwareInstallerFunc GetHostsNotInScopeForSoftwareInstallerFunc
GetHostsNotInScopeForSoftwareInstallerFuncInvoked bool
GetExcludedHostIDMapForSoftwareInstallerFunc GetExcludedHostIDMapForSoftwareInstallerFunc
GetExcludedHostIDMapForSoftwareInstallerFuncInvoked bool

ClearAutoInstallPolicyStatusForHostFunc ClearAutoInstallPolicyStatusForHostFunc
ClearAutoInstallPolicyStatusForHostFuncInvoked bool
ClearAutoInstallPolicyStatusForHostsFunc ClearAutoInstallPolicyStatusForHostsFunc
ClearAutoInstallPolicyStatusForHostsFuncInvoked bool

GetSoftwareInstallDetailsFunc GetSoftwareInstallDetailsFunc
GetSoftwareInstallDetailsFuncInvoked bool
Expand Down Expand Up @@ -6651,25 +6651,25 @@ func (s *DataStore) UpdateHostLockWipeStatusFromAppleMDMResult(ctx context.Conte
return s.UpdateHostLockWipeStatusFromAppleMDMResultFunc(ctx, hostUUID, cmdUUID, requestType, succeeded)
}

func (s *DataStore) GetHostsInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
func (s *DataStore) GetIncludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
s.mu.Lock()
s.GetHostsInScopeForSoftwareInstallerFuncInvoked = true
s.GetIncludedHostIDMapForSoftwareInstallerFuncInvoked = true
s.mu.Unlock()
return s.GetHostsInScopeForSoftwareInstallerFunc(ctx, installerID)
return s.GetIncludedHostIDMapForSoftwareInstallerFunc(ctx, installerID)
}

func (s *DataStore) GetHostsNotInScopeForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
func (s *DataStore) GetExcludedHostIDMapForSoftwareInstaller(ctx context.Context, installerID uint) (map[uint]struct{}, error) {
s.mu.Lock()
s.GetHostsNotInScopeForSoftwareInstallerFuncInvoked = true
s.GetExcludedHostIDMapForSoftwareInstallerFuncInvoked = true
s.mu.Unlock()
return s.GetHostsNotInScopeForSoftwareInstallerFunc(ctx, installerID)
return s.GetExcludedHostIDMapForSoftwareInstallerFunc(ctx, installerID)
}

func (s *DataStore) ClearAutoInstallPolicyStatusForHost(ctx context.Context, installerID uint, hostIDs []uint) error {
func (s *DataStore) ClearAutoInstallPolicyStatusForHosts(ctx context.Context, installerID uint, hostIDs []uint) error {
s.mu.Lock()
s.ClearAutoInstallPolicyStatusForHostFuncInvoked = true
s.ClearAutoInstallPolicyStatusForHostsFuncInvoked = true
s.mu.Unlock()
return s.ClearAutoInstallPolicyStatusForHostFunc(ctx, installerID, hostIDs)
return s.ClearAutoInstallPolicyStatusForHostsFunc(ctx, installerID, hostIDs)
}

func (s *DataStore) GetSoftwareInstallDetails(ctx context.Context, executionId string) (*fleet.SoftwareInstallDetails, error) {
Expand Down

0 comments on commit 0a34d34

Please sign in to comment.