From b50b6ca4f8a7fc1f36a3000eba940a0ca6dbdfa9 Mon Sep 17 00:00:00 2001 From: Hans Moulron Date: Tue, 6 Aug 2024 17:32:59 +0200 Subject: [PATCH] add onlyif && unless features --- Gemfile | 1 + REFERENCE.md | 88 ++++++ lib/puppet/provider/archive/ruby.rb | 142 +++++++++- lib/puppet/type/archive.rb | 262 +++++++++++++++++- manifests/nexus.pp | 16 +- spec/lib/puppet_spec/compiler.rb | 112 ++++++++ spec/lib/puppet_spec/files.rb | 107 +++++++ spec/spec_helper.rb | 9 + .../unit/puppet/provider/archive/ruby_spec.rb | 101 ++++++- spec/unit/puppet/type/archive_spec.rb | 165 +++++++++++ 10 files changed, 971 insertions(+), 32 deletions(-) create mode 100644 spec/lib/puppet_spec/compiler.rb create mode 100644 spec/lib/puppet_spec/files.rb diff --git a/Gemfile b/Gemfile index 7123c663..b84cf36b 100644 --- a/Gemfile +++ b/Gemfile @@ -23,6 +23,7 @@ group :release do gem 'voxpupuli-release', '~> 3.0', :require => false end +gem 'pry' gem 'rake', :require => false gem 'facter', ENV['FACTER_GEM_VERSION'], :require => false, :groups => [:test] diff --git a/REFERENCE.md b/REFERENCE.md index 1a842069..2d69cc0a 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -632,6 +632,11 @@ Parameters Examples -------- +#### Examples + +##### + +```puppet archive::nexus { '/tmp/jtstand-ui-0.98.jar': url => 'https://oss.sonatype.org', gav => 'org.codehaus.jtstand:jtstand-ui:0.98', @@ -639,6 +644,7 @@ archive::nexus { '/tmp/jtstand-ui-0.98.jar': packaging => 'jar', extract => false, } +``` #### Parameters @@ -895,6 +901,72 @@ whether archive file should be present/absent (default: present) Default value: `present` +##### `onlyif` + +A test command that checks the state of the target system and restricts +when the `archive` can run. If present, Puppet runs this test command +first, and only runs the main command if the test has an exit code of 0 +(success). For example: + + ``` + archive { '/tmp/jta-1.1.jar': + ensure => present, + extract => true, + extract_path => '/tmp', + source => 'http://central.maven.org/maven2/javax/transaction/jta/1.1/jta-1.1.jar', + onlyif => 'test `java -version 2>&1 | awk -F '"' '/version/ {print $2}' | awk -F '.' '{sub("^$", "0", $2); print $1$2}'` -gt 15', + cleanup => true, + env_path => ["/bin", "/usr/bin", "/sbin", "/usr/sbin"], + } + ``` + +Since this command is used in the process of determining whether the +`archive` is already in sync, it must be run during a noop Puppet run. + +This parameter can also take an array of commands. For example: + + onlyif => ['test -f /tmp/file1', 'test -f /tmp/file2'], + +or an array of arrays. For example: + + onlyif => [['test', '-f', '/tmp/file1'], 'test -f /tmp/file2'] + +This `archive` would only run if every command in the array has an +exit code of 0 (success). + +##### `unless` + +A test command that checks the state of the target system and restricts +when the `archive` can run. If present, Puppet runs this test command +first, then runs the main command unless the test has an exit code of 0 +(success). For example: + + ``` + archive { '/tmp/jta-1.1.jar': + ensure => present, + extract => true, + extract_path => '/tmp', + source => 'http://central.maven.org/maven2/javax/transaction/jta/1.1/jta-1.1.jar', + unless => 'test `java -version 2>&1 | awk -F '"' '/version/ {print $2}' | awk -F '.' '{sub("^$", "0", $2); print $1$2}'` -gt 15', + cleanup => true, + env_path => ["/bin", "/usr/bin", "/sbin", "/usr/sbin"], + } + ``` + +Since this command is used in the process of determining whether the +`archive` is already in sync, it must be run during a noop Puppet run. + +This parameter can also take an array of commands. For example: + + unless => ['test -f /tmp/file1', 'test -f /tmp/file2'], + +or an array of arrays. For example: + + unless => [['test', '-f', '/tmp/file1'], 'test -f /tmp/file2'] + +This `archive` would only run if every command in the array has a +non-zero exit code. + #### Parameters The following parameters are available in the `archive` type. @@ -910,6 +982,8 @@ The following parameters are available in the `archive` type. * [`digest_type`](#-archive--digest_type) * [`digest_url`](#-archive--digest_url) * [`download_options`](#-archive--download_options) +* [`env_path`](#-archive--env_path) +* [`environment`](#-archive--environment) * [`extract`](#-archive--extract) * [`extract_command`](#-archive--extract_command) * [`extract_flags`](#-archive--extract_flags) @@ -998,6 +1072,20 @@ archive file checksum source (instead of specifying checksum) provider download options (affects curl, wget, gs, and only s3 downloads for ruby provider) +##### `env_path` + +The search path used for check execution. +Commands must be fully qualified if no path is specified. Paths +can be specified as an array or as a ' + +##### `environment` + +An array of any additional environment variables you want to set for a +command, such as `[ 'HOME=/root', 'MAIL=root@example.com']`. +Note that if you use this to set PATH, it will override the `path` +attribute. Multiple environment variables should be specified as an +array. + ##### `extract` Valid values: `true`, `false` diff --git a/lib/puppet/provider/archive/ruby.rb b/lib/puppet/provider/archive/ruby.rb index c83c2e41..1ec7eaad 100644 --- a/lib/puppet/provider/archive/ruby.rb +++ b/lib/puppet/provider/archive/ruby.rb @@ -5,7 +5,9 @@ require 'securerandom' require 'tempfile' +require 'puppet/util/execution' +require 'pry' # This provider implements a simple state-machine. The following attempts to # # document it. In general, `def adjective?` implements a [state], while `def # verb` implements an {action}. @@ -59,6 +61,7 @@ # Puppet::Type.type(:archive).provide(:ruby) do + include Puppet::Util::Execution optional_commands aws: 'aws' optional_commands gsutil: 'gsutil' defaultfor feature: :microsoft_windows @@ -95,18 +98,6 @@ def tempfile_name end end - def creates - if resource[:extract] == :true - extracted? ? resource[:creates] : 'archive not extracted' - else - resource[:creates] - end - end - - def creates=(_value) - extract - end - def checksum resource[:checksum] || (resource[:checksum] = remote_checksum if resource[:checksum_url]) end @@ -127,7 +118,7 @@ def remote_checksum # returns boolean def checksum?(store_checksum = true) return false unless File.exist? archive_filepath - return true if resource[:checksum_type] == :none + return true if resource[:checksum_type] == :none archive = PuppetX::Bodeco::Archive.new(archive_filepath) archive_checksum = archive.checksum(resource[:checksum_type]) @@ -156,7 +147,7 @@ def extract end def extracted? - resource[:creates] && File.exist?(resource[:creates]) + resource.check_all_attributes end def transfer_download(archive_filepath) @@ -258,4 +249,127 @@ def optional_switch(value, option) [] end end + + # Verify that we have the executable + def checkexe(command) + exe = extractexe(command) + if Facter.value(:osfamily) == 'windows' + if absolute_path?(exe) + if !Puppet::FileSystem.exist?(exe) + raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe } + elsif !File.file?(exe) + raise ArgumentError, _("'%{exe}' is a %{klass}, not a file") % { exe: exe, klass: File.ftype(exe) } + end + end + else + if File.expand_path(exe) == exe + if !Puppet::FileSystem.exist?(exe) + raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe } + elsif !File.file?(exe) + raise ArgumentError, _("'%{exe}' is a %{klass}, not a file") % { exe: exe, klass: File.ftype(exe) } + elsif !File.executable?(exe) + raise ArgumentError, _("'%{exe}' is not executable") % { exe: exe } + end + end + end + + if resource[:env_path] + Puppet::Util.withenv :PATH => resource[:env_path].join(File::PATH_SEPARATOR) do + return if which(exe) + end + end + + # 'which' will only return the command if it's executable, so we can't + # distinguish not found from not executable + raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe } + end + def environment + env = {} + + if (path = resource[:env_path]) + env[:PATH] = path.join(File::PATH_SEPARATOR) + end + + return env unless (envlist = resource[:environment]) + + envlist = [envlist] unless envlist.is_a? Array + envlist.each do |setting| + unless (match = /^(\w+)=((.|\n)*)$/.match(setting)) + warning _("Cannot understand environment setting %{setting}") % { setting: setting.inspect } + next + end + var = match[1] + value = match[2] + + if env.include?(var) || env.include?(var.to_sym) + warning _("Overriding environment setting '%{var}' with '%{value}'") % { var: var, value: value } + end + + if value.nil? || value.empty? + msg = _("Empty environment setting '%{var}'") % { var: var } + Puppet.warn_once('undefined_variables', "empty_env_var_#{var}", msg, resource.file, resource.line) + end + + env[var] = value + end + + env + end + + def run(command, check = false) + output = nil + checkexe(command) + + debug "Executing#{check ? " check": ""} #{command}" + + cwd = resource[:extract] ? resource[:extract_path] : File.dirname(resource[:path]) + # It's ok if cwd is nil. In that case Puppet::Util::Execution.execute() simply will not attempt to + # change the working directory, which is exactly the right behavior when no cwd parameter is + # expressed on the resource. Moreover, attempting to change to the directory that is already + # the working directory can fail under some circumstances, so avoiding the directory change attempt + # is preferable to defaulting cwd to that directory. + + # note that we are passing "false" for the "override_locale" parameter, which ensures that the user's + # default/system locale will be respected. Callers may override this behavior by setting locale-related + # environment variables (LANG, LC_ALL, etc.) in their 'environment' configuration. + output = Puppet::Util::Execution.execute( + command, + failonfail: false, + combine: true, + cwd: cwd, + uid: resource[:user], + gid: resource[:group], + override_locale: false, + custom_environment: environment, + sensitive: false + ) + # The shell returns 127 if the command is missing. + if output.exitstatus == 127 + raise ArgumentError, output + end + # Return output twice as processstatus was returned before, but only exitstatus was ever called. + # Output has the exitstatus on it so it is returned instead. This is here twice as changing this + # would result in a change to the underlying API. + [output, output] + end + + def extractexe(command) + if command.is_a? Array + command.first + else + match = /^"([^"]+)"|^'([^']+)'/.match(command) + if match + # extract whichever of the two sides matched the content. + match[1] or match[2] + else + command.split(/ /)[0] + end + end + end + + def validatecmd(command) + exe = extractexe(command) + # if we're not fully qualified, require a path + self.fail _("'%{exe}' is not qualified and no path was specified. Please qualify the command or specify a path.") % { exe: exe } if !absolute_path?(exe) and resource[:path].nil? + end end diff --git a/lib/puppet/type/archive.rb b/lib/puppet/type/archive.rb index 6535e86b..467eebfb 100644 --- a/lib/puppet/type/archive.rb +++ b/lib/puppet/type/archive.rb @@ -4,10 +4,24 @@ require 'uri' require 'puppet/util' require 'puppet/parameter/boolean' +require 'pry' Puppet::Type.newtype(:archive) do @doc = 'Manage archive file download, extraction, and cleanup.' + # Create a new check mechanism. It's basically a parameter that + # provides one extra 'check' method. + def self.newcheck(name, options = {}, &block) + @checks ||= {} + + check = newparam(name, options, &block) + @checks[name] = check + end + + def self.checks + @checks.keys + end + ensurable do desc 'whether archive file should be present/absent (default: present)' @@ -103,14 +117,6 @@ def change_to_s(currentvalue, newvalue) defaultto(:undef) end - newproperty(:creates) do - desc 'if file/directory exists, will not download/extract archive.' - - def should_to_s(value) - "extracting in #{resource[:extract_path]} to create #{value}" - end - end - newparam(:cleanup) do desc 'whether archive file will be removed after extraction (true|false).' newvalues(:true, :false) @@ -251,6 +257,176 @@ def should_to_s(value) end end + newparam(:env_path) do + desc "The search path used for check execution. + Commands must be fully qualified if no path is specified. Paths + can be specified as an array or as a '#{File::PATH_SEPARATOR}' separated list." + + # Support both arrays and colon-separated fields. + def value=(*values) + @value = values.flatten.collect { |val| + val.split(File::PATH_SEPARATOR) + }.flatten + end + end + + newparam(:environment) do + desc "An array of any additional environment variables you want to set for a + command, such as `[ 'HOME=/root', 'MAIL=root@example.com']`. + Note that if you use this to set PATH, it will override the `path` + attribute. Multiple environment variables should be specified as an + array." + + validate do |values| + values = [values] unless values.is_a? Array + values.each do |value| + unless value =~ /\w+=/ + raise ArgumentError, _("Invalid environment setting '%{value}'") % { value: value } + end + end + end + end + + newcheck(:creates, :parent => Puppet::Parameter::Path) do + desc 'if file/directory exists, will not download/extract archive.' + + accept_arrays + + # If the file exists, return false (i.e., don't run the command), + # else return true + def check(value) + #TRANSLATORS 'creates' is a parameter name and should not be translated + debug(_("Checking that 'creates' path '%{creates_path}' exists") % { creates_path: value }) + Puppet::FileSystem.exist?(value) + end + end + + newcheck(:unless) do + desc <<-'EOT' + A test command that checks the state of the target system and restricts + when the `archive` can run. If present, Puppet runs this test command + first, then runs the main command unless the test has an exit code of 0 + (success). For example: + + ``` + archive { '/tmp/jta-1.1.jar': + ensure => present, + extract => true, + extract_path => '/tmp', + source => 'http://central.maven.org/maven2/javax/transaction/jta/1.1/jta-1.1.jar', + unless => 'test `java -version 2>&1 | awk -F '"' '/version/ {print $2}' | awk -F '.' '{sub("^$", "0", $2); print $1$2}'` -gt 15', + cleanup => true, + env_path => ["/bin", "/usr/bin", "/sbin", "/usr/sbin"], + } + ``` + + Since this command is used in the process of determining whether the + `archive` is already in sync, it must be run during a noop Puppet run. + + This parameter can also take an array of commands. For example: + + unless => ['test -f /tmp/file1', 'test -f /tmp/file2'], + + or an array of arrays. For example: + + unless => [['test', '-f', '/tmp/file1'], 'test -f /tmp/file2'] + + This `archive` would only run if every command in the array has a + non-zero exit code. + EOT + + validate do |cmds| + cmds = [cmds] unless cmds.is_a? Array + + cmds.each do |command| + provider.validatecmd(command) + end + end + + # Return true if the command does not return 0. + def check(value) + begin + output, status = provider.run(value, true) + rescue Timeout::Error + err _("Check %{value} exceeded timeout") % { value: value.inspect } + return false + end + + if sensitive + self.debug("[output redacted]") + else + output.split(/\n/).each { |line| + self.debug(line) + } + end + + status.exitstatus != 0 + end + end + + newcheck(:onlyif) do + desc <<-'EOT' + A test command that checks the state of the target system and restricts + when the `archive` can run. If present, Puppet runs this test command + first, and only runs the main command if the test has an exit code of 0 + (success). For example: + + ``` + archive { '/tmp/jta-1.1.jar': + ensure => present, + extract => true, + extract_path => '/tmp', + source => 'http://central.maven.org/maven2/javax/transaction/jta/1.1/jta-1.1.jar', + onlyif => 'test `java -version 2>&1 | awk -F '"' '/version/ {print $2}' | awk -F '.' '{sub("^$", "0", $2); print $1$2}'` -gt 15', + cleanup => true, + env_path => ["/bin", "/usr/bin", "/sbin", "/usr/sbin"], + } + ``` + + Since this command is used in the process of determining whether the + `archive` is already in sync, it must be run during a noop Puppet run. + + This parameter can also take an array of commands. For example: + + onlyif => ['test -f /tmp/file1', 'test -f /tmp/file2'], + + or an array of arrays. For example: + + onlyif => [['test', '-f', '/tmp/file1'], 'test -f /tmp/file2'] + + This `archive` would only run if every command in the array has an + exit code of 0 (success). + EOT + + validate do |cmds| + cmds = [cmds] unless cmds.is_a? Array + + cmds.each do |command| + provider.validatecmd(command) + end + end + + # Return true if the command returns 0. + def check(value) + begin + output, status = provider.run(value, true) + rescue Timeout::Error + err _("Check %{value} exceeded timeout") % { value: value.inspect } + return false + end + + if sensitive + self.debug("[output redacted]") + else + output.split(/\n/).each { |line| + self.debug(line) + } + end + + status.exitstatus == 0 + end + end + autorequire(:file) do [ Pathname.new(self[:path]).parent.to_s, @@ -280,4 +456,74 @@ def should_to_s(value) self[:proxy_type] = :none end end + + # Verify that we pass all of the checks. The argument determines whether + # we skip the :refreshonly check, which is necessary because we now check + # within refresh + def check_all_attributes(refreshing = false) + self.class.checks.each { |check| + + next unless @parameters.include?(check) + + val = @parameters[check].value + val = [val] unless val.is_a? Array + val.each do |value| + next if @parameters[check].check(value) + + # Give a debug message so users can figure out what command would have been + # but don't print sensitive commands or parameters in the clear + sourcestring = @parameters[:source].sensitive ? "[command redacted]" : @parameters[:source].value + + debug(_("'%{source}' won't be executed because of failed check '%{check}'") % { source: sourcestring, check: check }) + + return false + end + } + true + end + + def output + if property(:returns).nil? + nil + else + property(:returns).output + end + end + + # Run the command, or optionally run a separately-specified command. + def refresh + if check_all_attributes(true) + cmd = self[:refresh] + if cmd + provider.run(cmd) + else + property(:returns).sync + end + end + end + + private + + def set_sensitive_parameters(sensitive_parameters) + # If any are sensitive, mark all as sensitive + sensitive = false + parameters_to_check = [:command, :unless, :onlyif] + + parameters_to_check.each do |p| + if sensitive_parameters.include?(p) + sensitive_parameters.delete(p) + sensitive = true + end + end + + if sensitive + parameters_to_check.each do |p| + if parameters.include?(p) + parameter(p).sensitive = true + end + end + end + + super(sensitive_parameters) + end end diff --git a/manifests/nexus.pp b/manifests/nexus.pp index 2a9395b8..312e63b7 100644 --- a/manifests/nexus.pp +++ b/manifests/nexus.pp @@ -9,14 +9,14 @@ # # Examples # -------- -# -# archive::nexus { '/tmp/jtstand-ui-0.98.jar': -# url => 'https://oss.sonatype.org', -# gav => 'org.codehaus.jtstand:jtstand-ui:0.98', -# repository => 'codehaus-releases', -# packaging => 'jar', -# extract => false, -# } +# @example +# archive::nexus { '/tmp/jtstand-ui-0.98.jar': +# url => 'https://oss.sonatype.org', +# gav => 'org.codehaus.jtstand:jtstand-ui:0.98', +# repository => 'codehaus-releases', +# packaging => 'jar', +# extract => false, +# } # define archive::nexus ( String $url, diff --git a/spec/lib/puppet_spec/compiler.rb b/spec/lib/puppet_spec/compiler.rb new file mode 100644 index 00000000..bbecf804 --- /dev/null +++ b/spec/lib/puppet_spec/compiler.rb @@ -0,0 +1,112 @@ +module PuppetSpec::Compiler + module_function + + def compile_to_catalog(string, node = Puppet::Node.new('test')) + Puppet[:code] = string + # see lib/puppet/indirector/catalog/compiler.rb#filter + Puppet::Parser::Compiler.compile(node).filter { |r| r.virtual? } + end + + # Does not removed virtual resources in compiled catalog (i.e. keeps unrealized) + def compile_to_catalog_unfiltered(string, node = Puppet::Node.new('test')) + Puppet[:code] = string + # see lib/puppet/indirector/catalog/compiler.rb#filter + Puppet::Parser::Compiler.compile(node) + end + + def compile_to_ral(manifest, node = Puppet::Node.new('test')) + catalog = compile_to_catalog(manifest, node) + ral = catalog.to_ral + ral.finalize + ral + end + + def compile_to_relationship_graph(manifest, prioritizer = Puppet::Graph::SequentialPrioritizer.new) + ral = compile_to_ral(manifest) + graph = Puppet::Graph::RelationshipGraph.new(prioritizer) + graph.populate_from(ral) + graph + end + + def apply_compiled_manifest(manifest, prioritizer = Puppet::Graph::SequentialPrioritizer.new) + catalog = compile_to_ral(manifest) + if block_given? + catalog.resources.each { |res| yield res } + end + transaction = Puppet::Transaction.new(catalog, + Puppet::Transaction::Report.new, + prioritizer) + transaction.evaluate + transaction.report.finalize_report + + transaction + end + + def apply_with_error_check(manifest) + apply_compiled_manifest(manifest) do |res| + expect(res).not_to receive(:err) + end + end + + def order_resources_traversed_in(relationships) + order_seen = [] + relationships.traverse { |resource| order_seen << resource.ref } + order_seen + end + + def collect_notices(code, node = Puppet::Node.new('foonode')) + Puppet[:code] = code + compiler = Puppet::Parser::Compiler.new(node) + node.environment.check_for_reparse + logs = [] + Puppet::Util::Log.with_destination(Puppet::Test::LogCollector.new(logs)) do + yield(compiler) + end + logs = logs.select { |log| log.level == :notice }.map { |log| log.message } + logs + end + + def eval_and_collect_notices(code, node = Puppet::Node.new('foonode'), topscope_vars = {}) + collect_notices(code, node) do |compiler| + unless topscope_vars.empty? + scope = compiler.topscope + topscope_vars.each {|k,v| scope.setvar(k, v) } + end + if block_given? + compiler.compile do |catalog| + yield(compiler.topscope, catalog) + catalog + end + else + compiler.compile + end + end + end + + # Compiles a catalog, and if source is given evaluates it and returns its result. + # The catalog is returned if no source is given. + # Topscope variables are set before compilation + # Uses a created node 'testnode' if none is given. + # (Parameters given by name) + # + def evaluate(code: 'undef', source: nil, node: Puppet::Node.new('testnode'), variables: {}) + source_location = caller[0] + Puppet[:code] = code + compiler = Puppet::Parser::Compiler.new(node) + unless variables.empty? + scope = compiler.topscope + variables.each {|k,v| scope.setvar(k, v) } + end + + if source.nil? + compiler.compile + # see lib/puppet/indirector/catalog/compiler.rb#filter + return compiler.filter { |r| r.virtual? } + end + + # evaluate given source is the context of the compiled state and return its result + compiler.compile do |catalog | + Puppet::Pops::Parser::EvaluatingParser.singleton.evaluate_string(compiler.topscope, source, source_location) + end + end +end diff --git a/spec/lib/puppet_spec/files.rb b/spec/lib/puppet_spec/files.rb new file mode 100644 index 00000000..fa9726ba --- /dev/null +++ b/spec/lib/puppet_spec/files.rb @@ -0,0 +1,107 @@ +require 'fileutils' +require 'tempfile' +require 'tmpdir' +require 'pathname' + +# A support module for testing files. +module PuppetSpec::Files + def self.cleanup + $global_tempfiles ||= [] + while path = $global_tempfiles.pop do + begin + FileUtils.rm_rf path, :secure => true + rescue Errno::ENOENT + # nothing to do + end + end + end + + module_function + + def make_absolute(path) + path = File.expand_path(path) + path[0] = 'c' if Puppet::Util::Platform.windows? + path + end + + def tmpfile(name, dir = nil) + dir ||= Dir.tmpdir + path = Puppet::FileSystem.expand_path(make_tmpname(name, nil).encode(Encoding::UTF_8), dir) + record_tmp(File.expand_path(path)) + + path + end + + def file_containing(name, contents) + file = tmpfile(name) + File.open(file, 'wb') { |f| f.write(contents) } + file + end + + def script_containing(name, contents) + file = tmpfile(name) + if Puppet::Util::Platform.windows? + file += '.bat' + text = contents[:windows] + else + text = contents[:posix] + end + File.open(file, 'wb') { |f| f.write(text) } + Puppet::FileSystem.chmod(0755, file) + file + end + + def tmpdir(name) + dir = Puppet::FileSystem.expand_path(Dir.mktmpdir(name).encode!(Encoding::UTF_8)) + + record_tmp(dir) + + dir + end + + # Copied from ruby 2.4 source + def make_tmpname((prefix, suffix), n) + prefix = (String.try_convert(prefix) or + raise ArgumentError, "unexpected prefix: #{prefix.inspect}") + suffix &&= (String.try_convert(suffix) or + raise ArgumentError, "unexpected suffix: #{suffix.inspect}") + t = Time.now.strftime("%Y%m%d") + path = "#{prefix}#{t}-#{$$}-#{rand(0x100000000).to_s(36)}".dup + path << "-#{n}" if n + path << suffix if suffix + path + end + + def dir_containing(name, contents_hash) + dir_contained_in(tmpdir(name), contents_hash) + end + + def dir_contained_in(dir, contents_hash) + contents_hash.each do |k,v| + if v.is_a?(Hash) + Dir.mkdir(tmp = File.join(dir,k)) + dir_contained_in(tmp, v) + else + file = File.join(dir, k) + File.open(file, 'wb') {|f| f.write(v) } + end + end + dir + end + + def record_tmp(tmp) + # ...record it for cleanup, + $global_tempfiles ||= [] + $global_tempfiles << tmp + end + + def expect_file_mode(file, mode) + actual_mode = "%o" % Puppet::FileSystem.stat(file).mode + target_mode = if Puppet::Util::Platform.windows? + mode + else + "10" + "%04i" % mode.to_i + end + expect(actual_mode).to eq(target_mode) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ff92c7a2..caf3fd58 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,8 +7,17 @@ # We want to do this if lib exists and it hasn't been explicitly set. ENV['COVERAGE'] ||= 'yes' if Dir.exist?(File.expand_path('../lib', __dir__)) +dir = File.expand_path(File.dirname(__FILE__)) +$LOAD_PATH.unshift File.join(dir, 'lib') + require 'voxpupuli/test/spec_helper' +# So everyone else doesn't have to include this base constant. +module PuppetSpec + FIXTURE_DIR = File.join(File.expand_path(File.dirname(__FILE__)), "fixtures") unless defined?(FIXTURE_DIR) +end + + RSpec.configure do |c| c.facterdb_string_keys = true end diff --git a/spec/unit/puppet/provider/archive/ruby_spec.rb b/spec/unit/puppet/provider/archive/ruby_spec.rb index 532f4b7a..300248e7 100644 --- a/spec/unit/puppet/provider/archive/ruby_spec.rb +++ b/spec/unit/puppet/provider/archive/ruby_spec.rb @@ -2,10 +2,13 @@ # rubocop:disable RSpec/MultipleMemoizedHelpers require 'spec_helper' +require 'puppet_spec/compiler' +require 'puppet_spec/files' ruby_provider = Puppet::Type.type(:archive).provider(:ruby) - RSpec.describe ruby_provider do + include PuppetSpec::Compiler + include PuppetSpec::Files it_behaves_like 'an archive provider', ruby_provider describe 'ruby provider' do @@ -13,7 +16,7 @@ let(:resource_properties) do { name: name, - source: 's3://home.lan/example.zip' + source: 's3://home.lan/example.zip', } end let(:resource) { Puppet::Type::Archive.new(resource_properties) } @@ -129,6 +132,100 @@ expect { provider.transfer_download(name) }.to raise_error(Puppet::Error, %r{Download file checksum mismatch}) end end + + context "when handling checks", unless: Puppet::Util::Platform.jruby? do + before :each do + Puppet[:log_level] = 'debug' + end + + let(:onlyifsecret) { "onlyifsecret" } + let(:unlesssecret) { "unlesssecret" } + let(:supersecret) { 'supersecret' } + let(:path) do + if Puppet::Util::Platform.windows? + # The `apply_compiled_manifest` helper doesn't add the `path` fact, so + # we can't reference that in our manifest. Windows PATHs can contain + # double quotes and trailing backslashes, which confuse HEREDOC + # interpolation below. So sanitize it: + ENV['PATH'].split(File::PATH_SEPARATOR) + .map { |dir| dir.gsub(/"/, '\"').gsub(/\\$/, '') } + .map { |dir| Pathname.new(dir).cleanpath.to_s } + .join(File::PATH_SEPARATOR) + else + ENV['PATH'] + end + end + + def ruby_exit_0 + "ruby -e 'exit 0'" + end + + def echo_from_ruby_exit_0(message) + # Escape double quotes due to HEREDOC interpolation below + "ruby -e 'puts \"#{message}\"; exit 0'".gsub(/"/, '\"') + end + + def echo_from_ruby_exit_1(message) + # Escape double quotes due to HEREDOC interpolation below + "ruby -e 'puts \"#{message}\"; exit 1'".gsub(/"/, '\"') + end + + it "redacts command and onlyif outputs" do + onlyif = echo_from_ruby_exit_0(onlyifsecret) + + apply_compiled_manifest(<<-MANIFEST) + archive { '/tmp/favicon.ico': + ensure => present, + source => 'https://www.google.com/favicon.ico', + onlyif => "#{onlyif}", + env_path => "#{path}", + } + MANIFEST + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "Executing check ruby -e 'puts \"onlyifsecret\"; exit 0'", source: /Archive\[\/tmp\/favicon.ico\]/)) + end + + it "redacts the command that would have been executed but didn't due to onlyif" do + onlyif = echo_from_ruby_exit_1(onlyifsecret) + + apply_compiled_manifest(<<-MANIFEST) + archive { '/tmp/favicon.ico': + ensure => present, + source => 'https://www.google.com/favicon.ico', + onlyif => "#{onlyif}", + env_path => "#{path}", + } + MANIFEST + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "'https://www.google.com/favicon.ico' won't be executed because of failed check 'onlyif'")) + end + + it "redacts command and unless outputs" do + unlesscmd = echo_from_ruby_exit_1(unlesssecret) + + apply_compiled_manifest(<<-MANIFEST) + archive { '/tmp/favicon.ico': + ensure => present, + source => 'https://www.google.com/favicon.ico', + unless => "#{unlesscmd}", + env_path => "#{path}", + } + MANIFEST + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "Executing check ruby -e 'puts \"unlesssecret\"; exit 1'", source: /Archive\[\/tmp\/favicon.ico\]/)) + end + + it "redacts the command that would have been executed but didn't due to unless" do + unlesscmd = echo_from_ruby_exit_0(unlesssecret) + + apply_compiled_manifest(<<-MANIFEST) + archive { '/tmp/favicon.ico': + ensure => present, + source => 'https://www.google.com/favicon.ico', + unless => "#{unlesscmd}", + env_path => "#{path}", + } + MANIFEST + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "'https://www.google.com/favicon.ico' won't be executed because of failed check 'unless'")) + end + end end end # rubocop:enable RSpec/MultipleMemoizedHelpers diff --git a/spec/unit/puppet/type/archive_spec.rb b/spec/unit/puppet/type/archive_spec.rb index e249be08..9566cfa0 100644 --- a/spec/unit/puppet/type/archive_spec.rb +++ b/spec/unit/puppet/type/archive_spec.rb @@ -2,8 +2,10 @@ require 'spec_helper' require 'puppet' +require 'puppet_spec/files' describe Puppet::Type.type(:archive) do + include PuppetSpec::Files let(:resource) do Puppet::Type.type(:archive).new( path: '/tmp/example.zip', @@ -134,6 +136,169 @@ end.not_to raise_error end + describe "#check" do + describe ":creates" do + before :each do + @exist = tmpfile('exist') + FileUtils.touch(@exist) + @unexist = tmpfile('unexist') + end + + context "with a single item" do + it "should run when the item does not exist" do + resource[:creates] = @unexist + expect(resource.check_all_attributes).to eq(false) + end + + it "should not run when the item exists" do + resource[:creates] = @exist + expect(resource.check_all_attributes).to eq(true) + end + end + + context "with an array with one item" do + it "should run when the item does not exist" do + resource[:creates] = [@unexist] + expect(resource.check_all_attributes).to eq(false) + end + + it "should not run when the item exists" do + resource[:creates] = [@exist] + expect(resource.check_all_attributes).to eq(true) + end + + it "should not run when all items exist" do + resource[:creates] = [@exist] * 3 + end + + context "when creates is being checked" do + it "should be logged to debug when the path does exist" do + Puppet::Util::Log.level = :debug + resource[:creates] = @exist + expect(resource.check_all_attributes).to eq(true) + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "Checking that 'creates' path '#{@exist}' exists")) + end + + it "should be logged to debug when the path does not exist" do + Puppet::Util::Log.level = :debug + resource[:creates] = @unexist + expect(resource.check_all_attributes).to eq(false) + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "Checking that 'creates' path '#{@unexist}' exists")) + end + end + end + end + + + { :onlyif => { :pass => false, :fail => true }, + :unless => { :pass => true, :fail => false }, + }.each do |param, sense| + describe ":#{param}" do + before :each do + @pass = make_absolute("/magic/pass") + @fail = make_absolute("/magic/fail") + + @pass_status = double('status', :exitstatus => sense[:pass] ? 0 : 1) + @fail_status = double('status', :exitstatus => sense[:fail] ? 0 : 1) + + allow(resource.provider).to receive(:checkexe).and_return(true) + [true, false].each do |check| + allow(resource.provider).to receive(:run).with(@pass, check). + and_return(['test output', @pass_status]) + allow(resource.provider).to receive(:run).with(@fail, check). + and_return(['test output', @fail_status]) + end + end + + context "with a single item" do + it "should run if the command exits non-zero" do + resource[param] = @fail + expect(resource.check_all_attributes).to eq(true) + end + + it "should not run if the command exits zero" do + resource[param] = @pass + expect(resource.check_all_attributes).to eq(false) + end + end + + context "with an array with a single item" do + it "should run if the command exits non-zero" do + resource[param] = [@fail] + expect(resource.check_all_attributes).to eq(true) + end + + it "should not run if the command exits zero" do + resource[param] = [@pass] + expect(resource.check_all_attributes).to eq(false) + end + end + + context "with an array with multiple items" do + it "should run if all the commands exits non-zero" do + resource[param] = [@fail] * 3 + expect(resource.check_all_attributes).to eq(true) + end + + it "should not run if one command exits zero" do + resource[param] = [@pass, @fail, @pass] + expect(resource.check_all_attributes).to eq(false) + end + + it "should not run if all command exits zero" do + resource[param] = [@pass] * 3 + expect(resource.check_all_attributes).to eq(false) + end + end + + context 'with an array of arrays with multiple items' do + before do + [true, false].each do |check| + allow(resource.provider).to receive(:run).with([@pass, '--flag'], check). + and_return(['test output', @pass_status]) + allow(resource.provider).to receive(:run).with([@fail, '--flag'], check). + and_return(['test output', @fail_status]) + allow(resource.provider).to receive(:run).with([@pass], check). + and_return(['test output', @pass_status]) + allow(resource.provider).to receive(:run).with([@fail], check). + and_return(['test output', @fail_status]) + end + end + it "runs if all the commands exits non-zero" do + resource[param] = [[@fail, '--flag'], [@fail], [@fail, '--flag']] + expect(resource.check_all_attributes).to eq(true) + end + + it "does not run if one command exits zero" do + resource[param] = [[@pass, '--flag'], [@pass], [@fail, '--flag']] + expect(resource.check_all_attributes).to eq(false) + end + + it "does not run if all command exits zero" do + resource[param] = [[@pass, '--flag'], [@pass], [@pass, '--flag']] + expect(resource.check_all_attributes).to eq(false) + end + end + + it "should emit output to debug" do + Puppet::Util::Log.level = :debug + resource[param] = @fail + expect(resource.check_all_attributes).to eq(true) + expect(@logs.shift.message).to eq("test output") + end + + it "should not emit output to debug if sensitive is true" do + Puppet::Util::Log.level = :debug + resource[param] = @fail + allow(resource.parameters[param]).to receive(:sensitive).and_return(true) + expect(resource.check_all_attributes).to eq(true) + expect(@logs).not_to include(an_object_having_attributes(level: :debug, message: "test output")) + expect(@logs).to include(an_object_having_attributes(level: :debug, message: "[output redacted]")) + end + end + end + end + describe 'archive autorequire' do let(:file_resource) { Puppet::Type.type(:file).new(name: '/tmp') } let(:archive_resource) do