Skip to content

feat(AzLocal.DeploymentAutomation): v1.0.1 - optional dnsServers in -NetworkSettingsJson#15

Merged
NeilBird merged 2 commits into
mainfrom
users/nebird/development
May 12, 2026
Merged

feat(AzLocal.DeploymentAutomation): v1.0.1 - optional dnsServers in -NetworkSettingsJson#15
NeilBird merged 2 commits into
mainfrom
users/nebird/development

Conversation

@NeilBird

Copy link
Copy Markdown
Owner

Summary

Bumps AzLocal.DeploymentAutomation to v1.0.1. Adds an optional dnsServers array to the JSON payload accepted by the -NetworkSettingsJson parameter on Start-AzLocalTemplateDeployment. When present, it overrides the environment-wide defaults.dnsServers from naming-standards-config.json for that single deployment, without requiring a config file change. This makes it easier to support multiple sites with different DNS servers from one shared config file (e.g. CI/CD pipelines driving deployments to multiple regions).

Precedence (highest → lowest)

  1. -DnsServers parameter on Start-AzLocalTemplateDeployment (explicit CLI override)
  2. dnsServers array inside -NetworkSettingsJson (per-deployment override) — new in v1.0.1
  3. defaults.dnsServers in naming-standards-config.json (environment-wide default)

Start-AzLocalCsvDeployment is unchanged — the DnsServers column in cluster-deployments.csv continues to map to the top-level -DnsServers parameter and therefore still wins over anything embedded in the JSON, so existing CSV-driven pipelines are unaffected.

Schema

{
    "subnetMask": "255.255.255.0",
    "defaultGateway": "10.0.0.1",
    "startingIPAddress": "10.0.0.10",
    "endingIPAddress":  "10.0.0.50",
    "nodeIPAddresses":  ["10.0.0.100", "10.0.0.101"],

    "dnsServers": ["10.0.0.5", "10.0.0.6"],   // OPTIONAL - overrides config default

    "sanSettings": { /* unchanged */ }
}

Changes

File Change
Private/Get-AzLocalNetworkSettingsFromJson.ps1 Parses + validates optional dnsServers; each entry validated via [System.Net.IPAddress]::Parse; absent / $null / [] all mean "no override"
Public/Start-AzLocalTemplateDeployment.ps1 Three-tier precedence chain in DNS resolution block
AzLocal.DeploymentAutomation.psd1 ModuleVersion = '1.0.1'; detailed v1.0.1 release notes (8,657 chars, under PSGallery 10,000 cap)
AzLocal.DeploymentAutomation.psm1 Version banner: 1.0.1, Updated: May 12th 2026
README.md "Latest Version: 1.0.1"; dnsServers documented as optional field; new "DNS Servers Precedence (v1.0.1)" subsection
Tests/AzLocal.DeploymentAutomation.Tests.ps1 Version assertions bumped to 1.0.1; 6 new unit tests + 1 source-text precedence assertion; pre-existing stale TypeOfDeployment list fixed to include Disaggregated

Backwards-compatibility

  • All existing -NetworkSettingsJson payloads continue to work unchanged — dnsServers is strictly optional.
  • The interactive code path (Get-AzLocalDeploymentNetworkSettings) does not collect DNS servers; interactive deployments still inherit the config default. Set -DnsServers on the cmdlet to override interactively.
  • The shape of the object returned by Get-AzLocalNetworkSettingsFromJson gained a new dnsServers property (value $null when not provided); existing consumers that only read the previously-documented fields are unaffected.

Tests

Passed=532 Failed=0 Skipped=0 Total=532 Duration=00:00:19.00

New test cases under Describe 'Function: Get-AzLocalNetworkSettingsFromJson'Context 'Optional dnsServers override (v1.0.1)':

  • Returns $null when dnsServers omitted (back-compat)
  • Returns $null when dnsServers is []
  • Returns single entry when one provided
  • Returns multiple entries when several provided
  • Throws on invalid IP
  • Throws on whitespace-only entry

Plus a source-text guard under Describe 'Module Content' asserting the three-tier precedence block exists in the module source (prevents regression).

Notes

  • One unrelated pre-existing test was repaired in passing: 'CSV should have valid TypeOfDeployment values' had a stale $validTypes list missing Disaggregated (added back in v1.0.0). Without this fix, the suite was failing 1/532 on main already.

…NetworkSettingsJson

Adds optional 'dnsServers' array to the JSON payload accepted by -NetworkSettingsJson. When present, overrides defaults.dnsServers from naming-standards-config.json for that single deployment.

- Get-AzLocalNetworkSettingsFromJson: parse + validate optional dnsServers; absent/null/empty array means 'no override'
- Start-AzLocalTemplateDeployment: three-tier precedence (-DnsServers > JSON.dnsServers > config default)
- Start-AzLocalCsvDeployment: unchanged - CSV DnsServers column still maps to -DnsServers parameter (highest precedence)
- Tests: +6 unit tests for dnsServers parser + 1 source-text precedence assertion; bumped version assertions to 1.0.1; fixed stale Disaggregated TypeOfDeployment list
- README.md: documented new optional field + 'DNS Servers Precedence (v1.0.1)' subsection
- Manifest: ModuleVersion=1.0.1; detailed release notes (8657 chars, under 10000 PSGallery cap)
- psm1: version banner updated to 1.0.1

Pester: 532/532 passing.
@NeilBird
NeilBird requested a review from Copilot May 12, 2026 16:53

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Bumps AzLocal.DeploymentAutomation to v1.0.1 and adds a per-deployment DNS override via an optional dnsServers array inside -NetworkSettingsJson, with a defined precedence order.

Changes:

  • Add parsing/validation for optional dnsServers in Get-AzLocalNetworkSettingsFromJson.
  • Implement three-tier DNS precedence in Start-AzLocalTemplateDeployment (-DnsServers > JSON dnsServers > config default).
  • Update module versioning/docs and extend Pester coverage for the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
AzLocal.DeploymentAutomation/Private/Get-AzLocalNetworkSettingsFromJson.ps1 Parses + validates optional dnsServers and returns it to callers.
AzLocal.DeploymentAutomation/Public/Start-AzLocalTemplateDeployment.ps1 Applies the three-tier DNS precedence when resolving $DnsServers.
AzLocal.DeploymentAutomation/Tests/AzLocal.DeploymentAutomation.Tests.ps1 Bumps version assertions and adds unit tests + a precedence guard.
AzLocal.DeploymentAutomation/README.md Documents dnsServers in JSON and the DNS precedence rules.
AzLocal.DeploymentAutomation/AzLocal.DeploymentAutomation.psd1 Version bump + v1.0.1 release notes.
AzLocal.DeploymentAutomation/AzLocal.DeploymentAutomation.psm1 Updates the version banner/date.

Comment on lines +2124 to +2128
It 'Module should implement DnsServers precedence: -DnsServers > NetworkSettingsJson.dnsServers > config default (v1.0.1)' {
# Three-tier precedence introduced in v1.0.1. Guard against regression.
$script:ModuleContent | Should -Match 'NetworkSettings\.PSObject\.Properties\[''dnsServers''\]'
$script:ModuleContent | Should -Match 'NamingConfig\.defaults\.dnsServers'
}

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.

Addressed in cb1dc45.

Loosened the regex to match the intent (NetworkSettings ... dnsServers + defaults ... dnsServers) rather than exact identifiers, so it survives variable renames and whitespace. Behaviour-level coverage already lives in the dedicated Get-AzLocalNetworkSettingsFromJson Pester block; this guard's only job is to catch a regression where someone deletes the resolution block entirely.

Chose not to extract a Resolve-AzLocalDnsServers helper - it's 8 lines of straight-line code in one call site and adding an abstraction purely to make a test pretty is the kind of over-engineering the repo's implementationDiscipline guidance flags against.

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.

Addressed in cb1dc45.

Loosened the regex to match the intent (NetworkSettings ... dnsServers + defaults ... dnsServers) rather than exact identifiers, so it survives variable renames and whitespace. Behaviour-level coverage already lives in the dedicated Get-AzLocalNetworkSettingsFromJson Pester block; this guard's only job is to catch a regression where someone deletes the resolution block entirely.

Chose not to extract a Resolve-AzLocalDnsServers helper - it's 8 lines of straight-line code in one call site and adding an abstraction purely to make a test pretty is the kind of over-engineering the repo's implementationDiscipline guidance flags against.

Comment on lines +113 to +118
try {
[System.Net.IPAddress]::Parse([string]$dnsIP) | Out-Null
} catch {
Write-AzLocalLog "Invalid DNS server IP address '$dnsIP' in network settings JSON." -Level Error
throw "Invalid dnsServers entry '$dnsIP'. $($_.Exception.Message)"
}

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.

Addressed in cb1dc45.

Switched all four [IPAddress]::Parse sites in this file (subnetMask/defaultGateway/startingIPAddress/endingIPAddress, the nodeIPAddresses loop, and the new dnsServers block) to [IPAddress]::TryParse([ref]). This avoids the exception-handling cost and yields a consistent, actionable error message that no longer depends on localised .NET exception text. Applied uniformly across the function rather than only the new block, so the validation style stays consistent.

Error messages now look like:

Invalid IP address '10.0.0.999' for field 'defaultGateway' in network settings JSON. Provide a valid IPv4 or IPv6 address.
Invalid dnsServers entry 'not-an-ip'. Provide a valid IPv4 or IPv6 address.

Pester: 532/532 passing.

1. TryParse for IP validation (PR comment #2):
   - Convert all 4 [IPAddress]::Parse sites in Get-AzLocalNetworkSettingsFromJson
     to [IPAddress]::TryParse([ref]) - avoids exception-handling overhead and
     yields a stable, actionable error message (no longer depends on localised
     .NET exception text). Applied consistently across subnetMask, defaultGateway,
     startingIPAddress, endingIPAddress, nodeIPAddresses, and the new dnsServers
     block.

2. Soften source-text precedence guard (PR comment #1):
   - Replace exact-identifier regex with loose intent-based patterns so the test
     survives variable renames or whitespace changes. The behaviour-level
     coverage already lives in the dedicated parser Describe block; this guard
     remains as a cheap regression catch for someone deleting the resolution
     block entirely.
   - Did NOT extract a Resolve-AzLocalDnsServers helper as suggested - 8 lines
     of straight-line code in one call site doesn't warrant a new abstraction
     (per repo implementationDiscipline guidance).

Pester: 532/532 passing.
@NeilBird
NeilBird merged commit eb466c6 into main May 12, 2026
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.

2 participants