search

WahlNetwork / vester

Easily validate and remediate your vSphere configuration
https://wahlnetwork.com
Apache License 2.0
146 stars 45 forks source link

Error validating on "false" values #84

Closed haberstrohr closed 7 years ago

haberstrohr commented 7 years ago

The new unit tests that are being written fail when a value of false is specified in the config.json file. Setting the value to true and later remediating that test works.

Comparing the existing tests that have false values that work, like $cfg.host.esxsyslogfirewallexception, I noticed that the value is Boolean, whereas the tests that are failing are simple strings. I would suspect that a string of false should match another string of false, however I'm not seeing that. I will continue looking into it but wanted to log the issue until it gets cleared up.

haberstrohr commented 7 years ago

I submitted PR85 to address this issue. The issue stems from the values being Strings and not Boolean in addition to the values not existing by default. When the new-vesterconfig command is run it creates a null value. If a user were to change the value to true or false, the test/remediate would fail for a false value. To get it to work properly a value of "True" or "False" would need to be specified, which isn't inherently obvious. That said, the following is an example of what works regardless how the value was specified in the config.json file.

if (((Get-AdvancedSetting -Entity $Object | Where-Object -FilterScript {
    $_.Name -eq 'isolation.tools.setguioptions.enable'
}).Value) -eq $false) {$false} 
else {$true}

A similar method has been used in previous tests (see CDROM tests).

While the solution is a bit convoluted for its purpose, I like that it works even if not properly formatted. Once the settings are configured, if new-vesterconfig is run the proper formatting will show up in the config.json file.

brianbunke commented 7 years ago

I haven't investigated, but I'm curious:

  1. 85 doesn't change any existing tests right now, it just adds new ones. Is that on purpose?

  2. Could you provide instructions to reproduce the unit test fail? I didn't think we had anything looking at config.json, since we don't ship the module with one.

New-VesterConfig is due for a revisit...hoping to wrap up unit tests this week to free up my plate. I shipped it in an MVP state, and weird typing issues like this are coming up with the combination of New-VesterConfig / config.json / hardcoded types in the .Vester.ps1 files.

haberstrohr commented 7 years ago

To answer your questions from above

  1. 85 was held off in my fork until I felt comfortable submitting the PR. I had made local commits, but I'm guessing the PR doesn't track all of those which would explain why they aren't visible to you.

  2. I may be mixing terminology when I say "unit tests" config.json is only created after running New-VesterConfig. The failures I was talking about was related to trying to match a boolean value to a string value.

Hopefully that clears things up.