Skip to content

Add "force" option for compare-file-text #251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/advanced-compare-file-text.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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:
Expand Down
20 changes: 18 additions & 2 deletions lib/octocatalog-diff/cli/options/compare_file_text.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/octocatalog-diff/util/catalogs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
65 changes: 65 additions & 0 deletions spec/octocatalog-diff/integration/convert_file_resources_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 26 additions & 0 deletions spec/octocatalog-diff/tests/util/catalogs_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down