From 16770faa2978e76a529cf17b8b32a3ee5f45c9f0 Mon Sep 17 00:00:00 2001 From: Ashley Penney Date: Sat, 17 Aug 2013 20:51:18 -0400 Subject: [PATCH] Rename and refactor database_user to mysql_user. This work adds max_connections_per_hour, max_queries_per_hour, and max_updates_per_hour support to the provider and extends self.instances to add in the new parameters when checking existing users. It also adds self.prefetch in order to speed up Puppet runs. Provider is also switched to using mk_resource_methods to generate all the resource readers, and exists? and other methods now use the property_hash where appropriate. Tests rewritten to handle changes and extend code coverage. --- lib/puppet/provider/database_user/mysql.rb | 76 ---------- lib/puppet/provider/mysql_user/mysql.rb | 131 +++++++++++++++++ lib/puppet/type/database_user.rb | 27 +--- lib/puppet/type/mysql_user.rb | 45 ++++++ .../puppet/provider/mysql_user/mysql_spec.rb | 137 ++++++++++++++++++ spec/unit/puppet/type/mysql_user_spec.rb | 30 ++++ 6 files changed, 347 insertions(+), 99 deletions(-) delete mode 100644 lib/puppet/provider/database_user/mysql.rb create mode 100644 lib/puppet/provider/mysql_user/mysql.rb create mode 100644 lib/puppet/type/mysql_user.rb create mode 100644 spec/unit/puppet/provider/mysql_user/mysql_spec.rb create mode 100644 spec/unit/puppet/type/mysql_user_spec.rb diff --git a/lib/puppet/provider/database_user/mysql.rb b/lib/puppet/provider/database_user/mysql.rb deleted file mode 100644 index a2cc6f0..0000000 --- a/lib/puppet/provider/database_user/mysql.rb +++ /dev/null @@ -1,76 +0,0 @@ -Puppet::Type.type(:database_user).provide(:mysql) do - - desc 'manage users for a mysql database.' - - defaultfor :kernel => 'Linux' - - commands :mysql => 'mysql' - commands :mysqladmin => 'mysqladmin' - - def self.instances - users = mysql([defaults_file, 'mysql', '-BNe' "select concat(User, '@',Host) as User from mysql.user"].compact).split("\n") - users.select{ |user| user =~ /.+@/ }.collect do |name| - new(:name => name) - end - end - - def create - merged_name = @resource[:name].sub('@', "'@'") - password_hash = @resource.value(:password_hash) - max_user_connections = @resource.value(:max_user_connections) || 0 - - mysql([defaults_file, 'mysql', '-e', "grant usage on *.* to '#{merged_name}' identified by PASSWORD - '#{password_hash}' with max_user_connections #{max_user_connections}"].compact) - - exists? ? (return true) : (return false) - end - - def destroy - merged_name = @resource[:name].sub('@', "'@'") - mysql([defaults_file, 'mysql', '-e', "drop user '#{merged_name}'"].compact) - - exists? ? (return false) : (return true) - end - - def password_hash - mysql([defaults_file, 'mysql', '-NBe', "select password from mysql.user where CONCAT(user, '@', host) = '#{@resource[:name]}'"].compact).chomp - end - - def password_hash=(string) - mysql([defaults_file, 'mysql', '-e', "SET PASSWORD FOR '%s' = '%s'" % [ @resource[:name].sub('@', "'@'"), string ] ].compact) - - password_hash == string ? (return true) : (return false) - end - - def max_user_connections - mysql([defaults_file, "mysql", "-NBe", "select max_user_connections from mysql.user where CONCAT(user, '@', host) = '#{@resource[:name]}'"].compact).chomp - end - - def max_user_connections=(int) - mysql([defaults_file, "mysql", "-e", "grant usage on *.* to '%s' with max_user_connections #{int}" % [ @resource[:name].sub("@", "'@'")] ].compact).chomp - - max_user_connections == int ? (return true) : (return false) - end - - def exists? - not mysql([defaults_file, 'mysql', '-NBe', "select '1' from mysql.user where CONCAT(user, '@', host) = '%s'" % @resource.value(:name)].compact).empty? - end - - def flush - @property_hash.clear - mysqladmin([defaults_file, 'flush-privileges'].compact) - end - - # Optional defaults file - def self.defaults_file - if File.file?("#{Facter.value(:root_home)}/.my.cnf") - "--defaults-file=#{Facter.value(:root_home)}/.my.cnf" - else - nil - end - end - def defaults_file - self.class.defaults_file - end - -end diff --git a/lib/puppet/provider/mysql_user/mysql.rb b/lib/puppet/provider/mysql_user/mysql.rb new file mode 100644 index 0000000..043ecd1 --- /dev/null +++ b/lib/puppet/provider/mysql_user/mysql.rb @@ -0,0 +1,131 @@ +Puppet::Type.type(:mysql_user).provide(:mysql) do + + desc 'manage users for a mysql database.' + commands :mysql => 'mysql' + + # Optional defaults file + def self.defaults_file + if File.file?("#{Facter.value(:root_home)}/.my.cnf") + "--defaults-file=#{Facter.value(:root_home)}/.my.cnf" + else + nil + end + end + def defaults_file + self.class.defaults_file + end + + # Build a property_hash containing all the discovered information about MySQL + # users. + def self.instances + users = mysql([defaults_file, '-NBe', + "SELECT CONCAT(User, '@',Host) AS User FROM mysql.user"].compact).split("\n") + # To reduce the number of calls to MySQL we collect all the properties in + # one big swoop. + users.select{ |user| user =~ /.+@/ }.collect do |name| + query = "SELECT MAX_USER_CONNECTIONS, MAX_CONNECTIONS, MAX_QUESTIONS, MAX_UPDATES, PASSWORD FROM mysql.user WHERE CONCAT(user, '@', host) = '#{name}'" + @max_user_connections, @max_connections_per_hour, @max_queries_per_hour, + @max_updates_per_hour, @password = mysql([defaults_file, "-NBe", query].compact).split(/\s/) + + new(:name => name, + :ensure => :present, + :password_hash => @password, + :max_user_connections => @max_user_connections, + :max_connections_per_hour => @max_connections_per_hour, + :max_queries_per_hour => @max_queries_per_hour, + :max_updates_per_hour => @max_updates_per_hour + ) + end + end + + # We iterate over each mysql_user entry in the catalog and compare it against + # the contents of the property_hash generated by self.instances + def self.prefetch(resources) + users = instances + resources.keys.each do |name| + if provider = users.find { |user| user.name == name } + resources[name].provider = provider + end + end + end + + def create + merged_name = @resource[:name].sub('@', "'@'") + password_hash = @resource.value(:password_hash) + max_user_connections = @resource.value(:max_user_connections) || 0 + max_connections_per_hour = @resource.value(:max_connections_per_hour) || 0 + max_queries_per_hour = @resource.value(:max_queries_per_hour) || 0 + max_updates_per_hour = @resource.value(:max_updates_per_hour) || 0 + + mysql([defaults_file, '-e', "GRANT USAGE ON *.* TO '#{merged_name}' IDENTIFIED BY PASSWORD '#{password_hash}' WITH MAX_USER_CONNECTIONS #{max_user_connections} MAX_CONNECTIONS_PER_HOUR #{max_connections_per_hour} MAX_QUERIES_PER_HOUR #{max_queries_per_hour} MAX_UPDATES_PER_HOUR #{max_updates_per_hour}"].compact) + + @property_hash[:ensure] = :present + @property_hash[:password_hash] = password_hash + @property_hash[:max_user_connections] = max_user_connections + @property_hash[:max_connections_per_hour] = max_connections_per_hour + @property_hash[:max_queries_per_hour] = max_queries_per_hour + @property_hash[:max_updates_per_hour] = max_updates_per_hour + + exists? ? (return true) : (return false) + end + + def destroy + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "DROP USER '#{merged_name}'"].compact) + + @property_hash.clear + exists? ? (return false) : (return true) + end + + def exists? + @property_hash[:ensure] == :present || false + end + + def flush + @property_hash.clear + mysql([defaults_file, '-NBe', 'FLUSH PRIVILEGES'].compact) + end + + ## + ## MySQL user properties + ## + + # Generates method for all properties of the property_hash + mk_resource_methods + + def password_hash=(string) + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "SET PASSWORD FOR '#{merged_name}' = '#{string}'"].compact) + + password_hash == string ? (return true) : (return false) + end + + def max_user_connections=(int) + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "GRANT USAGE ON *.* TO '#{merged_name}' WITH MAX_USER_CONNECTIONS #{int}"].compact).chomp + + max_user_connections == int ? (return true) : (return false) + end + + def max_connections_per_hour=(int) + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "GRANT USAGE ON *.* TO '#{merged_name}' WITH MAX_CONNECTIONS_PER_HOUR #{int}"].compact).chomp + + max_connections_per_hour == int ? (return true) : (return false) + end + + def max_queries_per_hour=(int) + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "GRANT USAGE ON *.* TO '#{merged_name}' WITH MAX_QUERIES_PER_HOUR #{int}"].compact).chomp + + max_queries_per_hour == int ? (return true) : (return false) + end + + def max_updates_per_hour=(int) + merged_name = @resource[:name].sub('@', "'@'") + mysql([defaults_file, '-e', "GRANT USAGE ON *.* TO '#{merged_name}' WITH MAX_UPDATES_PER_HOUR #{int}"].compact).chomp + + max_updates_per_hour == int ? (return true) : (return false) + end + +end diff --git a/lib/puppet/type/database_user.rb b/lib/puppet/type/database_user.rb index 1dfe8e0..cf830d4 100644 --- a/lib/puppet/type/database_user.rb +++ b/lib/puppet/type/database_user.rb @@ -1,30 +1,11 @@ -# This has to be a separate type to enable collecting Puppet::Type.newtype(:database_user) do - @doc = 'Manage a database user. This includes management of users password as well as privileges' - ensurable - - newparam(:name, :namevar=>true) do - desc "The name of the user. This uses the 'username@hostname' or username@hostname." + newparam(:name, :namevar => true) do + desc 'Manage database users.' validate do |value| - # https://dev.mysql.com/doc/refman/5.1/en/account-names.html - # Regex should problably be more like this: /^[`'"]?[^`'"]*[`'"]?@[`'"]?[\w%\.]+[`'"]?$/ - raise(ArgumentError, "Invalid database user #{value}") unless value =~ /[\w-]*@[\w%\.:]+/ - username = value.split('@')[0] - if username.size > 16 - raise ArgumentError, 'MySQL usernames are limited to a maximum of 16 characters' - end + Puppet.warning("database_user has been deprecated in favor of mysql_user.") + true end end - newproperty(:password_hash) do - desc 'The password hash of the user. Use mysql_password() for creating such a hash.' - newvalue(/\w+/) - end - - newproperty(:max_user_connections) do - desc "Max concurrent connections for the user. 0 means no (or global) limit." - newvalue(/\d+/) - end - end diff --git a/lib/puppet/type/mysql_user.rb b/lib/puppet/type/mysql_user.rb new file mode 100644 index 0000000..38564a9 --- /dev/null +++ b/lib/puppet/type/mysql_user.rb @@ -0,0 +1,45 @@ +# This has to be a separate type to enable collecting +Puppet::Type.newtype(:mysql_user) do + @doc = 'Manage a MySQL user. This includes management of users password as well as privileges.' + + ensurable + + newparam(:name, :namevar => true) do + desc "The name of the user. This uses the 'username@hostname' or username@hostname." + validate do |value| + # https://dev.mysql.com/doc/refman/5.1/en/account-names.html + # Regex should problably be more like this: /^[`'"]?[^`'"]*[`'"]?@[`'"]?[\w%\.]+[`'"]?$/ + raise(ArgumentError, "Invalid database user #{value}") unless value =~ /[\w-]*@[\w%\.:]+/ + username = value.split('@')[0] + if username.size > 16 + raise ArgumentError, 'MySQL usernames are limited to a maximum of 16 characters' + end + end + end + + newproperty(:password_hash) do + desc 'The password hash of the user. Use mysql_password() for creating such a hash.' + newvalue(/\w+/) + end + + newproperty(:max_user_connections) do + desc "Max concurrent connections for the user. 0 means no (or global) limit." + newvalue(/\d+/) + end + + newproperty(:max_connections_per_hour) do + desc "Max connections per hour for the user. 0 means no (or global) limit." + newvalue(/\d+/) + end + + newproperty(:max_queries_per_hour) do + desc "Max queries per hour for the user. 0 means no (or global) limit." + newvalue(/\d+/) + end + + newproperty(:max_updates_per_hour) do + desc "Max updates per hour for the user. 0 means no (or global) limit." + newvalue(/\d+/) + end + +end diff --git a/spec/unit/puppet/provider/mysql_user/mysql_spec.rb b/spec/unit/puppet/provider/mysql_user/mysql_spec.rb new file mode 100644 index 0000000..360c924 --- /dev/null +++ b/spec/unit/puppet/provider/mysql_user/mysql_spec.rb @@ -0,0 +1,137 @@ +require 'spec_helper' + +describe Puppet::Type.type(:mysql_user).provider(:mysql) do + let(:defaults_file) { '--defaults-file=/root/.my.cnf' } + let(:newhash) { '*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF5' } + + let(:raw_users) do + <<-SQL_OUTPUT +root@127.0.0.1 +root@::1 +@localhost +debian-sys-maint@localhost +root@localhost +usvn_user@localhost +@vagrant-ubuntu-raring-64 + SQL_OUTPUT + end + + let(:parsed_users) { %w(root@127.0.0.1 root@::1 debian-sys-maint@localhost root@localhost usvn_user@localhost) } + + let(:resource) { Puppet::Type.type(:mysql_user).new( + { :ensure => :present, + :password_hash => '*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF4', + :name => 'joe@localhost', + :max_user_connections => '10', + :max_connections_per_hour => '10', + :max_queries_per_hour => '10', + :max_updates_per_hour => '10', + :provider => described_class.name + } + )} + let(:provider) { resource.provider } + + before :each do + # Set up the stubs for an instances call. + Facter.stubs(:value).with(:root_home).returns('/root') + Puppet::Util.stubs(:which).with('mysql').returns('/usr/bin/mysql') + File.stubs(:file?).with('/root/.my.cnf').returns(true) + provider.class.stubs(:mysql).with([defaults_file, '-NBe', "SELECT CONCAT(User, '@',Host) AS User FROM mysql.user"]).returns('joe@localhost') + provider.class.stubs(:mysql).with([defaults_file, '-NBe', "SELECT MAX_USER_CONNECTIONS, MAX_CONNECTIONS, MAX_QUESTIONS, MAX_UPDATES, PASSWORD FROM mysql.user WHERE CONCAT(user, '@', host) = 'joe@localhost'"]).returns('10 10 10 10 *6C8989366EAF75BB670AD8EA7A7FC1176A95CEF4') + end + + let(:instance) { provider.class.instances.first } + + describe 'self.instances' do + it 'returns an array of users' do + provider.class.stubs(:mysql).with([defaults_file, '-NBe', "SELECT CONCAT(User, '@',Host) AS User FROM mysql.user"]).returns(raw_users) + parsed_users.each do |user| + provider.class.stubs(:mysql).with([defaults_file, '-NBe', "SELECT MAX_USER_CONNECTIONS, MAX_CONNECTIONS, MAX_QUESTIONS, MAX_UPDATES, PASSWORD FROM mysql.user WHERE CONCAT(user, '@', host) = '#{user}'"]).returns('10 10 10 10 ') + end + + usernames = provider.class.instances.collect {|x| x.name } + parsed_users.should match_array(usernames) + end + end + + describe 'self.prefetch' do + it 'exists' do + provider.class.instances + provider.class.prefetch({}) + end + end + + describe 'create' do + it 'makes a user' do + provider.expects(:mysql).with([defaults_file, '-e', "GRANT USAGE ON *.* TO 'joe'@'localhost' IDENTIFIED BY PASSWORD '*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF4' WITH MAX_USER_CONNECTIONS 10 MAX_CONNECTIONS_PER_HOUR 10 MAX_QUERIES_PER_HOUR 10 MAX_UPDATES_PER_HOUR 10"]) + provider.expects(:exists?).returns(true) + provider.create.should be_true + end + end + + describe 'destroy' do + it 'removes a user if present' do + provider.expects(:mysql).with([defaults_file, '-e', "DROP USER 'joe'@'localhost'"]) + provider.expects(:exists?).returns(false) + provider.destroy.should be_true + end + end + + describe 'exists?' do + it 'checks if user exists' do + instance.exists?.should be_true + end + end + + describe 'flush' do + it 'removes cached privileges' do + provider.expects(:mysql).with([defaults_file, '-NBe', 'FLUSH PRIVILEGES']) + provider.flush + end + end + + describe 'self.defaults_file' do + it 'sets --defaults-file' do + File.stubs(:file?).with('/root/.my.cnf').returns(true) + provider.defaults_file.should eq '--defaults-file=/root/.my.cnf' + end + it 'fails if file missing' do + File.expects(:file?).with('/root/.my.cnf').returns(false) + provider.defaults_file.should be_nil + end + end + + describe 'password_hash' do + it 'returns a hash' do + instance.password_hash.should == '*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF4' + end + end + + describe 'password_hash=' do + it 'changes the hash' do + provider.expects(:mysql).with([defaults_file, '-e', "SET PASSWORD FOR 'joe'@'localhost' = '*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF5'"]).returns('0') + + provider.expects(:password_hash).returns('*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF5') + provider.password_hash=('*6C8989366EAF75BB670AD8EA7A7FC1176A95CEF5') + end + end + + ['max_user_connections', 'max_connections_per_hour', 'max_queries_per_hour', + 'max_updates_per_hour'].each do |property| + + describe property do + it "returns #{property}" do + instance.send("#{property}".to_sym).should == '10' + end + end + + describe "#{property}=" do + it "changes #{property}" do + provider.expects(:mysql).with([defaults_file, '-e', "GRANT USAGE ON *.* TO 'joe'@'localhost' WITH #{property.upcase} 42"]).returns('0') + provider.expects(property.to_sym).returns('42') + provider.send("#{property}=".to_sym, '42') + end + end + end + +end diff --git a/spec/unit/puppet/type/mysql_user_spec.rb b/spec/unit/puppet/type/mysql_user_spec.rb new file mode 100644 index 0000000..62aad7a --- /dev/null +++ b/spec/unit/puppet/type/mysql_user_spec.rb @@ -0,0 +1,30 @@ +require 'puppet' +require 'puppet/type/mysql_user' +describe Puppet::Type.type(:mysql_user) do + + before :each do + @user = Puppet::Type.type(:mysql_user).new(:name => 'foo@localhost', :password_hash => 'pass') + end + + it 'should accept a user name' do + @user[:name].should == 'foo@localhost' + end + + it 'should fail with a long user name' do + expect { + Puppet::Type.type(:mysql_user).new({:name => '12345678901234567@localhost', :password_hash => 'pass'}) + }.to raise_error /MySQL usernames are limited to a maximum of 16 characters/ + end + + it 'should accept a password' do + @user[:password_hash] = 'foo' + @user[:password_hash].should == 'foo' + end + + it 'should require a name' do + expect { + Puppet::Type.type(:mysql_user).new({}) + }.to raise_error(Puppet::Error, 'Title or name must be provided') + end + +end