fix #5 - Breaks when a Package title != name

lib/puppet/type/aptly_purge.rb

@@ -13,8 +13,6 @@
 be removed.  This type takes the resulting list and generates Puppet package
 resources with ensure=>absent for any unmanaged resources that apt-get would
 autoremove.
-
-NOTE: This type writes into the apt-mark system, even when run in noop mode.
 EOD
 
   newparam(:title) do
@@ -28,6 +26,10 @@
     defaultto false
   end
 
+  newparam(:purge, :boolean => true, :parent => Puppet::Parameter::Boolean) do
+    defaultto false
+  end
+
   newparam(:hold, :boolean => true, :parent => Puppet::Parameter::Boolean) do
     defaultto false
   end
@@ -45,7 +47,7 @@ def generate
     package.instances.select do |p|
       p.provider.is_a?(Puppet::Type::Package::ProviderDpkg)
     end.each do |r|
-      catalog_r = catalog.resource(r.ref)
+      catalog_r = catalog.resource(r.ref) || find_resource_alias(["Package", r.name, :held_apt])
       if catalog_r.nil?
         unmanaged_packages << r
       else
@@ -58,20 +60,26 @@ def generate
     unmanaged_package_names = unmanaged_packages.map(&:name)
     Puppet.debug "unmanaged_package_names: #{unmanaged_package_names}"
 
-    holds = []
-
-    if @parameters[:hold] then
+    if should_hold? then
       # You can't hold a package that isn't installed yet, so this should
       # really be done after all packages are installed.
 
-      holds = managed_packages.select do |p|
+      pinned = managed_packages.select do |p|
         # What we really want is to grab all packages with an explicit version
         # This is a cheap reproduction of what we really want.
         ![:latest, :absent, :present].include?(p.parameters[:ensure].value)
-      end.map do |p|
-        Puppet::Type.type(:dpkg_hold).new({ :name => p[:name], :ensure => :present })
       end
+
+      Puppet.debug "pinned: #{pinned.map(&:name)}"
+      unless noop?
+        holds = pinned.map do |p|
+          Puppet::Type.type(:dpkg_hold).new({ :name => p[:name], :ensure => :present })
+        end
+      end
+    else
+      holds = []
     end
+    Puppet.debug "holds: #{holds.map(&:name)}"
 
     unless all_packages_synced
       notice <<EOS
@@ -90,27 +98,59 @@ def generate
     # B is marked as 'auto' as it should
     # If some other process has marked A as auto, B will get ensure=>absent
     # Then dpkg will remove both A and B.  This is bad!
-    mark_manual managed_package_names, outfile
+    if should_purge?
+      mark_manual managed_package_names, outfile
 
-    mark_auto unmanaged_package_names, outfile
+      mark_auto unmanaged_package_names, outfile
+    end
 
     apt_would_purge = get_purges()
     Puppet.debug "apt_would_purge: #{apt_would_purge.to_a}"
 
-    removes = unmanaged_packages.select do |r|
-      # This is the crux.  We intersect the list of packages Puppet isn't
-      # managing with the list of packages that apt would purge.
-      apt_would_purge.include?(r.name)
-    end.each do |resource|
-      resource[:ensure] = 'absent'
-      @parameters.each do |name, param|
-        resource[name] = param.value if param.metaparam?
+    if should_purge?
+      removes = unmanaged_packages.select do |r|
+        # This is the crux.  We intersect the list of packages Puppet isn't
+        # managing with the list of packages that apt would purge.
+        apt_would_purge.include?(r.name)
+      end.each do |resource|
+        resource[:ensure] = 'absent'
+        @parameters.each do |name, param|
+          resource[name] = param.value if param.metaparam?
+        end
+
+        resource.purging
+        end
+    else
+      removes = []
+    end
+    Puppet.debug "removes: #{removes.map(&:name)}"
+
+    # un-hold packages
+    if should_hold?
+      dpkg_selections = Puppet::Util::Execution.execute('dpkg --get-selections')
+      dpkg_selections = Hash[*dpkg_selections.lines.map {|l| l.rstrip.split(/\s+/,2)}.flatten]
+      to_be_removed = Hash[removes.map(&:name).zip([])]
+      # unmanaged packages that are not already slated for removal
+      unholds = unmanaged_packages.select do |p|
+        !to_be_removed.include?(p.name)
       end
-
-      resource.purging
+      # managed packages with ensure => present
+      unholds += managed_packages.select do |p|
+        p.parameters[:ensure].value == :present
+      end
+      # if the packages to be un-held are currently held, generate a dpkg_hold resource with ensure => absent
+      unholds = unholds.select do |p|
+        dpkg_selections.include?(p.name) &&
+        dpkg_selections[p.name] == 'hold'
+      end.map do |p|
+        Puppet::Type.type(:dpkg_hold).new({ :name => p[:name], :ensure => :absent })
+      end
+    else
+      unholds = []
     end
