From 33833f37ee88d136a3901c6f90126d519a603847 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 10:29:44 -0600 Subject: [PATCH 01/15] Start integration test and fixture repo for multiple module paths --- .../repos/modulepath/environment.conf | 1 + .../repos/modulepath/manifests/site.pp | 4 ++ .../modules/modulestest/files/tmp/modulestest | 1 + .../modules/modulestest/manifests/init.pp | 5 ++ .../site/sitetest/files/tmp/sitetest | 1 + .../site/sitetest/manifests/init.pp | 5 ++ .../integration/modulepath_spec.rb | 47 +++++++++++++++++++ 7 files changed, 64 insertions(+) create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/environment.conf create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/manifests/site.pp create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/tmp/modulestest create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/files/tmp/sitetest create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/manifests/init.pp create mode 100644 spec/octocatalog-diff/integration/modulepath_spec.rb diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/environment.conf b/spec/octocatalog-diff/fixtures/repos/modulepath/environment.conf new file mode 100644 index 00000000..5362de3f --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/environment.conf @@ -0,0 +1 @@ +modulepath=modules:site:$basemodulepath diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/manifests/site.pp b/spec/octocatalog-diff/fixtures/repos/modulepath/manifests/site.pp new file mode 100644 index 00000000..3bf4e5bd --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/manifests/site.pp @@ -0,0 +1,4 @@ +node default { + include modulestest + include sitetest +} diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/tmp/modulestest b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/tmp/modulestest new file mode 100644 index 00000000..45ef27ef --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/tmp/modulestest @@ -0,0 +1 @@ +Modules Test diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp new file mode 100644 index 00000000..306867f5 --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp @@ -0,0 +1,5 @@ +class modulestest { + file { '/tmp/modulestest': + source => 'puppet:///modules/modulestest/tmp/modulestest', + } +} diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/files/tmp/sitetest b/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/files/tmp/sitetest new file mode 100644 index 00000000..7cbe5e7e --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/files/tmp/sitetest @@ -0,0 +1 @@ +Site Test diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/manifests/init.pp b/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/manifests/init.pp new file mode 100644 index 00000000..5fda225c --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/site/sitetest/manifests/init.pp @@ -0,0 +1,5 @@ +class sitetest { + file { '/tmp/sitetest': + source => 'puppet:///modules/sitetest/tmp/sitetest', + } +} diff --git a/spec/octocatalog-diff/integration/modulepath_spec.rb b/spec/octocatalog-diff/integration/modulepath_spec.rb new file mode 100644 index 00000000..5a7f4b0f --- /dev/null +++ b/spec/octocatalog-diff/integration/modulepath_spec.rb @@ -0,0 +1,47 @@ +require_relative 'integration_helper' +require OctocatalogDiff::Spec.require_path('/catalog') + +describe 'multiple module paths' do + # Make sure the catalog compiles correctly, without using any of the file + # conversion resources. If the catalog doesn't compile correctly this could + # indicate a problem that lies somewhere other than the comparison code. + describe 'catalog only' do + before(:all) do + @result = OctocatalogDiff::Integration.integration( + spec_fact_file: 'facts.yaml', + spec_repo: 'modulepath', + argv: [ + '--catalog-only', + '-n', 'rspec-node.github.net', + '--no-compare-file-text' + ] + ) + @catalog = OctocatalogDiff::Catalog.new( + backend: :json, + json: @result[:output] + ) + end + + it 'should compile' do + expect(@result[:exitcode]).not_to eq(-1), OctocatalogDiff::Integration.format_exception(@result) + end + + it 'should be a valid catalog' do + pending 'catalog failed to compile' if @result[:exitcode] == -1 + expect(@catalog.valid?).to eq(true) + end + + it 'should have expected resources in catalog' do + pending 'catalog was invalid' unless @catalog.valid? + expect(@catalog.resources).to be_a_kind_of(Array) + + mf = @catalog.resource(type: 'File', title: '/tmp/modulestest') + expect(mf).to be_a_kind_of(Hash) + expect(mf['parameters']).to eq('source' => 'puppet:///modules/modulestest/tmp/modulestest') + + sf = @catalog.resource(type: 'File', title: '/tmp/sitetest') + expect(sf).to be_a_kind_of(Hash) + expect(sf['parameters']).to eq('source' => 'puppet:///modules/sitetest/tmp/sitetest') + end + end +end From 2b9ba57e7467408898190512200c2fa841918a5f Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 10:49:19 -0600 Subject: [PATCH 02/15] Add file comparison tests --- .../integration/modulepath_spec.rb | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/spec/octocatalog-diff/integration/modulepath_spec.rb b/spec/octocatalog-diff/integration/modulepath_spec.rb index 5a7f4b0f..805a5a64 100644 --- a/spec/octocatalog-diff/integration/modulepath_spec.rb +++ b/spec/octocatalog-diff/integration/modulepath_spec.rb @@ -44,4 +44,71 @@ expect(sf['parameters']).to eq('source' => 'puppet:///modules/sitetest/tmp/sitetest') end end + + # Test the file comparison feature itself here in its various iterations. + describe 'file comparison feature' do + before(:each) do + @from_dir = Dir.mktmpdir + FileUtils.cp_r OctocatalogDiff::Spec.fixture_path('repos/modulepath'), @from_dir + + @to_dir = Dir.mktmpdir + FileUtils.cp_r OctocatalogDiff::Spec.fixture_path('repos/modulepath'), @to_dir + + file1 = File.join(@to_dir, 'modulepath', 'modules', 'modulestest', 'files', 'tmp', 'modulestest') + File.open(file1, 'w') { |f| f.write("New content of modulestest\n") } + + file2 = File.join(@to_dir, 'modulepath', 'site', 'sitetest', 'files', 'tmp', 'sitetest') + File.open(file2, 'w') { |f| f.write("New content of sitetest\n") } + end + + after(:each) do + OctocatalogDiff::Spec.clean_up_tmpdir(@from_dir) + OctocatalogDiff::Spec.clean_up_tmpdir(@to_dir) + end + + context 'with environment.conf' do + # The environment.conf is a fixture within the repository so there is no need + # to create it or manipulate it. + before(:each) do + @result = OctocatalogDiff::Integration.integration( + spec_fact_file: 'facts.yaml', + argv: [ + '-n', 'rspec-node.github.net', + '--bootstrapped-from-dir', @from_dir, + '--bootstrapped-to-dir', @to_dir + ] + ) + end + + it 'should compile catalogs and compute differences' do + expect(@result[:exitcode]).to eq(2), OctocatalogDiff::Integration.format_exception(@result) + expect(@result[:diffs]).to be_a_kind_of(Array) + expect(@result[:diffs].size).to eq(2) + end + + it 'should provide the correct differences' do + end + end + + context 'without environment.conf' do + before(:each) do + FileUtils.rm_f File.join(@to_dir, 'modulepath', 'environment.conf') + FileUtils.rm_f File.join(@from_dir, 'modulepath', 'environment.conf') + @result = OctocatalogDiff::Integration.integration( + spec_fact_file: 'facts.yaml', + argv: [ + '-n', 'rspec-node.github.net', + '--bootstrapped-from-dir', @from_dir, + '--bootstrapped-to-dir', @to_dir + ] + ) + end + + it 'should compile catalogs and compute differences' do + expect(@result[:exitcode]).to eq(0), OctocatalogDiff::Integration.format_exception(@result) + expect(@result[:diffs]).to be_a_kind_of(Array) + expect(@result[:diffs].size).to eq(0) + end + end + end end From c211ae2b77091e50445b92b5cbc062c8a84d3459 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:34:33 -0600 Subject: [PATCH 03/15] Add correct answers to integration test --- .../integration/modulepath_spec.rb | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/spec/octocatalog-diff/integration/modulepath_spec.rb b/spec/octocatalog-diff/integration/modulepath_spec.rb index 805a5a64..040756c6 100644 --- a/spec/octocatalog-diff/integration/modulepath_spec.rb +++ b/spec/octocatalog-diff/integration/modulepath_spec.rb @@ -66,6 +66,22 @@ OctocatalogDiff::Spec.clean_up_tmpdir(@to_dir) end + let(:module_answer) do + ['~', + "File\f/tmp/modulestest\fparameters\fcontent", + "Modules Test\n", + "New content of modulestest\n"] + end + + let(:site_answer) do + [ + '~', + "File\f/tmp/sitetest\fparameters\fcontent", + "Site Test\n", + "New content of sitetest\n" + ] + end + context 'with environment.conf' do # The environment.conf is a fixture within the repository so there is no need # to create it or manipulate it. @@ -74,8 +90,8 @@ spec_fact_file: 'facts.yaml', argv: [ '-n', 'rspec-node.github.net', - '--bootstrapped-from-dir', @from_dir, - '--bootstrapped-to-dir', @to_dir + '--bootstrapped-from-dir', File.join(@from_dir, 'modulepath'), + '--bootstrapped-to-dir', File.join(@to_dir, 'modulepath') ] ) end @@ -84,30 +100,31 @@ expect(@result[:exitcode]).to eq(2), OctocatalogDiff::Integration.format_exception(@result) expect(@result[:diffs]).to be_a_kind_of(Array) expect(@result[:diffs].size).to eq(2) - end - - it 'should provide the correct differences' do + expect(OctocatalogDiff::Spec.array_contains_partial_array?(@result[:diffs], module_answer)).to eq(true) + expect(OctocatalogDiff::Spec.array_contains_partial_array?(@result[:diffs], site_answer)).to eq(true) end end - context 'without environment.conf' do + context 'without environment.conf in one directory' do before(:each) do - FileUtils.rm_f File.join(@to_dir, 'modulepath', 'environment.conf') FileUtils.rm_f File.join(@from_dir, 'modulepath', 'environment.conf') + FileUtils.mv File.join(@from_dir, 'modulepath', 'site', 'sitetest'), File.join(@from_dir, 'modulepath', 'modules') @result = OctocatalogDiff::Integration.integration( spec_fact_file: 'facts.yaml', argv: [ '-n', 'rspec-node.github.net', - '--bootstrapped-from-dir', @from_dir, - '--bootstrapped-to-dir', @to_dir + '--bootstrapped-from-dir', File.join(@from_dir, 'modulepath'), + '--bootstrapped-to-dir', File.join(@to_dir, 'modulepath') ] ) end it 'should compile catalogs and compute differences' do - expect(@result[:exitcode]).to eq(0), OctocatalogDiff::Integration.format_exception(@result) + expect(@result[:exitcode]).to eq(2), OctocatalogDiff::Integration.format_exception(@result) expect(@result[:diffs]).to be_a_kind_of(Array) - expect(@result[:diffs].size).to eq(0) + expect(@result[:diffs].size).to eq(2) + expect(OctocatalogDiff::Spec.array_contains_partial_array?(@result[:diffs], module_answer)).to eq(true) + expect(OctocatalogDiff::Spec.array_contains_partial_array?(@result[:diffs], site_answer)).to eq(true) end end end From 105a1d94a19cb13822f52028a0caa51940d9c9db Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:35:21 -0600 Subject: [PATCH 04/15] Update file comparison logic to use modulepath from environment.conf --- .../catalog-util/fileresources.rb | 51 +++++++++++++++---- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 01112474..bf8572f7 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -21,6 +21,41 @@ def self.convert_file_resources(obj) end end + # Internal method: Locate a file that is referenced at puppet:///modules/xxx/yyy using the + # module path that is specified within the environment.conf file (assuming the default 'modules' + # directory doesn't exist or the module isn't found in there). If the file can't be found then + # this returns nil which may trigger an error. + # @param src [String] A file reference: puppet:///modules/xxx/yyy + # @param modulepath [Array] Cached module path + # @return [String] File system path to referenced file + def self.file_path(src, modulepaths) + raise "Bad parameter source #{src}" unless src =~ %r{^puppet:///modules/([^/]+)/(.+)} + path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2)) + + modulepaths.each do |mp| + file = File.join(mp, path) + return file if File.file?(file) + end + + nil + end + + # Internal method: Parse environment.conf to find the modulepath + # @param compilation_dir [String] Compilation directory + # @return [Array] Module paths + def self.module_path(compilation_dir) + environment_conf = File.join(compilation_dir, 'environment.conf') + return ['modules'] unless File.file?(environment_conf) + + # This doesn't support multi-line, continuations with backslash, etc. + # Does it need to?? + if File.read(environment_conf) =~ /^modulepath\s*=\s*(.+)/ + Regexp.last_match(1).split(/:/).map(&:strip).reject { |x| x =~ /^\$/ } + else + ['modules'] + end + end + # 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 @@ -34,18 +69,14 @@ def self._convert_file_resources(resources, compilation_dir) # that compilation_dir/environments/production is pointing at the right place). Otherwise, try to find # compilation_dir/modules. If neither of those exist, this code can't run. env_dir = File.join(compilation_dir, 'environments', 'production') - unless File.directory?(File.join(env_dir, 'modules')) - return unless File.directory?(File.join(compilation_dir, 'modules')) - env_dir = compilation_dir - end + modulepaths = module_path(env_dir).map { |x| File.join(env_dir, x) }.select { |x| File.directory?(x) } + return if modulepaths.empty? - # Modify the resources + # At least one existing module path was found! Run the code to modify the resources. resources.map! do |resource| if resource_convertible?(resource) - # Parse the 'source' parameter into a file on disk - src = resource['parameters']['source'] - raise "Bad parameter source #{src}" unless src =~ %r{^puppet:///modules/([^/]+)/(.+)} - path = File.join(env_dir, 'modules', Regexp.last_match(1), 'files', Regexp.last_match(2)) + path = file_path(resource['parameters']['source'], modulepaths) + raise Errno::ENOENT, "Unable to resolve '#{resource['parameters']['source']}'!" if path.nil? if File.file?(path) # If the file is found, read its content. If the content is all ASCII, substitute it into @@ -60,7 +91,7 @@ def self._convert_file_resources(resources, compilation_dir) # However, the fact that we found *something* at this location indicates that the catalog # is probably correct. Hence, the very general .exist? check. else - raise Errno::ENOENT, "Unable to find '#{src}' at #{path}!" + raise Errno::ENOENT, "Unable to find '#{resource['parameters']['source']}' at #{path}!" end end resource From 1c588e24f6ea94ee49306037bb8f443a632dc5ac Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:38:54 -0600 Subject: [PATCH 05/15] Update test to match new error message wording --- spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb index 58260b59..3de4b85c 100644 --- a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb @@ -97,7 +97,7 @@ def catalog_from_fixture(path) # Perform test expect do OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(obj) - end.to raise_error(Errno::ENOENT, %r{Unable to find 'puppet:///modules/this/does/not/exist'}) + end.to raise_error(Errno::ENOENT, %r{Unable to resolve 'puppet:///modules/this/does/not/exist'}) end it 'should return original if compilation_dir is not a string' do From 3cc93c8cc78946c9b585b298c5b370dc24d5f2f4 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:41:35 -0600 Subject: [PATCH 06/15] Try files in environment dir and compilation dir --- lib/octocatalog-diff/catalog-util/fileresources.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index bf8572f7..859eee71 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -45,14 +45,14 @@ def self.file_path(src, modulepaths) # @return [Array] Module paths def self.module_path(compilation_dir) environment_conf = File.join(compilation_dir, 'environment.conf') - return ['modules'] unless File.file?(environment_conf) + return [File.join(compilation_dir, 'modules')] unless File.file?(environment_conf) # This doesn't support multi-line, continuations with backslash, etc. # Does it need to?? if File.read(environment_conf) =~ /^modulepath\s*=\s*(.+)/ - Regexp.last_match(1).split(/:/).map(&:strip).reject { |x| x =~ /^\$/ } + Regexp.last_match(1).split(/:/).map(&:strip).reject { |x| x =~ /^\$/ }.map { |x| File.join(compilation_dir, x) } else - ['modules'] + [File.join(compilation_dir, 'modules')] end end @@ -69,7 +69,8 @@ def self._convert_file_resources(resources, compilation_dir) # that compilation_dir/environments/production is pointing at the right place). Otherwise, try to find # compilation_dir/modules. If neither of those exist, this code can't run. env_dir = File.join(compilation_dir, 'environments', 'production') - modulepaths = module_path(env_dir).map { |x| File.join(env_dir, x) }.select { |x| File.directory?(x) } + modulepaths = module_path(env_dir) + module_path(compilation_dir) + modulepaths.select! { |x| File.directory?(x) } return if modulepaths.empty? # At least one existing module path was found! Run the code to modify the resources. From 7466269bacf7e79d055f2c7a07c78022db2e3371 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:51:00 -0600 Subject: [PATCH 07/15] Add spec coverage for file_path --- .../catalog-util/fileresources.rb | 6 ++-- .../tests/catalog-util/fileresources_spec.rb | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 859eee71..811b8892 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -29,9 +29,11 @@ def self.convert_file_resources(obj) # @param modulepath [Array] Cached module path # @return [String] File system path to referenced file def self.file_path(src, modulepaths) - raise "Bad parameter source #{src}" unless src =~ %r{^puppet:///modules/([^/]+)/(.+)} - path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2)) + unless src =~ %r{^puppet:///modules/([^/]+)/(.+)} + raise ArgumentError, "Bad parameter source #{src}" + end + path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2)) modulepaths.each do |mp| file = File.join(mp, path) return file if File.file?(file) diff --git a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb index 3de4b85c..ec275102 100644 --- a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb @@ -11,6 +11,38 @@ def catalog_from_fixture(path) OctocatalogDiff::Catalog.new(json: File.read(OctocatalogDiff::Spec.fixture_path(path))) end + describe '#file_path' do + it 'should raise ArgumentError for unexpected format of file name' do + src = 'asldfkjwoeifjslakfj' + expect do + OctocatalogDiff::CatalogUtil::FileResources.file_path(src, []) + end.to raise_error(ArgumentError, /Bad parameter source/) + end + + it 'should return path if file is found' do + allow(File).to receive(:file?).with('/a/foo/files/bar').and_return(true) + result = OctocatalogDiff::CatalogUtil::FileResources.file_path('puppet:///modules/foo/bar', ['/a']) + expect(result).to eq('/a/foo/files/bar') + end + + it 'should return nil if file is not found' do + allow(File).to receive(:file?).with('/a/foo/files/bar').and_return(false) + result = OctocatalogDiff::CatalogUtil::FileResources.file_path('puppet:///modules/foo/bar', ['/a']) + expect(result).to eq(nil) + end + end + + describe '#module_path' do + it 'should return "modules" only when environment.conf is missing' do + end + + it 'should return "modules" if environment.conf has no modulepath' do + end + + it 'should return proper entries from environment.conf modulepath' do + end + end + describe '#convert_file_resources' do before(:each) do @tmpdir = Dir.mktmpdir From fa9b993ef38700ec2d8138e664227dac7b2183f7 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:54:31 -0600 Subject: [PATCH 08/15] Add spec tests for module_path --- lib/octocatalog-diff/catalog-util/fileresources.rb | 4 +++- .../tests/catalog-util/fileresources_spec.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 811b8892..04527808 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -47,7 +47,9 @@ def self.file_path(src, modulepaths) # @return [Array] Module paths def self.module_path(compilation_dir) environment_conf = File.join(compilation_dir, 'environment.conf') - return [File.join(compilation_dir, 'modules')] unless File.file?(environment_conf) + unless File.file?(environment_conf) + return [File.join(compilation_dir, 'modules')] + end # This doesn't support multi-line, continuations with backslash, etc. # Does it need to?? diff --git a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb index ec275102..22be51a7 100644 --- a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb @@ -34,12 +34,23 @@ def catalog_from_fixture(path) describe '#module_path' do it 'should return "modules" only when environment.conf is missing' do + allow(File).to receive(:file?).with('/a/environment.conf').and_return(false) + result = OctocatalogDiff::CatalogUtil::FileResources.module_path('/a') + expect(result).to eq(['/a/modules']) end it 'should return "modules" if environment.conf has no modulepath' do + allow(File).to receive(:file?).with('/a/environment.conf').and_return(true) + allow(File).to receive(:read).with('/a/environment.conf').and_return('foo') + result = OctocatalogDiff::CatalogUtil::FileResources.module_path('/a') + expect(result).to eq(['/a/modules']) end it 'should return proper entries from environment.conf modulepath' do + allow(File).to receive(:file?).with('/a/environment.conf').and_return(true) + allow(File).to receive(:read).with('/a/environment.conf').and_return('modulepath=modules:site:$basemoduledir') + result = OctocatalogDiff::CatalogUtil::FileResources.module_path('/a') + expect(result).to eq(['/a/modules', '/a/site']) end end From aab65029cbc1c96ef8050eb716635b2a534f4814 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 11:57:14 -0600 Subject: [PATCH 09/15] Do not need test coverage for the bug case --- lib/octocatalog-diff/catalog-util/fileresources.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 04527808..f873dfba 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -96,7 +96,10 @@ def self._convert_file_resources(resources, compilation_dir) # However, the fact that we found *something* at this location indicates that the catalog # is probably correct. Hence, the very general .exist? check. else - raise Errno::ENOENT, "Unable to find '#{resource['parameters']['source']}' at #{path}!" + # This is probably a bug + # :nocov: + raise "Unable to find '#{resource['parameters']['source']}' at #{path}!" + # :nocov: end end resource From 844b9da754ce4e1389988b0f350c28c5cff2b9ee Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 12:02:10 -0600 Subject: [PATCH 10/15] Fix comment --- lib/octocatalog-diff/catalog-util/fileresources.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index f873dfba..8f0a966f 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -26,7 +26,7 @@ def self.convert_file_resources(obj) # directory doesn't exist or the module isn't found in there). If the file can't be found then # this returns nil which may trigger an error. # @param src [String] A file reference: puppet:///modules/xxx/yyy - # @param modulepath [Array] Cached module path + # @param modulepaths [Array] Cached module path # @return [String] File system path to referenced file def self.file_path(src, modulepaths) unless src =~ %r{^puppet:///modules/([^/]+)/(.+)} From fc627b73ef33454caf82cd4df5bdedf65ab6b428 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 8 Nov 2016 12:07:55 -0600 Subject: [PATCH 11/15] Update integration test with proper error regexp --- .../octocatalog-diff/integration/convert_file_resources_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb index 881b440c..24c8a62f 100644 --- a/spec/octocatalog-diff/integration/convert_file_resources_spec.rb +++ b/spec/octocatalog-diff/integration/convert_file_resources_spec.rb @@ -118,7 +118,7 @@ expect(result[:exitcode]).to eq(-1) expect(result[:exception]).to be_a_kind_of(OctocatalogDiff::CatalogDiff::Cli::Catalogs::CatalogError) expect(result[:exception].message).to match(/failed to compile with Errno::ENOENT/) - expect(result[:exception].message).to match(%r{Unable to find 'puppet:///modules/test/foo-new' at}) + expect(result[:exception].message).to match(%r{Unable to resolve 'puppet:///modules/test/foo-new'}) end end end From 7857f7f1a70d1509e6784338a27251b367196ad5 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 15 Nov 2016 08:06:01 -0600 Subject: [PATCH 12/15] Version bump and changelog update --- .version | 2 +- doc/CHANGELOG.md | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.version b/.version index 7d856835..d1d899fa 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -0.5.4 +0.5.5 diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 80042f09..2d39a53b 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -8,6 +8,15 @@ +0.5.5 +2016-11-16 + + + + + 0.5.4 2016-11-07 From 170233fcc4f35fe84b05d11720a1784d8769c6e9 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 15 Nov 2016 08:32:23 -0600 Subject: [PATCH 13/15] Change .file? to .exist? to avoid failing if it points at a directory --- lib/octocatalog-diff/catalog-util/fileresources.rb | 2 +- .../repos/modulepath/modules/modulestest/files/foo/bar.txt | 1 + .../repos/modulepath/modules/modulestest/manifests/init.pp | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/foo/bar.txt diff --git a/lib/octocatalog-diff/catalog-util/fileresources.rb b/lib/octocatalog-diff/catalog-util/fileresources.rb index 8f0a966f..a6ba0d0b 100644 --- a/lib/octocatalog-diff/catalog-util/fileresources.rb +++ b/lib/octocatalog-diff/catalog-util/fileresources.rb @@ -36,7 +36,7 @@ def self.file_path(src, modulepaths) path = File.join(Regexp.last_match(1), 'files', Regexp.last_match(2)) modulepaths.each do |mp| file = File.join(mp, path) - return file if File.file?(file) + return file if File.exist?(file) end nil diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/foo/bar.txt b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/foo/bar.txt new file mode 100644 index 00000000..76f177f1 --- /dev/null +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/files/foo/bar.txt @@ -0,0 +1 @@ +# Hi diff --git a/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp index 306867f5..e09fcc49 100644 --- a/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp +++ b/spec/octocatalog-diff/fixtures/repos/modulepath/modules/modulestest/manifests/init.pp @@ -2,4 +2,10 @@ file { '/tmp/modulestest': source => 'puppet:///modules/modulestest/tmp/modulestest', } + + file { '/tmp/foobaz': + ensure => directory, + source => 'puppet:///modules/modulestest/foo', + recurse => true, + } } From b1b4dd01d43f34dc130a531d0be238b1cb9b9f8b Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 15 Nov 2016 08:45:49 -0600 Subject: [PATCH 14/15] Add rspec test covering both files and directories --- .../catalogs/catalog-modules-test.json | 58 +++++++++++++++++++ .../tests/catalog-util/fileresources_spec.rb | 44 +++++++++++++- 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 spec/octocatalog-diff/fixtures/catalogs/catalog-modules-test.json diff --git a/spec/octocatalog-diff/fixtures/catalogs/catalog-modules-test.json b/spec/octocatalog-diff/fixtures/catalogs/catalog-modules-test.json new file mode 100644 index 00000000..c37bad9f --- /dev/null +++ b/spec/octocatalog-diff/fixtures/catalogs/catalog-modules-test.json @@ -0,0 +1,58 @@ +{ + "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/modulestest/manifests/init.pp", + "line": 37, + "exported": false, + "parameters": { + "backup": false, + "mode": "0440", + "owner": "root", + "group": "root", + "source": "puppet:///modules/modulestest/tmp/modulestest" + } + }, + { + "type": "File", + "title": "/tmp/foobaz", + "tags": ["file","class"], + "file": "/x/modules/modulestest/manifests/init.pp", + "line": 37, + "exported": false, + "parameters": { + "backup": false, + "ensure": "directory", + "mode": "0755", + "owner": "root", + "group": "root", + "source": "puppet:///modules/modulestest/foo" + } + } + ], + "classes": [ + "test" + ] +} diff --git a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb index 22be51a7..0e38504c 100644 --- a/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb +++ b/spec/octocatalog-diff/tests/catalog-util/fileresources_spec.rb @@ -20,13 +20,13 @@ def catalog_from_fixture(path) end it 'should return path if file is found' do - allow(File).to receive(:file?).with('/a/foo/files/bar').and_return(true) + allow(File).to receive(:exist?).with('/a/foo/files/bar').and_return(true) result = OctocatalogDiff::CatalogUtil::FileResources.file_path('puppet:///modules/foo/bar', ['/a']) expect(result).to eq('/a/foo/files/bar') end it 'should return nil if file is not found' do - allow(File).to receive(:file?).with('/a/foo/files/bar').and_return(false) + allow(File).to receive(:exist?).with('/a/foo/files/bar').and_return(false) result = OctocatalogDiff::CatalogUtil::FileResources.file_path('puppet:///modules/foo/bar', ['/a']) expect(result).to eq(nil) end @@ -54,6 +54,46 @@ def catalog_from_fixture(path) end end + context 'with mixed files and directories' do + describe '#convert_file_resources' do + before(:each) do + @tmpdir = Dir.mktmpdir + FileUtils.cp_r OctocatalogDiff::Spec.fixture_path('repos/modulepath/manifests'), @tmpdir + FileUtils.cp_r OctocatalogDiff::Spec.fixture_path('repos/modulepath/modules'), @tmpdir + Dir.mkdir File.join(@tmpdir, 'environments') + File.symlink @tmpdir, File.join(@tmpdir, 'environments', 'production') + File.open(File.join(@tmpdir, 'manifests', 'site.pp'), 'w') { |f| f.write "include modulestest\n" } + + @obj = catalog_from_fixture('catalogs/catalog-modules-test.json') + @obj.compilation_dir = @tmpdir + @resources_save = @obj.resources.dup + OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(@obj) + end + + after(:each) do + FileUtils.remove_entry_secure @tmpdir if File.directory?(@tmpdir) + end + + it 'should populate content of a file' do + r = @obj.resources.select { |x| x['type'] == 'File' && x['title'] == '/tmp/foo' } + expect(r).to be_a_kind_of(Array) + expect(r.size).to eq(1) + expect(r.first).to be_a_kind_of(Hash) + expect(r.first['parameters'].key?('source')).to eq(false) + expect(r.first['parameters']['content']).to eq("Modules Test\n") + end + + it 'should leave a directory unmodified' do + r = @obj.resources.select { |x| x['type'] == 'File' && x['title'] == '/tmp/foobaz' } + expect(r).to be_a_kind_of(Array) + expect(r.size).to eq(1) + expect(r.first).to be_a_kind_of(Hash) + expect(r.first['parameters'].key?('content')).to eq(false) + expect(r.first['parameters']['source']).to eq('puppet:///modules/modulestest/foo') + end + end + end + describe '#convert_file_resources' do before(:each) do @tmpdir = Dir.mktmpdir From 583a419f6674ff007ac439712e1385a1ee7c08fc Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Tue, 15 Nov 2016 08:46:52 -0600 Subject: [PATCH 15/15] Version bump --- .version | 2 +- doc/CHANGELOG.md | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.version b/.version index d1d899fa..b49b2533 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -0.5.5 +0.5.6 diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 2d39a53b..c738e84d 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -8,7 +8,7 @@ -0.5.5 +0.5.6 2016-11-16
    @@ -17,6 +17,13 @@ +0.5.5 +- + +Unreleased internal version + + + 0.5.4 2016-11-07