Skip to content

Harden db defined type #1484

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 4 commits into from
Aug 23, 2022
Merged

Harden db defined type #1484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
547483f
Harden db defined type
chelnak Aug 16, 2022
211de4f
Add spec tests
chelnak Aug 16, 2022
603287f
Fixes for acceptance tests
chelnak Aug 17, 2022
cd2e485
Update manifests/db.pp
chelnak Aug 23, 2022
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
64 changes: 43 additions & 21 deletions manifests/db.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
# grant => ['SELECT', 'UPDATE'],
# }
#
# @param name
# The name of the database to create. Database names must:
# * be longer than 64 characters.
# * not contain / \ or . characters.
# * not contain characters that are not permitted in file names.
# * not end with space characters.
# @param user
# The user for the database you're creating.
# @param password
Expand All @@ -28,7 +34,7 @@
# @param grant_options
# The grant_options for the grant for user@host on the database.
# @param sql
# The path to the sqlfile you want to execute. This can be single file specified as string, or it can be an array of strings.
# The path to the sqlfile you want to execute. This can be a an array containing one or more file paths.
# @param enforce_sql
# Specifies whether executing the sqlfiles should happen on every run. If set to false, sqlfiles only run once.
# @param ensure
Expand All @@ -41,25 +47,41 @@
define mysql::db (
$user,
Variant[String, Sensitive[String]] $password,
$tls_options = undef,
$dbname = $name,
$charset = 'utf8',
$collate = 'utf8_general_ci',
$host = 'localhost',
$grant = 'ALL',
$grant_options = undef,
Optional[Variant[Array, Hash, String]] $sql = undef,
$enforce_sql = false,
Enum['absent', 'present'] $ensure = 'present',
$import_timeout = 300,
$import_cat_cmd = 'cat',
$mysql_exec_path = undef,
$tls_options = undef,
String $dbname = $name,
$charset = 'utf8',
$collate = 'utf8_general_ci',
$host = 'localhost',
$grant = 'ALL',
$grant_options = undef,
Optional[Array] $sql = undef,
$enforce_sql = false,
Enum['absent', 'present'] $ensure = 'present',
$import_timeout = 300,
Enum['cat', 'zcat', 'bzcat'] $import_cat_cmd = 'cat',
$mysql_exec_path = undef,
) {
$table = "${dbname}.*"
include 'mysql::client'

$sql_inputs = join([$sql], ' ')
# Ensure that the database name is valid.
if $dbname !~ /^[^\/?%*:|\""<>.\s;]{1,64}$/ {
$message = "The database name '${dbname}' is invalid. Values must:
* be longer than 64 characters.
* not contain // \\ or . characters.
* not contain characters that are not permitted in file names.
* not end with space characters."
fail($message)
}

include 'mysql::client'
# Ensure that the sql files passed are valid file paths.
if $sql {
$sql.each | $sqlfile | {
if $sqlfile !~ /^\/(?:[A-Za-z0-9_-]+\/?+)+(?:.[A-Za-z0-9]+)$/ {
$message = "The file '${sqlfile}' is invalid. A a valid file path is expected."
fail($message)
}
}
}

if ($mysql_exec_path) {
$_mysql_exec_path = $mysql_exec_path
Expand All @@ -84,6 +106,8 @@
ensure_resource('mysql_user', "${user}@${host}", $user_resource)

if $ensure == 'present' {
$table = "${dbname}.*"

mysql_grant { "${user}@${host}/${table}":
privileges => $grant,
provider => 'mysql',
Expand All @@ -96,14 +120,12 @@
],
}

$refresh = ! $enforce_sql

if $sql {
exec { "${dbname}-import":
command => "${import_cat_cmd} ${sql_inputs} | mysql ${dbname}",
command => "${import_cat_cmd} ${shell_join($sql)} | mysql ${dbname}",
logoutput => true,
environment => "HOME=${::root_home}",
refreshonly => $refresh,
refreshonly => ! $enforce_sql,
path => "/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:${_mysql_exec_path}",
require => Mysql_grant["${user}@${host}/${table}"],
subscribe => Mysql_database[$dbname],
Expand Down
2 changes: 1 addition & 1 deletion spec/acceptance/01_mysql_db_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
mysql::db { 'spec2':
user => 'root1',
password => 'password',
sql => '/tmp/spec.sql',
charset => '#{fetch_charset}',
collate => '#{fetch_charset}_general_ci',
sql => ['/tmp/spec.sql'],
}
MANIFEST
end
Expand Down
2 changes: 1 addition & 1 deletion spec/acceptance/types/mysql_grant_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ class { 'mysql::server': override_options => { 'root_password' => 'password' } }
mysql::db { 'grant_spec_db':
user => 'root1',
password => 'password',
sql => '/tmp/grant_spec_table.sql',
charset => '#{fetch_charset}',
sql => ['/tmp/grant_spec_table.sql'],
}
MANIFEST
it 'creates table' do
Expand Down
75 changes: 66 additions & 9 deletions spec/defines/mysql_db_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,42 @@
'mysql_exec_path' => '' }
end

let(:sql) { [ '/tmp/test.sql' ] }

it 'does not notify the import sql exec if no sql script was provided' do
is_expected.to contain_mysql_database('test_db').without_notify
end

it 'subscribes to database if sql script is given' do
params['sql'] = 'test_sql'
params['sql'] = sql
is_expected.to contain_mysql_database('test_db')
is_expected.to contain_exec('test_db-import').with_subscribe('Mysql_database[test_db]')
end

it 'onlies import sql script on creation if not enforcing' do
params.merge!('sql' => 'test_sql', 'enforce_sql' => false)
params.merge!('sql' => sql, 'enforce_sql' => false)
is_expected.to contain_exec('test_db-import').with_refreshonly(true)
end

it 'imports sql script on creation' do
params.merge!('sql' => 'test_sql', 'enforce_sql' => true)
params.merge!('sql' => sql, 'enforce_sql' => true)
# ' if enforcing #refreshonly'
is_expected.to contain_exec('test_db-import').with_refreshonly(false)
# 'if enforcing #command'
is_expected.to contain_exec('test_db-import').with_command('cat test_sql | mysql test_db')
is_expected.to contain_exec('test_db-import').with_command('cat /tmp/test.sql | mysql test_db')
end

it 'imports sql script with custom command on creation ' do
params.merge!('sql' => 'test_sql', 'enforce_sql' => true, 'import_cat_cmd' => 'zcat')
it 'imports sql script with custom command on creation' do
params.merge!('sql' => sql, 'enforce_sql' => true, 'import_cat_cmd' => 'zcat')
# if enforcing #refreshonly
is_expected.to contain_exec('test_db-import').with_refreshonly(false)
# if enforcing #command
is_expected.to contain_exec('test_db-import').with_command('zcat test_sql | mysql test_db')
is_expected.to contain_exec('test_db-import').with_command('zcat /tmp/test.sql | mysql test_db')
end

it 'imports sql scripts when more than one is specified' do
params['sql'] = ['test_sql', 'test_2_sql']
is_expected.to contain_exec('test_db-import').with_command('cat test_sql test_2_sql | mysql test_db')
params['sql'] = ['/tmp/test.sql', '/tmp/test_2.sql']
is_expected.to contain_exec('test_db-import').with_command('cat /tmp/test.sql /tmp/test_2.sql | mysql test_db')
end

it 'does not create database' do
Expand Down Expand Up @@ -79,6 +81,61 @@
params['grant_options'] = ['GRANT']
is_expected.to contain_mysql_grant('testuser@localhost/test_db.*').with_options(['GRANT'])
end

# Invalid file paths
[
'|| ls -la ||',
'|| touch /tmp/foo.txt ||',
'/tmp/foo.txt;echo',
'myPath;',
'\\myPath\\',
'//myPath has spaces//',
'/',
].each do |path|
it "fails when provided '#{path}' as a value to the 'sql' parameter" do
params['sql'] = [path]
is_expected.to raise_error(Puppet::PreformattedError, %r{The file '#{Regexp.escape(path)}' is invalid. A a valid file path is expected.})
end
end

# Valid file paths
[
'/tmp/test.txt',
'/tmp/.test',
'/foo.test',
].each do |path|
it "succeeds when provided '#{path}' as a value to the 'sql' parameter" do
params['sql'] = [path]
is_expected.to contain_exec('test_db-import').with_command("cat #{path} | mysql test_db")
end
end

# Invalid database names
[
'test db',
'test_db;',
'test/db',
'|| ls -la ||',
'|| touch /tmp/foo.txt ||',
].each do |name|
it "fails when provided '#{name}' as a value to the 'name' parameter" do
params['name'] = name
is_expected.to raise_error(Puppet::PreformattedError, %r{The database name '#{name}' is invalid.})
end
end

# Valid database names
[
'test_db',
'testdb',
'test-db',
'TESTDB',
].each do |name|
it "succeeds when the provided '#{name}' as a value to the 'dbname' parameter" do
params['dbname'] = name
is_expected.to contain_mysql_database(name)
end
end
end
end
end