diff options
| -rw-r--r-- | lib/chef/mixin/which.rb | 33 | ||||
| -rw-r--r-- | lib/chef/provider/package.rb | 2 | ||||
| -rw-r--r-- | lib/chef/provider/package/dnf/dnf_helper.py | 2 | ||||
| -rw-r--r-- | lib/chef/provider/package/dnf/python_helper.rb | 83 | ||||
| -rw-r--r-- | spec/unit/mixin/which.rb | 160 | ||||
| -rw-r--r-- | spec/unit/provider/package/dnf/python_helper_spec.rb | 29 | ||||
| -rw-r--r-- | spec/unit/util/selinux_spec.rb | 17 |
7 files changed, 278 insertions, 48 deletions
diff --git a/lib/chef/mixin/which.rb b/lib/chef/mixin/which.rb index 4fa79eeccb..fd386241a0 100644 --- a/lib/chef/mixin/which.rb +++ b/lib/chef/mixin/which.rb @@ -1,6 +1,6 @@ #-- # Author:: Lamont Granquist <lamont@chef.io> -# Copyright:: Copyright 2010-2016, Chef Software Inc. +# Copyright:: Copyright 2010-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -18,15 +18,32 @@ class Chef module Mixin module Which - def which(cmd, extra_path: nil) + def which(*cmds, extra_path: nil, &block) + where(*cmds, extra_path: extra_path, &block).first || false + end + + def where(*cmds, extra_path: nil, &block) # NOTE: unnecessarily duplicates function of path_sanity extra_path ||= [ "/bin", "/usr/bin", "/sbin", "/usr/sbin" ] - paths = ENV["PATH"].split(File::PATH_SEPARATOR) + extra_path - paths.each do |path| - filename = Chef.path_to(File.join(path, cmd)) - return filename if File.executable?(filename) - end - false + paths = env_path.split(File::PATH_SEPARATOR) + extra_path + cmds.map do |cmd| + paths.map do |path| + filename = Chef.path_to(File.join(path, cmd)) + filename if valid_executable?(filename, &block) + end.compact + end.flatten + end + + private + + # for test stubbing + def env_path + ENV["PATH"] + end + + def valid_executable?(filename, &block) + return false unless File.executable?(filename) && !File.directory?(filename) + block ? yield(filename) : true end end end diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb index f52614672a..edd27a0439 100644 --- a/lib/chef/provider/package.rb +++ b/lib/chef/provider/package.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob (<adam@chef.io>) -# Copyright:: Copyright 2008-2016, Chef Software, Inc. +# Copyright:: Copyright 2008-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/lib/chef/provider/package/dnf/dnf_helper.py b/lib/chef/provider/package/dnf/dnf_helper.py index 236b967710..ef08bb54c2 100644 --- a/lib/chef/provider/package/dnf/dnf_helper.py +++ b/lib/chef/provider/package/dnf/dnf_helper.py @@ -53,7 +53,7 @@ def query(command): if len(archq.run()) > 0: q = archq - pkgs = dnf.query.latest_limit_pkgs(q, 1) + pkgs = q.latest(1).run() if not pkgs: sys.stdout.write('{} nil nil\n'.format(command['provides'].split().pop(0))) diff --git a/lib/chef/provider/package/dnf/python_helper.rb b/lib/chef/provider/package/dnf/python_helper.rb index 466114b339..d6e278a9fb 100644 --- a/lib/chef/provider/package/dnf/python_helper.rb +++ b/lib/chef/provider/package/dnf/python_helper.rb @@ -1,5 +1,5 @@ # -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,6 +15,8 @@ # limitations under the License. # +require "chef/mixin/which" +require "chef/mixin/shell_out" require "chef/provider/package/dnf/version" require "timeout" @@ -24,7 +26,8 @@ class Chef class Dnf < Chef::Provider::Package class PythonHelper include Singleton - extend Chef::Mixin::Which + include Chef::Mixin::Which + include Chef::Mixin::ShellOut attr_accessor :stdin attr_accessor :stdout @@ -32,11 +35,16 @@ class Chef attr_accessor :wait_thr DNF_HELPER = ::File.expand_path(::File.join(::File.dirname(__FILE__), "dnf_helper.py")).freeze - DNF_COMMAND = "#{which("python3")} #{DNF_HELPER}" + + def dnf_command + @dnf_command ||= which("python", "python3", "python2", "python2.7") do |f| + shell_out("#{f} -c 'import dnf'").exitstatus == 0 + end + " #{DNF_HELPER}" + end def start ENV["PYTHONUNBUFFERED"] = "1" - @stdin, @stdout, @stderr, @wait_thr = Open3.popen3(DNF_COMMAND) + @stdin, @stdout, @stderr, @wait_thr = Open3.popen3(dnf_command) end def reap @@ -53,6 +61,27 @@ class Chef start if stdin.nil? end + # @returns Array<Version> + def query(action, provides, version = nil, arch = nil) + with_helper do + json = build_query(action, provides, version, arch) + Chef::Log.debug "sending '#{json}' to python helper" + stdin.syswrite json + "\n" + output = stdout.sysread(4096).chomp + Chef::Log.debug "got '#{output}' from python helper" + version = parse_response(output) + Chef::Log.debug "parsed #{version} from python helper" + version + end + end + + def restart + reap + start + end + + private + # i couldn't figure out how to decompose an evr on the python side, it seems reasonably # painless to do it in ruby (generally massaging nevras in the ruby side is HIGHLY # discouraged -- this is an "every rule has an exception" exception -- any additional @@ -83,35 +112,41 @@ class Chef array.each_slice(3).map { |x| Version.new(*x) }.first end - # @returns Array<Version> - def query(action, provides, version = nil, arch = nil) - with_helper do - json = build_query(action, provides, version, arch) - Chef::Log.debug "sending '#{json}' to python helper" - stdin.syswrite json + "\n" - output = stdout.sysread(4096).chomp - Chef::Log.debug "got '#{output}' from python helper" - version = parse_response(output) - Chef::Log.debug "parsed #{version} from python helper" - version + def drain_stderr + output = "" + until IO.select([stderr], nil, nil, 0).nil? + output += stderr.sysread(4096).chomp end - end - - def restart - reap - start + output + rescue + # we must rescue EOFError, and we don't much care about errors on stderr anyway + output end def with_helper max_retries ||= 5 + ret = nil Timeout.timeout(600) do check - yield + ret = yield end + output = drain_stderr + unless output.empty? + Chef::Log.debug "discarding output on stderr from python helper: #{output}" + end + ret rescue EOFError, Errno::EPIPE, Timeout::Error, Errno::ESRCH => e - raise e unless ( max_retries -= 1 ) > 0 - restart - retry + output = drain_stderr + if ( max_retries -= 1 ) > 0 + unless output.empty? + Chef::Log.debug "discarding output on stderr from python helper: #{output}" + end + restart + retry + else + raise e if output.empty? + raise "dnf-helper.py had stderr output:\n\n#{output}" + end end end end diff --git a/spec/unit/mixin/which.rb b/spec/unit/mixin/which.rb new file mode 100644 index 0000000000..1764b3b89f --- /dev/null +++ b/spec/unit/mixin/which.rb @@ -0,0 +1,160 @@ +# +# Copyright:: Copyright 2011-2017, Chef Software Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require "spec_helper" + +class TestClass + include Chef::Mixin::Which +end + +describe Chef::Mixin::Which do + + let(:test) { TestClass.new } + + describe "#which" do + def self.test_which(description, *args, finds: nil, others: [], directory: false, &block) + it description do + # stub the ENV['PATH'] + expect(test).to receive(:env_path).and_return(["/dir1", "/dir2" ].join(File::PATH_SEPARATOR)) + + # most files should not be found + allow(File).to receive(:executable?).and_return(false) + allow(File).to receive(:directory?).and_return(false) + + # stub the expectation + expect(File).to receive(:executable?).with(finds).and_return(true) if finds + + # if the file we find is a directory + expect(File).to receive(:directory?).with(finds).and_return(true) if finds && directory + + # allow for stubbing other paths to exist that we should not find + others.each do |other| + allow(File).to receive(:executable?).with(other).and_return(true) + end + + # setup the actual expectation on the return value + if finds && !directory + expect(test.which(*args, &block)).to eql(finds) + else + expect(test.which(*args, &block)).to eql(false) + end + end + end + + context "simple usage" do + test_which("returns false when it does not find anything", "foo1") + + ["/dir1", "/dir2", "/bin", "/usr/bin", "/sbin", "/usr/sbin" ].each do |dir| + test_which("finds `foo1` in #{dir} when it is stubbed", "foo1", finds: "#{dir}/foo1") + end + + test_which("does not find an executable directory", "foo1", finds: "/dir1/foo1", directory: true) + end + + context "with an array of args" do + test_which("finds the first arg", "foo1", "foo2", finds: "/dir2/foo1") + + test_which("finds the second arg", "foo1", "foo2", finds: "/dir2/foo2") + + test_which("finds the first arg when there's both", "foo1", "foo2", finds: "/dir2/foo1", others: [ "/dir1/foo2" ]) + + test_which("and the directory order can be reversed", "foo1", "foo2", finds: "/dir1/foo1", others: [ "/dir2/foo2" ]) + + test_which("or be the same", "foo1", "foo2", finds: "/dir1/foo1", others: [ "/dir1/foo2" ]) + end + + context "with a block" do + test_which("doesnt find it if its false", "foo1", others: [ "/dir1/foo1" ]) do |f| + false + end + + test_which("finds it if its true", "foo1", finds: "/dir1/foo1") do |f| + true + end + + test_which("passes in the filename as the arg", "foo1", finds: "/dir1/foo1") do |f| + raise "bad arg to block" unless f == "/dir1/foo1" + true + end + + test_which("arrays with blocks", "foo1", "foo2", finds: "/dir2/foo1", others: [ "/dir1/foo2" ]) do |f| + raise "bad arg to block" unless f == "/dir2/foo1" || f == "/dir1/foo2" + true + end + end + end + + describe "#where" do + def self.test_where(description, *args, finds: [], others: [], &block) + it description do + # stub the ENV['PATH'] + expect(test).to receive(:env_path).and_return(["/dir1", "/dir2" ].join(File::PATH_SEPARATOR)) + + # most files should not be found + allow(File).to receive(:executable?).and_return(false) + allow(File).to receive(:directory?).and_return(false) + + # allow for stubbing other paths to exist that we should not return + others.each do |other| + allow(File).to receive(:executable?).with(other).and_return(true) + end + + # stub the expectation + finds.each do |path| + expect(File).to receive(:executable?).with(path).and_return(true) + end + + # setup the actual expectation on the return value + expect(test.where(*args, &block)).to eql(finds) + end + end + + context "simple usage" do + test_where("returns empty array when it doesn't find anything", "foo1") + + ["/dir1", "/dir2", "/bin", "/usr/bin", "/sbin", "/usr/sbin" ].each do |dir| + test_where("finds `foo1` in #{dir} when it is stubbed", "foo1", finds: [ "#{dir}/foo1" ]) + end + + test_where("finds `foo1` in all directories", "foo1", finds: [ "/dir1/foo1", "/dir2/foo1" ]) + end + + context "with an array of args" do + test_where("finds the first arg", "foo1", "foo2", finds: [ "/dir2/foo1" ]) + + test_where("finds the second arg", "foo1", "foo2", finds: [ "/dir2/foo2" ]) + + test_where("finds foo1 before foo2", "foo1", "foo2", finds: [ "/dir2/foo1", "/dir1/foo2" ]) + + test_where("finds foo1 before foo2 if the dirs are reversed", "foo1", "foo2", finds: [ "/dir1/foo1", "/dir2/foo2" ]) + + test_where("finds them both in the same directory", "foo1", "foo2", finds: [ "/dir1/foo1", "/dir1/foo2" ]) + + test_where("finds foo2 first if they're reversed", "foo2", "foo1", finds: [ "/dir1/foo2", "/dir1/foo1" ]) + end + + context "with a block do" do + test_where("finds foo1 and foo2 if they exist and the block is true", "foo1", "foo2", finds: [ "/dir1/foo2", "/dir2/foo2" ]) do + true + end + + test_where("does not finds foo1 and foo2 if they exist and the block is false", "foo1", "foo2", others: [ "/dir1/foo2", "/dir2/foo2" ]) do + false + end + end + end +end diff --git a/spec/unit/provider/package/dnf/python_helper_spec.rb b/spec/unit/provider/package/dnf/python_helper_spec.rb new file mode 100644 index 0000000000..505217bf90 --- /dev/null +++ b/spec/unit/provider/package/dnf/python_helper_spec.rb @@ -0,0 +1,29 @@ +# +# Copyright:: Copyright 2017-2017, Chef Software Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require "spec_helper" + +# NOTE: most of the tests of this functionality are baked into the func tests for the dnf package provider + +describe Chef::Provider::Package::Dnf::PythonHelper do + let(:helper) { Chef::Provider::Package::Dnf::PythonHelper.instance } + + it "propagates stacktraces on stderr from the forked subprocess" do + allow(helper).to receive(:dnf_command).and_return("ruby -e 'raise \"your hands in the air\"'") + expect { helper.query(:whatprovides, "tcpdump") }.to raise_error(/your hands in the air/) + end +end diff --git a/spec/unit/util/selinux_spec.rb b/spec/unit/util/selinux_spec.rb index 609ff02215..751092bc9a 100644 --- a/spec/unit/util/selinux_spec.rb +++ b/spec/unit/util/selinux_spec.rb @@ -1,6 +1,6 @@ # # Author:: Serdar Sutay (<serdar@chef.io>) -# Copyright:: Copyright 2013-2016, Chef Software Inc. +# Copyright:: Copyright 2013-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -148,21 +148,10 @@ describe Chef::Util::Selinux do end describe "when restorecon doesn't exist on the system" do - before do - allow(File).to receive(:executable?) do |file_path| - expect(file_path.end_with?("restorecon")).to be_truthy - false - end - end - it "should log a warning message" do - log = [ ] - allow(Chef::Log).to receive(:warn) do |message| - log << message - end - + allow(File).to receive(:executable?).with(/restorecon$/).and_return(false) + expect(Chef::Log).to receive(:warn).with(/Can not find 'restorecon' on the system. Skipping selinux security context restore./).at_least(:once) @test_instance.restore_security_context(path) - expect(log).not_to be_empty expect(File).not_to receive(:executable?) @test_instance.restore_security_context(path) end |