From 62fcb11062e3b4a295b34feb9d4754fa00c3d3a9 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Wed, 26 Jun 2019 12:57:15 +0200 Subject: [PATCH] fix bug caught by tests and yoris feedback --- examples/postgres-public-ip/main.tf | 6 ++---- examples/postgres-replicas/main.tf | 6 ++---- modules/cloud-sql/main.tf | 10 +++++++--- modules/cloud-sql/variables.tf | 6 +++--- test/example_postgres_public_ip_test.go | 6 ++++++ test/example_postgres_replicas_test.go | 23 +++++++++++++++++++++++ test/test_util.go | 1 + 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/examples/postgres-public-ip/main.tf b/examples/postgres-public-ip/main.tf index b37a6f0..08bffc1 100644 --- a/examples/postgres-public-ip/main.tf +++ b/examples/postgres-public-ip/main.tf @@ -50,13 +50,11 @@ module "postgres" { machine_type = var.machine_type # These together will construct the master_user privileges, i.e. - # 'master_user_name'@'master_user_host' IDENTIFIED BY 'master_user_password'. + # 'master_user_name' IDENTIFIED BY 'master_user_password'. # These should typically be set as the environment variable TF_VAR_master_user_password, etc. # so you don't check these into source control." master_user_password = var.master_user_password - - master_user_name = var.master_user_name - master_user_host = "%" + master_user_name = var.master_user_name # To make it easier to test this example, we are giving the servers public IP addresses and allowing inbound # connections from anywhere. In real-world usage, your servers should live in private subnets, only have private IP diff --git a/examples/postgres-replicas/main.tf b/examples/postgres-replicas/main.tf index bb8642b..bb7c8ac 100644 --- a/examples/postgres-replicas/main.tf +++ b/examples/postgres-replicas/main.tf @@ -73,13 +73,11 @@ module "postgres" { read_replica_zones = var.read_replica_zones # These together will construct the master_user privileges, i.e. - # 'master_user_name'@'master_user_host' IDENTIFIED BY 'master_user_password'. + # 'master_user_name' IDENTIFIED BY 'master_user_password'. # These should typically be set as the environment variable TF_VAR_master_user_password, etc. # so you don't check these into source control." master_user_password = var.master_user_password - - master_user_name = var.master_user_name - master_user_host = "%" + master_user_name = var.master_user_name custom_labels = { test-id = "postgres-replicas-example" diff --git a/modules/cloud-sql/main.tf b/modules/cloud-sql/main.tf index 4da5669..daf117f 100644 --- a/modules/cloud-sql/main.tf +++ b/modules/cloud-sql/main.tf @@ -48,7 +48,7 @@ resource "google_sql_database_instance" "master" { dynamic "authorized_networks" { for_each = var.authorized_networks content { - name = authorized_networks.value.name + name = lookup(authorized_networks.value, "name", null) value = authorized_networks.value.value } } @@ -117,10 +117,13 @@ resource "google_sql_database" "default" { resource "google_sql_user" "default" { depends_on = [google_sql_database.default] - name = var.master_user_name project = var.project + name = var.master_user_name instance = google_sql_database_instance.master.name - host = var.master_user_host + # Postgres users don't have hosts, so the API will ignore this value which causes Terraform to attempt + # to recreate the user each time. + # See https://github.com/terraform-providers/terraform-provider-google/issues/1526 for more information. + host = local.is_postgres ? null : var.master_user_host password = var.master_user_password } @@ -295,6 +298,7 @@ resource "google_sql_database_instance" "read_replica" { # ------------------------------------------------------------------------------ # CREATE A TEMPLATE FILE TO SIGNAL ALL RESOURCES HAVE BEEN CREATED # ------------------------------------------------------------------------------ + data "template_file" "complete" { depends_on = [ google_sql_database_instance.master, diff --git a/modules/cloud-sql/variables.tf b/modules/cloud-sql/variables.tf index d17f54b..8fb5719 100644 --- a/modules/cloud-sql/variables.tf +++ b/modules/cloud-sql/variables.tf @@ -56,7 +56,7 @@ variable "activation_policy" { variable "authorized_networks" { description = "A list of authorized CIDR-formatted IP address ranges that can connect to this DB. Only applies to public IP instances." - type = list(any) + type = list(map(string)) default = [] # Example: @@ -173,7 +173,7 @@ variable "master_zone" { } variable "master_user_host" { - description = "The host part for the default user, i.e. 'master_user_name'@'master_user_host' IDENTIFIED BY 'master_user_password' " + description = "The host part for the default user, i.e. 'master_user_name'@'master_user_host' IDENTIFIED BY 'master_user_password'. Don't set this field for Postgres instances." type = string default = "%" } @@ -249,6 +249,6 @@ variable "resource_timeout" { variable "dependencies" { description = "Create a dependency between the resources in this module to the interpolated values in this list (and thus the source resources). In other words, the resources in this module will now depend on the resources backing the values in this list such that those resources need to be created before the resources in this module, and the resources in this module need to be destroyed before the resources in the list." - type = list(any) + type = list(string) default = [] } diff --git a/test/example_postgres_public_ip_test.go b/test/example_postgres_public_ip_test.go index 73e0d66..a825c0c 100644 --- a/test/example_postgres_public_ip_test.go +++ b/test/example_postgres_public_ip_test.go @@ -246,5 +246,11 @@ func TestPostgresPublicIP(t *testing.T) { if err = db.Ping(); err != nil { t.Fatalf("Failed to ping DB with forced SSL: %v", err) } + + // Drop the test table if it exists + logger.Logf(t, "Drop table: %s", POSTGRES_DROP_TEST_TABLE) + if _, err = db.Exec(POSTGRES_DROP_TEST_TABLE); err != nil { + t.Fatalf("Failed to drop table: %v", err) + } }) } diff --git a/test/example_postgres_replicas_test.go b/test/example_postgres_replicas_test.go index 03cf7f1..14792b3 100644 --- a/test/example_postgres_replicas_test.go +++ b/test/example_postgres_replicas_test.go @@ -45,6 +45,29 @@ func TestPostgresReplicas(t *testing.T) { test_structure.SaveString(t, exampleDir, KEY_PROJECT, projectId) }) + // AT THE END OF THE TESTS, CLEAN UP ANY POSTGRES OBJECTS THAT WERE CREATED + defer test_structure.RunTestStage(t, "cleanup_postgres_objects", func() { + terraformOptions := test_structure.LoadTerraformOptions(t, exampleDir) + + publicIp := terraform.Output(t, terraformOptions, OUTPUT_MASTER_PUBLIC_IP) + + connectionString := fmt.Sprintf("postgres://%s:%s@%s/%s?sslmode=disable", DB_USER, DB_PASS, publicIp, DB_NAME) + + // Does not actually open up the connection - just returns a DB ref + logger.Logf(t, "Connecting to: %s", publicIp) + db, err := sql.Open("postgres", connectionString) + require.NoError(t, err, "Failed to open DB connection") + + // Make sure we clean up properly + defer db.Close() + + // Drop table if it exists + logger.Logf(t, "Drop table: %s", POSTGRES_DROP_TEST_TABLE) + if _, err = db.Exec(POSTGRES_DROP_TEST_TABLE); err != nil { + t.Fatalf("Failed to drop table: %v", err) + } + }) + // AT THE END OF THE TESTS, RUN `terraform destroy` // TO CLEAN UP ANY RESOURCES THAT WERE CREATED defer test_structure.RunTestStage(t, "teardown", func() { diff --git a/test/test_util.go b/test/test_util.go index 051cfaf..6291d1d 100644 --- a/test/test_util.go +++ b/test/test_util.go @@ -44,6 +44,7 @@ const SQL_QUERY_ROW_COUNT = "SELECT count(*) FROM test" const POSTGRES_CREATE_TEST_TABLE_WITH_SERIAL = "CREATE TABLE IF NOT EXISTS test (id SERIAL, name varchar(10) NOT NULL, PRIMARY KEY (ID))" const POSTGRES_INSERT_TEST_ROW = "INSERT INTO test(name) VALUES('Grunty') RETURNING id" +const POSTGRES_DROP_TEST_TABLE = "DROP TABLE IF EXISTS test" func getRandomRegion(t *testing.T, projectID string) string { approvedRegions := []string{"europe-north1", "europe-west1", "europe-west2", "europe-west3", "us-central1", "us-east1", "us-west1"}