Add support for creating Managed Microsoft Active Directory and Directory Connectors in AWS

[jszwedko]

This action is almost exactly the same as creating a SimpleAD so we
reuse this resource and allow the user to specify the type when creating
the directory (ignoring the size if the type is MicrosoftAD).

I also thought about overloading the size parameter to allow the specification of MicrosoftAD or Enterprise there, but using the type felt closer to the actual API.

[jszwedko]

As an aside, are there downsides to marking these tests as t.Parallel()? The entire suite could probably benefit from this so I'm sure there are some reasons this hasn't been done. These tests in particular can take upwards of 10 minutes.

[jen20]

I think we should consider adding a ValidateFunc here in order to provide feedback as early as possible?

[jszwedko]

TIL about ValidateFunc! Will do.

[jen20]

Hi @jszwedko! Thanks for this - it looks good to me. I've left a comment inline on the diff, so if you could let me know your opinion on that it would be great. Thanks!

[jszwedko]

@jen20 added ValidateFunc in jszwedko@6bf1011

[jszwedko]

Pushed another commit adding support for AD connectors since I was making changes in the area anyway. Let me know what you think though, I can split that one off into a separate pull request if needed, just wanted to start the conversation (and to avoid others duplicating work).

[jen20]

Hi @jszwedko! This looks great! I'm going to pull it locally and run the acceptance tests but I don't see anything preventing this being merged currently.

[jen20]

Actually looks like the acceptance tests are leaving dangling resources - need to investigate whether this is an issue with the changes or the original work:

$ make testacc TEST=./builtin/providers/aws TESTARGS="-run AWSDirectoryService"
==> Checking that code complies with gofmt requirements...
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run AWSDirectoryService -timeout 90m
=== RUN TestAccAWSDirectoryServiceDirectory_basic
--- FAIL: TestAccAWSDirectoryServiceDirectory_basic (498.86s)
    testing.go:165: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Check failed: Expected all resources to be gone, but found: map[string]*terraform.ResourceState{"aws_subnet.foo":*terraform.ResourceState{Type:"aws_subnet", Dependencies:[]string{"aws_vpc.main"}, Primary:*terraform.InstanceState{ID:"subnet-bb6767de", Attributes:map[string]string{"availability_zone":"us-west-2a", "map_public_ip_on_launch":"false", "tags.#":"0", "id":"subnet-bb6767de", "vpc_id":"vpc-e496aa81", "cidr_block":"10.0.1.0/24"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string{}}, Meta:map[string]string(nil)}, Tainted:[]*terraform.InstanceState(nil), Deposed:[]*terraform.InstanceState(nil), Provider:""}, "aws_directory_service_directory.bar":*terraform.ResourceState{Type:"aws_directory_service_directory", Dependencies:[]string{"aws_vpc.main", "aws_subnet.foo", "aws_subnet.bar"}, Primary:*terraform.InstanceState{ID:"d-9267380520", Attributes:map[string]string{"id":"d-9267380520", "vpc_settings.0.subnet_ids.2142165099":"subnet-b3f8dfc4", "type":"SimpleAD", "dns_ip_addresses.1043607771":"10.0.1.155", "size":"Small", "vpc_settings.0.subnet_ids.#":"2", "access_url":"d-9267380520.awsapps.com", "vpc_settings.0.vpc_id":"vpc-e496aa81", "vpc_settings.#":"1", "password":"SuperSecretPassw0rd", "name":"corp.notexample.com", "enable_sso":"false", "connect_settings.#":"0", "vpc_settings.0.subnet_ids.730190149":"subnet-bb6767de", "short_name":"corp", "dns_ip_addresses.2432167742":"10.0.2.153", "dns_ip_addresses.#":"2", "alias":"d-9267380520"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string{}}, Meta:map[string]string(nil)}, Tainted:[]*terraform.InstanceState(nil), Deposed:[]*terraform.InstanceState(nil), Provider:""}, "aws_vpc.main":*terraform.ResourceState{Type:"aws_vpc", Dependencies:[]string{}, Primary:*terraform.InstanceState{ID:"vpc-e496aa81", Attributes:map[string]string{"default_security_group_id":"sg-9b9ba1ff", "cidr_block":"10.0.0.0/16", "tags.#":"0", "main_route_table_id":"rtb-14232c71", "default_network_acl_id":"acl-6fdfe70a", "dhcp_options_id":"dopt-60f40f05", "id":"vpc-e496aa81"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string{}}, Meta:map[string]string(nil)}, Tainted:[]*terraform.InstanceState(nil), Deposed:[]*terraform.InstanceState(nil), Provider:""}, "aws_subnet.bar":*terraform.ResourceState{Type:"aws_subnet", Dependencies:[]string{"aws_vpc.main"}, Primary:*terraform.InstanceState{ID:"subnet-b3f8dfc4", Attributes:map[string]string{"tags.#":"0", "id":"subnet-b3f8dfc4", "vpc_id":"vpc-e496aa81", "cidr_block":"10.0.2.0/24", "availability_zone":"us-west-2b", "map_public_ip_on_launch":"false"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string{}}, Meta:map[string]string(nil)}, Tainted:[]*terraform.InstanceState(nil), Deposed:[]*terraform.InstanceState(nil), Provider:""}}

        State: <no state>

[jszwedko]

Hi @jen20! I did see that locally, but I also saw the same error with master so I was hoping that it was just something with me. I should have some time next week to dig into it if you don't before me.

[catsby]

Hey all –

The Error: Check failed: Expected all resources to be gone, but found message should be resolved in #4510; I'm pulling this locally to verify the acceptance tests and then I'll merge

[catsby]

Things look good here, pulling in, thanks!

[jszwedko]

Awesome, thank you so much @catsby! I hadn't yet had a chance to look into this.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.