Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft AD Size ignored #2192

Closed
AndyFelton opened this issue Nov 6, 2017 · 16 comments · Fixed by #3421
Closed

Microsoft AD Size ignored #2192

AndyFelton opened this issue Nov 6, 2017 · 16 comments · Fixed by #3421
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Milestone

Comments

@AndyFelton
Copy link

Hi there,

Terraform Version

Terraform v0.10.7

Affected Resource(s)

aws_directory_service_directory

Terraform Configuration Files

resource "aws_directory_service_directory" "active-directory" {
  name     = "${var.name}"
  password = "${var.password}"
  type     = "MicrosoftAD"
  size     = "Small"

  vpc_settings {
    vpc_id     = "${var.vpc_id}"
    subnet_ids = ["${var.subnet1}", "${var.subnet2}"]
  }
}

Expected Behavior

I would expect a Microsoft AD to be created on AWS with a Size of Standard

Actual Behavior

Microsoft AD is created with a Size of Enterprise

Steps to Reproduce

  1. terraform apply
@edwardbartholomew
Copy link
Contributor

I couldn't find in the AWS docs but there is only one size (Large/Enterprise) for type = MicrosoftAD. If you try to create MicrosoftAD in the console, you will notice there are no options for size.

@lorengordon
Copy link
Contributor

lorengordon commented Nov 8, 2017

I think it's something of a new feature. You can see the option in the API docs.

Edit: Does look to be hard-coded to Large

Edit 2: Rather, the size option is not passed to the SDK call

@AndyFelton
Copy link
Author

AndyFelton commented Nov 8, 2017 via email

@edwardbartholomew
Copy link
Contributor

I just tried and didn’t have any option called Size like there is for SimpleAD. There is a distinction between Microsoft AD Standard Edition and Microsoft AD Enterprise Edition which I don’t remember from a couple months ago. Thanks for the link lorengordon. Seems size terminology could also refer to edition for type=MicrosoftAD going forward?

@lorengordon
Copy link
Contributor

Seems size terminology could also refer to edition for type=MicrosoftAD going forward?

@edwardbartholomew That's what it seems to me:

  • type=MicrosoftAD and size=Small would be Standard Edition.
  • type=MicrosoftAD and size=Large would be Enterprise Edition.

@paddycarver paddycarver added the enhancement Requests to existing resources that expand the functionality or scope. label Nov 21, 2017
@philharle
Copy link

I think this issue is two-fold...
a) We need to pass through size as part of the createActiveDirectoryService function.
b) Looking through Amazon's API it would seem like the size parameter is missing at their end too for MicrosoftAD. Has anyone already raised this as a feature request to AWS?

@lorengordon
Copy link
Contributor

@philharle it looks to me like it's present in the API: http://docs.aws.amazon.com/directoryservice/latest/devguide/API_CreateMicrosoftAD.html#DirectoryService-CreateMicrosoftAD-request-Size.

Where are you looking that makes it seem otherwise?

@philharle
Copy link

philharle commented Nov 28, 2017

@lorengordon Apologies my bad, I had incorrectly assumed Terraform had a dependency on the AWS SDK.

What I should be saying is that AWS haven't added the size parameter into their own Go SDK. It exists for SimpleAD here:
https://github.com/aws/aws-sdk-go/blob/master/service/directoryservice/api.go#L4694-L4697
but not for MicrosoftAD here:
https://github.com/aws/aws-sdk-go/blob/master/service/directoryservice/api.go#L4801-L4829

@lorengordon
Copy link
Contributor

@philharle you are quite right. I believe the SDK code is auto-generated from the model, https://github.com/aws/aws-sdk-go/blob/master/models/apis/ds/2015-04-16/api-2.json#L901-L915. That will need to be updated with a Size parameter before terraform can address this issue.

@lorengordon
Copy link
Contributor

I didn't see an issue on the aws-sdk-go project, so I created one: aws/aws-sdk-go#1664

@bflad
Copy link
Contributor

bflad commented Jan 9, 2018

Seems like the SDK just added this functionality in v1.12.58: https://github.com/aws/aws-sdk-go/releases/tag/v1.12.58

@bflad
Copy link
Contributor

bflad commented Jan 9, 2018

Ah sorry, edition (standard/enterprise) was added in v1.12.58. Created an issue for that specifically: #2912

@bflad
Copy link
Contributor

bflad commented Feb 16, 2018

Implementing Edition just happens to now have an open PR: #3421. 😄

@bflad
Copy link
Contributor

bflad commented Feb 20, 2018

This has been merged into master and will be released in v1.10.0 of the AWS provider, likely the end of this week.

@bflad
Copy link
Contributor

bflad commented Feb 27, 2018

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

@ghost
Copy link

ghost commented Apr 7, 2020

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!

@ghost ghost locked and limited conversation to collaborators Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants