Merge pull request #8173 from ekinanp/PUP-10528

(PUP-10528) Extend trusted_external_command to support a directory

acceptance/tests/ssl/trusted_external_facts.rb

@@ -2,67 +2,182 @@
   require 'puppet/acceptance/environment_utils'
   extend Puppet::Acceptance::EnvironmentUtils
 
+  ### HELPERS ###
+
+  SEPARATOR="<TRUSTED_JSON>"
+  def parse_trusted_json(puppet_output)
+    trusted_json = puppet_output.split(SEPARATOR)[1]
+    if trusted_json.nil?
+      raise "Puppet output does not contain the expected '#{SEPARATOR}<trusted_json>#{SEPARATOR}' output"
+    end
+    JSON.parse(trusted_json)
+  rescue => e
+    raise "Failed to parse the trusted JSON: #{e}"
+  end
+
+  ### END HELPERS ###
+
   tag 'audit:high',        # external facts
-      'server'
+    'server'
 
   skip_test 'requires a master for serving module content' if master.nil?
 
   testdir = master.tmpdir('trusted_external_facts')
   on(master, "chmod 755 #{testdir}")
-  external_trusted_fact_script_path = "#{testdir}/external_facts.sh"
   tmp_environment = mk_tmp_environment_with_teardown(master, File.basename(__FILE__, '.*'))
 
   teardown do
     on(master, "rm -r '#{testdir}'", :accept_all_exit_codes => true)
   end
 
-  step "Step 1: check we can run the trusted external fact" do
-    external_trusted_fact_script = <<EOF
-#!/bin/bash
-CERTNAME=$1
-printf '{"doot":"%s"}\n' "$CERTNAME"
-EOF
-    create_remote_file(master, external_trusted_fact_script_path, external_trusted_fact_script)
-    on(master, "chmod 777 #{external_trusted_fact_script_path}")
-  end
-
-  step "Step 2: create external module referencing trusted hash" do
+  step "create 'external' module referencing trusted hash" do
     on(master, "mkdir -p #{environmentpath}/#{tmp_environment}/modules/external/manifests")
     master_module_manifest = "#{environmentpath}/#{tmp_environment}/modules/external/manifests/init.pp"
     manifest = <<MANIFEST
 class external {
+  $trusted_json = inline_template('<%= @trusted.to_json %>')
   notify { 'trusted facts':
-    message => $::trusted
+    message => "#{SEPARATOR}${trusted_json}#{SEPARATOR}"
   }
 }
 MANIFEST
     create_remote_file(master, master_module_manifest, manifest)
     on(master, "chmod 644 '#{master_module_manifest}'")
   end
 
-  step "Step 3: Create site.pp to classify nodes to include module" do
+  step "create site.pp to classify nodes to include module" do
     site_pp_file = "#{environmentpath}/#{tmp_environment}/manifests/site.pp"
     site_pp      = <<-SITE_PP
 node default {
   include external
 }
-SITE_PP
+    SITE_PP
     create_remote_file(master, site_pp_file, site_pp)
     on(master, "chmod 644 '#{site_pp_file}'")
   end
 
