Skip to content

Resolve dependencies to the databases automatically #963

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

Closed
wants to merge 4 commits into from
Closed

Resolve dependencies to the databases automatically #963

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
927a455
Improve style of the role resource
hasegeli Mar 7, 2018
c24687b
Remove db argument from the role resource
hasegeli Mar 7, 2018
a51fefe
Improve style of the database resource
hasegeli Mar 15, 2018
9e7c6a0
Resolve dependencies to the databases automatically
hasegeli Mar 1, 2018
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
34 changes: 19 additions & 15 deletions manifests/server/database.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,23 @@
require => Class['postgresql::server::service']
}

# Automatically require the database for the statements that requires it
#
# XXX: We should probably do everything in this class before. Postgresql_psql resource could
# specify this better, but currently it is impossible to use "autorequire" against custom
# resources. We can improve this when a better way is available.
Postgresql_psql["CREATE DATABASE \"${dbname}\""] -> Postgresql_psql <| db == $dbname |>

# This will prevent users from connecting to the database unless they've been
# granted privileges.
~> postgresql_psql { "REVOKE ${public_revoke_privilege} ON DATABASE \"${dbname}\" FROM public":
postgresql_psql { "REVOKE ${public_revoke_privilege} ON DATABASE \"${dbname}\" FROM public":
refreshonly => true,
subscribe => Postgresql_psql["CREATE DATABASE \"${dbname}\""],
}

Postgresql_psql["CREATE DATABASE \"${dbname}\""]
-> postgresql_psql { "UPDATE pg_database SET datistemplate = ${istemplate} WHERE datname = '${dbname}'":
unless => "SELECT 1 FROM pg_database WHERE datname = '${dbname}' AND datistemplate = ${istemplate}",
postgresql_psql { "UPDATE pg_database SET datistemplate = ${istemplate} WHERE datname = '${dbname}'":
unless => "SELECT 1 FROM pg_database WHERE datname = '${dbname}' AND datistemplate = ${istemplate}",
require => Postgresql_psql["CREATE DATABASE \"${dbname}\""],
}

if $comment {
Expand All @@ -96,10 +104,11 @@
'8.1' => 'obj_description',
default => 'shobj_description',
}
Postgresql_psql["CREATE DATABASE \"${dbname}\""]
-> postgresql_psql { "COMMENT ON DATABASE \"${dbname}\" IS '${comment}'":
unless => "SELECT 1 FROM pg_catalog.pg_database d WHERE datname = '${dbname}' AND pg_catalog.${comment_information_function}(d.oid, 'pg_database') = '${comment}'",
db => $dbname,

postgresql_psql { "COMMENT ON DATABASE \"${dbname}\" IS '${comment}'":
unless => "SELECT 1 FROM pg_catalog.pg_database d WHERE datname = '${dbname}' AND pg_catalog.${comment_information_function}(d.oid, 'pg_database') = '${comment}'",
db => $dbname,
require => Postgresql_psql["CREATE DATABASE \"${dbname}\""],
}
}

Expand All @@ -109,9 +118,7 @@
require => Postgresql_psql["CREATE DATABASE \"${dbname}\""],
}

if defined(Postgresql::Server::Role[$owner]) {
Postgresql::Server::Role[$owner]->Postgresql_psql["ALTER DATABASE \"${dbname}\" OWNER TO \"${owner}\""]
}
Postgresql::Server::Role <| username == $owner |> -> Postgresql_psql["ALTER DATABASE \"${dbname}\" OWNER TO \"${owner}\""]
}

if $tablespace {
Expand All @@ -120,9 +127,6 @@
require => Postgresql_psql["CREATE DATABASE \"${dbname}\""],
}

if defined(Postgresql::Server::Tablespace[$tablespace]) {
# The tablespace must be there, before we create the database.
Postgresql::Server::Tablespace[$tablespace]->Postgresql_psql["CREATE DATABASE \"${dbname}\""]
}
Postgresql::Server::Tablespace <| spcname == $tablespace |> -> Postgresql_psql["CREATE DATABASE \"${dbname}\""]
}
}
7 changes: 0 additions & 7 deletions manifests/server/extension.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@
}
}

if( $database != 'postgres' ) {
# The database postgres cannot managed by this module, so it is exempt from this dependency
Postgresql_psql {
require => Postgresql::Server::Database[$database],
}
}

postgresql_psql { "${database}: ${command}":

psql_user => $user,
Expand Down
4 changes: 0 additions & 4 deletions manifests/server/grant.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,4 @@
if($role != undef and defined(Postgresql::Server::Role[$role])) {
Postgresql::Server::Role[$role]->Postgresql_psql["grant:${name}"]
}

if($db != undef and defined(Postgresql::Server::Database[$db])) {
Postgresql::Server::Database[$db]->Postgresql_psql["grant:${name}"]
}
}
4 changes: 0 additions & 4 deletions manifests/server/reassign_owned_by.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,4 @@
if($new_role != undef and defined(Postgresql::Server::Role[$new_role])) {
Postgresql::Server::Role[$new_role]->Postgresql_psql["reassign_owned_by:${db}:${sql_command}"]
}

