Merge branch 'master' into bugfix_crl

Author: Anthony Riley II

doc/advanced-compare-file-text.md

@@ -21,6 +21,7 @@ However, since applying the catalog could change the content of a file on the ta
 This feature is available only when the catalogs are being compiled from local code. This feature is not available, and will be automatically disabled, when pulling catalogs from PuppetDB or a Puppet server.
 
 Note: In Puppet >= 4.4 there is an option in Puppet itself called "static catalogs" which if enabled will cause the checksum of the file to be included in the catalog. However, the `octocatalog-diff` feature described here is still useful because it can be used to display a "diff" of the change rather than just displaying a "diff" of a checksum.
+
 ## Command line options
 
 ### `--compare-file-text` and `--no-compare-file-text`
@@ -39,6 +40,12 @@ If this feature is disabled by default in a configuration file, add `--compare-f
 
 Note that the feature will be automatically disabled, regardless of configuration or command line options, if catalogs are being pulled from PuppetDB or a Puppet server.
 
+### `--compare-file-text=force`
+
+To force the option to be on even in situations when it would be auto-disabled, set the command line argument `--compare-file-text=force`. When the Puppet source code is available, e.g. when compiling a catalog with `--catalog-only`, this will adjust the resulting catalog.
+
+If the Puppet source code is not available, forcing the feature on anyway may end up causing an exception. Use this option at your own risk.
+
 ### `--compare-file-text-ignore-tags`
 
 To disable this feature for specific `file` resources, set a tag on the resources for which the comparison is undesired. For example:

doc/advanced-filter.md

@@ -24,7 +24,7 @@ Here is the list of available filters and an explanation of each:
 
 #### Description
 
-When the `AbsentFile` filter is enabled, if any file is `ensure => absent` in the *new* catalog, then changes to any other parameters will be suppressed.
+When the `AbsentFile` filter is enabled, if any file is `ensure => absent` in the _new_ catalog, then changes to any other parameters will be suppressed.
 
 Consider that a file resource is declared as follows in two catalogs:
 
@@ -71,6 +71,25 @@ Wouldn't it be nice if the meaningless information didn't appear, and all you sa
       + absent
 ```
 
+## Equivalent Array (not considering datatypes)
+
+#### Usage
+
+```
+--filters EquivalentArrayNoDatatypes
+```
+
+#### Description
+
+In an array, ignore changes where the old and new arrays are "equivalent" as described below. This is useful when octocatalog-diff is comparing changes between a catalog with stringified values and a catalog with non-stringified values.
+
+The following are considered equivalent when this filter is engaged:
+
+- Stringified integers (`[0, 1]` and `['0', '1']`)
+- Stringified floats (`[0.0, 1.0]` and `['0.0', '1.0']`)
+- Numerically-equal integers and floats (`[0, 1]` and `[0.0, 1.0]`)
+- Symbols and corresponding strings (`[:foo, :bar]` and `[':foo', ':bar']`)
+
 ## JSON
 
 #### Usage
@@ -105,7 +124,7 @@ New: { "notify": [ "Service[foo]" ] }
 This filter will suppress differences for the value of a parameter when:
 
 - The value in one catalog is an object, AND
-- The value in the other catalog is an array containing *only* that same object
+- The value in the other catalog is an array containing _only_ that same object
 
 ## YAML
 

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

@@ -558,7 +558,7 @@ def hashdiff_initial(catalog1_in, catalog2_in)
 
           # Added a new key that points to some kind of data structure that we know how
           # to handle.
-          classes = [String, Integer, Float, TrueClass, FalseClass, Array, Hash]
+          classes = [String, Integer, Float, TrueClass, FalseClass, Array, Hash, NilClass]
           if obj[1] =~ /^(.+)\f([^\f]+)$/ && OctocatalogDiff::Util::Util.object_is_any_of?(obj[2], classes)
             hashdiff_add_remove.add(obj[1])
             next

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/equivalent_array_no_datatypes'
 require_relative 'filter/json'
 require_relative 'filter/single_item_array'
 require_relative 'filter/yaml'
