Skip to content

Commit 9b691e9

Browse files
author
Andre Ponert
committed
MODULES-8373 Fix mysql_grant resource to be idempodent on MySQL 8+
When the MySQL module is used against MySQL8, mysql_grant is no more idempodent. This is because MySQL 8+ no more returns "ALL" for SHOW GRANTS, when all privileges are granted. Instead, the separate privileges are shown. Unfortunately, the tests were just adapted to not error in this situation. This commit aims to fix the issue and also fixes the tests.
1 parent cb97c87 commit 9b691e9

File tree

7 files changed

+29
-46
lines changed

7 files changed

+29
-46
lines changed

lib/puppet/provider/mysql_grant/mysql.rb

+5-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ def self.instances
4545
(priv == 'ALL PRIVILEGES') ? 'ALL' : priv.strip
4646
end
4747
end
48+
sorted_privileges = stripped_privileges.sort
49+
if self.newer_than('mysql' => '8.0.0')
50+
sorted_privileges = (sorted_privileges == ['ALTER', 'ALTER ROUTINE', 'CREATE', 'CREATE ROLE', 'CREATE ROUTINE', 'CREATE TABLESPACE', 'CREATE TEMPORARY TABLES', 'CREATE USER', 'CREATE VIEW', 'DELETE', 'DROP', 'DROP ROLE', 'EVENT', 'EXECUTE', 'FILE', 'INDEX', 'INSERT', 'LOCK TABLES', 'PROCESS', 'REFERENCES', 'RELOAD', 'REPLICATION CLIENT', 'REPLICATION SLAVE', 'SELECT', 'SHOW DATABASES', 'SHOW VIEW', 'SHUTDOWN', 'SUPER', 'TRIGGER', 'UPDATE']) ? ['ALL'] : sorted_privileges
51+
end
4852
# Same here, but to remove OPTION leaving just GRANT.
4953
options = if %r{WITH\sGRANT\sOPTION}.match?(rest)
5054
['GRANT']
@@ -57,7 +61,7 @@ def self.instances
5761
instances << new(
5862
name: "#{user}@#{host}/#{table}",
5963
ensure: :present,
60-
privileges: stripped_privileges.sort,
64+
privileges: sorted_privileges,
6165
table: table,
6266
user: "#{user}@#{host}",
6367
options: options,

spec/acceptance/00_mysql_server_spec.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class { 'mysql::server':
3939
databases => {
4040
'somedb' => {
4141
ensure => 'present',
42-
charset => 'utf8',
42+
charset => #{$charset},
4343
},
4444
}
4545
}

spec/acceptance/01_mysql_db_spec.rb

+4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
describe 'mysql::db define' do
66
describe 'creating a database' do
7+
78
let(:pp) do
89
<<-MANIFEST
910
class { 'mysql::server':
@@ -14,6 +15,7 @@ class { 'mysql::server':
1415
mysql::db { 'spec1':
1516
user => 'root1',
1617
password => 'password',
18+
charset => #{$charset},
1719
}
1820
MANIFEST
1921
end
@@ -42,6 +44,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
4244
user => 'root1',
4345
password => 'password',
4446
sql => '/tmp/spec.sql',
47+
charset => #{$charset},
4548
}
4649
MANIFEST
4750
end
@@ -66,6 +69,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
6669
user => 'root1',
6770
password => 'password',
6871
dbname => 'realdb',
72+
charset => #{$charset},
6973
}
7074
MANIFEST
7175
end

spec/acceptance/04_mysql_backup_spec.rb

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class { 'mysql::server': root_password => 'password' }
1212
]:
1313
user => 'backup',
1414
password => 'secret',
15+
charset => #{$charset},
1516
}
1617
1718
class { 'mysql::server::backup':
@@ -72,6 +73,7 @@ class { 'mysql::server': root_password => 'password' }
7273
]:
7374
user => 'backup',
7475
password => 'secret',
76+
charset => #{$charset},
7577
}
7678
7779
class { 'mysql::server::backup':
@@ -136,6 +138,7 @@ class { 'mysql::server': root_password => 'password' }
136138
]:
137139
user => 'backup',
138140
password => 'secret',
141+
charset => #{$charset},
139142
}
140143
case $facts['os']['family'] {
141144
/Debian/: {
@@ -260,6 +263,7 @@ class { 'mysql::server': root_password => 'password' }
260263
]:
261264
user => 'backup',
262265
password => 'secret',
266+
charset => #{$charset},
263267
}
264268
case $facts['os']['family'] {
265269
/Debian/: {

spec/acceptance/types/mysql_database_spec.rb

+4-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ class { 'mysql::server': }
1515
describe 'creating database' do
1616
pp = <<-MANIFEST
1717
mysql_database { 'spec_db':
18-
ensure => present,
18+
ensure => present,
19+
charset => #{$charset},
1920
}
2021
MANIFEST
2122
it 'works without errors' do
@@ -37,7 +38,7 @@ class { 'mysql::server': }
3738
collate => 'latin1_swedish_ci',
3839
}
3940
mysql_database { 'spec_utf8':
40-
charset => 'utf8',
41+
charset => #{$charset},
4142
collate => 'utf8_general_ci',
4243
}
4344
MANIFEST
@@ -54,7 +55,7 @@ class { 'mysql::server': }
5455

5556
it 'finds utf8 db #stdout' do
5657
run_shell("mysql -NBe \"SHOW VARIABLES LIKE '%_database'\" spec_utf8") do |r|
57-
expect(r.stdout).to match(%r{^character_set_database\tutf8\ncollation_database\tutf8_general_ci$})
58+
expect(r.stdout).to match(%r{^character_set_database\tutf8(mb3)?\ncollation_database\tutf8_general_ci$})
5859
expect(r.stderr).to be_empty
5960
end
6061
end

spec/acceptance/types/mysql_grant_spec.rb

+4-41
Original file line numberDiff line numberDiff line change
@@ -268,51 +268,13 @@ class { 'mysql::server':
268268
end
269269
end
270270

271-
# On Ubuntu 20.04 'ALL' now returns as the sum of it's constitute parts and so require a specific test
272-
describe 'ALL privilege on newer MySQL versions', if: os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04} do
273-
pp_one = <<-MANIFEST
274-
mysql_user { 'all@localhost':
275-
ensure => present,
276-
}
277-
mysql_grant { 'all@localhost/*.*':
278-
user => 'all@localhost',
279-
privileges => ['ALL'],
280-
table => '*.*',
281-
require => Mysql_user['all@localhost'],
282-
}
283-
MANIFEST
284-
it "create ['ALL'] privs" do
285-
apply_manifest(pp_one, catch_failures: true)
286-
end
287-
288-
pp_two = <<-MANIFEST
289-
mysql_user { 'all@localhost':
290-
ensure => present,
291-
}
292-
mysql_grant { 'all@localhost/*.*':
293-
user => 'all@localhost',
294-
privileges => ['ALTER', 'ALTER ROUTINE', 'CREATE', 'CREATE ROLE', 'CREATE ROUTINE', 'CREATE TABLESPACE', 'CREATE TEMPORARY TABLES', 'CREATE USER', 'CREATE VIEW', 'DELETE', 'DROP', 'DROP ROLE', 'EVENT', 'EXECUTE', 'FILE', 'INDEX', 'INSERT', 'LOCK TABLES', 'PROCESS', 'REFERENCES', 'RELOAD', 'REPLICATION CLIENT', 'REPLICATION SLAVE', 'SELECT', 'SHOW DATABASES', 'SHOW VIEW', 'SHUTDOWN', 'SUPER', 'TRIGGER', 'UPDATE'],
295-
table => '*.*',
296-
require => Mysql_user['all@localhost'],
297-
}
298-
MANIFEST
299-
it "create ['ALL'] constitute parts privs" do
300-
apply_manifest(pp_two, catch_changes: true)
301-
end
302-
end
303-
304271
describe 'complex test' do
305-
# On Ubuntu 20.04 'ALL' now returns as the sum of it's constitute parts and so is no longer idempotent when set
306-
privileges = if os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04}
307-
"['SELECT', 'INSERT', 'UPDATE']"
308-
else
309-
"['ALL']"
310-
end
311272
pp = <<-MANIFEST
312273
$dbSubnet = '10.10.10.%'
313274
314275
mysql_database { 'foo':
315-
ensure => present,
276+
ensure => present,
277+
charset => '#{$charset}',
316278
}
317279
318280
exec { 'mysql-create-table':
@@ -325,7 +287,7 @@ class { 'mysql::server':
325287
Mysql_grant {
326288
ensure => present,
327289
options => ['GRANT'],
328-
privileges => #{privileges},
290+
privileges => ['ALL'],
329291
table => '*.*',
330292
require => [ Mysql_database['foo'], Exec['mysql-create-table'] ],
331293
}
@@ -724,6 +686,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
724686
user => 'root1',
725687
password => 'password',
726688
sql => '/tmp/grant_spec_table.sql',
689+
charset => #{$charset},
727690
}
728691
MANIFEST
729692
it 'creates table' do

spec/spec_helper_acceptance.rb

+7
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,10 @@
44
require 'spec_helper_acceptance_local' if File.file?(File.join(File.dirname(__FILE__), 'spec_helper_acceptance_local.rb'))
55

66
PuppetLitmus.configure!
7+
8+
# On Ubuntu 20.04 'utf8' charset now sets 'utf8mb3' internally and breaks idempotence
9+
$charset = if os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04}
10+
"utf8mb3"
11+
else
12+
"utf8"
13+
end

0 commit comments

Comments
 (0)