hashicorp / terraform-provider-ad

Active Directory provider for HashiCorp Terraform (experimental)
https://registry.terraform.io/providers/hashicorp/ad/latest
Mozilla Public License 2.0
138 stars 72 forks source link

ad_group_membership removes group memberships every other run #94

Open Nothing4You opened 3 years ago

Nothing4You commented 3 years ago

Terraform Version and Provider Version

Terraform v0.14.9
+ provider registry.terraform.io/hashicorp/ad v0.4.1

Windows Version

Windows 10 against Windows Server Core 2019

Affected Resource(s)

Terraform Configuration Files

data "ad_group" "TestGroup" {
  group_id = "CN=TestGroup,OU=TestOU,DC=ad,DC=mydomain,DC=tld"
}

resource "ad_group_membership" "gm" {
  group_id = data.ad_group.TestGroup.sam_account_name
  group_members = [
    "richard.schwab",
  ]
}

Expected Behavior

Once the group member list has the desired state it should not be modified in subsequent runs.

Actual Behavior

Users are removed from the group every other run.

Steps to Reproduce

This can probably be reproduced with any group member identifier that is accepted by ad_group_membership that is not matching the objectGUID. As per docs this can be

GUID, SID, Distinguished Name, or SAM Account Name

  1. Initial state: group is empty
  2. terraform apply
    
    ad_group_membership.gm: Refreshing state... [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7]

An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: ~ update in-place

Terraform will perform the following actions:

ad_group_membership.gm will be updated in-place

~ resource "ad_group_membership" "gm" { ~ group_members = [

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve.

Enter a value: yes

ad_group_membership.gm: Modifying... [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7] ad_group_membership.gm: Modifications complete after 2s [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

3. User got added, so far so good.
4. `terraform apply`

ad_group_membership.gm: Refreshing state... [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7]

An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: ~ update in-place

Terraform will perform the following actions:

ad_group_membership.gm will be updated in-place

~ resource "ad_group_membership" "gm" { ~ group_members = [

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve.

Enter a value: yes

ad_group_membership.gm: Modifying... [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7] ad_group_membership.gm: Modifications complete after 3s [id=TestGroup_74d6475d-4991-7c77-7e94-31ef7e0630f7]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.


5. Group is now empty. Goto 1.

### Community Note
* Please vote on this issue by adding a 👍 [reaction](https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request
* If you are interested in working on this issue or have submitted a pull request, please leave a comment
koikonom commented 3 years ago

Hi @Nothing4You, thanks for taking the time to report this issue .

This bug has to do with the fact that you're allowed to define any kind of identifier for a group member (GUID, DN, SAM) but we only store its GUID in the state. I will need to think about what's the best way to address this.

In the mean time as a workaround you could use the GUIDs of each group member. Resources and datasources for AD objects such as computers, users, and groups return their GUID by calling the id field.

For example your initial code could look like this:

data "ad_group" "TestGroup" {
  group_id = "CN=TestGroup,OU=TestOU,DC=ad,DC=mydomain,DC=tld"
}

data "ad_user" "u" {
    user_id = "richard.schwab" # This can be the user's SAM, DN, GUID, or SID
}

resource "ad_group_membership" "gm" {
  group_id = data.ad_group.TestGroup.sam_account_name
  group_members = [
    data.ad_user.u.id,
  ]
}
juneeighteen commented 3 years ago

@koikonom , I'm seeing some strange behavior with this. The .u.id response isn't the user's GUID, but rather the user's SID in the directory. I have no way of accessing the user's GUID. Is it possible to add .guid to the user properties? This would go a long way for me.

ncecere commented 3 years ago

another method I used to get around this problem is below. I allows you to just feed in a list of usernames, I have thought about wrapping this in a module too. The group membership stay consistent across runs.

# Create a variable to hold usernames
variable "users" {
  type = list
  default = ["user_name_1","user_name_2"]
}

# Get Group id
data "ad_group" "service_group" {
  group_id = "CN=TestGroup,OU=TestOU,DC=ad,DC=mydomain,DC=tld"
}

# loop through users and get there data
data "ad_user" "user_data" {
  for_each = toset(var.users)
    user_id = each.value
}
# create a local that holds all of the id's
locals {
  add_users = toset([for k, v in data.ad_user.user_data : v.id])
}
# use that list of ID's to add members to a group
resource "ad_group_membership" "app_group_members" {
  group_id = data.ad_group.service_group_app.id
  group_members = local.add_users
}
output "user_id" {
  value = toset([for k, bd in data.ad_user.user_data : bd.id])
}
juneeighteen commented 3 years ago

Happy to share it if anybody needs it, but my personal solution was to write a python script that runs just before any Terraform command. It's ugly in nature, but it queries Active Directory over LDAP and downloads a list of users and their GUIDS. I import that back into Terraform as a .json file.

mafitconsulting commented 2 years ago

Any movement on this @koikonom ? it would be good to get this bug fixed? @ncecere Your solution is fine for single group but im unsure yet how his can map to group membership when iterating over multiple groups and getting guid of each member. Anyone solutioned this yet?

ryno75 commented 1 year ago

Happy to share it if anybody needs it, but my personal solution was to write a python script that runs just before any Terraform command. It's ugly in nature, but it queries Active Directory over LDAP and downloads a list of users and their GUIDS. I import that back into Terraform as a .json file.

Curious... why not just use @koikonom's recommendation to push all group members (i.e. users/sub-groups) through a data source to get the GUID and reference that data source in the group_members attribute? That seems by far the cleanest approach and really the intended design of this provider and Terraform in general.

benjamin-rousseau-shift commented 11 months ago

I personnally tested @koikonom's solution and unlike what you said about a strange behavior with the .id , it worked for me. Although in my case I used the data on ad_computer