diff --git a/acceptance/tests/provider/package/dpkg_hold_true_package_is_latest.rb b/acceptance/tests/provider/package/dpkg_hold_true_package_is_latest.rb new file mode 100644 index 00000000000..b626231532e --- /dev/null +++ b/acceptance/tests/provider/package/dpkg_hold_true_package_is_latest.rb @@ -0,0 +1,27 @@ +test_name "dpkg ensure held package is latest installed" do + confine :to, :platform => /debian-8-amd64/ + tag 'audit:low' + + require 'puppet/acceptance/common_utils' + extend Puppet::Acceptance::PackageUtils + extend Puppet::Acceptance::ManifestUtils + + + package = "nginx" + + agents.each do |agent| + teardown do + package_absent(agent, package, '--force-yes') + end + end + + step"Ensure that package is installed first if not present" do + expected_package_version = on(agent.name, "apt-cache policy #{package} | sed -n -e 's/Candidate: //p'").stdout + package_manifest = resource_manifest('package', package, mark: "hold") + + apply_manifest_on(agent, package_manifest) do |result| + installed_package_version = on(agent.name, "apt-cache policy #{package} | sed -n -e 's/Installed: //p'").stdout + assert_match(expected_package_version, installed_package_version) + end + end +end diff --git a/acceptance/tests/provider/package/dpkg_hold_true_should_preserve_version.rb b/acceptance/tests/provider/package/dpkg_hold_true_should_preserve_version.rb new file mode 100644 index 00000000000..2fd3202b6b8 --- /dev/null +++ b/acceptance/tests/provider/package/dpkg_hold_true_should_preserve_version.rb @@ -0,0 +1,22 @@ +test_name "dpkg ensure held package should preserve version if package is allready installed" do + confine :to, :platform => /debian-8-amd64/ + tag 'audit:low' + + require 'puppet/acceptance/common_utils' + extend Puppet::Acceptance::PackageUtils + extend Puppet::Acceptance::ManifestUtils + + package = "openssl" + + step "Ensure held should lock to specific installed version" do + existing_installed_version = on(agent.name, "dpkg -s #{package} | sed -n -e 's/Version: //p'").stdout + existing_installed_version.delete!(' ') + + package_manifest_held = resource_manifest('package', package, mark: "hold") + apply_manifest_on(agent, package_manifest_held) do + installed_version = on(agent.name, "apt-cache policy #{package} | sed -n -e 's/Installed: //p'").stdout + installed_version.delete!(' ') + assert_match(existing_installed_version, installed_version) + end + end +end diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb index d3289e5d2e3..a214c44fd7f 100644 --- a/lib/puppet/provider/package/apt.rb +++ b/lib/puppet/provider/package/apt.rb @@ -70,7 +70,12 @@ def install cmd += install_options if @resource[:install_options] cmd << :install << str - aptget(*cmd) + self.unhold if self.properties[:mark] == :hold + begin + aptget(*cmd) + ensure + self.hold if @resource[:mark] == :hold + end end # What's the latest package version available? @@ -100,12 +105,18 @@ def run_preseed def uninstall self.run_preseed if @resource[:responsefile] - aptget "-y", "-q", :remove, @resource[:name] + args = ['-y', '-q'] + args << '--allow-change-held-packages' if self.properties[:mark] == :hold + args << :remove << @resource[:name] + aptget(*args) end def purge self.run_preseed if @resource[:responsefile] - aptget '-y', '-q', :remove, '--purge', @resource[:name] + args = ['-y', '-q'] + args << '--allow-change-held-packages' if self.properties[:mark] == :hold + args << :remove << '--purge' << @resource[:name] + aptget(*args) # workaround a "bug" in apt, that already removed packages are not purged super end diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb index e1a494eb274..edd7a757ae2 100644 --- a/lib/puppet/provider/package/dpkg.rb +++ b/lib/puppet/provider/package/dpkg.rb @@ -44,7 +44,7 @@ def self.instances # Note: self:: is required here to keep these constants in the context of what will # eventually become this Puppet::Type::Package::ProviderDpkg class. self::DPKG_QUERY_FORMAT_STRING = %Q{'${Status} ${Package} ${Version}\\n'} - self::FIELDS_REGEX = %r{^(\S+) +(\S+) +(\S+) (\S+) (\S*)$} + self::FIELDS_REGEX = %r{^'?(\S+) +(\S+) +(\S+) (\S+) (\S*)$} self::FIELDS= [:desired, :error, :status, :name, :ensure] # @param line [String] one line of dpkg-query output @@ -67,7 +67,7 @@ def self.parse_line(line) elsif ['config-files', 'half-installed', 'unpacked', 'half-configured'].include?(hash[:status]) hash[:ensure] = :absent end - hash[:ensure] = :held if hash[:desired] == 'hold' + hash[:mark] = :hold if hash[:desired] == 'hold' else Puppet.debug("Failed to match dpkg-query line #{line.inspect}") end @@ -83,8 +83,6 @@ def install end args = [] - # We always unhold when installing to remove any prior hold. - self.unhold if @resource[:configfiles] == :keep args << '--force-confold' @@ -93,7 +91,12 @@ def install end args << '-i' << file - dpkg(*args) + self.unhold if self.properties[:mark] == :hold + begin + dpkg(*args) + ensure + self.hold if @resource[:mark] == :hold + end end def update @@ -144,10 +147,14 @@ def purge dpkg "--purge", @resource[:name] end - def hold + def deprecated_hold if package_not_installed? self.install end + hold + end + + def hold Tempfile.open('puppet_dpkg_set_selection') do |tmpfile| tmpfile.write("#{@resource[:name]} hold\n") tmpfile.flush diff --git a/lib/puppet/provider/package/fink.rb b/lib/puppet/provider/package/fink.rb index 304d8c4e0e6..a5d0ff6bdbb 100644 --- a/lib/puppet/provider/package/fink.rb +++ b/lib/puppet/provider/package/fink.rb @@ -37,7 +37,12 @@ def install cmd << :install << str - finkcmd(cmd) + self.unhold if self.properties[:mark] == :hold + begin + finkcmd(cmd) + ensure + self.hold if @resource[:mark] == :hold + end end # What's the latest package version available? @@ -70,10 +75,22 @@ def update end def uninstall - finkcmd "-y", "-q", :remove, @model[:name] + self.unhold if self.properties[:mark] == :hold + begin + finkcmd "-y", "-q", :remove, @model[:name] + rescue StandardError, LoadError => e + self.hold if self.properties[:mark] == :hold + raise e + end end def purge - aptget '-y', '-q', 'remove', '--purge', @resource[:name] + self.unhold if self.properties[:mark] == :hold + begin + aptget '-y', '-q', 'remove', '--purge', @resource[:name] + rescue StandardError, LoadError => e + self.hold if self.properties[:mark] == :hold + raise e + end end end diff --git a/lib/puppet/provider/package/pkg.rb b/lib/puppet/provider/package/pkg.rb index 44aeb2849ae..7f4bcaf9f8c 100644 --- a/lib/puppet/provider/package/pkg.rb +++ b/lib/puppet/provider/package/pkg.rb @@ -49,7 +49,7 @@ def self.ifo_flag(flags) ).merge( case flags[1..1] when 'f' - {:ensure => 'held'} + {:mark => :hold} when '-' {} else @@ -107,6 +107,10 @@ def self.parse_line(line) end).merge({:provider => self.name}) end + def deprecated_hold + hold + end + def hold pkg(:freeze, @resource[:name]) end @@ -201,8 +205,6 @@ def latest def install(nofail = false) name = @resource[:name] should = @resource[:ensure] - # always unhold if explicitly told to install/update - self.unhold is = self.query if is[:ensure].to_sym == :absent command = 'install' @@ -216,7 +218,12 @@ def install(nofail = false) unless should.is_a? Symbol name += "@#{should}" end - r = exec_cmd(command(:pkg), command, *args, name) + self.unhold if self.properties[:mark] == :hold + begin + r = exec_cmd(command(:pkg), command, *args, name) + ensure + self.hold if @resource[:mark] == :hold + end return r if nofail raise Puppet::Error, _("Unable to update %{package}") % { package: r[:out] } if r[:exit] != 0 end @@ -230,7 +237,13 @@ def uninstall cmd << '-r' end cmd << @resource[:name] - pkg cmd + self.unhold if self.properties[:mark] == :hold + begin + pkg cmd + rescue StandardError, LoadError => e + self.hold if self.properties[:mark] == :hold + raise e + end end # update the package to the latest version available diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb index 0ecd429db21..0ce5a94686a 100644 --- a/lib/puppet/type/package.rb +++ b/lib/puppet/type/package.rb @@ -54,9 +54,8 @@ module Puppet feature :holdable, "The provider is capable of placing packages on hold such that they are not automatically upgraded as a result of other package dependencies unless explicit action is taken by - a user or another package. Held is considered a superset of - installed.", - :methods => [:hold] + a user or another package.", + :methods => [:hold, :unhold] feature :install_only, "The provider accepts options to only install packages never update (kernels, etc.)" feature :install_options, "The provider accepts options to be passed to the installer command." @@ -101,7 +100,7 @@ module Puppet end newvalue(:held, :event => :package_held, :required_features => :holdable) do - provider.hold + provider.deprecated_hold end # Alias the 'present' value. @@ -611,5 +610,59 @@ def refresh provider.reinstall end end + + newproperty(:mark, :required_features => :holdable) do + mark_doc='Valid values are: hold/none' + desc <<-EOT + Set to hold to tell Debian apt/Solaris pkg to hold the package version + + #{mark_doc} + Default is "none". Mark can be specified with or without `ensure`, + if `ensure` is missing will default to "present". + + Mark cannot be specified together with "purged", "absent" or "held" + values for `ensure`. + EOT + newvalues(:hold, :none) + munge do |value| + case value + when "hold", :hold + :hold + when "none", :none + :none + else + raise ArgumentError, _('Invalid hold value %{value}. %{doc}') % { value: value.inspect, doc: mark_doc} + end + end + + def insync?(is) + @should[0] == is + end + + def should + @should[0] if @should && @should.is_a?(Array) && @should.size == 1 + end + + def retrieve + provider.properties[:mark] + end + + def sync + if @should[0] == :hold + provider.hold + else + provider.unhold + end + end + end + + validate do + if :held == @parameters[:ensure].should + warning '"ensure=>held" has been deprecated and will be removed in a future version, use "mark=hold" instead' + end + if @parameters[:mark] && [:absent, :purged, :held].include?(@parameters[:ensure].should) + raise ArgumentError, _('You cannot use "mark" property while "ensure" is one of ["absent", "purged", "held"]') + end + end end end diff --git a/spec/unit/provider/package/apt_spec.rb b/spec/unit/provider/package/apt_spec.rb index fd27a9cbfd1..0f06d2e3a95 100644 --- a/spec/unit/provider/package/apt_spec.rb +++ b/spec/unit/provider/package/apt_spec.rb @@ -32,12 +32,14 @@ it "should use 'apt-get remove' to uninstall" do expect(provider).to receive(:aptget).with("-y", "-q", :remove, name) + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.uninstall end it "should use 'apt-get purge' and 'dpkg purge' to purge" do expect(provider).to receive(:aptget).with("-y", "-q", :remove, "--purge", name) expect(provider).to receive(:dpkg).with("--purge", name) + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.purge end @@ -88,14 +90,14 @@ it "should preseed if a responsefile is provided" do resource[:responsefile] = "/my/file" expect(provider).to receive(:run_preseed) - + expect(provider).to receive(:properties).and_return({:mark => :none}) allow(provider).to receive(:aptget) provider.install end it "should check for a cdrom" do expect(provider).to receive(:checkforcdrom) - + expect(provider).to receive(:properties).and_return({:mark => :none}) allow(provider).to receive(:aptget) provider.install end @@ -106,6 +108,7 @@ expect(command[-1]).to eq(name) expect(command[-2]).to eq(:install) end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -115,6 +118,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command[-1]).to eq("#{name}=1.0") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -124,6 +128,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command).to include("--force-yes") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -132,6 +137,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command).to include("-q") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -140,6 +146,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command).to include("-y") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -149,6 +156,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command).to include("DPkg::Options::=--force-confold") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -158,6 +166,7 @@ expect(provider).to receive(:aptget) do |*command| expect(command).to include("DPkg::Options::=--force-confnew") end + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -165,6 +174,7 @@ it 'should support string install options' do resource[:install_options] = ['--foo', '--bar'] expect(provider).to receive(:aptget).with('-q', '-y', '-o', 'DPkg::Options::=--force-confold', '--foo', '--bar', :install, name) + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end @@ -172,6 +182,7 @@ it 'should support hash install options' do resource[:install_options] = ['--foo', { '--bar' => 'baz', '--baz' => 'foo' }] expect(provider).to receive(:aptget).with('-q', '-y', '-o', 'DPkg::Options::=--force-confold', '--foo', '--bar=baz', '--baz=foo', :install, name) + expect(provider).to receive(:properties).and_return({:mark => :none}) provider.install end diff --git a/spec/unit/provider/package/aptitude_spec.rb b/spec/unit/provider/package/aptitude_spec.rb index 01c12649a8e..e3d8bbecaa5 100644 --- a/spec/unit/provider/package/aptitude_spec.rb +++ b/spec/unit/provider/package/aptitude_spec.rb @@ -33,6 +33,7 @@ expect(pkg.provider).to receive(:aptitude). with('-y', '-o', 'DPkg::Options::=--force-confold', :install, 'faff'). and_return(0) + expect(pkg.provider).to receive(:properties).and_return({:mark => :none}) pkg.provider.install end diff --git a/spec/unit/provider/package/dpkg_spec.rb b/spec/unit/provider/package/dpkg_spec.rb index aa18f63dc6b..709e93f0ec1 100644 --- a/spec/unit/provider/package/dpkg_spec.rb +++ b/spec/unit/provider/package/dpkg_spec.rb @@ -126,7 +126,10 @@ def dpkg_query_execution_returns(output) it "considers the package held if its state is 'hold'" do dpkg_query_execution_returns(bash_installed_output.gsub("install","hold")) - expect(provider.query[:ensure]).to eq(:held) + query=provider.query + expect(query[:ensure]).to eq("4.2-5ubuntu3") + expect(query[:mark]).to eq(:hold) + end context "parsing tests" do @@ -184,14 +187,15 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0) it "uses 'dpkg -i' to install the package" do expect(resource).to receive(:[]).with(:source).and_return("mypackagefile") + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:unhold) expect(provider).to receive(:dpkg).with(any_args, "-i", "mypackagefile") - provider.install end it "keeps old config files if told to do so" do expect(resource).to receive(:[]).with(:configfiles).and_return(:keep) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:unhold) expect(provider).to receive(:dpkg).with("--force-confold", any_args) @@ -200,6 +204,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0) it "replaces old config files if told to do so" do expect(resource).to receive(:[]).with(:configfiles).and_return(:replace) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:unhold) expect(provider).to receive(:dpkg).with("--force-confnew", any_args) @@ -207,6 +212,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0) end it "ensures any hold is removed" do + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:unhold).once expect(provider).to receive(:dpkg) provider.install @@ -222,18 +228,28 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0) end it "installs first if package is not present and ensure holding" do + allow(provider).to receive(:execute) + allow(provider).to receive(:package_not_installed?).and_return(true) + expect(provider).to receive(:install).once + expect(provider).to receive(:hold) + provider.deprecated_hold + end + + it "skips install new package if hold is true" do allow(provider).to receive(:execute) allow(provider).to receive(:package_not_installed?).and_return(true) expect(provider).to receive(:install).once - provider.hold + expect(provider).to receive(:hold) + provider.deprecated_hold end it "skips install new package if package is allready installed" do allow(provider).to receive(:execute) allow(provider).to receive(:package_not_installed?).and_return(false) expect(provider).not_to receive(:install) - provider.hold + expect(provider).to receive(:hold) + provider.deprecated_hold end it "executes dpkg --set-selections when holding" do diff --git a/spec/unit/provider/package/pkg_spec.rb b/spec/unit/provider/package/pkg_spec.rb index 4b7ca24a38d..a423dc34a99 100644 --- a/spec/unit/provider/package/pkg_spec.rb +++ b/spec/unit/provider/package/pkg_spec.rb @@ -90,7 +90,7 @@ def self.it_should_respond_to(*actions) { 'pkg://omnios/SUNWcs@0.5.11,5.11-0.151006:20130506T161045Z i--' => {:name => 'SUNWcs', :ensure => '0.5.11,5.11-0.151006:20130506T161045Z', :status => 'installed', :provider => :pkg, :publisher => 'omnios'}, - 'pkg://omnios/incorporation/jeos/illumos-gate@11,5.11-0.151006:20130506T183443Z if-' => {:name => 'incorporation/jeos/illumos-gate', :ensure => 'held', :status => 'installed', :provider => :pkg, :publisher => 'omnios'}, + 'pkg://omnios/incorporation/jeos/illumos-gate@11,5.11-0.151006:20130506T183443Z if-' => {:name => 'incorporation/jeos/illumos-gate', :ensure => "11,5.11-0.151006:20130506T183443Z", :mark => :hold, :status => 'installed', :provider => :pkg, :publisher => 'omnios'}, 'pkg://solaris/SUNWcs@0.5.11,5.11-0.151.0.1:20101105T001108Z installed -----' => {:name => 'SUNWcs', :ensure => '0.5.11,5.11-0.151.0.1:20101105T001108Z', :status => 'installed', :provider => :pkg, :publisher => 'solaris'}, }.each do |k, v| it "[#{k}] should correctly parse" do @@ -251,6 +251,7 @@ def self.it_should_respond_to(*actions) it "should accept all licenses" do expect(provider).to receive(:query).with(no_args).and_return({:ensure => :absent}) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(Puppet::Util::Execution).to receive(:execute) .with(['/bin/pkg', 'install', *hash[:flags], 'dummy'], {:failonfail => false, :combine => true}) .and_return(Puppet::Util::Execution::ProcessOutput.new('', 0)) @@ -265,6 +266,7 @@ def self.it_should_respond_to(*actions) # Should install also check if the version installed is the same version we are asked to install? or should we rely on puppet for that? resource[:ensure] = '0.0.7,5.11-0.151006:20131230T130000Z' allow($CHILD_STATUS).to receive(:exitstatus).and_return(0) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(Puppet::Util::Execution).to receive(:execute).with(['/bin/pkg', 'unfreeze', 'dummy'], {:failonfail => false, :combine => true}) expect(Puppet::Util::Execution).to receive(:execute) .with(['/bin/pkg', 'list', '-Hv', 'dummy'], {:failonfail => false, :combine => true}) @@ -277,6 +279,7 @@ def self.it_should_respond_to(*actions) it "should install specific version(2)" do resource[:ensure] = '0.0.8' + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(Puppet::Util::Execution).to receive(:execute).with(['/bin/pkg', 'unfreeze', 'dummy'], {:failonfail => false, :combine => true}) expect(Puppet::Util::Execution).to receive(:execute) .with(['/bin/pkg', 'list', '-Hv', 'dummy'], {:failonfail => false, :combine => true}) @@ -290,6 +293,7 @@ def self.it_should_respond_to(*actions) it "should downgrade to specific version" do resource[:ensure] = '0.0.7' + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:query).with(no_args).and_return({:ensure => '0.0.8,5.11-0.151106:20131230T130000Z'}) allow($CHILD_STATUS).to receive(:exitstatus).and_return(0) expect(Puppet::Util::Execution).to receive(:execute).with(['/bin/pkg', 'unfreeze', 'dummy'], {:failonfail => false, :combine => true}) @@ -301,6 +305,7 @@ def self.it_should_respond_to(*actions) it "should install any if version is not specified" do resource[:ensure] = :present + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:query).with(no_args).and_return({:ensure => :absent}) expect(Puppet::Util::Execution).to receive(:execute) .with(['/bin/pkg', 'install', *hash[:flags], 'dummy'], {:failonfail => false, :combine => true}) @@ -312,6 +317,7 @@ def self.it_should_respond_to(*actions) it "should install if no version was previously installed, and a specific version was requested" do resource[:ensure] = '0.0.7' + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(provider).to receive(:query).with(no_args).and_return({:ensure => :absent}) expect(Puppet::Util::Execution).to receive(:execute).with(['/bin/pkg', 'unfreeze', 'dummy'], {:failonfail => false, :combine => true}) expect(Puppet::Util::Execution).to receive(:execute) @@ -325,6 +331,7 @@ def self.it_should_respond_to(*actions) resource[:ensure] = '1.0-0.151006' is = :absent expect(provider).to receive(:query).with(no_args).and_return({:ensure => is}) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(described_class).to receive(:pkg) .with(:list, '-Hvfa', 'dummy@1.0-0.151006') .and_return(Puppet::Util::Execution::ProcessOutput.new(File.read(my_fixture('dummy_implicit_version')), 0)) @@ -340,6 +347,7 @@ def self.it_should_respond_to(*actions) resource[:ensure] = '1.0-0.151006' is = '1.0,5.11-0.151006:20140219T191204Z' expect(provider).to receive(:query).with(no_args).and_return({:ensure => is}) + expect(provider).to receive(:properties).and_return({:mark => :hold}) expect(described_class).to receive(:pkg).with(:list, '-Hvfa', 'dummy@1.0-0.151006').and_return(File.read(my_fixture('dummy_implicit_version'))) expect(Puppet::Util::Execution).to receive(:execute).with(['/bin/pkg', 'update', '-n', 'dummy@1.0,5.11-0.151006:20140220T084443Z'], {:failonfail => false, :combine => true}) expect(provider).to receive(:unhold).with(no_args) @@ -398,12 +406,16 @@ def self.it_should_respond_to(*actions) it "should support current pkg version" do expect(described_class).to receive(:pkg).with(:version).and_return('630e1ffc7a19') expect(described_class).to receive(:pkg).with([:uninstall, resource[:name]]) + expect(provider).to receive(:properties).and_return({:hold => false}) + provider.uninstall end it "should support original pkg commands" do expect(described_class).to receive(:pkg).with(:version).and_return('052adf36c3f4') expect(described_class).to receive(:pkg).with([:uninstall, '-r', resource[:name]]) + expect(provider).to receive(:properties).and_return({:hold => false}) + provider.uninstall end end