Skip to content

Improvements to mysql_grant. #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 2, 2013
Merged

Improvements to mysql_grant. #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions lib/puppet/provider/mysql_grant/mysql.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
def self.instances
instances = []
users.select{ |user| user =~ /.+@/ }.collect do |user|
user_string = "'#{user.sub('@', "'@'")}'"
user_string = self.cmd_user(user)
query = "SHOW GRANTS FOR #{user_string};"
grants = mysql([defaults_file, "-NBe", query].compact)
# Once we have the list of grants generate entries for each.
Expand Down Expand Up @@ -50,12 +50,9 @@ def self.prefetch(resources)
def grant(user, table, privileges, options)
user_string = self.class.cmd_user(user)
priv_string = self.class.cmd_privs(privileges)
# Table is optional.
if table
table_string = self.class.cmd_table(table)
end
table_string = self.class.cmd_table(table)
query = "GRANT #{priv_string}"
query << " ON #{table_string}" if table
query << " ON #{table_string}"
query << " TO #{user_string}"
query << self.class.cmd_options(options) unless options.nil?
mysql([defaults_file, '-e', query].compact)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't that if the table is empty or undef that would raise an exception, because it would actually generate grant "priv" ON TO test1@localhost; ? Base on the type definition the table parameter can be easily defined undef.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added:

  • fail('table parameter is required.') if self[:ensure] == :present and self[:table].nil?

Which should stop the table from being blank in the type. Originally I thought it was optional but I believe you always need a table of some kind to issue a grant against. Please correct me if I'm off base but I can't find a time when you can say:

GRANT X TO 'blah'@'blah' without having an ON y.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that should be OK. Yes you always need some kind of table or reserved sequence sush as ., *, etc and use ON.

Expand Down
13 changes: 13 additions & 0 deletions lib/puppet/type/mysql_grant.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ def initialize(*args)

validate do
fail('privileges parameter is required.') if self[:ensure] == :present and self[:privileges].nil?
fail('table parameter is required.') if self[:ensure] == :present and self[:table].nil?
fail('user parameter is required.') if self[:ensure] == :present and self[:user].nil?
end

newparam(:name, :namevar => true) do
Expand All @@ -40,10 +42,21 @@ def initialize(*args)
munge do |value|
value.delete("`")
end

newvalues(/.*\..*/)
end

newproperty(:user) do
desc 'User to operate on.'
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 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(:options, :array_matching => :all) do
Expand Down
52 changes: 35 additions & 17 deletions spec/system/types/mysql_grant_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ class { 'mysql::server': }
describe 'missing privileges for user' do
it 'should fail' do
pp = <<-EOS
mysql_grant { 'test@tester/test.*':
mysql_grant { 'test1@tester/test.*':
ensure => 'present',
table => 'test.*',
user => 'test@tester',
user => 'test1@tester',
}
EOS

Expand All @@ -28,8 +28,8 @@ class { 'mysql::server': }
end

it 'should not find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR test@tester\"") do |r|
r.stderr.should =~ /There is no such grant defined for user 'test' on host 'tester'/
shell("mysql -NBe \"SHOW GRANTS FOR test1@tester\"") do |r|
r.stderr.should =~ /There is no such grant defined for user 'test1' on host 'tester'/
r.exit_code.should eq 1
end
end
Expand All @@ -46,7 +46,7 @@ class { 'mysql::server': }
EOS

puppet_apply(pp) do |r|
r.exit_code.should eq 4
r.exit_code.should eq 1
end
end

Expand All @@ -61,10 +61,10 @@ class { 'mysql::server': }
describe 'adding privileges' do
it 'should work without errors' do
pp = <<-EOS
mysql_grant { 'test@tester/test.*':
mysql_grant { 'test2@tester/test.*':
ensure => 'present',
table => 'test.*',
user => 'test@tester',
user => 'test2@tester',
privileges => ['SELECT', 'UPDATE'],
}
EOS
Expand All @@ -73,8 +73,8 @@ class { 'mysql::server': }
end

it 'should find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR test@tester\"") do |r|
r.stdout.should =~ /GRANT SELECT, UPDATE.*TO 'test'@'tester'/
shell("mysql -NBe \"SHOW GRANTS FOR test2@tester\"") do |r|
r.stdout.should =~ /GRANT SELECT, UPDATE.*TO 'test2'@'tester'/
r.stderr.should be_empty
r.exit_code.should be_zero
end
Expand All @@ -84,10 +84,10 @@ class { 'mysql::server': }
describe 'adding option' do
it 'should work without errors' do
pp = <<-EOS
mysql_grant { 'test@tester/test.*':
mysql_grant { 'test3@tester/test.*':
ensure => 'present',
table => 'test.*',
user => 'test@tester',
user => 'test3@tester',
options => ['GRANT'],
privileges => ['SELECT', 'UPDATE'],
}
Expand All @@ -97,22 +97,40 @@ class { 'mysql::server': }
end

it 'should find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR test@tester\"") do |r|
r.stdout.should =~ /GRANT SELECT, UPDATE ON `test`.* TO 'test'@'tester' WITH GRANT OPTION$/
shell("mysql -NBe \"SHOW GRANTS FOR test3@tester\"") do |r|
r.stdout.should =~ /GRANT SELECT, UPDATE ON `test`.* TO 'test3'@'tester' WITH GRANT OPTION$/
r.stderr.should be_empty
r.exit_code.should be_zero
end
end
end

describe 'adding all privileges without table' do
it 'should fail' do
pp = <<-EOS
mysql_grant { 'test4@tester/test.*':
ensure => 'present',
user => 'test4@tester',
options => ['GRANT'],
privileges => ['SELECT', 'UPDATE', 'ALL'],
}
EOS

puppet_apply(pp) do |r|
r.stderr.should =~ /table parameter is required./
end
end

end


describe 'adding all privileges' do
it 'should only try to apply ALL' do
pp = <<-EOS
mysql_grant { 'test@tester/test.*':
mysql_grant { 'test4@tester/test.*':
ensure => 'present',
table => 'test.*',
user => 'test@tester',
user => 'test4@tester',
options => ['GRANT'],
privileges => ['SELECT', 'UPDATE', 'ALL'],
}
Expand All @@ -122,8 +140,8 @@ class { 'mysql::server': }
end

it 'should find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR test@tester\"") do |r|
r.stdout.should =~ /GRANT ALL PRIVILEGES ON `test`.* TO 'test'@'tester' WITH GRANT OPTION/
shell("mysql -NBe \"SHOW GRANTS FOR test4@tester\"") do |r|
r.stdout.should =~ /GRANT ALL PRIVILEGES ON `test`.* TO 'test4'@'tester' WITH GRANT OPTION/
r.stderr.should be_empty
r.exit_code.should be_zero
end
Expand Down