diff --git a/doc/advanced-compare-file-text.md b/doc/advanced-compare-file-text.md index 67cf913e..cd817374 100644 --- a/doc/advanced-compare-file-text.md +++ b/doc/advanced-compare-file-text.md @@ -40,11 +40,20 @@ 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` +### `--compare-file-text=soft` and `--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. +To enable the addition of generated "file" resources with content in each generated catalog, set the command line argument `--compare-file-text=soft` or `--compare-file-text=force`. When the Puppet source code is available, e.g. when compiling a catalog with `--catalog-only`, this will still adjust the resources in 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. +With this option set to `force`, if a file that is referenced cannot be resolved, an exception will raised. When set to `soft` no exception will be raised. + +If the Puppet source code is not available, either of these options may end up causing the program to malfunction. Use these options at your own risk. + +The configuration file equivalents of these options are: + +```ruby +settings[:compare_file_text] = :soft +settings[:compare_file_text] = :force +``` ### `--compare-file-text-ignore-tags` diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 0c8b24f9..b6ec7564 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -21,7 +21,7 @@ def self.convert_file_resources(obj, environment = 'production') obj.compilation_dir, environment, obj.options[:compare_file_text_ignore_tags], - obj.options[:tag] + should_raise_notfound?(obj) ) begin obj.catalog_json = ::JSON.generate(obj.catalog) @@ -30,6 +30,16 @@ def self.convert_file_resources(obj, environment = 'production') end end + # Internal method: Based on parameters, determine whether a "not found" for a file that fails + # to be located should result in an exception. + # @param obj [OctocatalogDiff::Catalog] Catalog object + # @return [Bool] Whether to raise if not found + def self.should_raise_notfound?(obj) + return true if obj.options[:compare_file_text] == :force + return false if obj.options[:compare_file_text] == :soft + obj.options[:tag] != 'from' + end + # Internal method: Locate a file that is referenced at puppet:///modules/xxx/yyy using the # module path that is specified within the environment.conf file (assuming the default 'modules' # directory doesn't exist or the module isn't found in there). If the file can't be found then @@ -80,8 +90,8 @@ def self.module_path(dir) # @param compilation_dir [String] Compilation directory # @param environment [String] Environment # @param ignore_tags [Array] Tags that exempt a resource from conversion - # @param to_from_tag [String] Either "to" or "from" for catalog type - def self._convert_file_resources(resources, compilation_dir, environment, ignore_tags, to_from_tag) + # @param raise_notfound [Bool] Whether to raise if a file could not be found + def self._convert_file_resources(resources, compilation_dir, environment, ignore_tags, raise_notfound) # Calculate compilation directory. There is not explicit error checking here because # there is on-demand, explicit error checking for each file within the modification loop. return unless compilation_dir.is_a?(String) && compilation_dir != '' @@ -113,7 +123,7 @@ def self._convert_file_resources(resources, compilation_dir, environment, ignore # We are not handling recursive file installs from a directory or anything else. # However, the fact that we found *something* at this location indicates that the catalog # is probably correct. Hence, the very general .exist? check. - elsif to_from_tag == 'from' + elsif !raise_notfound # Don't raise an exception for an invalid source in the "from" # catalog, because the developer may be fixing this in the "to" # catalog. If it's broken in the "to" catalog as well, the diff --git a/lib/octocatalog-diff/cli/options/compare_file_text.rb b/lib/octocatalog-diff/cli/options/compare_file_text.rb index b1892095..9b6ce214 100644 --- a/lib/octocatalog-diff/cli/options/compare_file_text.rb +++ b/lib/octocatalog-diff/cli/options/compare_file_text.rb @@ -11,8 +11,9 @@ # # 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. +# set --compare-file-text=force or --compare-file-text=soft. These options allow +# the content of the file to be substituted in to --catalog-only compilations, for example. +# 'force' will raise an exception if the underlying file can't be found; 'soft' won't. # # @param parser [OptionParser object] The OptionParser argument # @param options [Hash] Options hash being constructed; this is modified in this method. @@ -20,10 +21,10 @@ has_weight 210 def parse(parser, options) - 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 [true, false].include? x + parser.on('--[no-]compare-file-text[=force|soft]', 'Compare text, not source location, of file resources') do |x| + if x == 'force' || x == 'soft' + options[:compare_file_text] = x.to_sym + elsif x == true || x == false options[:compare_file_text] = x else raise OptionParser::NeedlessArgument("needless argument: --compare-file-text=#{x}") diff --git a/lib/octocatalog-diff/util/catalogs.rb b/lib/octocatalog-diff/util/catalogs.rb index 43d93877..50100353 100644 --- a/lib/octocatalog-diff/util/catalogs.rb +++ b/lib/octocatalog-diff/util/catalogs.rb @@ -72,7 +72,7 @@ def build_catalog_parallelizer result.each do |_key, builder_obj| next if builder_obj.convert_file_resources(true) - if @options[:compare_file_text] == :force + if @options[:compare_file_text] == :force || @options[:compare_file_text] == :soft @logger.debug "--compare-file-text is force-enabled even though it is not supported by #{builder_obj.builder}" next end diff --git a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb index b3cd50d1..5ab8c2c0 100644 --- a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb +++ b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb @@ -212,6 +212,47 @@ end end + context 'with option force-enabled and broken reference in catalog-only' do + it 'should fail' 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/broken'), + '--puppet-binary', OctocatalogDiff::Spec::PUPPET_BINARY, + '--debug' + ] + ) + expect(result[:exitcode]).to eq(1) + expect(result[:stderr]).to match(%r{Unable to resolve source=>'puppet:///modules/test/foo-new' in File\[/tmp/foo1\] \(modules/test/manifests/init.pp:\d+\)}) # rubocop:disable Metrics/LineLength + end + end + + context 'with option force-enabled to "from" and broken reference in catalog-only' do + it 'should not fail' 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=soft', + '--basedir', OctocatalogDiff::Spec.fixture_path('repos/convert-resources/broken'), + '--puppet-binary', OctocatalogDiff::Spec::PUPPET_BINARY, + '--debug' + ] + ) + expect(result[:exitcode]).to eq(0), OctocatalogDiff::Integration.format_exception(result) + catalog = OctocatalogDiff::Catalog::JSON.new(json: result[:stdout]) + resource = catalog.resource(type: 'File', title: '/tmp/foo1') + expect(resource).to be_a_kind_of(Hash) + expect(resource['parameters']).to be_a_kind_of(Hash) + expect(resource['parameters']['source']).to eq('puppet:///modules/test/foo-new') + expect(resource['parameters']['content']).to be nil + end + end + context 'with broken reference in to-catalog' do it 'should fail' do result = OctocatalogDiff::Integration.integration( @@ -223,7 +264,7 @@ '--compare-file-text' ] ) - expect(result[:exitcode]).to eq(-1) + expect(result[:exitcode]).to eq(-1), OctocatalogDiff::Integration.format_exception(result) expect(result[:exception]).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError) expect(result[:exception].message).to match(%r{\AUnable to resolve source=>'puppet:///modules/test/foo-new' in File\[/tmp/foo1\] \(modules/test/manifests/init.pp:\d+\)\z}) # rubocop:disable Metrics/LineLength end 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 9ecd6894..a2eefcb6 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 @@ -14,6 +14,11 @@ expect(result[:compare_file_text]).to eq(:force) end + it 'should set options[:compare_file_text] to :soft when --compare-file-text is set to soft' do + result = run_optparse(['--compare-file-text=soft']) + expect(result[:compare_file_text]).to eq(:soft) + 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)