Please review pull request #628: Added unless_uid option to User Resource management. opened by (barttenbrinke)
Description:
Currently there are only two options for user management: purge => true or purge => false.
Purge true works great if all your apps are deployed via puppet, but when you have a mix of puppet system uids and application uids, you are forced to turn purge off.
This has the undesired side-effect that users that are managed through puppet will never revoked for the servers that have purge disabled.
At our company Devops deploys the servers & frameworks using Puppet so that Devs can take over and deploy applications on their own.
Each of these application runs under its own uid. For this reason we have to run with purge => false, which is something we don't want.
The only option available to alter this behavior is "unless_system_user => true", but this just protects users with a UID < 500 and that is not where I want to have my application UIDs.
In order to fix this properly, I've added an extra option to puppet: unless_uid.
You van specify specific Uids here or even Ranges of UIDS. Uids that match these ranges will not be purged, even though purge => true.
This way Puppet still automatically revokes access to the servers for Devs and Devops that are removed, while allowing Devs to deploy their own application application.
Example:
class users::resources {
resources { 'user':
purge => true,
unless_system_user => true,
unless_uid => [10_000..20_000];
}
}
Unless_uid accepts Integers, Ranges or Arrays with both.
I also added specs for both the old and the new check_user behavior, as the old behaviour was not specced.
- Opened: Thu Apr 05 13:11:33 UTC 2012
- Based on: puppetlabs:master (ba112e94619e6d35ca1d2d89975f787e537fe1e3)
- Requested merge: barttenbrinke:master (d6c051531d76609817db6481bbb77ff72730bf76)
Diff follows:
diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb
index ae41b88..91d6b92 100644
--- a/lib/puppet/type/resources.rb
+++ b/lib/puppet/type/resources.rb
@@ -64,6 +64,28 @@
}
end
+ newparam(:unless_uid) do
+ desc "This keeps specific uids or ranges of uids from being purged when purge is true.
+ Accepts ranges, integers and (mixed) arrays of both."
+
+ munge do |value|
+ case value
+ when /^\d+/
+ [Integer(value)]
+ when Integer
+ [value]
+ when Range
+ [value]
+ when Array
+ value
+ when /^\[\d+/
+ values.split(',').collect{|x| x.include?('..') ? Integer(x.split('..')[0])..Integer(x.split('..')[1]) : Integer(x) }
+ else
+ raise ArgumentError, "Invalid value #{value.inspect}"
+ end
+ end
+ end
+
def check(resource)
@checkmethod ||= "#{self[:name]}_check"
@hascheck ||= respond_to?(@checkmethod)
@@ -111,18 +133,25 @@ def resource_type
@resource_type
end
- # Make sure we don't purge users below a certain uid, if the check
- # is enabled.
+ # Make sure we don't purge users with specific uids
def user_check(resource)
return true unless self[:name] == "user"
return true unless self[:unless_system_user]
-
resource[:audit] = :uid
current_values = resource.retrieve_resource
+ current_uid = current_values[resource.property(:uid)]
+ unless_uids = self[:unless_uid]
return false if system_users.include?(resource[:name])
- current_values[resource.property(:uid)] > self[:unless_system_user]
+ if unless_uids && unless_uids.length > 0
+ unless_uids.each do |unless_uid|
+ return false if unless_uid == current_uid
+ return false if unless_uid.respond_to?('include?') && unless_uid.include?(current_uid)
+ end
+ end
+
+ current_uid > self[:unless_system_user]
end
def system_users
diff --git a/spec/unit/type/resources_spec.rb b/spec/unit/type/resources_spec.rb
index ca2ffdd..05f78fa 100755
--- a/spec/unit/type/resources_spec.rb
+++ b/spec/unit/type/resources_spec.rb
@@ -21,6 +21,133 @@
end
end
+ describe "#check_user purge behaviour" do
+ describe "with unless_system_user => true" do
+ before do
+ @res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_system_user => true
+ @res.catalog = Puppet::Resource::Catalog.new
+ end
+
+ it "should never purge hardcoded system users" do
+ %w{root nobody bin noaccess daemon sys}.each do |sys_user|
+ @res.user_check(Puppet::Type.type(:user).new(:name => sys_user)).should be_false
+ end
+ end
+
+ it "should not purge system users if unless_system_user => true" do
+ user_hash = {:name => 'system_user', :uid => 125, :system => true}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_false
+ end
+
+ it "should purge manual users if unless_system_user => true" do
+ user_hash = {:name => 'system_user', :uid => 525, :system => true}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_true
+ end
+
+ it "should purge system users over 500 if unless_system_user => 600" do
+ res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_system_user => 600
+ res.catalog = Puppet::Resource::Catalog.new
+ user_hash = {:name => 'system_user', :uid => 525, :system => true}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ res.user_check(user).should be_false
+ end
+ end
+
+ describe "with unless_uid" do
+ describe "with a uid range" do
+ before do
+ @res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_uid => 10_000..20_000
+ @res.catalog = Puppet::Resource::Catalog.new
+ end
+
+ it "should purge uids that are not in a specified range" do
+ user_hash = {:name => 'special_user', :uid => 25_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_true
+ end
+
+ it "should not purge uids that are in a specified range" do
+ user_hash = {:name => 'special_user', :uid => 15_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_false
+ end
+ end
+
+ describe "with a uid range array" do
+ before do
+ @res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_uid => [10_000..15_000, 15_000..20_000]
+ @res.catalog = Puppet::Resource::Catalog.new
+ end
+
+ it "should purge uids that are not in a specified range array" do
+ user_hash = {:name => 'special_user', :uid => 25_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_true
+ end
+
+ it "should not purge uids that are in a specified range array" do
+ user_hash = {:name => 'special_user', :uid => 15_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_false
+ end
+
+ end
+
+ describe "with a uid array" do
+ before do
+ @res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_uid => [15_000, 15_001, 15_002]
+ @res.catalog = Puppet::Resource::Catalog.new
+ end
+
+ it "should purge uids that are not in a specified array" do
+ user_hash = {:name => 'special_user', :uid => 25_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_true
+ end
+
+ it "should not purge uids that are in a specified array" do
+ user_hash = {:name => 'special_user', :uid => 15000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_false
+ end
+
+ end
+
+ describe "with a single uid" do
+ before do
+ @res = Puppet::Type.type(:resources).new :name => :user, :purge => true, :unless_uid => 15_000
+ @res.catalog = Puppet::Resource::Catalog.new
+ end
+
+ it "should purge uids that are not specified" do
+ user_hash = {:name => 'special_user', :uid => 25_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_true
+ end
+
+ it "should not purge uids that are specified" do
+ user_hash = {:name => 'special_user', :uid => 15_000}
+ user = Puppet::Type.type(:user).new(user_hash)
+ user.stubs(:retrieve_resource).returns Puppet::Resource.new("user", user_hash[:name], :parameters => user_hash)
+ @res.user_check(user).should be_false
+ end
+
+ end
+ end
+ end
+
describe "#generate" do
before do
@host1 = Puppet::Type.type(:host).new(:name => 'localhost', :ip => '127.0.0.1')
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
