From ef50665dcb9a5ebf509425573d458595c86fd7c2 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 20 Feb 2018 06:22:49 -0500 Subject: [PATCH 1/2] resource/aws_directory_service_directory: Validate size argument and various code cleanup --- ...esource_aws_directory_service_directory.go | 163 +++++++++--------- 1 file changed, 83 insertions(+), 80 deletions(-) diff --git a/aws/resource_aws_directory_service_directory.go b/aws/resource_aws_directory_service_directory.go index 7219f56610d..46049dbaa93 100644 --- a/aws/resource_aws_directory_service_directory.go +++ b/aws/resource_aws_directory_service_directory.go @@ -8,18 +8,11 @@ import ( "github.com/hashicorp/terraform/helper/schema" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/directoryservice" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/validation" ) -var directoryCreationFuncs = map[string]func(*directoryservice.DirectoryService, *schema.ResourceData) (string, error){ - "SimpleAD": createSimpleDirectoryService, - "MicrosoftAD": createActiveDirectoryService, - "ADConnector": createDirectoryConnector, -} - func resourceAwsDirectoryServiceDirectory() *schema.Resource { return &schema.Resource{ Create: resourceAwsDirectoryServiceDirectoryCreate, @@ -47,6 +40,10 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { Optional: true, Computed: true, ForceNew: true, + ValidateFunc: validation.StringInSlice([]string{ + directoryservice.DirectorySizeLarge, + directoryservice.DirectorySizeSmall, + }, false), }, "alias": { Type: schema.TypeString, @@ -142,19 +139,13 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { "type": { Type: schema.TypeString, Optional: true, - Default: "SimpleAD", + Default: directoryservice.DirectoryTypeSimpleAd, ForceNew: true, - ValidateFunc: func(v interface{}, k string) (ws []string, es []error) { - validTypes := []string{"SimpleAD", "MicrosoftAD"} - value := v.(string) - for validType, _ := range directoryCreationFuncs { - if validType == value { - return - } - } - es = append(es, fmt.Errorf("%q must be one of %q", k, validTypes)) - return - }, + ValidateFunc: validation.StringInSlice([]string{ + directoryservice.DirectoryTypeAdconnector, + directoryservice.DirectoryTypeMicrosoftAd, + directoryservice.DirectoryTypeSimpleAd, + }, false), }, "edition": { Type: schema.TypeString, @@ -171,24 +162,26 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { } func buildVpcSettings(d *schema.ResourceData) (vpcSettings *directoryservice.DirectoryVpcSettings, err error) { - if v, ok := d.GetOk("vpc_settings"); !ok { + v, ok := d.GetOk("vpc_settings") + if !ok { return nil, fmt.Errorf("vpc_settings is required for type = SimpleAD or MicrosoftAD") - } else { - settings := v.([]interface{}) - - if len(settings) > 1 { - return nil, fmt.Errorf("Only a single vpc_settings block is expected") - } else if len(settings) == 1 { - s := settings[0].(map[string]interface{}) - var subnetIds []*string - for _, id := range s["subnet_ids"].(*schema.Set).List() { - subnetIds = append(subnetIds, aws.String(id.(string))) - } + } + settings := v.([]interface{}) - vpcSettings = &directoryservice.DirectoryVpcSettings{ - SubnetIds: subnetIds, - VpcId: aws.String(s["vpc_id"].(string)), - } + if len(settings) > 1 { + return nil, fmt.Errorf("Only a single vpc_settings block is expected") + } + + if len(settings) == 1 { + s := settings[0].(map[string]interface{}) + var subnetIds []*string + for _, id := range s["subnet_ids"].(*schema.Set).List() { + subnetIds = append(subnetIds, aws.String(id.(string))) + } + + vpcSettings = &directoryservice.DirectoryVpcSettings{ + SubnetIds: subnetIds, + VpcId: aws.String(s["vpc_id"].(string)), } } @@ -196,32 +189,34 @@ func buildVpcSettings(d *schema.ResourceData) (vpcSettings *directoryservice.Dir } func buildConnectSettings(d *schema.ResourceData) (connectSettings *directoryservice.DirectoryConnectSettings, err error) { - if v, ok := d.GetOk("connect_settings"); !ok { + v, ok := d.GetOk("connect_settings") + if !ok { return nil, fmt.Errorf("connect_settings is required for type = ADConnector") - } else { - settings := v.([]interface{}) + } + settings := v.([]interface{}) - if len(settings) > 1 { - return nil, fmt.Errorf("Only a single connect_settings block is expected") - } else if len(settings) == 1 { - s := settings[0].(map[string]interface{}) + if len(settings) > 1 { + return nil, fmt.Errorf("Only a single connect_settings block is expected") + } - var subnetIds []*string - for _, id := range s["subnet_ids"].(*schema.Set).List() { - subnetIds = append(subnetIds, aws.String(id.(string))) - } + if len(settings) == 1 { + s := settings[0].(map[string]interface{}) - var customerDnsIps []*string - for _, id := range s["customer_dns_ips"].(*schema.Set).List() { - customerDnsIps = append(customerDnsIps, aws.String(id.(string))) - } + var subnetIds []*string + for _, id := range s["subnet_ids"].(*schema.Set).List() { + subnetIds = append(subnetIds, aws.String(id.(string))) + } - connectSettings = &directoryservice.DirectoryConnectSettings{ - CustomerDnsIps: customerDnsIps, - CustomerUserName: aws.String(s["customer_username"].(string)), - SubnetIds: subnetIds, - VpcId: aws.String(s["vpc_id"].(string)), - } + var customerDnsIps []*string + for _, id := range s["customer_dns_ips"].(*schema.Set).List() { + customerDnsIps = append(customerDnsIps, aws.String(id.(string))) + } + + connectSettings = &directoryservice.DirectoryConnectSettings{ + CustomerDnsIps: customerDnsIps, + CustomerUserName: aws.String(s["customer_username"].(string)), + SubnetIds: subnetIds, + VpcId: aws.String(s["vpc_id"].(string)), } } @@ -330,13 +325,18 @@ func createActiveDirectoryService(dsconn *directoryservice.DirectoryService, d * func resourceAwsDirectoryServiceDirectoryCreate(d *schema.ResourceData, meta interface{}) error { dsconn := meta.(*AWSClient).dsconn - creationFunc, ok := directoryCreationFuncs[d.Get("type").(string)] - if !ok { - // Shouldn't happen as this is validated above - return fmt.Errorf("Unsupported directory type: %s", d.Get("type")) + var directoryId string + var err error + directoryType := d.Get("type").(string) + + if directoryType == directoryservice.DirectoryTypeAdconnector { + directoryId, err = createDirectoryConnector(dsconn, d) + } else if directoryType == directoryservice.DirectoryTypeMicrosoftAd { + directoryId, err = createActiveDirectoryService(dsconn, d) + } else if directoryType == directoryservice.DirectoryTypeSimpleAd { + directoryId, err = createSimpleDirectoryService(dsconn, d) } - directoryId, err := creationFunc(dsconn, d) if err != nil { return err } @@ -346,8 +346,12 @@ func resourceAwsDirectoryServiceDirectoryCreate(d *schema.ResourceData, meta int // Wait for creation log.Printf("[DEBUG] Waiting for DS (%q) to become available", d.Id()) stateConf := &resource.StateChangeConf{ - Pending: []string{"Requested", "Creating", "Created"}, - Target: []string{"Active"}, + Pending: []string{ + directoryservice.DirectoryStageRequested, + directoryservice.DirectoryStageCreating, + directoryservice.DirectoryStageCreated, + }, + Target: []string{directoryservice.DirectoryStageActive}, Refresh: func() (interface{}, string, error) { resp, err := dsconn.DescribeDirectories(&directoryservice.DescribeDirectoriesInput{ DirectoryIds: []*string{aws.String(d.Id())}, @@ -443,27 +447,23 @@ func resourceAwsDirectoryServiceDirectoryRead(d *schema.ResourceData, meta inter dir := out.DirectoryDescriptions[0] log.Printf("[DEBUG] Received DS directory: %s", dir) - d.Set("access_url", *dir.AccessUrl) - d.Set("alias", *dir.Alias) - if dir.Description != nil { - d.Set("description", *dir.Description) - } + d.Set("access_url", dir.AccessUrl) + d.Set("alias", dir.Alias) + d.Set("description", dir.Description) - if *dir.Type == "ADConnector" { + if *dir.Type == directoryservice.DirectoryTypeAdconnector { d.Set("dns_ip_addresses", schema.NewSet(schema.HashString, flattenStringList(dir.ConnectSettings.ConnectIps))) } else { d.Set("dns_ip_addresses", schema.NewSet(schema.HashString, flattenStringList(dir.DnsIpAddrs))) } - d.Set("name", *dir.Name) - if dir.ShortName != nil { - d.Set("short_name", *dir.ShortName) - } + d.Set("name", dir.Name) + d.Set("short_name", dir.ShortName) d.Set("size", dir.Size) d.Set("edition", dir.Edition) - d.Set("type", *dir.Type) + d.Set("type", dir.Type) d.Set("vpc_settings", flattenDSVpcSettings(dir.VpcSettings)) d.Set("connect_settings", flattenDSConnectSettings(dir.DnsIpAddrs, dir.ConnectSettings)) - d.Set("enable_sso", *dir.SsoEnabled) + d.Set("enable_sso", dir.SsoEnabled) if dir.VpcSettings != nil { d.Set("security_group_id", *dir.VpcSettings.SecurityGroupId) @@ -496,21 +496,24 @@ func resourceAwsDirectoryServiceDirectoryDelete(d *schema.ResourceData, meta int // Wait for deletion log.Printf("[DEBUG] Waiting for DS (%q) to be deleted", d.Id()) stateConf := &resource.StateChangeConf{ - Pending: []string{"Deleting"}, - Target: []string{"Deleted"}, + Pending: []string{ + directoryservice.DirectoryStageActive, + directoryservice.DirectoryStageDeleting, + }, + Target: []string{directoryservice.DirectoryStageDeleted}, Refresh: func() (interface{}, string, error) { resp, err := dsconn.DescribeDirectories(&directoryservice.DescribeDirectoriesInput{ DirectoryIds: []*string{aws.String(d.Id())}, }) if err != nil { - if dserr, ok := err.(awserr.Error); ok && dserr.Code() == "EntityDoesNotExistException" { - return 42, "Deleted", nil + if isAWSErr(err, directoryservice.ErrCodeEntityDoesNotExistException, "") { + return 42, directoryservice.DirectoryStageDeleted, nil } return nil, "error", err } if len(resp.DirectoryDescriptions) == 0 { - return 42, "Deleted", nil + return 42, directoryservice.DirectoryStageDeleted, nil } ds := resp.DirectoryDescriptions[0] From aacae9e8627aea5470ca680e8a5314c19db8c9b4 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 20 Feb 2018 10:11:25 -0500 Subject: [PATCH 2/2] resource/aws_directory_service_directory: Use MaxItems: 1 for connect_settings and vpc_settings attributes --- ...esource_aws_directory_service_directory.go | 56 ++++++++----------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/aws/resource_aws_directory_service_directory.go b/aws/resource_aws_directory_service_directory.go index 46049dbaa93..8d4b8eae0e4 100644 --- a/aws/resource_aws_directory_service_directory.go +++ b/aws/resource_aws_directory_service_directory.go @@ -65,6 +65,7 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { "tags": tagsSchema(), "vpc_settings": { Type: schema.TypeList, + MaxItems: 1, Optional: true, ForceNew: true, Elem: &schema.Resource{ @@ -86,6 +87,7 @@ func resourceAwsDirectoryServiceDirectory() *schema.Resource { }, "connect_settings": { Type: schema.TypeList, + MaxItems: 1, Optional: true, ForceNew: true, Elem: &schema.Resource{ @@ -167,22 +169,15 @@ func buildVpcSettings(d *schema.ResourceData) (vpcSettings *directoryservice.Dir return nil, fmt.Errorf("vpc_settings is required for type = SimpleAD or MicrosoftAD") } settings := v.([]interface{}) - - if len(settings) > 1 { - return nil, fmt.Errorf("Only a single vpc_settings block is expected") + s := settings[0].(map[string]interface{}) + var subnetIds []*string + for _, id := range s["subnet_ids"].(*schema.Set).List() { + subnetIds = append(subnetIds, aws.String(id.(string))) } - if len(settings) == 1 { - s := settings[0].(map[string]interface{}) - var subnetIds []*string - for _, id := range s["subnet_ids"].(*schema.Set).List() { - subnetIds = append(subnetIds, aws.String(id.(string))) - } - - vpcSettings = &directoryservice.DirectoryVpcSettings{ - SubnetIds: subnetIds, - VpcId: aws.String(s["vpc_id"].(string)), - } + vpcSettings = &directoryservice.DirectoryVpcSettings{ + SubnetIds: subnetIds, + VpcId: aws.String(s["vpc_id"].(string)), } return vpcSettings, nil @@ -194,30 +189,23 @@ func buildConnectSettings(d *schema.ResourceData) (connectSettings *directoryser return nil, fmt.Errorf("connect_settings is required for type = ADConnector") } settings := v.([]interface{}) + s := settings[0].(map[string]interface{}) - if len(settings) > 1 { - return nil, fmt.Errorf("Only a single connect_settings block is expected") + var subnetIds []*string + for _, id := range s["subnet_ids"].(*schema.Set).List() { + subnetIds = append(subnetIds, aws.String(id.(string))) } - if len(settings) == 1 { - s := settings[0].(map[string]interface{}) - - var subnetIds []*string - for _, id := range s["subnet_ids"].(*schema.Set).List() { - subnetIds = append(subnetIds, aws.String(id.(string))) - } - - var customerDnsIps []*string - for _, id := range s["customer_dns_ips"].(*schema.Set).List() { - customerDnsIps = append(customerDnsIps, aws.String(id.(string))) - } + var customerDnsIps []*string + for _, id := range s["customer_dns_ips"].(*schema.Set).List() { + customerDnsIps = append(customerDnsIps, aws.String(id.(string))) + } - connectSettings = &directoryservice.DirectoryConnectSettings{ - CustomerDnsIps: customerDnsIps, - CustomerUserName: aws.String(s["customer_username"].(string)), - SubnetIds: subnetIds, - VpcId: aws.String(s["vpc_id"].(string)), - } + connectSettings = &directoryservice.DirectoryConnectSettings{ + CustomerDnsIps: customerDnsIps, + CustomerUserName: aws.String(s["customer_username"].(string)), + SubnetIds: subnetIds, + VpcId: aws.String(s["vpc_id"].(string)), } return connectSettings, nil