[puppet/f17] Apply upstream fixes to avoid class level variables (#809911)
Todd Zullinger
tmz at fedoraproject.org
Wed Mar 20 14:12:44 UTC 2013
commit fbc6fe842a897b60154d423afe3c33c5ab8df4e2
Author: Todd Zullinger <tmz at pobox.com>
Date: Tue Mar 19 17:49:04 2013 -0400
Apply upstream fixes to avoid class level variables (#809911)
Thanks to Terje Rosten for the pointer to the upstream commit.
...s-level-variables-as-they-are-not-helpful.patch | 427 ++++++++++++++++++++
puppet.spec | 5 +
2 files changed, 432 insertions(+), 0 deletions(-)
---
diff --git a/0001-Avoid-class-level-variables-as-they-are-not-helpful.patch b/0001-Avoid-class-level-variables-as-they-are-not-helpful.patch
new file mode 100644
index 0000000..5d04fb2
--- /dev/null
+++ b/0001-Avoid-class-level-variables-as-they-are-not-helpful.patch
@@ -0,0 +1,427 @@
+From 6d635127378bbc2f070e479b882fe1a81717e6ca Mon Sep 17 00:00:00 2001
+From: Daniel Pittman <daniel at puppetlabs.com>
+Date: Fri, 9 Mar 2012 19:37:48 -0800
+Subject: [PATCH] Avoid class level variables, as they are not helpful.
+
+Ruby 1.9.3 highlights some of the strange and confusing problems around class
+level variables in Ruby; they tend to look and behave much more like global
+state than people imagine, and lead down the road to trouble.
+
+This migrates away from them to other mechanisms better fitted to solving the
+problem in the Ruby language - and, typically, in ways that are much more
+portable between the different Ruby interpreter implementations.
+
+Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>
+
+Cherry-picked-by: Todd Zullinger <tmz at pobox.com>
+
+Conflicts:
+ lib/puppet/provider/nameservice/directoryservice.rb
+---
+ lib/puppet/provider/nameservice.rb | 32 ++++----
+ .../provider/nameservice/directoryservice.rb | 89 +++++++++++-----------
+ lib/puppet/provider/service/bsd.rb | 12 +--
+ lib/puppet/provider/service/freebsd.rb | 18 ++---
+ lib/puppet/type/tidy.rb | 23 ++----
+ 5 files changed, 83 insertions(+), 91 deletions(-)
+
+diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
+index f4ef2c7..3f10def 100644
+--- a/lib/puppet/provider/nameservice.rb
++++ b/lib/puppet/provider/nameservice.rb
+@@ -115,7 +115,7 @@ class Puppet::Provider::NameService < Puppet::Provider
+ field = field.intern
+ id_generators = {:user => :uid, :group => :gid}
+ if id_generators[@resource.class.name] == field
+- return autogen_id(field)
++ return self.class.autogen_id(field, @resource.class.name)
+ else
+ if value = self.class.autogen_default(field)
+ return value
+@@ -127,33 +127,33 @@ class Puppet::Provider::NameService < Puppet::Provider
+ end
+ end
+
+- # Autogenerate either a uid or a gid. This is hard-coded: we can only
+- # generate one field type per class.
+- def autogen_id(field)
+- highest = 0
+-
+- group = method = nil
+- case @resource.class.name
+- when :user; group = :passwd; method = :uid
+- when :group; group = :group; method = :gid
++ # Autogenerate either a uid or a gid. This is not very flexible: we can
++ # only generate one field type per class, and get kind of confused if asked
++ # for both.
++ def self.autogen_id(field, resource_type)
++ # Figure out what sort of value we want to generate.
++ case resource_type
++ when :user; group = :passwd; method = :uid
++ when :group; group = :group; method = :gid
+ else
+ raise Puppet::DevError, "Invalid resource name #{resource}"
+ end
+
+- # Make sure we don't use the same value multiple times
+- if defined?(@@prevauto)
+- @@prevauto += 1
+- else
++ # Initialize from the data set, if needed.
++ unless @prevauto
++ highest = 0
++
+ Etc.send(group) { |obj|
+ if obj.gid > highest
+ highest = obj.send(method) unless obj.send(method) > 65000
+ end
+ }
+
+- @@prevauto = highest + 1
++ @prevauto = highest
+ end
+
+- @@prevauto
++ # ...and finally increment and return the next value.
++ @prevauto += 1
+ end
+
+ def create
+diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb
+index 51335fc..f1f3fb2 100644
+--- a/lib/puppet/provider/nameservice/directoryservice.rb
++++ b/lib/puppet/provider/nameservice/directoryservice.rb
+@@ -32,37 +32,36 @@ class DirectoryService < Puppet::Provider::NameService
+ # JJM: Note, this is de-coupled from the Puppet::Type, and must
+ # be actively maintained. There may also be collisions with different
+ # types (Users, Groups, Mounts, Hosts, etc...)
+- @@ds_to_ns_attribute_map = {
+- 'RecordName' => :name,
+- 'PrimaryGroupID' => :gid,
+- 'NFSHomeDirectory' => :home,
+- 'UserShell' => :shell,
+- 'UniqueID' => :uid,
+- 'RealName' => :comment,
+- 'Password' => :password,
+- 'GeneratedUID' => :guid,
+- 'IPAddress' => :ip_address,
+- 'ENetAddress' => :en_address,
+- 'GroupMembership' => :members,
+- }
++ def ds_to_ns_attribute_map; self.class.ds_to_ns_attribute_map; end
++ def self.ds_to_ns_attribute_map
++ {
++ 'RecordName' => :name,
++ 'PrimaryGroupID' => :gid,
++ 'NFSHomeDirectory' => :home,
++ 'UserShell' => :shell,
++ 'UniqueID' => :uid,
++ 'RealName' => :comment,
++ 'Password' => :password,
++ 'GeneratedUID' => :guid,
++ 'IPAddress' => :ip_address,
++ 'ENetAddress' => :en_address,
++ 'GroupMembership' => :members,
++ }
++ end
++
+ # JJM The same table as above, inverted.
+- @@ns_to_ds_attribute_map = {
+- :name => 'RecordName',
+- :gid => 'PrimaryGroupID',
+- :home => 'NFSHomeDirectory',
+- :shell => 'UserShell',
+- :uid => 'UniqueID',
+- :comment => 'RealName',
+- :password => 'Password',
+- :guid => 'GeneratedUID',
+- :en_address => 'ENetAddress',
+- :ip_address => 'IPAddress',
+- :members => 'GroupMembership',
+- }
+-
+- @@password_hash_dir = "/var/db/shadow/hash"
+- @@users_plist_dir = '/var/db/dslocal/nodes/Default/users'
++ def ns_to_ds_attribute_map; self.class.ns_to_ds_attribute_map end
++ def self.ns_to_ds_attribute_map
++ @ns_to_ds_attribute_map ||= ds_to_ns_attribute_map.invert
++ end
+
++ def self.password_hash_dir
++ '/var/db/shadow/hash'
++ end
++
++ def self.users_plist_dir
++ '/var/db/dslocal/nodes/Default/users'
++ end
+
+ def self.instances
+ # JJM Class method that provides an array of instance objects of this
+@@ -172,9 +171,9 @@ class DirectoryService < Puppet::Provider::NameService
+ attribute_hash = {}
+ input_hash.keys.each do |key|
+ ds_attribute = key.sub("dsAttrTypeStandard:", "")
+- next unless (@@ds_to_ns_attribute_map.keys.include?(ds_attribute) and type_properties.include? @@ds_to_ns_attribute_map[ds_attribute])
++ next unless (ds_to_ns_attribute_map.keys.include?(ds_attribute) and type_properties.include? ds_to_ns_attribute_map[ds_attribute])
+ ds_value = input_hash[key]
+- case @@ds_to_ns_attribute_map[ds_attribute]
++ case ds_to_ns_attribute_map[ds_attribute]
+ when :members
+ ds_value = ds_value # only members uses arrays so far
+ when :gid, :uid
+@@ -189,7 +188,7 @@ class DirectoryService < Puppet::Provider::NameService
+ end
+ else ds_value = ds_value[0]
+ end
+- attribute_hash[@@ds_to_ns_attribute_map[ds_attribute]] = ds_value
++ attribute_hash[ds_to_ns_attribute_map[ds_attribute]] = ds_value
+ end
+
+ # NBK: need to read the existing password here as it's not actually
+@@ -275,7 +274,7 @@ class DirectoryService < Puppet::Provider::NameService
+ # version '10.10' would be < '10.7' with simple string comparison. This
+ # if-statement only executes if the current version is less-than 10.7
+ if (Puppet::Util::Package.versioncmp(get_macosx_version_major, '10.7') == -1)
+- password_hash_file = "#{@@password_hash_dir}/#{guid}"
++ password_hash_file = "#{password_hash_dir}/#{guid}"
+ begin
+ File.open(password_hash_file, 'w') { |f| f.write(password_hash)}
+ rescue Errno::EACCES => detail
+@@ -313,13 +312,13 @@ class DirectoryService < Puppet::Provider::NameService
+ Please check your password and try again.")
+ end
+
+- if File.exists?("#{@@users_plist_dir}/#{resource_name}.plist")
++ if File.exists?("#{users_plist_dir}/#{resource_name}.plist")
+ # If a plist already exists in /var/db/dslocal/nodes/Default/users, then
+ # we will need to extract the binary plist from the 'ShadowHashData'
+ # key, log the new password into the resultant plist's 'SALTED-SHA512'
+ # key, and then save the entire structure back.
+ users_plist = Plist::parse_xml(plutil( '-convert', 'xml1', '-o', '/dev/stdout', \
+- "#{@@users_plist_dir}/#{resource_name}.plist"))
++ "#{users_plist_dir}/#{resource_name}.plist"))
+
+ # users_plist['ShadowHashData'][0].string is actually a binary plist
+ # that's nested INSIDE the user's plist (which itself is a binary
+@@ -345,8 +344,8 @@ class DirectoryService < Puppet::Provider::NameService
+ # a binary plist.
+ changed_plist = convert_xml_to_binary(converted_hash_plist)
+ users_plist['ShadowHashData'][0].string = changed_plist
+- Plist::Emit.save_plist(users_plist, "#{@@users_plist_dir}/#{resource_name}.plist")
+- plutil('-convert', 'binary1', "#{@@users_plist_dir}/#{resource_name}.plist")
++ Plist::Emit.save_plist(users_plist, "#{users_plist_dir}/#{resource_name}.plist")
++ plutil('-convert', 'binary1', "#{users_plist_dir}/#{resource_name}.plist")
+ end
+ end
+ end
+@@ -357,7 +356,7 @@ class DirectoryService < Puppet::Provider::NameService
+ # if-statement only executes if the current version is less-than 10.7
+ if (Puppet::Util::Package.versioncmp(get_macosx_version_major, '10.7') == -1)
+ password_hash = nil
+- password_hash_file = "#{@@password_hash_dir}/#{guid}"
++ password_hash_file = "#{password_hash_dir}/#{guid}"
+ if File.exists?(password_hash_file) and File.file?(password_hash_file)
+ fail("Could not read password hash file at #{password_hash_file}") if not File.readable?(password_hash_file)
+ f = File.new(password_hash_file)
+@@ -366,11 +365,11 @@ class DirectoryService < Puppet::Provider::NameService
+ end
+ password_hash
+ else
+- if File.exists?("#{@@users_plist_dir}/#{username}.plist")
++ if File.exists?("#{users_plist_dir}/#{username}.plist")
+ # If a plist exists in /var/db/dslocal/nodes/Default/users, we will
+ # extract the binary plist from the 'ShadowHashData' key, decode the
+ # salted-SHA512 password hash, and then return it.
+- users_plist = Plist::parse_xml(plutil('-convert', 'xml1', '-o', '/dev/stdout', "#{@@users_plist_dir}/#{username}.plist"))
++ users_plist = Plist::parse_xml(plutil('-convert', 'xml1', '-o', '/dev/stdout', "#{users_plist_dir}/#{username}.plist"))
+ if users_plist['ShadowHashData']
+ # users_plist['ShadowHashData'][0].string is actually a binary plist
+ # that's nested INSIDE the user's plist (which itself is a binary
+@@ -467,14 +466,14 @@ class DirectoryService < Puppet::Provider::NameService
+
+ def password=(passphrase)
+ exec_arg_vector = self.class.get_exec_preamble("-read", @resource.name)
+- exec_arg_vector << @@ns_to_ds_attribute_map[:guid]
++ exec_arg_vector << ns_to_ds_attribute_map[:guid]
+ begin
+ guid_output = execute(exec_arg_vector)
+ guid_plist = Plist.parse_xml(guid_output)
+ # Although GeneratedUID like all DirectoryService values can be multi-valued
+ # according to the schema, in practice user accounts cannot have multiple UUIDs
+ # otherwise Bad Things Happen, so we just deal with the first value.
+- guid = guid_plist["dsAttrTypeStandard:#{@@ns_to_ds_attribute_map[:guid]}"][0]
++ guid = guid_plist["dsAttrTypeStandard:#{ns_to_ds_attribute_map[:guid]}"][0]
+ self.class.set_password(@resource.name, guid, passphrase)
+ rescue Puppet::ExecutionFailure => detail
+ fail("Could not set #{param} on #{@resource.class.name}[#{@resource.name}]: #{detail}")
+@@ -500,7 +499,7 @@ class DirectoryService < Puppet::Provider::NameService
+ exec_arg_vector = self.class.get_exec_preamble("-create", @resource[:name])
+ # JJM: The following line just maps the NS name to the DS name
+ # e.g. { :uid => 'UniqueID' }
+- exec_arg_vector << @@ns_to_ds_attribute_map[param.intern]
++ exec_arg_vector << ns_to_ds_attribute_map[param.intern]
+ # JJM: The following line sends the actual value to set the property to
+ exec_arg_vector << value.to_s
+ begin
+@@ -529,7 +528,7 @@ class DirectoryService < Puppet::Provider::NameService
+ guid = %x{/usr/bin/uuidgen}.chomp
+
+ exec_arg_vector = self.class.get_exec_preamble("-create", @resource[:name])
+- exec_arg_vector << @@ns_to_ds_attribute_map[:guid] << guid
++ exec_arg_vector << ns_to_ds_attribute_map[:guid] << guid
+ begin
+ execute(exec_arg_vector)
+ rescue Puppet::ExecutionFailure => detail
+@@ -555,7 +554,7 @@ class DirectoryService < Puppet::Provider::NameService
+ add_members(nil, value)
+ else
+ exec_arg_vector = self.class.get_exec_preamble("-create", @resource[:name])
+- exec_arg_vector << @@ns_to_ds_attribute_map[property.intern]
++ exec_arg_vector << ns_to_ds_attribute_map[property.intern]
+ next if property == :password # skip setting the password here
+ exec_arg_vector << value.to_s
+ begin
+diff --git a/lib/puppet/provider/service/bsd.rb b/lib/puppet/provider/service/bsd.rb
+index 0c019fd..5b73dfc 100644
+--- a/lib/puppet/provider/service/bsd.rb
++++ b/lib/puppet/provider/service/bsd.rb
+@@ -9,7 +9,9 @@ Puppet::Type.type(:service).provide :bsd, :parent => :init do
+
+ confine :operatingsystem => [:freebsd, :netbsd, :openbsd]
+
+- @@rcconf_dir = '/etc/rc.conf.d'
++ def rcconf_dir
++ '/etc/rc.conf.d'
++ end
+
+ def self.defpath
+ superclass.defpath
+@@ -17,13 +19,13 @@ Puppet::Type.type(:service).provide :bsd, :parent => :init do
+
+ # remove service file from rc.conf.d to disable it
+ def disable
+- rcfile = File.join(@@rcconf_dir, @model[:name])
++ rcfile = File.join(rcconf_dir, @model[:name])
+ File.delete(rcfile) if File.exists?(rcfile)
+ end
+
+ # if the service file exists in rc.conf.d then it's already enabled
+ def enabled?
+- rcfile = File.join(@@rcconf_dir, @model[:name])
++ rcfile = File.join(rcconf_dir, @model[:name])
+ return :true if File.exists?(rcfile)
+
+ :false
+@@ -32,8 +34,8 @@ Puppet::Type.type(:service).provide :bsd, :parent => :init do
+ # enable service by creating a service file under rc.conf.d with the
+ # proper contents
+ def enable
+- Dir.mkdir(@@rcconf_dir) if not File.exists?(@@rcconf_dir)
+- rcfile = File.join(@@rcconf_dir, @model[:name])
++ Dir.mkdir(rcconf_dir) if not File.exists?(rcconf_dir)
++ rcfile = File.join(rcconf_dir, @model[:name])
+ open(rcfile, 'w') { |f| f << "%s_enable=\"YES\"\n" % @model[:name] }
+ end
+
+diff --git a/lib/puppet/provider/service/freebsd.rb b/lib/puppet/provider/service/freebsd.rb
+index 5e1a36d..6c09090 100644
+--- a/lib/puppet/provider/service/freebsd.rb
++++ b/lib/puppet/provider/service/freebsd.rb
+@@ -5,9 +5,9 @@ Puppet::Type.type(:service).provide :freebsd, :parent => :init do
+ confine :operatingsystem => [:freebsd]
+ defaultfor :operatingsystem => [:freebsd]
+
+- @@rcconf = '/etc/rc.conf'
+- @@rcconf_local = '/etc/rc.conf.local'
+- @@rcconf_dir = '/etc/rc.conf.d'
++ def rcconf() '/etc/rc.conf' end
++ def rcconf_local() '/etc/rc.conf.local' end
++ def rcconf_dir() '/etc/rc.conf.d' end
+
+ def self.defpath
+ superclass.defpath
+@@ -66,7 +66,7 @@ Puppet::Type.type(:service).provide :freebsd, :parent => :init do
+ def rc_replace(service, rcvar, yesno)
+ success = false
+ # Replace in all files, not just in the first found with a match
+- [@@rcconf, @@rcconf_local, @@rcconf_dir + "/#{service}"].each do |filename|
++ [rcconf, rcconf_local, rcconf_dir + "/#{service}"].each do |filename|
+ if File.exists?(filename)
+ s = File.read(filename)
+ if s.gsub!(/(#{rcvar}_enable)=\"?(YES|NO)\"?/, "\\1=\"#{yesno}\"")
+@@ -83,21 +83,21 @@ Puppet::Type.type(:service).provide :freebsd, :parent => :init do
+ def rc_add(service, rcvar, yesno)
+ append = "\# Added by Puppet\n#{rcvar}_enable=\"#{yesno}\"\n"
+ # First, try the one-file-per-service style
+- if File.exists?(@@rcconf_dir)
+- File.open(@@rcconf_dir + "/#{service}", File::WRONLY | File::APPEND | File::CREAT, 0644) {
++ if File.exists?(rcconf_dir)
++ File.open(rcconf_dir + "/#{service}", File::WRONLY | File::APPEND | File::CREAT, 0644) {
+ |f| f << append
+ self.debug("Appended to #{f.path}")
+ }
+ else
+ # Else, check the local rc file first, but don't create it
+- if File.exists?(@@rcconf_local)
+- File.open(@@rcconf_local, File::WRONLY | File::APPEND) {
++ if File.exists?(rcconf_local)
++ File.open(rcconf_local, File::WRONLY | File::APPEND) {
+ |f| f << append
+ self.debug("Appended to #{f.path}")
+ }
+ else
+ # At last use the standard rc.conf file
+- File.open(@@rcconf, File::WRONLY | File::APPEND | File::CREAT, 0644) {
++ File.open(rcconf, File::WRONLY | File::APPEND | File::CREAT, 0644) {
+ |f| f << append
+ self.debug("Appended to #{f.path}")
+ }
+diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
+index 6fe1e74..63f3e77 100755
+--- a/lib/puppet/type/tidy.rb
++++ b/lib/puppet/type/tidy.rb
+@@ -101,17 +101,16 @@ Puppet::Type.newtype(:tidy) do
+
+ Specifying 0 will remove all files."
+
+- @@ageconvertors = {
++ AgeConvertors = {
+ :s => 1,
+- :m => 60
++ :m => 60,
++ :h => 60 * 60,
++ :d => 60 * 60 * 24,
++ :w => 60 * 60 * 24 * 7,
+ }
+
+- @@ageconvertors[:h] = @@ageconvertors[:m] * 60
+- @@ageconvertors[:d] = @@ageconvertors[:h] * 24
+- @@ageconvertors[:w] = @@ageconvertors[:d] * 7
+-
+ def convert(unit, multi)
+- if num = @@ageconvertors[unit]
++ if num = AgeConvertors[unit]
+ return num * multi
+ else
+ self.fail "Invalid age unit '#{unit}'"
+@@ -148,16 +147,8 @@ Puppet::Type.newtype(:tidy) do
+ Only the first character is significant, so the full word can also
+ be used."
+
+- @@sizeconvertors = {
+- :b => 0,
+- :k => 1,
+- :m => 2,
+- :g => 3,
+- :t => 4
+- }
+-
+ def convert(unit, multi)
+- if num = @@sizeconvertors[unit]
++ if num = { :b => 0, :k => 1, :m => 2, :g => 3, :t => 4 }[unit]
+ result = multi
+ num.times do result *= 1024 end
+ return result
+--
+1.8.1
+
diff --git a/puppet.spec b/puppet.spec
index dd00ff4..34e9a9e 100644
--- a/puppet.spec
+++ b/puppet.spec
@@ -32,6 +32,9 @@ Patch1: 0001-18781-Be-more-tolerant-of-old-clients-in-WEBrick-server.pat
# https://bugzilla.redhat.com/show_bug.cgi?id=809911
# https://github.com/puppetlabs/puppet/commit/eea1ef5
Patch2: 0001-15190-Avoid-deprecated-iconv-on-Ruby-1.9.patch
+# https://bugzilla.redhat.com/show_bug.cgi?id=809911
+# https://github.com/puppetlabs/puppet/commit/32cc8ff
+Patch3: 0001-Avoid-class-level-variables-as-they-are-not-helpful.patch
Group: System Environment/Base
@@ -94,6 +97,7 @@ The server can also function as a certificate authority and file server.
%patch0 -p1
%patch1 -p1
%patch2 -p1
+%patch3 -p1
patch -s -p1 < conf/redhat/rundir-perms.patch
# Fix some rpmlint complaints
@@ -302,6 +306,7 @@ rm -rf %{buildroot}
- Apply upstream fix for fqdn_rand function with ruby-1.9 (#880959)
- Apply upstream fix to make WEBrick more tolerant of old clients (#831303)
- Apply upstream fix to avoid deprecated iconv (#809911)
+- Apply upstream fix to avoid class level variables (#809911)
* Wed Mar 13 2013 Michael Stahnke <stahnma at puppetlabs.com> - 2.7.21-1
- Fixes for CVE-2013-1640 CVE-2013-1652 CVE-2013-1653 CVE-2013-1654
More information about the scm-commits
mailing list