Common: Added Test-IPAddress function

[valainisgt]

Pull Request (PR) description

Refactored code from DefaultGatewayAddress, DnsServerAddress, IPAddress, and Route resources to use a common function for validating an IP Address.

This Pull Request (PR) fixes the following issues

Fixes #408

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource documentation added/updated in README.md in resource folder.
• Resource parameter descriptions added/updated in schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[PlagueHO]

Hi @valainisgt - awesome stuff! Thank you for submitting this. Looks like there a few tests to fix up, nothing major:

1. HQRM validation: There are some localization strings that are no longer used and so need to be removed: https://github.com/dsccommunity/NetworkingDsc/pull/449/checks?check_run_id=618477429
2. Unit Tests: It looks like the tests are failing that validated what would happen when an invalid IP Address is being used: https://dev.azure.com/dsccommunity/NetworkingDsc/_build/results?buildId=1600&view=logs&jobId=158455ef-2db5-56fd-6022-83ded799e050&j=158455ef-2db5-56fd-6022-83ded799e050&t=b737ddad-5217-50db-260a-b6e0dd548ea1

Let me know if you need a hand looking into these.

[PlagueHO]

Thanks for fixing tests @valainisgt - I'll get onto the review tonight. This function may also be a candidate for moving out into the wider DscResource.Common module to share among multiple resources. Tagging @johlju.

[johlju]

This is cool! I was just looking for a helper function just like this one. :)

[johlju]

Made a couple of comments.

Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on @PlagueHO and @valainisgt)

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1423 at r3 (raw file):

Test-IPAddress

Since this function does not return anything, but throws an exception if the test fails I would suggest to rename this function to Assert-IPAddress

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1431 at r3 (raw file):

$AddressFamily

Suggest that this property should not be mandatory. If it is passed the additional test can be done to validate the IP adress belongs to the correct family.

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1438 at r3 (raw file):

TryParse($Address, [ref]0)))

We can instead use [System.Net.IPAddress]::TryParse($Address, [ref] $ipAddress). If the parse succeeds then the IP adress is assigned to $ipAddress. We can then use the property $ipAddress.AddressFamily to do the additional validation for address family below (might simplify the logic?).

IPv6:

AddressFamily      : InterNetworkV6
ScopeId            : 0
IsIPv6Multicast    : False
IsIPv6LinkLocal    : False
IsIPv6SiteLocal    : False
IsIPv6Teredo       : False
IsIPv4MappedToIPv6 : False
Address            : 
IPAddressToString  : ::1

IPv4:

AddressFamily      : InterNetwork
ScopeId            : 
IsIPv6Multicast    : False
IsIPv6LinkLocal    : False
IsIPv6SiteLocal    : False
IsIPv6Teredo       : False
IsIPv4MappedToIPv6 : False
Address            : 16843009
IPAddressToString  : 1.1.1.1

[johlju]

@valainisgt would you like to submit the helper function Assert-IPAddress to the DscResource.Common module? I'm in need of a function like this in SqlServerDsc for a resource I'm currently coding.

[valainisgt]

@johlju absolutely. I will get a you a PR soon.

[johlju]

@valainisgt awesome! Tag me when you sent in the PR in DscResource.Common and I will review as soon as I can. :)

Another tiny comment.

Reviewable status: 0 of 16 files reviewed, 2 unresolved discussions (waiting on @PlagueHO and @valainisgt)

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1449 at r4 (raw file):

-and ($AddressFamily -ne 'IPv4')

Instead of this evaluation we could use switch ($AddressFamily) so that we only evaluate the required address family and not both each time.
Would work with if ($AddressFamily -eq 'IPv4') too.

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1457 at r4 (raw file):

-and ($AddressFamily -ne 'IPv6'

Same as the previous comment.

[PlagueHO]

Hi @valainisgt - It looks like the unit tests on WS2019 are failing. I'm not sure why however, so I'm kicking them off again.

[PlagueHO]

Reviewed 1 of 8 files at r1, 8 of 9 files at r2, 6 of 7 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @valainisgt)

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1414 at r5 (raw file):

<#
    .SYNOPSIS
    Check the Address details are valid and do not conflict with Address family.

Nit pick: Can you indent this (and the ones following) - just for consistency.

E.g.

.SYSNOPSIS
    Check the address...

.PARAMETER AddressFamily
    IP Address....

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1432 at r5 (raw file):

        [System.String]
        $AddressFamily,
        [Parameter(Mandatory = $true)]

Can you add a blank line before this one.

---

source/Modules/NetworkingDsc.Common/NetworkingDsc.Common.psm1, line 1439 at r5 (raw file):

    [System.Net.IPAddress] $ipAddress = $null
    if (-not ([System.Net.IPAddress]::TryParse($Address, [ref] $ipAddress)))

Can you add a blank line before this one?

[johlju]

I havd resolved my comments, so I leave the rest of the PR to @PlagueHO. Great work @valainisgt !

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @valainisgt)

[PlagueHO]

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

[PlagueHO]

@johlju - can you tick off your approval on this one?

[johlju]

Reviewed 1 of 8 files at r1, 8 of 9 files at r2, 4 of 7 files at r4, 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

@PlagueHO signed off and merged.

@valainisgt Thank you for this! Awesome work on this as well, add again thanks for adding it to DscResource.Common.