From fa75d6c3117c2343a5f78fa9bb73363371208fbd Mon Sep 17 00:00:00 2001 From: jordanbreen28 Date: Wed, 16 Aug 2023 16:35:30 +0100 Subject: [PATCH] (bug) - Fixes missing mandatory ID --- .../dsc_base_provider/dsc_base_provider.rb | 48 +++++----- .../dsc_base_provider_spec.rb | 87 +++++++++---------- 2 files changed, 66 insertions(+), 69 deletions(-) diff --git a/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb b/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb index 13bdb6bc..6bc42ffe 100644 --- a/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb +++ b/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb @@ -6,27 +6,25 @@ require 'json' class Puppet::Provider::DscBaseProvider - # Initializes the provider, preparing the class variables which cache: + # Initializes the provider, preparing the instance variables which cache: # - the canonicalized resources across calls # - query results # - logon failures def initialize - @@cached_canonicalized_resource ||= [] - @@cached_query_results ||= [] - @@cached_test_results ||= [] - @@logon_failures ||= [] + @cached_canonicalized_resource = [] + @cached_query_results = [] + @cached_test_results = [] + @logon_failures = [] super end - def cached_test_results - @@cached_test_results - end + attr_reader :cached_test_results # 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. # - # @param cache [Array] the class variable containing cached hashes to search through + # @param cache [Array] the instance variable containing cached hashes to search through # @param hashes [Array] the list of hashes to search the cache for # @return [Array] an array containing the matching hashes for the search condition, if any def fetch_cached_hashes(cache, hashes) @@ -52,14 +50,14 @@ def canonicalize(context, resources) # During RSAPI refresh runs mandatory parameters are stripped and not available; # Instead of checking again and failing, search the cache for a namevar match. namevarized_r = r.select { |k, _v| namevar_attributes(context).include?(k) } - cached_result = fetch_cached_hashes(@@cached_canonicalized_resource, [namevarized_r]).first + cached_result = fetch_cached_hashes(@cached_canonicalized_resource, [namevarized_r]).first if cached_result.nil? # If the resource is meant to be absent, skip canonicalization and rely on the manifest # value; there's no reason to compare system state to desired state for casing if the # resource is being removed. if r[:dsc_ensure] == 'absent' canonicalized = r.dup - @@cached_canonicalized_resource << r.dup + @cached_canonicalized_resource << r.dup else canonicalized = invoke_get_method(context, r) # If the resource could not be found or was returned as absent, skip case munging and @@ -67,7 +65,7 @@ def canonicalize(context, resources) # rubocop:disable Metrics/BlockNesting if canonicalized.nil? || canonicalized[:dsc_ensure] == 'absent' canonicalized = r.dup - @@cached_canonicalized_resource << r.dup + @cached_canonicalized_resource << r.dup else parameters = r.select { |name, _properties| parameter_attributes(context).include?(name) } canonicalized.merge!(parameters) @@ -91,7 +89,7 @@ def canonicalize(context, resources) canonicalized.delete(key) unless downcased_resource.key?(key) end # Cache the actually canonicalized resource separately - @@cached_canonicalized_resource << canonicalized.dup + @cached_canonicalized_resource << canonicalized.dup end # rubocop:enable Metrics/BlockNesting end @@ -123,13 +121,13 @@ def get(context, names = nil) context.debug('Collecting data from the DSC Resource') # If the resource has already been queried, do not bother querying for it again - cached_results = fetch_cached_hashes(@@cached_query_results, names) + cached_results = fetch_cached_hashes(@cached_query_results, names) return cached_results unless cached_results.empty? - if @@cached_canonicalized_resource.empty? + if @cached_canonicalized_resource.empty? mandatory_properties = {} else - canonicalized_resource = @@cached_canonicalized_resource[0].dup + canonicalized_resource = @cached_canonicalized_resource[0].dup mandatory_properties = canonicalized_resource.select do |attribute, _value| (mandatory_get_attributes(context) - namevar_attributes(context)).include?(attribute) end @@ -266,9 +264,9 @@ def invoke_dsc_resource(context, name_hash, props, method) if error.include?('Logon failure: the user has not been granted the requested logon type at this computer') logon_error = "PSDscRunAsCredential account specified (#{name_hash[:dsc_psdscrunascredential]['user']}) does not have appropriate logon rights; are they an administrator?" name_hash[:name].nil? ? context.err(logon_error) : context.err(name_hash[:name], logon_error) - @@logon_failures << name_hash[:dsc_psdscrunascredential].dup + @logon_failures << name_hash[:dsc_psdscrunascredential].dup # This is a hack to handle the query cache to prevent a second lookup - @@cached_query_results << name_hash # if fetch_cached_hashes(@@cached_query_results, [data]).empty? + @cached_query_results << name_hash # if fetch_cached_hashes(@cached_query_results, [data]).empty? else context.err(error) end @@ -292,7 +290,7 @@ def invoke_dsc_resource(context, name_hash, props, method) 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 = fetch_cached_hashes(@cached_test_results, [name]) prior_result.empty? ? invoke_test_method(context, name, should_hash) : prior_result.first[:in_desired_state] end @@ -361,7 +359,7 @@ def invoke_get_method(context, name_hash) data = recursively_sort(data) # Cache the query to prevent a second lookup - @@cached_query_results << data.dup if fetch_cached_hashes(@@cached_query_results, [data]).empty? + @cached_query_results << data.dup if fetch_cached_hashes(@cached_query_results, [data]).empty? context.debug("Returned to Puppet as #{data}") data end @@ -400,7 +398,7 @@ def invoke_test_method(context, name, should) return nil if data.nil? in_desired_state = data['indesiredstate'] - @@cached_test_results << name.merge({ in_desired_state: in_desired_state }) + @cached_test_results << name.merge({ in_desired_state: in_desired_state }) return in_desired_state if in_desired_state @@ -451,7 +449,7 @@ def vendored_modules_path(module_name) # path to allow multiple modules to with shared dsc_resources to be installed side by side # The old vendored_modules_path: puppet_x/dsc_resources # The new vendored_modules_path: puppet_x//dsc_resources - root_module_path = load_path.find { |path| path.match?(%r{#{puppetize_name(module_name)}/lib}) } + root_module_path = load_path.grep(%r{#{puppetize_name(module_name)}/lib}).first vendored_path = if root_module_path.nil? File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + "puppet_x/#{puppetize_name(module_name)}/dsc_resources") # rubocop:disable Style/StringConcatenation else @@ -509,12 +507,12 @@ def random_variable_name # # @return [Hash] containing all instantiated variables and the properties that they define def instantiated_variables - @@instantiated_variables ||= {} + @instantiated_variables ||= {} end # Clear the instantiated variables hash to be ready for the next run def clear_instantiated_variables! - @@instantiated_variables = {} + @instantiated_variables = {} end # Return true if the specified credential hash has already failed to execute a DSC resource due to @@ -523,7 +521,7 @@ def clear_instantiated_variables! # @param [Hash] a credential hash with a user and password keys where the password is a sensitive string # @return [Bool] true if the credential_hash has already failed logon, false otherwise def logon_failed_already?(credential_hash) - @@logon_failures.any? do |failure_hash| + @logon_failures.any? do |failure_hash| failure_hash['user'] == credential_hash['user'] && failure_hash['password'].unwrap == credential_hash['password'].unwrap end end diff --git a/spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb b/spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb index 8114d6fd..5295228c 100644 --- a/spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb +++ b/spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require 'puppet/resource_api' require 'puppet/type' require 'puppet/provider/dsc_base_provider/dsc_base_provider' require 'json' @@ -16,40 +15,40 @@ # Reset the caches after each run after 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 + described_class.instance_variable_set(:@cached_canonicalized_resource, []) + described_class.instance_variable_set(:@cached_query_results, []) + described_class.instance_variable_set(:@cached_test_results, []) + described_class.instance_variable_set(:@logon_failures, []) end describe '.initialize' do before do - # Need to initialize the provider to load the class variables + # Need to initialize the provider to load the instance variables provider end - it 'initializes the cached_canonicalized_resource class variable' do - expect(described_class.class_variable_get(:@@cached_canonicalized_resource)).to eq([]) + it 'initializes the cached_canonicalized_resource instance variable' do + expect(described_class.instance_variable_get(:@cached_canonicalized_resource)).to eq([]) end - it 'initializes the cached_query_results class variable' do - expect(described_class.class_variable_get(:@@cached_query_results)).to eq([]) + it 'initializes the cached_query_results instance variable' do + expect(described_class.instance_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([]) + it 'initializes the cached_test_results instance variable' do + expect(described_class.instance_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([]) + it 'initializes the logon_failures instance variable' do + expect(described_class.instance_variable_get(:@logon_failures)).to eq([]) end end describe '.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 + it 'returns the value of the @cached_test_results instance variable' do + described_class.instance_variable_set(:@cached_test_results, cache_value) expect(provider.cached_test_results).to eq(cache_value) end end @@ -238,11 +237,11 @@ describe '.get' do after do - described_class.class_variable_set(:@@cached_canonicalized_resource, []) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@cached_canonicalized_resource, []) end it 'checks the cached results, returning if one exists for the specified names' do - described_class.class_variable_set(:@@cached_canonicalized_resource, []) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@cached_canonicalized_resource, []) allow(context).to receive(:debug) expect(provider).to receive(:fetch_cached_hashes).with([], [{ name: 'foo' }]).and_return([{ name: 'foo', property: 'bar' }]) expect(provider).not_to receive(:invoke_get_method) @@ -250,7 +249,7 @@ end it 'adds mandatory properties to the name hash when calling invoke_get_method' do - described_class.class_variable_set(:@@cached_canonicalized_resource, [{ name: 'foo', property: 'bar', dsc_some_parameter: 'baz' }]) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@cached_canonicalized_resource, [{ name: 'foo', property: 'bar', dsc_some_parameter: 'baz' }]) allow(context).to receive(:debug) expect(provider).to receive(:fetch_cached_hashes).with([], [{ name: 'foo' }]).and_return([]) expect(provider).to receive(:namevar_attributes).and_return([:name]).exactly(3).times @@ -531,7 +530,7 @@ end after do - described_class.class_variable_set(:@@cached_query_results, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@cached_query_results, nil) end context 'when the invocation script returns data without errors' do @@ -558,7 +557,7 @@ it 'caches the result' do expect { result }.not_to raise_error - expect(described_class.class_variable_get(:@@cached_query_results)).to eq([result]) + expect(described_class.instance_variable_get(:@cached_query_results)).to eq([result]) end it 'removes unrelated properties from the result' do @@ -720,7 +719,7 @@ end after do - described_class.class_variable_set(:@@logon_failures, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@logon_failures, []) end it 'errors specifically for a logon failure and returns nil' do @@ -729,12 +728,12 @@ it 'caches the logon failure' do expect { result }.not_to raise_error - expect(described_class.class_variable_get(:@@logon_failures)).to eq([credential_hash]) + expect(described_class.instance_variable_get(:@logon_failures)).to eq([credential_hash]) end it 'caches the query results' do expect { result }.not_to raise_error - expect(described_class.class_variable_get(:@@cached_query_results)).to eq([name_hash]) + expect(described_class.instance_variable_get(:@cached_query_results)).to eq([name_hash]) end end @@ -982,11 +981,11 @@ end describe '.invoke_test_method' do - subject(:result) { provider.invoke_test_method(context, name, test_should) } + subject(:result) { provider.invoke_test_method(context, name, expect(subject).to) } let(:name) { { name: 'foo', dsc_name: 'bar' } } - let(:test_should) { name.merge(dsc_ensure: 'present') } - let(:test_properties) { test_should.reject { |k, _v| k == :name } } + let(:should) { name.merge(dsc_ensure: 'present') } + let(:test_properties) { expect(subject).to.reject { |k, _v| k == :name } } let(:invoke_dsc_resource_data) { nil } before do @@ -996,7 +995,7 @@ end after do - described_class.class_variable_set(:@@cached_test_results, []) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@cached_test_results, []) end context 'when something went wrong calling Invoke-DscResource' do @@ -1044,28 +1043,28 @@ describe '.instantiated_variables' do after do - described_class.class_variable_set(:@@instantiated_variables, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, []) end - it 'sets the instantiated_variables class variable to {} if not initialized' do + it 'sets the instantiated_variables instance variable to {} if not initialized' do expect(provider.instantiated_variables).to eq({}) end - it 'returns the instantiated_variables class variable if already initialized' do - described_class.class_variable_set(:@@instantiated_variables, { foo: 'bar' }) # rubocop:disable Style/ClassVars + it 'returns the instantiated_variables instance variable if already initialized' do + described_class.instance_variable_set(:@instantiated_variables, { foo: 'bar' }) expect(provider.instantiated_variables).to eq({ foo: 'bar' }) end end describe '.clear_instantiated_variables!' do after do - described_class.class_variable_set(:@@instantiated_variables, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, []) end - it 'sets the instantiated_variables class variable to {}' do - described_class.class_variable_set(:@@instantiated_variables, { foo: 'bar' }) # rubocop:disable Style/ClassVars + it 'sets the instantiated_variables instance variable to {}' do + described_class.instance_variable_set(:@instantiated_variables, { foo: 'bar' }) expect { provider.clear_instantiated_variables! }.not_to raise_error - expect(described_class.class_variable_get(:@@instantiated_variables)).to eq({}) + expect(described_class.instance_variable_get(:@instantiated_variables)).to eq({}) end end @@ -1088,16 +1087,16 @@ end after do - described_class.class_variable_set(:@@logon_failures, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@logon_failures, []) end it 'returns false if there have been no failed logons with the username/password combination' do - described_class.class_variable_set(:@@logon_failures, [bad_credential_hash]) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@logon_failures, [bad_credential_hash]) expect(provider.logon_failed_already?(good_credential_hash)).to be(false) end - it 'returns true if the username/password specified are found in the logon_failures class variable' do - described_class.class_variable_set(:@@logon_failures, [good_credential_hash, bad_credential_hash]) # rubocop:disable Style/ClassVars + it 'returns true if the username/password specified are found in the logon_failures instance variable' do + described_class.instance_variable_set(:@logon_failures, [good_credential_hash, bad_credential_hash]) expect(provider.logon_failed_already?(bad_credential_hash)).to be(true) end end @@ -1511,7 +1510,7 @@ end after do - described_class.class_variable_set(:@@instantiated_variables, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, []) end it 'writes the ruby representation of the credentials as the value of a key named for the new variable into the instantiated_variables cache' do @@ -1544,7 +1543,7 @@ subject(:result) { provider.prepare_cim_instances(test_resource) } after do - described_class.class_variable_set(:@@instantiated_variables, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, []) end context 'when a cim instance is passed without nested cim instances' do @@ -1653,7 +1652,7 @@ describe '.format_ciminstance' do after do - described_class.class_variable_set(:@@instantiated_variables, nil) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, []) end it 'defines and returns a new cim instance as a PowerShell variable, passing the class name and property hash' do @@ -1669,7 +1668,7 @@ end it 'interpolates variables in the case of a cim instance containing a nested instance' do - described_class.class_variable_set(:@@instantiated_variables, { 'SomeVariable' => { 'bar' => 'ope' } }) # rubocop:disable Style/ClassVars + described_class.instance_variable_set(:@instantiated_variables, { 'SomeVariable' => { 'bar' => 'ope' } }) property_hash = { 'foo' => { 'bar' => 'ope' } } expect(provider.format_ciminstance('foo', 'SomeClass', property_hash)).to match(/@\{'foo' = \$SomeVariable\}/) end