(PDK-895) basic array support

README.md

@@ -191,11 +191,12 @@ This gem is still under heavy development. This section is a living document of
 Currently working:
 * Basic type and provider definition, using `name`, `desc`, and `attributes`.
 * Scalar puppet 4 [data types](https://puppet.com/docs/puppet/5.3/lang_data_type.html#core-data-types):
-  * String
+  * String, Enum, Pattern
   * Integer, Float, Numeric
   * Boolean
-  * Enum[absent, present]
-  * simple Optionals
+  * Array
+  * Optional
+  * Variant
 * The `canonicalize`, `simple_get_filter`, and `remote_resource` features.
 * All the logging facilities.
 * Executing the new provider under the following commands:
@@ -205,7 +206,7 @@ Currently working:
   * `puppet device` (if applicable)
 
 There are still a few notable gaps between the implementation and the specification:
-* Complex data types, like Array, Hash, Tuple or Struct are not yet implemented.
+* Complex data types, like Hash, Tuple or Struct are not yet implemented.
 * Only a single runtime environment (the Puppet commands) is currently implemented.
 
 Restrictions of running under puppet:

appveyor.yml

@@ -9,17 +9,6 @@ install:
   - set PATH=C:\Ruby%RUBY_VERSION%\bin;C:\mingw-w64\x86_64-6.3.0-posix-seh-rt_v5-rev1\mingw64\bin;%PATH%
   - SET LOG_SPEC_ORDER=true
   - SET COVERAGE=yes
-  # Due to a bug in the version of OpenSSL shipped with Ruby 2.4.1 on Windows
-  # (https://bugs.ruby-lang.org/issues/11033). Errors are ignored because the
-  # mingw gem calls out to pacman to install OpenSSL which is already
-  # installed, causing gem to raise a warning that powershell determines to be
-  # a fatal error.
-  - ps: |
-      $ErrorActionPreference = "SilentlyContinue"
-      if($env:RUBY_VERSION -eq "24-x64") {
-        gem install openssl "~> 2.0.4" --no-rdoc --no-ri -- --with-openssl-dir=C:\msys64\mingw64
-      }
-      $host.SetShouldExit(0)
   - bundle install --jobs 4 --retry 2 --without development
 
 build: off

lib/puppet/resource_api.rb

@@ -145,76 +145,66 @@ def feature_support?(feature)
           end
 
           type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type])
-          validate do |value|
-            if options[:behaviour] == :read_only
-              raise Puppet::ResourceError, "Attempting to set `#{name}` read_only attribute value to `#{value}`"
+          if param_or_property == :newproperty
+            define_method(:should) do
+              @should ? @should.first : @should
             end
 
-            return true if type.instance?(value)
-
-            if value.is_a? String
-              # when running under `puppet resource`, we need to try to coerce from strings to the real type
-              case value
-              when %r{^-?\d+$}, Puppet::Pops::Patterns::NUMERIC
-                value = Puppet::Pops::Utils.to_n(value)
-              when %r{\Atrue|false\Z}
-                value = value == 'true'
+            define_method(:should=) do |value|
+              @shouldorig = value
+              # Puppet requires the @should value to always be stored as an array. We do not use this
+              # for anything else
+              # @see Puppet::Property.should=(value)
+              @should = [Puppet::ResourceApi.mungify(type, value, "#{definition[:name]}.#{name}")]
+            end
+          else
+            define_method(:value=) do |value|
+              if options[:behaviour] == :read_only
+                raise Puppet::ResourceError, "Attempting to set `#{name}` read_only attribute value to `#{value}`"
               end
-              return true if type.instance?(value)
 
-              inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
-              error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("#{definition[:name]}.#{name}", type, inferred_type)
-              raise Puppet::ResourceError, error_msg
+              @value = Puppet::ResourceApi.mungify(type, value, "#{definition[:name]}.#{name}")
             end
           end
 
           if type.instance_of? Puppet::Pops::Types::POptionalType
             type = type.type
           end
 
-          # provide better handling of the standard types
+          # puppet symbolizes some values through puppet/paramter/value.rb (see .convert()), but (especially) Enums
+          # are strings. specifying a munge block here skips the value_collection fallback in puppet/parameter.rb's
+          # default .unsafe_munge() implementation.
+          munge { |v| v }
+
+          # provide hints to `puppet type generate` for better parsing
           case type
           when Puppet::Pops::Types::PStringType
             # require any string value
             Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{})
-          # rubocop:disable Lint/BooleanSymbol
           when Puppet::Pops::Types::PBooleanType
             Puppet::ResourceApi.def_newvalues(self, param_or_property, 'true', 'false')
+            # rubocop:disable Lint/BooleanSymbol
             aliasvalue true, 'true'
             aliasvalue false, 'false'
             aliasvalue :true, 'true'
             aliasvalue :false, 'false'
