From 4c9bcc0ee4233575f3276368845b3f055bafa641 Mon Sep 17 00:00:00 2001 From: Jared Johnson Date: Wed, 18 Feb 2015 10:25:22 -0600 Subject: [PATCH] Configure but don't connect to DB in init_db() This allows us to start up QP even if e.g. Redis is down --- lib/Qpsmtpd/DB/File/DBM.pm | 16 ++++++++++++---- lib/Qpsmtpd/Plugin.pm | 10 ++++++++-- plugins/greylisting | 35 ++++++++++++++++------------------- t/plugin_tests/greylisting | 21 ++++++++++++++------- t/qpsmtpd-db-file-dbm.t | 10 ++++++++++ t/qpsmtpd-plugin.t | 22 ++++++++++++++++++++++ 6 files changed, 82 insertions(+), 32 deletions(-) diff --git a/lib/Qpsmtpd/DB/File/DBM.pm b/lib/Qpsmtpd/DB/File/DBM.pm index abedb83..c038618 100644 --- a/lib/Qpsmtpd/DB/File/DBM.pm +++ b/lib/Qpsmtpd/DB/File/DBM.pm @@ -180,15 +180,23 @@ sub flush { } sub dir { - my ( $self, @candidate_dirs ) = @_; - return $self->{dir} if $self->{dir} and ! @candidate_dirs; - push @candidate_dirs, ( $self->qphome . '/var/db', $self->qphome . '/config' ); - for my $d ( @candidate_dirs ) { + my ( $self, @arg ) = @_; + return $self->{dir} if $self->{dir} and ! @arg; + for my $d ( $self->candidate_dirs(@arg) ) { next if ! $self->validate_dir($d); return $self->{dir} = $d; # first match wins } } +sub candidate_dirs { + my ( $self, @arg ) = @_; + return @{ $self->{candidate_dirs} } if $self->{candidate_dirs} && ! @arg; + $self->{candidate_dirs} = \@arg if @arg; + push @{ $self->{candidate_dirs} }, + $self->qphome . '/var/db', $self->qphome . '/config'; + return @{ $self->{candidate_dirs} }; +} + sub validate_dir { my ( $self, $d ) = @_; return 0 if ! $d; diff --git a/lib/Qpsmtpd/Plugin.pm b/lib/Qpsmtpd/Plugin.pm index a92ca78..dd8ccf4 100644 --- a/lib/Qpsmtpd/Plugin.pm +++ b/lib/Qpsmtpd/Plugin.pm @@ -347,10 +347,16 @@ sub _register_standard_hooks { } } +sub db_args { + my ( $self, %arg ) = @_; + $self->{db_args} = \%arg if %arg; + $self->{db_args}{name} ||= $self->plugin_name; + return %{ $self->{db_args} }; +} + sub db { my ( $self, %arg ) = @_; - $arg{name} ||= $self->plugin_name; - return $self->{db} ||= Qpsmtpd::DB->new(%arg); + return $self->{db} ||= Qpsmtpd::DB->new( $self->db_args(%arg) ); } 1; diff --git a/plugins/greylisting b/plugins/greylisting index e727c8c..76b25bb 100644 --- a/plugins/greylisting +++ b/plugins/greylisting @@ -211,7 +211,7 @@ sub register { $config->{reject} = $config->{mode} =~ /testonly|off/i ? 0 : 1; } $self->{_args} = $config; - $self->init_db() or return; + $self->init_db(); $self->register_hooks(); $self->prune_db(); if ($self->{_args}{upgrade}) { @@ -233,22 +233,20 @@ sub register_hooks { sub init_db { my ($self) = @_; - return $self->init_redis if $self->{_args}{redis}; - return $self->init_dbm; + if ( $self->{_args}{redis} ) { + $self->init_redis; + } else { + $self->init_dbm; + } } sub init_redis { my ($self) = @_; - eval { - $self->db( - name => 'greylist', - class => 'Qpsmtpd::DB::Redis', - server => $self->parse_redis_server, - ) or die 'Unknown error'; - }; - return 1 if ! $@; - $self->log(LOGCRIT, "Unable to connect to redis, GREYLISTING DISABLED: $@"); - return 0; + $self->db_args( + name => 'greylist', + class => 'Qpsmtpd::DB::Redis', + server => $self->parse_redis_server, + ); } sub parse_redis_server { @@ -263,19 +261,18 @@ sub init_dbm { $self->db( name => 'greylist', class => 'Qpsmtpd::DB::File::DBM' - ) or return 0; + ); my $cdir = $self->{_args}{db_dir}; $cdir = $1 if $cdir and $cdir =~ m{^([-a-zA-Z0-9./_]+)$}; # greylisting-specific hints for where to store the greylist DB my $db_dir = $self->db->dir( $cdir, '/var/lib/qpsmtpd/greylisting' ); $self->db->nfs_locking( $self->{_args}{nfslock} ); - + # Work around old DBM filename - return 1 if -f "$db_dir/greylist.dbm"; my $oldname = 'denysoft_greylist'; - return 1 if ! -f "$db_dir/$oldname.dbm"; - $self->db->name($oldname); - return 1; + if ( ! -f "$db_dir/greylist.dbm" && -f "$db_dir/$oldname.dbm" ) { + $self->db->name($oldname); + } } sub load_exclude_files { diff --git a/t/plugin_tests/greylisting b/t/plugin_tests/greylisting index 7fd2a84..db96f73 100644 --- a/t/plugin_tests/greylisting +++ b/t/plugin_tests/greylisting @@ -333,19 +333,26 @@ sub rc { sub test_init_redis { my ($self) = @_; delete $self->{db}; - $self->{_args}{redis} = 'bogusserverasdfqwerty.:6379'; - ok( ! $self->init_redis, 'init_redis() fails on bogus server' ); - eval { Qpsmtpd::DB::Redis->new }; - return if $@; - $self->{_args}{redis} = 'localhost'; - ok( $self->init_redis, 'init_redis() succeeds when redis is up' ); + $self->{_args}{redis} = 'testredis'; + $self->init_db; + is( keyvals($self->db_args), + 'class=Qpsmtpd::DB::Redis;name=greylist;server=testredis:6379', + 'init_redis() sets redis args' ); +} + +sub keyvals { + my ( %h ) = @_; + return join ";", map { "$_=$h{$_}" } sort keys %h; } sub test_init_dbm { my ($self) = @_; delete $self->{db}; delete $self->{_args}{redis}; - ok( $self->init_db, 'init_db() works for DBM' ); + $self->init_db; + is( $self->db->name, 'greylist', 'init_dbm() sets correct db name' ); + is( $self->db->path, 't/config/greylist.dbm', 'init_dbm() sets correct path' ); + is( ref $self->db, 'Qpsmtpd::DB::File::DBM', 'init_dbm() gives DBM object' ); } sub test_parse_redis_server { diff --git a/t/qpsmtpd-db-file-dbm.t b/t/qpsmtpd-db-file-dbm.t index 38f052d..58a94a7 100644 --- a/t/qpsmtpd-db-file-dbm.t +++ b/t/qpsmtpd-db-file-dbm.t @@ -19,6 +19,7 @@ __get_keys(); __size(); __flush(); __qphome(); +__candidate_dirs(); __validate_dir(); __dir(); __untie_gotcha(); @@ -108,6 +109,15 @@ sub __qphome { is( $db->qphome, 't', 'qphome()' ); } +sub __candidate_dirs { + is( join('|', $db->candidate_dirs), 't/var/db|t/config', + 'candidate_dirs() default ' ); + is( join('|', $db->candidate_dirs('foo')), 'foo|t/var/db|t/config', + 'candidate_dirs() passed args plus defaults' ); + is( join('|', $db->candidate_dirs), 'foo|t/var/db|t/config', + 'candidate_dirs() cached values' ); +} + sub __validate_dir { is( $db->validate_dir(), 0, 'validate_dir(): false on no input' ); is( $db->validate_dir(undef), 0, 'validate_dir(): false on undef' ); diff --git a/t/qpsmtpd-plugin.t b/t/qpsmtpd-plugin.t index 7d9c605..a6596ca 100644 --- a/t/qpsmtpd-plugin.t +++ b/t/qpsmtpd-plugin.t @@ -9,11 +9,33 @@ use Test::Qpsmtpd; use_ok('Qpsmtpd::Plugin'); +__db_args(); __db(); __register_hook(); done_testing(); +sub __db_args { + my $plugin = FakePlugin->new; + is( keyvals($plugin->db_args), + 'name=___MockHook___', + 'default db args populated' ); + is( keyvals($plugin->db_args( arg1 => 1 )), + 'arg1=1;name=___MockHook___', + 'passed args in addition to defaults' ); + is( keyvals($plugin->db_args( name => 'bob', arg2 => 2 )), + 'arg2=2;name=bob', + 'passed args override defaults' ); + is( keyvals($plugin->db_args), + 'arg2=2;name=bob', + 'get previous args' ); +} + +sub keyvals { + my ( %h ) = @_; + return join ";", map { "$_=$h{$_}" } sort keys %h; +} + sub __db { my $plugin = FakePlugin->new; my $db = $plugin->db( class => 'FakeDB', name => 'testfoo' );