createcomputer is not optional - should maybe default to `Computers`

[TJM]

• Reference1: https://github.com/rnelson0/puppet-domain_join/blob/master/manifests/init.pp#L14

  $createcomputer = undef,            # Name of the container for the newly joined nodes. Optional.

• Reference 2: https://github.com/rnelson0/puppet-domain_join/blob/master/templates/domain_join.erb#L49-L54

<%- if @createcomputer then -%>container_ou='<%= @createcomputer %>'<%- end -%>

if [[ -z "${container_ou// }" ]] ; then
  echo "Unable to query without setting an OU. Puppet class variable 'createcomputer' was undef." 1>&2
  exit 1
fi

That if statement makes the createcomputer parameter not optional. It was made not optional by the "-q" query option. Maybe it would be best to default $createcomputer to Computers?

~tommy

[TJM]

actually... wouldn't a "query" like this work...

net ads dn $DOMAIN_DN dc -P -l

That uses the "computer" account to do a search, which wouldn't work if the computer wasn't bound to the domain, and keeps the specific OU "optional" :)

~tommy

[rnelson0]

What do you mean about the -q makes the createcomputer mandatory? I'm not sure I follow that, and everything seems to flow from that.

As for the -P option, as you said, that will not work if there's no computer account. The current query uses a user/pass to avoid relying on the computer account.

[TJM]

The command /usr/local/bin/domain-join -q (unless command https://github.com/rnelson0/puppet-domain_join/blob/master/manifests/init.pp#L68) requires createcomputer (as per the if statement pasted in the first note). The createcomputer parameter defaults to undef, and says in the comment that it is optional (and should be).

The "query" function is used to see if the machine is bound to AD. Using the "-P" (computer account) will fail if this is not the case, and succeed if it is. Using the service account means you have to look for that specific host's entry, which as I have mentioned is a bit interesting, since the default location is CN=Computers,(domain) (*), and that is hardcoding OU=(createcomputer),(DOMAIN).

(*) NOTE: We are using AWS Simple AD, which defaults to CN=Computers, perhaps MS-AD uses OU=Computers? (I realize in a "large" organization, it may be desirable to create different OU's in the AD tree and put computers within those, but along those same lines, it would probably be something like OU=Computers,OU=Location,OU=whatever,dc=domain,dc=com)

We noticed that our machines were attempting to join during every puppet run, which is not desirable, of course. My "PR" fixes that, and returns "createcomputer" to its optional status. It works in the case where you don't have a special OU to put computers in, and are just using the defaults.

EDIT:

Of course the unless command simply be net ads testjoin (seems logical to use the proper tool to test than some arbitrary search).

[TJM]

Any further thoughts on this one? is my wording unclear? I would be happy to chat live about it if you like.