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

ldap_object_attributes resource #5

Merged
merged 6 commits into from
Mar 26, 2021
Merged

Conversation

trevex
Copy link
Owner

@trevex trevex commented Mar 25, 2021

In our company we have the problem that some groups can not be "owned" by terraform as they are provisioned by other means. For those case ldap_object is not sufficient. We therefore need a generic way to introduce additional attribute values to existing objects.
That's what the ldap_object_attributes resource is for. It owns only a few attributes and their values.

For example imagine a group with the following members:

  • CN=foo,...

Now we want to add several members that are provisioned by terraform. This would look as follows:

resource "ldap_object_attributes" "bar" {
  # DN must be complete (no RDN!)
  dn = "CN=mygroup,OU=world,DC=hello,DC=lan"

  # attributes are specified as a set of 1-element maps
  attributes = [
    { member = "CN=bar,..." },
  ]
}

Applying this will add CN=bar,... as member to the group and leave the pre-existing members in tact. Same applies for changes or destroy.

@trevex
Copy link
Owner Author

trevex commented Mar 25, 2021

@oliverisaac I added another resource. Would you mind reviewing the changes? The PR also includes some minor typo fixes etc.

@trevex trevex assigned trevex and unassigned trevex Mar 25, 2021
@oliverisaac
Copy link
Contributor

Looks great, love the idea of being able to manage specific attributes on a DN without owning the DN!

As I was reading through this I realized we're missing Description attributes on the base resources, adding a description to the base resource would help with #4 :)

@oliverisaac
Copy link
Contributor

Also, to be clear, this is authoritative for the resource? So if I define member attribute then all non-defined members will be removed?

@trevex
Copy link
Owner Author

trevex commented Mar 25, 2021

Yes and no, essentially the ldap_object_attributes only owns specific values of attributes. Only the specific values that are defined for an attribute in the resource are added/changed/deleted. The object is otherwise left in-tact. Pre-existing values of an attribute or ones added by other means are ignored.

Another example:

  1. Group exists and has 5 members
  2. Two members are added using ldap_object_attributes
  3. The group now has 7 members
  4. Another member is added using ldapmodify
  5. Now we have 8 members
  6. We remove one member from the ldap_object_attributes resource
  7. The object is updated and contains 7 members
  8. We destroy our resource
  9. The object contains 6 members (pre-existing + ldapmodify)

So really only specific values of an attribute are owned if you think about the attribute as a key with multiple values. However not all attributes are multi-value, so if the attribute is single value only, this does not apply.

Does that make sense?
Does the name make that clear or are there alternatives you are considering?

This is why the READ operation was a bit tricky, because we are only interested in some values of attributes that are predefined. I was also struggling with the IMPORT operation for now, so I will look into this later.

Regarding #4 you are correct I wanted to add a docs/ folder and a proper README etc. I will do so in another PR.

@trevex
Copy link
Owner Author

trevex commented Mar 25, 2021

I also stumbled upon a bug I didn't notice before:

2021/03/25 15:36:57 [WARN] Provider "github.com/trevex/ldap" produced an invalid plan for ldap_object_attributes.foo, 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:
      - .attributes: planned value cty.SetVal([]cty.Value{cty.MapVal(map[string]cty.Value{"member":cty.StringVal("CN=VossNi,OU=Developer,OU=Dienstkonten,DC=envdevel,DC=lan")}), cty.MapValEmpty(cty.String)}) does not match config value cty.SetVal([]cty.Value{cty.MapVal(map[string]cty.Value{"member":cty.StringVal("CN=VossNi,OU=Developer,OU=Dienstkonten,DC=envdevel,DC=lan")})}) nor prior value cty.SetVal([]cty.Value{cty.MapVal(map[string]cty.Value{"member":cty.StringVal("CN=TestVossNi,OU=Developer,OU=Dienstkonten,DC=envdevel,DC=lan")})})

Which is visible in the plan:

  # ldap_object_attributes.foo will be updated in-place
  ~ resource "ldap_object_attributes" "foo" {
      ~ attributes = [
          - {
              - "member" = "CN=Test1..."
            },
          + {
              + "member" = "CN=Test2..."
            },
          + {},
        ]
        dn         = "CN=GroupA...."
        id         = "CN=GroupA..."
    }

There is an empty set element that pops up for updates, which also causes a warning. So the PR is not ready yet

Create: resourceLDAPObjectAttributesCreate,
Read: resourceLDAPObjectAttributesRead,
Update: resourceLDAPObjectAttributesUpdate,
Delete: resourceLDAPObjectAttributesDelete,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add Description here in prep for #4

@oliverisaac
Copy link
Contributor

ldap_object_attributes only owns specific values of attributes.

Ah! That makes sense. This is going to really help me clean up my .tf code around nested groups :)

and a proper README etc. I will do so in another PR.

For #4 and this PR, I just meant adding a Description attr here so that it's easier to move forward with #4

@trevex trevex merged commit c33fffa into main Mar 26, 2021
@trevex trevex deleted the feature/object-attributes-resource branch March 26, 2021 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants