From 53b946fa2d289c2bb1b9e5173f6ae872f749d47d Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Tue, 14 Mar 2017 14:03:20 +0000 Subject: [PATCH 1/2] Use versioncmp to check Puppet version for 4.10.x compatibility `Puppet.version.to_f` on Puppet 4.10.0 will evaluate to `4.1`, causing test and behavioural changes when conditionals check that the version is equal or greater than versions such as `4.3`. Version comparisons that are vulnerable to this have been changed to use Puppet's versioncmp implementation, while most others only check for for major version boundaries which is safe. --- lib/puppet-syntax/manifests.rb | 2 +- lib/puppet-syntax/tasks/puppet-syntax.rb | 2 +- spec/puppet-syntax/manifests_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/puppet-syntax/manifests.rb b/lib/puppet-syntax/manifests.rb index e6a4ab8..1f76d53 100644 --- a/lib/puppet-syntax/manifests.rb +++ b/lib/puppet-syntax/manifests.rb @@ -59,7 +59,7 @@ def check(filelist) private def validate_manifest(file) Puppet[:parser] = 'future' if PuppetSyntax.future_parser and Puppet::PUPPETVERSION.to_i < 4 - Puppet[:app_management] = true if PuppetSyntax.app_management && (Puppet::PUPPETVERSION.to_f >= 4.3 && Puppet::PUPPETVERSION.to_i < 5) + Puppet[:app_management] = true if PuppetSyntax.app_management && (Puppet::Util::Package.versioncmp(Puppet.version, '4.3.0') >= 0 && Puppet.version.to_i < 5) Puppet::Face[:parser, :current].validate(file) end end diff --git a/lib/puppet-syntax/tasks/puppet-syntax.rb b/lib/puppet-syntax/tasks/puppet-syntax.rb index 3f8e861..279b479 100644 --- a/lib/puppet-syntax/tasks/puppet-syntax.rb +++ b/lib/puppet-syntax/tasks/puppet-syntax.rb @@ -54,7 +54,7 @@ def initialize(*args) 'true'. The `future_parser setting will be ignored. EOS end - if Puppet::PUPPETVERSION.to_f < 4.3 and PuppetSyntax.app_management + if Puppet::Util::Package.versioncmp(Puppet.version, '4.3.0') < 0 and PuppetSyntax.app_management $stderr.puts <<-EOS [WARNING] Puppet `app_management` has been detected but the Puppet version is less then 4.3. The `app_management` setting will be ignored. diff --git a/spec/puppet-syntax/manifests_spec.rb b/spec/puppet-syntax/manifests_spec.rb index a74c778..ad3a0e4 100644 --- a/spec/puppet-syntax/manifests_spec.rb +++ b/spec/puppet-syntax/manifests_spec.rb @@ -155,7 +155,7 @@ before(:each) { PuppetSyntax.app_management = true } - if Puppet::PUPPETVERSION.to_f >= 4.3 + if Puppet::Util::Package.versioncmp(Puppet.version, '4.3.0') >= 0 it 'should successfully parse an application manifest on Puppet >= 4.3.0' do expect(PuppetSyntax.app_management).to eq(true) From 5b8944ad788c01a469a8a40f94fb78f530380a6c Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Tue, 14 Mar 2017 14:06:00 +0000 Subject: [PATCH 2/2] Replace PUPPETVERSION with public Puppet.version API --- lib/puppet-syntax.rb | 4 ++-- lib/puppet-syntax/manifests.rb | 2 +- lib/puppet-syntax/tasks/puppet-syntax.rb | 2 +- lib/puppet-syntax/templates.rb | 2 +- spec/puppet-syntax/manifests_spec.rb | 10 +++++----- spec/puppet-syntax/templates_spec.rb | 4 ++-- spec/puppet-syntax_spec.rb | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/puppet-syntax.rb b/lib/puppet-syntax.rb index bea66d7..98f83f9 100644 --- a/lib/puppet-syntax.rb +++ b/lib/puppet-syntax.rb @@ -13,7 +13,7 @@ module PuppetSyntax "hiera*.*yaml" ] @fail_on_deprecation_notices = true - @app_management = Puppet::PUPPETVERSION.to_i >= 5 ? true : false + @app_management = Puppet.version.to_i >= 5 ? true : false @check_hiera_keys = false class << self @@ -26,7 +26,7 @@ class << self attr_reader :app_management def app_management=(app_management) - raise 'app_management cannot be disabled on Puppet 5 or higher' if Puppet::PUPPETVERSION.to_i >= 5 && !app_management + raise 'app_management cannot be disabled on Puppet 5 or higher' if Puppet.version.to_i >= 5 && !app_management @app_management = app_management end end diff --git a/lib/puppet-syntax/manifests.rb b/lib/puppet-syntax/manifests.rb index 1f76d53..505ab33 100644 --- a/lib/puppet-syntax/manifests.rb +++ b/lib/puppet-syntax/manifests.rb @@ -58,7 +58,7 @@ def check(filelist) private def validate_manifest(file) - Puppet[:parser] = 'future' if PuppetSyntax.future_parser and Puppet::PUPPETVERSION.to_i < 4 + Puppet[:parser] = 'future' if PuppetSyntax.future_parser and Puppet.version.to_i < 4 Puppet[:app_management] = true if PuppetSyntax.app_management && (Puppet::Util::Package.versioncmp(Puppet.version, '4.3.0') >= 0 && Puppet.version.to_i < 5) Puppet::Face[:parser, :current].validate(file) end diff --git a/lib/puppet-syntax/tasks/puppet-syntax.rb b/lib/puppet-syntax/tasks/puppet-syntax.rb index 279b479..cff4276 100644 --- a/lib/puppet-syntax/tasks/puppet-syntax.rb +++ b/lib/puppet-syntax/tasks/puppet-syntax.rb @@ -48,7 +48,7 @@ def initialize(*args) desc 'Syntax check Puppet manifests' task :manifests do |t| - if Puppet::PUPPETVERSION.to_i >= 4 and PuppetSyntax.future_parser + if Puppet.version.to_i >= 4 and PuppetSyntax.future_parser $stderr.puts <<-EOS [INFO] Puppet 4 has been detected and `future_parser` has been set to 'true'. The `future_parser setting will be ignored. diff --git a/lib/puppet-syntax/templates.rb b/lib/puppet-syntax/templates.rb index 61efc2e..c62a706 100644 --- a/lib/puppet-syntax/templates.rb +++ b/lib/puppet-syntax/templates.rb @@ -27,7 +27,7 @@ def check(filelist) end def validate_epp(filename) - if Puppet::PUPPETVERSION.to_f < 3.7 + if Puppet.version.to_f < 3.7 raise "Cannot validate EPP without Puppet 4 or future parser (3.7+)" end diff --git a/spec/puppet-syntax/manifests_spec.rb b/spec/puppet-syntax/manifests_spec.rb index ad3a0e4..69d8d51 100644 --- a/spec/puppet-syntax/manifests_spec.rb +++ b/spec/puppet-syntax/manifests_spec.rb @@ -28,7 +28,7 @@ files = fixture_manifests('fail_error.pp') output, has_errors = subject.check(files) - if Puppet::PUPPETVERSION.to_i >= 4 + if Puppet.version.to_i >= 4 expect(output.size).to eq(3) expect(output[2]).to match(/2 errors. Giving up/) expect(has_errors).to eq(true) @@ -72,7 +72,7 @@ output, has_errors = subject.check(files) expect(has_errors).to eq(true) - if Puppet::PUPPETVERSION.to_i >= 4 + if Puppet.version.to_i >= 4 expect(output.size).to eq(5) expect(output[0]).to match(/This Name has no effect. A Host Class Definition can not end with a value-producing expression without other effect at \S*\/fail_error.pp:2:32$/) expect(output[1]).to match(/This Name has no effect. A value(-producing expression without other effect may only be placed last in a block\/sequence| was produced and then forgotten.*) at \S*\/fail_error.pp:2:3$/) @@ -138,10 +138,10 @@ describe 'app_management' do after do - PuppetSyntax.app_management = false if Puppet::PUPPETVERSION.to_i < 5 + PuppetSyntax.app_management = false if Puppet.version.to_i < 5 end - context 'app_management = false (default)', :if => (Puppet::PUPPETVERSION.to_i < 5) do + context 'app_management = false (default)', :if => (Puppet.version.to_i < 5) do it 'should fail to parse an application manifest' do files = fixture_manifests(['test_app.pp']) @@ -211,7 +211,7 @@ PuppetSyntax.future_parser = true } - if Puppet::Util::Package.versioncmp(Puppet.version, '3.2') >= 0 and Puppet::PUPPETVERSION.to_i < 4 + if Puppet::Util::Package.versioncmp(Puppet.version, '3.2') >= 0 and Puppet.version.to_i < 4 context 'Puppet >= 3.2 < 4' do it 'should pass with future option set to true on future manifest' do files = fixture_manifests(['future_syntax.pp']) diff --git a/spec/puppet-syntax/templates_spec.rb b/spec/puppet-syntax/templates_spec.rb index 9d6714d..cb97a16 100644 --- a/spec/puppet-syntax/templates_spec.rb +++ b/spec/puppet-syntax/templates_spec.rb @@ -67,7 +67,7 @@ expect(res).to match([]) end - if Puppet::PUPPETVERSION.to_f < 3.7 + if Puppet.version.to_f < 3.7 context 'on Puppet < 3.7' do it 'should throw an exception when parsing EPP files' do file = fixture_templates('pass.epp') @@ -87,7 +87,7 @@ end end - if Puppet::PUPPETVERSION.to_f >= 3.7 + if Puppet.version.to_f >= 3.7 context 'on Puppet >= 3.7' do it 'should return nothing from a valid file' do files = fixture_templates('pass.epp') diff --git a/spec/puppet-syntax_spec.rb b/spec/puppet-syntax_spec.rb index 2f96566..fe85b54 100644 --- a/spec/puppet-syntax_spec.rb +++ b/spec/puppet-syntax_spec.rb @@ -3,7 +3,7 @@ describe PuppetSyntax do after do PuppetSyntax.exclude_paths = [] - PuppetSyntax.app_management = false if Puppet::PUPPETVERSION.to_i < 5 + PuppetSyntax.app_management = false if Puppet.version.to_i < 5 end it 'should default exclude_paths to empty array' do @@ -30,7 +30,7 @@ expect(PuppetSyntax.app_management).to eq(true) end - it 'should raise error when app_management is disabled on 5.x', :if => (Puppet::PUPPETVERSION.to_i >= 5) do + it 'should raise error when app_management is disabled on 5.x', :if => (Puppet.version.to_i >= 5) do expect { PuppetSyntax.app_management = false }.to raise_error(/app_management cannot be disabled on Puppet 5 or higher/) end