Merge pull request #180 from github/kpaulisse-ruby-version-test-update

Update build matrix to reflect Puppet's supported ruby version

Author: kpaulisse

.travis.yml

@@ -6,23 +6,21 @@ script: "script/cibuild"
 
 matrix:
   include:
+    # Puppet supports ruby 2.4 with Puppet >= 5.0 so this should be considered the "official" build
+    - rvm: 2.4
+      env: RUBOCOP_TEST="true" RSPEC_TEST="true" PUPPET_VERSIONS="5.4.0" ENFORCE_COVERAGE="true"
     # Build with latest ruby
     - rvm: 2.5
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" ENFORCE_COVERAGE="true" PUPPET_VERSIONS="3.8.7"
-    - rvm: 2.5
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" ENFORCE_COVERAGE="true" PUPPET_VERSIONS="4.10.8"
-    - rvm: 2.5
-      env: RUBOCOP_TEST="true" RSPEC_TEST="true" ENFORCE_COVERAGE="true" PUPPET_VERSIONS="5.0.0"
-    # Build with older ruby versions
-    - rvm: 2.4
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="3.8.7"
-    - rvm: 2.3.2
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="4.10.8"
-    - rvm: 2.2.3
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="5.0.0"
+      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="5.4.0"
+    # Puppet supports ruby 2.0 and 2.1 with Puppet 4.x, but build with more recent versions too
+    - rvm: 2.3
+      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="4.10.10"
+    - rvm: 2.2
+      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="4.10.10"
+    - rvm: 2.1
+      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="4.10.10"
+    # For really old ruby versions only build 3.8.7
     - rvm: 2.1
       env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="3.8.7"
     - rvm: 2.0
       env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="3.8.7"
-    - rvm: 2.0
-      env: RUBOCOP_TEST="false" RSPEC_TEST="true" PUPPET_VERSIONS="5.0.0"

.version

@@ -1 +1 @@
-1.5.2
+1.5.3

doc/CHANGELOG.md

@@ -8,6 +8,14 @@
 </tr>
 </thead><tbody>
 
+<tr valign=top>
+<td>1.5.3</td>
+<td>2018-03-05</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/176">#176</a>: (Enhancement) Normalize file resource titles in reference checks</li>
+</td>
+</tr>
+
 <tr valign=top>
 <td>1.5.2</td>
 <td>2017-12-19</td>

doc/requirements.md

@@ -2,10 +2,14 @@
 
 To run `octocatalog-diff` you will need these basics:
 