-  step "Step 4: start the master" do
+  step "when trusted_external_command is a file" do
+    external_trusted_fact_script_path = "#{testdir}/external_facts.sh"
+
+    step "create the file" do
+      external_trusted_fact_script = <<EOF
+#!/bin/bash
+CERTNAME=$1
+printf '{"doot":"%s"}\n' "$CERTNAME"
+EOF
+      create_remote_file(master, external_trusted_fact_script_path, external_trusted_fact_script)
+      on(master, "chmod 777 #{external_trusted_fact_script_path}")
+    end
+
+    step "start the master and perform the test" do
+      master_opts = {
+        'main' => {
+          'trusted_external_command' => external_trusted_fact_script_path
+        }
+      }
+
+      with_puppet_running_on(master, master_opts) do
+        agents.each do |agent|
+          on(agent, puppet("agent", "-t", "--environment", tmp_environment), :acceptable_exit_codes => [0,2]) do |res|
+            trusted_hash = parse_trusted_json(res.stdout)
+            assert_includes(trusted_hash, 'external', "Trusted fact hash contains external key")
+            assert_equal(agent.to_s, trusted_hash['external']['doot'], "trusted facts contains certname")
+          end
+        end
+      end
+    end
+  end
+
+  step "when trusted_external_command is a directory" do
+    dir_path = "#{testdir}/commands"
+    executable_files = {
+      'no_extension' => <<EOF,
+#!/bin/bash
+CERTNAME=$1
+printf '{"no_extension_key":"%s"}\n' "$CERTNAME"
+EOF
+
+      'shell.sh' => <<EOF,
+#!/bin/bash
+CERTNAME=$1
+printf '{"shell_key":"%s"}\n' "$CERTNAME"
+EOF
+
+      'ruby.rb' => <<EOF,
+#!#{master[:privatebindir]}/ruby
+require 'json'
+CERTNAME=ARGV[0]
+data = { "ruby_key" => CERTNAME }
+print data.to_json
+EOF
+    }
+
+    step "create the directory" do
+      on(master, "mkdir #{dir_path}")
+      on(master, "chmod 755 #{dir_path}")
+
+      executable_files.each do |filename, content|
+        filepath = "#{dir_path}/#{filename}"
+        create_remote_file(master, filepath, content)
+        on(master, "chmod 777 #{filepath}")
+      end
+
+      # Create a non-executable file and an executable child-directory
+      # to ensure that these cases are skipped during external data
+      # retrieval
+
+      create_remote_file(master, "#{dir_path}/non_executable_file", "foo")
+
+      executable_child_dir = "#{dir_path}/child_dir"
+      on(master, "mkdir #{executable_child_dir}")
+      on(master, "chmod 777 #{executable_child_dir}")
+    end
+
     master_opts = {
       'main' => {
-        'trusted_external_command' => external_trusted_fact_script_path
+        'trusted_external_command' => dir_path
       }
     }
 
