From e7806b135f92e629f7158ba2e46dc1f7bb76a5ff Mon Sep 17 00:00:00 2001 From: Mike Walker Date: Tue, 8 Aug 2017 11:08:28 -0400 Subject: [PATCH 1/7] Adds domain and domain-iam-role-name parameters to resource aws_db_instance. --- aws/resource_aws_db_instance.go | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index 3009d32e083..3fa120370d4 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -350,6 +350,18 @@ func resourceAwsDbInstance() *schema.Resource { Computed: true, }, + "domain": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + + "domain_iam_role_name": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + "tags": tagsSchema(), }, } @@ -660,6 +672,14 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error opts.EnableIAMDatabaseAuthentication = aws.Bool(attr.(bool)) } + if attr, ok := d.GetOk("domain"); ok { + opts.Domain = aws.String(attr.(string)) + } + + if attr, ok := d.GetOk("domain_iam_role_name"); ok { + opts.DomainIAMRoleName = aws.String(attr.(string)) + } + log.Printf("[DEBUG] DB Instance create configuration: %#v", opts) var err error err = resource.Retry(5*time.Minute, func() *resource.RetryError { @@ -776,6 +796,11 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error { d.Set("monitoring_role_arn", v.MonitoringRoleArn) } + if v.DomainMemberships != nil { + d.Set("domain", v.DomainMemberships[0].Domain) + d.Set("domain-iam-role-name", v.DomainMemberships[0].IAMRoleName) + } + // list tags for resource // set tags conn := meta.(*AWSClient).rdsconn @@ -1028,6 +1053,18 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error requestUpdate = true } + if d.HasChange("domain") && !d.IsNewResource() { + d.SetPartial("domain") + req.Domain = aws.String(d.Get("domain").(string)) + requestUpdate = true + } + + if d.HasChange("domain_iam_role_name") && !d.IsNewResource() { + d.SetPartial("domain_iam_role_name") + req.DomainIAMRoleName = aws.String(d.Get("domain_iam_role_name").(string)) + requestUpdate = true + } + log.Printf("[DEBUG] Send DB Instance Modification request: %t", requestUpdate) if requestUpdate { log.Printf("[DEBUG] DB Instance Modification request: %s", req) From 9a2b871ce9adaff3a90018c25a7a39461941da27 Mon Sep 17 00:00:00 2001 From: Matthew Burtless Date: Sat, 11 Aug 2018 14:24:18 -0400 Subject: [PATCH 2/7] Removes Computed attributes from domain and domain_iam_role_name, adds further checking to DomainMemberships --- aws/resource_aws_db_instance.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index 4ce207ee0bc..8e626ebefa1 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -407,13 +407,11 @@ func resourceAwsDbInstance() *schema.Resource { "domain": { Type: schema.TypeString, Optional: true, - Computed: true, }, "domain_iam_role_name": { Type: schema.TypeString, Optional: true, - Computed: true, }, "tags": tagsSchema(), @@ -1032,7 +1030,7 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err) } - if v.DomainMemberships != nil { + if len(v.DomainMemberships) > 0 && v.DomainMemberships[0] != nil { d.Set("domain", v.DomainMemberships[0].Domain) d.Set("domain_iam_role_name", v.DomainMemberships[0].IAMRoleName) } From 6d7c4f5dce19827c23bfe92c5bef043e427910ed Mon Sep 17 00:00:00 2001 From: Matthew Burtless Date: Sat, 11 Aug 2018 14:42:41 -0400 Subject: [PATCH 3/7] Updated name of config for domain acceptance test to match standard --- aws/resource_aws_db_instance_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index cf309d9c7e8..05c35e01ac6 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -469,7 +469,7 @@ func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { CheckDestroy: testAccCheckAWSDBInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSDBMSSQL_domain(rInt), + Config: testAccAWSDBMSSQLDomain(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), resource.TestCheckResourceAttrSet( @@ -1660,7 +1660,7 @@ resource "aws_security_group_rule" "rds-mssql-1" { `, rInt, rInt, rInt) } -func testAccAWSDBMSSQL_domain(rInt int) string { +func testAccAWSDBMSSQLDomain(rInt int) string { return fmt.Sprintf(` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16" From 24ab317ef6425688e4db9ab2015cf5c6471bcbe0 Mon Sep 17 00:00:00 2001 From: Matthew Burtless Date: Sun, 19 Aug 2018 22:57:00 -0400 Subject: [PATCH 4/7] Minor bug fixes and adds acceptance test step to ensure updating domain of db instance works as expected. This commit integrates suggesting made by @bflad in review. It also merges any updates to domain or domain_iam_role_name into a single update request, as both arguments must be present in order to update either property of a DB instance. Finally, it adds an accpetance test step to ensure that after a db instance is joined to a domain, it can be succesfully moved to another by updating the domain argument.X --- aws/resource_aws_db_instance.go | 8 +- aws/resource_aws_db_instance_test.go | 192 +++++++++++++++++++++++++-- 2 files changed, 186 insertions(+), 14 deletions(-) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index 8e626ebefa1..c4caf66a5bb 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -1285,14 +1285,10 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error requestUpdate = true } - if d.HasChange("domain") && !d.IsNewResource() { + if (d.HasChange("domain") || d.HasChange("domain_iam_role_name")) && !d.IsNewResource() { d.SetPartial("domain") - req.Domain = aws.String(d.Get("domain").(string)) - requestUpdate = true - } - - if d.HasChange("domain_iam_role_name") && !d.IsNewResource() { d.SetPartial("domain_iam_role_name") + req.Domain = aws.String(d.Get("domain").(string)) req.DomainIAMRoleName = aws.String(d.Get("domain_iam_role_name").(string)) requestUpdate = true } diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index 05c35e01ac6..d7a9d4fa0ba 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -472,14 +472,22 @@ func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { Config: testAccAWSDBMSSQLDomain(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), + testAccCheckAWSDBInstanceDomainAttributes("foo.somedomain.com", &v), + resource.TestCheckResourceAttrSet( + "aws_db_instance.mssql", "domain"), + resource.TestCheckResourceAttrSet( + "aws_db_instance.mssql", "domain_iam_role_name"), + ), + }, + { + Config: testAccAWSDBMSSQLUpdateDomain(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), + testAccCheckAWSDBInstanceDomainAttributes("bar.somedomain.com", &v), resource.TestCheckResourceAttrSet( "aws_db_instance.mssql", "domain"), resource.TestCheckResourceAttrSet( "aws_db_instance.mssql", "domain_iam_role_name"), - resource.TestCheckResourceAttr( - "aws_db_instance.mssql", "allocated_storage", "20"), - resource.TestCheckResourceAttr( - "aws_db_instance.mssql", "engine", "sqlserver-ex"), ), }, }, @@ -700,6 +708,39 @@ func testAccCheckAWSDBInstanceAttributes_MSSQL(v *rds.DBInstance, tz string) res } } +func testAccCheckAWSDBInstanceDomainAttributes(domain string, v *rds.DBInstance) resource.TestCheckFunc { + return func(s *terraform.State) error { + // Grab domainmemberships list and compare domain to FQDN + for _, dm := range v.DomainMemberships { + if *dm.FQDN != domain { + continue + } + + return nil + } + + return fmt.Errorf("Domain %s not found in domain memberships", domain) + /*dsconn := testAccProvider.Meta().(*AWSClient).dsconn + out, err := dsconn.DescribeDirectories(&directoryservice.DescribeDirectoriesInput{ + DirectoryIds: []*string{aws.String(*v.Domain)}, + }) + + if err != nil { + return err + } + + if len(out.DirectoryDescriptions) < 1 { + return fmt.Errorf("DS directory %s not found", *v.Domain) + } + + if *out.DirectoryDescriptions[0].Name != domain { + return fmt.Errorf("DS directory name mismatch, expected: '%s', got: '%s'", domain, *out.DirectoryDescriptions[0].Name) + }*/ + + return nil + } +} + func testAccCheckAWSDBInstanceReplicaAttributes(source, replica *rds.DBInstance) resource.TestCheckFunc { return func(s *terraform.State) error { @@ -1708,7 +1749,130 @@ resource "aws_db_instance" "mssql" { backup_retention_period = 0 skip_final_snapshot = true - domain = "${aws_directory_service_directory.directory.id}" + domain = "${aws_directory_service_directory.foo.id}" + domain_iam_role_name = "${aws_iam_role.role.name}" + + vpc_security_group_ids = ["${aws_security_group.rds-mssql.id}"] +} + +resource "aws_security_group" "rds-mssql" { + name = "tf-rds-mssql-test-%d" + + description = "TF Testing" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_security_group_rule" "rds-mssql-1" { + type = "egress" + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + + security_group_id = "${aws_security_group.rds-mssql.id}" +} + +resource "aws_directory_service_directory" "foo" { + name = "foo.somedomain.com" + password = "SuperSecretPassw0rd" + type = "MicrosoftAD" + edition = "Standard" + + vpc_settings { + vpc_id = "${aws_vpc.foo.id}" + subnet_ids = ["${aws_subnet.main.id}", "${aws_subnet.other.id}"] + } +} + +resource "aws_directory_service_directory" "bar" { + name = "bar.somedomain.com" + password = "SuperSecretPassw0rd" + type = "MicrosoftAD" + edition = "Standard" + + vpc_settings { + vpc_id = "${aws_vpc.foo.id}" + subnet_ids = ["${aws_subnet.main.id}", "${aws_subnet.other.id}"] + } +} + +resource "aws_iam_role" "role" { + name = "tf-acc-db-instance-mssql-domain-role" + + assume_role_policy = < Date: Sun, 19 Aug 2018 23:25:39 -0400 Subject: [PATCH 5/7] Removed commented test code --- aws/resource_aws_db_instance_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index b8b2bf802db..a141e8d9d5a 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -822,7 +822,6 @@ func testAccCheckAWSDBInstanceAttributes_MSSQL(v *rds.DBInstance, tz string) res func testAccCheckAWSDBInstanceDomainAttributes(domain string, v *rds.DBInstance) resource.TestCheckFunc { return func(s *terraform.State) error { - // Grab domainmemberships list and compare domain to FQDN for _, dm := range v.DomainMemberships { if *dm.FQDN != domain { continue @@ -832,24 +831,6 @@ func testAccCheckAWSDBInstanceDomainAttributes(domain string, v *rds.DBInstance) } return fmt.Errorf("Domain %s not found in domain memberships", domain) - /*dsconn := testAccProvider.Meta().(*AWSClient).dsconn - out, err := dsconn.DescribeDirectories(&directoryservice.DescribeDirectoriesInput{ - DirectoryIds: []*string{aws.String(*v.Domain)}, - }) - - if err != nil { - return err - } - - if len(out.DirectoryDescriptions) < 1 { - return fmt.Errorf("DS directory %s not found", *v.Domain) - } - - if *out.DirectoryDescriptions[0].Name != domain { - return fmt.Errorf("DS directory name mismatch, expected: '%s', got: '%s'", domain, *out.DirectoryDescriptions[0].Name) - }*/ - - return nil } } From 5e2da4a16bc1bca54bde887bf786122a24eedd2a Mon Sep 17 00:00:00 2001 From: Matthew Burtless Date: Tue, 21 Aug 2018 21:27:03 -0400 Subject: [PATCH 6/7] Adds support for domain and domain_iam_role_name to RestoreDBInstanceFromDBSnapshotInput --- aws/resource_aws_db_instance.go | 8 ++ aws/resource_aws_db_instance_test.go | 171 ++++++++++++++++++++++++++- 2 files changed, 174 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index e32dbc5c276..2ac0b7e0b7b 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -704,6 +704,14 @@ func resourceAwsDbInstanceCreate(d *schema.ResourceData, meta interface{}) error opts.DBSubnetGroupName = aws.String(attr.(string)) } + if attr, ok := d.GetOk("domain"); ok { + opts.Domain = aws.String(attr.(string)) + } + + if attr, ok := d.GetOk("domain_iam_role_name"); ok { + opts.DomainIAMRoleName = aws.String(attr.(string)) + } + if attr, ok := d.GetOk("enabled_cloudwatch_logs_exports"); ok && len(attr.([]interface{})) > 0 { opts.EnableCloudwatchLogsExports = expandStringList(attr.([]interface{})) } diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index e41cc9c9b8c..14b4a578321 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -572,7 +572,7 @@ func TestAccAWSDBInstance_MSSQL_TZ(t *testing.T) { } func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { - var v rds.DBInstance + var vBefore, vAfter rds.DBInstance rInt := acctest.RandInt() resource.Test(t, resource.TestCase{ @@ -583,8 +583,8 @@ func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { { Config: testAccAWSDBMSSQLDomain(rInt), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), - testAccCheckAWSDBInstanceDomainAttributes("foo.somedomain.com", &v), + testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &vBefore), + testAccCheckAWSDBInstanceDomainAttributes("foo.somedomain.com", &vBefore), resource.TestCheckResourceAttrSet( "aws_db_instance.mssql", "domain"), resource.TestCheckResourceAttrSet( @@ -594,8 +594,8 @@ func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { { Config: testAccAWSDBMSSQLUpdateDomain(rInt), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), - testAccCheckAWSDBInstanceDomainAttributes("bar.somedomain.com", &v), + testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &vAfter), + testAccCheckAWSDBInstanceDomainAttributes("bar.somedomain.com", &vAfter), resource.TestCheckResourceAttrSet( "aws_db_instance.mssql", "domain"), resource.TestCheckResourceAttrSet( @@ -606,6 +606,31 @@ func TestAccAWSDBInstance_MSSQL_Domain(t *testing.T) { }) } +func TestAccAWSDBInstance_MSSQL_DomainSnapshotRestore(t *testing.T) { + var v, vRestoredInstance rds.DBInstance + rInt := acctest.RandInt() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDBInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSDBMSSQLDomainSnapshotRestore(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSDBInstanceExists("aws_db_instance.mssql_restore", &vRestoredInstance), + testAccCheckAWSDBInstanceExists("aws_db_instance.mssql", &v), + testAccCheckAWSDBInstanceDomainAttributes("foo.somedomain.com", &vRestoredInstance), + resource.TestCheckResourceAttrSet( + "aws_db_instance.mssql_restore", "domain"), + resource.TestCheckResourceAttrSet( + "aws_db_instance.mssql_restore", "domain_iam_role_name"), + ), + }, + }, + }) +} + func TestAccAWSDBInstance_MinorVersion(t *testing.T) { var v rds.DBInstance @@ -2067,6 +2092,142 @@ resource "aws_iam_role_policy_attachment" "attatch-policy" { `, rInt, rInt, rInt) } +func testAccAWSDBMSSQLDomainSnapshotRestore(rInt int) string { + return fmt.Sprintf(` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + enable_dns_hostnames = true + tags { + Name = "terraform-testacc-db-instance-mssql-domain" + } +} + +resource "aws_db_subnet_group" "rds_one" { + name = "tf_acc_test_%d" + description = "db subnets for rds_one" + + subnet_ids = ["${aws_subnet.main.id}", "${aws_subnet.other.id}"] +} + +resource "aws_subnet" "main" { + vpc_id = "${aws_vpc.foo.id}" + availability_zone = "us-west-2a" + cidr_block = "10.1.1.0/24" + tags { + Name = "tf-acc-db-instance-mssql-domain-main" + } +} + +resource "aws_subnet" "other" { + vpc_id = "${aws_vpc.foo.id}" + availability_zone = "us-west-2b" + cidr_block = "10.1.2.0/24" + tags { + Name = "tf-acc-db-instance-mssql-domain-other" + } +} + +resource "aws_db_instance" "mssql" { + identifier = "tf-test-mssql-%d" + + db_subnet_group_name = "${aws_db_subnet_group.rds_one.name}" + + instance_class = "db.t2.micro" + allocated_storage = 20 + username = "somecrazyusername" + password = "somecrazypassword" + engine = "sqlserver-ex" + backup_retention_period = 0 + skip_final_snapshot = true + + domain = "${aws_directory_service_directory.foo.id}" + domain_iam_role_name = "${aws_iam_role.role.name}" + + vpc_security_group_ids = ["${aws_security_group.rds-mssql.id}"] +} + +resource "aws_db_snapshot" "mssql-snap" { + db_instance_identifier = "${aws_db_instance.mssql.id}" + db_snapshot_identifier = "mssql-snap" +} + +resource "aws_db_instance" "mssql_restore" { + identifier = "tf-test-mssql-%d-restore" + + db_subnet_group_name = "${aws_db_subnet_group.rds_one.name}" + + instance_class = "db.t2.micro" + allocated_storage = 20 + username = "somecrazyusername" + password = "somecrazypassword" + engine = "sqlserver-ex" + backup_retention_period = 0 + skip_final_snapshot = true + snapshot_identifier = "${aws_db_snapshot.mssql-snap.id}" + + domain = "${aws_directory_service_directory.foo.id}" + domain_iam_role_name = "${aws_iam_role.role.name}" + + apply_immediately = true + vpc_security_group_ids = ["${aws_security_group.rds-mssql.id}"] +} + +resource "aws_security_group" "rds-mssql" { + name = "tf-rds-mssql-test-%d" + + description = "TF Testing" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_security_group_rule" "rds-mssql-1" { + type = "egress" + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + + security_group_id = "${aws_security_group.rds-mssql.id}" +} + +resource "aws_directory_service_directory" "foo" { + name = "foo.somedomain.com" + password = "SuperSecretPassw0rd" + type = "MicrosoftAD" + edition = "Standard" + + vpc_settings { + vpc_id = "${aws_vpc.foo.id}" + subnet_ids = ["${aws_subnet.main.id}", "${aws_subnet.other.id}"] + } +} + +resource "aws_iam_role" "role" { + name = "tf-acc-db-instance-mssql-domain-role" + + assume_role_policy = < Date: Thu, 6 Sep 2018 09:32:47 -0400 Subject: [PATCH 7/7] resource/aws_db_instance: Address #5378 PR feedback * Ensure d.Set() is always called for domain and domain_iam_role_name * Remove extraneous d.IsNewResource() check in Update function * Randomize IAM role naming for new domain acceptance tests --- aws/resource_aws_db_instance.go | 4 +++- aws/resource_aws_db_instance_test.go | 33 ++++++++++------------------ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/aws/resource_aws_db_instance.go b/aws/resource_aws_db_instance.go index e338d571b29..fc3a25947fc 100644 --- a/aws/resource_aws_db_instance.go +++ b/aws/resource_aws_db_instance.go @@ -1179,6 +1179,8 @@ func resourceAwsDbInstanceRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error setting enabled_cloudwatch_logs_exports: %s", err) } + d.Set("domain", "") + d.Set("domain_iam_role_name", "") if len(v.DomainMemberships) > 0 && v.DomainMemberships[0] != nil { d.Set("domain", v.DomainMemberships[0].Domain) d.Set("domain_iam_role_name", v.DomainMemberships[0].IAMRoleName) @@ -1443,7 +1445,7 @@ func resourceAwsDbInstanceUpdate(d *schema.ResourceData, meta interface{}) error requestUpdate = true } - if (d.HasChange("domain") || d.HasChange("domain_iam_role_name")) && !d.IsNewResource() { + if d.HasChange("domain") || d.HasChange("domain_iam_role_name") { d.SetPartial("domain") d.SetPartial("domain_iam_role_name") req.Domain = aws.String(d.Get("domain").(string)) diff --git a/aws/resource_aws_db_instance_test.go b/aws/resource_aws_db_instance_test.go index 3fc3ca9a7ae..90eaecda9a1 100644 --- a/aws/resource_aws_db_instance_test.go +++ b/aws/resource_aws_db_instance_test.go @@ -2586,7 +2586,7 @@ resource "aws_directory_service_directory" "bar" { } resource "aws_iam_role" "role" { - name = "tf-acc-db-instance-mssql-domain-role" + name = "tf-acc-db-instance-mssql-domain-role-%d" assume_role_policy = <