Skip to content

Fixed an issue where microsoft.ad.domain module did not error correctly#178

Open
AllRWeak wants to merge 1 commit into
ansible-collections:mainfrom
ClarifiedSecurity:microsoft.ad.domain-module-no-error-fix
Open

Fixed an issue where microsoft.ad.domain module did not error correctly#178
AllRWeak wants to merge 1 commit into
ansible-collections:mainfrom
ClarifiedSecurity:microsoft.ad.domain-module-no-error-fix

Conversation

@AllRWeak
Copy link
Copy Markdown

@AllRWeak AllRWeak commented Feb 6, 2025

SUMMARY

This pull requests fixes an issue where the microsoft.ad.domain module leaves a host in a limbo state if it finds that the forest already exists but the host itself is not a domain controller. The issue can occur when more then 1 domain controller exists in a forest and a user tries the re-install one with the microsoft.ad.domain module. When trying to do it manually the domain controller will give an error that the forest or netbios name already exists. With this fix the microsoft.ad.domain module will give the same error.

Fixes #177

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

microsoft.ad.domain module

ADDITIONAL INFORMATION

More info is described in the issue - #177

@softwarefactory-project-zuul
Copy link
Copy Markdown

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from 2c36018 to 384b077 Compare February 6, 2025 12:50
@softwarefactory-project-zuul
Copy link
Copy Markdown

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from 8780dbc to 384b077 Compare June 3, 2025 06:08
@softwarefactory-project-zuul
Copy link
Copy Markdown

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from 384b077 to 4ac6400 Compare August 7, 2025 08:30
@softwarefactory-project-zuul
Copy link
Copy Markdown

@softwarefactory-project-zuul
Copy link
Copy Markdown

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from fd6924a to fb409c8 Compare May 24, 2026 11:27
@centosinfra-prod-github-app
Copy link
Copy Markdown

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from 52282fa to 1448018 Compare May 25, 2026 06:05
@centosinfra-prod-github-app
Copy link
Copy Markdown

Comment thread plugins/modules/domain.ps1 Outdated
}

# Only installing the domain if the forest does not exist or the host is not a domain controller
# This is to avoid an issue where the domain already exists in another domain controller but the host itself is not a DC leaving the host in a limbo state
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the host is a DC but not for the forest requested then I would have through that this should fail rather than just skip the task.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

With this PR this does fail with the correct message:

    msg: |-
        Failed to run Install-ADDSForest, DCPromo exited with 45: The name example.com is already in use on this network. Specify a name that is not in use.
    reboot_required: false

without the PR just reports the task as changed but not actually having configured the domain controller.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The logic here is that the the previous variable $forestContext detects that the forest exists but if the host is not promoted to a domain controller then that cannot happen. That's why the extra check if (-not $forest -or -not $host_is_dc) {... in that case it'll still try and promote the host to a DC but will fail with an error message provided by the Install-ADDSForest command instead of just moving on and failing later with other modules.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah I see sorry it's an -or and not -and check so the code here runs when either $forest is falsey or if Get-ADDomainController "failed".

I'm not sure if Get-ADDomainController is a good check here as it means the host needs to have the ActiveDirectory module is installed which isn't guaranteed here. An alternative could be through WMI/CIM

$os = Get-CimInstance -ClassName Win32_OperatingSystem -Property ProductType
$isDC = $os.ProductType -eq 2

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes a valid point. I updated the PR to use Get-CimInstance instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Is it fine to be pulled now?

@AllRWeak AllRWeak force-pushed the microsoft.ad.domain-module-no-error-fix branch from 1448018 to 124bb0c Compare May 27, 2026 06:41
@centosinfra-prod-github-app
Copy link
Copy Markdown

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.

microsoft.ad.domain does not fail when the domain already exists

2 participants