Skip to content

Commit

Permalink
Add --compare-file-text=soft
Browse files Browse the repository at this point in the history
  • Loading branch information
kpaulisse committed Aug 23, 2021
1 parent 5071b97 commit f372333
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 14 deletions.
15 changes: 12 additions & 3 deletions doc/advanced-compare-file-text.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
18 changes: 14 additions & 4 deletions lib/octocatalog-diff/catalog-util/fileresources.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -80,8 +90,8 @@ def self.module_path(dir)
# @param compilation_dir [String] Compilation directory
# @param environment [String] Environment
# @param ignore_tags [Array<String>] 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 != ''
Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions lib/octocatalog-diff/cli/options/compare_file_text.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,19 @@
#
# 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.
OctocatalogDiff::Cli::Options::Option.newoption(:compare_file_text) do
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
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
Expand Down
2 changes: 1 addition & 1 deletion lib/octocatalog-diff/util/catalogs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 42 additions & 1 deletion spec/octocatalog-diff/integration/convert_file_resources_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f372333

Please sign in to comment.