-- Ruby 2.0 through 2.4 (we test octocatalog-diff with Ruby 2.0, 2.1, 2.2, 2.3, and 2.4)
+- An appropriate Puppet version and [corresponding ruby version](https://puppet.com/docs/puppet/5.4/system_requirements.html)
+  - Puppet 5.x officially supports Ruby 2.4
+  - Puppet 4.x officially supports Ruby 2.1, but seems to work fine with later versions as well
+  - Puppet 3.8.7 -- we attempt to maintain compatibility in `octocatalog-diff` to facilitate upgrades even though this version is no longer supported by Puppet
+  - We don't officially support Puppet 3.8.6 or before
 - Mac OS, Linux, or other Unix-line operating system (Windows is not supported)
 - Ability to install gems, e.g. with [rbenv](https://github.com/rbenv/rbenv) or [rvm](https://rvm.io/), or root privileges to install into the system Ruby
-- Puppet agent for [Linux](https://docs.puppet.com/puppet/latest/reference/install_linux.html) or [Mac OS X](https://docs.puppet.com/puppet/latest/reference/install_osx.html), or installed as a gem (we support Puppet 3.8.7 and all versions of Puppet 4.x)
+- Puppet agent for [Linux](https://docs.puppet.com/puppet/latest/reference/install_linux.html) or [Mac OS X](https://docs.puppet.com/puppet/latest/reference/install_osx.html), or installed as a gem
 
 We recommend that you also have the following to get the most out of `octocatalog-diff`, but these are not absolute requirements:
 

lib/octocatalog-diff/catalog.rb

@@ -304,17 +304,32 @@ def resources_missing_from_catalog(resources_to_check)
         unless res =~ /\A([\w:]+)\[(.+)\]\z/
           raise ArgumentError, "Resource #{res} is not in the expected format"
         end
-        resource(type: Regexp.last_match(1), title: Regexp.last_match(2)).nil?
+
+        type = Regexp.last_match(1)
+        title = normalized_title(Regexp.last_match(2), type)
+        resource(type: type, title: title).nil?
       end
     end
 
+    # Private method: Given a title string, normalize it according to the rules
+    # used by puppet 4.10.x for file resource title normalization:
+    # https://github.com/puppetlabs/puppet/blob/4.10.x/lib/puppet/type/file.rb#L42
+    def normalized_title(title_string, type)
+      return title_string if type != 'File'
+
+      matches = title_string.match(%r{^(?<normalized_path>/|.+:/|.*[^/])/*\Z}m)
+      matches[:normalized_path] || title_string
+    end
+
     # Private method: Build the resource hash to be used used for O(1) lookups by type and title.
     # This method is called the first time the resource hash is accessed.
     def build_resource_hash
       @resource_hash = {}
       resources.each do |resource|
         @resource_hash[resource['type']] ||= {}
-        @resource_hash[resource['type']][resource['title']] = resource
+
+        title = normalized_title(resource['title'], resource['type'])
+        @resource_hash[resource['type']][title] = resource
 
         if resource.key?('parameters') && resource['parameters'].key?('alias')
           @resource_hash[resource['type']][resource['parameters']['alias']] = resource

octocatalog-diff.gemspec

@@ -1,6 +1,6 @@
 require 'json'
 
-DEFAULT_PUPPET_VERSION = '4.10.8'.freeze
+DEFAULT_PUPPET_VERSION = '5.4.0'.freeze
 
 Gem::Specification.new do |s|
   s.required_ruby_version = '>= 2.0.0'

spec/octocatalog-diff/fixtures/repos/reference-validation/hieradata/roles/working-file.yaml

@@ -0,0 +1,3 @@
+---
+  classes:
+    - test::file_tests

spec/octocatalog-diff/fixtures/repos/reference-validation/modules/test/manifests/file_tests.pp

@@ -0,0 +1,10 @@
+class test::file_tests {
+  file { '/foo':
+    ensure => directory,
+  }
+
+  file { '/bar':
+    ensure  => directory,
+    require => File['/foo/'],
+  }
+}

spec/octocatalog-diff/integration/reference_validation_spec.rb

@@ -93,6 +93,26 @@ def self.catalog_contains_resource(result, type, title)
     end
   end
 
+  context 'with valid files that have trailing slashes' do
+    before(:all) do
+      @result = OctocatalogDiff::Spec.reference_validation_catalog('working-file', %w(require))
+    end
+
+    it 'should succeed' do
+      expect(@result.exitcode).to eq(0)
+    end
+
+    it 'should not raise any exceptions' do
+      expect(@result.exception).to be_nil, OctocatalogDiff::Integration.format_exception(@result)
+    end
+
+    it 'should contain representative resources' do
+      pending 'Catalog failed' unless @result.exitcode.zero?
+      expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'File', '/foo')).to eq(true)
+      expect(OctocatalogDiff::Spec.catalog_contains_resource(@result, 'File', '/bar')).to eq(true)
+    end
+  end
+
   context 'with broken subscribe' do
     before(:all) do
       @result = OctocatalogDiff::Spec.reference_validation_catalog('broken-subscribe', %w(subscribe))
@@ -109,7 +129,7 @@ def self.catalog_contains_resource(result, type, title)
     if OctocatalogDiff::Spec.is_puppet5?
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
-        expect(msg).to match(/Error: Could not find resource 'Exec\[subscribe target\]' in parameter 'subscribe' at/)
+        expect(msg).to match(/Error: Could not find resource 'Exec\[subscribe target\]' in parameter 'subscribe'/)
       end
     else
       # Multiple line numbers given because Puppet 4.x and 3.8 correspond to first and last line of resource, respectively.
@@ -142,7 +162,7 @@ def self.catalog_contains_resource(result, type, title)
     if OctocatalogDiff::Spec.is_puppet5?
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
-        expect(msg).to match(/Error: Could not find resource 'Exec\[before target\]' in parameter 'before' at/)
+        expect(msg).to match(/Error: Could not find resource 'Exec\[before target\]' in parameter 'before'/)
       end
     else
       # rubocop:disable Metrics/LineLength
@@ -170,7 +190,7 @@ def self.catalog_contains_resource(result, type, title)
     if OctocatalogDiff::Spec.is_puppet5?
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
-        expect(msg).to match(/Error: Could not find resource 'Test::Foo::Bar\[notify target\]' in parameter 'notify' at/)
+        expect(msg).to match(/Error: Could not find resource 'Test::Foo::Bar\[notify target\]' in parameter 'notify'/)
       end
     else
       # rubocop:disable Metrics/LineLength
@@ -198,7 +218,7 @@ def self.catalog_contains_resource(result, type, title)
     if OctocatalogDiff::Spec.is_puppet5?
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
-        expect(msg).to match(/Error: Could not find resource 'Exec\[require target\]' in parameter 'require' at/)
+        expect(msg).to match(/Error: Could not find resource 'Exec\[require target\]' in parameter 'require'/)
       end
     else
       # rubocop:disable Metrics/LineLength
@@ -226,7 +246,7 @@ def self.catalog_contains_resource(result, type, title)
 
       it 'should pass through the error messages from Puppet' do
         msg = @result.exception.message
-        expect(msg).to match(/Error: Could not find resource 'Exec\[subscribe target\]' in parameter 'subscribe' at/)
+        expect(msg).to match(/Error: Could not find resource 'Exec\[subscribe target\]' in parameter 'subscribe'/)
       end
     else
       it 'should succeed' do

spec/octocatalog-diff/tests/catalog_spec.rb

@@ -621,6 +621,14 @@
             'alias' => 'the exec',
             'command' => '/bin/true'
           }
+        },
+        {
+          'type' => 'File',
+          'title' => '/foo/'
+        },
+        {
+          'type' => 'File',
+          'title' => '/bar'
         }
       ]
       described_object = described_class.allocate
@@ -636,5 +644,14 @@
     it 'should contain the entry for the aliased resource' do
       expect(@resource_hash['Exec']['the exec']).to be_a_kind_of(Hash)
     end
+
+    it 'should normalize trailing slashes on file resources' do
+      expect(@resource_hash['File']['/foo']).to be_a_kind_of(Hash)
+      expect(@resource_hash['File']['/foo/']).to eq(nil)
+    end
+
+    it 'should not otherwise touch file resources that do not need to be normalized' do
+      expect(@resource_hash['File']['/bar']).to be_a_kind_of(Hash)
+    end
   end
 end