+    Puppet.debug "unholds: #{unholds.map(&:name)}"
 
-    holds + removes
+    holds + unholds + removes
   end
 
   private
@@ -157,4 +197,25 @@ def get_purges
       p
     end
   end
+
+  # ref is of the form: ["Package", "name", :provider]
+  # returns nil if no alias exist
+  def find_resource_alias ref
+    @resource_aliases ||= catalog.instance_variable_get(:@aliases)
+
+    result = @resource_aliases.find do |ref_str, aliases|
+      aliases.find do |candidate_ref|
+          candidate_ref == ref
+      end
+    end
+    return result.nil? ? nil : catalog.resource(result.first)
+  end
+
+  def should_purge?
+    @parameters[:purge] && @parameters[:purge].value && !noop?
+  end
+
+  def should_hold?
+    @parameters[:hold] && @parameters[:hold].value && !noop?
+  end
 end

spec/acceptance/00_purges_safely_spec.rb

@@ -0,0 +1,155 @@
+require 'spec_helper_acceptance'
+
+describe 'package_purging_with_apt' do
+  let :package_purging_manifest do
+    <<-EOS
+      package { 'ubuntu-minimal': }
+      package { 'puppetlabs-release-pc1': }
+      package { 'puppet-agent': }
+      package { 'fortunes': }
+      package { 'openssh-server': }
+      include package_purging::config
+      aptly_purge { 'packages':
+        purge => true,
+      }
+    EOS
+  end
+
+  def get_packages_state host
+    apt_mark = on(host, 'apt-mark showauto 2>&1').stdout
+    result = apt_mark.lines.each_with_object({}) { |line, h| h[line.rstrip] = 'auto' }
+    apt_mark = on(host, 'apt-mark showmanual 2>&1').stdout
+    apt_mark.lines.each_with_object(result) do |line, h|
+      package = line.rstrip
+      raise "Package #{package} appears both in apt-mark showauto and showmanual" if h.has_key?(package)
+      h[package] = 'manual'
+    end
+  end
+
+  before :all do
+    hosts.each do |host|
+      # install dict-jargon outside of Puppet
+      install_package host, 'dict-jargon'
+      # dictd gets automatically installed as a dependency of dict-jargon
+      expect(check_for_package host, 'dictd').to be true
+      # Normally, "apt-get autoremove" would only remove dictd if dict-jargon was manually
+      # uninstalled, because in that case dictd would become a "dangling dependency".
+      # aptly_purge marks any unmanaged package (any package that's been installed outside
+      # of Puppet) as automatically installed. This is counter-intuitive: because of aptly_purge
+      # manually installed packages are passed to "apt-mark auto" and will have "Auto-Installed: 1"
+      # in /var/lib/apt/extended_states .
+      # Any "Auto-Installed: 1" package shows up in the output of "apt-get -s autoremove" and,
+      # unless included in the Puppet catalog, will be purged by aptly_purge.
+
+      # fortunes is also manually installed but, as opposed to dict-jargon, a corresponding package
+      # resource is declared in the manifest. Therefore, aptly_purge will not uninstall fortunes
+      # and its tree of dependencies.
+      install_package host, 'fortunes'
+
+      # regardless of parse order, aptly_purge will be a noop until
+      # the APT::Get::Purge config option is set (which happens on the first puppet run)
+      on host, 'puppet config set ordering random'
+      on host, 'puppet config print ordering | grep -q random'
+      expect(@result.exit_code).to eq 0
+
+      packages_state = get_packages_state host
+      expect(packages_state['dict-jargon']).to eq 'manual'
+      expect(packages_state['dictd']).to eq 'auto'
+      expect(packages_state['fortunes']).to eq 'manual'
+      expect(check_for_package host, 'ubuntu-minimal').to be true
+    end
+  end
+
+  context 'aptly_purge with unmanaged packages on the system, first puppet run' do
+    it 'should not remove any packages' do
+      # aptly_purge generates the list of packages to purge at "parse time"
+      # before/require ordering constraints don't work on it
+      apply_manifest(package_purging_manifest)
+      expect(@result.exit_code).to eq 0
+      # The manifest has been applied, no packages will be removed until the next run
+      # because the settings at "include package_purging::config" have just been put
+      # in place.
+      expect(package('dict-jargon')).to be_installed
+      expect(package('dictd')).to be_installed
+    end
+
+    # Only 'fortunes' is in the catalog.
+    # 'dict-jargon' has been installed outside of puppet, 'dictd' is one
+    # of its dependencies. 'dict-jargon' gets apt-mark'ed as 'auto'.
+    it 'should correctly apt-mark packages' do
+      packages_state = get_packages_state default_node
+      expect(packages_state['dict-jargon']).to eq 'auto'
+      expect(packages_state['dictd']).to eq 'auto'
+      expect(packages_state['fortunes']).to eq 'manual'
+    end
+  end
+
+  context 'aptly_purge with unmanaged packages on the system, second puppet run' do
+    it 'should remove unmanaged packages' do
+      apply_manifest(package_purging_manifest, :debug => true)
+      expect(@result.exit_code).to eq 0
+      expect(package('dict-jargon')).to_not be_installed
+      expect(package('dictd')).to_not be_installed
+      expect(package('fortunes')).to be_installed
+      expect(package('fortunes-min')).to be_installed  # a dependency of fortune
+    end
+  end
+
+  RSpec.shared_examples 'aptly_purge noop' do |test_case|
+    let(:test_manifest) {
+      m = <<-EOS
+          package { 'ubuntu-minimal': }
+          package { 'puppetlabs-release-pc1': }
+          package { 'puppet-agent': }
+          package { 'fortunes': }
+          package { 'openssh-server': }
+          include package_purging::config
+      EOS
+      m + test_case
+    }
+
+    it 'before puppet runs' do
+      install_package default_node, 'dict-jargon'
+      # dictd gets automatically installed as a dependency of dict-jargon
+      expect(check_for_package default_node, 'dictd').to be true
+      packages_state = get_packages_state default_node
+      expect(packages_state['dict-jargon']).to eq 'manual'
+      expect(packages_state['dict']).to eq 'auto'
+      expect(packages_state['fortunes']).to eq 'manual'
+
+      # Purposely mark dict-jargon as auto. We really want it to look like
+      # something that could be purged and make sure it gets left alone
+      # when running with noop or purge => false .
+      on default_node, 'apt-mark auto dict-jargon'
+      packages_state = get_packages_state default_node
+      expect(packages_state['dict-jargon']).to eq 'auto'
+    end
+
+    it 'should not apt-mark packages' do
+      apply_manifest(test_manifest, :debug => true)
+      expect(@result.exit_code).to eq 0
+      packages_state = get_packages_state default_node
+      expect(packages_state['dict-jargon']).to eq 'auto'
+      expect(packages_state['dict']).to eq 'auto'
+      expect(packages_state['fortunes']).to eq 'manual'
+
+      expect(package('dict-jargon')).to be_installed
+      expect(package('dictd')).to be_installed
+      expect(package('fortunes')).to be_installed
+      expect(package('fortunes-min')).to be_installed  # a dependency of fortune
+    end
+  end
+
+  context 'aptly_purge in noop mode' do
+    it_behaves_like 'aptly_purge noop', "aptly_purge { 'packages': noop => true }"
+  end
+
+  context 'aptly_purge with purge => false' do
+    it_behaves_like 'aptly_purge noop', "aptly_purge { 'packages': purge => false }"
+  end
+
+  context 'aptly_purge by default' do
+    it_behaves_like 'aptly_purge noop', "aptly_purge { 'packages': }"
+  end
+
+end

