Skip to content

Make database validation optional #157

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
kbarber merged 1 commit into from
Jan 12, 2015
Merged

Make database validation optional #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ The name of the database instance to connect to (defaults to `puppetdb`; ignored
If true, puppetdb will use SSL to connect to the postgres database (defaults to false; ignored for `embedded` db).
Setting up proper trust- and keystores has to be managed outside of the puppetdb module.

####`database_validate`

If true, the module will attempt to connect to the database using the specified settings and fail if it is not able to do so. (defaults to true)

####`node_ttl`

The length of time a node can go without receiving any new data before it's automatically deactivated. (defaults to '0', which disables auto-deactivation). This option is supported in PuppetDB >= 1.1.0.
Expand Down
4 changes: 4 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
$database_name = $puppetdb::params::database_name,
$database_ssl = $puppetdb::params::database_ssl,
$database_listen_address = $puppetdb::params::postgres_listen_addresses,
$database_validate = $puppetdb::params::database_validate,
$node_ttl = $puppetdb::params::node_ttl,
$node_purge_ttl = $puppetdb::params::node_purge_ttl,
$report_ttl = $puppetdb::params::report_ttl,
Expand All @@ -48,6 +49,7 @@
$read_database_password = $puppetdb::params::read_database_password,
$read_database_name = $puppetdb::params::read_database_name,
$read_database_ssl = $puppetdb::params::read_database_ssl,
$read_database_validate = $puppetdb::params::read_database_validate,
$read_log_slow_statements = $puppetdb::params::read_log_slow_statements,
$read_conn_max_age = $puppetdb::params::read_conn_max_age,
$read_conn_keep_alive = $puppetdb::params::read_conn_keep_alive,
Expand Down Expand Up @@ -83,6 +85,7 @@
database_password => $database_password,
database_name => $database_name,
database_ssl => $database_ssl,
database_validate => $database_validate,
node_ttl => $node_ttl,
node_purge_ttl => $node_purge_ttl,
report_ttl => $report_ttl,
Expand All @@ -105,6 +108,7 @@
read_database_password => $read_database_password,
read_database_name => $read_database_name,
read_database_ssl => $read_database_ssl,
read_database_validate => $read_database_validate,
read_log_slow_statements => $read_log_slow_statements,
read_conn_max_age => $read_conn_max_age,
read_conn_keep_alive => $read_conn_keep_alive,
Expand Down
3 changes: 3 additions & 0 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
$database_username = 'puppetdb'
$database_password = 'puppetdb'
$database_ssl = false
$database_validate = true

