Skip to content

Commit

Permalink
Merge pull request #64 from github/kpaulisse-ignore-tagged-resources-…
Browse files Browse the repository at this point in the history
…move

Move ignore tagged resources out of CLI
  • Loading branch information
kpaulisse committed Jan 14, 2017
2 parents 3434233 + 3d8cdf5 commit 7cac8af
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 56 deletions.
40 changes: 40 additions & 0 deletions lib/octocatalog-diff/catalog-diff/differ.rb
Expand Up @@ -104,6 +104,32 @@ def ignore(ignores = [])
self
end

# Handle --ignore-tags option, the ability to tag resources within modules/manifests and
# have catalog-diff ignore them.
def ignore_tags
return unless @opts[:ignore_tags].is_a?(Array) && @opts[:ignore_tags].any?

# Go through the "to" catalog and identify any resources that have been tagged with one or more
# specified "ignore tags." Add any such items to the ignore list. The 'to' catalog has the authoritative
# list of dynamic ignores.
@catalog2_raw.resources.each do |resource|
next unless tagged_for_ignore?(resource)
ignore(type: resource['type'], title: resource['title'])
@logger.debug "Ignoring type='#{resource['type']}', title='#{resource['title']}' based on tag in to-catalog"
end

# Go through the "from" catalog and identify any resources that have been tagged with one or more
# specified "ignore tags." Only mark the resources for ignoring if they do not appear in the 'to'
# catalog, thereby allowing the 'to' catalog to be the authoritative ignore list. This allows deleted
# items that were previously ignored to continue to be ignored.
@catalog1_raw.resources.each do |resource|
next if @catalog2_raw.resource(type: resource['type'], title: resource['title'])
next unless tagged_for_ignore?(resource)
ignore(type: resource['type'], title: resource['title'])
@logger.debug "Ignoring type='#{resource['type']}', title='#{resource['title']}' based on tag in from-catalog"
end
end

# Return catalog1 with filter_and_cleanups applied.
# This is in the public section because it's called from spec tests as well
# as being called internally.
Expand All @@ -122,6 +148,20 @@ def catalog2

private

# Determine if a resource is tagged with any ignore-tag.
# @param resource [Hash] The resource
# @return [Boolean] true if tagged for ignore, false if not
def tagged_for_ignore?(resource)
return false unless @opts[:ignore_tags].is_a?(Array)
return false unless resource.key?('tags') && resource['tags'].is_a?(Array)
@opts[:ignore_tags].each do |tag|
# tag_with_type will be like: 'ignored_catalog_diff__mymodule__mytype'
tag_with_type = [tag, resource['type'].downcase.gsub(/\W/, '_')].join('__')
return true if resource['tags'].include?(tag) || resource['tags'].include?(tag_with_type)
end
false
end

# Actually perform the catalog diff. This implements the 3-part algorithm described in the
# comment block at the top of this file.
def catdiff
Expand Down
41 changes: 1 addition & 40 deletions lib/octocatalog-diff/cli/diffs.rb
Expand Up @@ -28,52 +28,13 @@ def diffs(catalogs)
differ = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalogs[:from], catalogs[:to])
differ.ignore(attr: 'tags') unless @options.fetch(:include_tags, false)
differ.ignore(@options.fetch(:ignore, []))

# Handle --ignore-tags option, the ability to tag resources within modules/manifests and
# have catalog-diff ignore them.
if @options[:ignore_tags].is_a?(Array) && @options[:ignore_tags].any?
# Go through the "to" catalog and identify any resources that have been tagged with one or more
# specified "ignore tags." Add any such items to the ignore list. The 'to' catalog has the authoritative
# list of dynamic ignores.
catalogs[:to].resources.each do |resource|
next unless tagged_for_ignore?(resource)
differ.ignore(type: resource['type'], title: resource['title'])
@logger.debug "Ignoring type='#{resource['type']}', title='#{resource['title']}' based on tag in to-catalog"
end

# Go through the "from" catalog and identify any resources that have been tagged with one or more
# specified "ignore tags." Only mark the resources for ignoring if they do not appear in the 'to'
# catalog, thereby allowing the 'to' catalog to be the authoritative ignore list. This allows deleted
# items that were previously ignored to continue to be ignored.
catalogs[:from].resources.each do |resource|
next if catalogs[:to].resource(type: resource['type'], title: resource['title'])
next unless tagged_for_ignore?(resource)
differ.ignore(type: resource['type'], title: resource['title'])
@logger.debug "Ignoring type='#{resource['type']}', title='#{resource['title']}' based on tag in from-catalog"
end
end
differ.ignore_tags

# Actually perform the diff
diff_result = differ.diff
@logger.debug 'Success compute diffs between catalogs'
diff_result
end

private

# Determine if a resource is tagged with any ignore-tag.
# @param resource [Hash] The resource
# @return [Boolean] true if tagged for ignore, false if not
def tagged_for_ignore?(resource)
return false unless @options[:ignore_tags].is_a?(Array)
return false unless resource.key?('tags') && resource['tags'].is_a?(Array)
@options[:ignore_tags].each do |tag|
# tag_with_type will be like: 'ignored_catalog_diff__mymodule__mytype'
tag_with_type = [tag, resource['type'].downcase.gsub(/\W/, '_')].join('__')
return true if resource['tags'].include?(tag) || resource['tags'].include?(tag_with_type)
end
false
end
end
end
end
17 changes: 17 additions & 0 deletions spec/octocatalog-diff/integration/ignore_tags_spec.rb
Expand Up @@ -98,4 +98,21 @@ def build_catalogs(old_repo, new_repo, argv)
end
end
end

