From a04f37d689d8425336dd84a54bed34a90d57c829 Mon Sep 17 00:00:00 2001 From: Hunter Haugen Date: Tue, 19 Jan 2016 13:05:27 -0800 Subject: [PATCH] Fix versioncmp when version is undef The versioncmp() function fails when the first argument is not a string, and we are often comparing undef values. This fix first checks that the variable is a true value (undef is falsy) before comparing it. --- manifests/params.pp | 48 +++++++++++++++-------------- manifests/repo.pp | 2 +- manifests/server/config.pp | 2 +- spec/classes/mongos_service_spec.rb | 20 +++++++----- spec/classes/repo_spec.rb | 17 +++++----- 5 files changed, 49 insertions(+), 40 deletions(-) diff --git a/manifests/params.pp b/manifests/params.pp index 0926f3946..7f47765b4 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -22,6 +22,8 @@ $manage_package = pick($mongodb::globals::manage_package, $mongodb::globals::manage_package_repo, false) + $version = $::mongodb::globals::version + # Amazon Linux's OS Family is 'Linux', operating system 'Amazon'. case $::osfamily { 'RedHat', 'Linux': { @@ -29,7 +31,7 @@ if $manage_package { $user = pick($::mongodb::globals::user, 'mongod') $group = pick($::mongodb::globals::group, 'mongod') - if ($::mongodb::globals::version == undef) { + if ($version == undef) { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-org-server') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-org-shell') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-org-mongos') @@ -38,20 +40,20 @@ $package_ensure_mongos = true } else { # check if the version is greater than 2.6 - if(versioncmp($::mongodb::globals::version, '2.6.0') >= 0) { + if $version and (versioncmp($version, '2.6.0') >= 0) { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-org-server') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-org-shell') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-org-mongos') - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version - $package_ensure_mongos = $::mongodb::globals::version + $package_ensure = $version + $package_ensure_client = $version + $package_ensure_mongos = $version } else { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-10gen') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-10gen') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-10gen') - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version #this is still needed in case they are only installing the client - $package_ensure_mongos = $::mongodb::globals::version + $package_ensure = $version + $package_ensure_client = $version #this is still needed in case they are only installing the client + $package_ensure_mongos = $version } } $service_name = pick($::mongodb::globals::service_name, 'mongod') @@ -70,14 +72,14 @@ } else { # RedHat/CentOS doesn't come with a prepacked mongodb # so we assume that you are using EPEL repository. - if ($::mongodb::globals::version == undef) { + if ($version == undef) { $package_ensure = true $package_ensure_client = true $package_ensure_mongos = true } else { - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version - $package_ensure_mongos = $::mongodb::globals::version + $package_ensure = $version + $package_ensure_client = $version + $package_ensure_mongos = $version } $user = pick($::mongodb::globals::user, 'mongodb') $group = pick($::mongodb::globals::group, 'mongodb') @@ -114,7 +116,7 @@ if $manage_package { $user = pick($::mongodb::globals::user, 'mongodb') $group = pick($::mongodb::globals::group, 'mongodb') - if ($::mongodb::globals::version == undef) { + if ($version == undef) { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-org-server') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-org-shell') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-org-mongos') @@ -125,21 +127,21 @@ $config = '/etc/mongod.conf' } else { # check if the version is greater than 2.6 - if(versioncmp($::mongodb::globals::version, '2.6.0') >= 0) { + if $version and (versioncmp($version, '2.6.0') >= 0) { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-org-server') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-org-shell') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-org-mongos') - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version - $package_ensure_mongos = $::mongodb::globals::version + $package_ensure = $version + $package_ensure_client = $version + $package_ensure_mongos = $version $service_name = pick($::mongodb::globals::service_name, 'mongod') $config = '/etc/mongod.conf' } else { $server_package_name = pick($::mongodb::globals::server_package_name, 'mongodb-10gen') $client_package_name = pick($::mongodb::globals::client_package_name, 'mongodb-10gen') $mongos_package_name = pick($::mongodb::globals::mongos_package_name, 'mongodb-10gen') - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version #this is still needed in case they are only installing the client + $package_ensure = $version + $package_ensure_client = $version #this is still needed in case they are only installing the client $service_name = pick($::mongodb::globals::service_name, 'mongodb') $config = '/etc/mongodb.conf' } @@ -155,14 +157,14 @@ # I would not recommend to use the prepacked # mongodb server on Ubuntu 12.04 or Debian 6/7, # because its really outdated - if ($::mongodb::globals::version == undef) { + if ($version == undef) { $package_ensure = true $package_ensure_client = true $package_ensure_mongos = true } else { - $package_ensure = $::mongodb::globals::version - $package_ensure_client = $::mongodb::globals::version - $package_ensure_mongos = $::mongodb::globals::version + $package_ensure = $version + $package_ensure_client = $version + $package_ensure_mongos = $version } $user = pick($::mongodb::globals::user, 'mongodb') $group = pick($::mongodb::globals::group, 'mongodb') diff --git a/manifests/repo.pp b/manifests/repo.pp index 70b6f6c97..aa9a99e33 100644 --- a/manifests/repo.pp +++ b/manifests/repo.pp @@ -16,7 +16,7 @@ $location = 'https://repo.mongodb.com/yum/redhat/$releasever/mongodb-enterprise/stable/$basearch/' $description = 'MongoDB Enterprise Repository' } - elsif (versioncmp($version, '3.0.0') >= 0) { + elsif $version and (versioncmp($version, '3.0.0') >= 0) { $mongover = split($version, '[.]') $location = $::architecture ? { 'x86_64' => "http://repo.mongodb.org/yum/redhat/${::operatingsystemmajrelease}/mongodb-org/${mongover[0]}.${mongover[1]}/x86_64/", diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 6c9df389e..636a12e44 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -100,7 +100,7 @@ #Pick which config content to use if $config_content { $cfg_content = $config_content - } elsif (versioncmp($version, '2.6.0') >= 0) { + } elsif $version and (versioncmp($version, '2.6.0') >= 0) { # Template uses: # - $auth # - $bind_ip diff --git a/spec/classes/mongos_service_spec.rb b/spec/classes/mongos_service_spec.rb index b95582944..41730d0c8 100644 --- a/spec/classes/mongos_service_spec.rb +++ b/spec/classes/mongos_service_spec.rb @@ -5,8 +5,9 @@ context 'on Debian with service_manage set to true' do let :facts do { - :osfamily => 'Debian', - :operatingsystem => 'Debian', + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :operatingsystemrelease => '7.0', } end @@ -29,8 +30,9 @@ context 'on Debian with service_manage set to false' do let :facts do { - :osfamily => 'Debian', - :operatingsystem => 'Debian', + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :operatingsystemrelease => '7.0', } end @@ -50,8 +52,9 @@ context 'on RedHat with service_manage set to true' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemrelease => '7.0', } end @@ -78,8 +81,9 @@ context 'on RedHat with service_manage set to false' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemrelease => '7.0', } end diff --git a/spec/classes/repo_spec.rb b/spec/classes/repo_spec.rb index eeb093740..79e14a5cd 100644 --- a/spec/classes/repo_spec.rb +++ b/spec/classes/repo_spec.rb @@ -5,9 +5,10 @@ context 'when deploying on Debian' do let :facts do { - :osfamily => 'Debian', - :operatingsystem => 'Debian', - :lsbdistid => 'Debian', + :osfamily => 'Debian', + :operatingsystem => 'Debian', + :operatingsystemrelease => '7.0', + :lsbdistid => 'Debian', } end @@ -19,8 +20,9 @@ context 'when deploying on CentOS' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'CentOS', + :osfamily => 'RedHat', + :operatingsystem => 'CentOS', + :operatingsystemrelease => '7.0', } end @@ -32,8 +34,9 @@ context 'when yumrepo has a proxy set' do let :facts do { - :osfamily => 'RedHat', - :operatingsystem => 'RedHat', + :osfamily => 'RedHat', + :operatingsystem => 'RedHat', + :operatingsystemrelease => '7.0', } end let :params do