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

AADAdministrativeUnit - fixes #4437 #4462

Merged
merged 5 commits into from
Mar 21, 2024

Conversation

salbeck-sit
Copy link
Contributor

Pull Request (PR) description

Update AADAdministrativeUnit to handle omitted Ensure and/or Id in config

This Pull Request (PR) fixes the following issues

Fixes #4437

@salbeck-sit
Copy link
Contributor Author

The following lines (original 105..115) have been removed:

    #region resource generator code
    if (-Not [string]::IsNullOrEmpty($Id))
    {
        $getValue = Get-MgBetaDirectoryAdministrativeUnit -AdministrativeUnitId $Id -ErrorAction Stop
    }

    if (-not $getValue -and -Not [string]::IsNullOrEmpty($DisplayName))
    {
        $getValue = Get-MgBetaDirectoryAdministrativeUnit -Filter "DisplayName eq '$DisplayName'" -ErrorAction Stop
    }
    #endregion

The issue is that "Get-MgBetaDirectoryAdministrativeUnit -AdministrativeId $Id -ErrorAction Stop" will throw an exception if the AU doesn't exist. This exception translates into the AU is reported as not being found which in turn results in a new AU being created regardless of existence of an AU with the same DisplayName.
The update tries to get the AU using ID but doesn't throw, then goes on to get the AU using DisplayName if it doesn't exist.
This in turn ensures (sic) that an AU will only be created if an AU with the same DisplayName doesn't exist.
The basic issue is using an export from a different tenant where the ID may be different.

@ricmestre
Copy link
Contributor

@salbeck-sit You need to call the cmdlets with ErrorAction = SilentlyContinue so it doesn't bounce into the catch section, please check the example below from L1044..L1067.

@salbeck-sit
Copy link
Contributor Author

That is exactly what is left after the 'offending' lines were removed. It seems that this was added in an effort to optimize performance but the older code-block wasn't removed. It is now. The current code now contains the following:

    $getValue = $null
    #region resource generator code
    if (-not [string]::IsNullOrEmpty($Id))
    {
        if ($null -ne $Script:exportedInstances -and $Script:ExportMode)
        {
            $getValue = $Script:exportedInstances | Where-Object -FilterScript {$_.Id -eq $Id}
        }
        else
        {
            $getValue = Get-MgBetaDirectoryAdministrativeUnit -AdministrativeUnitId $Id -ErrorAction SilentlyContinue
        }
    }

    if ($null -eq $getValue -and -not [string]::IsNullOrEmpty($DisplayName))
    {
        Write-Verbose -Message "Could not find an Azure AD Administrative Unit with Id {$Id}"
        if (-Not [string]::IsNullOrEmpty($DisplayName))
        {
            if ($null -ne $Script:exportedInstances -and $Script:ExportMode)
            {
                $getValue = $Script:exportedInstances | Where-Object -FilterScript {$_.DisplayName -eq $DisplayName}
            }
            else
            {
                $getValue = Get-MgBetaDirectoryAdministrativeUnit -Filter "DisplayName eq '$DisplayName'" -ErrorAction Stop
            }
        }
    }
    #endregion

Note that -EA Stop doesn't throw when -Filter is used and a resource isn't found.

@NikCharlebois NikCharlebois merged commit 21cbec1 into microsoft:Dev Mar 21, 2024
2 checks passed
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.

AADAdministrativeUnit: Cannot leave 'Ensure' as default
3 participants