Skip to content

MODULES-8373 Fix mysql_grant resource to be idempodent on MySQL 8+ #1427

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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: 8 additions & 1 deletion lib/puppet/provider/mysql_grant/mysql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ def self.instances
(priv == 'ALL PRIVILEGES') ? 'ALL' : priv.strip
end
end
sorted_privileges = stripped_privileges.sort
if newer_than('mysql' => '8.0.0') && 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']
sorted_privileges = ['ALL']
end
# Same here, but to remove OPTION leaving just GRANT.
options = if %r{WITH\sGRANT\sOPTION}.match?(rest)
['GRANT']
Expand All @@ -57,7 +64,7 @@ def self.instances
instances << new(
name: "#{user}@#{host}/#{table}",
ensure: :present,
privileges: stripped_privileges.sort,
privileges: sorted_privileges,
table: table,
user: "#{user}@#{host}",
options: options,
Expand Down
2 changes: 1 addition & 1 deletion spec/acceptance/00_mysql_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class { 'mysql::server':
databases => {
'somedb' => {
ensure => 'present',
charset => 'utf8',
charset => #{fetch_charset},
},
}
}
Expand Down
3 changes: 3 additions & 0 deletions spec/acceptance/01_mysql_db_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class { 'mysql::server':
mysql::db { 'spec1':
user => 'root1',
password => 'password',
charset => #{fetch_charset},
}
MANIFEST
end
Expand Down Expand Up @@ -42,6 +43,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
user => 'root1',
password => 'password',
sql => '/tmp/spec.sql',
charset => #{fetch_charset},
}
MANIFEST
end
Expand All @@ -66,6 +68,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
user => 'root1',
password => 'password',
dbname => 'realdb',
charset => #{fetch_charset},
}
MANIFEST
end
Expand Down
4 changes: 4 additions & 0 deletions spec/acceptance/04_mysql_backup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class { 'mysql::server': root_password => 'password' }
]:
user => 'backup',
password => 'secret',
charset => #{fetch_charset},
}

class { 'mysql::server::backup':
Expand Down Expand Up @@ -72,6 +73,7 @@ class { 'mysql::server': root_password => 'password' }
]:
user => 'backup',
password => 'secret',
charset => #{fetch_charset},
}

class { 'mysql::server::backup':
Expand Down Expand Up @@ -136,6 +138,7 @@ class { 'mysql::server': root_password => 'password' }
]:
user => 'backup',
password => 'secret',
charset => #{fetch_charset},
}
case $facts['os']['family'] {
/Debian/: {
Expand Down Expand Up @@ -260,6 +263,7 @@ class { 'mysql::server': root_password => 'password' }
]:
user => 'backup',
password => 'secret',
charset => #{fetch_charset},
}
case $facts['os']['family'] {
/Debian/: {
Expand Down
7 changes: 4 additions & 3 deletions spec/acceptance/types/mysql_database_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ class { 'mysql::server': }
describe 'creating database' do
pp = <<-MANIFEST
mysql_database { 'spec_db':
ensure => present,
ensure => present,
charset => #{fetch_charset},
}
MANIFEST
it 'works without errors' do
Expand All @@ -37,7 +38,7 @@ class { 'mysql::server': }
collate => 'latin1_swedish_ci',
}
mysql_database { 'spec_utf8':
charset => 'utf8',
charset => #{fetch_charset},
collate => 'utf8_general_ci',
}
MANIFEST
Expand All @@ -54,7 +55,7 @@ class { 'mysql::server': }

it 'finds utf8 db #stdout' do
run_shell("mysql -NBe \"SHOW VARIABLES LIKE '%_database'\" spec_utf8") do |r|
expect(r.stdout).to match(%r{^character_set_database\tutf8\ncollation_database\tutf8_general_ci$})
expect(r.stdout).to match(%r{^character_set_database\tutf8(mb3)?\ncollation_database\tutf8_general_ci$})
expect(r.stderr).to be_empty
end
end
Expand Down
45 changes: 4 additions & 41 deletions spec/acceptance/types/mysql_grant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,51 +268,13 @@ class { 'mysql::server':
end
end

# On Ubuntu 20.04 'ALL' now returns as the sum of it's constitute parts and so require a specific test
describe 'ALL privilege on newer MySQL versions', if: os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04} do
pp_one = <<-MANIFEST
mysql_user { 'all@localhost':
ensure => present,
}
mysql_grant { 'all@localhost/*.*':
user => 'all@localhost',
privileges => ['ALL'],
table => '*.*',
require => Mysql_user['all@localhost'],
}
MANIFEST
it "create ['ALL'] privs" do
apply_manifest(pp_one, catch_failures: true)
end

pp_two = <<-MANIFEST
mysql_user { 'all@localhost':
ensure => present,
}
mysql_grant { 'all@localhost/*.*':
user => 'all@localhost',
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'],
table => '*.*',
require => Mysql_user['all@localhost'],
}
MANIFEST
it "create ['ALL'] constitute parts privs" do
apply_manifest(pp_two, catch_changes: true)
end
end

describe 'complex test' do
# On Ubuntu 20.04 'ALL' now returns as the sum of it's constitute parts and so is no longer idempotent when set
privileges = if os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04}
"['SELECT', 'INSERT', 'UPDATE']"
else
"['ALL']"
end
pp = <<-MANIFEST
$dbSubnet = '10.10.10.%'

mysql_database { 'foo':
ensure => present,
ensure => present,
charset => '#{fetch_charset}',
}

exec { 'mysql-create-table':
Expand All @@ -325,7 +287,7 @@ class { 'mysql::server':
Mysql_grant {
ensure => present,
options => ['GRANT'],
privileges => #{privileges},
privileges => ['ALL'],
table => '*.*',
require => [ Mysql_database['foo'], Exec['mysql-create-table'] ],
}
Expand Down Expand Up @@ -724,6 +686,7 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
user => 'root1',
password => 'password',
sql => '/tmp/grant_spec_table.sql',
charset => #{fetch_charset},
}
MANIFEST
it 'creates table' do
Expand Down
8 changes: 8 additions & 0 deletions spec/spec_helper_acceptance_local.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ def export_locales
LitmusHelper.instance.run_shell('source ~/.bashrc')
end

def fetch_charset
@charset ||= if os[:family] == 'ubuntu' && os[:release] =~ %r{^20\.04}
'utf8mb3'
else
'utf8'
end
end

RSpec.configure do |c|
c.before :suite do
if os[:family] == 'debian' || os[:family] == 'ubuntu'
Expand Down