it 'should remove tagged-for-ignore resources' do
@logger, @logger_str = OctocatalogDiff::Spec.setup_logger
cat1 = OctocatalogDiff::Catalog.new(json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/ignore-tags-old.json')))
cat2 = OctocatalogDiff::Catalog.new(json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/ignore-tags-new.json')))
opts = { ignore_tags: ['ignored_catalog_diff'] }
answer = JSON.parse(File.read(OctocatalogDiff::Spec.fixture_path('diffs/ignore-tags-partial.json')))
obj = OctocatalogDiff::Cli::Diffs.new(opts, @logger)
diffs = obj.diffs(from: cat1, to: cat2)
expect(diffs.size).to eq(8)
answer.each do |x|
expect(diffs).to include(x), "Does not contain: #{x}"
end
expect(@logger_str.string).to match(/Ignoring type='Mymodule::Resource1', title='one' based on tag in to-catalog/)
r = %r{Ignoring type='File', title='/tmp/old-file/ignored/one' based on tag in from-catalog}
expect(@logger_str.string).to match(r)
end
end
45 changes: 45 additions & 0 deletions spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb
Expand Up @@ -1287,6 +1287,51 @@
end
end

describe '#ignore_tags' do
let(:catalog_1) { OctocatalogDiff::Catalog.new(json: OctocatalogDiff::Spec.fixture_read('catalogs/ignore-tags-old.json')) }
let(:catalog_2) { OctocatalogDiff::Catalog.new(json: OctocatalogDiff::Spec.fixture_read('catalogs/ignore-tags-new.json')) }
let(:opts) { { ignore_tags: ['ignored_catalog_diff'] } }
let(:answer) { JSON.parse(OctocatalogDiff::Spec.fixture_read('diffs/ignore-tags-partial.json')) }

it 'should remove tagged-for-ignore resources' do
logger, logger_str = OctocatalogDiff::Spec.setup_logger
subject = described_class.new(opts.merge(logger: logger), catalog_1, catalog_2)
subject.ignore_tags

ignore_answer = [
{ type: 'Mymodule::Resource1', title: 'one', attr: '*' },
{ type: 'Mymodule::Resource1', title: 'two', attr: '*' },
{ type: 'Mymodule::Resource1', title: 'three', attr: '*' },
{ type: 'Mymodule::Resource1', title: 'four', attr: '*' },
{ type: 'Mymodule::Resource2', title: 'five', attr: '*' },
{ type: 'File', title: '/tmp/ignored/one', attr: '*' },
{ type: 'File', title: '/tmp/new-file/ignored/one', attr: '*' },
{ type: 'File', title: '/tmp/ignored/two', attr: '*' },
{ type: 'File', title: '/tmp/new-file/ignored/two', attr: '*' },
{ type: 'File', title: '/tmp/ignored/three', attr: '*' },
{ type: 'File', title: '/tmp/new-file/ignored/three', attr: '*' },
{ type: 'File', title: '/tmp/ignored/four', attr: '*' },
{ type: 'File', title: '/tmp/new-file/ignored/four', attr: '*' },
{ type: 'File', title: '/tmp/resource2/five', attr: '*' },
{ type: 'File', title: '/tmp/ignored/five', attr: '*' },
{ type: 'File', title: '/tmp/new-file/ignored/five', attr: '*' },
{ type: 'File', title: '/tmp/old-file/ignored/one', attr: '*' },
{ type: 'File', title: '/tmp/old-file/ignored/two', attr: '*' },
{ type: 'File', title: '/tmp/old-file/ignored/three', attr: '*' },
{ type: 'File', title: '/tmp/old-file/ignored/four', attr: '*' },
{ type: 'File', title: '/tmp/old-file/ignored/five', attr: '*' }
]

ignores = subject.instance_variable_get('@ignore')
expect(ignores.size).to eq(ignore_answer.size)
ignore_answer.each { |answer| expect(ignores).to include(answer) }

expect(logger_str.string).to match(/Ignoring type='Mymodule::Resource1', title='one' based on tag in to-catalog/)
r = %r{Ignoring type='File', title='/tmp/old-file/ignored/one' based on tag in from-catalog}
expect(logger_str.string).to match(r)
end
end

describe '#hashdiff_nested_changes' do
it 'should return array with proper results' do
hashdiff_add_remove = [
Expand Down
16 changes: 0 additions & 16 deletions spec/octocatalog-diff/tests/cli/diffs_spec.rb
Expand Up @@ -67,21 +67,5 @@
expect(result).to eq([])
expect(@logger_str.string).not_to match(/WARN/)
end

it 'should remove tagged-for-ignore resources' do
cat1 = OctocatalogDiff::Catalog.new(json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/ignore-tags-old.json')))
cat2 = OctocatalogDiff::Catalog.new(json: File.read(OctocatalogDiff::Spec.fixture_path('catalogs/ignore-tags-new.json')))
opts = { ignore_tags: ['ignored_catalog_diff'] }
answer = JSON.parse(File.read(OctocatalogDiff::Spec.fixture_path('diffs/ignore-tags-partial.json')))
obj = OctocatalogDiff::Cli::Diffs.new(opts, @logger)
diffs = obj.diffs(from: cat1, to: cat2)
expect(diffs.size).to eq(8)
answer.each do |x|
expect(diffs).to include(x), "Does not contain: #{x}"
end
expect(@logger_str.string).to match(/Ignoring type='Mymodule::Resource1', title='one' based on tag in to-catalog/)
r = %r{Ignoring type='File', title='/tmp/old-file/ignored/one' based on tag in from-catalog}
expect(@logger_str.string).to match(r)
end
end
end

0 comments on commit 7cac8af

Please sign in to comment.