UpdateServicesDSC/UpdateServicesServer - Fix Multiple Languages comparison

jcoconnor commented 5 years ago

Pull Request (PR) description

(Issue-36) Correct Multiple Language Comparison

Ensure that WSUS.Languages is returned as an array of strings to allow multiple languages to be compared correctly in Test-TargetResource

Thanks to @RandomNoun for assistance debugging/preparing this PR.

This Pull Request (PR) fixes the following issues

See #36 for a full description of the problem. UpdateServicesServer was failing to configure WSUS if more than one Update Language was set.

Task list

[x] 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.
[-] Resource parameter descriptions added/updated in README.md, 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.
[x] Unit tests added/updated. See DSC Resource Testing Guidelines.
[-] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
[x] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

jcoconnor commented 5 years ago

So - have done some testing around this and discovered that if you specify 'languages' = '*'; or 'languages' = 'en'; it fails.

However, looking at the MOF definition and specifically https://github.com/mgreenegit/UpdateServicesDsc/blob/dev/DSCResources/MSFT_UpdateServicesServer/MSFT_UpdateServicesServer.schema.mof#L18 this looks to be the correct behaviour.

At the same time I'm a bit concerned that this may break existing code that's using the module.

jcoconnor commented 5 years ago

Have done a bit of further checking and the original module (1.0.75.0) gives the same error if you specify languages as a string rather than a single valued array. So concerns above are a non-issue.

RandomNoun7 commented 5 years ago

Given your latest testing, is this still a breaking change? If not perhaps change the title so it looks a little less scary?

jcoconnor commented 5 years ago

Agreed - I've changed it thanks

jcoconnor commented 5 years ago

Hi @gaelcolas Would you able to take a look at this. Also - have raised issue on issue list -https://github.com/mgreenegit/UpdateServicesDsc/issues/37

jcoconnor commented 5 years ago

@gaelcolas This is your Tuesday ping on a Wednesday

jcoconnor commented 5 years ago

@gaelcolas I've put up a Gist with a test script Test-Invoke.ps1 at https://gist.github.com/jcoconnor/6d32e4afca974dca7c5d6648958292fc

This also includes a failed log before the patch is applied followed by a successful log.

You might get faster testing if the line 'synchronizeautomatically' = $true; is set to 'synchronizeautomatically' = $false;

This pre-supposes an installed WSUS - so probably need to put together that scripting too. (tended to rely on Puppet to put that together for us).

jcoconnor commented 5 years ago

Ok - the following commands will add the necessary features:

# Following commands will initialise the features etc

Find-Module -Name UpdateServicesDsc | Install-Module -Force

Install-WindowsFeature -Name UpdateServices
Install-WindowsFeature -Name UpdateServices-RSAT

gaelcolas commented 5 years ago

Thanks, as discussed, this is very helpful. I'll try to reproduce and move forward.

jcoconnor commented 5 years ago

One thing to note - I was testing this on the fly to get a case I could send you. If you update the MSFT_UpdateServicesServer.psm1 on the fly you need to make sure that the module is loaded again.

I have also added the 'synchronize' = $true;

It can slow things down - especially for the initial sync but if you don't do this you have to make sure you are using one of the "earlier" products - e.g. Windows Server 2008

gaelcolas commented 5 years ago

FTR: I've tested this resource/PR with Test-kitchen and a basic pester test based on the UpdateServicesDsc_Config. It's only a local test on my vSphere environment for now, but better than nothing until I implement a proper pipeline.

Here's my .kitchen.yml:

---
driver:
  name: vcenter
  vcenter_username: 'administrator@vsphere.local'
  vcenter_password: P@ssw0rd!
  vcenter_host:  vcsa.lab.local
  vcenter_disable_ssl_verify: true
  datacenter: lab1
  cluster: cluster

transport:
  username: vagrant
  password: vagrant

verifier:
  name: pester
  test_folder: Tests\Integration

provisioner:
  name: dsc
  dsc_local_configuration_manager_version: wmf5
  configuration_name:
    - UpdateServicesConfig
  configuration_script_folder: Tests\Integration
  configuration_script: UpdateServicesConfig.ps1
  configuration_data:
    AllNodes:
      - nodename: localhost
  modules_from_gallery:
    - chocolatey

platforms:
  - name: win2019_RAW
    driver:
      template: packer-templates/win2019_RAW

suites:
  - name: UpdateServicesConfig

And the quick test I ran after DSC converged, to verify the change to that PR:

$script:DSCModuleName      = 'UpdateServicesDsc'
$script:DSCResourceName    = 'MSFT_UpdateServicesServer'

Describe "$($script:DSCResourceName)_Integration" {

    BeforeAll {
        $Server = Get-WsusServer
        $Config = $Server.GetConfiguration()
        $LanguagesEnabled = $Config.GetEnabledUpdateLanguages()
    }

    It "Should Set language Fr Es En" {
        $LanguagesEnabled | Should Be 'fr','es','en'
    }
}

It's not perfect but it's a good foundation we need to popularise. I'll be working to get that also working on Azure and share the setup so that anyone can test in a timely fashion, and we can improve the missing integration tests.

gaelcolas commented 5 years ago

I will publish this soon on the Gallery, although I want to have a double check to see whether breaking changes were introduced prior to this PR since the last published version.

dsccommunity / UpdateServicesDsc