From dbfa88f6840f1af3721280584be968e8fe17967a Mon Sep 17 00:00:00 2001 From: Cody Herriges Date: Tue, 10 Nov 2015 14:34:12 -0800 Subject: [PATCH] Make sure Facter is only executed on agent This commit moves conditionals based on values obtained from Facter inside code blocks that are only executed on agents and replaces the use of Facter['osfamily'].value with Facter.value:(:osfamily). Without this change we are returning values based off the master's facts, so if a Debian agent checks into a RedHat master then the type is currently making a decision based off of RedHat and not the actual OS of the agent running the catalog. Code ran outside blocks is evaluated on masters and inside a block is evaluated by the agent when the catalog is executed. We do not notice this because all our testing uses "puppet apply" and autorequire only matters when they find a matching resource name in the catalog. The latter results in no error because the relationship is simply ignored on the Debian agent if a package resource with the RedHat name is not found. This issue was found by running facter 3, the C++11 re-implementation under the jruby and clojure implemented puppetserver. The clojure jni shim written so that facter can be ran inside of puppetserver does not implement the [] method for the Facter module but the ruby shim does. So I noticed that the code had an issue when Facter['osfamily'].value was executed on the master, which returned a not method error. Once again, something we didn't notice becuase we do not test against a master. I decided to migrate to Facter.value(:osfamily) to simply keep to an API that is consistent across both puppet and puppetserver but fixing where the code runs does actually solve the not method found error. Change-Id: I24b0b20b2839c7bc33a76ac14849783f2285579f --- lib/puppet/type/swift_object_expirer_config.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/puppet/type/swift_object_expirer_config.rb b/lib/puppet/type/swift_object_expirer_config.rb index 413373fb..14a529f9 100644 --- a/lib/puppet/type/swift_object_expirer_config.rb +++ b/lib/puppet/type/swift_object_expirer_config.rb @@ -44,12 +44,10 @@ Puppet::Type.newtype(:swift_object_expirer_config) do end # Require the package providing object-expirer to be present - if Facter['osfamily'].value == 'Debian' - autorequire(:package) do + autorequire(:package) do + if Facter.value(:osfamily) == 'Debian' 'swift-object-expirer' - end - elsif Facter['osfamily'].value == 'RedHat' - autorequire(:package) do + elsif Facter.value(:osfamily) == 'RedHat' 'openstack-swift-proxy' end end