Skip to content

convert ERB templates to EPP #1399

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 2 commits into from
Feb 15, 2023
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
7 changes: 6 additions & 1 deletion manifests/server/instance/systemd.pp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
unit => "${service_name}.service",
owner => 'root',
group => 'root',
content => template('postgresql/systemd-override.erb'),
content => epp('postgresql/systemd-override.conf.epp', {
port => $port,
datadir => $datadir,
extra_systemd_config => $extra_systemd_config,
}
),
notify => Class['postgresql::server::service'],
before => Class['postgresql::server::reload'],
}
Expand Down
29 changes: 21 additions & 8 deletions manifests/server/pg_hba_rule.pp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# lint:ignore:140chars
# @summary This resource manages an individual rule that applies to the file defined in target.
#
# @param type Sets the type of rule.
Expand All @@ -10,13 +11,14 @@
# @param order Sets an order for placing the rule in pg_hba.conf. This can be either a string or an integer. If it is an integer, it will be converted to a string by zero-padding it to three digits. E.g. 42 will be zero-padded to the string '042'. The pg_hba_rule fragments are sorted using the alpha sorting order. Default value: 150.
# @param target Provides the target for the rule, and is generally an internal only property. Use with caution.
# @param postgresql_version Manages pg_hba.conf without managing the entire PostgreSQL instance.
# lint:endignore:140chars
define postgresql::server::pg_hba_rule (
Postgresql::Pg_hba_rule_type $type,
String $database,
String $user,
String $auth_method,
String[1] $database,
String[1] $user,
String[1] $auth_method,
Optional[Postgresql::Pg_hba_rule_address] $address = undef,
String $description = 'none',
String[1] $description = 'none',
Optional[String] $auth_option = undef,
Variant[String, Integer] $order = 150,

Expand All @@ -34,7 +36,7 @@
}

