From b011f425911294527b1724e952cc2c438c55ef13 Mon Sep 17 00:00:00 2001
From: Cody Herriges <cody@herriges.org>
Date: Tue, 10 Nov 2015 13:51:21 -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: I773f20e0bc1e917f6ec5fa8830f5bbfa60b6cd8a
---
 lib/puppet/type/glance_api_config.rb         | 8 +++-----
 lib/puppet/type/glance_api_paste_ini.rb      | 8 +++-----
 lib/puppet/type/glance_cache_config.rb       | 8 +++-----
 lib/puppet/type/glance_registry_config.rb    | 8 +++-----
 lib/puppet/type/glance_registry_paste_ini.rb | 8 +++-----
 5 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/lib/puppet/type/glance_api_config.rb b/lib/puppet/type/glance_api_config.rb
index 76673e1b..2a96e5c9 100644
--- a/lib/puppet/type/glance_api_config.rb
+++ b/lib/puppet/type/glance_api_config.rb
@@ -46,12 +46,10 @@ Puppet::Type.newtype(:glance_api_config) do
     defaultto('<SERVICE DEFAULT>')
   end
 
-  if Facter['osfamily'].value == 'Debian'
-    autorequire(:package) do
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
       'glance-api'
-    end
-  elsif Facter['osfamily'].value == 'RedHat'
-    autorequire(:package) do
+    elsif Facter.value(:osfamily) == 'RedHat'
       'openstack-glance'
     end
   end
diff --git a/lib/puppet/type/glance_api_paste_ini.rb b/lib/puppet/type/glance_api_paste_ini.rb
index f83467c2..aefcca4f 100644
--- a/lib/puppet/type/glance_api_paste_ini.rb
+++ b/lib/puppet/type/glance_api_paste_ini.rb
@@ -45,12 +45,10 @@ Puppet::Type.newtype(:glance_api_paste_ini) do
     defaultto('<SERVICE DEFAULT>')
   end
 
-  if Facter['osfamily'].value == 'Debian'
-    autorequire(:package) do
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
       'glance-api'
-    end
-  elsif Facter['osfamily'].value == 'RedHat'
-    autorequire(:package) do
+    elsif Facter.value(:osfamily) == 'RedHat'
       'openstack-glance'
     end
   end
diff --git a/lib/puppet/type/glance_cache_config.rb b/lib/puppet/type/glance_cache_config.rb
index bacf0fd8..1ed4a3e1 100644
--- a/lib/puppet/type/glance_cache_config.rb
+++ b/lib/puppet/type/glance_cache_config.rb
@@ -46,12 +46,10 @@ Puppet::Type.newtype(:glance_cache_config) do
     defaultto('<SERVICE DEFAULT>')
   end
 
-  if Facter['osfamily'].value == 'Debian'
-    autorequire(:package) do
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
       'glance-api'
-    end
-  elsif Facter['osfamily'].value == 'RedHat'
-    autorequire(:package) do
+    elsif Facter.value(:osfamily) == 'RedHat'
       'openstack-glance'
     end
   end
diff --git a/lib/puppet/type/glance_registry_config.rb b/lib/puppet/type/glance_registry_config.rb
index e5d8ab6d..a8e0b1ed 100644
--- a/lib/puppet/type/glance_registry_config.rb
+++ b/lib/puppet/type/glance_registry_config.rb
@@ -46,12 +46,10 @@ Puppet::Type.newtype(:glance_registry_config) do
     defaultto('<SERVICE DEFAULT>')
   end
 
-  if Facter['osfamily'].value == 'Debian'
-    autorequire(:package) do
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
       'glance-registry'
-    end
-  elsif Facter['osfamily'].value == 'RedHat'
-    autorequire(:package) do
+    elsif Facter.value(:osfamily) == 'RedHat'
       'openstack-glance'
     end
   end
diff --git a/lib/puppet/type/glance_registry_paste_ini.rb b/lib/puppet/type/glance_registry_paste_ini.rb
index b9414fd0..0c5b73c4 100644
--- a/lib/puppet/type/glance_registry_paste_ini.rb
+++ b/lib/puppet/type/glance_registry_paste_ini.rb
@@ -45,12 +45,10 @@ Puppet::Type.newtype(:glance_registry_paste_ini) do
     defaultto('<SERVICE DEFAULT>')
   end
 
-  if Facter['osfamily'].value == 'Debian'
-    autorequire(:package) do
+  autorequire(:package) do
+    if Facter.value(:osfamily) == 'Debian'
       'glance-registry'
-    end
-  elsif Facter['osfamily'].value == 'RedHat'
-    autorequire(:package) do
+    elsif Facter.value(:osfamily) == 'RedHat'
       'openstack-glance'
     end
   end