diff --git a/templates/install_puppet.ps1.erb b/files/install_puppet.ps1 similarity index 92% rename from templates/install_puppet.ps1.erb rename to files/install_puppet.ps1 index 678015945..9e88c4c7d 100644 --- a/templates/install_puppet.ps1.erb +++ b/files/install_puppet.ps1 @@ -30,14 +30,17 @@ param( # correctly from the module. [parameter(Position=0)] [String] $PuppetPID, - [String] $Source='<%= @_msi_location %>', - [String] $Logfile='<%= @_logfile %>', - [String] $InstallDir='<%= @install_dir %>', - [String] $PuppetMaster='<%= @_puppet_master %>', - [String] $PuppetStartType='<%= @_agent_startup_mode %>', - [String] $InstallArgs='<%= @_install_options %>', - [String] $InstallScriptPIDFile='<%= @_install_pid_file_loc %>', - [Bool] $UseLockedFilesWorkaround=<%= @_move_dll_workaround %> + [String] $Source, + [String] $Logfile, + [AllowEmptyString()] + [String] $InstallDir, + [AllowEmptyString()] + [String] $PuppetMaster, + [AllowEmptyString()] + [String] $PuppetStartType, + [AllowEmptyString()] + [String] $InstallArgs, + [switch] $UseLockedFilesWorkaround ) # Find-InstallDir, Move-PuppetresDLL and Reset-PuppetresDLL serve as a workaround for older # installations of puppet: we used to need to move puppetres.dll out of the way during puppet @@ -336,13 +339,10 @@ try { } } catch { Write-Log "ERROR: $_" - try { - if ($UseLockedFilesWorkaround) { - Reset-PuppetresDLL $temp_puppetres - } - } finally { - Reset-PuppetServices $services_before + if ($UseLockedFilesWorkaround) { + Reset-PuppetresDLL $temp_puppetres } } finally { + Reset-PuppetServices $services_before Unlock-Installation $install_pid_lock } diff --git a/lib/puppet/parser/functions/windows_msi_installargs.rb b/lib/puppet/parser/functions/windows_msi_installargs.rb index 97becdd22..2d2e15a41 100644 --- a/lib/puppet/parser/functions/windows_msi_installargs.rb +++ b/lib/puppet/parser/functions/windows_msi_installargs.rb @@ -14,6 +14,10 @@ module Puppet::Parser::Functions option end end - return arg_string.join(' ') + # When the MSI installargs parameter is passed to the powershell script it's inside + # a cmd.exe instance, so we need to escape the quotes correctly so they show up as + # plaintext double quotes to the powershell command. (To correctly escape to a + # plaintext " you use three "'s in cmd.exe) + return arg_string.join(' ').gsub('"', '"""') end end diff --git a/manifests/windows/install.pp b/manifests/windows/install.pp index d91b017fc..6f03e1bd5 100644 --- a/manifests/windows/install.pp +++ b/manifests/windows/install.pp @@ -13,8 +13,6 @@ $service_names = $::puppet_agent::service_names - $_installps1 = windows_native_path("${::env_temp_variable}/install_puppet.ps1") - $_msi_location = $::puppet_agent::prepare::package::local_package_file_path $_install_options = $install_options ? { @@ -29,25 +27,40 @@ } if $msi_move_locked_files { - $_move_dll_workaround = '$true' + $_move_dll_workaround = '-UseLockedFilesWorkaround' } else { - $_move_dll_workaround = '$false' + $_move_dll_workaround = undef } $_timestamp = strftime('%Y_%m_%d-%H_%M') $_logfile = windows_native_path("${::env_temp_variable}/puppet-${_timestamp}-installer.log") - $_puppet_master = $::puppet_master_server - $_install_pid_file_loc = windows_native_path("${::env_temp_variable}/puppet_agent_install.pid") notice ("Puppet upgrade log file at ${_logfile}") debug ("Installing puppet from ${_msi_location}") + $_installps1 = windows_native_path("${::env_temp_variable}/install_puppet.ps1") file { "${_installps1}": ensure => file, - content => template('puppet_agent/install_puppet.ps1.erb') + content => file('puppet_agent/install_puppet.ps1') } -> exec { 'install_puppet.ps1': - command => "${::system32}\\cmd.exe /c start /b ${::system32}\\WindowsPowerShell\\v1.0\\powershell.exe -ExecutionPolicy Bypass -NoProfile -NoLogo -NonInteractive -File ${_installps1} ${::puppet_agent_pid}", + # The powershell execution uses -Command and not -File because -File will interpolate the quotes + # in a context like cmd.exe: https://docs.microsoft.com/en-us/powershell/scripting/components/console/powershell.exe-command-line-help?view=powershell-6#-file-- + # Because of this it's much cleaner to use -Command and use single quotes for each powershell param + command => "${::system32}\\cmd.exe /S /c start /b ${::system32}\\WindowsPowerShell\\v1.0\\powershell.exe \ + -ExecutionPolicy Bypass \ + -NoProfile \ + -NoLogo \ + -NonInteractive \ + -Command ${_installps1} \ + -PuppetPID ${::puppet_agent_pid} \ + -Source '${_msi_location}' \ + -Logfile '${_logfile}' \ + -InstallDir '${install_dir}' \ + -PuppetMaster '${::puppet_master_server}' \ + -PuppetStartType '${_agent_startup_mode}' \ + -InstallArgs '${_install_options}' \ + ${_move_dll_workaround}", path => $::path, } diff --git a/spec/classes/puppet_agent_windows_install_spec.rb b/spec/classes/puppet_agent_windows_install_spec.rb index 06bb22aa2..7ea20e44b 100644 --- a/spec/classes/puppet_agent_windows_install_spec.rb +++ b/spec/classes/puppet_agent_windows_install_spec.rb @@ -68,9 +68,7 @@ })} it { is_expected.to contain_class('puppet_agent::windows::install') } - it { is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content( - %r[#{Regexp.escape("\$Source=\'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{arch}.msi\'")}]) - } + it { is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source \'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{arch}.msi\'/) } it { is_expected.to contain_exec('fix inheritable SYSTEM perms') } end end @@ -78,13 +76,13 @@ context 'install_options =>' do describe 'OPTION1=value1 OPTION2=value2' do let(:params) { global_params.merge( - {:install_options => ['OPTION1=value1','OPTION2=value2'],}) + {:install_options => ['OPTION1=value1','OPTION2="value2"'],}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$InstallArgs='OPTION1=value1 OPTION2=value2'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-InstallArgs 'OPTION1=value1 OPTION2="""value2"""'/) } it { - is_expected.not_to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$InstallArgs='REINSTALLMODE="amus"'/) + is_expected.not_to contain_exec('install_puppet.ps1').with_command(/\-InstallArgs 'REINSTALLMODE="""amus"""'/) } end end @@ -92,7 +90,7 @@ context 'Default INSTALLMODE Option' do describe 'REINSTALLMODE=amus' do it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$InstallArgs='REINSTALLMODE="amus"'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-InstallArgs 'REINSTALLMODE="""amus"""'/) } end end @@ -103,8 +101,8 @@ {:source => 'https://alternate.com/puppet-agent.msi',}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Logfile='C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Logfile 'C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) } it { is_expected.to contain_exec('fix inheritable SYSTEM perms') } end @@ -114,8 +112,8 @@ {:source => 'C:/tmp/puppet-agent-x64.msi',}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Logfile='C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Logfile 'C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) } it { is_expected.to contain_exec('fix inheritable SYSTEM perms') } end @@ -125,16 +123,16 @@ {:source => "\\\\garded\\c$\\puppet-agent-x64.msi",}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Logfile='C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Logfile 'C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) } it { is_expected.to contain_exec('fix inheritable SYSTEM perms') } end describe 'default source' do it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Logfile='C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Logfile 'C:\\tmp\\puppet-\d+_\d+_\d+-\d+_\d+-installer.log'/) } it { should contain_exec('install_puppet.ps1').with { { @@ -152,7 +150,7 @@ {:source => 'puppet:///puppet_agent/puppet-agent-1.1.0-x86.msi'}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-#{arch}\.msi'/) } it { is_expected.to contain_exec('fix inheritable SYSTEM perms') } end @@ -164,7 +162,7 @@ {:arch => 'x86'}) } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$Source='C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-x86.msi'/ + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-Source 'C:\\ProgramData\\Puppetlabs\\packages\\puppet-agent-#{package_version}-x86.msi'/ ) } end @@ -192,7 +190,7 @@ context 'msi_move_locked_files =>' do describe 'default' do it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$UseLockedFilesWorkaround=\$false/) + is_expected.not_to contain_exec('install_puppet.ps1').with_command(/\-UseLockedFilesWorkaround/) } end @@ -202,7 +200,7 @@ } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$UseLockedFilesWorkaround=\$false/) + is_expected.not_to contain_exec('install_puppet.ps1').with_command(/\-UseLockedFilesWorkaround/) } end @@ -212,7 +210,7 @@ } it { - is_expected.to contain_file('C:\tmp\install_puppet.ps1').with_content(/\$UseLockedFilesWorkaround=\$true/) + is_expected.to contain_exec('install_puppet.ps1').with_command(/\-UseLockedFilesWorkaround/) } end end