New Resource: aws_workspaces_directory

[Tensho]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Example

resource "aws_vpc" "main" {
  cidr_block = "10.0.0.0/16"
}

resource "aws_subnet" "private-a" {
  vpc_id = "${aws_vpc.main.id}"
  availability_zone = "us-east-1a"
  cidr_block = "10.0.0.0/24"
}

resource "aws_subnet" "private-b" {
  vpc_id = "${aws_vpc.main.id}"
  availability_zone = "us-east-1b"
  cidr_block = "10.0.1.0/24"
}

resource "aws_directory_service_directory" "main" {
  name = "corp.example.com"
  password = "#S1ncerely"
  size = "Small"
  vpc_settings {
    vpc_id = "${aws_vpc.main.id}"
    subnet_ids = ["${aws_subnet.private-a.id}","${aws_subnet.private-b.id}"]
  }
}

resource "aws_workspaces_directory" "main" {
  directory_id = "${aws_directory_service_directory.main.id}"
}

Test

Output from acceptance testing:

$ AWS_DEFAULT_REGION=us-east-1 make testacc TESTARGS='-run=TestAccAwsWorkspacesDirectory_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesDirectory_basic -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
--- PASS: TestAccAwsWorkspacesDirectory_basic (557.79s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	561.184s
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap	0.307s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags	0.234s [no tests to run]

Notes

Workspaces API still lacks a lot of features support and is not consistent. However, this resource opens the doors to other Workspaces resource, check out #434. I've already requested a couple of AWS API features to support other attributes support and will add them to the resource as soon as it will be possible.

References

• AWS Workspaces RegisterWorkspaceDirectory
• AWS Workspaces DescribeWorkspaceDirectories
• AWS Workspaces DeregisterWorkspaceDirectory

References

• Support for AWS Workspaces

[gdavison]

Thanks for this submission, @Tensho, it's a great start. There are a few things to look at before we merge it in.

The Update function is empty, and I assume that this is because the ForceNew flag is set on the directory_id field. What is the expected behaviour if the subnet_ids change?

The Workspaces Directory needs the IAM role workspaces_DefaultRole before the directory can be registered, as described at https://docs.aws.amazon.com/workspaces/latest/adminguide/workspaces-access-control.html#create-default-role, and the tests require the role to exist. Please add the role to the tests, as our automated tests run on an empty account and resources are destroyed after test runs.

In your tests, you've specified availability zones by name. Please either add a provisioner block with the region or make the availability zones region-independent, for example by using the availability_zones data source.

Please add a reference to the resource documentation in the file https://github.com/terraform-providers/terraform-provider-aws/blob/master/website/aws.erb

[Tensho]

@gdavison Hi Graham, thank you for the quick assistance 🙇

Nor AWS Web Console neither AWS CLI/API don't allow to change subnet_ids after workspaces directory registration. The only possible way for now to change subnets – re-register directory. In general "update" part of API (ModifyWorkspaceCreationProperties, ModifySelfservicePermissions) is really messy and incomplete, so I keep waiting for some requested improvements there.

The reason I specified exact AZs in tests – Workspaces service was not available in all regions and AZs. Once I ran into the issue registering directory in us-east-1a and us-east-1b AZs, but the service couldn't create a new workspace in us-east-1b. Only us-east-1a and us-east-1c AZs worked for me. Here is a pice of advice from the related AWS forum thread on how to determine the right set of AZs :

As a prerequisite, Workspaces service needs to connect to a directory in the same AZ. One way to pick the right AZ is to use the Workspaces quick-start wizard.

or

Open a support case and they will be able to tell you quickly.

Anyway, I tried availability_zones data source and got this error:

Error: OperationNotSupportedException: Create two subnets from [use1-az4, use1-az6, use1-az2] that are in separate Availability Zones.

I understand it's reasonable to diverse test coverage across the different regions and AZs, but how do you setup acc test for resources that are unsupported in a particular region yet?
For example, do you blacklist AZs with data.availability_zones.blacklisted_zone_ids or something like that?

As for workspaces_DefaultRole – you are totally right. Thanks for pointing that out, I overlooked this requirement because didn't have a clean space for tests. Added to the test configuration.

[gdavison]

Our tests default to running in us-west-2, where all AZs seem to work. If the environment variable AWS_DEFAULT_REGION is set, it overrides the region where the tests are run. It looks like all AZs in us-west-2 support this, but only a subset in us-east-1.

To make things a little more complicated, AZ names are not consistent across accounts (https://docs.aws.amazon.com/ram/latest/userguide/working-with-az-ids.html), so using the AZ names isn't enough.

However, I did manage to figure out how to make it work in either us-east-1 or us-west-2.

data "aws_region" "current" {}

data "aws_availability_zones" "available" {
  state = "available"
}

locals {
  region_allowed_az_ids = {
    "us-east-1" = formatlist("use1-az%d", [2, 4, 6])
  }

  allowed_az_ids = lookup(local.region_allowed_az_ids, data.aws_region.current.name, data.aws_availability_zones.available.zone_ids)
}
  
resource "aws_subnet" "test-a" {
  vpc_id = "${aws_vpc.test.id}"
  availability_zone_id = local.allowed_az_ids[0]
  cidr_block = "10.0.1.0/24"
}

resource "aws_subnet" "test-c" {
  vpc_id = "${aws_vpc.test.id}"
  availability_zone_id = local.allowed_az_ids[1]
  cidr_block = "10.0.2.0/24"
}

[gdavison]

For the case of changing subnets assigned to the directory, I was thinking more of an error case where the user creating the Terraform configuration didn't assign all subnets and needs to correct their code. For example (I've excluded unimportant attributes):

First pass:

resource "aws_subnet" "test-a" {}

resource "aws_subnet" "test-c" {}

resource "aws_directory_service_directory" "test" {
  vpc_settings {
    subnet_ids = ["${aws_subnet.test-a.id}","${aws_subnet.test-c.id}"]
  }
}

resource "aws_workspaces_directory" "test" {
  directory_id = "${aws_directory_service_directory.test.id}"
  subnet_ids = ["${aws_subnet.test-a.id}"]
}

Second pass, they add the second subnet:

resource "aws_workspaces_directory" "test" {
  directory_id = "${aws_directory_service_directory.test.id}"
  subnet_ids = ["${aws_subnet.test-a.id}", "${aws_subnet.test-c.id}"]
}

This won't update the aws_workspaces_directory resource. As you mentioned, there is no way to update this, so it would need to be re-created.

[Tensho]

@gdavison Added us-east-1 region support to acceptance tests. Thank you for your help.

Regarding subnets I agree, despite there is no update API for them, I guess it's reasonable to add a validation function to make sure specified subnets are correct. I'll check subnets behavior with AWS Support to get a better understanding of their implications.

[gdavison]

It's looking good. Just a couple changes and we can get this merged in. Thanks!

[gdavison]

Since both attributes are ForceNew, you'll need to remove the Update function. I learned something new today 😄

[Tensho]

From what I can tell, workspaces.CreateTags sets the tags instead of adding tags.

I've tested workspaces tags update within AWS CLI and it works as expected:

$ aws workspaces create-tags --resource-id d-d-xxxxxxxxxx --tags Key=A,Value=1 Key=B,Value=2
$ aws workspaces describe-tags --resource-id d-xxxxxxxxxx
{
    "TagList": [
        {
            "Key": "A",
            "Value": "1"
        },
        {
            "Key": "B",
            "Value": "2"
        }
    ]
}
$ aws workspaces delete-tags --resource-id d-xxxxxxxxxx --tag-keys A # delete tag
$ aws workspaces describe-tags --resource-id d-xxxxxxxxxx
{
    "TagList": [
        {
            "Key": "B",
            "Value": "2"
        }
    ]
}
$ aws workspaces create-tags --resource-id d-xxxxxxxxxx --tags Key=C,Value=3 # add new tag
$ aws workspaces describe-tags --resource-id d-xxxxxxxxxx
{
    "TagList": [
        {
            "Key": "B",
            "Value": "2"
        },
        {
            "Key": "C",
            "Value": "3"
        }
    ]
}
$ # tag B is still there, tags have been updated successfully

Seems like the problem in legacy AWS Go SDK according to the warning I see:

[WARN] Provider "aws" produced an unexpected new value for aws_workspaces_directory.main, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .tags: element "Directory" has vanished

I've added custom WorkspacesUpdateTags function as you suggested, all tests become green.

[Tensho]

@gdavison Please let me know if I can do anything else here 🙇 It would be nice to squash all review commits on merge.

[gdavison]

LGTM 🚀 The acceptance tests pass on my workstation, but have a race condition around the IAM role workspaces_DefaultRole in CI. The tests will need to be serialized in a subsequent PR.

--- PASS: TestAccAwsWorkspacesDirectory_basic (631.95s)
--- PASS: TestAccAwsWorkspacesDirectory_subnetIds (608.25s)
--- PASS: TestAccAwsWorkspacesIpGroup_basic (32.88s)

[ghost]

This has been released in version 2.44.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!