(MODULES-7253) Replace verbose parameter names

[michaeltlombardi]

Prior to this commit the dsc resource used the following long,
slightly repetitive names for the parameters:

• dsc_resource_name
• dsc_resource_module
• dsc_resource_properties

This commit replaces those names with the following:

• resource_name
• module
• properties

[michaeltlombardi]

So, I went to go and alias the values backwards... and discovered this isn't actually a capability inside of puppet as far as I can see.

Advice I got from the modules team was to consider following the pattern from the firewall resource:

https://github.com/puppetlabs/puppetlabs-firewall/blob/9a17911503d0b93016349a086842a03a6256f795/lib/puppet/type/firewall.rb#L267-L331

  newproperty(:dport, array_matching: :all) do
    desc <<-PUPPETCODE
      The destination port to match for this filter (if the protocol supports
      ports). Will accept a single element or an array.


      For some firewall providers you can pass a range of ports in the format:


          <start_number>-<ending_number>


      For example:


          1-1024


      This would cover ports 1 to 1024.
    PUPPETCODE


    munge do |value|
      @resource.string_to_port(value, :proto)
    end


    def to_s?(value)
      should_to_s(value)
    end


    def should_to_s(value)
      value = [value] unless value.is_a?(Array)
      value.join(',')
    end
  end


  newproperty(:port, array_matching: :all) do
    desc <<-PUPPETCODE
      DEPRECATED


      The destination or source port to match for this filter (if the protocol
      supports ports). Will accept a single element or an array.


      For some firewall providers you can pass a range of ports in the format:


          <start_number>-<ending_number>


      For example:


          1-1024


      This would cover ports 1 to 1024.
    PUPPETCODE


    validate do |_value|
      Puppet.warning('Passing port to firewall is deprecated and will be removed. Use dport and/or sport instead.')
    end


    munge do |value|
      @resource.string_to_port(value, :proto)
    end


    def to_s?(value)
      should_to_s(value)
    end


    def should_to_s(value)
      value = [value] unless value.is_a?(Array)
      value.join(',')
    end
  end

Notice that here they copied the original parameter over to the new name. The old parameter has a validate block that raises a deprecation warning. I can implement this if this is the direction we want to take.

[jpogran]

Prefer renamed rather than replaced.

[jpogran]

This format breaks what we have been doing so far with the CHANGELOG. As you have it now, its <Severity>: Message <Ticket link> Another Message. Up until now we had <Severity>: Short Message <Ticket link>.

I think this information is better served in the ticket and this line kept short:

BREAKING: Renamed all parameters for resource declarations, will require manifest modifications. (MODULES-7253)

[michaeltlombardi]

In that case, we should link to the section of the docs that includes reference for these parameters.

[michaeltlombardi]

On the other hand, I don't actually know if that link will be available on the forge. I suppose we'll just have to trust users to change tabs on their own. :)

Updated based on feedback

[Iristyle]

I don't think we have to address in this PR, but what about the nested dsc_type and dsc_properties for embedded CimInstance hashes? I'm less concerned about those as they should be "rarer"... and we probably do need some "namespacing" to avoid collisions. But wondering if this has been discussed?

[michaeltlombardi]

@Iristyle ah, crap, no, that topic didn't come up at all in this ticket until now.

[michaeltlombardi]

After talking it over with @glennsarti, filed MODULES-7352 for further discussion and implementation.

[jpogran]

I didn't suggest those to change because they need to be unique enough to find in the hash. They can't be something 'normal' as it could conflict with parameters used by DSC Resources.