(GH-127) Canonicalize enums correctly

[michaeltlombardi]

Prior to this PR, some enums would be incorrectly canonicalized to the value returned from invoke_get_method; while this is fine for most strings, enums are case sensitive and validated in Puppet. For example, when the result of invoke_get_method returns a value of DWord for an enum of Dword, an error was raised when attempting to canonicalize to the incorrect casing of DWord.

This PR:

1. Adds a new enum_attributes method which, like the namevar_attributes and parameter_attributes methods, filters the type attributes to return the list of attribute names (as keys); in this case, for any attribute whose data type includes an Enum.
2. Ensures that enums are not erroneously cased to match system state.
3. Includes new unit tests for the canonicalize behavior as well as the new enum_attributes method.

• Resolves DSC Base Provider: Default to correct enum casing during canonicalization #127

[michaeltlombardi]

This PR depends on #129 and #130 and should be rebased after they are merged.

[Clebam]

Hi @michaeltlombardi , I'm facing an issue and it seems to be related to this PR.

I already add a discussion about this on slack (here : https://puppetcommunity.slack.com/archives/CFD8Z9A4T/p1696342031154069 )

I've had some time to dig into it further and here what I can see.

Here is the manifest

  1 class sw_iis {
  2   dsc_webapppool { 'DefaultAppPool':
  3     dsc_ensure                => 'Present',
  4     dsc_name                  => 'DefaultAppPool',
  5     dsc_managedruntimeversion => '',
  6     dsc_state                 => stopped,
  7     tag                       => 'pouet',
  8   }
  9 
 10 }

Let's focus on dsc_state => stopped
With the autogenerated resource WebAdministrationDsc, we have this file ../webadministrationdsc/lib/puppet/type/dsc_webapppool.rb
And the declaration of the parameter dsc_state

dsc_state: {
      type: "Optional[Enum['Started', 'started', 'Stopped', 'stopped']]",
      desc: 'Indicates the state of the application pool. The values that are allowed for this property are: Started, Stopped.',

      mandatory_for_get: false,
      mandatory_for_set: false,
      mof_type: 'String',
      mof_is_embedded: false,
    },

It is an Enum that accepts both PascalCased and downcased values.

In my manifest the value stopped is downcased.
When canonicalized function is called, I output the values at different time with puts
Here is an extract of the updated code of dsc_base_provider.rb where I added some output

  84             downcased_result.each do |key, value|
  85               puts "### Current Key = " + key.to_s + " ###"
  86               puts "    Current Value = " + value.to_s
  87               puts "    Unchanged canonicalized[key] = " + canonicalized[key].to_s
  88               puts "    r[key] = " + r[key].to_s
  89               puts "    downcased_resource[key] = " + downcased_resource[key].to_s
  90               puts "    same as downcased_resource? = " + same?(value, downcased_resource[key]).to_s
  91               puts "    not an enum? = " + (!enum_attributes(context).include?(key)).to_s
  92               # Canonicalize to the manifest value unless the downcased strings match and the attribute is not an enum:
  93               # - When the values don't match at all, the manifest value is desired;
  94               # - When the values match case insensitively but the attribute is an enum, prefer the casing of the manifest enum.
  95               # - When the values match case insensitively and the attribute is not an enum, prefer the casing from invoke_get_method
  96               canonicalized[key] = r[key] unless same?(value, downcased_resource[key]) && !enum_attributes(context).include?(key)
  97               canonicalized.delete(key) unless downcased_resource.key?(key)
  98               puts "    Updated Canonicalized[key] = " + canonicalized[key].to_s
  99             end

and also

110       puts "canonicalized resource = " + canonicalized.to_s

And here is the output concerning the parameter dsc_state

### Current Key = dsc_state ###
    Current Value = stopped
    Unchanged canonicalized[key] = Stopped
    r[key] = stopped
    downcased_resource[key] = stopped
    same as downcased_resource? = true
    not an enum? = false
    Updated Canonicalized[key] = stopped
[...]
canonicalized resource = {:dsc_ensure=>"Present", :dsc_managedruntimeversion=>"", :dsc_name=>"DefaultAppPool", :dsc_state=>"stopped", :name=>"DefaultAppPool", :tag=>"pouet"}

We can see that the value that is going to be compared is downcased instead using the PascalCased that is actually set.

It ends up trying to apply the resource on each run.

Notice: /Stage[main]/Sw_iis/Dsc_webapppool[DefaultAppPool]/dsc_state: dsc_state changed 'Stopped' to 'stopped' (corrective)
Notice: dsc_webapppool[{:name=>"DefaultAppPool", :dsc_name=>"DefaultAppPool"}]: Updating: Finished in 0.834560 seconds

If I comment out the call to enum_attributes, it works fine.

(I should also mention that the comparison with the empty_string dsc_managedruntimeversion=>"" also fails. It reads 'nil' but the Enum required and empty string

Notice: /Stage[main]/Sw_iis/Dsc_webapppool[DefaultAppPool]/dsc_managedruntimeversion: dsc_managedruntimeversion changed  to '' (corrective)

)

I tried a quick fix:

  [...]
  95               is_enum_type = enum_attributes(context).include?(key)
  96               if is_enum_type
  97                 cononicalized_value_is_valid_enum = is_valid_enum(context, key, canonicalized[key])
  98               else
  99                 canonicalized_value_is_valid_enum = false
 100               end
 101               canonicalized[key] = r[key] unless same?(value, downcased_resource[key]) && !canonicalized_value_is_valid_enum
 102               canonicalized.delete(key) unless downcased_resource.key?(key)
 [...]
 654   def enum_attributes(context)
 655     context.type.attributes.select { |_name, properties| properties[:type].include?('Enum[') }.keys
 656   end
 657 
 658   def is_valid_enum(context, key, value)
 659     enums = context.type.attributes.select { |_name, properties| properties[:type].include?('Enum[') }
 660     valid_values = enums[key][:type].scan(/Enum\[(.*?)\]/).first.first.split(',').map { |val| val.strip.delete("'") }
 661     valid_values.include?(value)
 662   end

It definitely should be improved but it solves the problem I described and also solves I think the former issue that this PR fixed.

Hopefully you'll give me some feedback :)

Thanks