diff --git a/.gitignore b/.gitignore index 2803e566..af66731a 100644 --- a/.gitignore +++ b/.gitignore @@ -15,7 +15,6 @@ /junit/ /log/ /pkg/ -/spec/fixtures/manifests/ /spec/fixtures/modules/* /tmp/ /vendor/ diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 03a9e5c4..d9edc3f1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,11 +1,18 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-10-01 08:51:22 UTC using RuboCop version 1.73.2. +# on 2025-10-29 07:31:19 UTC using RuboCop version 1.73.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. +# Offense count: 3 +# This cop supports safe autocorrection (--autocorrect). +RSpec/ExpectActual: + Exclude: + - '**/spec/routing/**/*' + - 'spec/acceptance/deferred_spec.rb' + # Offense count: 2 # Configuration parameters: Include, CustomTransform, IgnoreMethods, IgnoreMetadata. # Include: **/*_spec.rb @@ -20,3 +27,11 @@ RSpec/SpecFilePathFormat: Style/BitwisePredicate: Exclude: - 'lib/puppet/type/dsc.rb' + +# Offense count: 1 +# This cop supports unsafe autocorrection (--autocorrect-all). +# Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength. +# AllowedMethods: present?, blank?, presence, try, try! +Style/SafeNavigation: + Exclude: + - 'lib/puppet/provider/base_dsc_lite/powershell.rb' diff --git a/lib/puppet/provider/base_dsc_lite/powershell.rb b/lib/puppet/provider/base_dsc_lite/powershell.rb index c0122c33..36682ff3 100644 --- a/lib/puppet/provider/base_dsc_lite/powershell.rb +++ b/lib/puppet/provider/base_dsc_lite/powershell.rb @@ -2,6 +2,11 @@ require 'pathname' require 'json' +require 'erb' # ensure ERB is available +require 'puppet' +require 'puppet/node' +require 'puppet/parser/script_compiler' +require 'puppet/pops/evaluator/deferred_resolver' require_relative '../../../puppet_x/puppetlabs/dsc_lite/powershell_hash_formatter' Puppet::Type.type(:base_dsc_lite).provide(:powershell) do @@ -40,6 +45,80 @@ def self.vendored_modules_path File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + 'puppet_x/dsc_resources') end + # ---------- Resolution helpers (Option 2) ---------- + + # 1) Catalog-wide resolve: replace all Deferreds/futures in the catalog + def force_resolve_catalog_deferred! + cat = resource&.catalog + return unless cat + + facts = Puppet.lookup(:facts) { nil } + env = if cat.respond_to?(:environment_instance) + cat.environment_instance + else + Puppet.lookup(:current_environment) { nil } + end + begin + Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(facts, cat, env, true) + Puppet.notice('DSC PROVIDER SENTINEL → resolve_and_replace invoked') + rescue => e + Puppet.notice("DSC PROVIDER resolve_and_replace error: #{e.class}: #{e.message}") + end + end + + # Build a compiler on the agent for local resolution (ScriptCompiler on Puppet 8) + def build_agent_compiler(env) + node_name = Puppet[:node_name_value] + node = Puppet::Node.new(node_name, environment: env) + + # If you can attach facts, do so—it can influence function behavior during resolve + begin + facts = Puppet.lookup(:facts) { nil } + node.add_facts(facts) if facts + rescue => e + Puppet.debug("DSC_lite: could not attach facts to node for local resolve: #{e.class}: #{e.message}") + end + + if defined?(Puppet::Parser::ScriptCompiler) + Puppet::Parser::ScriptCompiler.new(node, env) + else + Puppet::Parser::Compiler.new(node) + end + end + + # 2) Targeted resolve: explicitly resolve the :properties value using a resolver that + # can handle both Deferred and evaluator futures, then write it back to the resource. + def force_resolve_properties! + return unless resource.parameters.key?(:properties) + + cat = resource&.catalog + env = if cat&.respond_to?(:environment_instance) + cat.environment_instance + else + Puppet.lookup(:current_environment) { nil } + end + + begin + compiler = build_agent_compiler(env) if env + return unless compiler # without a compiler, local resolve can't proceed + + facts = Puppet.lookup(:facts) { nil } + + resolver = Puppet::Pops::Evaluator::DeferredResolver.new(compiler, true) + resolver.set_facts_variable(facts) if facts + + props = resource.parameters[:properties].value + resolved = resolver.resolve(props) # handles nested Deferred and futures + resource.parameters[:properties].value = resolved + + Puppet.debug('DSC_lite: explicitly resolved resource[:properties] on agent') + rescue => e + Puppet.debug("DSC_lite: explicit properties resolve failed: #{e.class}: #{e.message}") + end + end + + # ---------- Existing provider helpers ---------- + def dsc_parameters resource.parameters_with_value.select do |p| p.name.to_s.include? 'dsc_' @@ -65,18 +144,31 @@ def ps_manager Pwsh::Manager.instance(command(:powershell), Pwsh::Manager.powershell_args, debug: debug_output) end + # If your ERBs call provider.format_for_ps(...), keep this minimal helper + def format_for_ps(value) + self.class.format_dsc_lite(value) + end + + # ---------- Provider operations ---------- + def exists? + Puppet.notice("DSC PROVIDER SENTINEL → #{__FILE__}") + + # Two-stage resolve before we render ERB + force_resolve_catalog_deferred! + force_resolve_properties! + timeout = set_timeout Puppet.debug "Dsc Timeout: #{timeout} milliseconds" version = Facter.value(:powershell_version) Puppet.debug "PowerShell Version: #{version}" + script_content = ps_script_content('test') Puppet.debug "\n" + self.class.redact_content(script_content) if Pwsh::Manager.windows_powershell_supported? output = ps_manager.execute(script_content, timeout) raise Puppet::Error, output[:errormessage] if output[:errormessage]&.match?(%r{PowerShell module timeout \(\d+ ms\) exceeded while executing}) - output = output[:stdout] else self.class.upgrade_message @@ -95,15 +187,21 @@ def exists? end def create + Puppet.notice("DSC PROVIDER SENTINEL → #{__FILE__}") + + # Two-stage resolve before we render ERB + force_resolve_catalog_deferred! + force_resolve_properties! + timeout = set_timeout Puppet.debug "Dsc Timeout: #{timeout} milliseconds" + script_content = ps_script_content('set') Puppet.debug "\n" + self.class.redact_content(script_content) if Pwsh::Manager.windows_powershell_supported? output = ps_manager.execute(script_content, timeout) raise Puppet::Error, output[:errormessage] if output[:errormessage]&.match?(%r{PowerShell module timeout \(\d+ ms\) exceeded while executing}) - output = output[:stdout] else self.class.upgrade_message @@ -114,7 +212,6 @@ def create data = JSON.parse(output) raise(data['errormessage']) unless data['errormessage'].empty? - notify_reboot_pending if data['rebootrequired'] == true data @@ -147,12 +244,7 @@ def self.escape_quotes(text) end def self.redact_content(content) - # Note that here we match after an equals to ensure we redact the value being passed, but not the key. - # This means a redaction of a string not including '= ' before the string value will not redact. - # Every secret unwrapped in this module will unwrap as "'secret' # PuppetSensitive" and, currently, - # always inside a hash table to be passed along. This means we can (currently) expect the value to - # always come after an equals sign. - # Note that the line may include a semi-colon and/or a newline character after the sensitive unwrap. + # NOTE: match after '=' so we redact the value being passed, but not the key content.gsub(%r{= '.+' # PuppetSensitive;?(\\n)?$}, "= '[REDACTED]'") end @@ -165,6 +257,10 @@ def self.ps_script_content(mode, resource, provider) @param_hash = resource template_name = resource.generic_dsc ? '/invoke_generic_dsc_resource.ps1.erb' : '/invoke_dsc_resource.ps1.erb' file = File.new(template_path + template_name, encoding: Encoding::UTF_8) + + # Make vendored_modules_path visible in ERB if the template uses it + vendored_modules_path = self.vendored_modules_path + template = ERB.new(file.read, trim_mode: '-') template.result(binding) end diff --git a/lib/puppet/type/dsc.rb b/lib/puppet/type/dsc.rb index c72d0937..d049bbd6 100644 --- a/lib/puppet/type/dsc.rb +++ b/lib/puppet/type/dsc.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'pathname' +require 'puppet/pops/evaluator/deferred_resolver' # ← add: resolver API Puppet::Type.newtype(:dsc) do desc <<-DOC @@ -17,6 +18,7 @@ module => 'PSDesiredStateConfiguration', } } DOC + require Pathname.new(__FILE__).dirname + '../../' + 'puppet/type/base_dsc_lite' require Pathname.new(__FILE__).dirname + '../../puppet_x/puppetlabs/dsc_lite/dsc_type_helpers' @@ -105,7 +107,33 @@ def change_to_s(currentvalue, newvalue) end munge do |value| - PuppetX::DscLite::TypeHelpers.munge_sensitive_hash(value) + Puppet.notice("DSC TYPE SENTINEL → #{__FILE__}") + compiler = Puppet.lookup(:compiler) { nil } + if compiler + begin + resolved = Puppet::Pops::Evaluator::DeferredResolver.resolve(value, compiler) + rescue => e + Puppet.notice("DSC TYPE resolve() error: #{e.class}: #{e.message}") + resolved = value + end + else + Puppet.notice('DSC TYPE SENTINEL → no compiler available during munge') + resolved = value + end + + # Quick check: did any Deferred remain? + def contains_deferred?(x) + case x + when Hash then x.any? { |k, v| contains_deferred?(k) || contains_deferred?(v) } + when Array then x.any? { |v| contains_deferred?(v) } + else x && x.class.name.to_s.include?('Deferred') + end + end + if contains_deferred?(resolved) + Puppet.notice('DSC TYPE SENTINEL → properties still contains Deferred AFTER resolve()') + end + + PuppetX::DscLite::TypeHelpers.munge_sensitive_hash(resolved) end end diff --git a/spec/acceptance/deferred_spec.rb b/spec/acceptance/deferred_spec.rb new file mode 100644 index 00000000..b992e14e --- /dev/null +++ b/spec/acceptance/deferred_spec.rb @@ -0,0 +1,102 @@ +# spec/acceptance/deferred_spec.rb +# frozen_string_literal: true + +require 'spec_helper_acceptance' + +def read_fixture(name) + File.read(File.join(__dir__, '..', 'fixtures', 'manifests', name)) +end + +def read_win_file_if_exists(path) + # Use a script block with literals; avoid $variables to prevent transport/quoting expansion + # Also keep exit 0 regardless of existence so run_shell doesn't raise. + ps = %{& { if (Test-Path -LiteralPath '#{path}') { Get-Content -Raw -LiteralPath '#{path}' } else { '<<>>' } } } + r = run_shell(%(powershell.exe -NoProfile -NonInteractive -Command "#{ps}")) + body = (r.stdout || '').to_s + exists = !body.include?('<<>>') + { exists: exists, content: exists ? body : '' } +end + +describe 'deferred values with dsc_lite' do + let(:control_manifest) { read_fixture('01_file_deferred.pp') } + let(:dsc_control_manifest_epp) { read_fixture('01b_file_deferred_with_epp.pp') } + let(:dsc_deferred_direct) { read_fixture('02_dsc_deferred_direct.pp') } + let(:dsc_deferred_inline) { read_fixture('02b_dsc_deferred_inline.pp') } # ← NEW + let(:dsc_deferred_epp_inline) { read_fixture('02c_dsc_deferred_inline_epp.pp') } # ← NEW + let(:dsc_deferred_stringified) { read_fixture('03a_dsc_deferred_stringified.pp') } + let(:dsc_deferred_bad_unwrap) { read_fixture('03b_dsc_deferred_bad_unwrap.pp') } + + it 'control (01): native file + Deferred resolves to hello-file' do + result = idempotent_apply(control_manifest) + expect(result.exit_code).to eq(0) + out = read_win_file_if_exists('C:/Temp/deferred_ok.txt') + expect(out[:exists]).to be(true) + expect(out[:content].strip).to eq('hello-file') + end + + it 'control (01b): native file + Deferred resolves to hello-file (EPP)' do + result = idempotent_apply(dsc_control_manifest_epp) + expect(result.exit_code).to eq(0) + out = read_win_file_if_exists('C:/Temp/deferred_ok.txt') + expect(out[:exists]).to be(true) + expect(out[:content].strip).to eq('hello-file') + end + + it '02: passing Deferred via variable to DSC resolves to hello-dsc (otherwise flag bug)' do + apply = apply_manifest(dsc_deferred_direct) + out = read_win_file_if_exists('C:/Temp/from_dsc.txt') + content = out[:content].strip + if out[:exists] && content == 'hello-dsc' + expect(true).to be(true) + elsif out[:exists] && content =~ %r{Deferred\s*\(|Puppet::Pops::Types::Deferred}i + raise "BUG: 02 wrote stringified Deferred: #{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + else + raise "Unexpected 02 outcome. Exists=#{out[:exists]} Content=#{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + end + end + + # NEW 02b: inline Deferred on the DSC property (no variable intermediary) + it '02b: passing Deferred inline to DSC resolves to hello-dsc-inline (otherwise flag bug)' do + apply = apply_manifest(dsc_deferred_inline) + out = read_win_file_if_exists('C:/Temp/from_dsc_inline.txt') + content = out[:content].strip + if out[:exists] && content == 'hello-dsc-inline' + expect(true).to be(true) + elsif out[:exists] && content =~ %r{Deferred\s*\(|Puppet::Pops::Types::Deferred}i + raise "BUG: 02b wrote stringified Deferred: #{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + else + raise "Unexpected 02b outcome. Exists=#{out[:exists]} Content=#{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + end + end + + # NEW 02c: inline Deferred on the DSC property (no variable intermediary) + it '02c: passing a Deferred inline while calling an epp' do + apply = apply_manifest(dsc_deferred_epp_inline) + out = read_win_file_if_exists('C:/Temp/from_dsc_inline.txt') + content = out[:content].strip + if out[:exists] && content == 'hello-dsc-epp' + expect(true).to be(true) + elsif out[:exists] && content =~ %r{Deferred\s*\(|Puppet::Pops::Types::Deferred}i + raise "BUG: 02c wrote stringified Deferred: #{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + else + raise "Unexpected 02c outcome. Exists=#{out[:exists]} Content=#{content.inspect}\nApply:\n#{apply.stdout}#{apply.stderr}" + end + end + + it '03a: stringifying a Deferred writes the function form (reproduces customer report)' do + apply_manifest(dsc_deferred_stringified) + out = read_win_file_if_exists('C:/Temp/from_dsc_var_string.txt') + expect(out[:exists]).to be(true) + expect(out[:content]).to match(%r{Deferred\s*\(|Puppet::Pops::Types::Deferred}i) + expect(out[:content]).not_to match(%r{\bhello-var\b}) + end + + it '03b: unwrap on a non‑Sensitive is a no‑op; also writes the function form' do + apply_manifest(dsc_deferred_bad_unwrap) + out = read_win_file_if_exists('C:/Temp/from_dsc_var_bad_unwrap.txt') + out = read_win_file_if_exists('C:/Temp/from_dsc_var.txt') unless out[:exists] + expect(out[:exists]).to be(true) + expect(out[:content]).to match(%r{Deferred\s*\(|Puppet::Pops::Types::Deferred}i) + expect(out[:content]).not_to match(%r{\bhello-var\b}) + end +end diff --git a/spec/fixtures/manifests/01_file_deferred.pp b/spec/fixtures/manifests/01_file_deferred.pp new file mode 100644 index 00000000..4f12b74e --- /dev/null +++ b/spec/fixtures/manifests/01_file_deferred.pp @@ -0,0 +1,12 @@ +# spec/fixtures/manifests/01_file_deferred.pp +file { 'C:/Temp': + ensure => directory, +} + +$deferred = Deferred('join', [['hello','-','file'], '']) + +file { 'C:/Temp/deferred_ok.txt': + ensure => file, + content => $deferred, + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/01b_file_deferred_with_epp.pp b/spec/fixtures/manifests/01b_file_deferred_with_epp.pp new file mode 100644 index 00000000..9f9dfe2c --- /dev/null +++ b/spec/fixtures/manifests/01b_file_deferred_with_epp.pp @@ -0,0 +1,12 @@ +# spec/fixtures/manifests/01_file_deferred.pp +file { 'C:/Temp': + ensure => directory, +} + +$deferred = Deferred('join', [['hello','-','file'], '']) + +file { 'C:/Temp/deferred_ok.txt': + ensure => file, + content => Deferred('inline_epp', ['<%= $content.unwrap %>', { content => $deferred }]), + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/02_dsc_deferred_direct.pp b/spec/fixtures/manifests/02_dsc_deferred_direct.pp new file mode 100644 index 00000000..3fac8289 --- /dev/null +++ b/spec/fixtures/manifests/02_dsc_deferred_direct.pp @@ -0,0 +1,18 @@ +# spec/fixtures/manifests/02_dsc_deferred_direct.pp +file { 'C:/Temp': + ensure => directory, +} + +$deferred = Deferred('join', [['hello','-','dsc'], '']) + +dsc { 'WriteFileViaDSC': + resource_name => 'File', + module => 'PSDesiredStateConfiguration', + properties => { + 'DestinationPath' => 'C:\Temp\from_dsc.txt', + 'Type' => 'File', + 'Ensure' => 'Present', + 'Contents' => $deferred, + }, + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/02b_dsc_deferred_inline.pp b/spec/fixtures/manifests/02b_dsc_deferred_inline.pp new file mode 100644 index 00000000..6acd41f7 --- /dev/null +++ b/spec/fixtures/manifests/02b_dsc_deferred_inline.pp @@ -0,0 +1,16 @@ +# spec/fixtures/manifests/02b_dsc_deferred_inline.pp +file { 'C:/Temp': + ensure => directory, +} + +dsc { 'WriteFileViaDSCInline': + resource_name => 'File', + module => 'PSDesiredStateConfiguration', + properties => { + 'DestinationPath' => 'C:\Temp\from_dsc_inline.txt', + 'Type' => 'File', + 'Ensure' => 'Present', + 'Contents' => Deferred('join', [['hello','-','dsc-inline'], '']), + }, + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/02c_dsc_deferred_inline_epp.pp b/spec/fixtures/manifests/02c_dsc_deferred_inline_epp.pp new file mode 100644 index 00000000..40b29fbc --- /dev/null +++ b/spec/fixtures/manifests/02c_dsc_deferred_inline_epp.pp @@ -0,0 +1,18 @@ +# spec/fixtures/manifests/02_dsc_deferred_direct.pp +file { 'C:/Temp': + ensure => directory, +} + +$deferred = Deferred('join', [['hello','-','dsc', '-', 'epp'], '']) + +dsc { 'WriteFileViaDSC': + resource_name => 'File', + module => 'PSDesiredStateConfiguration', + properties => { + 'DestinationPath' => 'C:\Temp\from_dsc.txt', + 'Type' => 'File', + 'Ensure' => 'Present', + 'Contents' => Deferred('inline_epp', ['<%= $content.unwrap %>', { content => $deferred }]), + }, + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/03a_dsc_deferred_stringified.pp b/spec/fixtures/manifests/03a_dsc_deferred_stringified.pp new file mode 100644 index 00000000..819abf8d --- /dev/null +++ b/spec/fixtures/manifests/03a_dsc_deferred_stringified.pp @@ -0,0 +1,19 @@ +# spec/fixtures/manifests/03a_dsc_deferred_stringified.pp +file { 'C:/Temp': ensure => directory } + +$deferred = Deferred('join', [['hello','-','var'], '']) + +# WRONG on purpose: coerces Deferred to a String at compile time +$stringified = String($deferred) + +dsc { 'WriteFileViaDSCVarStringified': + resource_name => 'File', + module => 'PSDesiredStateConfiguration', + properties => { + 'DestinationPath' => 'C:\Temp\from_dsc_var_string.txt', + 'Type' => 'File', + 'Ensure' => 'Present', + 'Contents' => $stringified, + }, + require => File['C:/Temp'], +} diff --git a/spec/fixtures/manifests/03b_dsc_deferred_bad_unwrap.pp b/spec/fixtures/manifests/03b_dsc_deferred_bad_unwrap.pp new file mode 100644 index 00000000..c908f74a --- /dev/null +++ b/spec/fixtures/manifests/03b_dsc_deferred_bad_unwrap.pp @@ -0,0 +1,19 @@ +# spec/fixtures/manifests/03b_dsc_deferred_bad_unwrap.pp +file { 'C:/Temp': ensure => directory } + +$deferred = Deferred('join', [['hello','-','var'], '']) + +# WRONG: unwrap applies to Sensitive, not Deferred; this should compile-fail +$unwrapped_deferred = String($deferred.unwrap) + +dsc { 'WriteFileViaDSCVarBadUnwrap': + resource_name => 'File', + module => 'PSDesiredStateConfiguration', + properties => { + 'DestinationPath' => 'C:\Temp\from_dsc_var_bad_unwrap.txt', + 'Type' => 'File', + 'Ensure' => 'Present', + 'Contents' => $unwrapped_deferred, + }, + require => File['C:/Temp'], +}