l-with / terraform-provider-ldap

7 stars 4 forks source link

Set dn as a mutable property. #86

Closed suminhong closed 3 weeks ago

suminhong commented 3 weeks ago

hello. I often want to change the dn of an ldap user. However, when using this provider, the dn is marked as force replacement, which causes the object itself to be regenerated when changed. Therefore, in my short opinion, I think setting ForceNew to false will do the trick, so I'm raising a PR. However, because the current id value is set to dn, this change alone does not work properly. Have you ever had a similar problem as me? In Active Directory, dn is usually changeable. However, to work with terraform you must regenerate it.

However, if only ForceNew is treated as false, the id value changes and an error occurs. When setting id to a different value, there is no more unique and appropriate value than dn. Therefore, if the dn changes, the id should also change.

Please think about it once. Thank you always.

l-with commented 3 weeks ago

The id of a terraform resource identifies the resource (in the state) and thus cannot be changed. In my opinion this matches perfectly to the LDAP attribute dn (distinguished name).

suminhong commented 3 weeks ago

@l-with I agree that DN is the most appropriate as an ID value. However, could you agree that DN should be changeable? DN should be able to change, but the ID value should remain unchanged. I'm not quite sure how to resolve this yet. Could you perhaps help me think it through?

l-with commented 3 weeks ago

This is the terraform flow The resource has to be identified in the state (Resource in state) and via the LDAP-API (Read ()). Both steps need an identifier and the identifier for the API has to be derivable from the identifier in the state.

suminhong commented 3 weeks ago

I think it might be possible to update both the DN and ID values in the Update() function. Would that be a difficult task? Couldn’t we just keep track of the old ID value and move it within the state as well?

suminhong commented 3 weeks ago

I didn't mean to close it on purpose, but I'll think about it anyway. If you come up with any good ideas, call me back! thank you.

l-with commented 3 weeks ago

I think it might be possible to update both the DN and ID values in the Update() function. Would that be a difficult task? Couldn’t we just keep track of the old ID value and move it within the state as well?

There is a co-operation between the provider implementation (implementing read, create, update, delete) and the framework (handling the state (read, lock, update) and calling the implemented provider functions).

l-with commented 3 weeks ago

Which LDAP attributes do you want to change? Please think about changing the dn construction and use sAMAccountName and not cn.

suminhong commented 3 weeks ago

@l-with I came up with this idea because the middle OU, not the name, is often changed. Actually, this isn't a very important issue. However, in actual AD, the dn can be changed, but it is unfortunate that it is not possible in terraform, so I uploaded the pr.