Enhance absent_file filter to suppress diffs when file is absent in b…

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -202,7 +202,9 @@ def catdiff
         filter_opts = {
           logger: @logger,
           from_compilation_dir: @catalog1_raw.compilation_dir,
-          to_compilation_dir: @catalog2_raw.compilation_dir
+          to_compilation_dir: @catalog2_raw.compilation_dir,
+          from_resources: @catalog1_raw.resources,
+          to_resources: @catalog2_raw.resources
         }
         OctocatalogDiff::CatalogDiff::Filter.apply_filters(result, @opts[:filters], filter_opts) if @opts[:filters].any?
 

lib/octocatalog-diff/catalog-diff/filter/absent_file.rb

@@ -10,6 +10,7 @@ class Filter
       # Filter out changes in parameters when the "to" resource has ensure => absent.
       class AbsentFile < OctocatalogDiff::CatalogDiff::Filter
         KEEP_ATTRIBUTES = (Set.new %w(ensure backup force provider)).freeze
+        ABSENT_VALUES = ['absent', 'false', false].freeze
 
         # Constructor: Since this filter requires knowledge of the entire array of diffs,
         # override the inherited method to store those diffs in an instance variable.
@@ -28,34 +29,69 @@ def initialize(diffs, _logger = nil)
         # @param diff [OctocatalogDiff::API::V1::Diff] Difference
         # @param _options [Hash] Additional options (there are none for this filter)
         # @return [Boolean] true if this difference is a YAML file with identical objects, false otherwise
-        def filtered?(diff, _options = {})
-          build_results if @results.nil?
+        def filtered?(diff, options = {})
+          build_results(options) if @results.nil?
           @results.include?(diff)
         end
 
         private
 
         # Private: The first time `.filtered?` is called, build up the cache of results.
         # Returns nothing, but populates @results.
-        def build_results
-          # Which files can we ignore?
-          @files_to_ignore = Set.new
+        def build_results(options)
+          @files_absent_in_to = Set.new
+          @files_absent_in_from = Set.new
+
+          if options[:to_resources].is_a?(Array)
+            options[:to_resources].each do |resource|
+              next unless resource.is_a?(Hash) && resource['type'] == 'File'
+              next unless resource.key?('parameters') && resource['parameters'].is_a?(Hash)
+              next unless ABSENT_VALUES.include?(resource['parameters']['ensure'])
+              @files_absent_in_to.add file_title_for_resource(resource)
+            end
+          end
+
+          if options[:from_resources].is_a?(Array)
+            options[:from_resources].each do |resource|
+              next unless resource.is_a?(Hash) && resource['type'] == 'File'
+              next unless resource.key?('parameters') && resource['parameters'].is_a?(Hash)
+              next unless ABSENT_VALUES.include?(resource['parameters']['ensure'])
+              @files_absent_in_from.add file_title_for_resource(resource)
+            end
+          end
+
+          # Backward-compatible behavior: if an ensure diff explicitly shows ensure => absent in the new
+          # catalog, make sure we ignore that file even if resource lists were not provided.
           @diffs.each do |diff|
             next unless diff.change? && diff.type == 'File' && diff.structure == %w(parameters ensure)
-            next unless ['absent', 'false', false].include?(diff.new_value)
-            @files_to_ignore.add diff.title
+            next unless ABSENT_VALUES.include?(diff.new_value)
+            @files_absent_in_to.add diff.title
           end
 
+          @files_absent_in_both = @files_absent_in_to & @files_absent_in_from
+
           # Based on that, which diffs can we ignore?
           @results = Set.new @diffs.reject { |diff| keep_diff?(diff) }
         end
 
+        def file_title_for_resource(resource)
+          return resource['title'] unless resource.key?('parameters') && resource['parameters'].is_a?(Hash)
+          return resource['title'] unless resource['parameters'].key?('path')
+
+          resource['parameters']['path']
+        end
+
         # Private: Determine whether to keep a particular diff.
         # @param diff [OctocatalogDiff::API::V1::Diff] Difference under consideration
         # @return [Boolean] true = keep, false = discard
         def keep_diff?(diff)
-          return true unless diff.change? && diff.type == 'File' && diff.structure.first == 'parameters'
-          return true unless @files_to_ignore.include?(diff.title)
+          return true unless diff.change? && diff.type == 'File'
+
+          # If both catalogs declare the same file as absent, suppress all diffs for that file.
+          return false if @files_absent_in_both.include?(diff.title)
+
+          return true unless diff.structure.first == 'parameters'
+          return true unless @files_absent_in_to.include?(diff.title)
           return true if KEEP_ATTRIBUTES.include?(diff.structure.last)
           false
         end

spec/octocatalog-diff/tests/catalog-diff/filter/absent_file_spec.rb

@@ -22,4 +22,22 @@
     result = obj.reject { |x| testobj.filtered?(x) }
     expect(result).to eq(obj.values_at(0, 2, 3, 4, 5, 6, 7))
   end
+
+  it 'should suppress diffs when file is ensure=>absent in both catalogs' do
+    orig = [
+      ['~', "File\f/tmp/foo\fparameters\fowner", 'root', 'nobody'],
+      ['~', "File\f/tmp/foo\fparameters\fforce", false, true],
+      ['~', "File\f/tmp/bar\fparameters\fmode", '0644', '0600']
+    ]
+    obj = orig.map { |x| OctocatalogDiff::API::V1::Diff.factory(x) }
+    testobj = described_class.new(obj)
+    result = obj.reject do |x|
+      testobj.filtered?(x, {
+        from_resources: [{ 'type' => 'File', 'title' => '/tmp/foo', 'parameters' => { 'ensure' => 'absent', 'owner' => 'root', 'force' => false } }],
+        to_resources: [{ 'type' => 'File', 'title' => '/tmp/foo', 'parameters' => { 'ensure' => 'absent', 'owner' => 'nobody', 'force' => true } }]
+      })
+    end
+
+    expect(result).to eq(obj.values_at(2))
+  end
 end