spec/acceptance/01_title_and_name_differ_spec.rb

@@ -0,0 +1,37 @@
+require 'spec_helper_acceptance'
+
+describe 'title_and_name_differ' do
+  before :all do
+    hosts.each do |host|
+      install_package host, 'dict-jargon'
+      expect(check_for_package host, 'dictd').to be true
+      install_package host, 'fortunes'
+      expect(check_for_package host, 'fortunes-min').to be true
+      # same as `include package_purging::config`, saves a Puppet run
+      create_remote_file host, '/etc/apt/apt.conf.d/99always-purge', "APT::Get::Purge \"true\";\n";
+    end
+  end
+
+  context 'manifest contains a package resource where title != name' do
+    it 'should apply' do
+      m = <<-EOS
+        package { 'ubuntu-minimal': }
+        package { 'puppetlabs-release-pc1': }
+        package { 'puppet-agent': }
+        package { 'openssh-server': }
+        package {'fortunespkg':
+          name => 'fortunes',
+        }
+        aptly_purge {'packages':
+          purge => true,
+        }
+      EOS
+      apply_manifest m, :debug => true
+      expect(@result.exit_code).to eq 0
+      expect(package('dict-jargon')).to_not be_installed
+      expect(package('dictd')).to_not be_installed
+      expect(package('fortunes')).to be_installed
+      expect(package('fortunes-min')).to be_installed  # a dependency of fortune
+    end
+  end
+end