-    with_puppet_running_on(master, master_opts) do
-      agents.each do |agent|
-        on(agent, puppet("agent", "-t", "--environment", tmp_environment), :acceptable_exit_codes => [0,2]) do |res|
-          assert_match(/external/, res.stdout, "Trusted fact hash contains external key")
-          assert_match(/doot.*#{agent}/, res.stdout, "trusted facts contains certname")
+    step "start the master and perform the test" do
+      with_puppet_running_on(master, master_opts) do
+        agents.each do |agent|
+          on(agent, puppet("agent", "-t", "--environment", tmp_environment), :acceptable_exit_codes => [0,2]) do |res|
+            trusted_hash = parse_trusted_json(res.stdout)
+            assert_includes(trusted_hash, 'external', "Trusted fact hash contains external key")
+
+
+            external_keys = [
+              'no_extension',
+              'shell',
+              'ruby'
+            ]
+            assert_equal(external_keys.sort, trusted_hash['external'].keys.sort, "trusted['external'] does not contain <basename> keys of all executable files")
+
+            external_keys.each do |key|
+              expected_data = { "#{key}_key" => agent.to_s }
+              data = trusted_hash['external'][key]
+              assert_equal(expected_data, data, "trusted['external'][#{key}] does not contain #{key}'s data")
+            end
+          end
+        end
+      end
+    end
+
+    step "when there's more than executable <basename> script" do
+      step "create the conflicting file" do
+        filepath = "#{dir_path}/shell.rb"
+        create_remote_file(master, filepath, executable_files['shell.sh'])
+        on(master, "chmod 777 #{filepath}")
+      end
+
+      step "start the master and perform the test" do
+        with_puppet_running_on(master, master_opts) do
+          agents.each do |agent|
+            on(agent, puppet("agent", "-t", "--environment", tmp_environment), :acceptable_exit_codes => [1]) do |res|
+              assert_match(/.*shell.*#{Regexp.escape(dir_path)}/, res.stderr)
+            end
+          end
         end
       end
     end

lib/puppet/defaults.rb

@@ -590,13 +590,22 @@ def self.initialize_default_settings!(settings)
     },
     :trusted_external_command => {
       :default  => nil,
-      :desc     => "The external trusted facts script to use.
+      :type     => :file_or_directory,
+      :desc     => "The external trusted facts script or directory to use.
         This setting's value can be set to the path to an executable command that
-        can produce external trusted facts. The command must:
+        can produce external trusted facts or to a directory containing those
+        executable commands. The command(s) must:
 
         * Take the name of a node as a command-line argument.
         * Return a JSON hash with the external trusted facts for this node.
-        * For unknown or invalid nodes, exit with a non-zero exit code.",
+        * For unknown or invalid nodes, exit with a non-zero exit code.
+
+        If the setting points to an executable command, then the external trusted
+        facts will be stored in the 'external' key of the trusted facts hash. Otherwise
+        for each executable file in the directory, the external trusted facts will be
+        stored in the `<basename>` key of the `trusted['external']` hash. For example,
+        if the files foo.rb and bar.sh are in the directory, then `trusted['external']`
+        will be the hash `{ 'foo' => <foo.rb output>, 'bar' => <bar.sh output> }`.",
     },
     :default_file_terminus => {
       :type       => :terminus,

lib/puppet/trusted_external.rb

@@ -3,11 +3,39 @@ module Puppet::TrustedExternal
   def retrieve(certname)
     command = Puppet[:trusted_external_command]
     return nil unless command
+    Puppet.debug _("Retrieving trusted external data from %{command}") % {command: command}
+    setting_type = Puppet.settings.setting(:trusted_external_command).type
+    if setting_type == :file
+      return fetch_data(command, certname)
+    end
+    # command is a directory. Thus, data is a hash of <basename> => <data> for
+    # each executable file in command. For example, if the files 'servicenow.rb',
+    # 'unicorn.sh' are in command, then data is the following hash:
+    #   { 'servicenow' => <servicenow.rb output>, 'unicorn' => <unicorn.sh output> }
+    data = {}
+    Puppet::FileSystem.children(command).each do |file|
+      abs_path = Puppet::FileSystem.expand_path(file)
+      executable_file = Puppet::FileSystem.file?(abs_path) && Puppet::FileSystem.executable?(abs_path)
+      unless executable_file
+        Puppet.debug _("Skipping non-executable file %{file}")  % { file: abs_path }
+        next
+      end
+      basename = file.basename(file.extname).to_s
+      unless data[basename].nil?
+        raise Puppet::Error, _("There is more than one '%{basename}' script in %{dir}") % { basename: basename, dir: command }
+      end
+      data[basename] = fetch_data(abs_path, certname)
+    end
+    data
+  end
+  module_function :retrieve
+
+  def fetch_data(command, certname)
     result = Puppet::Util::Execution.execute([command, certname], {
       :combine => false,
       :failonfail => true,
     })
     JSON.parse(result)
   end
-  module_function :retrieve
+  module_function :fetch_data
 end

spec/unit/context/trusted_information_spec.rb

@@ -41,8 +41,12 @@
   }
 
   def allow_external_trusted_data(certname, data)
-    Puppet[:trusted_external_command] = '/usr/bin/generate_data.sh'
-    allow(Puppet::Util::Execution).to receive(:execute).with(['/usr/bin/generate_data.sh', certname], anything).and_return(JSON.dump(data))
+    command = 'generate_data.sh'
+    Puppet[:trusted_external_command] = command
+    # The expand_path bit is necessary b/c :trusted_external_command is a
+    # file_or_directory setting, and file_or_directory settings automatically
+    # expand the given path.
+    allow(Puppet::Util::Execution).to receive(:execute).with([File.expand_path(command), certname], anything).and_return(JSON.dump(data))
   end
 
   it "defaults external to an empty hash" do
@@ -99,9 +103,11 @@ def allow_external_trusted_data(certname, data)
     end
 
     it 'only runs the trusted external command the first time it is invoked' do
-      Puppet[:trusted_external_command] = '/usr/bin/generate_data.sh'
+      command = 'generate_data.sh'
+      Puppet[:trusted_external_command] = command
 
-      expect(Puppet::Util::Execution).to receive(:execute).with(['/usr/bin/generate_data.sh', 'cert name'], anything).and_return(JSON.dump(external_data)).once
+      # See allow_external_trusted_data to understand why expand_path is necessary
+      expect(Puppet::Util::Execution).to receive(:execute).with([File.expand_path(command), 'cert name'], anything).and_return(JSON.dump(external_data)).once
 
       trusted = Puppet::Context::TrustedInformation.remote(true, 'cert name', cert)
       trusted.external