l-with / terraform-provider-ldap

7 stars 4 forks source link

Problem when data_json must contain a value that has not yet been determined #85

Closed suminhong closed 4 weeks ago

suminhong commented 4 weeks ago

hello. I am creating user and user group with ldap_entry resource. First, look at the code below.

locals {
  ad_users = {
    for user in csvdecode(file("./ad_users.csv")) : user.email_id => user
  }
  ad_groups = toset(distinct([
    for user in local.ad_users : user.group
  ]))

  ds_domain  = ""
  default_dn = ""
}

resource "ldap_entry" "user" {
  for_each = local.ad_users
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass        = ["top", "person", "organizationalPerson", "user"]
    givenName          = [substr(each.value.name, 1, -1)]
    sn                 = [substr(each.value.name, 0, 1)] 
    displayName        = [each.key]
    sAMAccountName     = [each.key]
    userPrincipalName  = ["${each.key}@${upper(local.ds_domain)}"]
    userAccountControl = ["544"]
    mail               = ["${each.key}@${local.ds_domain}"]
    name               = [each.key]
  })
}

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : ldap_entry.user[k].id
      if v.group == each.key
    ]
  })
}

As you can see in the code, I am creating user and use_group as written in the csv file. If you are configuring a group resource using the ID output of the user resource, but the user has not been created in advance, that is, if you want to create a user and a group at the same time, the plan result will appear as shown below.

Terraform will perform the following actions:

  # ldap_entry.group["honglab"] will be created
  + resource "ldap_entry" "group" {
      + dn        = "CN=honglab,OU=..."
      + id        = (known after apply)
        # (1 unchanged attribute hidden)
    }

(The user is created normally.) As you can see, the data_json value is not written at all. If you run Apply as is, the following error message appears.

image
When expanding the plan for ldap_entry.group["honglab"] to include new values learned so far during apply, provider "registry.terraform.io/l-with/ldap" produced an invalid new value for .data_json: was cty.StringVal(""), but now cty.StringVal("...").

If run plan and apply again with the user already created, it works normally. In other words, I think the problem is that the data_json value must contain an unknown value during planning.

I am modifying and using it by creating a string directly rather than outputting the user object as shown below.

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : "CN=${k},${local.default_dn}"
      if v.group == each.key
    ]
  })

  depends_on = [ldap_entry.user]
}

However, I left an issue because I think the provider needs to fix it.

Thx.

l-with commented 4 weeks ago

@suminhong Thanks for the detailed description. Before I start reproducing, could you provide the output of terraform graph from the version with the plan error (after the second apply), for a simple example (referencing the ldap_entry in data_json of another ldap entry).

I would like to see if the terraform plugin sdk correctly detects the dependency inside jsonencode.

A simple, self-contained example would be very useful for troubleshooting or writing a test in the provider anyway.

Please let me know the used version of terraform and the provider too.

suminhong commented 4 weeks ago

Hi, thanks for the quick reply. But this has nothing to do with the dependency graph. In the first case (when an error occurs), there is an implicit dependency because it directly references the ID of the user object, and in the second case (when the error is resolved), this is because the dependency is explicitly set through depens_on.

First of all, as you said, I extracted the graph files.

1. error occurs

tf code

resource "ldap_entry" "user" {
  for_each = local.ad_users
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass        = ["top", "person", "organizationalPerson", "user"]
    givenName          = [substr(each.value.name, 1, -1)]
    sn                 = [substr(each.value.name, 0, 1)]
    displayName        = [each.key]
    sAMAccountName     = [each.key]
    userPrincipalName  = ["${each.key}@${upper(local.ds_domain)}"]
    userAccountControl = ["544"]
    mail               = ["${each.key}@${local.ds_domain}"]
    name               = [each.key]
  })
}

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : ldap_entry.user[k].id
      if v.group == each.key
    ]
  })
}

paln

Terraform will perform the following actions:

  # ldap_entry.group["honglab"] will be created
  + resource "ldap_entry" "group" {
      + dn        = "CN=honglab,OU=..."
      + id        = (known after apply)
        # (1 unchanged attribute hidden)
    }

graph

graph_1

2. error resolved

tf code

