Disallow target ids null altogether (#1578)

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.

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
+}

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)
+}