Clear policy results and stats when setting or changing an installer (#22053)

Follow up PR for #21428.

After some discussions with Noah we want to clear policy results when a
user sets for the first time or changes an installer on a policy.
This commit is contained in:
Lucas Manuel Rodriguez 2024-09-12 16:56:12 -03:00 committed by GitHub
parent 3541ad6fa7
commit 169d9de24c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 242 additions and 11 deletions

View file

@ -753,9 +753,10 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
// Get the query and platforms of the current policies so that we can check if query or platform changed later, if needed
type policyLite struct {
Name string `db:"name"`
Query string `db:"query"`
Platforms string `db:"platforms"`
Name string `db:"name"`
Query string `db:"query"`
Platforms string `db:"platforms"`
SoftwareInstallerID *uint `db:"software_installer_id"`
}
teamIDToPoliciesByName := make(map[*uint]map[string]policyLite, len(teamIDToPolicies))
for teamID, teamPolicySpecs := range teamIDToPolicies {
@ -769,10 +770,10 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
var args []interface{}
var err error
if teamID == nil {
query, args, err = sqlx.In("SELECT name, query, platforms FROM policies WHERE team_id IS NULL AND name IN (?)", policyNames)
query, args, err = sqlx.In("SELECT name, query, platforms, software_installer_id FROM policies WHERE team_id IS NULL AND name IN (?)", policyNames)
} else {
query, args, err = sqlx.In(
"SELECT name, query, platforms FROM policies WHERE team_id = ? AND name IN (?)", *teamID, policyNames,
"SELECT name, query, platforms, software_installer_id FROM policies WHERE team_id = ? AND name IN (?)", *teamID, policyNames,
)
}
if err != nil {
@ -838,12 +839,21 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
shouldRemoveAllPolicyMemberships bool
removePolicyStats bool
)
// Figure out if the query or platform changed
// Figure out if the query, platform or software installer changed.
var softwareInstallerID *uint
if spec.SoftwareTitleID != nil {
softwareInstallerID = softwareInstallerIDs[teamID][*spec.SoftwareTitleID]
}
if prev, ok := teamIDToPoliciesByName[teamID][spec.Name]; ok {
switch {
case prev.Query != spec.Query:
shouldRemoveAllPolicyMemberships = true
removePolicyStats = true
case teamID != nil &&
((prev.SoftwareInstallerID == nil && spec.SoftwareTitleID != nil) ||
(prev.SoftwareInstallerID != nil && softwareInstallerID != nil && *prev.SoftwareInstallerID != *softwareInstallerID)):
shouldRemoveAllPolicyMemberships = true
removePolicyStats = true
case prev.Platforms != spec.Platform:
removePolicyStats = true
}

View file

@ -4061,6 +4061,24 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
require.NoError(t, err)
team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"})
require.NoError(t, err)
newHost := func(name string, teamID *uint, platform string) *fleet.Host {
h, err := ds.NewHost(ctx, &fleet.Host{
OsqueryHostID: ptr.String(uuid.New().String()),
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now(),
NodeKey: ptr.String(uuid.New().String()),
UUID: uuid.New().String(),
Hostname: name,
TeamID: teamID,
Platform: platform,
})
require.NoError(t, err)
return h
}
host1Team1 := newHost("host1Team1", &team1.ID, "darwin")
installer1ID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
InstallScript: "hello",
@ -4113,6 +4131,24 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
installer3, err := ds.GetSoftwareInstallerMetadataByID(ctx, installer3ID)
require.NoError(t, err)
require.NotNil(t, installer3.TitleID)
// Another installer on team1 to test changing installers.
installer5ID, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{
InstallScript: "hello5",
PreInstallQuery: "SELECT 5;",
PostInstallScript: "world5",
InstallerFile: bytes.NewReader([]byte("hello5")),
StorageID: "storage5",
Filename: "file5",
Title: "file5",
Version: "1.0",
Source: "programs",
UserID: user1.ID,
TeamID: &team1.ID,
})
require.NoError(t, err)
installer5, err := ds.GetSoftwareInstallerMetadataByID(ctx, installer5ID)
require.NoError(t, err)
require.NotNil(t, installer5.TitleID)
// Installers cannot be assigned to global policies.
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
@ -4164,6 +4200,7 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Len(t, team1Policies, 1)
require.NotNil(t, team1Policies[0].SoftwareInstallerID)
policy1Team1 := team1Policies[0]
require.Equal(t, installer1.InstallerID, *team1Policies[0].SoftwareInstallerID)
team2Policies, _, err := ds.ListTeamPolicies(ctx, team2.ID, fleet.ListOptions{}, fleet.ListOptions{})
require.NoError(t, err)
@ -4176,6 +4213,14 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
require.NotNil(t, noTeamPolicies[0].SoftwareInstallerID)
require.Equal(t, installer3.InstallerID, *noTeamPolicies[0].SoftwareInstallerID)
// Record policy execution on policy1Team1.
err = ds.RecordPolicyQueryExecutions(ctx, host1Team1, map[uint]*bool{
policy1Team1.ID: ptr.Bool(false),
}, time.Now(), false)
require.NoError(t, err)
err = ds.UpdateHostPolicyCounts(ctx)
require.NoError(t, err)
// Unset software installer from "Team policy 1".
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
{
@ -4193,6 +4238,8 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Len(t, team1Policies, 1)
require.Nil(t, team1Policies[0].SoftwareInstallerID)
// Should not clear results because we've cleared not changed/set-new installer.
require.Equal(t, uint(1), team1Policies[0].FailingHostCount)
// Set "Team policy 1" to a software installer on team2.
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
@ -4317,11 +4364,82 @@ func testApplyPolicySpecWithInstallers(t *testing.T, ds *Datastore) {
require.Len(t, team1Policies, 1)
require.NotNil(t, team1Policies[0].SoftwareInstallerID)
require.Equal(t, installer1.InstallerID, *team1Policies[0].SoftwareInstallerID)
// Should clear results because we've are setting an installer.
require.Equal(t, uint(0), team1Policies[0].FailingHostCount)
countBiggerThanZero := true
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&countBiggerThanZero,
`SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`,
team1Policies[0].ID,
)
})
require.False(t, countBiggerThanZero)
team2Policies, _, err = ds.ListTeamPolicies(ctx, team2.ID, fleet.ListOptions{}, fleet.ListOptions{})
require.NoError(t, err)
require.Len(t, team2Policies, 1)
require.NotNil(t, team2Policies[0].SoftwareInstallerID)
require.Equal(t, installer4.InstallerID, *team2Policies[0].SoftwareInstallerID)
// Record policy execution on policy1Team1 to test that setting the same installer won't clear results.
err = ds.RecordPolicyQueryExecutions(ctx, host1Team1, map[uint]*bool{
policy1Team1.ID: ptr.Bool(false),
}, time.Now(), false)
require.NoError(t, err)
err = ds.UpdateHostPolicyCounts(ctx)
require.NoError(t, err)
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
{
Name: "Team policy 1",
Query: "SELECT 1;",
Description: "Description 1",
Resolution: "Resolution 1",
Team: "team1",
Platform: "darwin",
SoftwareTitleID: installer1.TitleID,
},
})
require.NoError(t, err)
team1Policies, _, err = ds.ListTeamPolicies(ctx, team1.ID, fleet.ListOptions{}, fleet.ListOptions{})
require.NoError(t, err)
require.Len(t, team1Policies, 1)
require.Equal(t, uint(1), team1Policies[0].FailingHostCount)
countBiggerThanZero = false
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&countBiggerThanZero,
`SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`,
team1Policies[0].ID,
)
})
require.True(t, countBiggerThanZero)
// Now change the installer, should clear results.
err = ds.ApplyPolicySpecs(ctx, user1.ID, []*fleet.PolicySpec{
{
Name: "Team policy 1",
Query: "SELECT 1;",
Description: "Description 1",
Resolution: "Resolution 1",
Team: "team1",
Platform: "darwin",
SoftwareTitleID: installer5.TitleID,
},
})
require.NoError(t, err)
team1Policies, _, err = ds.ListTeamPolicies(ctx, team1.ID, fleet.ListOptions{}, fleet.ListOptions{})
require.NoError(t, err)
require.Len(t, team1Policies, 1)
require.Equal(t, uint(0), team1Policies[0].FailingHostCount)
countBiggerThanZero = true
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&countBiggerThanZero,
`SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`,
team1Policies[0].ID,
)
})
require.False(t, countBiggerThanZero)
}
func testTeamPoliciesNoTeam(t *testing.T, ds *Datastore) {

View file

@ -13553,6 +13553,35 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID)
require.NoError(t, err)
require.Nil(t, policy1Team1.SoftwareInstallerID)
host1LastInstall, err := s.ds.GetHostLastInstallData(ctx, host1Team1.ID, dummyInstallerPkgInstallerID)
require.NoError(t, err)
require.Nil(t, host1LastInstall)
// Add some results and stats that should be cleared after setting an installer again.
distributedResp := submitDistributedQueryResultsResponse{}
s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults(
host1Team1,
map[uint]*bool{
policy1Team1.ID: ptr.Bool(false),
},
), http.StatusOK, &distributedResp)
err = s.ds.UpdateHostPolicyCounts(ctx)
require.NoError(t, err)
policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID)
require.NoError(t, err)
require.Equal(t, uint(0), policy1Team1.PassingHostCount)
require.Equal(t, uint(1), policy1Team1.FailingHostCount)
passes := true
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&passes,
`SELECT passes FROM policy_membership WHERE policy_id = ? AND host_id = ?`,
policy1Team1.ID, host1Team1.ID,
)
})
require.False(t, passes)
// Back to associating dummy_installer.pkg to policy1Team1.
mtplr = modifyTeamPolicyResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{
@ -13564,6 +13593,77 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
require.NoError(t, err)
require.NotNil(t, policy1Team1.SoftwareInstallerID)
require.Equal(t, dummyInstallerPkgInstallerID, *policy1Team1.SoftwareInstallerID)
// Policy stats and membership should be cleared from policy1Team1.
require.Equal(t, uint(0), policy1Team1.PassingHostCount)
require.Equal(t, uint(0), policy1Team1.FailingHostCount)
countBiggerThanZero := true
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&countBiggerThanZero,
`SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`,
policy1Team1.ID,
)
})
require.False(t, countBiggerThanZero)
// Add (again) some results and stats that should be cleared after changing an existing installer.
distributedResp = submitDistributedQueryResultsResponse{}
s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults(
host1Team1,
map[uint]*bool{
policy1Team1.ID: ptr.Bool(false),
},
), http.StatusOK, &distributedResp)
err = s.ds.UpdateHostPolicyCounts(ctx)
require.NoError(t, err)
policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID)
require.NoError(t, err)
require.Equal(t, uint(0), policy1Team1.PassingHostCount)
require.Equal(t, uint(1), policy1Team1.FailingHostCount)
passes = true
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&passes,
`SELECT passes FROM policy_membership WHERE policy_id = ? AND host_id = ?`,
policy1Team1.ID, host1Team1.ID,
)
})
require.False(t, passes)
// Change the installer (temporarily to test that changing an installer will clear results)
// Associate ruby.deb to policy1Team1.
mtplr = modifyTeamPolicyResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{
ModifyPolicyPayload: fleet.ModifyPolicyPayload{
SoftwareTitleID: &rubyDebTitleID,
},
}, http.StatusOK, &mtplr)
// After changing the installer, membership and stats should be cleared.
policy1Team1, err = s.ds.Policy(ctx, policy1Team1.ID)
require.NoError(t, err)
require.NotNil(t, policy1Team1.SoftwareInstallerID)
require.Equal(t, rubyDebInstallerID, *policy1Team1.SoftwareInstallerID)
// Policy stats and membership should be cleared from policy1Team1.
require.Equal(t, uint(0), policy1Team1.PassingHostCount)
require.Equal(t, uint(0), policy1Team1.FailingHostCount)
countBiggerThanZero = true
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q,
&countBiggerThanZero,
`SELECT COUNT(*) > 0 FROM policy_membership WHERE policy_id = ?`,
policy1Team1.ID,
)
})
require.False(t, countBiggerThanZero)
// Back to (again) associating dummy_installer.pkg to policy1Team1.
mtplr = modifyTeamPolicyResponse{}
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/policies/%d", team1.ID, policy1Team1.ID), modifyTeamPolicyRequest{
ModifyPolicyPayload: fleet.ModifyPolicyPayload{
SoftwareTitleID: &dummyInstallerPkgTitleID,
},
}, http.StatusOK, &mtplr)
// Associate ruby.deb to policy2Team1.
mtplr = modifyTeamPolicyResponse{}
@ -13573,10 +13673,6 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
},
}, http.StatusOK, &mtplr)
host1LastInstall, err := s.ds.GetHostLastInstallData(ctx, host1Team1.ID, dummyInstallerPkgInstallerID)
require.NoError(t, err)
require.Nil(t, host1LastInstall)
// We use DoJSONWithoutAuth for distributed/write because we want the requests to not have the
// current user's "Authorization: Bearer <API_TOKEN>" header.
@ -13584,7 +13680,7 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
// Failing policy1Team1 means an install request must be generated.
// Failing policy2Team1 should not trigger a install request because it has a .deb attached to it (does not apply to macOS hosts).
// Failing policy3Team1 should do nothing because it doesn't have any installers associated to it.
distributedResp := submitDistributedQueryResultsResponse{}
distributedResp = submitDistributedQueryResultsResponse{}
s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults(
host1Team1,
map[uint]*bool{

View file

@ -493,6 +493,13 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f
if err != nil {
return nil, err
}
// If the associated installer is changed (or it's set and the policy didn't have an associated installer)
// then we clear the results of the policy so that automation can be triggered upon failure
// (automation is currently triggered on the first failure or when it goes from passing to failure).
if softwareInstallerID != nil && (policy.SoftwareInstallerID == nil || *policy.SoftwareInstallerID != *softwareInstallerID) {
removeAllMemberships = true
removeStats = true
}
policy.SoftwareInstallerID = softwareInstallerID
}