From eccd2ecbd08d0933ebb639061fb93957796ce7d9 Mon Sep 17 00:00:00 2001 From: bculberson Date: Fri, 18 Jan 2019 17:16:48 -0700 Subject: [PATCH 1/4] GH-7212 allow for optional parameters on cluster with global rds --- aws/resource_aws_rds_cluster.go | 27 ++++++++++++++++++------ website/docs/r/rds_cluster.html.markdown | 5 +++-- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_rds_cluster.go b/aws/resource_aws_rds_cluster.go index ba3f28c8836..3136b9a4889 100644 --- a/aws/resource_aws_rds_cluster.go +++ b/aws/resource_aws_rds_cluster.go @@ -712,12 +712,15 @@ func resourceAwsRDSClusterCreate(d *schema.ResourceData, meta interface{}) error } } else { - if _, ok := d.GetOk("master_password"); !ok { - return fmt.Errorf(`provider.aws: aws_rds_cluster: %s: "master_password": required field is not set`, d.Get("database_name").(string)) - } - if _, ok := d.GetOk("master_username"); !ok { - return fmt.Errorf(`provider.aws: aws_rds_cluster: %s: "master_username": required field is not set`, d.Get("database_name").(string)) + if _, ok := d.GetOk("global_cluster_identifier"); !ok { + if _, ok := d.GetOk("master_password"); !ok { + return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_password": required field is not set`, d.Get("name").(string)) + } + + if _, ok := d.GetOk("master_username"); !ok { + return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_username": required field is not set`, d.Get("name").(string)) + } } createOpts := &rds.CreateDBClusterInput{ @@ -725,12 +728,18 @@ func resourceAwsRDSClusterCreate(d *schema.ResourceData, meta interface{}) error DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)), Engine: aws.String(d.Get("engine").(string)), EngineMode: aws.String(d.Get("engine_mode").(string)), - MasterUserPassword: aws.String(d.Get("master_password").(string)), - MasterUsername: aws.String(d.Get("master_username").(string)), ScalingConfiguration: expandRdsScalingConfiguration(d.Get("scaling_configuration").([]interface{})), Tags: tags, } + if v, ok := d.GetOk("master_password"); ok && v.(string) != "" { + createOpts.MasterUserPassword = aws.String(v.(string)) + } + + if v, ok := d.GetOk("master_username"); ok && v.(string) != "" { + createOpts.MasterUsername = aws.String(v.(string)) + } + // Need to check value > 0 due to: // InvalidParameterValue: Backtrack is not enabled for the aurora-postgresql engine. if v, ok := d.GetOk("backtrack_window"); ok && v.(int) > 0 { @@ -785,6 +794,10 @@ func resourceAwsRDSClusterCreate(d *schema.ResourceData, meta interface{}) error createOpts.KmsKeyId = aws.String(attr.(string)) } + if attr, ok := d.GetOk("source_region"); ok { + createOpts.SourceRegion = aws.String(attr.(string)) + } + if attr, ok := d.GetOk("iam_database_authentication_enabled"); ok { createOpts.EnableIAMDatabaseAuthentication = aws.Bool(attr.(bool)) } diff --git a/website/docs/r/rds_cluster.html.markdown b/website/docs/r/rds_cluster.html.markdown index 748a56ca3cb..2d376a2ae59 100644 --- a/website/docs/r/rds_cluster.html.markdown +++ b/website/docs/r/rds_cluster.html.markdown @@ -85,9 +85,9 @@ The following arguments are supported: * `cluster_identifier_prefix` - (Optional, Forces new resource) Creates a unique cluster identifier beginning with the specified prefix. Conflicts with `cluster_identifer`. * `database_name` - (Optional) Name for an automatically created database on cluster creation. There are different naming restrictions per database engine: [RDS Naming Constraints][5] * `deletion_protection` - (Optional) If the DB instance should have deletion protection enabled. The database can't be deleted when this value is set to `true`. The default is `false`. -* `master_password` - (Required unless a `snapshot_identifier` is provided) Password for the master DB user. Note that this may +* `master_password` - (Required unless a `snapshot_identifier` or `global_cluster_identifier` is provided) Password for the master DB user. Note that this may show up in logs, and it will be stored in the state file. Please refer to the [RDS Naming Constraints][5] -* `master_username` - (Required unless a `snapshot_identifier` is provided) Username for the master DB user. Please refer to the [RDS Naming Constraints][5] +* `master_username` - (Required unless a `snapshot_identifier` or `global_cluster_identifier` is provided) Username for the master DB user. Please refer to the [RDS Naming Constraints][5] * `final_snapshot_identifier` - (Optional) The name of your final DB snapshot when this DB cluster is deleted. If omitted, no final snapshot will be made. @@ -103,6 +103,7 @@ Default: A 30-minute window selected at random from an 8-hour block of time per * `vpc_security_group_ids` - (Optional) List of VPC security groups to associate with the Cluster * `snapshot_identifier` - (Optional) Specifies whether or not to create this cluster from a snapshot. You can use either the name or ARN when specifying a DB cluster snapshot, or the ARN when specifying a DB snapshot. +* `global_cluster_identifier` - (Optional) The global cluster identifier specified on [`aws_rds_global_cluster`](/docs/providers/aws/r/rds_global_cluster.html). * `storage_encrypted` - (Optional) Specifies whether the DB cluster is encrypted. The default is `false` for `provisioned` `engine_mode` and `true` for `serverless` `engine_mode`. * `replication_source_identifier` - (Optional) ARN of a source DB cluster or DB instance if this DB cluster is to be created as a Read Replica. * `apply_immediately` - (Optional) Specifies whether any cluster modifications From 9b7a9b0d0861b46d84ad10d35de32950a6142822 Mon Sep 17 00:00:00 2001 From: bculberson Date: Mon, 11 Feb 2019 15:42:06 -0700 Subject: [PATCH 2/4] add test coverage --- aws/resource_aws_rds_cluster_test.go | 60 ++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_rds_cluster_test.go b/aws/resource_aws_rds_cluster_test.go index d85ffe9f677..783d0e094af 100644 --- a/aws/resource_aws_rds_cluster_test.go +++ b/aws/resource_aws_rds_cluster_test.go @@ -701,7 +701,7 @@ func TestAccAWSRDSCluster_GlobalClusterIdentifier_Add(t *testing.T) { CheckDestroy: testAccCheckAWSClusterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRDSClusterConfig_EngineMode(rName, "global"), + Config: testAccAWSRDSClusterConfig_GlobalEngineMode(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSClusterExists(resourceName, &dbCluster1), resource.TestCheckResourceAttr(resourceName, "global_cluster_identifier", ""), @@ -735,7 +735,7 @@ func TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove(t *testing.T) { ), }, { - Config: testAccAWSRDSClusterConfig_EngineMode(rName, "global"), + Config: testAccAWSRDSClusterConfig_GlobalEngineMode(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSClusterExists(resourceName, &dbCluster1), resource.TestCheckResourceAttr(resourceName, "global_cluster_identifier", ""), @@ -2250,6 +2250,22 @@ resource "aws_rds_cluster" "test" { `, rName, deletionProtection) } +func testAccAWSRDSClusterConfig_GlobalEngineMode(rName string) string { + return fmt.Sprintf(` +%s + +resource "aws_rds_cluster" "test" { + depends_on = ["aws_db_subnet_group.dbsubnet"] + cluster_identifier = %q + engine_mode = "global" + db_subnet_group_name = %q + master_password = "barbarbarbar" + master_username = "foo" + skip_final_snapshot = true +} +`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName) +} + func testAccAWSRDSClusterConfig_EngineMode(rName, engineMode string) string { return fmt.Sprintf(` resource "aws_rds_cluster" "test" { @@ -2262,25 +2278,59 @@ resource "aws_rds_cluster" "test" { `, rName, engineMode) } +func testAccAWSRDSClusterConfig_GlobalNetwork(rName string) string { + return fmt.Sprintf(` +data "aws_availability_zones" "azs" { } + +resource "aws_vpc" "vpc" { + cidr_block = "10.0.0.0/16" + tags = { + Name = "terraform-acctest-rds-cluster-global-cross-region" + } +} + +resource "aws_subnet" "subnets" { + count = 2 + vpc_id = "${aws_vpc.vpc.id}" + availability_zone = "${data.aws_availability_zones.azs.names[count.index]}" + cidr_block = "10.0.${count.index}.0/24" + tags = { + Name = "tf-acc-rds-cluster-global-cross-region-replica-${count.index}" + } +} + +resource "aws_db_subnet_group" "dbsubnet" { + name = %q + subnet_ids = ["${aws_subnet.subnets.*.id}"] +}`, rName) + +} + func testAccAWSRDSClusterConfig_GlobalClusterIdentifier(rName string) string { return fmt.Sprintf(` +%s + resource "aws_rds_global_cluster" "test" { global_cluster_identifier = %q } resource "aws_rds_cluster" "test" { + depends_on = ["aws_db_subnet_group.dbsubnet"] cluster_identifier = %q global_cluster_identifier = "${aws_rds_global_cluster.test.id}" engine_mode = "global" master_password = "barbarbarbar" master_username = "foo" skip_final_snapshot = true + db_subnet_group_name = "${aws_db_subnet_group.dbsubnet.name}" } -`, rName, rName) +`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName) } func testAccAWSRDSClusterConfig_GlobalClusterIdentifier_Update(rName, globalClusterIdentifierResourceName string) string { return fmt.Sprintf(` +%s + resource "aws_rds_global_cluster" "test" { count = 2 @@ -2288,14 +2338,16 @@ resource "aws_rds_global_cluster" "test" { } resource "aws_rds_cluster" "test" { + depends_on = ["aws_db_subnet_group.dbsubnet"] cluster_identifier = %q global_cluster_identifier = "${%s.id}" engine_mode = "global" master_password = "barbarbarbar" master_username = "foo" skip_final_snapshot = true + db_subnet_group_name = %q } -`, rName, rName, globalClusterIdentifierResourceName) +`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName, globalClusterIdentifierResourceName, rName) } func testAccAWSRDSClusterConfig_ScalingConfiguration(rName string, autoPause bool, maxCapacity, minCapacity, secondsUntilAutoPause int) string { From 8ebb1743d97e554d731cf8daf19415595806de21 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 22 Feb 2019 02:11:51 -0500 Subject: [PATCH 3/4] Revert 9b7a9b0d0861b46d84ad10d35de32950a6142822 See also: https://github.com/terraform-providers/terraform-provider-aws/pull/7213#discussion_r259234983 --- aws/resource_aws_rds_cluster_test.go | 60 ++-------------------------- 1 file changed, 4 insertions(+), 56 deletions(-) diff --git a/aws/resource_aws_rds_cluster_test.go b/aws/resource_aws_rds_cluster_test.go index 31540e66512..cd8b8af2c83 100644 --- a/aws/resource_aws_rds_cluster_test.go +++ b/aws/resource_aws_rds_cluster_test.go @@ -701,7 +701,7 @@ func TestAccAWSRDSCluster_GlobalClusterIdentifier_Add(t *testing.T) { CheckDestroy: testAccCheckAWSClusterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSRDSClusterConfig_GlobalEngineMode(rName), + Config: testAccAWSRDSClusterConfig_EngineMode(rName, "global"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSClusterExists(resourceName, &dbCluster1), resource.TestCheckResourceAttr(resourceName, "global_cluster_identifier", ""), @@ -735,7 +735,7 @@ func TestAccAWSRDSCluster_GlobalClusterIdentifier_Remove(t *testing.T) { ), }, { - Config: testAccAWSRDSClusterConfig_GlobalEngineMode(rName), + Config: testAccAWSRDSClusterConfig_EngineMode(rName, "global"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSClusterExists(resourceName, &dbCluster1), resource.TestCheckResourceAttr(resourceName, "global_cluster_identifier", ""), @@ -2245,22 +2245,6 @@ resource "aws_rds_cluster" "test" { `, rName, deletionProtection) } -func testAccAWSRDSClusterConfig_GlobalEngineMode(rName string) string { - return fmt.Sprintf(` -%s - -resource "aws_rds_cluster" "test" { - depends_on = ["aws_db_subnet_group.dbsubnet"] - cluster_identifier = %q - engine_mode = "global" - db_subnet_group_name = %q - master_password = "barbarbarbar" - master_username = "foo" - skip_final_snapshot = true -} -`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName) -} - func testAccAWSRDSClusterConfig_EngineMode(rName, engineMode string) string { return fmt.Sprintf(` resource "aws_rds_cluster" "test" { @@ -2273,59 +2257,25 @@ resource "aws_rds_cluster" "test" { `, rName, engineMode) } -func testAccAWSRDSClusterConfig_GlobalNetwork(rName string) string { - return fmt.Sprintf(` -data "aws_availability_zones" "azs" { } - -resource "aws_vpc" "vpc" { - cidr_block = "10.0.0.0/16" - tags = { - Name = "terraform-acctest-rds-cluster-global-cross-region" - } -} - -resource "aws_subnet" "subnets" { - count = 2 - vpc_id = "${aws_vpc.vpc.id}" - availability_zone = "${data.aws_availability_zones.azs.names[count.index]}" - cidr_block = "10.0.${count.index}.0/24" - tags = { - Name = "tf-acc-rds-cluster-global-cross-region-replica-${count.index}" - } -} - -resource "aws_db_subnet_group" "dbsubnet" { - name = %q - subnet_ids = ["${aws_subnet.subnets.*.id}"] -}`, rName) - -} - func testAccAWSRDSClusterConfig_GlobalClusterIdentifier(rName string) string { return fmt.Sprintf(` -%s - resource "aws_rds_global_cluster" "test" { global_cluster_identifier = %q } resource "aws_rds_cluster" "test" { - depends_on = ["aws_db_subnet_group.dbsubnet"] cluster_identifier = %q global_cluster_identifier = "${aws_rds_global_cluster.test.id}" engine_mode = "global" master_password = "barbarbarbar" master_username = "foo" skip_final_snapshot = true - db_subnet_group_name = "${aws_db_subnet_group.dbsubnet.name}" } -`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName) +`, rName, rName) } func testAccAWSRDSClusterConfig_GlobalClusterIdentifier_Update(rName, globalClusterIdentifierResourceName string) string { return fmt.Sprintf(` -%s - resource "aws_rds_global_cluster" "test" { count = 2 @@ -2333,16 +2283,14 @@ resource "aws_rds_global_cluster" "test" { } resource "aws_rds_cluster" "test" { - depends_on = ["aws_db_subnet_group.dbsubnet"] cluster_identifier = %q global_cluster_identifier = "${%s.id}" engine_mode = "global" master_password = "barbarbarbar" master_username = "foo" skip_final_snapshot = true - db_subnet_group_name = %q } -`, testAccAWSRDSClusterConfig_GlobalNetwork(rName), rName, rName, globalClusterIdentifierResourceName, rName) +`, rName, rName, globalClusterIdentifierResourceName) } func testAccAWSRDSClusterConfig_ScalingConfiguration(rName string, autoPause bool, maxCapacity, minCapacity, secondsUntilAutoPause int) string { From cc6cfc0713564cb4b63f37bc09b67b29c5718ec0 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 22 Feb 2019 02:34:30 -0500 Subject: [PATCH 4/4] resource/aws_rds_cluster: Prevent panic with missing master_password or master_username Reference: https://github.com/terraform-providers/terraform-provider-aws/pull/7213#discussion_r259233400 Previous output from acceptance testing: ``` === CONT TestAccAWSRDSCluster_missingUserNameCausesError ------- Stderr: ------- panic: interface conversion: interface {} is nil, not string goroutine 137 [running]: github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsRDSClusterCreate(0xc000349260, 0x3c28320, 0xc0001adc00, 0xc000349260, 0x0) /opt/teamcity-agent/work/2e10e023da0c7520/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_rds_cluster.go:718 +0x6d9d ``` Output from acceptance testing: ``` --- PASS: TestAccAWSRDSCluster_missingUserNameCausesError (3.24s) ``` --- aws/resource_aws_rds_cluster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_rds_cluster.go b/aws/resource_aws_rds_cluster.go index 3136b9a4889..6b78667fdda 100644 --- a/aws/resource_aws_rds_cluster.go +++ b/aws/resource_aws_rds_cluster.go @@ -715,11 +715,11 @@ func resourceAwsRDSClusterCreate(d *schema.ResourceData, meta interface{}) error if _, ok := d.GetOk("global_cluster_identifier"); !ok { if _, ok := d.GetOk("master_password"); !ok { - return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_password": required field is not set`, d.Get("name").(string)) + return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_password": required field is not set`, d.Get("database_name").(string)) } if _, ok := d.GetOk("master_username"); !ok { - return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_username": required field is not set`, d.Get("name").(string)) + return fmt.Errorf(`provider.aws: aws_db_instance: %s: "master_username": required field is not set`, d.Get("database_name").(string)) } }