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

Added if for checking if the OU is existing and to avoid a cryptical … #52243

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@ChrisRo89
Copy link

ChrisRo89 commented Feb 14, 2019

Whenever an wrong OU was added in the inventory file, the PowerShell script fires the following error message: Failed to join domain: Computer ''Computername'' failed to join domain ''Domain'' from it's current workgroup ''WORKGROUP'' with following error message: The system cannot find the file specified. Which is a kind of confusing, so I added some if block which is checking if the OU is existing.

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

win_domain_membership

ADDITIONAL INFORMATION
        if([System.DirectoryServices.DirectoryEntry]::Exists("LDPA://$domain_ou_path")){
            $add_args["OUPath"] = $domain_ou_path
        }
        else {
            Fail-Json -obj $result -message "The OU $domain_ou_path object was not found."
        }
Christian Rohr Christian Rohr
@ansibot

This comment has been minimized.

Christian Rohr added some commits Feb 14, 2019

Christian Rohr Christian Rohr

@samdoran samdoran removed the needs_triage label Feb 19, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 19, 2019

@jborean93
Copy link
Contributor

jborean93 left a comment

I would also like to see a changelog fragment for this bugfix. It will allow us to backport this fix to ealier Ansible releases.

@@ -123,7 +123,12 @@ Function Join-Domain {

if($domain_ou_path){
Write-DebugLog "adding OU destination arg to Add-Computer args"
$add_args["OUPath"] = $domain_ou_path
if([System.DirectoryServices.DirectoryEntry]::Exists("LDPA://$domain_ou_path")){

This comment has been minimized.

@jborean93

jborean93 Feb 25, 2019

Contributor

Wouldn't this be LDAP instead of LDPA?

This comment has been minimized.

@ChrisRo89

ChrisRo89 Feb 28, 2019

Author

Yes, you are right - that would be better, I'll fix it

This comment has been minimized.

@jborean93

jborean93 Feb 28, 2019

Contributor

I'm also wondering whether we can use the Get-ADObject cmdlet instead. It would keep things more PowerShell like and can handle all the LDAP details inside it. What are your thoughts?

This comment has been minimized.

@ChrisRo89

ChrisRo89 Mar 20, 2019

Author

Yes, we can also use this; I'll check the syntax and push some change

This comment has been minimized.

@ChrisRo89

ChrisRo89 Mar 20, 2019

Author

But the issue here is, that this will only work on Server Systems and not on Windows Client OS, but the System.DirectoryServices Namespaces, is as well on clients as on servers

$add_args["OUPath"] = $domain_ou_path
}
else {
Throw "The specified OU $domain_ou_path was not found."

This comment has been minimized.

@jborean93

jborean93 Feb 25, 2019

Contributor

Best to just call `Fail-Json -obj $result -message "The specified OU '$domain_out_path' was not found." instead of throwing the exception.

This comment has been minimized.

@ChrisRo89

ChrisRo89 Feb 28, 2019

Author

Ok, thank you for the advice

@ansibot ansibot removed the stale_ci label Feb 28, 2019

Christian.Rohr Christian.Rohr
@mattclay

This comment has been minimized.

Copy link
Member

mattclay commented Feb 28, 2019

Christian.Rohr Christian.Rohr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.