if $manage_pg_hba_conf == false {
fail('postgresql::server::manage_pg_hba_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests')
fail('postgresql::server::manage_pg_hba_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests') # lint:ignore:140chars
} else {
if($type =~ /^host/ and $address == undef) {
fail('You must specify an address property when type is host based')
Expand All @@ -48,7 +50,7 @@
}

$allowed_auth_methods = $postgresql_version ? {
'10' => ['trust', 'reject', 'scram-sha-256', 'md5', 'password', 'gss', 'sspi', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam', 'bsd'],
'10' => ['trust', 'reject', 'scram-sha-256', 'md5', 'password', 'gss', 'sspi', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam', 'bsd'], # lint:ignore:140chars
'9.6' => ['trust', 'reject', 'md5', 'password', 'gss', 'sspi', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam', 'bsd'],
'9.5' => ['trust', 'reject', 'md5', 'password', 'gss', 'sspi', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam'],
'9.4' => ['trust', 'reject', 'md5', 'password', 'gss', 'sspi', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam'],
Expand All @@ -60,7 +62,7 @@
'8.3' => ['trust', 'reject', 'md5', 'crypt', 'password', 'gss', 'sspi', 'krb5', 'ident', 'ldap', 'pam'],
'8.2' => ['trust', 'reject', 'md5', 'crypt', 'password', 'krb5', 'ident', 'ldap', 'pam'],
'8.1' => ['trust', 'reject', 'md5', 'crypt', 'password', 'krb5', 'ident', 'pam'],
default => ['trust', 'reject', 'scram-sha-256', 'md5', 'password', 'gss', 'sspi', 'krb5', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam', 'crypt', 'bsd']
default => ['trust', 'reject', 'scram-sha-256', 'md5', 'password', 'gss', 'sspi', 'krb5', 'ident', 'peer', 'ldap', 'radius', 'cert', 'pam', 'crypt', 'bsd'] # lint:ignore:140chars
}

assert_type(Enum[$allowed_auth_methods], $auth_method)
Expand All @@ -69,7 +71,18 @@
$fragname = "pg_hba_rule_${name}"
concat::fragment { $fragname:
target => $target,
content => template('postgresql/pg_hba_rule.conf'),
content => epp('postgresql/pg_hba_rule.conf.epp', {
name => $name,
description => $description,
order => $order,
type => $type,
database => $database,
user => $user,
address => $address,
auth_method => $auth_method,
auth_option => $auth_option,
}
),
order => $_order,
}
}
Expand Down
20 changes: 16 additions & 4 deletions manifests/server/pg_ident_rule.pp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@
#
# @param map_name Sets the name of the user map that is used to refer to this mapping in pg_hba.conf.
# @param system_username Specifies the operating system user name (the user name used to connect to the database).
# @param database_username Specifies the user name of the database user. The system_username is mapped to this user name.
# @param description Sets a longer description for this rule if required. This description is placed in the comments above the rule in pg_ident.conf. Default value: 'none'.
# @param database_username
# Specifies the user name of the database user.
# The system_username is mapped to this user name.
# @param description
# Sets a longer description for this rule if required.
# This description is placed in the comments above the rule in pg_ident.conf.
# @param order Defines an order for placing the mapping in pg_ident.conf. Default value: 150.
# @param target Provides the target for the rule and is generally an internal only property. Use with caution.
define postgresql::server::pg_ident_rule (
Expand All @@ -18,13 +22,21 @@
$target = $postgresql::server::pg_ident_conf_path
) {
if $postgresql::server::manage_pg_ident_conf == false {
fail('postgresql::server::manage_pg_ident_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests')
fail('postgresql::server::manage_pg_ident_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests') # lint:ignore:140chars
} else {
# Create a rule fragment
$fragname = "pg_ident_rule_${name}"
concat::fragment { $fragname:
target => $target,
content => template('postgresql/pg_ident_rule.conf'),
content => epp('postgresql/pg_ident_rule.conf.epp', {
name => $name,
description => $description,
order => $order,
map_name => $map_name,
system_username => $system_username,
database_username => $database_username,
}
),
order => $order,
}
}
Expand Down
25 changes: 22 additions & 3 deletions manifests/server/recovery.pp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# lint:ignore:140chars
# @summary This resource manages the parameters that applies to the recovery.conf template.
#
# @note
Expand All @@ -22,6 +23,7 @@
# @param trigger_file Specifies a trigger file whose presence ends recovery in the standby.
# @param recovery_min_apply_delay This parameter allows you to delay recovery by a fixed period of time, measured in milliseconds if no unit is specified.
# @param target Provides the target for the rule, and is generally an internal only property. Use with caution.
# lint:endignore:140chars
define postgresql::server::recovery (
$restore_command = undef,
$archive_cleanup_command = undef,
Expand All @@ -41,14 +43,14 @@
$target = $postgresql::server::recovery_conf_path
) {
if $postgresql::server::manage_recovery_conf == false {
fail('postgresql::server::manage_recovery_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests')
fail('postgresql::server::manage_recovery_conf has been disabled, so this resource is now unused and redundant, either enable that option or remove this resource from your manifests') # lint:ignore:140chars
} else {
if($restore_command == undef and $archive_cleanup_command == undef and $recovery_end_command == undef
and $recovery_target_name == undef and $recovery_target_time == undef and $recovery_target_xid == undef
and $recovery_target_inclusive == undef and $recovery_target == undef and $recovery_target_timeline == undef
and $pause_at_recovery_target == undef and $standby_mode == undef and $primary_conninfo == undef
and $primary_slot_name == undef and $trigger_file == undef and $recovery_min_apply_delay == undef) {
fail('postgresql::server::recovery use this resource but do not pass a parameter will avoid creating the recovery.conf, because it makes no sense.')
fail('postgresql::server::recovery use this resource but do not pass a parameter will avoid creating the recovery.conf, because it makes no sense.') # lint:ignore:140chars
}

concat { $target:
Expand All @@ -63,7 +65,24 @@
# Create the recovery.conf content
concat::fragment { "${name}-recovery.conf":
target => $target,
content => template('postgresql/recovery.conf.erb'),
content => epp('postgresql/recovery.conf.epp', {
restore_command => $restore_command,
archive_cleanup_command => $archive_cleanup_command,
recovery_end_command => $recovery_end_command,
recovery_target_name => $recovery_target_name,
recovery_target_time => $recovery_target_time,
recovery_target_xid => $recovery_target_xid,
recovery_target_inclusive => $recovery_target_inclusive,
recovery_target => $recovery_target,
recovery_target_timeline => $recovery_target_timeline,
pause_at_recovery_target => $pause_at_recovery_target,
standby_mode => $standby_mode,
primary_conninfo => $primary_conninfo,
primary_slot_name => $primary_slot_name,
trigger_file => $trigger_file,
recovery_min_apply_delay => $recovery_min_apply_delay,
}
),
}
}
}
13 changes: 13 additions & 0 deletions templates/pg_dump.sh.epp
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
<%- |
Boolean $compress,
Array $databases,
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could use Array[String[1]] $databases = [] and simplify the code. Right now it still deals with undef and it not being an Array.

Optional[String[1]] $db_user,
Boolean $delete_before_dump,
String[1] $dir,
Enum['plain','custom','directory','tar'] $format,
Array $optional_args,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this Array[String[1]] or do we also expect Integer in there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took over the current types in #1397 or these which are currently in the module. If there are adjustments over all, I would do these later.

Optional[String[1]] $post_script,
Optional[String[1]] $pre_script,
Comment on lines +9 to +10
Copy link
Collaborator

Choose a reason for hiding this comment

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

It calls flatten($pre_script) and flatten($post_script) which implies an Array is supported/expected. In addition to that, it may even be nested (given flatten is called). If you're doing a breaking change, I'd suggest Array[String[1]] and remove the flatten() call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see comment above

Integer[0] $rotate,
Stdlib::Absolutepath $success_file_path,
| -%>
<%- if $facts['kernel'] == 'Linux' { -%>
#!/bin/bash
<%- } else { -%>
Expand Down
5 changes: 0 additions & 5 deletions templates/pg_hba_rule.conf

This file was deleted.

15 changes: 15 additions & 0 deletions templates/pg_hba_rule.conf.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<%- |
String[1] $name,
String[1] $description,
Variant[String, Integer] $order,
Postgresql::Pg_hba_rule_type $type,
String[1] $database,
String[1] $user,
Optional[Postgresql::Pg_hba_rule_address] $address,
String[1] $auth_method,
Optional[String] $auth_option,
| -%>
# Rule Name: <%= $name %>
# Description: <%= $description %>
# Order: <%= $order %>
<%= $type %> <%= $database %> <%= $user %> <%= $address %> <%= $auth_method %> <%= $auth_option %>
5 changes: 0 additions & 5 deletions templates/pg_ident_rule.conf

This file was deleted.

12 changes: 12 additions & 0 deletions templates/pg_ident_rule.conf.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<%- |
String[1] $name,
String[1] $description,
String[1] $order,
String[1] $map_name,
String[1] $system_username,
String[1] $database_username,
| -%>
# Rule Name: <%= $name %>
# Description: <%= $description %>
# Order: <%= $order %>
<%= $map_name %> <%= $system_username %> <%= $database_username %>
64 changes: 64 additions & 0 deletions templates/recovery.conf.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<%- |
Optional[String] $restore_command,
Optional[String[1]] $archive_cleanup_command,
Optional[String[1]] $recovery_end_command,
Optional[String[1]] $recovery_target_name,
Optional[String[1]] $recovery_target_time,
Optional[String[1]] $recovery_target_xid,
Optional[Boolean] $recovery_target_inclusive,
Optional[String[1]] $recovery_target,
Optional[String[1]] $recovery_target_timeline,
Optional[Boolean] $pause_at_recovery_target,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I always find Optional[Boolean] interesting because it's usually equal to Boolean $var = false. Not always, since you can have a tri-state where undefined is special cased, but not here. So I'd suggest to simplify it here.

Optional[String[1]] $standby_mode,
Optional[String[1]] $primary_conninfo,
Optional[String[1]] $primary_slot_name,
Optional[String[1]] $trigger_file,
Optional[Integer] $recovery_min_apply_delay,
| -%>
<% if $restore_command { -%>
restore_command = '<%= $restore_command %>'
<% } -%>
<% if $archive_cleanup_command { -%>
archive_cleanup_command = '<%= $archive_cleanup_command %>'
<% } -%>
<% if $recovery_end_command { -%>
recovery_end_command = '<%= $recovery_end_command %>'
<% } -%>

<% if $recovery_target_name { -%>
recovery_target_name = '<%= $recovery_target_name %>'
<% } -%>
<% if $recovery_target_time { -%>
recovery_target_time = '<%= $recovery_target_time %>'
<% } -%>
<% if $recovery_target_xid { -%>
recovery_target_xid = '<%= $recovery_target_xid %>'
<% } -%>
<% if $recovery_target_inclusive { -%>
recovery_target_inclusive = <%= $recovery_target_inclusive %>
<% } -%>
<% if $recovery_target { -%>
recovery_target = '<%= $recovery_target %>'
<% } -%>
<% if $recovery_target_timeline { -%>
recovery_target_timeline = '<%= $recovery_target_timeline %>'
<% } -%>
<% if $pause_at_recovery_target { -%>
pause_at_recovery_target = <%= $pause_at_recovery_target %>
<% } -%>

<% if $standby_mode { -%>
standby_mode = <%= $standby_mode %>
<% } -%>
<% if $primary_conninfo { -%>
primary_conninfo = '<%= $primary_conninfo %>'
<% } -%>
<% if $primary_slot_name { -%>
primary_slot_name = '<%= $primary_slot_name %>'
<% } -%>
<% if $trigger_file { -%>
trigger_file = '<%= $trigger_file %>'
<% } -%>
<% if $recovery_min_apply_delay { -%>
recovery_min_apply_delay = <%= $recovery_min_apply_delay %>
<% } -%>
47 changes: 0 additions & 47 deletions templates/recovery.conf.erb

This file was deleted.

15 changes: 15 additions & 0 deletions templates/systemd-override.conf.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<%- |
Variant[String[1], Stdlib::Port] $port,
Stdlib::Absolutepath $datadir,
Optional[String[1]] $extra_systemd_config,
| -%>
[Service]
Environment=PGPORT=<%= $port %>
<%- if $facts['os']['family'] == 'Gentoo' { -%>
Environment=DATA_DIR=<%= $datadir %>
<%- } else { -%>
Environment=PGDATA=<%= $datadir %>
<%- } -%>
<% if $extra_systemd_config { -%>
<%= $extra_systemd_config %>
<% } -%>
8 changes: 0 additions & 8 deletions templates/systemd-override.erb

This file was deleted.