Issue 3707 clean targets on delete (#3739)

* wip

* Delete targets when deleting teams, hosts, and labels

* Add changes file

* Fix error message

* Remove unused teamsTable

* Cleanup new pack

* Clean new packs at end of test
This commit is contained in:
Tomas Touceda 2022-01-19 10:28:08 -03:00 committed by GitHub
parent 331a04121a
commit eee539cccc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 100 additions and 5 deletions

View file

@ -0,0 +1 @@
* Cleanup pack targets when deleting hosts, labels, and teams

View file

@ -343,6 +343,12 @@ func (d *Datastore) DeleteHost(ctx context.Context, hid uint) error {
return err
}
}
_, err = tx.ExecContext(ctx, `DELETE FROM pack_targets WHERE type=? AND target_id=?`, fleet.TargetHost, hid)
if err != nil {
return ctxerr.Wrapf(ctx, err, "deleting pack_targets for host %d", hid)
}
return nil
})
}
@ -984,6 +990,17 @@ func (d *Datastore) DeleteHosts(ctx context.Context, ids []uint) error {
if err != nil {
return ctxerr.Wrap(ctx, err, "deleting host emails")
}
query, args, err = sqlx.In(`DELETE FROM pack_targets WHERE type=? AND target_id in (?)`, fleet.TargetHost, ids)
if err != nil {
return ctxerr.Wrapf(ctx, err, "building delete pack_targets query")
}
_, err = d.writer.ExecContext(ctx, query, args...)
if err != nil {
return ctxerr.Wrapf(ctx, err, "deleting pack_targets for hosts")
}
return nil
}

View file

@ -170,12 +170,24 @@ func testSaveHost(t *testing.T, ds *Datastore, saveHostFunc func(context.Context
require.NoError(t, err)
require.NotNil(t, host)
p, err := ds.NewPack(context.Background(), &fleet.Pack{
Name: t.Name(),
HostIDs: []uint{host.ID},
})
require.NoError(t, err)
err = ds.DeleteHost(context.Background(), host.ID)
assert.Nil(t, err)
newP, err := ds.Pack(context.Background(), p.ID)
require.NoError(t, err)
require.Empty(t, newP.Hosts)
host, err = ds.Host(context.Background(), host.ID, false)
assert.NotNil(t, err)
assert.Nil(t, host)
require.NoError(t, ds.DeletePack(context.Background(), newP.Name))
}
func testHostsDeleteWithSoftware(t *testing.T, ds *Datastore) {

View file

@ -245,6 +245,12 @@ func (d *Datastore) DeleteLabel(ctx context.Context, name string) error {
if err != nil {
return ctxerr.Wrapf(ctx, err, "delete label_membership")
}
_, err = tx.ExecContext(ctx, `DELETE FROM pack_targets WHERE type=? AND target_id=?`, fleet.TargetLabel, labelID)
if err != nil {
return ctxerr.Wrapf(ctx, err, "deleting pack_targets for label %d", labelID)
}
return nil
})
}

View file

@ -59,6 +59,7 @@ func TestLabels(t *testing.T) {
{"Save", testLabelsSave},
{"QueriesForCentOSHost", testLabelsQueriesForCentOSHost},
{"RecordNonExistentQueryLabelExecution", testLabelsRecordNonexistentQueryLabelExecution},
{"DeleteLabel", testDeleteLabel},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
@ -753,3 +754,25 @@ func testLabelsRecordNonexistentQueryLabelExecution(t *testing.T, db *Datastore)
require.NoError(t, db.RecordLabelQueryExecutions(context.Background(), h1, map[uint]*bool{99999: ptr.Bool(true)}, time.Now(), false))
}
func testDeleteLabel(t *testing.T, db *Datastore) {
l, err := db.NewLabel(context.Background(), &fleet.Label{
Name: t.Name(),
Query: "query1",
})
require.NoError(t, err)
p, err := db.NewPack(context.Background(), &fleet.Pack{
Name: t.Name(),
LabelIDs: []uint{l.ID},
})
require.NoError(t, err)
require.NoError(t, db.DeleteLabel(context.Background(), l.Name))
newP, err := db.Pack(context.Background(), p.ID)
require.NoError(t, err)
require.Empty(t, newP.Labels)
require.NoError(t, db.DeletePack(context.Background(), newP.Name))
}

View file

@ -74,7 +74,6 @@ var (
packsTable = entity{"packs"}
queriesTable = entity{"queries"}
sessionsTable = entity{"sessions"}
teamsTable = entity{"teams"}
usersTable = entity{"users"}
)

View file

@ -80,10 +80,19 @@ func saveTeamSecretsDB(ctx context.Context, exec sqlx.ExecerContext, team *fleet
}
func (d *Datastore) DeleteTeam(ctx context.Context, tid uint) error {
if err := d.deleteEntity(ctx, teamsTable, tid); err != nil {
return ctxerr.Wrapf(ctx, err, "delete team id %d", tid)
}
return nil
return d.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
_, err := tx.ExecContext(ctx, `DELETE FROM teams WHERE id = ?`, tid)
if err != nil {
return ctxerr.Wrapf(ctx, err, "delete team %d", tid)
}
_, err = tx.ExecContext(ctx, `DELETE FROM pack_targets WHERE type=? AND target_id=?`, fleet.TargetTeam, tid)
if err != nil {
return ctxerr.Wrapf(ctx, err, "deleting pack_targets for team %d", tid)
}
return nil
})
}
func (d *Datastore) TeamByName(ctx context.Context, name string) (*fleet.Team, error) {

View file

@ -64,11 +64,23 @@ func testTeamsGetSetDelete(t *testing.T, ds *Datastore) {
assert.Equal(t, tt.name, team.Name)
assert.Equal(t, tt.description, team.Description)
p, err := ds.NewPack(context.Background(), &fleet.Pack{
Name: tt.name,
TeamIDs: []uint{team.ID},
})
require.NoError(t, err)
err = ds.DeleteTeam(context.Background(), team.ID)
require.NoError(t, err)
newP, err := ds.Pack(context.Background(), p.ID)
require.NoError(t, err)
require.Empty(t, newP.Teams)
team, err = ds.TeamByName(context.Background(), tt.name)
require.Error(t, err)
require.NoError(t, ds.DeletePack(context.Background(), newP.Name))
})
}
}

View file

@ -511,6 +511,17 @@ func (s *integrationTestSuite) TestBulkDeleteHostsFromTeam() {
team1, err := s.ds.NewTeam(context.Background(), &fleet.Team{Name: t.Name() + "team1"})
require.NoError(t, err)
p, err := s.ds.NewPack(context.Background(), &fleet.Pack{
Name: t.Name(),
Hosts: []fleet.Target{
{
Type: fleet.TargetHost,
TargetID: hosts[0].ID,
},
},
})
require.NoError(t, err)
require.NoError(t, s.ds.AddHostsToTeam(context.Background(), &team1.ID, []uint{hosts[0].ID}))
req := deleteHostsRequest{
@ -533,6 +544,11 @@ func (s *integrationTestSuite) TestBulkDeleteHostsFromTeam() {
err = s.ds.DeleteHosts(context.Background(), []uint{hosts[1].ID, hosts[2].ID})
require.NoError(t, err)
newP, err := s.ds.Pack(context.Background(), p.ID)
require.NoError(t, err)
require.Empty(t, newP.Hosts)
require.NoError(t, s.ds.DeletePack(context.Background(), newP.Name))
}
func (s *integrationTestSuite) TestBulkDeleteHostsInLabel() {