From 895de869ab02bb0f83c0ec66ef957a0099c2a419 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Fri, 9 Jul 2021 08:19:16 -0700 Subject: [PATCH] internal/db: add enumeration constraints for the server table (#1386) This adds enumeration constraints to the server table, ensuring that no servers can be added other than the types "controller" and "worker". This should already be impossible to do in code, but this adds a check on the database level to ensure it's the case as well. --- .../postgres/11/01_server_type_enum.up.sql | 27 +++ .../postgres/11/01_server_type_enum_test.go | 156 ++++++++++++++++++ internal/db/schema/postgres_migration.gen.go | 27 ++- 3 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 internal/db/schema/migrations/postgres/11/01_server_type_enum.up.sql create mode 100644 internal/db/schema/migrations/postgres/11/01_server_type_enum_test.go diff --git a/internal/db/schema/migrations/postgres/11/01_server_type_enum.up.sql b/internal/db/schema/migrations/postgres/11/01_server_type_enum.up.sql new file mode 100644 index 0000000000..843c2bfc76 --- /dev/null +++ b/internal/db/schema/migrations/postgres/11/01_server_type_enum.up.sql @@ -0,0 +1,27 @@ +begin; + +create table server_type_enm ( + name text primary key + constraint only_predefined_server_types_allowed + check ( + name in ( + 'controller', + 'worker' + ) + ) +); +comment on table server_type_enm is + 'server_type_enm is an enumeration table for server types. ' + 'It contains rows for representing servers as either a controller or worker.'; + +insert into server_type_enm (name) values + ('controller'), + ('worker'); + +alter table server + add constraint server_type_enm_fkey + foreign key (type) references server_type_enm(name) + on update cascade + on delete restrict; + +commit; diff --git a/internal/db/schema/migrations/postgres/11/01_server_type_enum_test.go b/internal/db/schema/migrations/postgres/11/01_server_type_enum_test.go new file mode 100644 index 0000000000..bd77068411 --- /dev/null +++ b/internal/db/schema/migrations/postgres/11/01_server_type_enum_test.go @@ -0,0 +1,156 @@ +package migration + +import ( + "context" + "database/sql" + "testing" + "time" + + "github.com/hashicorp/boundary/internal/db/schema" + "github.com/hashicorp/boundary/internal/docker" + "github.com/stretchr/testify/require" +) + +const ( + insertServerQuery = "insert into server (private_id, type, address) values ($1, $2, $3)" + selectServerQuery = "select * from server where private_id = $1" + expectEnumConstraintErr = `pq: new row for relation "server_type_enm" violates check constraint "only_predefined_server_types_allowed"` + expectServerConstraintErr = `pq: insert or update on table "server" violates foreign key constraint "server_type_enm_fkey"` +) + +type testServer struct { + PrivateId string + Type string + Description sql.NullString + Address string + CreateTime time.Time + UpdateTime time.Time +} + +// this is a sequential test which relies on: +// 1) initializing the db using a migration up to the "priorMigration" +// +// 2) seeding the database with a test controller and worker +// +// 3) running the enum migration +// +// 4) asserting some bits about the state of the db. +func Test_ServerEnumChanges(t *testing.T) { + t.Parallel() + require := require.New(t) + const priorMigration = 10007 + const serverEnumMigration = 11001 + dialect := "postgres" + ctx := context.Background() + + c, u, _, err := docker.StartDbInDocker(dialect) + require.NoError(err) + t.Cleanup(func() { + require.NoError(c()) + }) + d, err := sql.Open(dialect, u) + require.NoError(err) + + // migration to the prior migration (before the one we want to test) + oState := schema.TestCloneMigrationStates(t) + nState := schema.TestCreatePartialMigrationState(oState["postgres"], priorMigration) + oState["postgres"] = nState + + m, err := schema.NewManager(ctx, dialect, d, schema.WithMigrationStates(oState)) + require.NoError(err) + + require.NoError(m.RollForward(ctx)) + state, err := m.CurrentState(ctx) + require.NoError(err) + require.Equal(priorMigration, state.DatabaseSchemaVersion) + require.False(state.Dirty) + + // Seed the data + expectedController := insertServer(ctx, t, d, "test-controller", "controller", "127.0.0.1") + expectedWorker := insertServer(ctx, t, d, "test-worker", "worker", "127.0.0.1") + + // now we're ready for the migration we want to test. + oState = schema.TestCloneMigrationStates(t) + nState = schema.TestCreatePartialMigrationState(oState["postgres"], serverEnumMigration) + oState["postgres"] = nState + + m, err = schema.NewManager(ctx, dialect, d, schema.WithMigrationStates(oState)) + require.NoError(err) + + require.NoError(m.RollForward(ctx)) + state, err = m.CurrentState(ctx) + require.NoError(err) + require.Equal(serverEnumMigration, state.DatabaseSchemaVersion) + require.False(state.Dirty) + + t.Log("current migration version: ", state.DatabaseSchemaVersion) + + // Assert servers + actualController := selectServer(ctx, t, d, "test-controller") + actualWorker := selectServer(ctx, t, d, "test-worker") + require.Equal(expectedController, actualController) + require.Equal(expectedWorker, actualWorker) + + // Assert the state of the newly created enum table + rows, err := d.QueryContext(ctx, "select * from server_type_enm") + require.NoError(err) + + // Inserted rows + var actualEnm []string + for rows.Next() { + var a string + require.NoError(rows.Scan(&a)) + actualEnm = append(actualEnm, a) + } + require.Equal([]string{"controller", "worker"}, actualEnm) + + // Try inserting a broken row + result, err := d.ExecContext(ctx, "insert into server_type_enm values ($1)", "bad") + require.EqualError(err, expectEnumConstraintErr) + require.Nil(result) + + // Try adding a broken server type + result, err = d.ExecContext(ctx, insertServerQuery, []interface{}{"test-bad", "bad", "127.0.0.1"}...) + require.EqualError(err, expectServerConstraintErr) + require.Nil(result) + + // Add another controller and worker + insertServer(ctx, t, d, "test-controller-new", "controller", "127.0.0.1") + insertServer(ctx, t, d, "test-worker-new", "worker", "127.0.0.1") + + // Assert length + var actualLen int + row := d.QueryRowContext(ctx, "select count(*) from server") + require.NoError(row.Scan(&actualLen)) + require.Equal(4, actualLen) +} + +func insertServer(ctx context.Context, t *testing.T, d *sql.DB, privateId, serverType, address string) *testServer { + t.Helper() + require := require.New(t) + execResult, err := d.ExecContext(ctx, insertServerQuery, privateId, serverType, address) + require.NoError(err) + rowsAffected, err := execResult.RowsAffected() + require.NoError(err) + require.Equal(int64(1), rowsAffected) + + return selectServer(ctx, t, d, privateId) +} + +func selectServer(ctx context.Context, t *testing.T, d *sql.DB, privateId string) *testServer { + t.Helper() + require := require.New(t) + + serverResult := new(testServer) + row := d.QueryRowContext(ctx, selectServerQuery, privateId) + require.NoError(row.Scan( + &serverResult.PrivateId, + &serverResult.Type, + &serverResult.Description, + &serverResult.Address, + &serverResult.CreateTime, + &serverResult.UpdateTime, + )) + + return serverResult +} diff --git a/internal/db/schema/postgres_migration.gen.go b/internal/db/schema/postgres_migration.gen.go index ac5b625b4f..522016df98 100644 --- a/internal/db/schema/postgres_migration.gen.go +++ b/internal/db/schema/postgres_migration.gen.go @@ -4,7 +4,7 @@ package schema func init() { migrationStates["postgres"] = migrationState{ - binarySchemaVersion: 10007, + binarySchemaVersion: 11001, upMigrations: map[int][]byte{ 1: []byte(` create domain wt_public_id as text @@ -6075,6 +6075,31 @@ comment on domain wt_sentinel is 'A non-empty string with a Unicode prefix of U+FFFE and suffix of U+FFFF to indicate it is a sentinel value'; drop function wt_to_sentinel; -- wt_to_sentinel is not needed, dropping and not re-creating +`), + 11001: []byte(` +create table server_type_enm ( + name text primary key + constraint only_predefined_server_types_allowed + check ( + name in ( + 'controller', + 'worker' + ) + ) +); +comment on table server_type_enm is + 'server_type_enm is an enumeration table for server types. ' + 'It contains rows for representing servers as either a controller or worker.'; + +insert into server_type_enm (name) values + ('controller'), + ('worker'); + +alter table server + add constraint server_type_enm_fkey + foreign key (type) references server_type_enm(name) + on update cascade + on delete restrict; `), 2001: []byte(` -- log_migration entries represent logs generated during migrations