Allow updating the policy platform (part 1 of the ticket) (#4311)

This commit is contained in:
Martin Angers
2022-02-22 13:42:03 -05:00
committed by GitHub
parent e39ba93a90
commit ad5146c491
13 changed files with 318 additions and 33 deletions
+2 -2
View File
@@ -404,9 +404,9 @@ func (ds *Datastore) Host(ctx context.Context, id uint, skipLoadingExtras bool)
return host, nil
}
func amountEnrolledHostsDB(db sqlx.Queryer) (int, error) {
func amountEnrolledHostsDB(ctx context.Context, db sqlx.QueryerContext) (int, error) {
var amount int
err := sqlx.Get(db, &amount, `SELECT count(*) FROM hosts`)
err := sqlx.GetContext(ctx, db, &amount, `SELECT count(*) FROM hosts`)
if err != nil {
return 0, err
}
+2 -2
View File
@@ -790,9 +790,9 @@ func (ds *Datastore) AsyncBatchUpdateLabelTimestamp(ctx context.Context, ids []u
})
}
func amountLabelsDB(db sqlx.Queryer) (int, error) {
func amountLabelsDB(ctx context.Context, db sqlx.QueryerContext) (int, error) {
var amount int
err := sqlx.Get(db, &amount, `SELECT count(*) FROM labels`)
err := sqlx.GetContext(ctx, db, &amount, `SELECT count(*) FROM labels`)
if err != nil {
return 0, err
}
+30
View File
@@ -916,3 +916,33 @@ func (ds *Datastore) ProcessList(ctx context.Context) ([]fleet.MySQLProcess, err
}
return processList, nil
}
func insertOnDuplicateDidUpdate(res sql.Result) bool {
// From mysql's documentation:
//
// With ON DUPLICATE KEY UPDATE, the affected-rows value per row is 1 if
// the row is inserted as a new row, 2 if an existing row is updated, and
// 0 if an existing row is set to its current values. If you specify the
// CLIENT_FOUND_ROWS flag to the mysql_real_connect() C API function when
// connecting to mysqld, the affected-rows value is 1 (not 0) if an
// existing row is set to its current values.
//
// https://dev.mysql.com/doc/refman/5.7/en/insert-on-duplicate.html
//
// Note that connection string sets CLIENT_FOUND_ROWS (see
// generateMysqlConnectionString in this package), so it does return 1 when
// an existing row is set to its current values, but with a last inserted id
// of 0.
//
// Also note that with our mysql driver, Result.LastInsertId and
// Result.RowsAffected can never return an error, they are retrieved at the
// time of the Exec call, and the result simply returns the integers it
// already holds:
// https://github.com/go-sql-driver/mysql/blob/bcc459a906419e2890a50fc2c99ea6dd927a88f2/result.go
//
// TODO(mna): would that work on mariadb too?
lastID, _ := res.LastInsertId()
aff, _ := res.RowsAffected()
return lastID == 0 || aff != 1
}
+51 -10
View File
@@ -83,10 +83,10 @@ func policyDB(ctx context.Context, q sqlx.QueryerContext, id uint, teamID *uint)
func (ds *Datastore) SavePolicy(ctx context.Context, p *fleet.Policy) error {
sql := `
UPDATE policies
SET name = ?, query = ?, description = ?, resolution = ?
SET name = ?, query = ?, description = ?, resolution = ?, platforms = ?
WHERE id = ?
`
result, err := ds.writer.ExecContext(ctx, sql, p.Name, p.Query, p.Description, p.Resolution, p.ID)
result, err := ds.writer.ExecContext(ctx, sql, p.Name, p.Query, p.Description, p.Resolution, p.Platform, p.ID)
if err != nil {
return ctxerr.Wrap(ctx, err, "updating policy")
}
@@ -97,7 +97,8 @@ func (ds *Datastore) SavePolicy(ctx context.Context, p *fleet.Policy) error {
if rows == 0 {
return ctxerr.Wrap(ctx, notFound("Policy").WithID(p.ID))
}
return nil
return cleanupPolicyMembership(ctx, ds.writer, p.ID, p.Platform)
}
// FlippingPoliciesForHost fetches previous policy membership results and returns:
@@ -438,8 +439,7 @@ func (ds *Datastore) TeamPolicy(ctx context.Context, teamID uint, policyID uint)
// NOTE: Similar to ApplyQueries, ApplyPolicySpecs will update the author_id of the policies
// that are updated.
//
// Currently ApplyPolicySpecs does not allow updating the team or platform of an existing policy,
// such functionality will be implemented in #3220.
// Currently ApplyPolicySpecs does not allow updating the team of an existing policy.
func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs []*fleet.PolicySpec) error {
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
sql := `
@@ -457,22 +457,34 @@ func (ds *Datastore) ApplyPolicySpecs(ctx context.Context, authorID uint, specs
query = VALUES(query),
description = VALUES(description),
author_id = VALUES(author_id),
resolution = VALUES(resolution)
resolution = VALUES(resolution),
platforms = VALUES(platforms)
`
for _, spec := range specs {
if _, err := tx.ExecContext(ctx,
res, err := tx.ExecContext(ctx,
sql, spec.Name, spec.Query, spec.Description, authorID, spec.Resolution, spec.Team, spec.Platform,
); err != nil {
)
if err != nil {
return ctxerr.Wrap(ctx, err, "exec ApplyPolicySpecs insert")
}
if insertOnDuplicateDidUpdate(res) {
// when the upsert results in an UPDATE that *did* change some values,
// it returns the updated ID as last inserted id.
if lastID, _ := res.LastInsertId(); lastID > 0 {
if err := cleanupPolicyMembership(ctx, tx, uint(lastID), spec.Platform); err != nil {
return err
}
}
}
}
return nil
})
}
func amountPoliciesDB(db sqlx.Queryer) (int, error) {
func amountPoliciesDB(ctx context.Context, db sqlx.QueryerContext) (int, error) {
var amount int
err := sqlx.Get(db, &amount, `SELECT count(*) FROM policies`)
err := sqlx.GetContext(ctx, db, &amount, `SELECT count(*) FROM policies`)
if err != nil {
return 0, err
}
@@ -523,3 +535,32 @@ func (ds *Datastore) AsyncBatchUpdatePolicyTimestamp(ctx context.Context, ids []
return ctxerr.Wrap(ctx, err, "update hosts.policy_updated_at")
})
}
func cleanupPolicyMembership(ctx context.Context, db sqlx.ExecerContext, policyID uint, platforms string) error {
if platforms == "" {
// all platforms allowed, nothing to clean up
return nil
}
delStmt := `
DELETE
pm
FROM
policy_membership pm
LEFT JOIN
hosts h
ON
pm.host_id = h.id
WHERE
pm.policy_id = ? AND
( h.id IS NULL OR
FIND_IN_SET(h.platform, ?) = 0 )`
var expandedPlatforms []string
splitPlatforms := strings.Split(platforms, ",")
for _, platform := range splitPlatforms {
expandedPlatforms = append(expandedPlatforms, fleet.ExpandPlatform(strings.TrimSpace(platform))...)
}
_, err := db.ExecContext(ctx, delStmt, policyID, strings.Join(expandedPlatforms, ","))
return ctxerr.Wrap(ctx, err, "cleanup policy membership")
}
+203 -5
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"sort"
"strconv"
"strings"
"testing"
"time"
@@ -14,6 +15,7 @@ import (
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/fleetdm/fleet/v4/server/test"
"github.com/google/uuid"
"github.com/jmoiron/sqlx"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -39,6 +41,7 @@ func TestPolicies(t *testing.T) {
{"Save", testPoliciesSave},
{"DelUser", testPoliciesDelUser},
{"FlippingPoliciesForHost", testFlippingPoliciesForHost},
{"PlatformUpdate", testPolicyPlatformUpdate},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
@@ -1100,16 +1103,16 @@ func testApplyPolicySpec(t *testing.T, ds *Datastore) {
Query: "select 1 from updated;",
Description: "query1 desc updated",
Resolution: "some resolution updated",
Team: "", // TODO(lucas): no effect, #3220.
Platform: "", // TODO(lucas): no effect, #3220.
Team: "", // TODO(lucas): no effect.
Platform: "",
},
{
Name: "query2",
Query: "select 2 from updated;",
Description: "query2 desc updated",
Resolution: "some other resolution updated",
Team: "team1", // TODO(lucas): no effect, #3220.
Platform: "windows", // TODO(lucas): no effect, #3220.
Team: "team1", // TODO(lucas): no effect.
Platform: "windows",
},
}))
policies, err = ds.ListGlobalPolicies(ctx)
@@ -1137,7 +1140,7 @@ func testApplyPolicySpec(t *testing.T, ds *Datastore) {
assert.Equal(t, team1.ID, *teamPolicies[0].TeamID)
require.NotNil(t, teamPolicies[0].Resolution)
assert.Equal(t, "some other resolution updated", *teamPolicies[0].Resolution)
assert.Equal(t, "darwin", teamPolicies[0].Platform)
assert.Equal(t, "windows", teamPolicies[0].Platform)
}
func testPoliciesSave(t *testing.T, ds *Datastore) {
@@ -1433,3 +1436,198 @@ func testFlippingPoliciesForHost(t *testing.T, ds *Datastore) {
require.Empty(t, newFailing)
require.Empty(t, newPassing)
}
func testPolicyPlatformUpdate(t *testing.T, ds *Datastore) {
ctx := context.Background()
user := test.NewUser(t, ds, "Alice", "alice@example.com", true)
tm, err := ds.NewTeam(ctx, &fleet.Team{Name: t.Name()})
require.NoError(t, err)
const hostWin, hostMac, hostDeb, hostLin = 0, 1, 2, 3
platforms := []string{"windows", "darwin", "debian", "linux"}
// create hosts with different platforms, for that team
teamHosts := make([]*fleet.Host, len(platforms))
for i, pl := range platforms {
id := fmt.Sprintf("%s-%d", strings.ReplaceAll(t.Name(), "/", "_"), i)
h, err := ds.NewHost(ctx, &fleet.Host{
OsqueryHostID: id,
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now(),
NodeKey: id,
UUID: id,
Hostname: id,
Platform: pl,
TeamID: ptr.Uint(tm.ID),
})
require.NoError(t, err)
teamHosts[i] = h
}
// create hosts with different platforms, without team
globalHosts := make([]*fleet.Host, len(platforms))
for i, pl := range platforms {
id := fmt.Sprintf("g%s-%d", strings.ReplaceAll(t.Name(), "/", "_"), i)
h, err := ds.NewHost(ctx, &fleet.Host{
OsqueryHostID: id,
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now(),
NodeKey: id,
UUID: id,
Hostname: id,
Platform: pl,
})
require.NoError(t, err)
globalHosts[i] = h
}
// new global policy for any platform
_, err = ds.NewGlobalPolicy(ctx, ptr.Uint(user.ID), fleet.PolicyPayload{Name: "g1", Query: "select 1", Platform: ""})
require.NoError(t, err)
// new team policy for any platform
_, err = ds.NewTeamPolicy(ctx, tm.ID, ptr.Uint(user.ID), fleet.PolicyPayload{Name: "t1", Query: "select 1", Platform: ""})
require.NoError(t, err)
// new global and team policies for Linux, via apply spec
err = ds.ApplyPolicySpecs(ctx, user.ID, []*fleet.PolicySpec{
{Name: "g2", Query: "select 2", Platform: "linux"},
{Name: "t2", Query: "select 2", Team: tm.Name, Platform: "linux"},
})
require.NoError(t, err)
// load the global policies
gpols, err := ds.ListGlobalPolicies(ctx)
require.NoError(t, err)
require.Len(t, gpols, 2)
// load the team policies
tpols, err := ds.ListTeamPolicies(ctx, tm.ID)
require.NoError(t, err)
require.Len(t, tpols, 2)
// index the policies by name for easier access in the rest of the test
polsByName := make(map[string]*fleet.Policy, len(gpols)+len(tpols))
for _, tpol := range tpols {
polsByName[tpol.Name] = tpol
}
for _, gpol := range gpols {
polsByName[gpol.Name] = gpol
}
// updating without change works fine
err = ds.SavePolicy(ctx, polsByName["g1"])
require.NoError(t, err)
err = ds.SavePolicy(ctx, polsByName["t2"])
require.NoError(t, err)
// apply specs that result in an update (without change) works fine
err = ds.ApplyPolicySpecs(ctx, user.ID, []*fleet.PolicySpec{
{Name: polsByName["g2"].Name, Query: polsByName["g2"].Query, Platform: polsByName["g2"].Platform},
{Name: polsByName["t1"].Name, Query: polsByName["t1"].Query, Team: tm.Name, Platform: polsByName["t1"].Platform},
})
require.NoError(t, err)
pol, err := ds.Policy(ctx, polsByName["g2"].ID)
require.NoError(t, err)
require.Equal(t, polsByName["g2"], pol)
pol, err = ds.Policy(ctx, polsByName["t1"].ID)
require.NoError(t, err)
require.Equal(t, polsByName["t1"], pol)
// record some results for each policy
for i, h := range teamHosts {
res := map[uint]*bool{
polsByName["t1"].ID: ptr.Bool(true),
}
if i == hostDeb || i == hostLin {
// also record a result for linux policy
res[polsByName["t2"].ID] = ptr.Bool(true)
}
err = ds.RecordPolicyQueryExecutions(ctx, h, res, time.Now(), false)
require.NoError(t, err)
}
for i, h := range globalHosts {
res := map[uint]*bool{
polsByName["g1"].ID: ptr.Bool(true),
}
if i == hostDeb || i == hostLin {
// also record a result for linux policy
res[polsByName["g2"].ID] = ptr.Bool(true)
}
err = ds.RecordPolicyQueryExecutions(ctx, h, res, time.Now(), false)
require.NoError(t, err)
}
policyIDs := make([]uint, 0, len(polsByName))
for _, pol := range polsByName {
policyIDs = append(policyIDs, pol.ID)
}
loadMembershipStmt, args, err := sqlx.In(`SELECT policy_id, host_id FROM policy_membership WHERE policy_id IN (?)`, policyIDs)
require.NoError(t, err)
assertPolicyMembership := func(want map[string][]uint) {
type polHostIDs struct {
PolicyID uint `db:"policy_id"`
HostID uint `db:"host_id"`
}
var rows []polHostIDs
err := ds.writer.SelectContext(ctx, &rows, loadMembershipStmt, args...)
require.NoError(t, err)
// index the host IDs by policy ID
hostIDsByPolID := make(map[uint][]uint, len(policyIDs))
for _, row := range rows {
hostIDsByPolID[row.PolicyID] = append(hostIDsByPolID[row.PolicyID], row.HostID)
}
// assert that they match the expected list of hosts by policy
for polNm, hostIDs := range want {
polID := polsByName[polNm].ID
got := hostIDsByPolID[polID]
require.ElementsMatch(t, hostIDs, got)
}
}
wantHostsByPol := map[string][]uint{
"g1": {globalHosts[hostWin].ID, globalHosts[hostMac].ID, globalHosts[hostDeb].ID, globalHosts[hostLin].ID},
"g2": {globalHosts[hostDeb].ID, globalHosts[hostLin].ID},
"t1": {teamHosts[hostWin].ID, teamHosts[hostMac].ID, teamHosts[hostDeb].ID, teamHosts[hostLin].ID},
"t2": {teamHosts[hostDeb].ID, teamHosts[hostLin].ID},
}
assertPolicyMembership(wantHostsByPol)
// update global policy g1 from any => linux
g1 := polsByName["g1"]
g1.Platform = "linux"
polsByName["g1"] = g1
err = ds.SavePolicy(ctx, g1)
require.NoError(t, err)
wantHostsByPol["g1"] = []uint{globalHosts[hostDeb].ID, globalHosts[hostLin].ID}
assertPolicyMembership(wantHostsByPol)
// update team policy t1 from any => windows, darwin
t1 := polsByName["t1"]
t1.Platform = "windows,darwin"
polsByName["t1"] = t1
err = ds.SavePolicy(ctx, t1)
require.NoError(t, err)
wantHostsByPol["t1"] = []uint{teamHosts[hostWin].ID, teamHosts[hostMac].ID}
assertPolicyMembership(wantHostsByPol)
// update g2 from linux => any, t2 from linux => debian, via ApplySpecs
t2, g2 := polsByName["t2"], polsByName["g2"]
g2.Platform = ""
t2.Platform = "debian"
polsByName["t2"], polsByName["g2"] = t2, g2
err = ds.ApplyPolicySpecs(ctx, user.ID, []*fleet.PolicySpec{
{Name: g2.Name, Query: g2.Query, Platform: g2.Platform},
{Name: t2.Name, Query: t2.Query, Team: tm.Name, Platform: t2.Platform},
})
require.NoError(t, err)
// nothing should've changed for g2 (platform changed to any, so nothing to cleanup),
// while t2 should now only accept debian
wantHostsByPol["t2"] = []uint{teamHosts[hostDeb].ID}
assertPolicyMembership(wantHostsByPol)
}
+5 -5
View File
@@ -18,23 +18,23 @@ type statistics struct {
}
func (ds *Datastore) ShouldSendStatistics(ctx context.Context, frequency time.Duration, license *fleet.LicenseInfo) (fleet.StatisticsPayload, bool, error) {
amountEnrolledHosts, err := amountEnrolledHostsDB(ds.writer)
amountEnrolledHosts, err := amountEnrolledHostsDB(ctx, ds.writer)
if err != nil {
return fleet.StatisticsPayload{}, false, ctxerr.Wrap(ctx, err, "amount enrolled hosts")
}
amountUsers, err := amountUsersDB(ds.writer)
amountUsers, err := amountUsersDB(ctx, ds.writer)
if err != nil {
return fleet.StatisticsPayload{}, false, ctxerr.Wrap(ctx, err, "amount users")
}
amountTeams, err := amountTeamsDB(ds.writer)
amountTeams, err := amountTeamsDB(ctx, ds.writer)
if err != nil {
return fleet.StatisticsPayload{}, false, ctxerr.Wrap(ctx, err, "amount teams")
}
amountPolicies, err := amountPoliciesDB(ds.writer)
amountPolicies, err := amountPoliciesDB(ctx, ds.writer)
if err != nil {
return fleet.StatisticsPayload{}, false, ctxerr.Wrap(ctx, err, "amount policies")
}
amountLabels, err := amountLabelsDB(ds.writer)
amountLabels, err := amountLabelsDB(ctx, ds.writer)
if err != nil {
return fleet.StatisticsPayload{}, false, ctxerr.Wrap(ctx, err, "amount labels")
}
+2 -2
View File
@@ -280,9 +280,9 @@ func (ds *Datastore) TeamEnrollSecrets(ctx context.Context, teamID uint) ([]*fle
return secrets, nil
}
func amountTeamsDB(db sqlx.Queryer) (int, error) {
func amountTeamsDB(ctx context.Context, db sqlx.QueryerContext) (int, error) {
var amount int
err := sqlx.Get(db, &amount, `SELECT count(*) FROM teams`)
err := sqlx.GetContext(ctx, db, &amount, `SELECT count(*) FROM teams`)
if err != nil {
return 0, err
}
+2 -2
View File
@@ -275,9 +275,9 @@ func (ds *Datastore) DeleteUser(ctx context.Context, id uint) error {
return ds.deleteEntity(ctx, usersTable, id)
}
func amountUsersDB(db sqlx.Queryer) (int, error) {
func amountUsersDB(ctx context.Context, db sqlx.QueryerContext) (int, error) {
var amount int
err := sqlx.Get(db, &amount, `SELECT count(*) FROM users`)
err := sqlx.GetContext(ctx, db, &amount, `SELECT count(*) FROM users`)
if err != nil {
return 0, err
}
+8
View File
@@ -103,6 +103,9 @@ type ModifyPolicyPayload struct {
Description *string `json:"description"`
// Resolution indicate the steps needed to solve a failing policy.
Resolution *string `json:"resolution"`
// Platform is a comma-separated string to indicate the target platforms.
// If non-nil, empty string targets all platforms.
Platform *string `json:"platform"`
}
// Verify verifies the policy payload is valid.
@@ -117,6 +120,11 @@ func (p ModifyPolicyPayload) Verify() error {
return err
}
}
if p.Platform != nil {
if err := verifyPolicyPlatforms(*p.Platform); err != nil {
return err
}
}
return nil
}
+2 -2
View File
@@ -270,7 +270,7 @@ func modifyGlobalPolicyEndpoint(ctx context.Context, request interface{}, svc fl
return modifyGlobalPolicyResponse{Policy: resp}, nil
}
func (svc Service) ModifyGlobalPolicy(ctx context.Context, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
func (svc *Service) ModifyGlobalPolicy(ctx context.Context, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
return svc.modifyPolicy(ctx, nil, id, p)
}
@@ -298,7 +298,7 @@ func applyPolicySpecsEndpoint(ctx context.Context, request interface{}, svc flee
}
// TODO: add tests for activities?
func (svc Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.PolicySpec) error {
func (svc *Service) ApplyPolicySpecs(ctx context.Context, policies []*fleet.PolicySpec) error {
checkGlobalPolicyAuth := false
for _, policy := range policies {
if err := policy.Verify(); err != nil {
+5 -2
View File
@@ -277,11 +277,11 @@ func modifyTeamPolicyEndpoint(ctx context.Context, request interface{}, svc flee
return modifyTeamPolicyResponse{Policy: resp}, nil
}
func (svc Service) ModifyTeamPolicy(ctx context.Context, teamID uint, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
func (svc *Service) ModifyTeamPolicy(ctx context.Context, teamID uint, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
return svc.modifyPolicy(ctx, &teamID, id, p)
}
func (svc Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p fleet.ModifyPolicyPayload) (*fleet.Policy, error) {
// First make sure the user can read the policies.
if err := svc.authz.Authorize(ctx, &fleet.Policy{
PolicyData: fleet.PolicyData{
@@ -317,6 +317,9 @@ func (svc Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p fl
if p.Resolution != nil {
policy.Resolution = p.Resolution
}
if p.Platform != nil {
policy.Platform = *p.Platform
}
logging.WithExtras(ctx, "name", policy.Name, "sql", policy.Query)
err = svc.ds.SavePolicy(ctx, policy)