Merge pull request #46 from github/kpaulisse-yaml-diff

Add option to ignore whitespace in yaml file diff

.version

@@ -1 +1 @@
-0.6.0
+0.6.1

doc/CHANGELOG.md

@@ -8,6 +8,15 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>0.6.1</td>
+<td>2017-01-07</td>
+<td>
+<ul>
+<li><a href="https://github.com/github/octocatalog-diff/pull/46">#46</a>: Add option to ignore whitespace in yaml file diff</li>
+</ul>
+</td>
+</tr>
+<tr valign=top>
 <td>0.6.0</td>
 <td>2017-01-04</td>
 <td>

doc/advanced-filter.md

@@ -0,0 +1,26 @@
+# Additional output filters
+
+It is possible to enable additional filters for output results via the `--filters` command line option. This command line option accepts a comma-separated list of additional filters, and applies them to the results in the order you specify. The default behavior is not to use any of these filters.
+
+Please note that there are other options to ignore specified diffs, including:
+
+- [Ignoring certain changes via command line options](/doc/advanced-ignores.md)
+- [Dynamic ignoring of changes via tags in Puppet manifests](/doc/advanced-dynamic-ignores.md)
+
+Here is the list of available filters and an explanation of each:
+
+- [YAML](#YAML) - Ignore whitespace/comment differences if YAML parses to the same object
+
+## YAML
+
+#### Usage
+
+```
+--filters YAML
+```
+
+#### Description
+
+If a file resource has extension `.yml` or `.yaml` and a difference in its content is observed, YAML objects are constructed from the previous and new values. If these YAML objects are identical, the difference is ignored.
+
+This allows you to ignore changes in whitespace, comments, etc., that are not meaningful to a machine parsing the file. Please note that by filtering these changes, you are ignoring changes to comments, which may be meaningful to humans.

doc/advanced.md

@@ -27,6 +27,7 @@ See also:
 ### Controlling output
 
 - [Ignoring certain changes via command line options](/doc/advanced-ignores.md)
+- [Additional output filters](/doc/advanced-filter.md)
 - [Dynamic ignoring of changes via tags in Puppet manifests](/doc/advanced-dynamic-ignores.md)
 - [Output formats](/doc/advanced-output-formats.md)
 - [Useful output hacks](/doc/advanced-output-hacks.md)

doc/optionsref.md

@@ -46,6 +46,8 @@ Usage: octocatalog-diff [command line options]
         --no-hiera-path-strip        Do not use any default hiera path strip settings
         --ignore-attr "attr1,attr2,..."
                                      Attributes to ignore
+        --filters FILTER1[,FILTER2[,...]]
+                                     Filters to apply
         --[no-]display-source        Show source file and line for each difference
         --[no-]validate-references "before,require,subscribe,notify"
                                      References to validate
@@ -424,6 +426,20 @@ on which this is running. (<a href="../lib/octocatalog-diff/catalog-diff/cli/opt
     </td>
   </tr>
 
+  <tr>
+    <td valign=top>
+      <pre><code>--filters FILTER1[,FILTER2[,...]]</code></pre>
+    </td>
+    <td valign=top>
+      Filters to apply
+    </td>
+    <td valign=top>
+      Specify one or more filters to apply to the results of the catalog difference.
+For a list of available filters and further explanation, please refer to
+[Filtering results](/doc/advanced-filter.md). (<a href="../lib/octocatalog-diff/catalog-diff/cli/options/filters.rb">filters.rb</a>)
+    </td>
+  </tr>
+
   <tr>
     <td valign=top>
       <pre><code>-f FROM_BRANCH

lib/octocatalog-diff/catalog-diff/cli/options/filters.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# Specify one or more filters to apply to the results of the catalog difference.
+# For a list of available filters and further explanation, please refer to
+# [Filtering results](/doc/advanced-filter.md).
+# @param parser [OptionParser object] The OptionParser argument
+# @param options [Hash] Options hash being constructed; this is modified in this method.
+OctocatalogDiff::CatalogDiff::Cli::Options::Option.newoption(:filters) do
+  has_weight 199
+
+  def parse(parser, options)
+    parser.on('--filters FILTER1[,FILTER2[,...]]', Array, 'Filters to apply') do |x|
+      options[:filters] ||= []
+      options[:filters].concat x
+    end
+  end
+end

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

@@ -7,6 +7,7 @@
 require 'stringio'
 
 require_relative '../catalog'
+require_relative 'filter'
 
 module OctocatalogDiff
   module CatalogDiff
@@ -153,6 +154,9 @@ def catdiff
         # out any such parameters from the result array.
         filter_diffs_for_absent_files(result) if @opts[:suppress_absent_file_details]
 
+        # Apply any additional pluggable filters.
+        OctocatalogDiff::CatalogDiff::Filter.apply_filters(result, @opts[:filters])
+
         # That's it!
         @logger.debug "Exiting catdiff; change count: #{result.size}"
         result
@@ -175,7 +179,7 @@ def filter_diffs_for_absent_files(result)
         absent_files = Set.new
         result.each do |diff|
           next unless diff[0] == '~' || diff[0] == '!'
-          next unless diff[1] =~ /^File\f(.+)\fparameters\fensure$/
+          next unless diff[1] =~ /^File\f([^\f]+)\fparameters\fensure$/
           next unless ['absent', 'false', false].include?(diff[3])
           absent_files.add Regexp.last_match(1)
         end

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

@@ -0,0 +1,38 @@
+require_relative 'filter/yaml'
+
+module OctocatalogDiff
+  module CatalogDiff
+    # Filtering of diffs, and parent class for inheritance.
+    class Filter
+      # Public: Apply multiple filters by repeatedly calling the `filter` method for each
+      # filter in an array. This method returns nothing.
+      #
+      # @param result [Array] Difference array (mutated)
+      # @param filter_names [Array] Filters to run
+      # @param options [Hash] Options for each filter (hashed by name)
+      def self.apply_filters(result, filter_names, options = {})
+        return unless filter_names.is_a?(Array)
+        filter_names.each { |x| filter(result, x, options[x] || {}) }
+      end
+
+      # Public: Perform a filter on `result` using the specified filter class.
+      # This mutates `result` by removing items that are ignored. This method
+      # returns nothing.
+      #
+      # @param result [Array] Difference array (mutated)
+      # @param filter_class_name [String] Filter class name (from `filter` subdirectory)
+      # @param options [Hash] Additional options (optional) to pass to filtered? method
+      def self.filter(result, filter_class_name, options = {})
+        filter_class_name = [name.to_s, filter_class_name].join('::')
+        clazz = Kernel.const_get(filter_class_name)
+        result.reject! { |item| clazz.filtered?(item, options) }
+      end
+
+      # Inherited: Construct a default `filtered?` method for the subclass via inheritance.
+      # Each subclass must implement this method, so the default method errors.
+      def self.filtered?(_item, _options = {})
+        raise "No `filtered?` method is implemented in #{name}"
+      end
+    end
+  end
+end

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

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'yaml'
+
+module OctocatalogDiff
+  module CatalogDiff
+    class Filter
+      # Filter based on equivalence of YAML objects for file resources with named extensions.
+      class YAML < OctocatalogDiff::CatalogDiff::Filter
+        # Public: Actually do the comparison of YAML objects for appropriate resources.
+        # Return true if the YAML objects are known to be equivalent. Return false if they
+        # are not equivalent, or if equivalence cannot be determined.
+        #
+        # @param diff [Array] 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 self.filtered?(diff, _options = {})
+          # Skip additions or removals - focus only on changes
+          return false unless diff[0] == '~' || diff[0] == '!'
+
+          # Make sure we are comparing file content for a file ending in .yaml or .yml extension
+          return false unless diff[1] =~ /^File\f([^\f]+)\.ya?ml\fparameters\fcontent$/
+
+          # Attempt to convert the old (diff[2]) and new (diff[3]) into YAML objects. Assuming
+          # that doesn't error out, the return value is whether or not they're equal.
+          obj_old = ::YAML.load(diff[2])
+          obj_new = ::YAML.load(diff[3])
+          obj_old == obj_new
+        rescue # Rescue everything - if something failed, we aren't sure what's going on, so we'll return false.
+          false
+        end
+      end
+    end
+  end
+end

spec/octocatalog-diff/fixtures/catalogs/ignore-equivalent-yaml-1.json

@@ -0,0 +1,22 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": ["settings"],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "File",
+        "title": "/tmp/foo.yaml",
+        "exported": false,
+        "parameters": {
+          "content": "foo:\n  bar: baz\n"
+        }
+      }
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/fixtures/catalogs/ignore-equivalent-yaml-2.json

@@ -0,0 +1,22 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": ["settings"],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "File",
+        "title": "/tmp/foo.yaml",
+        "exported": false,
+        "parameters": {
+          "content": "---\n  foo:\n    bar: baz\n"
+        }
+      }
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/fixtures/repos/yaml-diff/hiera.yaml