-
-            munge do |v|
-              case v
-              when 'true', :true
-                true
-              when 'false', :false
-                false
-              else
-                v
-              end
-            end
-          # rubocop:enable Lint/BooleanSymbol
+            # rubocop:enable Lint/BooleanSymbol
           when Puppet::Pops::Types::PIntegerType
             Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{^-?\d+$})
-            munge do |v|
-              Puppet::Pops::Utils.to_n(v)
-            end
           when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
             Puppet::ResourceApi.def_newvalues(self, param_or_property, Puppet::Pops::Patterns::NUMERIC)
-            munge do |v|
-              Puppet::Pops::Utils.to_n(v)
-            end
+          end
+
+          if param_or_property == :newproperty
+            # stop puppet from trying to call into the provider when
+            # no pre-defined values have been specified
+            # "This is not the provider you are looking for." -- Obi-Wan Kaniesobi.
+            def call_provider(value); end
           end
 
           case options[:type]
           when 'Enum[present, absent]'
             Puppet::ResourceApi.def_newvalues(self, param_or_property, 'absent', 'present')
-            # puppet symbolizes these values through puppet/paramter/value.rb (see .convert()), but Enums are strings
-            # specifying a munge block here skips the value_collection fallback in puppet/parameter.rb's
-            # default .unsafe_munge() implementation
-            munge { |v| v }
           end
         end
       end
@@ -385,4 +375,87 @@ def self.def_newvalues(this, param_or_property, *values)
       end
     end
   end
+
+  # Validates and munges values coming from puppet into a shape palatable to the provider.
+  # This includes parsing values from strings, e.g. when running in `puppet resource`.
+  # @param type[Puppet::Pops::Types::TypedModelObject] the type to check/clean against
+  # @param value the value to clean
+  # @param error_msg_prefix[String] a prefix for the error messages
+  # @return [type] the cleaned value
+  # @raise [Puppet::ResourceError] if `value` could not be parsed into `type`
+  def self.mungify(type, value, error_msg_prefix)
+    cleaned_value, error = try_mungify(type, value, error_msg_prefix)
+
+    raise Puppet::ResourceError, error if error
+
+    cleaned_value
+  end
+
+  # Recursive implementation part of #mungify. Uses a multi-valued return value to avoid excessive
+  #   exception throwing for regular usage
+  # @return [Array] if the mungify worked, the first element is the cleaned value, and the second
+  #   element is nil. If the mungify failed, the first element is nil, and the second element is an error
+  #   message
+  # @private
+  def self.try_mungify(type, value, error_msg_prefix)
+    case type
+    when Puppet::Pops::Types::PArrayType
+      if value.is_a? Array
+        conversions = value.map do |v|
+          try_mungify(type.element_type, v, error_msg_prefix)
+        end
+        # only convert the values if none failed. otherwise fall through and rely on puppet to render a proper error
+        if conversions.all? { |c| c[1].nil? }
+          value = conversions.map { |c| c[0] }
+        end
+      end
+    when Puppet::Pops::Types::PBooleanType
+      value = case value
+              when 'true', :true # rubocop:disable Lint/BooleanSymbol
+                true
+              when 'false', :false # rubocop:disable Lint/BooleanSymbol
+                false
+              else
+                value
+              end
+    when Puppet::Pops::Types::PIntegerType, Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
+      if value =~ %r{^-?\d+$} || value =~ Puppet::Pops::Patterns::NUMERIC
+        value = Puppet::Pops::Utils.to_n(value)
+      end
+    when Puppet::Pops::Types::PEnumType, Puppet::Pops::Types::PStringType, Puppet::Pops::Types::PPatternType
+      if value.is_a? Symbol
+        value = value.to_s
+      end
+    when Puppet::Pops::Types::POptionalType
+      return value.nil? ? [nil, nil] : try_mungify(type.type, value, error_msg_prefix)
+    when Puppet::Pops::Types::PVariantType
+      # try converting to anything except string first
+      string_type = type.types.find { |t| t.is_a? Puppet::Pops::Types::PStringType }
+      conversion_results = (type.types - [string_type]).map do |t|
+        try_mungify(t, value, error_msg_prefix)
+      end
+
+      # only consider valid results
+      conversion_results = conversion_results.select { |r| r[1].nil? }.to_a
+
+      # use the conversion result if unambiguous
+      return conversion_results[0] if conversion_results.length == 1
+
+      # return an error if ambiguous
+      return [nil, "#{error_msg_prefix} #{value.inspect} is not unabiguously convertable to #{type}"] if conversion_results.length > 1
+
+      # try to interpret as string
+      return try_mungify(string_type, value, error_msg_prefix) if string_type
+
+      # fall through to default handling
+    end
+
+    # a match!
+    return [value, nil] if type.instance?(value)
+
+    # an error :-(
+    inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
+    error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch(error_msg_prefix, type, inferred_type)
+    return [nil, error_msg] # the entire function is using returns for clarity # rubocop:disable Style/RedundantReturn
+  end
 end

spec/acceptance/array_spec.rb

