Merge pull request #219 from apenney/fixes

Fixes suggested by RubyMine (just playing around with it)
This commit is contained in:
Ashley Penney 2013-07-11 16:43:46 -07:00
commit 3205b83ac8
19 changed files with 75 additions and 76 deletions

View file

@ -7,7 +7,7 @@ module Puppet::Parser::Functions
EOS
) do |args|
raise(Puppet::ParseError, "mysql_password(): Wrong number of arguments " +
raise(Puppet::ParseError, 'mysql_password(): Wrong number of arguments ' +
"given (#{args.size} for 1)") if args.size != 1
'*' + Digest::SHA1.hexdigest(Digest::SHA1.digest(args[0])).upcase

View file

@ -1,5 +1,5 @@
Puppet::Type.type(:database).provide(:mysql) do
desc "Manages MySQL database."
desc 'Manages MySQL database.'
defaultfor :kernel => 'Linux'
@ -19,7 +19,7 @@ Puppet::Type.type(:database).provide(:mysql) do
end
def self.instances
mysql([defaults_file, '-NBe', "show databases"].compact).split("\n").collect do |name|
mysql([defaults_file, '-NBe', 'show databases'].compact).split("\n").collect do |name|
new(:name => name)
end
end
@ -42,7 +42,7 @@ Puppet::Type.type(:database).provide(:mysql) do
def exists?
begin
mysql([defaults_file, '-NBe', "show databases"].compact).match(/^#{@resource[:name]}$/)
mysql([defaults_file, '-NBe', 'show databases'].compact).match(/^#{@resource[:name]}$/)
rescue => e
debug(e.message)
return nil

View file

@ -5,7 +5,7 @@
Puppet::Type.type(:database_grant).provide(:mysql) do
desc "Uses mysql as database."
desc 'Uses mysql as database.'
defaultfor :kernel => 'Linux'
@ -34,19 +34,19 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
end
def self.query_user_privs
results = mysql([defaults_file, "mysql", "-Be", "describe user"].compact)
results = mysql([defaults_file, 'mysql', '-Be', 'describe user'].compact)
column_names = results.split(/\n/).map { |l| l.chomp.split(/\t/)[0] }
@user_privs = column_names.delete_if { |e| !( e =~/_priv$/) }
end
def self.query_db_privs
results = mysql([defaults_file, "mysql", "-Be", "describe db"].compact)
results = mysql([defaults_file, 'mysql', '-Be', 'describe db'].compact)
column_names = results.split(/\n/).map { |l| l.chomp.split(/\t/)[0] }
@db_privs = column_names.delete_if { |e| !(e =~/_priv$/) }
end
def mysql_flush
mysqladmin([defaults_file, "flush-privileges"].compact)
mysqladmin([defaults_file, 'flush-privileges'].compact)
end
# this parses the
@ -74,11 +74,11 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
name = split_name(@resource[:name])
case name[:type]
when :user
mysql([defaults_file, "mysql", "-e", "INSERT INTO user (host, user) VALUES ('%s', '%s')" % [
mysql([defaults_file, 'mysql', '-e', "INSERT INTO user (host, user) VALUES ('%s', '%s')" % [
name[:host], name[:user],
]].compact)
when :db
mysql([defaults_file, "mysql", "-e", "INSERT INTO db (host, user, db) VALUES ('%s', '%s', '%s')" % [
mysql([defaults_file, 'mysql', '-e', "INSERT INTO db (host, user, db) VALUES ('%s', '%s', '%s')" % [
name[:host], name[:user], name[:db],
]].compact)
end
@ -87,7 +87,7 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
end
def destroy
mysql([defaults_file, "mysql", "-e", "REVOKE ALL ON '%s'.* FROM '%s@%s'" % [ @resource[:privileges], @resource[:database], @resource[:name], @resource[:host] ]].compact)
mysql([defaults_file, 'mysql', '-e', "REVOKE ALL ON '%s'.* FROM '%s@%s'" % [ @resource[:privileges], @resource[:database], @resource[:name], @resource[:host] ]].compact)
end
def row_exists?
@ -96,7 +96,7 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
if name[:type] == :db
fields << :db
end
not mysql([defaults_file, "mysql", '-NBe', "SELECT '1' FROM %s WHERE %s" % [ name[:type], fields.map do |f| "%s='%s'" % [f, name[f]] end.join(' AND ')]].compact).empty?
not mysql([defaults_file, 'mysql', '-NBe', "SELECT '1' FROM %s WHERE %s" % [ name[:type], fields.map do |f| "%s='%s'" % [f, name[f]] end.join(' AND ')]].compact).empty?
end
def all_privs_set?
@ -106,21 +106,21 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
when :db
db_privs
end
all_privs = all_privs.collect do |p| p.downcase end.sort.join("|")
privs = privileges.collect do |p| p.downcase end.sort.join("|")
all_privs = all_privs.collect do |p| p.downcase end.sort.join('|')
privs = privileges.collect do |p| p.downcase end.sort.join('|')
all_privs == privs
end
def privileges
name = split_name(@resource[:name])
privs = ""
privs = ''
case name[:type]
when :user
privs = mysql([defaults_file, "mysql", "-Be", "select * from mysql.user where user='%s' and host='%s'" % [ name[:user], name[:host] ]].compact)
privs = mysql([defaults_file, 'mysql', '-Be', "select * from mysql.user where user='%s' and host='%s'" % [ name[:user], name[:host] ]].compact)
when :db
privs = mysql([defaults_file, "mysql", "-Be", "select * from mysql.db where user='%s' and host='%s' and db='%s'" % [ name[:user], name[:host], name[:db] ]].compact)
privs = mysql([defaults_file, 'mysql', '-Be', "select * from mysql.db where user='%s' and host='%s' and db='%s'" % [ name[:user], name[:host], name[:db] ]].compact)
end
if privs.match(/^$/)
@ -172,7 +172,7 @@ Puppet::Type.type(:database_grant).provide(:mysql) do
stmt = stmt << set << where
validate_privs privs, all_privs
mysql([defaults_file, "mysql", "-Be", stmt].compact)
mysql([defaults_file, 'mysql', '-Be', stmt].compact)
mysql_flush
end

View file

@ -1,6 +1,6 @@
Puppet::Type.type(:database_user).provide(:mysql) do
desc "manage users for a mysql database."
desc 'manage users for a mysql database.'
defaultfor :kernel => 'Linux'
@ -8,36 +8,36 @@ Puppet::Type.type(:database_user).provide(:mysql) do
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 = 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("@", "'@'")
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
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)
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
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)
mysql([defaults_file, 'mysql', '-e', "SET PASSWORD FOR '%s' = '%s'" % [ @resource[:name].sub('@', "'@'"), string ] ].compact)
password_hash == string ? (return true) : (return false)
end
@ -53,12 +53,12 @@ Puppet::Type.type(:database_user).provide(:mysql) do
end
def exists?
not mysql([defaults_file, "mysql", "-NBe", "select '1' from mysql.user where CONCAT(user, '@', host) = '%s'" % @resource.value(:name)].compact).empty?
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)
mysqladmin([defaults_file, 'flush-privileges'].compact)
end
# Optional defaults file

View file

@ -1,15 +1,15 @@
# This has to be a separate type to enable collecting
Puppet::Type.newtype(:database) do
@doc = "Manage databases."
@doc = 'Manage databases.'
ensurable
newparam(:name, :namevar=>true) do
desc "The name of the database."
desc 'The name of the database.'
end
newproperty(:charset) do
desc "The characterset to use for a database"
desc 'The characterset to use for a database'
defaultto :utf8
newvalue(/^\S+$/)
end

View file

@ -19,25 +19,25 @@ Puppet::Type.newtype(:database_grant) do
reqs = []
matches = self[:name].match(/^([^@]+)@([^\/]+).*$/)
unless matches.nil?
reqs << "%s@%s" % [ matches[1], matches[2] ]
reqs << '%s@%s' % [ matches[1], matches[2] ]
end
# puts "Autoreq: '%s'" % reqs.join(" ")
reqs
end
newparam(:name, :namevar=>true) do
desc "The primary key: either user@host for global privilges or user@host/database for database specific privileges"
desc 'The primary key: either user@host for global privilges or user@host/database for database specific privileges'
end
newproperty(:privileges, :array_matching => :all) do
desc "The privileges the user should have. The possible values are implementation dependent."
desc 'The privileges the user should have. The possible values are implementation dependent.'
def should_to_s(newvalue = @should)
if newvalue
unless newvalue.is_a?(Array)
newvalue = [ newvalue ]
end
newvalue.collect do |v| v.downcase end.sort.join ", "
newvalue.collect do |v| v.downcase end.sort.join ', '
else
nil
end
@ -48,7 +48,7 @@ Puppet::Type.newtype(:database_grant) do
unless currentvalue.is_a?(Array)
currentvalue = [ currentvalue ]
end
currentvalue.collect do |v| v.downcase end.sort.join ", "
currentvalue.collect do |v| v.downcase end.sort.join ', '
else
nil
end
@ -58,7 +58,7 @@ Puppet::Type.newtype(:database_grant) do
def insync?(is)
if defined? @should and @should
case self.should_to_s
when "all"
when 'all'
self.provider.all_privs_set?
when self.is_to_s(is)
true

View file

@ -1,6 +1,6 @@
# 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"
@doc = 'Manage a database user. This includes management of users password as well as privileges'
ensurable
@ -12,13 +12,13 @@ Puppet::Type.newtype(:database_user) do
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"
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."
desc 'The password hash of the user. Use mysql_password() for creating such a hash.'
newvalue(/\w+/)
end

View file

@ -33,7 +33,7 @@ class mysql::params {
$query_cache_limit = '1M'
$query_cache_size = '16M'
$expire_logs_days = 10
$max_binlog_size = 100M
$max_binlog_size = '100M'
case $::operatingsystem {
'Ubuntu': {

View file

@ -10,13 +10,13 @@ describe 'mysql::backup' do
'delete_before_dump' => true,
}
}
context "standard conditions" do
context 'standard conditions' do
let(:params) { default_params }
it { should contain_database_user('testuser@localhost')}
it { should contain_database_grant('testuser@localhost').with(
:privileges => [ 'Select_priv', 'Reload_priv', 'Lock_tables_priv', 'Show_view_priv' ]
:privileges => %w(Select_priv Reload_priv Lock_tables_priv Show_view_priv)
)}
it { should contain_cron('mysql-backup').with(
@ -46,7 +46,7 @@ describe 'mysql::backup' do
end
end
context "with compression disabled" do
context 'with compression disabled' do
let(:params) do
{ :backupcompress => false }.merge(default_params)
end

View file

@ -135,7 +135,6 @@ describe 'mysql::config' do
{
:service_name => 'dans_service',
:config_file => '/home/dan/mysql.conf',
:service_name => 'dans_mysql',
:pidfile => '/home/dan/mysql.pid',
:socket => '/home/dan/mysql.sock',
:bind_address => '0.0.0.0',

View file

@ -14,5 +14,5 @@ describe 'mysql::server::monitor' do
}
end
it { should contain_database_user("monitoruser@monitorhost")}
it { should contain_database_user('monitoruser@monitorhost')}
end

View file

@ -21,7 +21,7 @@ describe 'mysql class' do
it { should be_installed }
end
describe service('mysqld') do
describe service('mysql') do
it { should_not be_running }
it { should_not be_enabled }
end

View file

@ -1,7 +1,7 @@
#!/usr/bin/env rspec
require 'spec_helper'
describe "the mysql_password function" do
describe 'the mysql_password function' do
before :all do
Puppet::Parser::Functions.autoloader.loadall
end
@ -14,20 +14,20 @@ describe "the mysql_password function" do
end
end
it "should exist" do
Puppet::Parser::Functions.function("mysql_password").should == "function_mysql_password"
it 'should exist' do
Puppet::Parser::Functions.function('mysql_password').should == 'function_mysql_password'
end
it "should raise a ParseError if there is less than 1 arguments" do
it 'should raise a ParseError if there is less than 1 arguments' do
lambda { @scope.function_mysql_password([]) }.should( raise_error(Puppet::ParseError))
end
it "should raise a ParseError if there is more than 1 arguments" do
lambda { @scope.function_mysql_password(['foo', 'bar']) }.should( raise_error(Puppet::ParseError))
it 'should raise a ParseError if there is more than 1 arguments' do
lambda { @scope.function_mysql_password(%w(foo bar)) }.should( raise_error(Puppet::ParseError))
end
it "should convert password into a hash" do
result = @scope.function_mysql_password(["password"])
it 'should convert password into a hash' do
result = @scope.function_mysql_password(%w(password))
result.should(eq('*2470C0C06DEE42FD1618BB99005ADCA2EC9D1E19'))
end

View file

@ -18,7 +18,7 @@ test
SQL_OUTPUT
end
let(:parsed_databases) { ['information_schema', 'mydb', 'mysql', 'performance_schema', 'test'] }
let(:parsed_databases) { %w(information_schema mydb mysql performance_schema test) }
before :each do
@resource = Puppet::Type::Database.new(
@ -26,14 +26,14 @@ test
)
@provider = provider_class.new(@resource)
Facter.stubs(:value).with(:root_home).returns(root_home)
Puppet::Util.stubs(:which).with("mysql").returns("/usr/bin/mysql")
subject.stubs(:which).with("mysql").returns("/usr/bin/mysql")
Puppet::Util.stubs(:which).with('mysql').returns('/usr/bin/mysql')
subject.stubs(:which).with('mysql').returns('/usr/bin/mysql')
subject.stubs(:defaults_file).returns('--defaults-file=/root/.my.cnf')
end
describe 'self.instances' do
it 'returns an array of databases' do
subject.stubs(:mysql).with([defaults_file, "-NBe", "show databases"]).returns(raw_databases)
subject.stubs(:mysql).with([defaults_file, '-NBe', 'show databases']).returns(raw_databases)
databases = subject.instances.collect {|x| x.name }
parsed_databases.should match_array(databases)
@ -71,7 +71,7 @@ test
describe 'exists?' do
it 'checks if user exists' do
subject.expects(:mysql).with([defaults_file, '-NBe', "show databases"]).returns('information_schema\nmydb\nmysql\nperformance_schema\ntest')
subject.expects(:mysql).with([defaults_file, '-NBe', 'show databases']).returns('information_schema\nmydb\nmysql\nperformance_schema\ntest')
@provider.exists?
end
end

View file

@ -36,8 +36,8 @@ Select_priv enum('N','Y') NO N
Insert_priv enum('N','Y') NO N
Update_priv enum('N','Y') NO N
EOT
provider_class.user_privs.should == [ 'Select_priv', 'Insert_priv', 'Update_priv' ]
provider_class.db_privs.should == [ 'Select_priv', 'Insert_priv', 'Update_priv' ]
provider_class.user_privs.should == %w(Select_priv Insert_priv Update_priv)
provider_class.db_privs.should == %w(Select_priv Insert_priv Update_priv)
end
it 'should query set privileges' do
@ -45,7 +45,7 @@ EOT
Host User Password Select_priv Insert_priv Update_priv
host user Y N Y
EOT
@provider.privileges.should == [ 'Select_priv', 'Update_priv' ]
@provider.privileges.should == %w(Select_priv Update_priv)
end
it 'should recognize when all privileges are set' do
@ -67,29 +67,29 @@ EOT
it 'should be able to set all privileges' do
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-NBe', "SELECT '1' FROM user WHERE user='user' AND host='host'"]).returns "1\n"
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-Be', "update user set Select_priv = 'Y', Insert_priv = 'Y', Update_priv = 'Y' where user='user' and host='host'"])
provider_class.expects(:mysqladmin).with(["--defaults-file=#{root_home}/.my.cnf", "flush-privileges"])
@provider.privileges=(['all'])
provider_class.expects(:mysqladmin).with(%W(--defaults-file=#{root_home}/.my.cnf flush-privileges))
@provider.privileges=(%w(all))
end
it 'should be able to set partial privileges' do
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-NBe', "SELECT '1' FROM user WHERE user='user' AND host='host'"]).returns "1\n"
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-Be', "update user set Select_priv = 'Y', Insert_priv = 'N', Update_priv = 'Y' where user='user' and host='host'"])
provider_class.expects(:mysqladmin).with(["--defaults-file=#{root_home}/.my.cnf", "flush-privileges"])
@provider.privileges=(['Select_priv', 'Update_priv'])
provider_class.expects(:mysqladmin).with(%W(--defaults-file=#{root_home}/.my.cnf flush-privileges))
@provider.privileges=(%w(Select_priv Update_priv))
end
it 'should be case insensitive' do
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-NBe', "SELECT '1' FROM user WHERE user='user' AND host='host'"]).returns "1\n"
provider_class.expects(:mysql).with(["--defaults-file=#{root_home}/.my.cnf", 'mysql', '-Be', "update user set Select_priv = 'Y', Insert_priv = 'Y', Update_priv = 'Y' where user='user' and host='host'"])
provider_class.expects(:mysqladmin).with(["--defaults-file=#{root_home}/.my.cnf", 'flush-privileges'])
@provider.privileges=(['SELECT_PRIV', 'insert_priv', 'UpDaTe_pRiV'])
@provider.privileges=(%w(SELECT_PRIV insert_priv UpDaTe_pRiV))
end
it 'should not pass --defaults-file if $root_home/.my.cnf is absent' do
File.stubs(:file?).with("#{root_home}/.my.cnf").returns(false)
provider_class.expects(:mysql).with(['mysql', '-NBe', "SELECT '1' FROM user WHERE user='user' AND host='host'"]).returns "1\n"
provider_class.expects(:mysql).with(['mysql', '-Be', "update user set Select_priv = 'Y', Insert_priv = 'N', Update_priv = 'Y' where user='user' and host='host'"])
provider_class.expects(:mysqladmin).with(["flush-privileges"])
@provider.privileges=(['Select_priv', 'Update_priv'])
provider_class.expects(:mysqladmin).with(%w(flush-privileges))
@provider.privileges=(%w(Select_priv Update_priv))
end
end

View file

@ -21,7 +21,7 @@ usvn_user@localhost
SQL_OUTPUT
end
let(:parsed_users) { ['root@127.0.0.1', 'root@::1', 'debian-sys-maint@localhost', 'root@localhost', 'usvn_user@localhost'] }
let(:parsed_users) { %w(root@127.0.0.1 root@::1 debian-sys-maint@localhost root@localhost usvn_user@localhost) }
before :each do
# password hash = mypass
@ -33,8 +33,8 @@ usvn_user@localhost
)
@provider = provider_class.new(@resource)
Facter.stubs(:value).with(:root_home).returns(root_home)
Puppet::Util.stubs(:which).with("mysql").returns("/usr/bin/mysql")
subject.stubs(:which).with("mysql").returns("/usr/bin/mysql")
Puppet::Util.stubs(:which).with('mysql').returns('/usr/bin/mysql')
subject.stubs(:which).with('mysql').returns('/usr/bin/mysql')
subject.stubs(:defaults_file).returns('--defaults-file=/root/.my.cnf')
end

View file

@ -92,7 +92,7 @@ default-storage-engine = <%= @default_engine %>
<% if @character_set != 'UNSET' -%>
character-set-server = <%= @character_set %>
<% end -%>
<% if @ssl == true %>
<% if @ssl %>
ssl-ca = <%= @ssl_ca %>
ssl-cert = <%= @ssl_cert %>
ssl-key = <%= @ssl_key %>

View file

@ -3,7 +3,7 @@
[<%= section %>]
<% content.sort.each do |key, values| -%>
<% [values].flatten.sort.each do |value| -%>
<%= value == false ? '#' : '' %><%= key -%><%=
<%= !value ? '#' : '' %><%= key -%><%=
case value
when true, false
''

View file

@ -34,7 +34,7 @@ cleanup
mysqldump -u${USER} -p${PASS} --opt --flush-logs --single-transaction \
--all-databases <% if @backupcompress %>| bzcat -zc <% end %>> ${DIR}/${PREFIX}`date +%Y%m%d-%H%M%S`.sql<% if @backupcompress %>.bz2<% end %>
<% if not @delete_before_dump -%>
<% unless @delete_before_dump -%>
if [ $? -eq 0 ] ; then
cleanup
fi