diff --git a/doc/advanced-compare-file-text.md b/doc/advanced-compare-file-text.md index 34a88240..67cf913e 100644 --- a/doc/advanced-compare-file-text.md +++ b/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: diff --git a/lib/octocatalog-diff/cli/options/compare_file_text.rb b/lib/octocatalog-diff/cli/options/compare_file_text.rb index 051d21de..a5b465b6 100644 --- a/lib/octocatalog-diff/cli/options/compare_file_text.rb +++ b/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 diff --git a/lib/octocatalog-diff/util/catalogs.rb b/lib/octocatalog-diff/util/catalogs.rb index 27b28478..43d93877 100644 --- a/lib/octocatalog-diff/util/catalogs.rb +++ b/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| diff --git a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb index 811acaa6..b3cd50d1 100644 --- a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb +++ b/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( diff --git a/spec/octocatalog-diff/tests/cli/options/compare_file_text_spec.rb b/spec/octocatalog-diff/tests/cli/options/compare_file_text_spec.rb index 40f259a6..9ecd6894 100644 --- a/spec/octocatalog-diff/tests/cli/options/compare_file_text_spec.rb +++ b/spec/octocatalog-diff/tests/cli/options/compare_file_text_spec.rb @@ -4,6 +4,27 @@ 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 diff --git a/spec/octocatalog-diff/tests/util/catalogs_spec.rb b/spec/octocatalog-diff/tests/util/catalogs_spec.rb index 9c4acadd..27f3e2ca 100644 --- a/spec/octocatalog-diff/tests/util/catalogs_spec.rb +++ b/spec/octocatalog-diff/tests/util/catalogs_spec.rb @@ -290,6 +290,32 @@ def valid? 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'),