if($db != undef and defined(Postgresql::Server::Database[$db])) {
Postgresql::Server::Database[$db]->Postgresql_psql["reassign_owned_by:${db}:${sql_command}"]
}
}
58 changes: 32 additions & 26 deletions manifests/server/role.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
$password_hash = false,
$createdb = false,
$createrole = false,
$db = $postgresql::server::default_database,
Copy link
Contributor

@hunner hunner Mar 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This parameter is considered public API and removing it would be breaking, even if it doesn't actually do anything. If the db param is not needed then we could just make it unused and deprecated it, and remove it later.

Does the database specified not affect the ALTER ROLE commands below? Are roles always defined globally and not db-specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes roles are always global on Postgres. This option was also not documented. I find it hard to believe anyone would search and find the argument that does nothing. Should we still keep it and emit a warning()?

Copy link
Contributor

@hunner hunner Apr 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting that it was not documented. Yes, I think it should still be kept and just emit a warning and do nothing. It has been there ever since postgresql::server::role was created in 2013 and even before that for postgresql::role. And it's in the acceptance tests. I'm sure there are people using it for years :.

$port = undef,
$login = true,
$inherit = true,
Expand All @@ -15,6 +14,7 @@
$connect_settings = $postgresql::server::default_connect_settings,
Enum['present', 'absent'] $ensure = 'present',
) {
$psql_db = $postgresql::server::default_database
$psql_user = $postgresql::server::user
$psql_group = $postgresql::server::group
$psql_path = $postgresql::server::psql_path
Expand All @@ -40,14 +40,13 @@
}

Postgresql_psql {
db => $db,
port => $port_override,
psql_user => $psql_user,
psql_group => $psql_group,
psql_path => $psql_path,
db => $psql_db,
port => $port_override,
psql_user => $psql_user,
psql_group => $psql_group,
psql_path => $psql_path,
connect_settings => $connect_settings,
cwd => $module_workdir,
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
cwd => $module_workdir,
}

if $ensure == 'present' {
Expand All @@ -69,43 +68,50 @@
command => "CREATE ROLE \"${username}\" ${password_sql} ${login_sql} ${createrole_sql} ${createdb_sql} ${superuser_sql} ${replication_sql} CONNECTION LIMIT ${connection_limit}",
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}'",
environment => $environment,
require => undef,
}

