Error on boundary database migrate if the db hasn't been initialized (#1184)

CHANGELOG.md

@@ -90,6 +90,9 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 
 * cors: Fix allowing all origins by default
   [PR](https://github.com/hashicorp/boundary/pull/1134)
+* cli: It is now an error to run `boundary database migrate` on an uninitalized db.
+  Use `boundary database init` instead.
+  ([PR](https://github.com/hashicorp/boundary/pull/1184))
 
 ## 0.2.0 (2021/04/14)
 

internal/cmd/commands/database/funcs.go

@@ -13,9 +13,10 @@ import (
 
 // migrateDatabase updates the schema to the most recent version known by the binary.
 // It owns the reporting to the UI any errors.
+// We expect the database already to be initialized iff initialized is set to true.
 // Returns a cleanup function which must be called even if an error is returned and
 // an error code where a non-zero value indicates an error happened.
-func migrateDatabase(ctx context.Context, ui cli.Ui, dialect, u string, requireFresh bool) (func(), int) {
+func migrateDatabase(ctx context.Context, ui cli.Ui, dialect, u string, initialized bool) (func(), int) {
 	noop := func() {}
 	// This database is used to keep an exclusive lock on the database for the
 	// remainder of the command
@@ -52,7 +53,11 @@ func migrateDatabase(ctx context.Context, ui cli.Ui, dialect, u string, requireF
 		ui.Error(fmt.Errorf("Error getting database state: %w", err).Error())
 		return unlock, 2
 	}
-	if requireFresh && st.InitializationStarted {
+	if initialized && !st.InitializationStarted {
+		ui.Output(base.WrapAtLength("Database has not been initialized. Please use 'boundary database init' to initialize the boundary database."))
+		return unlock, -1
+	}
+	if !initialized && st.InitializationStarted {
 		ui.Output(base.WrapAtLength("Database has already been initialized. Please use 'boundary database migrate' for any upgrade needs."))
 		return unlock, -1
 	}

internal/cmd/commands/database/funcs_test.go

@@ -19,14 +19,15 @@ func TestMigrateDatabase(t *testing.T) {
 
 	cases := []struct {
 		name           string
-		requireFresh   bool
+		initialized    bool
 		urlProvider    func() string
 		expectedCode   int
 		expectedOutput string
 		expectedError  string
 	}{
 		{
-			name: "basic",
+			name: "not_initialized_expected_not_intialized",
+			initialized: false,
 			urlProvider: func() string {
 				c, u, _, err := db.StartDbInDocker(dialect)
 				require.NoError(t, err)
@@ -39,7 +40,60 @@ func TestMigrateDatabase(t *testing.T) {
 			expectedOutput: "Migrations successfully run.\n",
 		},
 		{
-			name: "old_version_table_used",
+			name:        "basic_initialized_expects_initialized",
+			initialized: true,
+			urlProvider: func() string {
+				c, u, _, err := db.StartDbInDocker(dialect)
+				require.NoError(t, err)
+				t.Cleanup(func() {
+					require.NoError(t, c())
+				})
+
+				dBase, err := sql.Open(dialect, u)
+				require.NoError(t, err)
+
+				earlyMigrationVersion := 2000
+				oState := schema.TestCloneMigrationStates(t)
+				nState := schema.TestCreatePartialMigrationState(oState["postgres"], earlyMigrationVersion)
+				oState["postgres"] = nState
+
+				man, err := schema.NewManager(ctx, dialect, dBase, schema.WithMigrationStates(oState))
+				require.NoError(t, err)
+				require.NoError(t, man.RollForward(ctx))
+				return u
+			},
+			expectedCode:   0,
+			expectedOutput: "Migrations successfully run.\n",
+		},
+		{
+			name:        "basic_initialized_expects_not_initialized",
+			initialized: false,
+			urlProvider: func() string {
+				c, u, _, err := db.StartDbInDocker(dialect)
+				require.NoError(t, err)
+				t.Cleanup(func() {
+					require.NoError(t, c())
+				})
+
+				dBase, err := sql.Open(dialect, u)
+				require.NoError(t, err)
+
+				earlyMigrationVersion := 2000
+				oState := schema.TestCloneMigrationStates(t)
+				nState := schema.TestCreatePartialMigrationState(oState["postgres"], earlyMigrationVersion)
+				oState["postgres"] = nState
+
+				man, err := schema.NewManager(ctx, dialect, dBase, schema.WithMigrationStates(oState))
+				require.NoError(t, err)
+				require.NoError(t, man.RollForward(ctx))
+				return u
+			},
+			expectedCode:   -1,
+			expectedOutput: "Database has already been initialized. Please use 'boundary database migrate'\nfor any upgrade needs.\n",
+		},
+		{
+			name: "old_version_table_used_intialized",
+			initialized: true,
 			urlProvider: func() string {
 				c, u, _, err := db.StartDbInDocker(dialect)
 				require.NoError(t, err)
@@ -58,13 +112,42 @@ func TestMigrateDatabase(t *testing.T) {
 			expectedOutput: "Migrations successfully run.\n",
 		},
 		{
-			name:          "bad_url",
+			name:        "old_version_table_used_not_intialized",
+			initialized: false,
+			urlProvider: func() string {
+				c, u, _, err := db.StartDbInDocker(dialect)
+				require.NoError(t, err)
+				t.Cleanup(func() {
+					require.NoError(t, c())
+				})
+				dBase, err := sql.Open(dialect, u)
+				require.NoError(t, err)
+
+				createStmt := `create table if not exists schema_migrations (version bigint primary key, dirty boolean not null)`
+				_, err = dBase.Exec(createStmt)
+				require.NoError(t, err)
+				return u
+			},
+			expectedCode:   -1,
+			expectedOutput: "Database has already been initialized. Please use 'boundary database migrate'\nfor any upgrade needs.\n",
+		},
+		{
+			name:          "bad_url_initialized",
+			initialized: true,
 			urlProvider:   func() string { return "badurl" },
 			expectedCode:  2,
 			expectedError: "Unable to connect to the database at \"badurl\"\n",
 		},
 		{
-			name: "cant_get_lock",
+			name:          "bad_url_not_intialized",
+			initialized:   false,
+			urlProvider:   func() string { return "badurl" },
+			expectedCode:  2,
+			expectedError: "Unable to connect to the database at \"badurl\"\n",
+		},
+		{
+			name: "cant_get_lock_initialized",
+			initialized: true,
 			urlProvider: func() string {
 				c, u, _, err := db.StartDbInDocker(dialect)
 				require.NoError(t, err)
@@ -86,49 +169,8 @@ func TestMigrateDatabase(t *testing.T) {
 			expectedError: "Unable to capture a lock on the database.\n",
 		},
 		{
-			name:         "basic_require_fresh",
-			requireFresh: true,
-			urlProvider: func() string {
-				c, u, _, err := db.StartDbInDocker(dialect)
-				require.NoError(t, err)
-				t.Cleanup(func() {
-					require.NoError(t, c())
-				})
-				return u
-			},
-			expectedCode:   0,
-			expectedOutput: "Migrations successfully run.\n",
-		},
-		{
-			name:         "old_version_table_used_require_fresh",
-			requireFresh: true,
-			urlProvider: func() string {
-				c, u, _, err := db.StartDbInDocker(dialect)
-				require.NoError(t, err)
-				t.Cleanup(func() {
-					require.NoError(t, c())
-				})
-				dBase, err := sql.Open(dialect, u)
-				require.NoError(t, err)
-
-				createStmt := `create table if not exists schema_migrations (version bigint primary key, dirty boolean not null)`
-				_, err = dBase.Exec(createStmt)
-				require.NoError(t, err)
-				return u
-			},
-			expectedCode:   -1,
-			expectedOutput: "Database has already been initialized. Please use 'boundary database migrate'\nfor any upgrade needs.\n",
-		},
-		{
-			name:          "bad_url_require_fresh",
-			requireFresh:  true,
-			urlProvider:   func() string { return "badurl" },
-			expectedCode:  2,
-			expectedError: "Unable to connect to the database at \"badurl\"\n",
-		},
-		{
-			name:         "cant_get_lock_require_fresh",
-			requireFresh: true,
+			name:        "cant_get_lock_not_initialized",
+			initialized: false,
 			urlProvider: func() string {
 				c, u, _, err := db.StartDbInDocker(dialect)
 				require.NoError(t, err)
@@ -155,7 +197,7 @@ func TestMigrateDatabase(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			u := tc.urlProvider()
 			ui := cli.NewMockUi()
-			clean, errCode := migrateDatabase(ctx, ui, dialect, u, tc.requireFresh)
+			clean, errCode := migrateDatabase(ctx, ui, dialect, u, tc.initialized)
 			clean()
 			assert.EqualValues(t, tc.expectedCode, errCode)
 			assert.Equal(t, tc.expectedOutput, ui.OutputWriter.String())

internal/cmd/commands/database/init.go

@@ -239,7 +239,7 @@ func (c *InitCommand) Run(args []string) (retCode int) {
 		return base.CommandUserError
 	}
 
-	clean, errCode := migrateDatabase(c.Context, c.UI, dialect, migrationUrl, true)
+	clean, errCode := migrateDatabase(c.Context, c.UI, dialect, migrationUrl, false)
 	defer clean()
 	switch errCode {
 	case 0:

internal/cmd/commands/database/migrate.go

@@ -180,7 +180,7 @@ func (c *MigrateCommand) Run(args []string) (retCode int) {
 		return base.CommandUserError
 	}
 
-	clean, errCode := migrateDatabase(c.Context, c.UI, dialect, migrationUrl, false)
+	clean, errCode := migrateDatabase(c.Context, c.UI, dialect, migrationUrl, true)
 	defer clean()
 	if errCode != 0 {
 		return errCode