diff --git a/changes/issue-1569-disallow-target-id-null b/changes/issue-1569-disallow-target-id-null new file mode 100644 index 0000000000..f46c12c6b3 --- /dev/null +++ b/changes/issue-1569-disallow-target-id-null @@ -0,0 +1 @@ +* Disallow target_id NULL for pack_targets to prevent issues when listing packs. This could happen because of a pack spec applied with a label name that was not existent anymore. diff --git a/server/datastore/mysql/migrations/tables/20210806112844_NotNullTargetID.go b/server/datastore/mysql/migrations/tables/20210806112844_NotNullTargetID.go new file mode 100644 index 0000000000..aef4bb912d --- /dev/null +++ b/server/datastore/mysql/migrations/tables/20210806112844_NotNullTargetID.go @@ -0,0 +1,26 @@ +package tables + +import ( + "database/sql" + + "github.com/pkg/errors" +) + +func init() { + MigrationClient.AddMigration(Up_20210806112844, Down_20210806112844) +} + +func Up_20210806112844(tx *sql.Tx) error { + if _, err := tx.Exec(`DELETE FROM pack_targets WHERE target_id is NULL`); err != nil { + return errors.Wrap(err, "delete target_id null pack targets") + } + + if _, err := tx.Exec(`ALTER TABLE pack_targets MODIFY target_id int unsigned NOT NULL`); err != nil { + return errors.Wrap(err, "make pack_targets.target_id not null") + } + return nil +} + +func Down_20210806112844(tx *sql.Tx) error { + return nil +} diff --git a/server/datastore/mysql/packs_test.go b/server/datastore/mysql/packs_test.go index 806b886537..e98abd4266 100644 --- a/server/datastore/mysql/packs_test.go +++ b/server/datastore/mysql/packs_test.go @@ -493,3 +493,23 @@ func TestEnsureTeamPack(t *testing.T) { assert.Equal(t, fmt.Sprintf("team-%d", team2.ID), *tp2.Type) assert.Equal(t, []uint{team2.ID}, tp2.TeamIDs) } + +func TestApplyPackSpecFailsOnTargetIDNull(t *testing.T) { + ds := CreateMySQLDS(t) + defer ds.Close() + + // Do not define queries mentioned in spec + specs := []*fleet.PackSpec{ + { + ID: 1, + Name: "test_pack", + Targets: fleet.PackSpecTargets{ + Labels: []string{"UnexistentLabel"}, + }, + }, + } + + // Should error due to unkown label target id + err := ds.ApplyPackSpecs(specs) + require.Error(t, err) +}