Fix empty string nullification

[Clebam]

Summary

Linked to #264

Related Issues (if any)

#264

Checklist

• 🟢 Spec tests.
• 🟢 Acceptance tests.
• Manually verified.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.98%. Comparing base (900a944) to head (3c1b39e).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #292   +/-   ##
=======================================
  Coverage   91.98%   91.98%           
=======================================
  Files           6        6           
  Lines         711      711           
=======================================
  Hits          654      654           
  Misses         57       57           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Clebam]

Hi @jordanbreen28 , for this one I 'm not sure what is going on.
I can see the specs are using dsc_xwebsite resources and not dsc_website (https://github.com/dsccommunity/WebAdministrationDsc) (though I don't think this is the main cause of the failure)

[jordanbreen28]

hey @Clebam - yeah i don't think thats the issue as our nightly builds are passing. Something that has changed in this PR has caused it - Not to worry though! I'll be happy to collaborate, spin up an environment and take a look at this myself shortly. Appreciate all your work on these!

[jordanbreen28]

@Clebam I've been looking into this some more, running tests and trying to get a working solution, but the more time I've spent on this I'm starting to think this maybe isn't the way to go 👎 I think a fix is more appropriate in the puppet-resource_api, and how it's compares the empty or nil enum values.

[Clebam]

I gave a look at resource-api, it's a bit overwhelming at first glance. Not sure if I can spot something useful.

[jordanbreen28]

@Clebam agreed, it is definitely a biggy.
No pressure. I can make note of this and bring it up with the team to get prioritised!

[Clebam]

@jordanbreen28 However I'm curious to know what you saw that is going wrong with my fix. Maybe there is more I can do to it 🤔 (and I'm also interested if I can still try to use my quick fix in my env without breaking something. From my point of view, it did not have any wrong effects, but given you found issues, I'm a bit worried to merge it into my production environment 😓 )

[jordanbreen28]

@Clebam actually disregard my previous message! This should work. I'm going to spend some more time in prying into the code to see if I can spot what is exactly happening here.

Could you rebase this off main? Some changes have went in, wondering if they effect the behaviour in anyway.

[Clebam]

@jordanbreen28, I rebased from main. If you have any logs from the puppet run that fails, I could give a look.

[jordanbreen28]

@jordanbreen28, I rebased from main. If you have any logs from the puppet run that fails, I could give a look.

@Clebam apologies, I was off last week. I'll get to looking at this and providing some logs as soon as I can.
Whats interesting is I noticed this only fails on puppet 8 - with changes to strict variable settings from puppet 7 ~> 8, I can bet thats the issue.

[jordanbreen28]

@Clebam so the issue here is that we are converting nil enums to empty strings even when that is not a valid enum value. So I would recommend not running this in any of your environments as this will cause unexpected behaviour! I'm sure there is a way we can convert only valid values..

Error: /Stage[main]/Main/Dsc_xwebsite[DefaultSite]: Could not evaluate: Provider returned data that does not match the Type Schema for `dsc_xwebsite[DefaultSite]`
 Value type mismatch:
    * dsc_bindinginfo: [{"bindinginformation"=>"*:80:", "certificatestorename"=>"", "certificatesubject"=>nil, "certificatethumbprint"=>"", "hostname"=>"", "ipaddress"=>"*", "port"=>80, "protocol"=>"http", "s
slflags"=>"0"}] (index 0 entry 'certificatestorename' expects an undef value or a match for Enum['My', 'WebHosting'], got '')

Debug: dsc_xwindowsfeature: Collecting data from the DSC Resource
Debug: dsc_xwindowsfeature: Canonicalized Resources: [{:dsc_displayname=>"ASP.NET 4.7", :dsc_ensure=>"Present", :dsc_includeallsubfeature=>false, :dsc_logpath=>nil, :dsc_name=>"Web-Asp-Net45", :name=>"AspNet4
5"}]
Debug: Current State: {:dsc_displayname=>"ASP.NET 4.7", :dsc_ensure=>"Present", :dsc_includeallsubfeature=>false, :dsc_logpath=>nil, :dsc_name=>"Web-Asp-Net45", :name=>"AspNet45"}
Debug: dsc_xwebsite: Collecting data from the DSC Resource
Error: /Stage[main]/Main/Dsc_xwebsite[NewWebsite]: Could not evaluate: Provider returned data that does not match the Type Schema for `dsc_xwebsite[NewWebsite]`
 Value type mismatch:
    * dsc_bindinginfo: [{"bindinginformation"=>"*:80:", "certificatestorename"=>"", "certificatesubject"=>nil, "certificatethumbprint"=>"", "hostname"=>"", "ipaddress"=>"*", "port"=>80, "protocol"=>"http", "s
slflags"=>"0"}] (index 0 entry 'certificatestorename' expects an undef value or a match for Enum['My', 'WebHosting'], got '')

The only way we can access the resource type properties is within the dsc_base_provider.rb, there is a similiar method implemented here which could be tweaked with some regex to fit the use case here.

[Clebam]

I see. Not at work today but will give a shot tomorrow. At first glance the enum_values function I added might be useful here to know if '' (empty string) is among the valid values of the enum. Though I have to be careful to not override undefined values.

Enum['something', 'else', ''] myParam

myParam = '' ==> should be empty string
myParam (undefined) ==> should be nil

Will keep you updated tomorrow

[Clebam]

@jordanbreen28 Seems to be working 😄
Please check if I did not alter the behavior you intended first. (I focused only on values that are supplied by in name_hash in order to avoid setting '' value instead of leaving the value unmanaged (for instance if for some reason it was set manually or another way)

[jordanbreen28]

@Clebam amazing! Yes, like you said, let me spin up an environment and to just reassure this doesn't break the existing logic :) Looking good though!

[jordanbreen28]

LGTM. Thanks for your work on this @Clebam