Merge pull request #251 from kpaulisse/kpaulisse-file-compare-force

Add "force" option for compare-file-text

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:

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 @@ module OctocatalogDiff
         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/integration/convert_file_resources_spec.rb

@@ -147,6 +147,71 @@ describe 'convert file resources' do
     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(

spec/octocatalog-diff/tests/cli/options/compare_file_text_spec.rb

@@ -4,6 +4,27 @@ require_relative '../options_helper'
 
 describe OctocatalogDiff::Cli::Options do
   describe '#opt_compare_file_text' do
-    include_examples 'true/false option', 'compare-file-text', :compare_file_text
+    it 'should set options[:compare_file_text] to true when --compare-file-text is set with no extra argument' do
+      result = run_optparse(['--compare-file-text'])
+      expect(result[:compare_file_text]).to eq(true)
+    end
+
+    it 'should set options[:compare_file_text] to :force when --compare-file-text is set to force' do
+      result = run_optparse(['--compare-file-text=force'])
+      expect(result[:compare_file_text]).to eq(:force)
+    end
+
+    it 'should set options[:compare_file_text] to false when --no-compare-file-text is set' do
+      result = run_optparse(['--no-compare-file-text'])
+      expect(result[:compare_file_text]).to eq(false)
+    end
+
+    it 'should raise if an unnecessary argument is passed to --compare-file-text' do
+      expect { run_optparse(['--no-compare-file-text=blah']) }.to raise_error(OptionParser::NeedlessArgument)
+    end
+
+    it 'should raise if an argument is passed to --no-compare-file-text' do
+      expect { run_optparse(['--no-compare-file-text=force']) }.to raise_error(OptionParser::NeedlessArgument)
+    end
   end
 end

spec/octocatalog-diff/tests/util/catalogs_spec.rb

@@ -290,6 +290,32 @@ describe OctocatalogDiff::Util::Catalogs do
       expect(result[:to].builder.to_s).to eq('OctocatalogDiff::Catalog::JSON')
     end
 
+    it 'should leave --compare-file-text enabled when forced while using a backend that does not support it' do
+      options = {
+        from_puppetdb: true,
+        to_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/tiny-catalog.json'),
+        compare_file_text: :force,
+        node: 'tiny-catalog-2-puppetdb',
+        basedir: '/asdflkj/asdflkj/asldfjk',
+        from_branch: 'foo'
+      }
+      allow(OctocatalogDiff::PuppetDB).to receive(:new) do |*_arg|
+        OctocatalogDiff::Mocks::PuppetDB.new
+      end
+      logger, logger_str = OctocatalogDiff::Spec.setup_logger
+      testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
+      result = testobj.send(:build_catalog_parallelizer)
+      expect(logger_str.string).to match(/Initialized OctocatalogDiff::Catalog::JSON for to-catalog/)
+      expect(logger_str.string).to match(/Initialized OctocatalogDiff::Catalog::PuppetDB for from-catalog/)
+      rexp = /--compare-file-text is force-enabled even though it is not supported by OctocatalogDiff::Catalog::PuppetDB/
+      expect(logger_str.string).to match(rexp)
+      expect(result).to be_a_kind_of(Hash)
+      expect(result[:from]).to be_a_kind_of(OctocatalogDiff::Catalog)
+      expect(result[:to]).to be_a_kind_of(OctocatalogDiff::Catalog)
+      expect(result[:from].builder.to_s).to eq('OctocatalogDiff::Catalog::PuppetDB')
+      expect(result[:to].builder.to_s).to eq('OctocatalogDiff::Catalog::JSON')
+    end
+
     it 'should raise OctocatalogDiff::Errors::CatalogError if either catalog fails' do
       options = {
         to_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/tiny-catalog.json'),