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

FreeBusyChecker Final Version #2264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MarcoLFrancisco
Copy link
Contributor

Issue:

Free/Busy cases are a challenge as they involve many moving parts and configurations in hybrid topology.

Reason:

Free/Busy cases are very time-consuming and demanding due to the complexity of verifying different settings across both Exchange On-Premises and Exchange Online.

Fix:

  • Automates Free/Busy configuration checks in a hybrid topology.
  • Validates settings for both Exchange On-Premises and Exchange Online.
  • Verifies both DAuth and OAuth configurations.

Validation:

  • Tested across multiple hybrid environments.
  • Cross-checked results with manual verification.
  • Provides clear output for support engineers.
  • Reviewed in previous PR by David Paulson

@MarcoLFrancisco MarcoLFrancisco requested a review from a team as a code owner January 7, 2025 22:53
@MarcoLFrancisco
Copy link
Contributor Author

MarcoLFrancisco commented Jan 7, 2025

New PR for FreeBusyChecker script due to difficulty in squashing previous commits

@dpaulson45
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dpaulson45
Copy link
Member

Old PR #1934


$Script:ExchangeOnlineDomain = ($Script:UserOnline -split "@")[1]

if ($Script:ExchangeOnlineDomain -like "*.mail.onmicrosoft.com") {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about other cloud? Is the script expected to work on these (e.g., GCC, Gallatin…)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were not considered at start, I had no reference for this clouds and no way to test.
We can include.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is 100% required before we release. It would be nice yes, but I would rather get something out there and us create an issue for this and address it soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's okay to address this with a later update. We should mention in the documentation that the script works with WW cloud only.

$Script:IntraOrgCon = Get-IntraOrganizationConnector -WarningAction SilentlyContinue -ErrorAction SilentlyContinue | Select-Object Name, TarGetAddressDomains, DiscoveryEndpoint, Enabled
ShowParameters
CheckParameters
if ($Script:IntraOrgCon.enabled -like "True") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn’t the object type bool here? If it is -eq $true would be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to -eq

image

if ($Script:IntraOrgCon.enabled -like "True") {
$Auth = hostOutputIntraOrgConEnabled($Auth)
}
if ($Script:IntraOrgCon.enabled -like "False") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to -eq

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you check what the return type of $Script:IntraOrgCon.enabled is?

$Script:IntraOrgCon.enabled.GetType()

If it's Boolean the check should be like this:

$Script:IntraOrgCon.enabled -eq $true or $Script:IntraOrgCon.enabled -eq $false

Write-Host -ForegroundColor Green " Checking Exchange Online Configuration"
Write-Host " Testing Connection to Exchange Online with EO Prefix."
try {
$CheckExoMailbox = get-EOMailbox $Script:UserOnline -ErrorAction Stop
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Get-EOMailbox (Pascal Case) as we do with the other cmdlets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected

image

@MarcoLFrancisco
Copy link
Contributor Author

Removed line 252 on file FreeBusyChecker.ps1, it's incorrect:

image

}
function FetchEWSInformation {
if (-not $Script:WebServicesVirtualDirectory -or -not $Script:WebServicesVirtualDirectoryOAuth) {
$Script:WebServicesVirtualDirectory = Get-WebServicesVirtualDirectory -Server $Script:Server | Select-Object Identity, Name, ExchangeVersion, *Authentication*, *url -ErrorAction SilentlyContinue
Copy link
Contributor

@lusassl-msft lusassl-msft Jan 13, 2025

Choose a reason for hiding this comment

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

These Get- calls to query the virtual directory information are very time consuming as we query IIS metabase. Can we check if the required information are available in AD tool (by using -ADPropertiesOnly switch parameter). This would improve the execution time of the script. Not all information are stored in AD but we should at least confirm for the use-case of the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, great point! Will do.

$MissingParameters += "Exchange Online Domain. Example: contoso.mail.onmicrosoft.com"
}
if ([string]::IsNullOrWhiteSpace($Script:ExchangeOnPremLocalDomain)) {
$MissingParameters += "Exchange On Premises Local Domain. Example: . 'C:\scripts\FreeBusyChecker\FreeBusyChecker.ps1' -OnPremisesUser [email protected]"
Copy link
Contributor

@lusassl-msft lusassl-msft Jan 13, 2025

Choose a reason for hiding this comment

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

It would be better to do something like this for the example as this will reflect the name of the script (even if it was renamed for whatever reason):

.\$($script:MyInvocation.MyCommand.Name) -OnPremisesUser [email protected]

Same for the following examples (line 51, 54 and 57)

Copy link
Member

Choose a reason for hiding this comment

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

Will $($script:MyInvocation.MyCommand.Name) return the script name here as we are in a different function.

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I remember it should return the name of the script even if $($script:MyInvocation.MyCommand.Name) is part of a function / method within the main script.

```
Imports and Installs the following Modules (if not available):

PSSnapin: microsoft.exchange.management.powershell.snapin
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that we should ask customers to load the PSSnapin this way. You can use the shared Confirm-ExchangeShell function to check and import it in a supported way.


Example Screen Output:

![image](./image1.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to redact the valid tenant information (e.g., DomainNames). We are not allowed to use non-CELA approved domains in our documentations.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch, this needs to be done and squashed to avoid a commit history showing the incorrect names.


Example TXT Output:

![image](./image2.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to redact the valid tenant information (e.g., DomainNames). We are not allowed to use non-CELA approved domains in our documentations.


Example HTML Output

![image](./image3.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to redact the valid tenant information (e.g., DomainNames). We are not allowed to use non-CELA approved domains in our documentations.

$Script:WebServicesVirtualDirectoryOAuth = $Script:WebServicesVirtualDirectory
}
}
function CheckIfExchangeServer {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend using Confirm-ExchangeShell here (shared function).

[Parameter(Mandatory = $false, ParameterSetName = "Test")]
[string]$OnPremLocalDomain
)
begin {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add GenericScriptUpdate.ps1 here? This adds update capabilities to the script and allows the script to perform an auto-update whenever a new version is released.

@lusassl-msft
Copy link
Contributor

@iserrano76 would you be interested of being a co-owner of this script? Similar to what you do with the MDO script.

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.

3 participants