From 5e85d3d8243bd20d66ff6ad5680ee9755ed0a792 Mon Sep 17 00:00:00 2001 From: Andrew Babichev Date: Fri, 4 Dec 2020 14:30:36 +0200 Subject: [PATCH 1/4] aws_workspaces_directory: Fix empty custom_security_group_id & default_ou --- aws/resource_aws_workspaces_directory.go | 14 +- aws/resource_aws_workspaces_directory_test.go | 346 ++++++++++++++---- 2 files changed, 279 insertions(+), 81 deletions(-) diff --git a/aws/resource_aws_workspaces_directory.go b/aws/resource_aws_workspaces_directory.go index d00cbfd05f8..a2f90e7e696 100644 --- a/aws/resource_aws_workspaces_directory.go +++ b/aws/resource_aws_workspaces_directory.go @@ -419,13 +419,21 @@ func expandWorkspaceCreationProperties(properties []interface{}) *workspaces.Wor p := properties[0].(map[string]interface{}) - return &workspaces.WorkspaceCreationProperties{ - CustomSecurityGroupId: aws.String(p["custom_security_group_id"].(string)), - DefaultOu: aws.String(p["default_ou"].(string)), + result := &workspaces.WorkspaceCreationProperties{ EnableInternetAccess: aws.Bool(p["enable_internet_access"].(bool)), EnableMaintenanceMode: aws.Bool(p["enable_maintenance_mode"].(bool)), UserEnabledAsLocalAdministrator: aws.Bool(p["user_enabled_as_local_administrator"].(bool)), } + + if p["custom_security_group_id"].(string) != "" { + result.CustomSecurityGroupId = aws.String(p["custom_security_group_id"].(string)) + } + + if p["default_ou"].(string) != "" { + result.DefaultOu = aws.String(p["default_ou"].(string)) + } + + return result } func flattenSelfServicePermissions(permissions *workspaces.SelfservicePermissions) []interface{} { diff --git a/aws/resource_aws_workspaces_directory_test.go b/aws/resource_aws_workspaces_directory_test.go index 0b4f927003a..16f3e1a4863 100644 --- a/aws/resource_aws_workspaces_directory_test.go +++ b/aws/resource_aws_workspaces_directory_test.go @@ -283,6 +283,55 @@ func TestAccAwsWorkspacesDirectory_workspaceCreationProperties(t *testing.T) { }) } +func TestAccAwsWorkspacesDirectory_workspaceCreationProperties_customSecurityGroupId_defaultOu(t *testing.T) { + var v workspaces.WorkspaceDirectory + rName := acctest.RandString(8) + + resourceName := "aws_workspaces_directory.main" + resourceSecurityGroup := "aws_security_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckWorkspacesDirectory(t) + testAccPreCheckAWSDirectoryServiceSimpleDirectory(t) + testAccPreCheckHasIAMRole(t, "workspaces_DefaultRole") + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsWorkspacesDirectoryDestroy, + Steps: []resource.TestStep{ + { + Config: testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurityGroupId_defaultOu_Absent(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAwsWorkspacesDirectoryExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.#", "1"), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.0.custom_security_group_id", ""), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.0.default_ou", ""), + ), + }, + { + Config: testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurityGroupId_defaultOu_Present(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAwsWorkspacesDirectoryExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "workspace_creation_properties.0.custom_security_group_id", resourceSecurityGroup, "id"), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.0.default_ou", "OU=AWS,DC=Workgroup,DC=Example,DC=com"), + ), + }, + { + Config: testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurityGroupId_defaultOu_Absent(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckAwsWorkspacesDirectoryExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.#", "1"), + resource.TestCheckResourceAttrPair(resourceName, "workspace_creation_properties.0.custom_security_group_id", resourceSecurityGroup, "id"), + resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.0.default_ou", "OU=AWS,DC=Workgroup,DC=Example,DC=com"), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccAwsWorkspacesDirectory_ipGroupIds(t *testing.T) { var v workspaces.WorkspaceDirectory rName := acctest.RandString(8) @@ -325,6 +374,181 @@ func TestAccAwsWorkspacesDirectory_ipGroupIds(t *testing.T) { }) } +func TestExpandSelfServicePermissions(t *testing.T) { + cases := []struct { + input []interface{} + expected *workspaces.SelfservicePermissions + }{ + // Empty + { + input: []interface{}{}, + expected: nil, + }, + // Full + { + input: []interface{}{ + map[string]interface{}{ + "change_compute_type": false, + "increase_volume_size": false, + "rebuild_workspace": true, + "restart_workspace": true, + "switch_running_mode": true, + }, + }, + expected: &workspaces.SelfservicePermissions{ + ChangeComputeType: aws.String(workspaces.ReconnectEnumDisabled), + IncreaseVolumeSize: aws.String(workspaces.ReconnectEnumDisabled), + RebuildWorkspace: aws.String(workspaces.ReconnectEnumEnabled), + RestartWorkspace: aws.String(workspaces.ReconnectEnumEnabled), + SwitchRunningMode: aws.String(workspaces.ReconnectEnumEnabled), + }, + }, + } + + for _, c := range cases { + actual := expandSelfServicePermissions(c.input) + if !reflect.DeepEqual(actual, c.expected) { + t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) + } + } +} + +func TestFlattenSelfServicePermissions(t *testing.T) { + cases := []struct { + input *workspaces.SelfservicePermissions + expected []interface{} + }{ + // Empty + { + input: nil, + expected: []interface{}{}, + }, + // Full + { + input: &workspaces.SelfservicePermissions{ + ChangeComputeType: aws.String(workspaces.ReconnectEnumDisabled), + IncreaseVolumeSize: aws.String(workspaces.ReconnectEnumDisabled), + RebuildWorkspace: aws.String(workspaces.ReconnectEnumEnabled), + RestartWorkspace: aws.String(workspaces.ReconnectEnumEnabled), + SwitchRunningMode: aws.String(workspaces.ReconnectEnumEnabled), + }, + expected: []interface{}{ + map[string]interface{}{ + "change_compute_type": false, + "increase_volume_size": false, + "rebuild_workspace": true, + "restart_workspace": true, + "switch_running_mode": true, + }, + }, + }, + } + + for _, c := range cases { + actual := flattenSelfServicePermissions(c.input) + if !reflect.DeepEqual(actual, c.expected) { + t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) + } + } +} + +func TestExpandWorkspaceCreationProperties(t *testing.T) { + cases := []struct { + input []interface{} + expected *workspaces.WorkspaceCreationProperties + }{ + // Empty + { + input: []interface{}{}, + expected: nil, + }, + // Full + { + input: []interface{}{ + map[string]interface{}{ + "custom_security_group_id": "sg-123456789012", + "default_ou": "OU=AWS,DC=Workgroup,DC=Example,DC=com", + "enable_internet_access": true, + "enable_maintenance_mode": true, + "user_enabled_as_local_administrator": true, + }, + }, + expected: &workspaces.WorkspaceCreationProperties{ + CustomSecurityGroupId: aws.String("sg-123456789012"), + DefaultOu: aws.String("OU=AWS,DC=Workgroup,DC=Example,DC=com"), + EnableInternetAccess: aws.Bool(true), + EnableMaintenanceMode: aws.Bool(true), + UserEnabledAsLocalAdministrator: aws.Bool(true), + }, + }, + // Without Custom Security Group ID & Default OU + { + input: []interface{}{ + map[string]interface{}{ + "custom_security_group_id": "", + "default_ou": "", + "enable_internet_access": true, + "enable_maintenance_mode": true, + "user_enabled_as_local_administrator": true, + }, + }, + expected: &workspaces.WorkspaceCreationProperties{ + CustomSecurityGroupId: nil, + DefaultOu: nil, + EnableInternetAccess: aws.Bool(true), + EnableMaintenanceMode: aws.Bool(true), + UserEnabledAsLocalAdministrator: aws.Bool(true), + }, + }, + } + + for _, c := range cases { + actual := expandWorkspaceCreationProperties(c.input) + if !reflect.DeepEqual(actual, c.expected) { + t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) + } + } +} + +func TestFlattenWorkspaceCreationProperties(t *testing.T) { + cases := []struct { + input *workspaces.DefaultWorkspaceCreationProperties + expected []interface{} + }{ + // Empty + { + input: nil, + expected: []interface{}{}, + }, + // Full + { + input: &workspaces.DefaultWorkspaceCreationProperties{ + CustomSecurityGroupId: aws.String("sg-123456789012"), + DefaultOu: aws.String("OU=AWS,DC=Workgroup,DC=Example,DC=com"), + EnableInternetAccess: aws.Bool(true), + EnableMaintenanceMode: aws.Bool(true), + UserEnabledAsLocalAdministrator: aws.Bool(true), + }, + expected: []interface{}{ + map[string]interface{}{ + "custom_security_group_id": "sg-123456789012", + "default_ou": "OU=AWS,DC=Workgroup,DC=Example,DC=com", + "enable_internet_access": true, + "enable_maintenance_mode": true, + "user_enabled_as_local_administrator": true, + }, + }, + }, + } + + for _, c := range cases { + actual := flattenWorkspaceCreationProperties(c.input) + if !reflect.DeepEqual(actual, c.expected) { + t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) + } + } +} + func testAccPreCheckHasIAMRole(t *testing.T, roleName string) { conn := testAccProvider.Meta().(*AWSClient).iamconn @@ -406,84 +630,6 @@ func testAccCheckAwsWorkspacesDirectoryExists(n string, v *workspaces.WorkspaceD } } -func TestExpandSelfServicePermissions(t *testing.T) { - cases := []struct { - input []interface{} - expected *workspaces.SelfservicePermissions - }{ - // Empty - { - input: []interface{}{}, - expected: nil, - }, - // Full - { - input: []interface{}{ - map[string]interface{}{ - "change_compute_type": false, - "increase_volume_size": false, - "rebuild_workspace": true, - "restart_workspace": true, - "switch_running_mode": true, - }, - }, - expected: &workspaces.SelfservicePermissions{ - ChangeComputeType: aws.String(workspaces.ReconnectEnumDisabled), - IncreaseVolumeSize: aws.String(workspaces.ReconnectEnumDisabled), - RebuildWorkspace: aws.String(workspaces.ReconnectEnumEnabled), - RestartWorkspace: aws.String(workspaces.ReconnectEnumEnabled), - SwitchRunningMode: aws.String(workspaces.ReconnectEnumEnabled), - }, - }, - } - - for _, c := range cases { - actual := expandSelfServicePermissions(c.input) - if !reflect.DeepEqual(actual, c.expected) { - t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) - } - } -} - -func TestFlattenSelfServicePermissions(t *testing.T) { - cases := []struct { - input *workspaces.SelfservicePermissions - expected []interface{} - }{ - // Empty - { - input: nil, - expected: []interface{}{}, - }, - // Full - { - input: &workspaces.SelfservicePermissions{ - ChangeComputeType: aws.String(workspaces.ReconnectEnumDisabled), - IncreaseVolumeSize: aws.String(workspaces.ReconnectEnumDisabled), - RebuildWorkspace: aws.String(workspaces.ReconnectEnumEnabled), - RestartWorkspace: aws.String(workspaces.ReconnectEnumEnabled), - SwitchRunningMode: aws.String(workspaces.ReconnectEnumEnabled), - }, - expected: []interface{}{ - map[string]interface{}{ - "change_compute_type": false, - "increase_volume_size": false, - "rebuild_workspace": true, - "restart_workspace": true, - "switch_running_mode": true, - }, - }, - }, - } - - for _, c := range cases { - actual := flattenSelfServicePermissions(c.input) - if !reflect.DeepEqual(actual, c.expected) { - t.Fatalf("expected\n\n%#+v\n\ngot\n\n%#+v", c.expected, actual) - } - } -} - func testAccPreCheckWorkspacesDirectory(t *testing.T) { conn := testAccProvider.Meta().(*AWSClient).workspacesconn @@ -658,6 +804,50 @@ resource "aws_workspaces_directory" "main" { `, rName)) } +func testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurityGroupId_defaultOu_Absent(rName string) string { + return composeConfig( + testAccAwsWorkspacesDirectoryConfig_Prerequisites(rName), + fmt.Sprintf(` +resource "aws_security_group" "test" { + vpc_id = aws_vpc.main.id + name = "tf-acctest-%[1]s" +} + +resource "aws_workspaces_directory" "main" { + directory_id = aws_directory_service_directory.main.id + + workspace_creation_properties { + enable_internet_access = true + enable_maintenance_mode = false + user_enabled_as_local_administrator = false + } +} +`, rName)) +} + +func testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurityGroupId_defaultOu_Present(rName string) string { + return composeConfig( + testAccAwsWorkspacesDirectoryConfig_Prerequisites(rName), + fmt.Sprintf(` +resource "aws_security_group" "test" { + vpc_id = aws_vpc.main.id + name = "tf-acctest-%[1]s" +} + +resource "aws_workspaces_directory" "main" { + directory_id = aws_directory_service_directory.main.id + + workspace_creation_properties { + custom_security_group_id = aws_security_group.test.id + default_ou = "OU=AWS,DC=Workgroup,DC=Example,DC=com" + enable_internet_access = true + enable_maintenance_mode = false + user_enabled_as_local_administrator = false + } +} +`, rName)) +} + func testAccWorkspacesDirectoryConfig_ipGroupIds_create(rName string) string { return composeConfig( testAccAwsWorkspacesDirectoryConfig_Prerequisites(rName), From 35cbae73ce3208faf1cc16434f841a00276ef7ab Mon Sep 17 00:00:00 2001 From: Andrew Babichev Date: Fri, 4 Dec 2020 18:33:11 +0200 Subject: [PATCH 2/4] @review Drop redundant aws_security_group test resource --- aws/resource_aws_workspaces_directory_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/aws/resource_aws_workspaces_directory_test.go b/aws/resource_aws_workspaces_directory_test.go index 16f3e1a4863..bb67c9a2f56 100644 --- a/aws/resource_aws_workspaces_directory_test.go +++ b/aws/resource_aws_workspaces_directory_test.go @@ -808,11 +808,6 @@ func testAccWorkspacesDirectoryConfig_workspaceCreationProperties_customSecurity return composeConfig( testAccAwsWorkspacesDirectoryConfig_Prerequisites(rName), fmt.Sprintf(` -resource "aws_security_group" "test" { - vpc_id = aws_vpc.main.id - name = "tf-acctest-%[1]s" -} - resource "aws_workspaces_directory" "main" { directory_id = aws_directory_service_directory.main.id From 20304a18eb84bae50d9eb14a68bb7c410e02970d Mon Sep 17 00:00:00 2001 From: Andrew Babichev Date: Mon, 7 Dec 2020 13:35:47 +0200 Subject: [PATCH 3/4] Add terraform acctest tags --- aws/resource_aws_workspaces_directory_test.go | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/aws/resource_aws_workspaces_directory_test.go b/aws/resource_aws_workspaces_directory_test.go index bb67c9a2f56..87d78d4270a 100644 --- a/aws/resource_aws_workspaces_directory_test.go +++ b/aws/resource_aws_workspaces_directory_test.go @@ -717,6 +717,10 @@ func testAccWorkspacesDirectoryConfig(rName string) string { testAccAwsWorkspacesDirectoryConfig_Prerequisites(rName), ` resource "aws_workspaces_directory" "main" { directory_id = aws_directory_service_directory.main.id + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } data "aws_iam_role" "workspaces-default" { @@ -738,6 +742,10 @@ resource "aws_workspaces_directory" "main" { restart_workspace = false switch_running_mode = true } + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `) } @@ -800,6 +808,10 @@ resource "aws_workspaces_directory" "main" { enable_maintenance_mode = false user_enabled_as_local_administrator = false } + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `, rName)) } @@ -816,6 +828,10 @@ resource "aws_workspaces_directory" "main" { enable_maintenance_mode = false user_enabled_as_local_administrator = false } + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `, rName)) } @@ -839,6 +855,10 @@ resource "aws_workspaces_directory" "main" { enable_maintenance_mode = false user_enabled_as_local_administrator = false } + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `, rName)) } @@ -857,6 +877,10 @@ resource "aws_workspaces_directory" "test" { ip_group_ids = [ aws_workspaces_ip_group.test_alpha.id ] + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `, rName)) } @@ -880,6 +904,10 @@ resource "aws_workspaces_directory" "test" { aws_workspaces_ip_group.test_beta.id, aws_workspaces_ip_group.test_gamma.id ] + + tags = { + Name = "tf-testacc-workspaces-directory-%[1]s" + } } `, rName)) } From ce1c1bc8b2872b1376b0fedc838e734617cca49c Mon Sep 17 00:00:00 2001 From: Andrew Babichev Date: Mon, 7 Dec 2020 16:02:44 +0200 Subject: [PATCH 4/4] Fix custom security attribute check --- aws/resource_aws_workspaces_directory_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_workspaces_directory_test.go b/aws/resource_aws_workspaces_directory_test.go index 87d78d4270a..48ed67ded79 100644 --- a/aws/resource_aws_workspaces_directory_test.go +++ b/aws/resource_aws_workspaces_directory_test.go @@ -323,8 +323,8 @@ func TestAccAwsWorkspacesDirectory_workspaceCreationProperties_customSecurityGro Check: resource.ComposeAggregateTestCheckFunc( testAccCheckAwsWorkspacesDirectoryExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.#", "1"), - resource.TestCheckResourceAttrPair(resourceName, "workspace_creation_properties.0.custom_security_group_id", resourceSecurityGroup, "id"), - resource.TestCheckResourceAttr(resourceName, "workspace_creation_properties.0.default_ou", "OU=AWS,DC=Workgroup,DC=Example,DC=com"), + resource.TestCheckResourceAttrSet(resourceName, "workspace_creation_properties.0.custom_security_group_id"), + resource.TestCheckResourceAttrSet(resourceName, "workspace_creation_properties.0.default_ou"), ), ExpectNonEmptyPlan: true, },