# These settings manage the various auto-deactivation and auto-purge settings
$node_ttl = '0s'
Expand All @@ -47,6 +48,7 @@
$read_database_username = 'puppetdb'
$read_database_password = 'puppetdb'
$read_database_ssl = false
$read_database_validate = true
$read_log_slow_statements = '10'
$read_conn_max_age = '60'
$read_conn_keep_alive = '45'
Expand Down Expand Up @@ -98,4 +100,5 @@
$ssl_key = undef
$ssl_cert = undef
$ssl_ca_cert = undef

}
4 changes: 4 additions & 0 deletions manifests/server.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
$database_password = $puppetdb::params::database_password,
$database_name = $puppetdb::params::database_name,
$database_ssl = $puppetdb::params::database_ssl,
$database_validate = $puppetdb::params::database_validate,
$node_ttl = $puppetdb::params::node_ttl,
$node_purge_ttl = $puppetdb::params::node_purge_ttl,
$report_ttl = $puppetdb::params::report_ttl,
Expand All @@ -45,6 +46,7 @@
$read_database_password = $puppetdb::params::read_database_password,
$read_database_name = $puppetdb::params::read_database_name,
$read_database_ssl = $puppetdb::params::read_database_ssl,
$read_database_validate = $puppetdb::params::read_database_validate,
$read_log_slow_statements = $puppetdb::params::read_log_slow_statements,
$read_conn_max_age = $puppetdb::params::read_conn_max_age,
$read_conn_keep_alive = $puppetdb::params::read_conn_keep_alive,
Expand Down Expand Up @@ -122,6 +124,7 @@
database_password => $database_password,
database_name => $database_name,
database_ssl => $database_ssl,
database_validate => $database_validate,
node_ttl => $node_ttl,
node_purge_ttl => $node_purge_ttl,
report_ttl => $report_ttl,
Expand All @@ -142,6 +145,7 @@
database_password => $read_database_password,
database_name => $read_database_name,
database_ssl => $read_database_ssl,
database_validate => $read_database_validate,
log_slow_statements => $read_log_slow_statements,
conn_max_age => $read_conn_max_age,
conn_keep_alive => $read_conn_keep_alive,
Expand Down
39 changes: 23 additions & 16 deletions manifests/server/database_ini.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
$database_password = $puppetdb::params::database_password,
$database_name = $puppetdb::params::database_name,
$database_ssl = $puppetdb::params::database_ssl,
$database_validate = $puppetdb::params::database_validate,
$node_ttl = $puppetdb::params::node_ttl,
$node_purge_ttl = $puppetdb::params::node_purge_ttl,
$report_ttl = $puppetdb::params::report_ttl,
Expand All @@ -18,29 +19,35 @@
$confdir = $puppetdb::params::confdir,
) inherits puppetdb::params {

# Validate the database connection. If we can't connect, we want to fail
# and skip the rest of the configuration, so that we don't leave puppetdb
# in a broken state.
#
# NOTE:
# Because of a limitation in the postgres module this will break with
# a duplicate declaration if read and write database host+name are the
# same.
class { 'puppetdb::server::validate_db':
database => $database,
database_host => $database_host,
database_port => $database_port,
database_username => $database_username,
database_password => $database_password,
database_name => $database_name,
if str2bool($database_validate) {
# Validate the database connection. If we can't connect, we want to fail
# and skip the rest of the configuration, so that we don't leave puppetdb
# in a broken state.
#
# NOTE:
# Because of a limitation in the postgres module this will break with
# a duplicate declaration if read and write database host+name are the
# same.
class { 'puppetdb::server::validate_db':
database => $database,
database_host => $database_host,
database_port => $database_port,
database_username => $database_username,
database_password => $database_password,
database_name => $database_name,
}
}

$ini_setting_require = str2bool($database_validate) ? {
false => undef,
default => Class['puppetdb::server::validate_db'],
Copy link
Copy Markdown
Contributor

@ajroetker ajroetker Dec 16, 2014

Choose a reason for hiding this comment

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

You could avoid this entirely by setting the class's (puppetdb::server::validate_db) before param to Ini_setting <| |> ; collecting on all Ini_setting's :)

Copy link
Copy Markdown
Contributor

@kbarber kbarber Jan 12, 2015

Choose a reason for hiding this comment

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

Probably not worth doing it this time around, but worth thinking about later. If we ever get around to implementing a defined resource to wrap these ini settings or something, probably better to do it using that resource instead of inifile, since inifile could bleed into other modules and create more edges than necessary in the graph. Extraneous edges == bad sometimes, because it makes for a larger catalog.

}
# Set the defaults
Ini_setting {
path => "${confdir}/database.ini",
ensure => present,
section => 'database',
require => Class['puppetdb::server::validate_db'],
require => $ini_setting_require
}

if $database == 'embedded' {
Expand Down
39 changes: 23 additions & 16 deletions manifests/server/read_database_ini.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
$database_password = $puppetdb::params::read_database_password,
$database_name = $puppetdb::params::read_database_name,
$database_ssl = $puppetdb::params::read_database_ssl,
$database_validate = $puppetdb::params::read_database_validate,
$log_slow_statements = $puppetdb::params::read_log_slow_statements,
$conn_max_age = $puppetdb::params::read_conn_max_age,
$conn_keep_alive = $puppetdb::params::read_conn_keep_alive,
Expand All @@ -18,21 +19,23 @@

# Only add the read database configuration if database host is defined.
if $database_host != undef {
# Validate the database connection. If we can't connect, we want to fail
# and skip the rest of the configuration, so that we don't leave puppetdb
# in a broken state.
#
# NOTE:
# Because of a limitation in the postgres module this will break with
# a duplicate declaration if read and write database host+name are the
# same.
class { 'puppetdb::server::validate_read_db':
database => $database,
database_host => $database_host,
database_port => $database_port,
database_username => $database_username,
database_password => $database_password,
database_name => $database_name,
if str2bool($database_validate) {
# Validate the database connection. If we can't connect, we want to fail
# and skip the rest of the configuration, so that we don't leave puppetdb
# in a broken state.
#
# NOTE:
# Because of a limitation in the postgres module this will break with
# a duplicate declaration if read and write database host+name are the
# same.
class { 'puppetdb::server::validate_read_db':
database => $database,
database_host => $database_host,
database_port => $database_port,
database_username => $database_username,
database_password => $database_password,
database_name => $database_name,
}
}

file { "${confdir}/read_database.ini":
Expand All @@ -42,12 +45,16 @@
mode => '0600';
}

$ini_setting_require = str2bool($database_validate) ? {
false => undef,
default => Class['puppetdb::server::validate_read_db'],
Copy link
Copy Markdown
Contributor

@wkalt wkalt Jan 2, 2015

Choose a reason for hiding this comment

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

we're replacing this with puppetdb::server::validate_read_db -- was that an existing bug or is this changing behavior? ping @kbarber

Copy link
Copy Markdown
Contributor Author

@robinbowes robinbowes Jan 8, 2015

Choose a reason for hiding this comment

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

This is a change of behaviour. By all means switch to validate_read_db as necessary.

My only requirement is that I can disable validation so we can build AWS AMIs without requiring a postgresql connection.

Copy link
Copy Markdown
Contributor

@kbarber kbarber Jan 8, 2015

Choose a reason for hiding this comment

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

From the top of my head I think the change to validate_read_db is correct IMHO. If it works, I would keep it and call the old way a bug. In theory, it shouldn't fundamentally change how this works, at least I don't believe so. Users shouldn't notice unless they were hitting an ordering bug, I presume.

}
# Set the defaults
Ini_setting {
path => "${confdir}/read_database.ini",
ensure => present,
section => 'read-database',
require => Class['puppetdb::server::validate_db'],
require => $ini_setting_require,
}

if $database == 'postgres' {
Expand Down
⚡