OfficeOnlineServerHost: New resource proposal

[JonasCassier]

Pull Request (PR) description

Adds new resource OfficeOnlineServerHost

This Pull Request (PR) fixes the following issues

• Fixes OfficeOnlineServerHost: New resource proposal #62

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).
• 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.
• Unit tests added/updated. See DSC Community Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
• New/changed code adheres to DSC Community Style Guidelines.

---

This change is 

[codecov]

Codecov Report

Merging #71 (60df501) into master (869fff3) will increase coverage by 0%.
Report is 1 commits behind head on master.
The diff coverage is 94%.

Additional details and impacted files

@@         Coverage Diff          @@
##           master   #71   +/-   ##
====================================
  Coverage      80%   81%           
====================================
  Files           6     7    +1     
  Lines         750   804   +54     
====================================
+ Hits          606   657   +51     
- Misses        144   147    +3     

Files	Coverage Δ
...eOnlineServerHost/MSFT_OfficeOnlineServerHost.psm1	94% <94%> (ø)

[JonasCassier]

@johlju I didn't know that

git checkout Feature/OfficeOnlineServerHost
git reset origin/master --hard
git push origin --force

would cause the linked PR to be closed.

Here a new one that i created form my Backup :D.

[johlju]

You shouldn’t use git reset, at least not in this case. I only use reset when trying to undo commits in working branches.

The steps I mentioned in the first PR should work. I do them daily. If you have to do it again. If you get any error on an issued command, don’t issue other commands and let me know what error you encounter on that last issues command.

[johlju]

Reviewable status: 0 of 10 files reviewed, 3 unresolved discussions (waiting on @JonasCassier)

---

src/DSCResources/MSFT_OfficeOnlineServerHost/MSFT_OfficeOnlineServerHost.psm1 line 36 at r1 (raw file):

    Write-Verbose -Message $script:localizedData.GetAllowList

    Import-Module -Name 'OfficeWebApps' -ErrorAction 'Stop' -Verbose:$false

Is it possible to use Assert-Module instead? E.g. Assert-Module -ModuleName 'OfficeWebApps' -ImportModule

---

src/DSCResources/MSFT_OfficeOnlineServerHost/MSFT_OfficeOnlineServerHost.psm1 line 38 at r1 (raw file):

    Import-Module -Name 'OfficeWebApps' -ErrorAction 'Stop' -Verbose:$false

    Confirm-OosDscEnvironmentVariables

Minor. Looking at what this function does we could instead uses these commands: Set-PSModulePath -Path (Get-PSModulePath -FromTarget 'Machine'). Commands from DscResource.Common. It wouldn't do the exactly the same thing, but the end result would be the same, setting the values of the machine PSModulePath to the session one, probably reason is that after installation and prior to first reboot this is necessary.

---

src/DSCResources/MSFT_OfficeOnlineServerHost/MSFT_OfficeOnlineServerHost.psm1 line 50 at r1 (raw file):

    catch
    {
        throw $_

Maybe we can use New-InvalidOperationException, then there is no need to throw and we can provide our own message too (from the localization string file)?

 $errorMessage = $script:localizedData.FailedGettingAllowList
 New-InvalidOperationException -Message $errorMessage -ErrorRecord $_

Though, sometimes there isn't a good idea for the Get-function to throw since that might prevent a configuration to get the current state before everything is installed (e.g. running Get-DscConfiguration before node is in desired state. So we could consider just outputting the exception message with Write-Debug? 🤔 E.g. Write-Debug -Message $_.Exception.Message, or maybe Write-Debug -Message $_.ToString() is better to get full stack trace?

[johlju]

Just had time for a quick glance of the Get-function. Continue as soon as I have time. Back to the reality for while now (real work). 🙂

[JonasCassier]

Reviewable status: 0 of 10 files reviewed, 3 unresolved discussions (waiting on @johlju)

---

src/DSCResources/MSFT_OfficeOnlineServerHost/MSFT_OfficeOnlineServerHost.psm1 line 38 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Minor. Looking at what this function does we could instead uses these commands: Set-PSModulePath -Path (Get-PSModulePath -FromTarget 'Machine'). Commands from DscResource.Common. It wouldn't do the exactly the same thing, but the end result would be the same, setting the values of the machine PSModulePath to the session one, probably reason is that after installation and prior to first reboot this is necessary.

I don't understand that yet. "Assert-Module -ModuleName 'OfficeWebApps' -ImportModule" imports the module. "Set-PSModulePath -Path (Get-PSModulePath -FromTarget 'Machine')" does not import the module. Or do you want to use module auto-loading?