(PDK-919) Workaround PUP-2368 "using booleans result in unmanaged pro…

lib/puppet/resource_api.rb

@@ -147,7 +147,12 @@ def feature_support?(feature)
           type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type])
           if param_or_property == :newproperty
             define_method(:should) do
-              @should ? @should.first : @should
+              if type.is_a? Puppet::Pops::Types::PBooleanType
+                # work around https://tickets.puppetlabs.com/browse/PUP-2368
+                rs_value ? :true : :false # rubocop:disable Lint/BooleanSymbol
+              else
+                rs_value
+              end
             end
 
             define_method(:should=) do |value|
@@ -157,21 +162,37 @@ def feature_support?(feature)
               # @see Puppet::Property.should=(value)
               @should = [Puppet::ResourceApi.mungify(type, value, "#{definition[:name]}.#{name}")]
             end
+
+            # used internally
+            # @returns the final mungified value of this property
+            define_method(:rs_value) do
+              @should ? @should.first : @should
+            end
           else
+            define_method(:value) do
+              @value
+            end
+
             define_method(:value=) do |value|
               if options[:behaviour] == :read_only
                 raise Puppet::ResourceError, "Attempting to set `#{name}` read_only attribute value to `#{value}`"
               end
 
               @value = Puppet::ResourceApi.mungify(type, value, "#{definition[:name]}.#{name}")
             end
+
+            # used internally
+            # @returns the final mungified value of this parameter
+            define_method(:rs_value) do
+              @value
+            end
           end
 
           if type.instance_of? Puppet::Pops::Types::POptionalType
             type = type.type
           end
 
-          # puppet symbolizes some values through puppet/paramter/value.rb (see .convert()), but (especially) Enums
+          # puppet symbolizes some values through puppet/parameter/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 }
@@ -256,9 +277,8 @@ def call_provider(value); end
 
       define_method(:flush) do
         # puts 'flush'
-        target_state = Hash[@parameters.map { |k, v| [k, v.value] }]
-        # remove puppet's injected metaparams
-        target_state.delete(:loglevel)
+        # skip puppet's injected metaparams
+        target_state = Hash[@parameters.reject { |k, _v| [:loglevel, :noop].include? k }.map { |k, v| [k, v.rs_value] }]
         target_state = my_provider.canonicalize(context, [target_state]).first if feature_support?('canonicalize')
 
         retrieve unless @rapi_current_state

spec/acceptance/boolean_spec.rb

@@ -0,0 +1,22 @@
+require 'spec_helper'
+require 'tempfile'
+
+RSpec.describe 'a provider using booleans' do
+  let(:common_args) { '--verbose --trace --strict=error --modulepath spec/fixtures' }
+
+  describe 'using `puppet apply`' do
+    it 'applies a catalog with no changes' do
+      stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_bool { foo: test_bool => true, test_bool_param => true; bar: test_bool => false, test_bool_param => false }\"")
+      expect(stdout_str).not_to match %r{Updating|ensure|test_bool}i
+      expect(stdout_str).not_to match %r{Error:}
+    end
+    it 'applies a catalog with bool changes' do
+      stdout_str, _status = Open3.capture2e("puppet apply #{common_args} -e \"test_bool { foo: test_bool => false, test_bool_param => false; bar: test_bool => true, test_bool_param => true }\"")
+      expect(stdout_str).to match %r{Test_bool\[foo\]/test_bool: test_bool changed (true|'true') to 'false'}
+      expect(stdout_str).to match %r{Test_bool\[bar\]/test_bool: test_bool changed (false|'false') to 'true'}
+      expect(stdout_str).to match %r{Updating: Updating 'foo' with \{:name=>"foo", :test_bool=>false, :test_bool_param=>false, :ensure=>"present"\}}
+      expect(stdout_str).to match %r{Updating: Updating 'bar' with \{:name=>"bar", :test_bool=>true, :test_bool_param=>true, :ensure=>"present"\}}
+      expect(stdout_str).not_to match %r{Error:}
+    end
+  end
+end

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

@@ -0,0 +1,34 @@
+require 'puppet/resource_api'
+require 'puppet/resource_api/simple_provider'
+
+# Implementation for the test_bool type using the Resource API.
+class Puppet::Provider::TestBool::TestBool < Puppet::ResourceApi::SimpleProvider
+  def get(_context)
+    [
+      {
+        name: 'foo',
+        ensure: 'present',
+        test_bool: true,
+        test_bool_param: true,
+      },
+      {
+        name: 'bar',
+        ensure: 'present',
+        test_bool: false,
+        test_bool_param: false,
+      },
+    ]
+  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_bool.rb

@@ -0,0 +1,30 @@
+require 'puppet/resource_api'
+
+Puppet::ResourceApi.register_type(
+  name: 'test_bool',
+  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,
+    },
+    test_bool:   {
+      type:      'Boolean',
+      desc:      'A boolean property for testing.',
+    },
+    test_bool_param: {
+      type:      'Boolean',
+      desc:      'A boolean parameter for testing.',
+      behaviour: :parameter,
+    },
+  },
+)

spec/fixtures/test_module/spec/unit/puppet/provider/test_bool/test_bool_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+# TODO: needs some cleanup/helper to avoid this misery
+module Puppet::Provider::TestBool; end
+require 'puppet/provider/test_bool/test_bool'
+
+RSpec.describe Puppet::Provider::TestBool::TestBool 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_bool_spec.rb

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

spec/puppet/resource_api_spec.rb

@@ -236,41 +236,43 @@ def extract_values(function)
           }
         end
 
+        # rubocop:disable Lint/BooleanSymbol
         context 'when using :true' do
-          let(:the_boolean) { :true } # rubocop:disable Lint/BooleanSymbol
+          let(:the_boolean) { :true }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq true }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :true }
         end
         context 'when using :false' do
-          let(:the_boolean) { :false } # rubocop:disable Lint/BooleanSymbol
+          let(:the_boolean) { :false }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq false }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :false }
         end
         context 'when using "true"' do
           let(:the_boolean) { 'true' }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq true }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :true }
         end
         context 'when using "false"' do
           let(:the_boolean) { 'false' }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq false }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :false }
         end
         context 'when using true' do
           let(:the_boolean) { true }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq true }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :true }
         end
         context 'when using false' do
           let(:the_boolean) { false }
 
-          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq false }
+          it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :false }
         end
         context 'when using an unparsable value' do
           let(:the_boolean) { 'flubb' }
 
           it('the test_boolean value is set correctly') { expect { instance }.to raise_error Puppet::ResourceError, %r{test_boolean expect.* Boolean .* got String} }
         end
+        # rubocop:enable Lint/BooleanSymbol
       end
     end
   end