Merge pull request #124 from michaeltlombardi/gh-145/main/insync

(GH-145) Add insync? and invoke_test_method to dsc provider

lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb

@@ -13,10 +13,15 @@ class Puppet::Provider::DscBaseProvider
   def initialize
     @@cached_canonicalized_resource ||= []
     @@cached_query_results ||= []
+    @@cached_test_results ||= []
     @@logon_failures ||= []
     super
   end
 
+  def cached_test_results
+    @@cached_test_results
+  end
+
   # Look through a cache to retrieve the hashes specified, if they have been cached.
   # Does so by seeing if each of the specified hashes is a subset of any of the hashes
   # in the cache, so {foo: 1, bar: 2} would return if {foo: 1} was the search hash.
@@ -270,6 +275,24 @@ def invoke_dsc_resource(context, name_hash, props, method)
     data
   end
 
+  # Determine if the DSC Resource is in the desired state, invoking the `Test` method unless it's
+  #   already been run for the resource, in which case reuse the result instead of checking for each
+  #   property. This behavior is only triggered if the validation_mode is set to resource; by default
+  #   it is set to property and uses the default property comparison logic in Puppet::Property.
+  #
+  # @param context [Object] the Puppet runtime context to operate in and send feedback to
+  # @param name [String] the name of the resource being tested
+  # @param is_hash [Hash] the current state of the resource on the system
+  # @param should_hash [Hash] the desired state of the resource per the manifest
+  # @return [Boolean, Void] returns true/false if the resource is/isn't in the desired state and
+  #   the validation mode is set to resource, otherwise nil.
+  def insync?(context, name, _property_name, _is_hash, should_hash)
+    return nil if should_hash[:validation_mode] != 'resource'
+
+    prior_result = fetch_cached_hashes(@@cached_test_results, [name])
+    prior_result.empty? ? invoke_test_method(context, name, should_hash) : prior_result.first[:in_desired_state]
+  end
+
   # Invokes the `Get` method, passing the name_hash as the properties to use with `Invoke-DscResource`
   # The PowerShell script returns a JSON representation of the DSC Resource's CIM Instance munged as
   # best it can be for Ruby. Once that JSON is parsed into a hash this method further munges it to
@@ -357,6 +380,31 @@ def invoke_set_method(context, name, should)
     # notify_reboot_pending if data['rebootrequired'] == true
   end
 
+  # Invokes the `Test` method, passing the should hash as the properties to use with `Invoke-DscResource`
+  #   The PowerShell script returns a JSON hash with key-value pairs indicating whether or not the resource
+  #   is in the desired state and any error messages captured.
+  #
+  # @param context [Object] the Puppet runtime context to operate in and send feedback to
+  # @param should [Hash] the desired state represented definition to pass as properties to Invoke-DscResource
+  # @return [Boolean] returns true if the resource is in the desired state, otherwise false
+  def invoke_test_method(context, name, should)
+    context.debug("Relying on DSC Test method for validating if '#{name}' is in the desired state")
+    context.debug("Invoking Test Method for '#{name}' with #{should.inspect}")
+
+    test_props = should.select { |k, _v| k.to_s =~ /^dsc_/ }
+    data = invoke_dsc_resource(context, name, test_props, 'test')
+    # Something went wrong with Invoke-DscResource; fall back on property state comparisons
+    return nil if data.nil?
+
+    in_desired_state = data['indesiredstate']
+    @@cached_test_results << name.merge({ in_desired_state: in_desired_state })
+
+    return in_desired_state if in_desired_state
+
+    change_log = 'DSC reported that this resource is not in the desired state; treating all properties as out-of-sync'
+    [in_desired_state, change_log]
+  end
+
   # Converts a Puppet resource hash into a hash with the information needed to call Invoke-DscResource,
   # including the desired state, the path to the PowerShell module containing the resources, the invoke
   # method, and metadata about the DSC Resource and Puppet Type.

spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb

@@ -3,20 +3,21 @@
 require 'spec_helper'
 require 'puppet/type'
 require 'puppet/provider/dsc_base_provider/dsc_base_provider'
+require 'json'
 
 RSpec.describe Puppet::Provider::DscBaseProvider do
   subject(:provider) { described_class.new }
 
   let(:context) { instance_double('Puppet::ResourceApi::PuppetContext') }
   let(:type) { instance_double('Puppet::ResourceApi::TypeDefinition') }
   let(:ps_manager) { instance_double('Pwsh::Manager') }
-  let(:command) { 'command' }
   let(:execute_response) { { stdout: nil, stderr: nil, exitcode: 0 } }
 
   # Reset the caches after each run
   after(:each) do
     described_class.class_variable_set(:@@cached_canonicalized_resource, nil) # rubocop:disable Style/ClassVars
     described_class.class_variable_set(:@@cached_query_results, nil) # rubocop:disable Style/ClassVars
+    described_class.class_variable_set(:@@cached_test_results, nil) # rubocop:disable Style/ClassVars
     described_class.class_variable_set(:@@logon_failures, nil) # rubocop:disable Style/ClassVars
   end
 