postgresql_psql {"ALTER ROLE \"${username}\" ${superuser_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolsuper = ${superuser}",
postgresql_psql { "ALTER ROLE \"${username}\" ${superuser_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolsuper = ${superuser}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

postgresql_psql {"ALTER ROLE \"${username}\" ${createdb_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcreatedb = ${createdb}",
postgresql_psql { "ALTER ROLE \"${username}\" ${createdb_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcreatedb = ${createdb}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

postgresql_psql {"ALTER ROLE \"${username}\" ${createrole_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcreaterole = ${createrole}",
postgresql_psql { "ALTER ROLE \"${username}\" ${createrole_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcreaterole = ${createrole}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

postgresql_psql {"ALTER ROLE \"${username}\" ${login_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcanlogin = ${login}",
postgresql_psql { "ALTER ROLE \"${username}\" ${login_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolcanlogin = ${login}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

postgresql_psql {"ALTER ROLE \"${username}\" ${inherit_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolinherit = ${inherit}",
postgresql_psql { "ALTER ROLE \"${username}\" ${inherit_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolinherit = ${inherit}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

if(versioncmp($version, '9.1') >= 0) {
if $replication_sql == '' {
postgresql_psql {"ALTER ROLE \"${username}\" NOREPLICATION":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolreplication = ${replication}",
postgresql_psql { "ALTER ROLE \"${username}\" NOREPLICATION":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolreplication = ${replication}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}
} else {
postgresql_psql {"ALTER ROLE \"${username}\" ${replication_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolreplication = ${replication}",
postgresql_psql { "ALTER ROLE \"${username}\" ${replication_sql}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolreplication = ${replication}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}
}
}

postgresql_psql {"ALTER ROLE \"${username}\" CONNECTION LIMIT ${connection_limit}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolconnlimit = ${connection_limit}",
postgresql_psql { "ALTER ROLE \"${username}\" CONNECTION LIMIT ${connection_limit}":
unless => "SELECT 1 FROM pg_roles WHERE rolname = '${username}' AND rolconnlimit = ${connection_limit}",
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}

if $password_hash and $update_password {
Expand All @@ -119,13 +125,13 @@
command => "ALTER ROLE \"${username}\" ${password_sql}",
unless => "SELECT 1 FROM pg_shadow WHERE usename = '${username}' AND passwd = '${pwd_hash_sql}'",
environment => $environment,
require => Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
}
}
} else {
# ensure == absent
postgresql_psql { "DROP ROLE \"${username}\"":
onlyif => "SELECT 1 FROM pg_roles WHERE rolname = '${username}'",
require => undef,
}
}
}
8 changes: 0 additions & 8 deletions spec/acceptance/server/grant_role_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ class { 'postgresql::server': }

# Create a role to grant to the user
postgresql::server::role { $group:
db => $db,
login => false,
require => Postgresql::Server::Database[$db],
}

# Grant the role to the user
Expand Down Expand Up @@ -106,9 +104,7 @@ class { 'postgresql::server': }

# Create a role to grant to the user
postgresql::server::role { $group:
db => $db,
login => false,
require => Postgresql::Server::Database[$db],
}

# Grant the role to the user
Expand Down Expand Up @@ -160,9 +156,7 @@ class { 'postgresql::server': }

# Create a role to grant to the user
postgresql::server::role { $group:
db => $db,
login => false,
require => Postgresql::Server::Database[$db],
}

# Grant the role to the user
Expand Down Expand Up @@ -197,9 +191,7 @@ class { 'postgresql::server': }

# Create a role to grant to the nonexistent user
postgresql::server::role { $group:
db => $db,
login => false,
require => Postgresql::Server::Database[$db],
}

# Grant the role to the nonexistent user
Expand Down
7 changes: 0 additions & 7 deletions spec/acceptance/server/grant_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class { 'postgresql::server': }

# Create a user to grant privileges to
postgresql::server::role { $user:
db => $db,
require => Postgresql::Server::Database[$db],
}

# Make a local user for ident auth
Expand Down Expand Up @@ -60,7 +58,6 @@ class { 'postgresql::server': }
db => $db,
psql_user => '#{superuser}',
unless => "SELECT 1 from pg_language where lanname = 'plpgsql'",
require => Postgresql::Server::Database[$db],
}

postgresql::server::grant { 'grant usage on plpgsql':
Expand Down Expand Up @@ -140,7 +137,6 @@ class { 'postgresql::server': }
db => $db,
psql_user => $owner,
unless => "SELECT 1 FROM information_schema.sequences WHERE sequence_name = 'test_seq'",
require => Postgresql::Server::Database[$db],
}

postgresql::server::grant { 'grant usage on test_seq':
Expand All @@ -161,7 +157,6 @@ class { 'postgresql::server': }
db => $db,
psql_user => $owner,
unless => "SELECT 1 FROM information_schema.sequences WHERE sequence_name = 'test_seq'",
require => Postgresql::Server::Database[$db],
}

postgresql::server::grant { 'grant update on test_seq':
Expand Down Expand Up @@ -226,7 +221,6 @@ class { 'postgresql::server': }
db => $db,
psql_user => $owner,
unless => "SELECT 1 FROM information_schema.sequences WHERE sequence_name = 'test_seq2'",
require => Postgresql::Server::Database[$db],
}

postgresql::server::grant { 'grant usage on all sequences':
Expand All @@ -248,7 +242,6 @@ class { 'postgresql::server': }
db => $db,
psql_user => $owner,
unless => "SELECT 1 FROM information_schema.sequences WHERE sequence_name = 'test_seq2'",
require => Postgresql::Server::Database[$db],
}

postgresql::server::grant { 'grant usage on all sequences':
Expand Down
5 changes: 1 addition & 4 deletions spec/acceptance/server/reassign_owned_by_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ class { 'postgresql::server': }

# Create a user to reassign ownership to
postgresql::server::role { $new_owner:
db => $db,
require => Postgresql::Server::Database[$db],
}

# Make a local user for ident auth
Expand Down Expand Up @@ -80,14 +78,13 @@ class { 'postgresql::server': }
db => '#{db}',
psql_user => '#{old_owner}',
unless => "SELECT tablename FROM pg_catalog.pg_tables WHERE tablename = 'test_tbl'",
require => Postgresql::Server::Database['#{db}'],
}
postgresql_psql { 'create test sequence':
command => 'CREATE SEQUENCE test_seq',
db => '#{db}',
psql_user => '#{old_owner}',
unless => "SELECT relname FROM pg_catalog.pg_class WHERE relkind='S' AND relname = 'test_seq'",
require => [ Postgresql_psql['create test table'], Postgresql::Server::Database['#{db}'] ],
require => Postgresql_psql['create test table'],
}
MANIFEST
end
Expand Down
1 change: 0 additions & 1 deletion spec/acceptance/server/schema_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class { 'postgresql::server': }
postgresql::server::schema { $user:
db => $db,
owner => $user,
require => Postgresql::Server::Database[$db],
}
MANIFEST
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/defines/server/db_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

it { is_expected.to contain_postgresql__server__db('test') }
it { is_expected.to contain_postgresql__server__database('test').with_owner('tester') }
it { is_expected.to contain_postgresql__server__role('test').that_comes_before('Postgresql::Server::Database[test]') }
it { is_expected.to contain_postgresql__server__role('test') }
it { is_expected.to contain_postgresql__server__database_grant('GRANT test - ALL - test') }
end

Expand Down
Loading