@@ -0,0 +1,14 @@
+require 'spec_helper'
+require 'tempfile'
+
+RSpec.describe 'a provider using arrays' do
+  let(:common_args) { '--verbose --trace --strict=error --modulepath spec/fixtures' }
+
+  describe 'using `puppet apply`' do
+    it 'applies a catalog successfully' do
+      stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_array { foo: some_array => [a, c, b] }\"")
+      expect(stdout_str).to match %r{Updating 'foo' with \{:name=>"foo", :some_array=>\["a", "c", "b"\], :ensure=>"present"\}}
+      expect(stdout_str).not_to match %r{Error:}
+    end
+  end
+end

spec/acceptance/device_spec.rb

@@ -104,7 +104,7 @@
     context 'with a device resource in the catalog' do
       it 'applies the catalog successfully' do
         stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path} --apply 'device_provider{\"foo\": "\
-                                              'ensure => "present", boolean => true, integer => 15, float => 1.23, variant_pattern => 0xA245EED, '\
+                                              'ensure => "present", boolean => true, integer => 15, float => 1.23, variant_pattern => "0x1234ABCD", '\
                                               'url => "http://www.google.com", boolean_param => false, integer_param => 99, float_param => 3.21, '\
                                               "ensure_param => \"present\", variant_pattern_param => \"9A2222ED\", url_param => \"http://www.puppet.com\"}'")
         expect(stdout_str).not_to match %r{Error:}

spec/fixtures/test_module/lib/puppet/provider/test_array/test_array.rb

@@ -0,0 +1,32 @@
+require 'puppet/resource_api'
+require 'puppet/resource_api/simple_provider'
+
+# Implementation for the test_array type using the Resource API.
+class Puppet::Provider::TestArray::TestArray < Puppet::ResourceApi::SimpleProvider
+  def get(_context)
+    [
+      {
+        name: 'foo',
+        ensure: 'present',
+        some_array: %w[a b c],
+      },
+      {
+        name: 'bar',
+        ensure: 'present',
+        some_array: [],
+      },
+    ]
+  end
+
+  def create(context, name, should)
+    context.notice("Creating '#{name}' with #{should.inspect}")
+  end
+
+  def update(context, name, should)
+    context.notice("Updating '#{name}' with #{should.inspect}")
+  end
+
+  def delete(context, name)
+    context.notice("Deleting '#{name}'")
+  end
+end

spec/fixtures/test_module/lib/puppet/type/test_array.rb

@@ -0,0 +1,25 @@
+require 'puppet/resource_api'
+
+Puppet::ResourceApi.register_type(
+  name: 'test_array',
+  docs: <<-EOS,
+      This type provides Puppet with the capabilities to manage ...
+    EOS
+  features: [],
+  attributes:   {
+    ensure:      {
+      type:    'Enum[present, absent]',
+      desc:    'Whether this resource should be present or absent on the target system.',
+      default: 'present',
+    },
+    name:        {
+      type:      'String',
+      desc:      'The name of the resource you want to manage.',
+      behaviour: :namevar,
+    },
+    some_array:  {
+      type:      'Array[String]',
+      desc:      'An array aiding array attestation',
+    }
+  },
+)

spec/fixtures/test_module/spec/unit/puppet/provider/test_array/test_array_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+# TODO: needs some cleanup/helper to avoid this misery
+module Puppet::Provider::TestArray; end
+require 'puppet/provider/test_array/test_array'
+
+RSpec.describe Puppet::Provider::TestArray::TestArray do
+  subject(:provider) { described_class.new }
+
+  let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') }
+
+  describe '#get' do
+    it 'processes resources' do
+      expect(provider.get(context)).to eq [
+        {
+          name: 'foo',
+          ensure: :present,
+        },
+        {
+          name: 'bar',
+          ensure: :present,
+        },
+      ]
+    end
+  end
+
+  describe 'create(context, name, should)' do
+    it 'creates the resource' do
+      expect(context).to receive(:notice).with(%r{\ACreating 'a'})
+
+      provider.create(context, 'a', name: 'a', ensure: 'present')
+    end
+  end
+
+  describe 'update(context, name, should)' do
+    it 'updates the resource' do
+      expect(context).to receive(:notice).with(%r{\AUpdating 'foo'})
+
+      provider.update(context, 'foo', name: 'foo', ensure: 'present')
+    end
+  end
+
+  describe 'delete(context, name, should)' do
+    it 'deletes the resource' do
+      expect(context).to receive(:notice).with(%r{\ADeleting 'foo'})
+
+      provider.delete(context, 'foo')
+    end
+  end
+end

spec/fixtures/test_module/spec/unit/puppet/type/test_array_spec.rb

@@ -0,0 +1,8 @@
+require 'spec_helper'
+require 'puppet/type/test_array'
+
+RSpec.describe 'the test_array type' do
+  it 'loads' do
+    expect(Puppet::Type.type(:test_array)).not_to be_nil
+  end
+end