Separate tables from data migrations (#745)

This PR separates the table migrations from the data population migrations. Table migrations run before data migrations.

Now, we have the ability to create the database tables without populating them with data. This can be useful for running "unit" tests against a MySQL store that doesn't have any pre-populated data. When performing real migrations, or for more "integration" style testing, the data migrations can also be executed.

Note there are some special cases that must be observed with these migrations, and the README is updated to reflect those.

README.md

@@ -141,20 +141,33 @@ To update, use [`glide up`](https://github.com/Masterminds/glide#glide-update-al
 
 #### Database Modifications
 
-  * From the project root run the following commands:
+##### Adding/Updating tables
+
+Database schemas are managed by a series of migrations defined in go code. We
+use a customized version of the Goose migrations tool to handle these
+migrations.
+
+Note: Once committed to the Kolide repo, table migrations should be considered
+immutable. Any changes to an existing table should take place in a new
+migration executing ALTERs.
+
+  * From the project root run the following shell commands:
+
   ``` bash
-  > go get github.com/pressly/goose
-  > cd server/datastore/mysql/migrations
-  > goose create AddColumnFooToUsers
+  go get github.com/kolide/goose
+  cd server/datastore/mysql/migrations/tables
+  goose create AddColumnFooToUsers
   ```
+
   * Find the file you created in the migrations directory and edit it
+
   ``` go
   package migration
 
   import (
   	"database/sql"
 
-  	"github.com/pressly/goose"
+  	"github.com/kolide/goose"
   )
 
   func init() {
@@ -171,12 +184,35 @@ To update, use [`glide up`](https://github.com/Masterminds/glide#glide-update-al
   	return err
   }
   ```
-  * Update the database by running the following command
+
+  * Update the database by running the following shell commands:
+
   ``` bash
-  > make build
-  > build/kolide prepare db
+  make build
+  build/kolide prepare db
   ```
 
+##### Populating the database
+
+Populating built in data is also performed through migrations. All table
+migrations are performed before any data migrations.
+
+Note: Data migrations can be mutable. If tables are altered in a way that would
+render a data migration invalid (columns changed/removed), data migrations
+should be updated to comply with the new schema. Data migrations will not be
+re-run when they have already been run against a database, but they must be
+updated to maintain compatibility with a fresh DB.
+
+  * From the project root run the following shell commands:
+
+  ``` bash
+  go get github.com/kolide/goose
+  cd server/datastore/mysql/migrations/data
+  goose create PopulateFoo
+  ```
+
+  * Proceed as for table migrations, editing and running the newly created
+    migration file.
 
 
 ### Testing

cli/prepare.go

@@ -38,9 +38,13 @@ To setup kolide infrastructure, use one of the available commands.
 				initFatal(err, "creating db connection")
 			}
 
-			if err := ds.Migrate(); err != nil {
+			if err := ds.MigrateTables(); err != nil {
 				initFatal(err, "migrating db schema")
 			}
+
+			if err := ds.MigrateData(); err != nil {
+				initFatal(err, "migrating builtin data")
+			}
 		},
 	}
 

glide.lock

@@ -1,5 +1,5 @@
-hash: 4a87ddb9e91081df70590230979f58985cc7a9a1f9f6be2e1033e4b84cf2c6e0
-updated: 2016-11-18T19:27:52.047338531+08:00
+hash: e9cd204c0dd7e3e9102a2e0c87cc722101473f0d8fd31b05ec7f88837dd1d529
+updated: 2017-01-04T15:27:20.412164218-08:00
 imports:
 - name: github.com/alecthomas/template
   version: a0175ee3bccc567396460bf5acd36800cb10c49c
@@ -11,6 +11,10 @@ imports:
   version: 4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9
   subpackages:
   - quantile
+- name: github.com/davecgh/go-spew
+  version: 6d212800a42e8ab5c146b8ace3490ee17e5225f9
+  subpackages:
+  - spew
 - name: github.com/dgrijalva/jwt-go
   version: 01aeca54ebda6e0fbfafd0a524d234159c05ec20
 - name: github.com/elazarl/go-bindata-assetfs
