From 89d6c993ffbba91e9cf3a7eac2746c052d966a8a Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Mon, 26 Mar 2018 15:43:37 -0400 Subject: [PATCH 1/5] Create acceptance test demonstrating #175 --- .../integration/ignore_regexp_spec.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 spec/octocatalog-diff/integration/ignore_regexp_spec.rb diff --git a/spec/octocatalog-diff/integration/ignore_regexp_spec.rb b/spec/octocatalog-diff/integration/ignore_regexp_spec.rb new file mode 100644 index 0000000..0318ba1 --- /dev/null +++ b/spec/octocatalog-diff/integration/ignore_regexp_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require_relative 'integration_helper' + +describe 'ignore regexp integration' do + before(:all) do + @result = OctocatalogDiff::API::V1.catalog_diff( + to_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/default-catalog-changed.json'), + from_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/default-catalog-v4.json'), + ignore: [ + { + type: Regexp.new('\ASsh_authorized_key\z'), + attr: Regexp.new("\\Aparameters\f(foo|bar)\\z") + } + ] + ) + end + + it 'should succeed' do + expect(@result.diffs).to be_a_kind_of(Array) + end + + it 'should contain a non-ignored diff in another type' do + lookup = { diff_type: '+', type: 'Group', title: 'bill' } + expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, lookup)).to eq(true), @result.diffs.inspect + end + + it 'should contain a non-ignored removal in the same type' do + lookup = { type: 'Ssh_authorized_key', title: 'root@6def27049c06f48eea8b8f37329f40799d07dc84' } + expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, lookup)).to eq(true), @result.diffs.inspect + end + + it 'should contain a non-ignored diff in the same type' do + lookup = { type: 'Ssh_authorized_key', title: 'bob@local', structure: %w[parameters type] } + expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, lookup)).to eq(true), @result.diffs.inspect + end + + it 'should not contain an ignored diff in the same type' do + lookup = { type: 'Ssh_authorized_key', title: 'bob@local', structure: %w[parameters foo] } + expect(OctocatalogDiff::Spec.diff_match?(@result.diffs, lookup)).to eq(false), @result.diffs.inspect + end +end From b00015b9e321ff96c6ef4857752c5f1bf45179ce Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 27 Mar 2018 07:42:10 -0300 Subject: [PATCH 2/5] Update differ to support regular expression attributes --- lib/octocatalog-diff/catalog-diff/differ.rb | 7 +++++++ .../tests/catalog-diff/differ_spec.rb | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/octocatalog-diff/catalog-diff/differ.rb b/lib/octocatalog-diff/catalog-diff/differ.rb index fe945fc..d511466 100644 --- a/lib/octocatalog-diff/catalog-diff/differ.rb +++ b/lib/octocatalog-diff/catalog-diff/differ.rb @@ -394,6 +394,13 @@ module OctocatalogDiff return false unless rule[:title].casecmp(hsh[:title]).zero? end + # If rule[:attr] is a regular expression, handle that case here. + if rule[:attr].is_a?(Regexp) + return false unless hsh[:attr].is_a?(String) + return false unless rule[:attr].match(hsh[:attr]) + return ignore_match_true(hsh, rule) + end + # Special 'attributes': Ignore specific diff types (+ add, - remove, ~ and ! change) if rule[:attr] =~ /\A[\-\+~!]+\Z/ return ignore_match_true(hsh, rule) if rule[:attr].include?(diff_type) diff --git a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb index e4b0b40..73396de 100644 --- a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb @@ -1285,6 +1285,24 @@ describe OctocatalogDiff::CatalogDiff::Differ do expect(logger_str.string).not_to match(/Ignoring .+ matches {:type=>"\*", :title=>"dell", :attr=>"\*"}/) end end + + context 'attrs regexp' do + it 'should filter on regular expression match' do + rule = { type: 'Apple', title: 'delicious', attr: Regexp.new("\\Aparameters\f(color|taste)\\z") } + logger, logger_str = OctocatalogDiff::Spec.setup_logger + testobj.instance_variable_set('@logger', logger) + expect(testobj.send(:"ignore_match?", rule, '+', resource, 'old_value', 'new_value')).to eq(true) + expect(logger_str.string).to match(/Ignoring .+ matches {:type=>"Apple", :title=>"delicious", :attr=>/) + end + + it 'should not filter on regular expression non-match' do + rule = { type: 'Apple', title: 'delicious', attr: Regexp.new("\\Aparameters\f(odor|has_worms)\\z") } + logger, logger_str = OctocatalogDiff::Spec.setup_logger + testobj.instance_variable_set('@logger', logger) + expect(testobj.send(:"ignore_match?", rule, '+', resource, 'old_value', 'new_value')).to eq(false) + expect(logger_str.string).not_to match(/Ignoring .+ matches/) + end + end end describe '#ignore_tags' do From da69d171fded1b8e8fedd56d658e4de773a739e7 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Wed, 28 Mar 2018 08:46:32 -0300 Subject: [PATCH 3/5] Better newline handling when performing regexp diffs/ignores --- lib/octocatalog-diff/catalog-diff/differ.rb | 5 +- .../tests/catalog-diff/differ_spec.rb | 130 ++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/lib/octocatalog-diff/catalog-diff/differ.rb b/lib/octocatalog-diff/catalog-diff/differ.rb index d511466..c56234c 100644 --- a/lib/octocatalog-diff/catalog-diff/differ.rb +++ b/lib/octocatalog-diff/catalog-diff/differ.rb @@ -294,10 +294,13 @@ module OctocatalogDiff # Use diffy to get only the lines that have changed in a text object. # As we iterate through the diff, jump out if we have our answer: either # true if '=~>' finds ANY match, or false if '=&>' fails to find a match. - Diffy::Diff.new(old_val, new_val, context: 0).each do |line| + diffy_result = Diffy::Diff.new(old_val, new_val, context: 0) + newline_alerts = diffy_result.count { |line| line.strip == '\\ No newline at end of file' } + diffy_result.each do |line| if regex.match(line.strip) return true if operator == '=~>' elsif operator == '=&>' + next if line.strip == '\\ No newline at end of file' && newline_alerts == 2 return false end end diff --git a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb index 73396de..facc078 100644 --- a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb @@ -1391,4 +1391,134 @@ describe OctocatalogDiff::CatalogDiff::Differ do expect(result[0]).to eq(['!', "Class\fOpenssl::Package\fparameters\fcommon-array", [1, 2, 3], [1, 5, 25], fileref, fileref]) end end + + describe '#regexp_operator_match?' do + let(:subject) { described_class.allocate } + + context 'for a multi-line diff' do + context 'for operator =~>' do + let(:operator) { '=~>' } + let(:regex) { Regexp.new('\\A.(kittens|cats)\\z') } + + it 'should return true when at least one line matches' do + old_val = "kittens\nkittens\ncats\n" + new_val = "puppies\ndogs\n" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when neither line matches' do + old_val = "puppies\ndogs\ndonkeys\n" + new_val = "puppies\ndogs\n" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + + context 'for operator =&>' do + let(:operator) { '=&>' } + let(:regex) { Regexp.new('\\A(-|\\+)(kittens )*(kittens|cats)\\z') } + + it 'should return true when all lines match' do + old_val = "kittens\nkittens\ncats\n" + new_val = "kittens\nkittens\n" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when the regex does not match line' do + old_val = "kittens\nkittens\ncats\n" + new_val = "kittens\nkittens\ndogs\n" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + + it 'should return true if both old and new do not end in a newline' do + old_val = "kittens\nkittens\ncats" + new_val = "kittens\nkittens\nkittens" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false if one ends in a newline and the other does not' do + old_val = "kittens\nkittens\ncats\n" + new_val = "kittens\nkittens\nkittens" + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + end + + context 'for a single-line diff' do + context 'for operator =~>' do + let(:operator) { '=~>' } + let(:regex) { Regexp.new('\\A(-|\\+)(kittens )+(kittens|cats)\\z') } + + it 'should return true when at least one line matches' do + old_val = 'kittens kittens kittens' + new_val = 'kittens cats' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when neither line matches' do + old_val = 'kittens dogs cats kittens kittens' + new_val = 'kittens cats dogs' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + + context 'for operator =&>' do + let(:operator) { '=&>' } + let(:regex) { Regexp.new('\\A(-|\\+)(kittens )+(kittens|cats)\\z') } + + it 'should return true when both lines match' do + old_val = 'kittens kittens kittens' + new_val = 'kittens cats' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when the regex does not match line' do + old_val = 'kittens kittens dogs kittens' + new_val = 'kittens cats' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + end + + context 'for a multi-line versus a single-line diff' do + context 'for operator =~>' do + let(:operator) { '=~>' } + let(:regex) { Regexp.new('\\A.(kittens|cats)\\z') } + + it 'should return true when at least one line matches' do + old_val = "kittens\nkittens\ncats\n" + new_val = 'puppies' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when neither line matches' do + old_val = "puppies\ndogs\ndonkeys\n" + new_val = 'puppies' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + + context 'for operator =&>' do + let(:operator) { '=&>' } + let(:regex) { Regexp.new('\\A(-|\\+)(kittens )*(kittens|cats)\\z') } + + it 'should return true when all lines match and both old and new do not end in newline' do + old_val = "kittens\nkittens\ncats" + new_val = 'kittens' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true) + end + + it 'should return false when all lines match but ending in newlines differs' do + old_val = "kittens\nkittens\ncats\n" + new_val = 'kittens' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + + it 'should return false when the regex does not match line' do + old_val = "kittens\nkittens\ncats" + new_val = 'dogs' + expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false) + end + end + end + end end From 2ec1ca47dfc7e14b5d9f369232e1ef2eb8a67e77 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Thu, 29 Mar 2018 10:02:05 -0400 Subject: [PATCH 4/5] Quoting shenanigans --- doc/dev/api/v1/calls/catalog-diff.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/dev/api/v1/calls/catalog-diff.md b/doc/dev/api/v1/calls/catalog-diff.md index 649a65c..9927945 100644 --- a/doc/dev/api/v1/calls/catalog-diff.md +++ b/doc/dev/api/v1/calls/catalog-diff.md @@ -167,12 +167,16 @@ In this case, "owner", "notify", and "content" are nested under "parameters". In ``` # Ignore all changes to the `owner` attribute of a file. - [ { type: Regexp.new('\AFile\z'), attr: Regexp.new("\Aparameters\fowner\z" } ] + [ { type: Regexp.new('\AFile\z'), attr: Regexp.new("\\Aparameters\fowner\\z" } ] # Ignore changes to `owner` or `group` for a file or an exec. - [ { type: Regexp.new('\A(File|Exec)\z'), attr: Regexp.new("\Aparameters\f(owner|group)\z" } ] + [ { type: Regexp.new('\A(File|Exec)\z'), attr: Regexp.new("\\Aparameters\f(owner|group)\\z" } ] ``` +When using regular expressions, `\f` (form feed character) is used to separate the structure (e.g. `parameters\fowner` refers to the `parameters` hash, `owner` key). + +:bulb: Note that `\A` in Ruby matches the beginning of the string and `\z` matches the end, but these are not actual characters. Therefore, if you are using `\A` or `\z` in double quotes (`"`), be sure to heed the examples above and write your expression like: `Regexp.new("\\Aparameters\fowner\\z")`. + #### `:validate_references` (Array<String>, Optional) Invoke the [catalog validation](/doc/advanced-catalog-validation.md) feature to ensure resources targeted by `before`, `notify`, `require`, and/or `subscribe` exist in the catalog. If this parameter is not defined, no reference validation occurs. From 178f7cbdf5087424f241a460ebbcbd508fd31ddd Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Thu, 29 Mar 2018 10:50:25 -0400 Subject: [PATCH 5/5] Work around travis-ci/travis-ci/issues/9333 with latest ruby --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 990b18d..2080628 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ language: ruby install: true script: "script/cibuild" +before_install: gem install bundler matrix: include: