Merge pull request #869 from puppetlabs/GH-811-reimplement_reverted_PR

docker_run_flags: Shellescape any provided values

Author: GSPatton

lib/puppet/parser/functions/docker_run_flags.rb

@@ -1,21 +1,32 @@
 # frozen_string_literal: true
 
-require 'shellwords'
 #
 # docker_run_flags.rb
 #
 module Puppet::Parser::Functions
+  newfunction(:'docker::escape', type: :rvalue) do |args|
+    subject = args[0]
+
+    escape_function = if self['facts'] && self['facts']['os']['family'] == 'windows'
+                        'powershell_escape'
+                      else
+                        'shell_escape'
+                      end
+
+    call_function(escape_function, subject)
+  end
+
   # Transforms a hash into a string of docker flags
   newfunction(:docker_run_flags, type: :rvalue) do |args|
     opts = args[0] || {}
     flags = []
 
     if opts['username']
-      flags << "-u '#{opts['username'].shellescape}'"
+      flags << "-u #{call_function('docker::escape', [opts['username']])}"
     end
 
     if opts['hostname']
-      flags << "-h '#{opts['hostname'].shellescape}'"
+      flags << "-h #{call_function('docker::escape', [opts['hostname']])}"
     end
 
     if opts['restart']
@@ -24,9 +35,9 @@ module Puppet::Parser::Functions
 
     if opts['net']
       if opts['net'].is_a? String
-        flags << "--net #{opts['net'].shellescape}"
+        flags << "--net #{call_function('docker::escape', [opts['net']])}"
       elsif opts['net'].is_a? Array
-        flags << "--net #{opts['net'].join(' --net ').shellescape}"
+        flags += opts['net'].map { |item| ["--net #{call_function('docker::escape', [item])}"] }
       end
     end
 
@@ -72,17 +83,17 @@ module Puppet::Parser::Functions
 
     multi_flags = ->(values, fmt) {
       filtered = [values].flatten.compact
-      filtered.map { |val| (fmt + params_join_char) % val }
+      filtered.map { |val| (fmt + params_join_char) % call_function('docker::escape', [val]) }
     }
 
     [
       ['--dns %s',          'dns'],
       ['--dns-search %s',   'dns_search'],
       ['--expose=%s',       'expose'],
       ['--link %s',         'links'],
-      ['--lxc-conf="%s"',   'lxc_conf'],
+      ['--lxc-conf=%s',     'lxc_conf'],
       ['--volumes-from %s', 'volumes_from'],
-      ['-e "%s"',           'env'],
+      ['-e %s',             'env'],
       ['--env-file %s',     'env_file'],
       ['-p %s',             'ports'],
       ['-l %s',             'labels'],

metadata.json

@@ -10,7 +10,7 @@
   "dependencies": [
     {
       "name": "puppetlabs/stdlib",
-      "version_requirement": ">= 4.24.0 < 9.0.0"
+      "version_requirement": ">= 8.2.0 < 9.0.0"
     },
     {
       "name": "puppetlabs/apt",

spec/acceptance/docker_params_changed_spec.rb

@@ -23,7 +23,7 @@
 else
   docker_args = ''
   docker_network = 'bridge'
-  volume_location = '/opt'
+  volume_location = '/opt/'
   docker_image = 'hello-world:linux'
 end
 
@@ -33,8 +33,8 @@
       install_pp = "class { 'docker': #{docker_args}}"
       apply_manifest(install_pp)
     end
-    run_shell("mkdir #{volume_location}/volume_1")
-    run_shell("mkdir #{volume_location}/volume_2")
+    run_shell("mkdir #{volume_location}volume_1")
+    run_shell("mkdir #{volume_location}volume_2")
   end
 
   context 'when image is changed' do
@@ -78,8 +78,8 @@ class {'docker': #{docker_args}}
       volumes1 = "volumes => ['volume-1:C:\\volume_1']"
       volumes2 = "volumes => ['volume-1:C:\\volume_1', 'volume-2:C:\\volume_2']"
     else
-      volumes1 = "volumes => ['volume-1:#{volume_location}/volume_1']"
-      volumes2 = "volumes => ['volume-1:#{volume_location}/volume_1', 'volume-2:#{volume_location}/volume_2']"
+      volumes1 = "volumes => ['volume-1:#{volume_location}volume_1']"
+      volumes2 = "volumes => ['volume-1:#{volume_location}volume_1', 'volume-2:#{volume_location}volume_2']"
     end
 
     let(:pp1) do
@@ -143,7 +143,7 @@ class {'docker': #{docker_args}}
   end
 
   after(:all) do
-    run_shell("rm -r #{volume_location}/volume_1")
-    run_shell("rm -r #{volume_location}/volume_2")
+    run_shell("rm -r #{volume_location}volume_1")
+    run_shell("rm -r #{volume_location}volume_2")
   end
 end

spec/unit/lib/puppet/parser/functions/docker_run_flags_spec.rb

@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe 'the "docker_run_flags" parser function' do
+  before :each do
+    Puppet[:modulepath] = 'spec/fixtures/modules'
+  end
+
+  let :scope do
+    node     = Puppet::Node.new('localhost')
+    compiler = Puppet::Parser::Compiler.new(node)
+    scope    = Puppet::Parser::Scope.new(compiler)
+    allow(scope).to receive(:environment).and_return(nil)
+    allow(scope).to receive(:[]).with('facts').and_return({ 'os' => { 'family' => os_family } })
+    scope
+  end
+
+  context 'on POSIX system' do
+    let(:os_family) { 'Linux' }
+
+    it 'escapes special chars' do
+      expect(scope.function_docker_run_flags([{ 'env' => [%.MYSQL_PASSWORD='"$()[]{}<>.], 'extra_params' => [] }])).to eq(%(-e MYSQL_PASSWORD\\=\\'\\"\\$\\(\\)\\[\\]\\{\\}\\<\\> \\\n))
+    end
+  end
+
+  context 'on windows' do
+    let(:os_family) { 'windows' }
+
+    it 'escapes special chars' do
+      expect(scope.function_docker_run_flags([{ 'env' => [%.MYSQL_PASSWORD='"$()[]{}<>.], 'extra_params' => [] }])).to eq(%^-e MYSQL_PASSWORD=`'\\`"`$()[]{}<> \\\n^)
+    end
+  end
+end