@@ -14,7 +15,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 JSON SingleItemArray YAML).freeze
+      AVAILABLE_FILTERS = %w(AbsentFile CompilationDir EquivalentArrayNoDatatypes JSON SingleItemArray 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/equivalent_array_no_datatypes.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require_relative '../filter'
+
+module OctocatalogDiff
+  module CatalogDiff
+    class Filter
+      # Filter out changes in parameters where the elements of an array are the
+      # same values but different data types. For example, this would filter out
+      # the following diffs:
+      #   Exec[some command] =>
+      #    parameters =>
+      #      returns =>
+      #        - ["0", "1"]
+      #        + [0, 1]
+      class EquivalentArrayNoDatatypes < OctocatalogDiff::CatalogDiff::Filter
+        # Public: Implement the filter for arrays that have the same elements
+        # but possibly different data types.
+        #
+        # @param diff [OctocatalogDiff::API::V1::Diff] Difference
+        # @param _options [Hash] Additional options (there are none for this filter)
+        # @return [Boolean] true if this should be filtered out, false otherwise
+        def filtered?(diff, _options = {})
+          # Skip additions or removals - focus only on changes
+          return false unless diff.change?
+          old_value = diff.old_value
+          new_value = diff.new_value
+
+          # Skip unless both the old and new values are arrays.
+          return false unless old_value.is_a?(Array) && new_value.is_a?(Array)
+
+          # Avoid generating comparable values if the arrays are a different
+          # size, because there's no possible way that they are equivalent.
+          return false unless old_value.size == new_value.size
+
+          # Generate and then compare the comparable arrays.
+          old_value.map { |x| comparable_value(x) } == new_value.map { |x| comparable_value(x) }
+        end
+
+        # Private: Get a more easily comparable value for an array element.
+        # Integers, floats, and strings that look like integers or floats become
+        # floats, and symbols are converted to string representation.
+        #
+        # @param input [any] Value to convert
+        # @return [any] "Comparable" value
+        def comparable_value(input)
+          # Any string that looks like a number is converted to a float.
+          if input.is_a?(String) && input =~ /\A-?(([0-9]*\.[0-9]+)|([0-9]+))\z/
+            return input.to_f
+          end
+
+          # Any number is converted to a float
+          return input.to_f if input.is_a?(Integer) || input.is_a?(Float)
+
+          # Symbols are converted to ":xxx" strings.
+          return ":#{input}" if input.is_a?(Symbol)
+
+          # Everything else is unconverted.
+          input
+        end
+      end
+    end
+  end
+end

lib/octocatalog-diff/cli/options/compare_file_text.rb

@@ -4,14 +4,30 @@
 # the 'source' attribute and populate the 'content' attribute with the text of the file.
 # This allows for a diff of the content, rather than a diff of the location, which is
 # what is most often desired.
+#
+# This has historically been a binary option, so --compare-file-text with no argument will
+# set this to `true` and --no-compare-file-text will set this to `false`. Note that
+# --no-compare-file-text does not accept an argument.
+#
+# File text comparison will be auto-disabled in circumstances other than compiling and
+# comparing two catalogs. To force file text comparison to be enabled at other times,
+# set --compare-file-text=force. This allows the content of the file to be substituted
+# in to --catalog-only compilations, for example.
+#
 # @param parser [OptionParser object] The OptionParser argument
 # @param options [Hash] Options hash being constructed; this is modified in this method.
 OctocatalogDiff::Cli::Options::Option.newoption(:compare_file_text) do
   has_weight 210
 
   def parse(parser, options)
-    parser.on('--[no-]compare-file-text', 'Compare text, not source location, of file resources') do |x|
-      options[:compare_file_text] = x
+    parser.on('--[no-]compare-file-text[=force]', 'Compare text, not source location, of file resources') do |x|
+      if x == 'force'
+        options[:compare_file_text] = :force
+      elsif x == true || x == false
+        options[:compare_file_text] = x
+      else
+        raise OptionParser::NeedlessArgument("needless argument: --compare-file-text=#{x}")
+      end
     end
   end
 end

lib/octocatalog-diff/util/catalogs.rb

@@ -71,6 +71,12 @@ def build_catalog_parallelizer
         if @options.fetch(:compare_file_text, false)
           result.each do |_key, builder_obj|
             next if builder_obj.convert_file_resources(true)
+
+            if @options[:compare_file_text] == :force
+              @logger.debug "--compare-file-text is force-enabled even though it is not supported by #{builder_obj.builder}"
+              next
+            end
+
             @logger.debug "Disabling --compare-file-text; not supported by #{builder_obj.builder}"
             @options[:compare_file_text] = false
             catalog_tasks.map! do |x|

spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-1.json

@@ -0,0 +1,55 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": [
+      "settings"
+    ],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Exec",
+        "title": "run-my-command 1",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": "0"
+        }
+      },
+      {
+        "type": "Exec",
+        "title": "run-my-command 2",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": ["0", "1"]
+        }
+      },
+      {
+        "type": "Exec",
+        "title": "run-my-command 3",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": ["0", "1", "2"]
+        }
+      }
+    ],
+    "classes": [
+      "settings"
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-2.json

@@ -0,0 +1,55 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": [
+      "settings"
+    ],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Exec",
+        "title": "run-my-command 1",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": 0
+        }
+      },
+      {
+        "type": "Exec",
+        "title": "run-my-command 2",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": [0, 1]
+        }
+      },
+      {
+        "type": "Exec",
+        "title": "run-my-command 3",
+        "file": "/environments/production/modules/foo/manifests/init.pp",
+        "line": 10,
+        "exported": false,
+        "parameters": {
+          "path": "/usr/bin",
+          "command": "id",
+          "returns": [0, 1, 2, 3]
+        }
+      }
+    ],
+    "classes": [
+      "settings"
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/integration/convert_file_resources_spec.rb

@@ -147,6 +147,71 @@
     end
   end
 
+  context 'with option auto-disabled' do
+    before(:all) do
+      @result = OctocatalogDiff::Integration.integration_cli(
+        [
+          '--fact-file', OctocatalogDiff::Spec.fixture_path('facts/facts.yaml'),
+          '--catalog-only',
+          '-n', 'rspec-node.github.net',
+          '--compare-file-text',
+          '--basedir', OctocatalogDiff::Spec.fixture_path('repos/convert-resources/new'),
+          '--puppet-binary', OctocatalogDiff::Spec::PUPPET_BINARY,
+          '--debug'
+        ]
+      )
+    end
+
+    it 'should compile the catalog' do
+      expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result)
+      expect(@result[:exitcode]).to eq(0), "Runtime error: #{@result[:logs]}"
+    end
+
+    it 'should indicate that the option was disabled' do
+      expect(@result[:stderr]).to match(/Disabling --compare-file-text; not supported by OctocatalogDiff::Catalog::Noop/)
+    end
+
+    it 'should not have converted resources in the catalog' do
+      catalog = OctocatalogDiff::Catalog::JSON.new(json: @result[:stdout])
+      resource = catalog.resource(type: 'File', title: '/tmp/foo2')
+      expect(resource).to be_a_kind_of(Hash)
+      expect(resource['parameters']).to eq('source' => 'puppet:///modules/test/foo-old')
+    end
+  end
+
+  context 'with option force-enabled' do
+    before(:all) do
+      @result = OctocatalogDiff::Integration.integration_cli(
+        [
+          '--fact-file', OctocatalogDiff::Spec.fixture_path('facts/facts.yaml'),
+          '--catalog-only',
+          '-n', 'rspec-node.github.net',
+          '--compare-file-text=force',
+          '--basedir', OctocatalogDiff::Spec.fixture_path('repos/convert-resources/new'),
+          '--puppet-binary', OctocatalogDiff::Spec::PUPPET_BINARY,
+          '--debug'
+        ]
+      )
+    end
+
+    it 'should compile the catalog' do
+      expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result)
+      expect(@result[:exitcode]).to eq(0), "Runtime error: #{@result[:logs]}"
+    end
+
+    it 'should indicate that the option was force-enabled' do
+      rexp = /--compare-file-text is force-enabled even though it is not supported by OctocatalogDiff::Catalog::Noop/
+      expect(@result[:stderr]).to match(rexp)
+    end
+
+    it 'should have converted resources in the catalog' do
+      catalog = OctocatalogDiff::Catalog::JSON.new(json: @result[:stdout])
+      resource = catalog.resource(type: 'File', title: '/tmp/foo2')
+      expect(resource).to be_a_kind_of(Hash)
+      expect(resource['parameters']).to eq('content' => "content of foo-old\n")
+    end
+  end
+
   context 'with broken reference in to-catalog' do
     it 'should fail' do
       result = OctocatalogDiff::Integration.integration(