@@ -68,6 +72,8 @@ imports:
   - reflectx
 - name: github.com/jordan-wright/email
   version: fd703108daeb23d77c124d12978e9b6c4f28f034
+- name: github.com/kolide/goose
+  version: 5221ed7545f6b931a24f99d9bd3e7bcf36aee71b
 - name: github.com/kr/fs
   version: 2788f0dbd16903de03cb8186e5c7d97b69ad387b
 - name: github.com/kr/logfmt
@@ -94,12 +100,11 @@ imports:
   version: 792786c7400a136282c1664665ae0a8db921c6c2
   subpackages:
   - difflib
-- name: github.com/pressly/goose
-  version: d6e8fe0292718c30342b3ed7f324dc21634b3bc6
 - name: github.com/prometheus/client_golang
   version: c5b7fccd204277076155f10851dad72b76a49317
   subpackages:
   - prometheus
+  - prometheus/promhttp
 - name: github.com/prometheus/client_model
   version: fa8ad6fec33561be4280a8f0514318c79d7f6cb6
   subpackages:
@@ -162,11 +167,7 @@ imports:
 - name: gopkg.in/go-playground/validator.v8
   version: 5f57d2222ad794d0dffb07e664ea05e2ee07d60c
 - name: gopkg.in/natefinch/lumberjack.v2
-  version: e21e5cbec0cd0861b9dc302736ad5666c529d93f
+  version: dd45e6a67c53f673bb49ca8a001fd3a63ceb640e
 - name: gopkg.in/yaml.v2
   version: e4d366fc3c7938e2958e662b4258c7a89e1f0e3e
-testImports:
-- name: github.com/davecgh/go-spew
-  version: 6d212800a42e8ab5c146b8ace3490ee17e5225f9
-  subpackages:
-  - spew
+testImports: []

glide.yaml

@@ -59,4 +59,4 @@ import:
   subpackages:
   - redis
 - package: github.com/jmoiron/sqlx
-- package: github.com/pressly/goose
+- package: github.com/kolide/goose

main.go

