Skip to content

SPSUpdate: harmonize Boolean config switches to accept 1/0 - #77

Merged
luigilink merged 3 commits into
mainfrom
fix/harmonize-config-booleans-76
Oct 8, 2026
Merged

luigilink merged 3 commits into
mainfrom
fix/harmonize-config-booleans-76

Conversation

@luigilink

Copy link
Copy Markdown
Owner

Pull Request (PR) description

Harmonizes the type handling of every Boolean switch in the configuration so they all behave the same way, after a CA-GIP operator hit Configuration property 'Reboot.Enable' must be a Boolean ($true or $false), not '1'. when setting Reboot.Enable = 1.

Root cause: the Reboot, Remoting and Execution switches enforced a strict -isnot [bool] guard (rejecting 1), while the older flags (Binaries.ProductUpdate, Binaries.ShutdownServices, MountContentDatabase, UpgradeContentDatabase, SideBySideToken.Enable) had no guard and were coerced in Boolean contexts — so 1 worked on those, but so did a typo like 'false' (which evaluates to $true).

Change:

  • New public helper ConvertTo-SPSConfigBoolean (validate + normalize to [bool]): accepts $true/$false and the integers 1/0; throws a clear, property-named error on any other value (out-of-range integer, string such as '1'/'yes'/'false', $null). Only genuine integer types are accepted for the 1/0 form, so the [bool]'false' = $true footgun is avoided.
  • Applied to all nine Boolean config switches in Get-SPSUpdateConfiguration, so each one now accepts $true/$false/1/0 and rejects everything else. This also hardens the previously unguarded flags against typos.
  • Updated the example CONTOSO-PROD.example.psd1 comments to note that 1/0 are accepted.

Validation: PSSA 0 warning/error; Pester 310/0/1 (+11 new helper unit tests). End-to-end check via Get-SPSUpdateConfiguration confirms 1→$true, 0→$false, 2/'yes'→throw.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Added/updated documentation and descriptions where appropriate?
  • New/changed code adheres to [Style Guidelines]?

Add a shared ConvertTo-SPSConfigBoolean helper that validates and normalizes
every Boolean configuration switch to [bool]: it accepts a real Boolean or the
integers 1/0 and throws a clear, property-named error on anything else.

Previously the Reboot/Remoting/Execution switches enforced a strict [bool] and
threw on 1, while the older flags coerced any truthy value (so 1 worked but a
typo like 'false' silently became $true). All nine switches now behave the same.

Co-authored-by: Copilot <[email protected]>
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Pester Test Results

  1 files   48 suites   11s ⏱️
199 tests 199 ✅ 0 💤 0 ❌
313 runs  313 ✅ 0 💤 0 ❌

Results for commit 50eb144.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The readiness checker remains inconsistent with the new contract, documentation is incomplete, and several integral types are incorrectly rejected.

3 open findings
What changed in this PR

Harmonizes Boolean configuration handling by accepting Booleans and integer 1/0 while rejecting unsafe coercions.

Changes:

  • Adds and exports a shared Boolean normalizer.
  • Applies normalization to all nine configuration switches.
  • Updates tests, example configuration, and changelog.
File Description
CHANGELOG.md Documents the new Boolean contract.
src/​Config/​CONTOSO-PROD.example.psd1 Documents accepted Boolean values.
src/​Modules/​SPSUpdate.Common/​SPSUpdate.Common.psd1 Exports the new helper.
src/​Modules/​SPSUpdate.Common/​Public/​ConvertTo-SPSConfigBoolean.ps1 Implements validation and normalization.
src/​SPSUpdate.ps1 Normalizes all nine Boolean settings.
tests/​ConvertTo-SPSConfigBoolean.Tests.ps1 Tests accepted and rejected values.
tests/​SPSUpdate.Common.Tests.ps1 Verifies the helper export.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

# binaries file share) may fail unless Kerberos delegation (KCD/RBCD) is configured.
# Leave off for secure, strict environments.
# Possible values : $true | $false. Default if omitted: $false
# Possible values : $true | $false (1 | 0 also accepted). Default if omitted: $false

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 08f60cb. Test-SPSUpdateReadiness.ps1 now normalizes both Reboot.Enable and Remoting.AllowFallback through ConvertTo-SPSConfigBoolean, so the pre-flight check shares the runtime contract: 1/0 are accepted (and AllowFallback = 1 enables the fallback probes), while an invalid value is reported as a readiness FAIL with the helper's message.

Comment thread src/Modules/SPSUpdate.Common/Public/ConvertTo-SPSConfigBoolean.ps1 Outdated
Binaries = @{
# ProductUpdate : allow the binary installation step.
# Possible values : $true | $false. Default if omitted: $true
# Possible values : $true | $false (1 | 0 also accepted). Default if omitted: $true

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 08f60cb. wiki/Configuration.md now carries one shared Boolean switches rule listing all nine switches and stating they accept $true/$false or 1/0, with anything else rejected. The example psd1 comments were already updated to match.

- Test-SPSUpdateReadiness normalizes Reboot.Enable and Remoting.AllowFallback
  through ConvertTo-SPSConfigBoolean so the pre-flight check shares the runtime
  contract (1/0 accepted; a bad value becomes a readiness FAIL).
- ConvertTo-SPSConfigBoolean accepts every integral type (byte/sbyte/int16/uint16/
  int32/uint32/int64/uint64) valued 0/1, not just four, matching its documented API.
- Document the shared 1/0 Boolean rule for all nine switches in wiki/Configuration.md.

Co-authored-by: Copilot <[email protected]>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The readiness check silently ignores an invalid explicit Reboot.Enable value that runtime configuration loading rejects.

2 open findings
1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject invalid explicit Reboot.Enable values during readiness checks

src/​Test-SPSUpdateReadiness.ps1:349

An invalid explicit Reboot.Enable is silently treated as $false here. The readiness check can therefore exit successfully (and skip the reboot status-store warning), even though Get-SPSUpdateConfiguration will reject the same configuration at runtime. Distinguish an omitted optional value from a present invalid value and record the latter as a readiness failure, as the Remoting.AllowFallback check does below.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Distinguish an omitted Reboot.Enable (legitimate default $false) from a present
but invalid value: the latter is now recorded as a readiness FAIL (matching the
Remoting.AllowFallback check and the runtime config loader) instead of being
silently treated as disabled.

Co-authored-by: Copilot <[email protected]>
@luigilink

Copy link
Copy Markdown
Owner Author

Addressed the remaining 🔵 finding in 50eb144: Test-SPSUpdateReadiness.ps1 now distinguishes an omitted Reboot.Enable (legitimate default $false, no finding) from a present-but-invalid value, which is recorded as a readiness FAIL — matching the Remoting.AllowFallback check and the runtime loader. PSSA 0, Pester 312/0/1.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation consistently fulfills the documented contract with appropriate validation, integration, and tests.

2 open findings

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@luigilink luigilink left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

LGTM

@luigilink
luigilink merged commit 884a80a into main Oct 8, 2026
4 checks passed
@luigilink luigilink mentioned this pull request Oct 8, 2026
3 tasks done
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.

Config: harmonize Boolean switches to accept 1/0 (Reboot.Enable = 1 is rejected while other flags accept it)

2 participants