--validate-references
diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb
index 8621b0a0..0c8b24f9 100644
--- a/lib/octocatalog-diff/catalog-util/fileresources.rb
+++ b/lib/octocatalog-diff/catalog-util/fileresources.rb
@@ -13,9 +13,16 @@ class FileResources
# Public method: Convert file resources to text. See the description of the class
# just above for details.
# @param obj [OctocatalogDiff::Catalog] Catalog object (will be modified)
+ # @param environment [String] Environment (defaults to production)
def self.convert_file_resources(obj, environment = 'production')
return unless obj.valid? && obj.compilation_dir.is_a?(String) && !obj.compilation_dir.empty?
- _convert_file_resources(obj.resources, obj.compilation_dir, environment)
+ _convert_file_resources(
+ obj.resources,
+ obj.compilation_dir,
+ environment,
+ obj.options[:compare_file_text_ignore_tags],
+ obj.options[:tag]
+ )
begin
obj.catalog_json = ::JSON.generate(obj.catalog)
rescue ::JSON::GeneratorError => exc
@@ -70,8 +77,11 @@ def self.module_path(dir)
# Internal method: Static method to convert file resources. The compilation directory is
# required, or else this is a no-op. The passed-in array of resources is modified by this method.
# @param resources [Array] Array of catalog resources
- # @param compilation_dir [String] Compilation directory (so files can be looked up)
- def self._convert_file_resources(resources, compilation_dir, environment = 'production')
+ # @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)
# 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 != ''
@@ -88,14 +98,10 @@ def self._convert_file_resources(resources, compilation_dir, environment = 'prod
resources.map! do |resource|
if resource_convertible?(resource)
path = file_path(resource['parameters']['source'], modulepaths)
- if path.nil?
- # Pass this through as a wrapped exception, because it's more likely to be something wrong
- # in the catalog itself than it is to be a broken setup of octocatalog-diff.
- message = "Errno::ENOENT: Unable to resolve '#{resource['parameters']['source']}'!"
- raise OctocatalogDiff::Errors::CatalogError, message
- end
- if File.file?(path)
+ if resource['tags'] && ignore_tags && (resource['tags'] & ignore_tags).any?
+ # Resource tagged not to be converted -- do nothing.
+ elsif path && File.file?(path)
# If the file is found, read its content. If the content is all ASCII, substitute it into
# the 'content' parameter for easier comparison. If not, instead populate the md5sum.
# Delete the 'source' attribute as well.
@@ -103,23 +109,40 @@ def self._convert_file_resources(resources, compilation_dir, environment = 'prod
is_ascii = content.force_encoding('UTF-8').ascii_only?
resource['parameters']['content'] = is_ascii ? content : '{md5}' + Digest::MD5.hexdigest(content)
resource['parameters'].delete('source')
- elsif File.exist?(path)
+ elsif path && File.exist?(path)
# 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'
+ # 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
+ # exception will be raised when this code runs on that catalog.
else
- # This is probably a bug
- # :nocov:
- raise "Unable to find '#{resource['parameters']['source']}' at #{path}!"
- # :nocov:
+ # Pass this through as a wrapped exception, because it's more likely to be something wrong
+ # in the catalog itself than it is to be a broken setup of octocatalog-diff.
+ #
+ # Example error: 'puppet:///modules/test/tmp/bar' in File[/tmp/bar]
+ # (/x/modules/test/manifests/init.pp:46)>
+ source = resource['parameters']['source']
+ type = resource['type']
+ title = resource['title']
+ file = resource['file'].sub(Regexp.new('^' + Regexp.escape(env_dir) + '/'), '')
+ line = resource['line']
+ message = "Unable to resolve source=>'#{source}' in #{type}[#{title}] (#{file}:#{line})"
+ raise OctocatalogDiff::Errors::CatalogError, message
end
end
+
resource
end
end
# Internal method: Determine if a resource is convertible. It is convertible if it
# is a file resource with no declared 'content' and with a declared and parseable 'source'.
+ # It is not convertible if the resource is tagged with one of the tags declared by
+ # the option `--compare-file-text-ignore-tags`.
# @param resource [Hash] Resource to check
# @return [Boolean] True of resource is convertible, false if not
def self.resource_convertible?(resource)
diff --git a/lib/octocatalog-diff/cli/options/compare_file_text.rb b/lib/octocatalog-diff/cli/options/compare_file_text.rb
index 9db998e6..051d21de 100644
--- a/lib/octocatalog-diff/cli/options/compare_file_text.rb
+++ b/lib/octocatalog-diff/cli/options/compare_file_text.rb
@@ -15,3 +15,21 @@ def parse(parser, options)
end
end
end
+
+# Sometimes there is a particular file resource for which the file text
+# comparison is not desired, while not disabling that option globally. Similar
+# to --ignore_tags, it's possible to tag the file resource and exempt it from
+# the --compare_file_text checks.
+# @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_ignore_tags) do
+ has_weight 415
+
+ def parse(parser, options)
+ description = 'Tags that exclude a file resource from text comparison'
+ parser.on('--compare-file-text-ignore-tags STRING1[,STRING2[,...]]', Array, description) do |x|
+ options[:compare_file_text_ignore_tags] ||= []
+ options[:compare_file_text_ignore_tags].concat x
+ end
+ end
+end
diff --git a/spec/octocatalog-diff/fixtures/catalogs/catalog-test-file-v4-tagged.json b/spec/octocatalog-diff/fixtures/catalogs/catalog-test-file-v4-tagged.json
new file mode 100644
index 00000000..b4cc2db6
--- /dev/null
+++ b/spec/octocatalog-diff/fixtures/catalogs/catalog-test-file-v4-tagged.json
@@ -0,0 +1,57 @@
+{
+ "document_type": "Catalog",
+ "tags": ["settings","test"],
+ "name": "my.rspec.node",
+ "version": "production",
+ "environment": "production",
+ "resources": [
+ {
+ "type": "Stage",
+ "title": "main",
+ "tags": ["stage"],
+ "exported": false,
+ "parameters": {
+ "name": "main"
+ }
+ },
+ {
+ "type": "Class",
+ "title": "Settings",
+ "tags": ["class","settings"],
+ "exported": false
+ },
+ {
+ "type": "File",
+ "title": "/tmp/foo",
+ "tags": ["file","class"],
+ "file": "/x/modules/test/manifests/init.pp",
+ "line": 37,
+ "exported": false,
+ "parameters": {
+ "backup": false,
+ "mode": "0440",
+ "owner": "root",
+ "group": "root",
+ "source": "puppet:///modules/test/tmp/foo"
+ }
+ },
+ {
+ "type": "File",
+ "title": "/tmp/bar",
+ "tags": ["file","class","no_compare_tag"],
+ "file": "/x/modules/test/manifests/init.pp",
+ "line": 46,
+ "exported": false,
+ "parameters": {
+ "backup": false,
+ "mode": "0440",
+ "owner": "root",
+ "group": "root",
+ "source": "puppet:///modules/test/tmp/bar"
+ }
+ }
+ ],
+ "classes": [
+ "test"
+ ]
+}
diff --git a/spec/octocatalog-diff/fixtures/repos/convert-resources/broken/modules/test/manifests/init.pp b/spec/octocatalog-diff/fixtures/repos/convert-resources/broken/modules/test/manifests/init.pp
index 5be6fa10..6f436741 100644
--- a/spec/octocatalog-diff/fixtures/repos/convert-resources/broken/modules/test/manifests/init.pp
+++ b/spec/octocatalog-diff/fixtures/repos/convert-resources/broken/modules/test/manifests/init.pp
@@ -1,9 +1,11 @@
class test {
file { '/tmp/foo1':
+ tag => ['_convert_file_resources_foo1_'],
source => 'puppet:///modules/test/foo-new',
}
file { '/tmp/foo2':
+ tag => ['_convert_file_resources_foo2_'],
source => 'puppet:///modules/test/foo-old',
}
}
diff --git a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb
index c7327599..811acaa6 100644
--- a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb
+++ b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb
@@ -147,7 +147,7 @@
end
end
- context 'with broken repo' do
+ context 'with broken reference in to-catalog' do
it 'should fail' do
result = OctocatalogDiff::Integration.integration(
spec_fact_file: 'facts.yaml',
@@ -160,8 +160,84 @@
)
expect(result[:exitcode]).to eq(-1)
expect(result[:exception]).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
- expect(result[:exception].message).to match(/Errno::ENOENT/)
- expect(result[:exception].message).to match(%r{Unable to resolve 'puppet:///modules/test/foo-new'})
+ 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
+
+ context 'with --convert-file-resources-ignore-tags' do
+ before(:all) do
+ @result = OctocatalogDiff::Integration.integration(
+ spec_fact_file: 'facts.yaml',
+ spec_repo_old: 'convert-resources/old',
+ spec_repo_new: 'convert-resources/broken',
+ argv: [
+ '-n', 'rspec-node.github.net',
+ '--compare-file-text',
+ '--compare-file-text-ignore-tags', '_convert_file_resources_foo1_'
+ ]
+ )
+ end
+
+ it 'should compile the catalog' do
+ expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result)
+ expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}"
+ expect(@result[:diffs]).to be_a_kind_of(Array)
+ end
+
+ it 'should leave /tmp/foo1 parameters:source alone in the new catalog' do
+ resource = {
+ diff_type: '!',
+ type: 'File',
+ title: '/tmp/foo1',
+ structure: %w(parameters source),
+ old_value: nil,
+ new_value: 'puppet:///modules/test/foo-new'
+ }
+ expect(OctocatalogDiff::Spec.diff_match?(@result[:diffs], resource)).to eq(true)
+ end
+ end
+ end
+
+ context 'with broken reference in from-catalog' do
+ before(:all) do
+ @result = OctocatalogDiff::Integration.integration(
+ spec_fact_file: 'facts.yaml',
+ spec_repo_old: 'convert-resources/broken',
+ spec_repo_new: 'convert-resources/old',
+ argv: [
+ '-n', 'rspec-node.github.net',
+ '--compare-file-text'
+ ]
+ )
+ end
+
+ it 'should succeed' do
+ expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result)
+ expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}"
+ expect(@result[:diffs]).to be_a_kind_of(Array)
+ end
+
+ it 'should leave /tmp/foo1 parameters:source alone in the old catalog' do
+ resource = {
+ diff_type: '!',
+ type: 'File',
+ title: '/tmp/foo1',
+ structure: %w(parameters source),
+ old_value: 'puppet:///modules/test/foo-new',
+ new_value: nil
+ }
+ expect(OctocatalogDiff::Spec.diff_match?(@result[:diffs], resource)).to eq(true)
+ end
+
+ it 'should populate /tmp/foo1 parameters:content in the new catalog' do
+ resource = {
+ diff_type: '!',
+ type: 'File',
+ title: '/tmp/foo1',
+ structure: %w(parameters content),
+ old_value: nil,
+ new_value: "content of foo-old\n"
+ }
+ expect(OctocatalogDiff::Spec.diff_match?(@result[:diffs], resource)).to eq(true)
end
end
@@ -256,9 +332,7 @@
it 'should fail' do
expect(@result[:exitcode]).to eq(-1)
expect(@result[:exception]).to be_a_kind_of(OctocatalogDiff::Errors::CatalogError)
- expect(@result[:exception].message).to match(/Errno::ENOENT/)
- rexp = Regexp.new(Regexp.escape("Unable to resolve '[\"puppet:///modules/test/foo-bar\""))
- expect(@result[:exception].message).to match(rexp)
+ expect(@result[:exception].message).to match(%r{\AUnable to resolve source=>'\["puppet:///modules/test/foo-bar", "puppet:///modules/test/foo-baz"\]' in File\[/tmp/foo\] \(modules/test/manifests/array3.pp:\d+\)\z}) # rubocop:disable Metrics/LineLength
end
end
end
diff --git a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb
index 1bb4cef3..10d37898 100644
--- a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb
+++ b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb
@@ -225,9 +225,10 @@ def catalog_from_fixture(path)
obj.resources[2]['parameters']['source'] = 'puppet:///modules/this/does/not/exist'
# Perform test
+ expected_error = "Unable to resolve source=>'puppet:///modules/this/does/not/exist' in File[/tmp/foo] (/x/modules/test/manifests/init.pp:37)" # rubocop:disable Metrics/LineLength
expect do
OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
- end.to raise_error(OctocatalogDiff::Errors::CatalogError, %r{Unable to resolve 'puppet:///modules/this/does/not/exist'})
+ end.to raise_error(OctocatalogDiff::Errors::CatalogError, expected_error)
end
it 'should return original if compilation_dir is not a string' do
@@ -276,6 +277,61 @@ def catalog_from_fixture(path)
OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
expect(obj.error_message).to eq('Failed to generate JSON: test')
end
+
+ it 'should pass options[:compare_file_text_ignore_tags] properly' do
+ # Set up test
+ obj = catalog_from_fixture('catalogs/catalog-test-file-v4-tagged.json')
+ obj.compilation_dir = @tmpdir
+ obj.options[:compare_file_text_ignore_tags] = %w[no_compare_tag]
+ resources_save = obj.resources.dup
+ File.open(File.join(@tmpdir, 'modules', 'test', 'files', 'tmp', 'foo'), 'w') { |f| f.write "\u0256" }
+
+ # Perform test
+ OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
+ expect(obj.resources).to be_a_kind_of(Array)
+ expect(obj.resources.size).to eq(4)
+ expect(obj.resources[0]).to eq(resources_save[0])
+ expect(obj.resources[1]).to eq(resources_save[1])
+ expect(obj.resources[2]['parameters']['content']).to eq('{md5}165406e473f38ababa17a05696e2ef70')
+ expect(obj.resources[2]['parameters'].key?('source')).to eq(false)
+ expect(obj.resources[3]).to eq(resources_save[3])
+ end
+
+ it 'should raise the proper error when a file is missing' do
+ # Set up test
+ obj = catalog_from_fixture('catalogs/catalog-test-file-v4-tagged.json')
+ obj.compilation_dir = @tmpdir
+ File.open(File.join(@tmpdir, 'modules', 'test', 'files', 'tmp', 'foo'), 'w') { |f| f.write "\u0256" }
+
+ # Perform test
+ expected_error = "Unable to resolve source=>'puppet:///modules/test/tmp/bar' in File[/tmp/bar] (/x/modules/test/manifests/init.pp:46)" # rubocop:disable Metrics/LineLength
+ expect do
+ OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
+ end.to raise_error(OctocatalogDiff::Errors::CatalogError, expected_error)
+ end
+
+ it 'should not raise due to a missing file when error is suppressed by a tag' do
+ obj = catalog_from_fixture('catalogs/catalog-test-file-v4.json')
+ obj.compilation_dir = @tmpdir
+ obj.options[:compare_file_text_ignore_tags] = %w(suppress-error-test-tag)
+ obj.resources[2]['parameters']['source'] = 'puppet:///modules/this/does/not/exist'
+ obj.resources[2]['tags'] = %w(suppress-error-test-tag)
+
+ expect do
+ OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
+ end.not_to raise_error
+ end
+
+ it 'should not raise due to a missing file in the "from" catalog' do
+ obj = catalog_from_fixture('catalogs/catalog-test-file-v4.json')
+ obj.compilation_dir = @tmpdir
+ obj.resources[2]['parameters']['source'] = 'puppet:///modules/this/does/not/exist'
+ obj.options[:tag] = 'from'
+
+ expect do
+ OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj)
+ end.not_to raise_error
+ end
end
describe '#resource_convertible?' do
|