@@ -6,7 +6,6 @@ import (
 
 	_ "github.com/go-sql-driver/mysql"
 	"github.com/kolide/kolide-ose/cli"
-	_ "github.com/kolide/kolide-ose/server/datastore/mysql/migrations"
 )
 
 func init() {

server/datastore/datastore_options_test.go

@@ -11,6 +11,8 @@ import (
 )
 
 func testOptions(t *testing.T, ds kolide.Datastore) {
+	require.Nil(t, ds.MigrateData())
+
 	// were options pre-loaded?
 	opts, err := ds.ListOptions()
 	require.Nil(t, err)
@@ -58,6 +60,8 @@ func testOptions(t *testing.T, ds kolide.Datastore) {
 }
 
 func testOptionsToConfig(t *testing.T, ds kolide.Datastore) {
+	require.Nil(t, ds.MigrateData())
+
 	resp, err := ds.GetOsqueryConfigOptions()
 	require.Nil(t, err)
 	assert.Len(t, resp, 10)

server/datastore/inmem/inmem.go

@@ -43,7 +43,7 @@ func New(config config.KolideConfig) (*Datastore, error) {
 		config: &config,
 	}
 
-	if err := ds.Migrate(); err != nil {
+	if err := ds.MigrateTables(); err != nil {
 		return nil, err
 	}
 
@@ -69,9 +69,10 @@ func sortResults(slice interface{}, opt kolide.ListOptions, fields map[string]st
 	return nil
 }
 
-func (d *Datastore) Migrate() error {
+func (d *Datastore) MigrateTables() error {
 	d.mtx.Lock()
 	defer d.mtx.Unlock()
+
 	d.nextIDs = make(map[interface{}]uint)
 	d.users = make(map[uint]*kolide.User)
 	d.sessions = make(map[uint]*kolide.Session)
@@ -89,6 +90,13 @@ func (d *Datastore) Migrate() error {
 	d.distributedQueryCampaignTargets = make(map[uint]kolide.DistributedQueryCampaignTarget)
 	d.options = make(map[uint]*kolide.Option)
 
+	return nil
+}
+
+func (d *Datastore) MigrateData() error {
+	d.mtx.Lock()
+	defer d.mtx.Unlock()
+
 	for _, initData := range appstate.Options {
 		opt := kolide.Option{
 			Name:     initData.Name,
@@ -107,11 +115,12 @@ func (d *Datastore) Migrate() error {
 		SMTPEnableStartTLS: true,
 		SMTPVerifySSLCerts: true,
 	}
+
 	return nil
 }
 
 func (d *Datastore) Drop() error {
-	return d.Migrate()
+	return d.MigrateTables()
 }
 
 func (d *Datastore) Initialize() error {

server/datastore/mysql/migrations/data/20161223115449_InsertOsqueryOptions.go

@@ -1,26 +1,25 @@
-package migration
+package data
 
 import (
 	"database/sql"
 
 	"github.com/kolide/kolide-ose/server/datastore/internal/appstate"
 	"github.com/kolide/kolide-ose/server/kolide"
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161223115449, Down_20161223115449)
+	MigrationClient.AddMigration(Up_20161223115449, Down_20161223115449)
 }
 
 func Up_20161223115449(tx *sql.Tx) error {
 	sqlStatement := `
-    INSERT INTO options (
-      name,
-      type,
-      value,
-      read_only
-    ) VALUES( ?, ?, ?, ?)
-  `
+		INSERT INTO options (
+			name,
+			type,
+			value,
+			read_only
+		) VALUES (?, ?, ?, ?)
+	`
 
 	for _, opt := range appstate.Options {
 		ov := kolide.Option{

server/datastore/mysql/migrations/data/migration.go

@@ -0,0 +1,7 @@
+package data
+
+import "github.com/kolide/goose"
+
+var (
+	MigrationClient = goose.New("migration_status_data", goose.MySqlDialect{})
+)

server/datastore/mysql/migrations/tables/20161118193812_CreateTableAppConfigs.go

@@ -1,13 +1,9 @@
-package migration
+package tables
 
-import (
-	"database/sql"
-
-	"github.com/pressly/goose"
-)
+import "database/sql"
 
 func init() {
-	goose.AddMigration(Up_20161118193812, Down_20161118193812)
+	MigrationClient.AddMigration(Up_20161118193812, Down_20161118193812)
 }
 
 func Up_20161118193812(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118211713_CreateTableDistributedQueryCampaignTargets.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118211713, Down_20161118211713)
+	MigrationClient.AddMigration(Up_20161118211713, Down_20161118211713)
 }
 
 func Up_20161118211713(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212436_CreateTableDistributedQueryCampaigns.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212436, Down_20161118212436)
+	MigrationClient.AddMigration(Up_20161118212436, Down_20161118212436)
 }
 
 func Up_20161118212436(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212515_CreateTableDistributedQueryExecutions.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212515, Down_20161118212515)
+	MigrationClient.AddMigration(Up_20161118212515, Down_20161118212515)
 }
 
 func Up_20161118212515(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212528_CreateTableHosts.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212528, Down_20161118212528)
+	MigrationClient.AddMigration(Up_20161118212528, Down_20161118212528)
 }
 
 func Up_20161118212528(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212538_CreateTableInvites.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212538, Down_20161118212538)
+	MigrationClient.AddMigration(Up_20161118212538, Down_20161118212538)
 }
 
 func Up_20161118212538(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212549_CreateTableLabelQueryExecutions.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212549, Down_20161118212549)
+	MigrationClient.AddMigration(Up_20161118212549, Down_20161118212549)
 }
 
 func Up_20161118212549(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212557_CreateTableLabels.go

@@ -1,15 +1,14 @@
-package migration
+package tables
 
 import (
 	"database/sql"
 	"fmt"
 
 	"github.com/kolide/kolide-ose/server/kolide"
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212557, Down_20161118212557)
+	MigrationClient.AddMigration(Up_20161118212557, Down_20161118212557)
 }
 
 func Up_20161118212557(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212604_CreateTableOptions.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212604, Down_20161118212604)
+	MigrationClient.AddMigration(Up_20161118212604, Down_20161118212604)
 }
 
 func Up_20161118212604(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212613_CreateTableScheduledQueries.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212613, Down_20161118212613)
+	MigrationClient.AddMigration(Up_20161118212613, Down_20161118212613)
 }
 
 func Up_20161118212613(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212621_CreateTablePackTargets.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212621, Down_20161118212621)
+	MigrationClient.AddMigration(Up_20161118212621, Down_20161118212621)
 }
 
 func Up_20161118212621(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212630_CreateTablePacks.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212630, Down_20161118212630)
+	MigrationClient.AddMigration(Up_20161118212630, Down_20161118212630)
 }
 
 func Up_20161118212630(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212641_CreateTablePasswordResetRequests.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212641, Down_20161118212641)
+	MigrationClient.AddMigration(Up_20161118212641, Down_20161118212641)
 }
 
 func Up_20161118212641(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212649_CreateTableUsers.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212649, Down_20161118212649)
+	MigrationClient.AddMigration(Up_20161118212649, Down_20161118212649)
 }
 
 func Up_20161118212649(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212656_CreateTableSessions.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212656, Down_20161118212656)
+	MigrationClient.AddMigration(Up_20161118212656, Down_20161118212656)
 }
 
 func Up_20161118212656(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161118212758_CreateTableQueries.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161118212758, Down_20161118212758)
+	MigrationClient.AddMigration(Up_20161118212758, Down_20161118212758)
 }
 
 func Up_20161118212758(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161128234849_CreateTableNetworkInterfaces.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161128234849, Down_20161128234849)
+	MigrationClient.AddMigration(Up_20161128234849, Down_20161128234849)
 }
 
 func Up_20161128234849(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20161230162221_AddUniqueIndexToPackTargets.go

@@ -1,13 +1,11 @@
-package migration
+package tables
 
 import (
 	"database/sql"
-
-	"github.com/pressly/goose"
 )
 
 func init() {
-	goose.AddMigration(Up_20161230162221, Down_20161230162221)
+	MigrationClient.AddMigration(Up_20161230162221, Down_20161230162221)
 }
 
 func Up_20161230162221(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/20170104113816_AddSeenTimeToHosts.go

@@ -1,13 +1,9 @@
-package migration
+package tables
 
-import (
-	"database/sql"
-
-	"github.com/pressly/goose"
-)
+import "database/sql"
 
 func init() {
-	goose.AddMigration(Up_20170104113816, Down_20170104113816)
+	MigrationClient.AddMigration(Up_20170104113816, Down_20170104113816)
 }
 
 func Up_20170104113816(tx *sql.Tx) error {

server/datastore/mysql/migrations/tables/migration.go

@@ -0,0 +1,7 @@
+package tables
+
+import "github.com/kolide/goose"
+
+var (
+	MigrationClient = goose.New("migration_status_tables", goose.MySqlDialect{})
+)

server/datastore/mysql/mysql.go

@@ -8,8 +8,9 @@ import (
 	"github.com/go-kit/kit/log"
 	"github.com/jmoiron/sqlx"
 	"github.com/kolide/kolide-ose/server/config"
+	"github.com/kolide/kolide-ose/server/datastore/mysql/migrations/data"
+	"github.com/kolide/kolide-ose/server/datastore/mysql/migrations/tables"
 	"github.com/kolide/kolide-ose/server/kolide"
-	"github.com/pressly/goose"
 )
 
 const (
@@ -73,17 +74,20 @@ func (d *Datastore) Name() string {
 	return "mysql"
 }
 
-// Migrate creates database
-func (d *Datastore) Migrate() error {
-
-	goose.SetDialect("mysql")
-
-	if err := goose.Run("up", d.db.DB, "."); err != nil {
+func (d *Datastore) MigrateTables() error {
+	if err := tables.MigrationClient.Up(d.db.DB, ""); err != nil {
 		return err
 	}
 
 	return nil
+}
 
+func (d *Datastore) MigrateData() error {
+	if err := data.MigrationClient.Up(d.db.DB, ""); err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // Initialize preload data needed by the application

server/datastore/mysql_test.go

@@ -9,7 +9,6 @@ import (
 	_ "github.com/go-sql-driver/mysql"
 	"github.com/kolide/kolide-ose/server/config"
 	"github.com/kolide/kolide-ose/server/datastore/mysql"
-	_ "github.com/kolide/kolide-ose/server/datastore/mysql/migrations"
 	"github.com/stretchr/testify/require"
 )
 
@@ -49,7 +48,7 @@ func TestMySQL(t *testing.T) {
 
 		t.Run(functionName(f), func(t *testing.T) {
 			defer func() { require.Nil(t, ds.Drop()) }()
-			require.Nil(t, ds.Migrate())
+			require.Nil(t, ds.MigrateTables())
 
 			f(t, ds)
 		})

server/kolide/datastore.go

@@ -16,7 +16,10 @@ type Datastore interface {
 	OptionStore
 	Name() string
 	Drop() error
-	Migrate() error
+	// MigrateTables creates and migrates the table schemas
+	MigrateTables() error
+	// MigrateData populates built-in data
+	MigrateData() error
 }
 
 // NotFoundError is returned when the datastore resource cannot be found.

server/service/endpoint_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/kolide/kolide-ose/server/config"
 	"github.com/kolide/kolide-ose/server/datastore/inmem"
 	"github.com/kolide/kolide-ose/server/kolide"
+	"github.com/stretchr/testify/require"
 )
 
 type testResource struct {
@@ -29,8 +30,11 @@ type testResource struct {
 func setupEndpointTest(t *testing.T) *testResource {
 	test := &testResource{}
 
-	jwtKey := "CHANGEME"
-	test.ds, _ = inmem.New(config.TestConfig())
+	var err error
+	test.ds, err = inmem.New(config.TestConfig())
+	require.Nil(t, err)
+	require.Nil(t, test.ds.MigrateData())
+
 	devOrgInfo := &kolide.AppConfig{
 		OrgName:                "Kolide",
 		OrgLogoURL:             "http://foo.bar/image.png",
@@ -45,6 +49,7 @@ func setupEndpointTest(t *testing.T) *testResource {
 	createTestUsers(t, test.ds)
 	logger := kitlog.NewLogfmtLogger(os.Stdout)
 
+	jwtKey := "CHANGEME"
 	opts := []kithttp.ServerOption{
 		kithttp.ServerBefore(setRequestsContexts(svc, jwtKey)),
 		kithttp.ServerErrorLogger(logger),

server/service/service_appconfig_test.go

@@ -14,6 +14,8 @@ import (
 func TestCreateAppConfig(t *testing.T) {
 	ds, err := inmem.New(config.TestConfig())
 	require.Nil(t, err)
+	require.Nil(t, ds.MigrateData())
+
 	svc, err := newTestService(ds, nil)
 	require.Nil(t, err)
 	var appConfigTests = []struct {

server/service/service_osquery_test.go

@@ -377,7 +377,8 @@ func TestLabelQueries(t *testing.T) {
 
 func TestGetClientConfig(t *testing.T) {
 	ds, err := inmem.New(config.TestConfig())
-	assert.Nil(t, err)
+	require.Nil(t, err)
+	require.Nil(t, ds.MigrateData())
 
 	mockClock := clock.NewMockClock()