From 3bf053a5fc41948c8f6686ea8e4193f192a4760e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Wed, 14 Jul 2021 11:57:28 +0200 Subject: [PATCH 1/3] Add a test to ensure that workspaces must have a unique name in pg backend --- .../backend/remote-state/pg/backend_test.go | 48 ++++++++++++++----- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/internal/backend/remote-state/pg/backend_test.go b/internal/backend/remote-state/pg/backend_test.go index 815adf54cc..da058483d8 100644 --- a/internal/backend/remote-state/pg/backend_test.go +++ b/internal/backend/remote-state/pg/backend_test.go @@ -89,11 +89,13 @@ func TestBackendConfigSkipOptions(t *testing.T) { SkipSchemaCreation bool SkipTableCreation bool SkipIndexCreation bool + TestIndexIsPresent bool Setup func(t *testing.T, db *sql.DB, schemaName string) }{ { Name: "skip_schema_creation", SkipSchemaCreation: true, + TestIndexIsPresent: true, Setup: func(t *testing.T, db *sql.DB, schemaName string) { // create the schema as a prerequisites _, err := db.Query(fmt.Sprintf(`CREATE SCHEMA IF NOT EXISTS %s`, schemaName)) @@ -103,8 +105,9 @@ func TestBackendConfigSkipOptions(t *testing.T) { }, }, { - Name: "skip_table_creation", - SkipTableCreation: true, + Name: "skip_table_creation", + SkipTableCreation: true, + TestIndexIsPresent: true, Setup: func(t *testing.T, db *sql.DB, schemaName string) { // since the table needs to be already created the schema must be too _, err := db.Query(fmt.Sprintf(`CREATE SCHEMA %s`, schemaName)) @@ -122,8 +125,9 @@ func TestBackendConfigSkipOptions(t *testing.T) { }, }, { - Name: "skip_index_creation", - SkipIndexCreation: true, + Name: "skip_index_creation", + SkipIndexCreation: true, + TestIndexIsPresent: true, Setup: func(t *testing.T, db *sql.DB, schemaName string) { // Everything need to exists for the index to be created _, err := db.Query(fmt.Sprintf(`CREATE SCHEMA %s`, schemaName)) @@ -144,6 +148,10 @@ func TestBackendConfigSkipOptions(t *testing.T) { } }, }, + { + Name: "missing_index", + SkipIndexCreation: true, + }, } for _, tc := range testCases { @@ -163,7 +171,9 @@ func TestBackendConfigSkipOptions(t *testing.T) { t.Fatal(err) } - tc.Setup(t, db, schemaName) + if tc.Setup != nil { + tc.Setup(t, db, schemaName) + } defer db.Query(fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE", schemaName)) b := backend.TestBackendConfig(t, New(), config).(*Backend) @@ -179,14 +189,16 @@ func TestBackendConfigSkipOptions(t *testing.T) { if err != nil { t.Fatal(err) } - // Make sure that the index exists - query := `select count(*) from pg_indexes where schemaname=$1 and tablename=$2 and indexname=$3;` - var count int - if err := b.db.QueryRow(query, tc.Name, statesTableName, statesIndexName).Scan(&count); err != nil { - t.Fatal(err) - } - if count != 1 { - t.Fatalf("The index has not been created (%d)", count) + if tc.TestIndexIsPresent { + // Make sure that the index exists + query := `select count(*) from pg_indexes where schemaname=$1 and tablename=$2 and indexname=$3;` + var count int + if err := b.db.QueryRow(query, tc.Name, statesTableName, statesIndexName).Scan(&count); err != nil { + t.Fatal(err) + } + if count != 1 { + t.Fatalf("The index has not been created (%d)", count) + } } _, err = b.StateMgr(backend.DefaultStateName) @@ -202,6 +214,16 @@ func TestBackendConfigSkipOptions(t *testing.T) { if c.Name != backend.DefaultStateName { t.Fatal("RemoteClient name is not configured") } + + // Make sure that all workspace must have a unique name + _, err = db.Exec(fmt.Sprintf(`INSERT INTO %s.%s VALUES (100, 'unique_name_test', '')`, schemaName, statesTableName)) + if err != nil { + t.Fatal(err) + } + _, err = db.Exec(fmt.Sprintf(`INSERT INTO %s.%s VALUES (101, 'unique_name_test', '')`, schemaName, statesTableName)) + if err == nil { + t.Fatal("Creating two workspaces with the same name did not raise an error") + } }) } From b8e0d6f41864d595a49a6e714ad499d1c6f03a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Wed, 14 Jul 2021 11:59:14 +0200 Subject: [PATCH 2/3] Add UNIQUE constraint in the states table for the pg backend --- internal/backend/remote-state/pg/backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backend/remote-state/pg/backend.go b/internal/backend/remote-state/pg/backend.go index e7e63028d3..cdcfb3a6e4 100644 --- a/internal/backend/remote-state/pg/backend.go +++ b/internal/backend/remote-state/pg/backend.go @@ -111,7 +111,7 @@ func (b *Backend) configure(ctx context.Context) error { query = `CREATE TABLE IF NOT EXISTS %s.%s ( id bigint NOT NULL DEFAULT nextval('public.global_states_id_seq') PRIMARY KEY, - name text, + name text UNIQUE, data text )` if _, err := db.Exec(fmt.Sprintf(query, b.schemaName, statesTableName)); err != nil { From 4177bd98b995b0570d0aca410fe76ef765cc9903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Wed, 14 Jul 2021 12:01:54 +0200 Subject: [PATCH 3/3] Change wording for the skip_... options of the pg backend --- website/docs/language/settings/backends/pg.html.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/language/settings/backends/pg.html.md b/website/docs/language/settings/backends/pg.html.md index e83d6512ac..4c72d7ffe7 100644 --- a/website/docs/language/settings/backends/pg.html.md +++ b/website/docs/language/settings/backends/pg.html.md @@ -73,9 +73,9 @@ The following configuration options or environment variables are supported: * `conn_str` - (Required) Postgres connection string; a `postgres://` URL * `schema_name` - Name of the automatically-managed Postgres schema, default `terraform_remote_state`. - * `skip_schema_creation` - If set to `true`, the Postgres schema must already exist. Terraform won't try to create the schema. Useful when the Postgres user does not have "create schema" permission on the database. - * `skip_table_creation` - If set to `true`, the Postgres table must already exist. Terraform won't try to create the table. Useful when the Postgres user does not have "create table" permission on the database. - * `skip_index_creation` - If set to `true`, the Postgres index must already exist. Terraform won't try to create the index. Useful when the Postgres user does not have "create index" permission on the database. + * `skip_schema_creation` - If set to `true`, the Postgres schema must already exist. Terraform won't try to create the schema, this is useful when it has already been created by a database administrator. + * `skip_table_creation` - If set to `true`, the Postgres table must already exist. Terraform won't try to create the table, this is useful when it has already been created by a database administrator. + * `skip_index_creation` - If set to `true`, the Postgres index must already exist. Terraform won't try to create the index, this is useful when it has already been created by a database administrator. ## Technical Design