@@ -0,0 +1,10 @@
+---
+:backends:
+  - yaml
+:yaml:
+  :datadir: /var/lib/puppet/environments/%{::environment}/hieradata
+:hierarchy:
+  - roles/%{::role}
+  - common
+:merge_behavior: deeper
+:logger: console

spec/octocatalog-diff/fixtures/repos/yaml-diff/hieradata/common.yaml

@@ -0,0 +1,2 @@
+---
+  bar::parameter: default

spec/octocatalog-diff/fixtures/repos/yaml-diff/hieradata/roles/bar.yaml

@@ -0,0 +1,2 @@
+---
+  bar::parameter: bar

spec/octocatalog-diff/fixtures/repos/yaml-diff/manifests/site.pp

@@ -0,0 +1,3 @@
+node default {
+  include bar
+}

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/different-yaml.bar

@@ -0,0 +1,2 @@
+---
+  value: bar

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/different-yaml.default

@@ -0,0 +1,2 @@
+---
+  value: default

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/identical-yaml

@@ -0,0 +1,2 @@
+---
+  value: identical

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/not-yaml.bar

@@ -0,0 +1 @@
+---{ "title": "This is not YAML", "value": "bar" }

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/not-yaml.default

@@ -0,0 +1 @@
+---{ "title": "This is not YAML", "value": "default" }

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/similar-yaml.bar

