From 483ee60efd32db8ba0777e569fd72592cfee7bf6 Mon Sep 17 00:00:00 2001 From: Peter Palfrader Date: Sun, 29 Sep 2019 15:53:43 +0200 Subject: [PATCH] roles::postgresql::server now sets up postgres::cluster for all clusters Setting up backup moved to postgres::cluster which includes postgres::backup_cluster if requested. All the backup firewall access should be done via pg_hba entries now. --- .../nodes/postgresql-manda-01.debian.org.yaml | 3 +- modules/postgres/manifests/backup_cluster.pp | 17 +---- modules/postgres/manifests/backup_server.pp | 14 ---- .../register_backup_clienthost.pp | 8 +-- modules/postgres/manifests/backup_source.pp | 2 + modules/postgres/manifests/cluster.pp | 71 +++++++++++++++---- .../postgres/manifests/cluster/hba_entry.pp | 3 + .../manifests/postgresql/cluster_bacula.pp | 15 ---- modules/roles/manifests/postgresql/server.pp | 23 +++--- modules/salsa/manifests/database.pp | 3 +- 10 files changed, 77 insertions(+), 82 deletions(-) delete mode 100644 modules/roles/manifests/postgresql/cluster_bacula.pp diff --git a/data/nodes/postgresql-manda-01.debian.org.yaml b/data/nodes/postgresql-manda-01.debian.org.yaml index c1b01ac2b..7a8d76688 100644 --- a/data/nodes/postgresql-manda-01.debian.org.yaml +++ b/data/nodes/postgresql-manda-01.debian.org.yaml @@ -1,4 +1,5 @@ --- classes: - roles::postgresql::server - - roles::postgresql::cluster_bacula + +roles::postgresql::server::manage_clusters_hba: true diff --git a/modules/postgres/manifests/backup_cluster.pp b/modules/postgres/manifests/backup_cluster.pp index cceb94e65..172f33c08 100644 --- a/modules/postgres/manifests/backup_cluster.pp +++ b/modules/postgres/manifests/backup_cluster.pp @@ -30,6 +30,8 @@ define postgres::backup_cluster( Boolean $do_role = false, Boolean $do_hba = false, ) { + include postgres::backup_source + $datadir = "/var/lib/postgresql/${pg_version}/${pg_cluster}" file { "${datadir}/.nobackup": content => '' @@ -58,21 +60,6 @@ define postgres::backup_cluster( } } - # Send connections to the port to the pg-backup chain - # there, the register_backup_clienthost class will have - # realized the exported allows from the backup servers. - # - # Any non-matching traffic will fall through and it can - # be allowed elsewhere - # - # this rule is only needed for clusters that we do not manage - # with postgres::cluster. Hopefully these will go away with time - ferm::rule::simple { "dsa-postgres-backup-${pg_port}": - description => 'Check for postgres access from backup host', - port => $pg_port, - target => 'pg-backup', - } - postgres::cluster::hba_entry { "backup-replication::${pg_version}::${pg_cluster}": pg_version => $pg_version, pg_cluster => $pg_cluster, diff --git a/modules/postgres/manifests/backup_server.pp b/modules/postgres/manifests/backup_server.pp index 84f4c0dd1..bf8efa042 100644 --- a/modules/postgres/manifests/backup_server.pp +++ b/modules/postgres/manifests/backup_server.pp @@ -98,18 +98,4 @@ class postgres::backup_server { mode => '0400' } Concat::Fragment <<| tag == $postgres::backup_server::globals::tag_source_pgpassline |>> - - #### - # Let us connect to the clusters we want - # - # We export this, and the backup clients collect it - # - # this rule is only needed for clusters that we do not manage - # with postgres::cluster. Hopefully these will go away with time - @@ferm::rule::simple { "pg-backup_server::${::fqdn}": - tag => 'postgres::backup_server::to-client', - description => 'Allow access access from backup host', - chain => 'pg-backup', - saddr => $base::public_addresses, - } } diff --git a/modules/postgres/manifests/backup_server/register_backup_clienthost.pp b/modules/postgres/manifests/backup_server/register_backup_clienthost.pp index ff0f8f1b1..fc4cc0c1b 100644 --- a/modules/postgres/manifests/backup_server/register_backup_clienthost.pp +++ b/modules/postgres/manifests/backup_server/register_backup_clienthost.pp @@ -7,9 +7,9 @@ # # @param allow_read_basedir directory under which files can be read # @param allow_read_hosts subdirectories under base to allow -define postgres::backup_server::register_backup_clienthost ( +class postgres::backup_server::register_backup_clienthost ( String $allow_read_basedir = '/srv/backups/pg', - Array[Stdlib::Fqdn] $allow_read_hosts = lookup( { 'name' => 'postgres::backup_server::register_backup_clienthost::allow_read_hosts', 'default_value' => [] } ), + Array[Stdlib::Fqdn] $allow_read_hosts = [], ) { include postgres::backup_server::globals @@ -23,8 +23,4 @@ define postgres::backup_server::register_backup_clienthost ( from => $base::public_addresses, collect_tag => $postgres::backup_server::globals::tag_source_sshkey, } - - # this rule is only needed for clusters that we do not manage - # with postgres::cluster. Hopefully these will go away with time - Ferm::Rule::Simple <<| tag == 'postgres::backup_server::to-client' |>> } diff --git a/modules/postgres/manifests/backup_source.pp b/modules/postgres/manifests/backup_source.pp index d843243f2..304e7d6cd 100644 --- a/modules/postgres/manifests/backup_source.pp +++ b/modules/postgres/manifests/backup_source.pp @@ -2,6 +2,8 @@ # See dsa-wiki input/howto/postgres-backup.creole for some documentation # class postgres::backup_source { + include postgres::backup_server::register_backup_clienthost + file { '/usr/local/bin/pg-backup-file': mode => '0555', source => 'puppet:///modules/postgres/backup_source/pg-backup-file', diff --git a/modules/postgres/manifests/cluster.pp b/modules/postgres/manifests/cluster.pp index b541be517..424d35458 100644 --- a/modules/postgres/manifests/cluster.pp +++ b/modules/postgres/manifests/cluster.pp @@ -5,42 +5,83 @@ # @param pg_port port of the postgres cluster # @param manage_hba manage pg_hba # @param confdir directory where the configuration resides +# @param backups make backups of this cluster (unless it is recovering/a replication target) define postgres::cluster( - String $pg_version, - String $pg_cluster = 'main', - Integer $pg_port = 5432, + Optional[Integer] $pg_port = undef, + Optional[String] $pg_cluster = undef, + Optional[String] $pg_version = undef, Boolean $manage_hba = false, String $confdir = "/etc/postgresql/${pg_version}/${pg_cluster}", + Boolean $backups = true, ) { - $reload = "postgresql ${pg_version}/${pg_cluster} reload" + # get remaining cluster info and verify consistency + ### + $clusters = $facts['postgresql_clusters'] + if $pg_port { + $filtered = $clusters.filter |$cluster| { $cluster['port'] == $pg_port } + if $filtered.length != 1 { + fail("Did not find exactly one cluster with port ${pg_port}") + } + $cluster = $filtered[0] + } elsif $pg_cluster and $pg_version { + $filtered = $clusters.filter |$cluster| { $cluster['version'] == $pg_version and $cluster['cluster'] == $pg_cluster} + if $filtered.length != 1 { + fail("Did not find exactly one cluster ${pg_version}/${pg_cluster}") + } + $cluster = $filtered[0] + } else { + fail('postgres::cluster::hba_entry needs either the port of both a pg version and cluster name') + } + $real_port = $cluster['port'] + $real_version = $cluster['version'] + $real_cluster = $cluster['cluster'] + if $pg_version and $pg_version != $real_version { + fail("Inconsisten cluster version information: ${pg_version} != ${real_version}") + } + if $pg_cluster and $pg_cluster != $real_cluster { + fail("Inconsisten cluster name information: ${pg_cluster} != ${real_cluster}") + } + ### + + # basic infra + ### + $reload = "postgresql ${real_version}/${real_cluster} reload" exec { $reload: - command => "systemctl reload postgresql@${pg_version}-${pg_cluster}.service", + command => "systemctl reload postgresql@${real_version}-${real_cluster}.service", refreshonly => true, } + ferm::rule::simple { "postgres::cluster::hba_entry::${real_version}::${real_cluster}": + description => "check access to pg${real_version}/${real_cluster}", + port => $real_port, + target => "pg-${real_port}", + } + ### - ferm::rule::simple { "postgres::cluster::hba_entry::${pg_version}::${pg_cluster}": - description => "check access to pg${pg_version}/${pg_cluster}", - port => $pg_port, - target => "pg-${pg_port}", + if $backups and !$cluster['status']['recovery'] { + postgres::backup_cluster { "${real_version}::${real_cluster}": + pg_version => $real_version, + pg_cluster => $real_cluster, + pg_port => $real_port, + } } # hba entries and firewall rules - Postgres::Cluster::Hba_entry <<| tag == "postgres::cluster::${pg_version}::${pg_cluster}::hba::${::fqdn}" |>> - Postgres::Cluster::Hba_entry <<| tag == "postgres::cluster::${pg_port}::hba::${::fqdn}" |>> + Postgres::Cluster::Hba_entry <<| tag == "postgres::cluster::${real_version}::${real_cluster}::hba::${::fqdn}" |>> + Postgres::Cluster::Hba_entry <<| tag == "postgres::cluster::${real_port}::hba::${::fqdn}" |>> if $manage_hba { - concat { "postgres::cluster::${pg_version}::${pg_cluster}::hba": + concat { "postgres::cluster::${real_version}::${real_cluster}::hba": path => "${confdir}/pg_hba.conf", mode => '0440', group => 'postgres', ensure_newline => true, notify => Exec[$reload], } - concat::fragment{ "postgres::cluster::pg_hba-head::${pg_version}::${pg_cluster}": - target => "postgres::cluster::${pg_version}::${pg_cluster}::hba", + concat::fragment{ "postgres::cluster::pg_hba-head::${real_version}::${real_cluster}": + target => "postgres::cluster::${real_version}::${real_cluster}::hba", order => '00', content => template('postgres/cluster/pg_hba.conf-head.erb'), } - Concat::Fragment <| tag == "postgres::cluster::${pg_version}::${pg_cluster}::hba" |> + Concat::Fragment <| tag == "postgres::cluster::${real_version}::${real_cluster}::hba" |> } } diff --git a/modules/postgres/manifests/cluster/hba_entry.pp b/modules/postgres/manifests/cluster/hba_entry.pp index b708fa962..40e24f590 100644 --- a/modules/postgres/manifests/cluster/hba_entry.pp +++ b/modules/postgres/manifests/cluster/hba_entry.pp @@ -37,6 +37,8 @@ define postgres::cluster::hba_entry ( } } + # get remaining cluster info and verify consistency + ### $clusters = $facts['postgresql_clusters'] if $pg_port { $filtered = $clusters.filter |$cluster| { $cluster['port'] == $pg_port } @@ -62,6 +64,7 @@ define postgres::cluster::hba_entry ( if $pg_cluster and $pg_cluster != $real_cluster { fail("Inconsisten cluster name information: ${pg_cluster} != ${real_cluster}") } + ### if ($address) { ferm::rule::simple { "postgres::cluster::hba_entry::${name}": diff --git a/modules/roles/manifests/postgresql/cluster_bacula.pp b/modules/roles/manifests/postgresql/cluster_bacula.pp deleted file mode 100644 index 12fc33b4e..000000000 --- a/modules/roles/manifests/postgresql/cluster_bacula.pp +++ /dev/null @@ -1,15 +0,0 @@ -# -# postgresql bacula cluster -# -class roles::postgresql::cluster_bacula { - $pg_port = 5432 - $pg_cluster = 'bacula' - $pg_version = '11' - - postgres::cluster { 'bacula': - pg_version => $pg_version, - pg_cluster => $pg_cluster, - pg_port => $pg_port, - manage_hba => true, - } -} diff --git a/modules/roles/manifests/postgresql/server.pp b/modules/roles/manifests/postgresql/server.pp index 785bc0936..cd774baad 100644 --- a/modules/roles/manifests/postgresql/server.pp +++ b/modules/roles/manifests/postgresql/server.pp @@ -1,25 +1,20 @@ # # postgresql server role # -class roles::postgresql::server { - include postgres::backup_source - +# @param manage_clusters_hba manage clusters' pg_hba.conf using postgres::cluster. Eventually should should be true for every host and we can drop the param +class roles::postgresql::server( + Boolean $manage_clusters_hba = false, +) { $clusters = $facts['postgresql_clusters'] $clusters.each |$cluster| { - # Do not backup clusters that are replication targets, - # like the dak mirror or snapshot secondaries $version = $cluster['version'] $cluster_name = $cluster['cluster'] $port = $cluster['port'] - if ! $cluster['status']['recovery'] { - postgres::backup_cluster { "${::hostname}-${version}-${cluster_name}": - pg_version => $version, - pg_cluster => $cluster_name, - pg_port => $port, - } + postgres::cluster { 'bacula': + pg_version => $version, + pg_cluster => $cluster_name, + pg_port => $port, + manage_hba => $manage_clusters_hba, } } - - postgres::backup_server::register_backup_clienthost { "backup-clienthost-${::fqdn}}": - } } diff --git a/modules/salsa/manifests/database.pp b/modules/salsa/manifests/database.pp index 6a7d8c7c2..24647a3b1 100644 --- a/modules/salsa/manifests/database.pp +++ b/modules/salsa/manifests/database.pp @@ -46,6 +46,5 @@ class salsa::database inherits salsa { do_hba => true, } - postgres::backup_server::register_backup_clienthost { "backup-clienthost-${::fqdn}}": - } + include postgres::backup_server::register_backup_clienthost } -- 2.20.1