@@ -32,11 +33,23 @@
     it 'initializes the cached_query_results class variable' do
       expect(described_class.class_variable_get(:@@cached_query_results)).to eq([])
     end
+    it 'initializes the cached_test_results class variable' do
+      expect(described_class.class_variable_get(:@@cached_test_results)).to eq([])
+    end
     it 'initializes the logon_failures class variable' do
       expect(described_class.class_variable_get(:@@logon_failures)).to eq([])
     end
   end
 
+  context '.cached_test_results' do
+    let(:cache_value) { %w[foo bar] }
+
+    it 'returns the value of the @@cached_test_results class variable' do
+      described_class.class_variable_set(:@@cached_test_results, cache_value) # rubocop:disable Style/ClassVars
+      expect(provider.cached_test_results).to eq(cache_value)
+    end
+  end
+
   context '.fetch_cached_hashes' do
     let(:cached_hashes) { [{ foo: 1, bar: 2, baz: 3 }, { foo: 4, bar: 5, baz: 6 }] }
     let(:findable_full_hash) { { foo: 1, bar: 2, baz: 3 } }
@@ -354,6 +367,35 @@
     end
   end
 
+  context '.insync?' do
+    let(:name)               { { name: 'foo' } }
+    let(:attribute_name)     { :foo }
+    let(:is_hash)            { { name: 'foo', foo: 1 } }
+    let(:cached_test_result) { [{ name: 'foo', in_desired_state: true }] }
+    let(:should_hash_validate_by_property) { { name: 'foo', foo: 1, validation_mode: 'property' } }
+    let(:should_hash_validate_by_resource) { { name: 'foo', foo: 1, validation_mode: 'resource' } }
+
+    context 'when the validation_mode is "resource"' do
+      it 'calls invoke_test_method if the result of a test is not already cached' do
+        expect(provider).to receive(:fetch_cached_hashes).and_return([])
+        expect(provider).to receive(:invoke_test_method).and_return(true)
+        expect(provider.send(:insync?, context, name, attribute_name, is_hash, should_hash_validate_by_resource)).to be true
+      end
+      it 'does not call invoke_test_method if the result of a test is already cached' do
+        expect(provider).to receive(:fetch_cached_hashes).and_return(cached_test_result)
+        expect(provider).not_to receive(:invoke_test_method)
+        expect(provider.send(:insync?, context, name, attribute_name, is_hash, should_hash_validate_by_resource)).to be true
+      end
+    end
+    context 'when the validation_mode is "property"' do
+      it 'does not call invoke_test_method and returns nil' do
+        expect(provider).not_to receive(:fetch_cached_hashes)
+        expect(provider).not_to receive(:invoke_test_method)
+        expect(provider.send(:insync?, context, name, attribute_name, is_hash, should_hash_validate_by_property)).to be nil
+      end
+    end
+  end
+
   context '.invoke_get_method' do
     subject(:result) { provider.invoke_get_method(context, name_hash) }
 
@@ -908,6 +950,56 @@
     end
   end
 
+  context '.invoke_test_method' do
+    subject(:result) { provider.invoke_test_method(context, name, should) }
+
+    let(:name) { { name: 'foo', dsc_name: 'bar' } }
+    let(:should) { name.merge(dsc_ensure: 'present') }
+    let(:test_properties) { should.reject { |k, _v| k == :name } }
+    let(:invoke_dsc_resource_data) { nil }
+
+    before(:each) do
+      allow(context).to receive(:notice)
+      allow(context).to receive(:debug)
+      allow(provider).to receive(:invoke_dsc_resource).with(context, name, test_properties, 'test').and_return(invoke_dsc_resource_data)
+    end
+
+    after(:each) do
+      described_class.class_variable_set(:@@cached_test_results, []) # rubocop:disable Style/ClassVars
+    end
+
+    context 'when something went wrong calling Invoke-DscResource' do
+      it 'falls back on property-by-property state comparison and does not cache anything' do
+        expect(context).not_to receive(:err)
+        expect(result).to be(nil)
+        expect(provider.cached_test_results).to eq([])
+      end
+    end
+
+    context 'when the DSC Resource is in the desired state' do
+      let(:invoke_dsc_resource_data) { { 'indesiredstate' => true, 'errormessage' => '' } }
+
+      it 'returns true and caches the result' do
+        expect(context).not_to receive(:err)
+        expect(result).to eq(true)
+        expect(provider.cached_test_results).to eq([name.merge(in_desired_state: true)])
+      end
+    end
+
+    context 'when the DSC Resource is not in the desired state' do
+      let(:invoke_dsc_resource_data) { { 'indesiredstate' => false, 'errormessage' => '' } }
+
+      it 'returns false and caches the result' do
+        expect(context).not_to receive(:err)
+        # Resource is not in the desired state
+        expect(result.first).to eq(false)
+        # Custom out-of-sync message passed
+        expect(result.last).to match(/not in the desired state/)
+        expect(provider.cached_test_results).to eq([name.merge(in_desired_state: false)])
+      end
+    end
+  end
+
   context '.random_variable_name' do
     it 'creates random variables' do
       expect(provider.random_variable_name).not_to be_nil