Merge pull request #84 from github/kpaulisse-json-filter

Add JSON equivalence filter

.version

@@ -1 +1 @@
-1.0.0
+1.0.1

doc/CHANGELOG.md

@@ -8,6 +8,15 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>1.0.1</td>
+<td>2017-02-14</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/84">#84</a>: Add JSON equivalence filter</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/83">#83</a>: Retries for Puppet Master retrieval</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/82">#82</a>: Command line option for Puppet Master timeout</li>
+</td>
+</tr>
+<tr valign=top>
 <td>1.0.0</td>
 <td>2017-02-06</td>
 <td>

doc/advanced-filter.md

@@ -10,6 +10,7 @@ Please note that there are other options to ignore specified diffs, including:
 Here is the list of available filters and an explanation of each:
 
 - [Absent File](/doc/advanced-filter.md#absent-file) - Ignore parameter changes of a file that is declared to be absent
+- [JSON](/doc/advanced-filter.md#json) - Ignore whitespace differences if JSON parses to the same object
 - [YAML](/doc/advanced-filter.md#yaml) - Ignore whitespace/comment differences if YAML parses to the same object
 
 ## Absent File
@@ -69,6 +70,20 @@ Wouldn't it be nice if the meaningless information didn't appear, and all you sa
       + absent
 ```
 
+## JSON
+
+#### Usage
+
+```
+--filters JSON
+```
+
+#### Description
+
+If a file resource has extension `.json` and a difference in its content is observed, JSON objects are constructed from the previous and new values. If these JSON 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. Note that changes to files may still trigger Puppet to restart services even though these changes are not displayed in the octocatalog-diff output.
+
 ## YAML
 
 #### Usage
@@ -81,4 +96,4 @@ Wouldn't it be nice if the meaningless information didn't appear, and all you sa
 
 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.
+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. Also, changes to files may still trigger Puppet to restart services even though these changes are not displayed in the octocatalog-diff output.

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

@@ -1,6 +1,7 @@
 require_relative '../api/v1/diff'
 require_relative 'filter/absent_file'
 require_relative 'filter/compilation_dir'
+require_relative 'filter/json'
 require_relative 'filter/yaml'
 
 require 'stringio'
@@ -12,7 +13,7 @@ class Filter
       attr_accessor :logger
 
       # List the available filters here (by class name) for use in the validator method.
-      AVAILABLE_FILTERS = %w(AbsentFile CompilationDir YAML).freeze
+      AVAILABLE_FILTERS = %w(AbsentFile CompilationDir JSON YAML).freeze
 
       # Public: Determine whether a particular filter exists. This can be used to validate
       # a user-submitted filter.

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

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require_relative '../filter'
+
+require 'json'
+
+module OctocatalogDiff
+  module CatalogDiff
+    class Filter
+      # Filter based on equivalence of JSON objects for file resources with named extensions.
+      class JSON < OctocatalogDiff::CatalogDiff::Filter
+        # Public: Actually do the comparison of JSON objects for appropriate resources.
+        # Return true if the JSON objects are known to be equivalent. Return false if they
+        # are not equivalent, or if equivalence cannot be determined.
+        #
+        # @param diff_in [OctocatalogDiff::API::V1::Diff] Difference
+        # @param _options [Hash] Additional options (there are none for this filter)
+        # @return [Boolean] true if this difference is a JSON file with identical objects, false otherwise
+        def filtered?(diff, _options = {})
+          # Skip additions or removals - focus only on changes
+          return false unless diff.change?
+
+          # Make sure we are comparing file content for a file ending in .json extension
+          return false unless diff.type == 'File' && diff.structure == %w(parameters content)
+          return false unless diff.title =~ /\.json\z/i
+
+          # Attempt to convert the old value and new value into JSON objects. Assuming
+          # that doesn't error out, the return value is whether or not they're equal.
+          obj_old = ::JSON.parse(diff.old_value)
+          obj_new = ::JSON.parse(diff.new_value)
+          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/repos/json-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/json-diff/hieradata/common.yaml

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

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

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

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

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

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

@@ -0,0 +1 @@
+{"value": "bar"}

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

@@ -0,0 +1 @@
+{"value": "default"}

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

@@ -0,0 +1 @@
+{"value": "identical"}

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

@@ -0,0 +1,3 @@
+---
+  title: "This is not JSON"
+  value: "bar"

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

@@ -0,0 +1,3 @@
+---
+  title: "This is not JSON"
+  value: "default"

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

@@ -0,0 +1,10 @@
+{
+  "structure": {
+    "value": "bar",
+    "array": [
+      "foo",
+      "bar",
+      "baz"
+    ]
+  }
+}

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

@@ -0,0 +1 @@
+{"structure":{"value":"bar","array":["foo","bar","baz"]}}

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

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

spec/octocatalog-diff/fixtures/repos/json-diff/modules/bar/templates/different-json.erb

@@ -0,0 +1 @@
+{ "value": "<%= @parameter %>" }

spec/octocatalog-diff/fixtures/repos/json-diff/modules/bar/templates/identical-json.erb

@@ -0,0 +1 @@
+{ "value": "nachos" }

spec/octocatalog-diff/fixtures/repos/json-diff/modules/bar/templates/not-json.erb

@@ -0,0 +1,3 @@
+---
+  title: "This is not JSON"
+  value: "<%= @parameter %>"

spec/octocatalog-diff/fixtures/repos/json-diff/modules/bar/templates/similar-json.erb

@@ -0,0 +1,14 @@
+<% if @parameter == 'default' %>
+{
+  "structure": {
+    "value": "bar",
+    "array": [
+      "foo",
+      "bar",
+      "baz"
+    ]
+  }
+}
+<% else %>
+{"structure":{"value":"bar","array":["foo","bar","baz"]}}
+<% end %>

spec/octocatalog-diff/integration/json_diff_spec.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require_relative 'integration_helper'
+require 'json'
+
+describe 'JSON file suppression for identical diffs' do
+  context 'with JSON diff suppression disabled' do
+    before(:all) do
+      argv = ['-n', 'rspec-node.github.net', '--to-fact-override', 'role=bar']
+      hash = { hiera_config: 'hiera.yaml', spec_fact_file: 'facts.yaml', spec_repo: 'json-diff' }
+      @result = OctocatalogDiff::Integration.integration(hash.merge(argv: argv))
+    end
+
+    it 'should compile without error' do
+      expect(@result.exitcode).to eq(2), OctocatalogDiff::Integration.format_exception(@result)
+      expect(@result.exception).to be_nil
+    end
+
+    it 'should contain the correct number of diffs' do
+      expect(@result.diffs.size).to eq(16)
+    end
+
+    it 'should contain the "similar JSON" static file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/static/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+
+    it 'should contain the "similar JSON" static file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/static/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+
+    it 'should contain the "similar JSON" template file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/template/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+
+    it 'should contain the "similar JSON" template file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/template/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+  end
+
+  context 'with JSON diff suppression enabled' do
+    before(:all) do
+      argv = ['-n', 'rspec-node.github.net', '--to-fact-override', 'role=bar', '--filters', 'JSON']
+      hash = { hiera_config: 'hiera.yaml', spec_fact_file: 'facts.yaml', spec_repo: 'json-diff' }
+      @result = OctocatalogDiff::Integration.integration(hash.merge(argv: argv))
+    end
+
+    it 'should compile without error' do
+      expect(@result.exitcode).to eq(2), OctocatalogDiff::Integration.format_exception(@result)
+      expect(@result.exception).to be_nil
+    end
+
+    it 'should contain the correct number of diffs' do
+      expect(@result.diffs.size).to eq(14)
+    end
+
+    it 'should not contain the "similar JSON" static file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/static/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(false)
+    end
+
+    it 'should contain the "similar JSON" YAML static file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/static/similar-json.yaml', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+
+    it 'should not contain the "similar JSON" template file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/template/similar-json.json', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(false)
+    end
+
+    it 'should contain the "similar JSON" YAML template file as a diff' do
+      resource = { diff_type: '~', type: 'File', title: '/tmp/template/similar-json.yaml', structure: %w(parameters content) }
+      expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, resource)).to eq(true)
+    end
+  end
+end