resource "ldap_entry" "user" {
  for_each = local.ad_users
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass        = ["top", "person", "organizationalPerson", "user"]
    givenName          = [substr(each.value.name, 1, -1)]
    sn                 = [substr(each.value.name, 0, 1)]
    displayName        = [each.key]
    sAMAccountName     = [each.key]
    userPrincipalName  = ["${each.key}@${upper(local.ds_domain)}"]
    userAccountControl = ["544"]
    mail               = ["${each.key}@${local.ds_domain}"]
    name               = [each.key]
  })
}

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : "CN=${k},${local.default_dn}"
      if v.group == each.key
    ]
  })

  depends_on = [ldap_entry.user]
}

plan

Terraform will perform the following actions:

  # ldap_entry.group["honglab"] will be created
  + resource "ldap_entry" "group" {
      + data_json = jsonencode(
            {
              + member      = [
                  + "CN=honglab,OU=...",
                ]
              + objectClass = [
                  + "top",
                  + "group",
                ]
            }
        )
      + dn        = "CN=honglab,OU=..."
      + id        = (known after apply)
    }

graph

graph_2


As you can see, the dependency graph is the same. Please check! thank you!

l-with commented 4 weeks ago

@suminhong Thanks for the dependent graphs. Please provide a self contained example code by which I can reproduce the problem.

The terraform binary and provider version would be nice.

As a first step please check if

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : ldap_entry.user[k].id
      if v.group == each.key
    ]
  })
  depends_on = [ldap_entry.user]
}

works.

By now my assumption is a timing problem: best practice ist to read the resource after creating. I do this here by return resourceLDAPEntryRead(ctx, d, m).

I found commented lines above this line with a comment sleep, I do not remember why I temporarily add this. If the read of the resource does not reflect the changes immediately this could be the reason of the problem.

Why do I assume this? It is not the responsibility of the provider code to manage the dependencies, this is the responsibility of the provider sdk.

The difference is that the depends_on has the effect that all groups depend on all users, the implicit dependency causes dedicated dependencies from each group to the user in the member attribute.

I would like to check this with your example with a test ldap instance. It is also possible that different LDAP servers have a different timing. Then I would have to remove the reading of the resource after create.

This please name the LDAP server you use, too.

l-with commented 4 weeks ago

@suminhong I could also build a new release with an attribute defining if the re read should happen, the you can check if thus makes a difference in your situation. This could be smarter for you and me. Would do think?

suminhong commented 4 weeks ago
resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : ldap_entry.user[k].id
      if v.group == each.key
    ]
  })
  depends_on = [ldap_entry.user]
}

The same goes for this code.

image

I've tested it several times, but if the ldap_entry.user[k].id value does not exist "at the time of executing the plan," data_json disappears from the result of the plan. If run plan & apply again after the user object has already been created (when the ldap_entry.user[k].id value already exists), it will work normally.

For reference, my Terraform version is 1.9.0, and the ldap provider version is 0.8.2.

The example code is almost exactly what I wrote above. Other than that, I can't provide any more code because it connects to AD within our company's private network.

l-with commented 4 weeks ago

Thanks, I need some time to have a deeper look.

Quick fix: use dn instead id

resource "ldap_entry" "group" {
  for_each = local.ad_groups
  dn       = "CN=${each.key},${local.default_dn}"
  data_json = jsonencode({
    objectClass = ["top", "group"]
    member = [
      for k, v in local.ad_users : ldap_entry.user[k].dn
      if v.group == each.key
    ]
  })
}

This is also somehow more stable code, since the id of a resource is typically not part of the documentation and could be subject of change.

l-with commented 4 weeks ago

You are right: the id value does not exist at the time of the executing of the plan.

The id of a terraform resource can only be part of the Terraform plan, when the resource already exists. This is why two consecutive applies work. The first apply creates the users, then the ids of the users are known for the plan.

Thus using the attribute dn (which is used as id) is the correct way.

It is not a bug of the provider.

l-with commented 4 weeks ago

My assumption that the reason is a timing problem was wrong.

suminhong commented 4 weeks ago

wow! It was resolved by changing the dn reference as you said. thank you so much! You can end this issue here. Thank you very much for your quick support.