@@ -0,0 +1,7 @@
+---
+structure:
+  value: bar
+  array:
+  - foo
+  - bar
+  - baz

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/similar-yaml.default

@@ -0,0 +1,7 @@
+---
+  structure:
+    value: bar
+    array:
+      - foo
+      - bar
+      - baz

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/unparseable-yaml.bar

@@ -0,0 +1,3 @@
+--- !ruby/object:This::Does::Not::Exist
+  name: foo
+  value: bar

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/files/unparseable-yaml.default

@@ -0,0 +1,3 @@
+--- !ruby/object:This::Does::Not::Exist
+  name: foo
+  value: default

spec/octocatalog-diff/fixtures/repos/yaml-diff/modules/bar/manifests/init.pp

@@ -0,0 +1,89 @@
+class bar ( $parameter ) {
+  file { '/tmp/template/different-yaml.yaml':
+    content => template('bar/different-yaml.erb')
+  }
+
+  file { "/tmp/template/different-yaml-${parameter}.yaml":
+    content => template('bar/different-yaml.erb')
+  }
+
+  file { '/tmp/template/identical-yaml.yaml':
+    content => template('bar/identical-yaml.erb')
+  }
+
+  file { '/tmp/template/not-yaml.yaml':
+    content => '{ "title": "This is not YAML" }'
+  }
+
+  file { '/tmp/template/not-yaml-2.yaml':
+    content => template('bar/not-yaml.erb')
+  }
+
+  file { "/tmp/template/not-yaml-${parameter}.yaml":
+    content => template('bar/not-yaml.erb')
+  }
+
+  file { '/tmp/template/similar-yaml.yaml':
+    content => template('bar/similar-yaml.erb')
+  }
+
+  file { '/tmp/template/similar-yaml.json':
+    content => template('bar/similar-yaml.erb')
+  }
+
+  file { '/tmp/template/unparseable-yaml.yaml':
+    content => template('bar/unparseable-yaml.erb'),
+  }
+
+  file { '/tmp/template/unparseable-yaml-2.yaml':
+    content => template('bar/unparseable-yaml-2.erb'),
+  }
+
+  file { "/tmp/template/unparseable-yaml-${parameter}.yaml":
+    content => template('bar/unparseable-yaml-2.erb'),
+  }
+
+  file { '/tmp/static/different-yaml.yaml':
+    source => "puppet:///modules/bar/different-yaml.${parameter}"
+  }
+
+  file { "/tmp/static/different-yaml-${parameter}.yaml":
+    source => "puppet:///modules/bar/different-yaml.${parameter}"
+  }
+
+  file { '/tmp/static/identical-yaml.yaml':
+    source => 'puppet:///modules/bar/identical-yaml',
+  }
+
+  file { '/tmp/static/not-yaml.yaml':
+    source => 'puppet:///modules/bar/not-yaml.default',
+  }
+
+  file { '/tmp/static/not-yaml-2.yaml':
+    source => "puppet:///modules/bar/not-yaml.${parameter}",
+  }
+
+  file { "/tmp/static/not-yaml-${parameter}.yaml":
+    source => "puppet:///modules/bar/not-yaml.${parameter}",
+  }
+
+  file { '/tmp/static/similar-yaml.yaml':
+    source => "puppet:///modules/bar/similar-yaml.${parameter}"
+  }
+
+  file { '/tmp/static/similar-yaml.json':
+    source => "puppet:///modules/bar/similar-yaml.${parameter}"
+  }
+
+  file { '/tmp/static/unparseable-yaml.yaml':
+    source => 'puppet:///modules/bar/unparseable-yaml.default',
+  }
+
+  file { '/tmp/static/unparseable-yaml-2.yaml':
+    source => "puppet:///modules/bar/unparseable-yaml.${parameter}",
+  }
+
+  file { "/tmp/static/unparseable-yaml-${parameter}.yaml":
+    source => "puppet:///modules/bar/unparseable-yaml.${parameter}",
+  }
+}