suminhong commented 4 weeks ago

@l-with Hello. I have additional questions.

Isn't the phenomenon of data_json not being entered at all when using id (a value that cannot be confirmed during planning) a problem with the ldap provider? Is it a problem with the jsonencode function?

l-with commented 3 weeks ago

@suminhong There is a DiffSuppressFunc for data_json in the provider implementation of the resource ldap_entry because of the attributes ignore_attributes and ignore_attribute_patterns. I added returning false in version 0.9.0 when the id is empty (e.g. the resource is not existing). You can try with this version and using id. Keep in mind that using dn is nonetheless the more stable coding.

suminhong commented 3 weeks ago

@l-with Wow, thanks for the quick fix. But friend, there seems to be a problem with version 0.9.0. As you said, I will be using dn from now on. I just tested whether id can be used in version 0.9.0, but the following error occurred.

image
Error: LDAP Result Code 68 "Entry Already Exists": 00000524: UpdErr: DSID-031A11DA, problem 6005 (ENTRY_EXISTS), data 0

If the existing problem occurred when creating a group object, an error now occurs when creating a user object. This error occurs even though the user in question clearly does not exist. (I tested it by changing the name several times.) Moreover, a user is actually created in AD, and an error occurs even if apply is executed multiple times. I think there is a serious bug in version 0.9.0. I think I might have to delete the release. Please check. For reference, the user plan I attempted to create is as follows.

  # ldap_entry.user["test-user"] will be created
  + resource "ldap_entry" "user" {
      + data_json = jsonencode(
            {
              + description        = [
                  + "Managed By Terraform",
                ]
              + displayName        = [
                  + "test-user",
                ]
              + givenName          = [
                  + "est",
                ]
              + mail               = [
                  + "test-user@domain",
                ]
              + name               = [
                  + "test-user",
                ]
              + objectClass        = [
                  + "top",
                  + "person",
                  + "organizationalPerson",
                  + "user",
                ]
              + sAMAccountName     = [
                  + "test-user",
                ]
              + sn                 = [
                  + "t",
                ]
              + userAccountControl = [
                  + "544",
                ]
              + userPrincipalName  = [
                  + "test-user@domain",
                ]
            }
        )
      + dn        = "CN=test-user,OU=Users,OU=domain,DC=domain,DC=co,DC=kr"
      + id        = (known after apply)
    }
l-with commented 3 weeks ago

@suminhong Ups. Thank you very much for the fast check! Could you try v0.9.1, please.

suminhong commented 3 weeks ago

@l-with I'm really sorry, but the same error occurs as in v0.9.0.

l-with commented 3 weeks ago

@suminhong I am lost. Could you please try locally the following:

provider "ldap" { host = "localhost" port = 389 bind_password = "admin" bind_user = "cn=admin,dc=example,dc=com" }

locals { users = { for user in csvdecode(file("./users.csv")) : user.id => user } groups = toset(distinct([ for user in local.users : user.group ])) default_dn = "dc=example,dc=com" }

resource "ldap_entry" "users" { for_each = local.users

dn = "uid=${each.key},${local.default_dn}" data_json = jsonencode( { objectClass = ["inetOrgPerson"] ou = ["users"] sn = [each.value.sn] cn = ["${each.value.givenName} ${each.value.sn}"] } ) }

resource "ldap_entry" "groups" { for_each = local.groups

dn = "cn=${each.key},${local.default_dn}" data_json = jsonencode({ objectClass = ["top", "posixGroup"], memberUid = [ for id, user in local.users : split("=",split(",",ldap_entry.users[id].id)[0])[1] if user.group == each.key ] gidNumber = ["1"] }) }


- execute `terraform init`, `terraform plan`, `terraform apply`

[users.csv](https://github.com/user-attachments/files/16648006/users.csv)

The example is in my opinion equivalent to yours and works for me (provider version 0.9.1). (I hade some coding effort to depend on the id of the ldap_entry user resources.
suminhong commented 3 weeks ago

@l-with Oh, I'm so sorry. The problem occurred because I had the same name for the user and user group. I confirmed that it works well with id in v0.9.1. Thank you so much for the quick response!