From c2db690efb457657075c9cc5d09c97b62f4a9c18 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 02:29:42 -0400 Subject: [PATCH 01/45] Whitespace fixes --- ddclient.in | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ddclient.in b/ddclient.in index c586a4a..51c88c8 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1595,17 +1595,15 @@ sub write_recap { ## read_recap($file) - called before reading the .conf ###################################################################### sub read_recap { - my $file = shift; - my $config = shift; + my $file = shift; + my $config = shift; my $globals = {}; - %{$config} = (); if (-e $file) { my %saved = %opt; - %opt = (); + %opt = (); $saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); - %opt = %saved; - + %opt = %saved; for my $h (keys(%recap)) { next if !exists($config->{$h}); # TODO: Why is this limited to this set of variables? Why not copy every recap var @@ -1846,7 +1844,7 @@ sub _read_config { %locals = (%globals, %locals); ## override login and password if specified the old way. - $locals{'login'} = $login if defined $login; + $locals{'login'} = $login if defined $login; $locals{'password'} = $password if defined $password; ## allow {host} to be a comma separated list of hosts From cf54da50e400231b3baa83d1ade7dbb4e829acf9 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 02:26:17 -0400 Subject: [PATCH 02/45] read_recap: Invert condition (for readability) --- ddclient.in | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/ddclient.in b/ddclient.in index 51c88c8..9d9d11a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1599,23 +1599,22 @@ sub read_recap { my $config = shift; my $globals = {}; %{$config} = (); - if (-e $file) { - my %saved = %opt; - %opt = (); - $saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); - %opt = %saved; - for my $h (keys(%recap)) { - next if !exists($config->{$h}); - # TODO: Why is this limited to this set of variables? Why not copy every recap var - # defined for the host's protocol? - for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6)) { - # TODO: Isn't $config equal to \%recap here? If so, this is a no-op. What was the - # original intention behind this? To copy %recap values into %config? If so, is - # it better to just delete this and live with the current behavior (which doesn't - # seem to be causing users any problems) or to "fix" it to match the original - # intention, which might introduce a bug? - $config->{$h}{$_} = $recap{$h}{$_} if exists $recap{$h}{$_}; - } + return if !(-e $file); + my %saved = %opt; + %opt = (); + $saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); + %opt = %saved; + for my $h (keys(%recap)) { + next if !exists($config->{$h}); + # TODO: Why is this limited to this set of variables? Why not copy every recap var defined + # for the host's protocol? + for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6)) { + # TODO: Isn't $config equal to \%recap here? If so, this is a no-op. What was the + # original intention behind this? To copy %recap values into %config? If so, is it + # better to just delete this and live with the current behavior (which doesn't seem to + # be causing users any problems) or to "fix" it to match the original intention, which + # might introduce a bug? + $config->{$h}{$_} = $recap{$h}{$_} if exists $recap{$h}{$_}; } } } From 4b5f28b2f0e9c6dad653a085aba1df35b39e75f3 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 02:22:25 -0400 Subject: [PATCH 03/45] Add/update TODO comments for problematic bits of code --- ddclient.in | 99 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 17 deletions(-) diff --git a/ddclient.in b/ddclient.in index 9d9d11a..5adcd68 100755 --- a/ddclient.in +++ b/ddclient.in @@ -138,11 +138,31 @@ $ENV{'PATH'} = (exists($ENV{PATH}) ? "$ENV{PATH}:" : "") . "/sbin:/usr/sbin:/bin our %globals; our %config; -# %recap holds details about recent updates (and attempts) that are needed to implement various +# `%recap` holds details about recent updates (and attempts) that are needed to implement various # service-specific and protocol-independent mechanisms such as `min-interval`. This data is # persisted in the cache file (`--cache`) so that it survives ddclient restarts. This hash maps a -# hostname to a hashref containing those protocol variables that have their `recap` property set to -# true. +# hostname to a hashref with entries that map variable names to values. Only entries for the +# host's "recap variables" -- the host's protocol's variables with a truthy `recap` property -- are +# included. +# +# `%config` is the source of truth for recap variable values. The recap variable values in +# `%config` are initialized with values from the cache file (via `%recap`). After initialization, +# any change to a recap variable's value is made to `%config` and later copied to `%recap` before +# being written to the cache file. Changes made directly to `%recap` are erroneous because they +# will be overwritten by the value in `%config`. +# +# There are two classes of recap variables: +# * "Status" variables: These track update success/failure, the IP address of the last successful +# update, etc. These do not hold configuration data. These should always be kept in sync with +# `%config`. +# * "Configuration change detection" variables: These are used to force an update if the value in +# the same-named entry in `%config` has changed since the previous update attempt. The value +# stored in `%config` is the desired setting; the value in `%recap` is the desired setting as +# it was just before the previous update attempt. Values are synchronized from `%config` to +# `%recap` during each update attempt. +# +# The set of config change detection variables is currently hard-coded; all other recap variables +# are assumed to be status variables. # # A note about terminology: This was previously named `%cache`, but "cache" implies that the # purpose is to reduce the cost or latency of data retrieval or computation, and that deletion only @@ -642,9 +662,12 @@ our %variables = ( 'ip' => setv(T_IP, 0, 0, undef, undef), # As a recap value, this is the IPv4 address most recently saved at the DDNS service. As a # setting, this is the desired IPv4 address that should be saved at the DDNS service. - # Unfortunately, these two meanings are conflated, causing the bug "skipped: IP address was - # already set to a.b.c.d" when the IP was never set to a.b.c.d. - # TODO: Move the recap value elsewhere to fix the bug. + # TODO: The use of `ipv4` as a recap status variable is supposed to be independent of the + # use of `ipv4` as a configuration setting. Unfortunately, because the `%config` value is + # synced to `%recap`, the two uses conflict which causes the bug "skipped: IP address was + # already set to a.b.c.d" when the IP was never set to a.b.c.d. Rename the `%recap` status + # variable to something like `saved-ipv4` to avoid the collision (and confusion) with the + # `%config` setting variable. 'ipv4' => setv(T_IPV4, 0, 1, undef, undef), # As `ipv4`, but for an IPv6 address. 'ipv6' => setv(T_IPV6, 0, 1, undef, undef), @@ -1539,8 +1562,17 @@ sub write_recap { # nic_updateable (called from update_nics for each host) sets `$config{$h}{update}` # according to whether an update was attempted. # - # TODO: Why are different variables saved to the recap depending on whether an update was - # attempted or not? Shouldn't the same variables be saved every time? + # TODO: The "configuration change detection" variables are also syncronized if `$recap{$h}` + # doesn't exist. Why is this done? Perhaps the intention was to prevent the + # force-if-config-changed logic from triggering the very first cycle (when there is no + # previous update attempt). However: + # * There's a bug: This function isn't called until after `update_nics` has already + # checked the force-if-config-changed condition, so syncing the values here is too late + # to suppress the first cycle's force-if-config-changed logic. + # * It's unnecessary: If there have not been any update attempts, an update attempt + # should be made anyway, and already is. (`update_nics` already forces an update if + # `$recap{$h}` doesn't exist, and that check is done before the force-if-config-changed + # check.) if (!exists $recap{$h} || $config{$h}{'update'}) { my $vars = $protocols{opt('protocol', $h)}{variables}; for my $v (keys(%$vars)) { @@ -1548,6 +1580,8 @@ sub write_recap { $recap{$h}{$v} = opt($v, $h); } } else { + # TODO: Why aren't all "status" variables included in this update? (missing: mtime + # ipv4 ipv6 warned-min-interval warned-min-error-interval) for my $v (qw(atime wtime status-ipv4 status-ipv6)) { $recap{$h}{$v} = opt($v, $h); } @@ -1604,16 +1638,29 @@ sub read_recap { %opt = (); $saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); %opt = %saved; + # TODO: Why is this iterating over `keys(%recap)`? Shouldn't it iterate over `keys(%config)` + # instead? for my $h (keys(%recap)) { + # TODO: `$config->{$h}` is guaranteed to exist because `$config` is `\%recap` and we're + # iterating over `keys(%recap)`. next if !exists($config->{$h}); - # TODO: Why is this limited to this set of variables? Why not copy every recap var defined - # for the host's protocol? + # Only status variables are copied from `%recap` into `%config` because the recap should + # not change the configuration. See the documenting comments for `%recap`. + # TODO: Why aren't all status variables included in this update? (missing: + # warned-min-interval warned-min-error-interval) + # TODO: Hard-coding this list impairs readability and maintainability. In particular, + # different pieces of code need to know the list of status variables, and keeping them all + # in sync is error-prone (DRY). Also, different protocols might need different status + # variables. for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6)) { - # TODO: Isn't $config equal to \%recap here? If so, this is a no-op. What was the - # original intention behind this? To copy %recap values into %config? If so, is it - # better to just delete this and live with the current behavior (which doesn't seem to - # be causing users any problems) or to "fix" it to match the original intention, which - # might introduce a bug? + # TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent + # was to copy values that convey update status from `%recap` into `%config` so that + # ddclient can be restarted and resume where it left off and not violate + # `min-interval`, `min-error-interval`, etc. For the short term, this should copy the + # values into `%config`, not `%recap`. In the long term, nothing should be copied from + # `%recap` into `%config` because `%config` is not the semantically appropriate place + # to record update status. Code that currently reads/sets `$config{$h}{'status-ipv4'}` + # (and friends) should instead access `$recap{$h}{'status-ipv4'}` directly. $config->{$h}{$_} = $recap{$h}{$_} if exists $recap{$h}{$_}; } } @@ -1812,13 +1859,29 @@ sub _read_config { ## verify that keywords are valid...and check the value for my $k (keys %locals) { $locals{$k} = $passwords{$k} if defined $passwords{$k}; + # TODO: The checks below are incorrect for a few reasons: + # + # * It is not protocol-aware. Different protocols can have different sets of + # variables, with different normalization and validation behaviors. + # * It does not check for missing required values. Note that a later line or a + # command-line argument might define a missing required value. + # * A later line or command-line argument might override an invalid value, changing + # it to valid. + # + # Fixing this is not simple. Values should be checked and normalized after processing + # the entire file and command-line arguments, but then we lose line number context. + # The line number could be recorded along with each variable's value to provide context + # in case validation fails, but that adds considerable complexity. Fortunately, a + # variable's type is unlikely to change even if the protocol changes + # (`$variables{merged}{$var}{type}` will likely equal + # `$protocols{$proto}{variables}{$var}{type}` for each variable `$var` for each + # protocol `$proto`), so normalizing and validating values on a line-by-line basis is + # likely to be safe. if (!exists $variables{'merged'}{$k}) { warning("unrecognized keyword '%s' (ignored)", $k); delete $locals{$k}; next; } - # TODO: This might grab an arbitrary protocol-specific variable definition, which could - # cause surprising behavior. my $def = $variables{'merged'}{$k}; if (!eval { $locals{$k} = check_value($locals{$k}, $def); 1; }) { warning("invalid variable value '$k=$locals{$k}': $@"); @@ -3549,6 +3612,8 @@ sub nic_updateable { $config{$host}{'update'} = 1; $config{$host}{'atime'} = $now; delete($config{$host}{$_}) for qw(wtime warned-min-interval warned-min-error-interval); + # TODO: Why aren't all of the above "status" variables (the ones deleted from `%config`) + # also removed from `%recap`? (missing: wtime status status-ipv4 status-ipv6) delete $recap{$host}{'warned-min-interval'}; delete $recap{$host}{'warned-min-error-interval'}; } else { From 7181152c78ad8fb31c03ebbd9dcdcfe4552beb5d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 24 Aug 2024 17:27:27 -0400 Subject: [PATCH 04/45] cloudflare: Delete unused variable declarations --- ddclient.in | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ddclient.in b/ddclient.in index 5adcd68..2a0c680 100755 --- a/ddclient.in +++ b/ddclient.in @@ -732,14 +732,9 @@ our %protocols = ( 'examples' => \&nic_cloudflare_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, - 'backupmx' => setv(T_BOOL, 0, 1, 0, undef), 'login' => setv(T_LOGIN, 0, 0, 'token', undef), 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'mx' => setv(T_FQDN, 0, 1, undef, undef), 'server' => setv(T_FQDNP, 0, 0, 'api.cloudflare.com/client/v4', undef), - 'static' => setv(T_BOOL, 0, 1, 0, undef), - 'ttl' => setv(T_NUMBER, 0, 0, 1, undef), - 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, }, From 0a9ee106e4a01be7ea7c805390125a4da8445e41 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 13:01:02 -0400 Subject: [PATCH 05/45] tests: Debug log when in protocol `update` callback --- t/update_nics.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/t/update_nics.pl b/t/update_nics.pl index 9807703..60ef231 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -49,12 +49,16 @@ local %ddclient::protocols = ( # `ipv6`, `status-ipv4`, and `status-ipv6`.) It always succeeds. legacy => { update => sub { + ddclient::debug('in update'); for my $h (@_) { + local $ddclient::_l = ddclient::pushlogctx($h); + ddclient::debug('updating host'); push(@updates, [@_]); $ddclient::config{$h}{status} = 'good'; $ddclient::config{$h}{ip} = delete($ddclient::config{$h}{wantip}); $ddclient::config{$h}{mtime} = $ddclient::now; } + ddclient::debug('returning from update'); }, variables => { %{$ddclient::variables{'protocol-common-defaults'}}, From c943d7c0d9747d963fd4250cf3c6c67b860ebf5c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 1 Sep 2024 01:37:55 -0400 Subject: [PATCH 06/45] tests: Add some unit tests for `read_recap` --- Makefile.am | 1 + configure.ac | 2 +- ddclient.in | 2 +- t/read_recap.pl | 207 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 t/read_recap.pl diff --git a/Makefile.am b/Makefile.am index 4f0f916..a191082 100644 --- a/Makefile.am +++ b/Makefile.am @@ -78,6 +78,7 @@ handwritten_tests = \ t/protocol_directnic.pl \ t/protocol_dnsexit2.pl \ t/protocol_dyndns2.pl \ + t/read_recap.pl \ t/skip.pl \ t/ssl-validate.pl \ t/update_nics.pl \ diff --git a/configure.ac b/configure.ac index 75edda5..fa0c856 100644 --- a/configure.ac +++ b/configure.ac @@ -78,6 +78,7 @@ m4_foreach_w([_m], [ File::Spec::Functions File::Temp List::Util + Scalar::Util re ], [AX_PROG_PERL_MODULES([_m], [], [AC_MSG_WARN([some tests will fail due to missing module _m])])]) @@ -95,7 +96,6 @@ m4_foreach_w([_m], [ HTTP::Response JSON::PP LWP::UserAgent - Scalar::Util Test::MockModule Test::TCP Test::Warnings diff --git a/ddclient.in b/ddclient.in index 2a0c680..7c9e292 100755 --- a/ddclient.in +++ b/ddclient.in @@ -103,7 +103,7 @@ our $version = humanize_version($VERSION); my $programd = $0; $programd =~ s%^.*/%%; -my $program = $programd; +our $program = $programd; $program =~ s/d$//; our $now = time; my $hostname = hostname(); diff --git a/t/read_recap.pl b/t/read_recap.pl new file mode 100644 index 0000000..47df70b --- /dev/null +++ b/t/read_recap.pl @@ -0,0 +1,207 @@ +use Test::More; +use File::Temp; +use Scalar::Util qw(refaddr); +SKIP: { eval { require Test::Warnings; } or skip($@, 1); } +eval { require 'ddclient'; } or BAIL_OUT($@); + +local $ddclient::globals{debug} = 1; +local $ddclient::globals{verbose} = 1; +local %ddclient::protocols = ( + protocol_a => { + variables => { + 'host' => {type => ddclient::T_STRING(), recap => 1}, + 'mtime' => {type => ddclient::T_NUMBER(), recap => 1}, + 'atime' => {type => ddclient::T_NUMBER(), recap => 1}, + 'wtime' => {type => ddclient::T_NUMBER(), recap => 1}, + 'ip' => {type => ddclient::T_IP(), recap => 1}, + 'ipv4' => {type => ddclient::T_IPV4(), recap => 1}, + 'ipv6' => {type => ddclient::T_IPV6(), recap => 1}, + 'status' => {type => ddclient::T_ANY(), recap => 1}, + 'status-ipv4' => {type => ddclient::T_ANY(), recap => 1}, + 'status-ipv6' => {type => ddclient::T_ANY(), recap => 1}, + 'warned-min-error-interval' => {type => ddclient::T_ANY(), recap => 1}, + 'warned-min-interval' => {type => ddclient::T_ANY(), recap => 1}, + + 'var_a' => {type => ddclient::T_BOOL(), recap => 1}, + }, + }, + protocol_b => { + variables => { + 'host' => {type => ddclient::T_STRING(), recap => 1}, + 'mtime' => {type => ddclient::T_NUMBER()}, # Intentionally not a recap var. + 'var_b' => {type => ddclient::T_NUMBER(), recap => 1}, + }, + }, +); +local %ddclient::variables = + (merged => {map({ %{$ddclient::protocols{$_}{variables}}; } sort(keys(%ddclient::protocols)))}); + +# Sentinel value that means "this hash entry should be deleted." +my $DOES_NOT_EXIST = []; + +my @test_cases = ( + { + desc => "ok value", + cachefile_lines => ["var_a=yes host_a"], + want => {host_a => {host => 'host_a', var_a => 1}}, + # No config changes are expected because `var_a` is not a "status" recap var. + }, + { + desc => "unknown host", + cachefile_lines => ["var_a=yes host_c"], + want => {}, + want_TODO => "longstanding minor issue, doesn't affect functionality", + }, + { + desc => "unknown var", + cachefile_lines => ["var_b=123 host_a"], + want => {host_a => {host => 'host_a'}}, + want_TODO => "longstanding minor issue, doesn't affect functionality", + }, + { + desc => "invalid value", + cachefile_lines => ["var_a=wat host_a"], + want => {host_a => {host => 'host_a'}}, + }, + { + desc => "multiple entries", + cachefile_lines => [ + "var_a=yes host_a", + "var_b=123 host_b", + ], + want => { + host_a => {host => 'host_a', var_a => 1}, + host_b => {host => 'host_b', var_b => 123}, + }, + # No config changes are expected because `var_a` and `var_b` are not "status" recap vars. + }, + { + desc => "used to be status vars", + cachefile_lines => ["ip=192.0.2.1,status=good host_a"], + want => {host_a => {host => 'host_a', ip => '192.0.2.1', status => 'good'}}, + # No config changes are expected because `ip` and `status` are no longer "status" recap + # vars. + }, + { + desc => "status vars", + cachefile_lines => ["mtime=1234567890,atime=1234567891,wtime=1234567892,ipv4=192.0.2.1,ipv6=2001:db8::1,status-ipv4=good,status-ipv6=bad,warned-min-interval=1234567893,warned-min-error-interval=1234567894 host_a"], + want => {host_a => { + 'host' => 'host_a', + 'mtime' => 1234567890, + 'atime' => 1234567891, + 'wtime' => 1234567892, + 'ipv4' => '192.0.2.1', + 'ipv6' => '2001:db8::1', + 'status-ipv4' => 'good', + 'status-ipv6' => 'bad', + 'warned-min-interval' => 1234567893, + 'warned-min-error-interval' => 1234567894, + }}, + want_config_changes => {host_a => { + 'mtime' => 1234567890, + 'atime' => 1234567891, + 'wtime' => 1234567892, + 'ipv4' => '192.0.2.1', + 'ipv6' => '2001:db8::1', + 'status-ipv4' => 'good', + 'status-ipv6' => 'bad', + 'warned-min-interval' => 1234567893, + 'warned-min-error-interval' => 1234567894, + }}, + want_config_changes_TODO => "longstanding bug", + }, + { + desc => "unset status var clears config", + cachefile_lines => ["host_a"], + config => {host_a => { + 'mtime' => 1234567890, + 'atime' => 1234567891, + 'wtime' => 1234567892, + 'ipv4' => '192.0.2.1', + 'ipv6' => '2001:db8::1', + 'status-ipv4' => 'good', + 'status-ipv6' => 'bad', + 'warned-min-interval' => 1234567893, + 'warned-min-error-interval' => 1234567894, + 'var_a' => 1, + }}, + want => {host_a => {host => 'host_a'}}, + want_config_changes => {host_a => { + 'mtime' => $DOES_NOT_EXIST, + 'atime' => $DOES_NOT_EXIST, + 'wtime' => $DOES_NOT_EXIST, + 'ipv4' => $DOES_NOT_EXIST, + 'ipv6' => $DOES_NOT_EXIST, + 'status-ipv4' => $DOES_NOT_EXIST, + 'status-ipv6' => $DOES_NOT_EXIST, + 'warned-min-interval' => $DOES_NOT_EXIST, + 'warned-min-error-interval' => $DOES_NOT_EXIST, + # `var_a` should remain untouched. + }}, + want_config_changes_TODO => "longstanding bug", + }, + { + desc => "non-recap vars are not loaded to %recap or copied to %config", + cachefile_lines => ["mtime=1234567890 host_b"], + want => {host_b => {host => 'host_b'}}, + want_TODO => "longstanding minor issue, doesn't affect functionality", + }, + { + desc => "non-recap vars are scrubbed from %recap", + cachefile_lines => ["mtime=1234567890 host_b"], + recap => {host_b => {host => 'host_b', mtime => 1234567891}}, + want => {host_b => {host => 'host_b'}}, + want_TODO => "longstanding minor issue, doesn't affect functionality", + }, + { + desc => "unknown hosts are scrubbed from %recap", + cachefile_lines => ["host_a", "host_c"], + recap => {host_a => {host => 'host_a'}, host_c => {host => 'host_c'}}, + want => {host_a => {host => 'host_a'}}, + want_TODO => "longstanding minor issue, doesn't affect functionality", + }, +); + +for my $tc (@test_cases) { + my $cachef = File::Temp->new(); + print($cachef join('', map("$_\n", "## $ddclient::program-$ddclient::version", + @{$tc->{cachefile_lines}}))); + $cachef->close(); + local $ddclient::globals{cache} = "$cachef"; + local %ddclient::recap = %{$tc->{recap} // {}}; + my %want_config = ( + host_a => {protocol => 'protocol_a'}, + host_b => {protocol => 'protocol_b'}, + ); + $tc->{config} //= {}; + $want_config{$_} = {%{$want_config{$_} // {}}, %{$tc->{config}{$_}}} for keys(%{$tc->{config}}); + # Deep clone %want_config so we can check for changes. + local %ddclient::config; + $ddclient::config{$_} = {%{$want_config{$_}}} for keys(%want_config); + + ddclient::read_recap($cachef->filename(), \%ddclient::recap); + + TODO: { + local $TODO = $tc->{want_TODO}; + is_deeply(\%ddclient::recap, $tc->{want}, "$tc->{desc}: %recap") + or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{want}], + Names => ['*got', '*want'])); + } + TODO: { + local $TODO = $tc->{want_config_changes_TODO}; + $tc->{want_config_changes} //= {}; + $want_config{$_} = {%{$want_config{$_} // {}}, %{$tc->{want_config_changes}{$_}}} + for keys(%{$tc->{want_config_changes}}); + for my $h (keys(%want_config)) { + for my $k (keys(%{$want_config{$h}})) { + my $a = refaddr($want_config{$h}{$k}); + delete($want_config{$h}{$k}) if defined($a) && $a == refaddr($DOES_NOT_EXIST); + } + } + is_deeply(\%ddclient::config, \%want_config, "$tc->{desc}: %config") + or diag(ddclient::repr(Values => [\%ddclient::config, \%want_config], + Names => ['*got', '*want'])); + } +} + +done_testing(); From e8b3d9168b70c62538a115ebe3c23f0b3ffcd8b7 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 15 Jun 2024 19:05:33 -0400 Subject: [PATCH 07/45] Remove unnecessary variables from the recap The logic does not use the persisted values so they do not need to be persisted. --- ddclient.in | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ddclient.in b/ddclient.in index 7c9e292..ca24ba5 100755 --- a/ddclient.in +++ b/ddclient.in @@ -773,7 +773,7 @@ our %protocols = ( 'variables' => { %{$variables{'protocol-common-defaults'}}, 'min-error-interval' => setv(T_DELAY, 0, 0, interval('8m'), 0), - 'script' => setv(T_STRING, 0, 1, '/special/api.php', undef), + 'script' => setv(T_STRING, 0, 0, '/special/api.php', undef), 'server' => setv(T_FQDNP, 0, 0, 'dinahosting.com', undef), }, }, @@ -793,7 +793,7 @@ our %protocols = ( 'examples' => \&nic_dnsmadeeasy_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, - 'script' => setv(T_STRING, 0, 1, '/servlet/updateip', undef), + 'script' => setv(T_STRING, 0, 0, '/servlet/updateip', undef), 'server' => setv(T_FQDNP, 0, 0, 'cp.dnsmadeeasy.com', undef), }, }, @@ -845,7 +845,7 @@ our %protocols = ( 'variables' => { %{$variables{'protocol-common-defaults'}}, %{$variables{'dyndns-common-defaults'}}, - 'script' => setv(T_STRING, 0, 1, '/nic/update', undef), + 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), }, }, 'easydns' => { @@ -859,7 +859,7 @@ our %protocols = ( 'min-interval' => setv(T_DELAY, 0, 0, interval('10m'), 0), 'mx' => setv(T_FQDN, 0, 1, undef, undef), 'server' => setv(T_FQDNP, 0, 0, 'api.cp.easydns.com', undef), - 'script' => setv(T_STRING, 0, 1, '/dyn/generic.php', undef), + 'script' => setv(T_STRING, 0, 0, '/dyn/generic.php', undef), 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), }, }, @@ -889,7 +889,7 @@ our %protocols = ( 'login' => undef, 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), 'server' => setv(T_FQDNP, 0, 0, 'api.gandi.net', undef), - 'script' => setv(T_STRING, 0, 1, '/v5', undef), + 'script' => setv(T_STRING, 0, 0, '/v5', undef), 'use-personal-access-token' => setv(T_BOOL, 0, 0, 0, undef), 'ttl' => setv(T_DELAY, 0, 0, undef, interval('5m')), 'zone' => setv(T_FQDN, 1, 0, undef, undef), @@ -973,7 +973,7 @@ our %protocols = ( %{$variables{'protocol-common-defaults'}}, 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'njal.la', undef), - 'quietreply' => setv(T_BOOL, 0, 1, 0, undef), + 'quietreply' => setv(T_BOOL, 0, 0, 0, undef), }, }, 'noip' => { @@ -990,9 +990,9 @@ our %protocols = ( 'variables' => { %{$variables{'protocol-common-defaults'}}, 'login' => setv(T_LOGIN, 0, 0, '/usr/bin/nsupdate', undef), - 'tcp' => setv(T_BOOL, 0, 1, 0, undef), - 'ttl' => setv(T_NUMBER, 0, 1, 600, undef), - 'zone' => setv(T_STRING, 1, 1, undef, undef), + 'tcp' => setv(T_BOOL, 0, 0, 0, undef), + 'ttl' => setv(T_NUMBER, 0, 0, 600, undef), + 'zone' => setv(T_STRING, 1, 0, undef, undef), }, }, 'ovh' => { @@ -1000,7 +1000,7 @@ our %protocols = ( 'examples' => \&nic_ovh_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, - 'script' => setv(T_STRING, 0, 1, '/nic/update', undef), + 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), 'server' => setv(T_FQDNP, 0, 0, 'www.ovh.com', undef), }, }, From ce1bcaa68bc9925dcc4ecccd7147929fbe7ec555 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 16:50:56 -0400 Subject: [PATCH 08/45] nic_updateable: Set `warned-min-*` in `%config`, not `%recap` Currently the semantics of recap variables are that values are updated in `%config` and propagate to `%recap`. Before this commit, `warned-min-interval` and `warned-min-error-interval` were set in `%recap` instead of `%config`, meaning if they followed the semantics they would be overwritten or deleted when synced with `%config`. Now the values are set in `%config` to match the behavior of other recap variables. --- ddclient.in | 30 ++++++++++++++---------------- t/update_nics.pl | 6 ++++++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/ddclient.in b/ddclient.in index ca24ba5..b06f8c1 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1576,8 +1576,9 @@ sub write_recap { } } else { # TODO: Why aren't all "status" variables included in this update? (missing: mtime - # ipv4 ipv6 warned-min-interval warned-min-error-interval) - for my $v (qw(atime wtime status-ipv4 status-ipv6)) { + # ipv4 ipv6) + for my $v (qw(atime wtime status-ipv4 status-ipv6 + warned-min-interval warned-min-error-interval)) { $recap{$h}{$v} = opt($v, $h); } } @@ -1641,13 +1642,12 @@ sub read_recap { next if !exists($config->{$h}); # Only status variables are copied from `%recap` into `%config` because the recap should # not change the configuration. See the documenting comments for `%recap`. - # TODO: Why aren't all status variables included in this update? (missing: - # warned-min-interval warned-min-error-interval) # TODO: Hard-coding this list impairs readability and maintainability. In particular, # different pieces of code need to know the list of status variables, and keeping them all # in sync is error-prone (DRY). Also, different protocols might need different status # variables. - for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6)) { + for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6 + warned-min-interval warned-min-error-interval)) { # TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent # was to copy values that convey update status from `%recap` into `%config` so that # ddclient can be restarted and resume where it left off and not violate @@ -3533,14 +3533,14 @@ sub nic_updateable { if (($recap{$host}{'status-ipv4'} // '') eq 'good' && !interval_expired($host, 'mtime', 'min-interval')) { warning("skipped update from $previpv4 to $ipv4 because it has been less than $prettyi{'min-interval'} since the previous update (on $prettyt{'mtime'})") - if opt('verbose') || !($recap{$host}{'warned-min-interval'} // 0); + if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); - $recap{$host}{'warned-min-interval'} = $now; + $config{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv4'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { - if (opt('verbose') || (!$recap{$host}{'warned-min-error-interval'} && + if (opt('verbose') || (!$config{$host}{'warned-min-error-interval'} && ($warned_ipv4{$host} // 0) < $inv_ip_warn_count)) { warning("skipped update from $previpv4 to $ipv4 because it has been less than $prettyi{'min-error-interval'} since the previous update attempt (on $prettyt{'atime'}), which failed"); if (!$ipv4 && !opt('verbose')) { @@ -3550,7 +3550,7 @@ sub nic_updateable { } } - $recap{$host}{'warned-min-error-interval'} = $now; + $config{$host}{'warned-min-error-interval'} = $now; } else { $update = 1; @@ -3560,14 +3560,14 @@ sub nic_updateable { if (($recap{$host}{'status-ipv6'} // '') eq 'good' && !interval_expired($host, 'mtime', 'min-interval')) { warning("skipped update from $previpv6 to $ipv6 because it has been less than $prettyi{'min-interval'} since the previous update (on $prettyt{'mtime'})") - if opt('verbose') || !($recap{$host}{'warned-min-interval'} // 0); + if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); - $recap{$host}{'warned-min-interval'} = $now; + $config{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv6'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { - if (opt('verbose') || (!$recap{$host}{'warned-min-error-interval'} && + if (opt('verbose') || (!$config{$host}{'warned-min-error-interval'} && ($warned_ipv6{$host} // 0) < $inv_ip_warn_count)) { warning("skipped update from $previpv6 to $ipv6 because it has been less than $prettyi{'min-error-interval'} since the previous update attempt (on $prettyt{'atime'}, which failed"); if (!$ipv6 && !opt('verbose')) { @@ -3577,7 +3577,7 @@ sub nic_updateable { } } - $recap{$host}{'warned-min-error-interval'} = $now; + $config{$host}{'warned-min-error-interval'} = $now; } else { $update = 1; @@ -3608,9 +3608,7 @@ sub nic_updateable { $config{$host}{'atime'} = $now; delete($config{$host}{$_}) for qw(wtime warned-min-interval warned-min-error-interval); # TODO: Why aren't all of the above "status" variables (the ones deleted from `%config`) - # also removed from `%recap`? (missing: wtime status status-ipv4 status-ipv6) - delete $recap{$host}{'warned-min-interval'}; - delete $recap{$host}{'warned-min-error-interval'}; + # also removed from `%recap`? } else { for (qw(status-ipv4 status-ipv6)) { $config{$host}{$_} = $recap{$host}{$_} if defined($recap{$host}{$_}); diff --git a/t/update_nics.pl b/t/update_nics.pl index 60ef231..c7f8a9c 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -211,6 +211,9 @@ my @test_cases = ( want_recap_changes => { 'warned-min-interval' => $ddclient::now, }, + want_cfg_changes => { + 'warned-min-interval' => $ddclient::now, + }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -262,6 +265,9 @@ my @test_cases = ( want_recap_changes => { 'warned-min-error-interval' => $ddclient::now, }, + want_cfg_changes => { + 'warned-min-error-interval' => $ddclient::now, + }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), From 65d247321363cb05c37f928c3c01eccbeab14ce3 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 24 Aug 2024 17:01:03 -0400 Subject: [PATCH 09/45] read_recap: Don't load `ip` from recap This should have been part of commit de5d894c914d6ff707e30db37d999a2d00496573 but I forgot. --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index b06f8c1..c907dc4 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1646,7 +1646,7 @@ sub read_recap { # different pieces of code need to know the list of status variables, and keeping them all # in sync is error-prone (DRY). Also, different protocols might need different status # variables. - for (qw(atime mtime wtime ip ipv4 ipv6 status-ipv4 status-ipv6 + for (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { # TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent # was to copy values that convey update status from `%recap` into `%config` so that From 31740006d0ed93179bd495e7b81baa758d380065 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 27 Aug 2024 19:28:57 -0400 Subject: [PATCH 10/45] read_recap: Use a named loop variable (for readability) --- ddclient.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ddclient.in b/ddclient.in index c907dc4..b81d1b4 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1646,8 +1646,8 @@ sub read_recap { # different pieces of code need to know the list of status variables, and keeping them all # in sync is error-prone (DRY). Also, different protocols might need different status # variables. - for (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 - warned-min-interval warned-min-error-interval)) { + for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 + warned-min-interval warned-min-error-interval)) { # TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent # was to copy values that convey update status from `%recap` into `%config` so that # ddclient can be restarted and resume where it left off and not violate @@ -1656,7 +1656,7 @@ sub read_recap { # `%recap` into `%config` because `%config` is not the semantically appropriate place # to record update status. Code that currently reads/sets `$config{$h}{'status-ipv4'}` # (and friends) should instead access `$recap{$h}{'status-ipv4'}` directly. - $config->{$h}{$_} = $recap{$h}{$_} if exists $recap{$h}{$_}; + $config->{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); } } } From 35cbc8d20049b28da7e52a436c7170263d5c79a6 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 02:30:21 -0400 Subject: [PATCH 11/45] read_recap: Reference `%recap` directly (for readability) There's no need to pass a reference to `%recap` as an argument when that is the only way `read_recap` is ever used. --- ddclient.in | 30 ++++++++++++++---------------- t/read_recap.pl | 2 +- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/ddclient.in b/ddclient.in index b81d1b4..03e6735 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1286,7 +1286,7 @@ sub main { %opt = %saved_opt; read_config(opt('file'), \%config, \%globals); init_config(); - read_recap(opt('cache'), \%recap); + read_recap(opt('cache')); print_info() if opt('debug') && opt('verbose'); $daemon = opt('daemon'); @@ -1626,20 +1626,18 @@ sub write_recap { ###################################################################### sub read_recap { my $file = shift; - my $config = shift; my $globals = {}; - %{$config} = (); + %recap = (); return if !(-e $file); my %saved = %opt; %opt = (); - $saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); + $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file); %opt = %saved; # TODO: Why is this iterating over `keys(%recap)`? Shouldn't it iterate over `keys(%config)` # instead? for my $h (keys(%recap)) { - # TODO: `$config->{$h}` is guaranteed to exist because `$config` is `\%recap` and we're - # iterating over `keys(%recap)`. - next if !exists($config->{$h}); + # TODO: `$recap{$h}` is guaranteed to exist because we're iterating over `keys(%recap)`. + next if !exists($recap{$h}); # Only status variables are copied from `%recap` into `%config` because the recap should # not change the configuration. See the documenting comments for `%recap`. # TODO: Hard-coding this list impairs readability and maintainability. In particular, @@ -1648,15 +1646,15 @@ sub read_recap { # variables. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - # TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent - # was to copy values that convey update status from `%recap` into `%config` so that - # ddclient can be restarted and resume where it left off and not violate - # `min-interval`, `min-error-interval`, etc. For the short term, this should copy the - # values into `%config`, not `%recap`. In the long term, nothing should be copied from - # `%recap` into `%config` because `%config` is not the semantically appropriate place - # to record update status. Code that currently reads/sets `$config{$h}{'status-ipv4'}` - # (and friends) should instead access `$recap{$h}{'status-ipv4'}` directly. - $config->{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); + # TODO: This is a no-op. I believe the original intent was to copy values that convey + # update status from `%recap` into `%config` so that ddclient can be restarted and + # resume where it left off and not violate `min-interval`, `min-error-interval`, etc. + # For the short term, this should copy the values into `%config`, not `%recap`. In the + # long term, nothing should be copied from `%recap` into `%config` because `%config` is + # not the semantically appropriate place to record update status. Code that currently + # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access + # `$recap{$h}{'status-ipv4'}` directly. + $recap{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); } } } diff --git a/t/read_recap.pl b/t/read_recap.pl index 47df70b..5577738 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -179,7 +179,7 @@ for my $tc (@test_cases) { local %ddclient::config; $ddclient::config{$_} = {%{$want_config{$_}}} for keys(%want_config); - ddclient::read_recap($cachef->filename(), \%ddclient::recap); + ddclient::read_recap($cachef->filename()); TODO: { local $TODO = $tc->{want_TODO}; From fbd7167b94ab6108b12309a404c6df9e9489e8d7 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 02:56:50 -0400 Subject: [PATCH 12/45] read_recap: Fix iteration over hosts --- ddclient.in | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ddclient.in b/ddclient.in index 03e6735..3bf4a7a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1633,11 +1633,8 @@ sub read_recap { %opt = (); $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file); %opt = %saved; - # TODO: Why is this iterating over `keys(%recap)`? Shouldn't it iterate over `keys(%config)` - # instead? for my $h (keys(%recap)) { - # TODO: `$recap{$h}` is guaranteed to exist because we're iterating over `keys(%recap)`. - next if !exists($recap{$h}); + next if !exists($config{$h}); # Only status variables are copied from `%recap` into `%config` because the recap should # not change the configuration. See the documenting comments for `%recap`. # TODO: Hard-coding this list impairs readability and maintainability. In particular, From c9cdb9608666cdcf044dfa3bf3a80f68bd12b991 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 03:16:32 -0400 Subject: [PATCH 13/45] read_recap: Fix copying of recap values into `%config` --- ddclient.in | 14 +++++--------- t/read_recap.pl | 3 ++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/ddclient.in b/ddclient.in index 3bf4a7a..743993f 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1641,17 +1641,13 @@ sub read_recap { # different pieces of code need to know the list of status variables, and keeping them all # in sync is error-prone (DRY). Also, different protocols might need different status # variables. + # TODO: None of the recap variables should be copied into `%config` because `%config` is + # not the semantically appropriate place to record update status. Code that currently + # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access + # `$recap{$h}{'status-ipv4'}` directly. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - # TODO: This is a no-op. I believe the original intent was to copy values that convey - # update status from `%recap` into `%config` so that ddclient can be restarted and - # resume where it left off and not violate `min-interval`, `min-error-interval`, etc. - # For the short term, this should copy the values into `%config`, not `%recap`. In the - # long term, nothing should be copied from `%recap` into `%config` because `%config` is - # not the semantically appropriate place to record update status. Code that currently - # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access - # `$recap{$h}{'status-ipv4'}` directly. - $recap{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); + $config{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); } } } diff --git a/t/read_recap.pl b/t/read_recap.pl index 5577738..ebec050 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -108,7 +108,6 @@ my @test_cases = ( 'warned-min-interval' => 1234567893, 'warned-min-error-interval' => 1234567894, }}, - want_config_changes_TODO => "longstanding bug", }, { desc => "unset status var clears config", @@ -145,6 +144,7 @@ my @test_cases = ( cachefile_lines => ["mtime=1234567890 host_b"], want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", + want_config_changes_TODO => "longstanding bug", }, { desc => "non-recap vars are scrubbed from %recap", @@ -152,6 +152,7 @@ my @test_cases = ( recap => {host_b => {host => 'host_b', mtime => 1234567891}}, want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", + want_config_changes_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "unknown hosts are scrubbed from %recap", From 989f8be8c3cd3338da284491e35851d4f7e79435 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 03:19:45 -0400 Subject: [PATCH 14/45] read_recap: Delete from `%config` any status values missing from recap --- ddclient.in | 8 +++++++- t/read_recap.pl | 1 - 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 743993f..3ddd4a5 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1633,6 +1633,7 @@ sub read_recap { %opt = (); $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file); %opt = %saved; + $recap{$_} //= {} for keys(%config); for my $h (keys(%recap)) { next if !exists($config{$h}); # Only status variables are copied from `%recap` into `%config` because the recap should @@ -1647,8 +1648,13 @@ sub read_recap { # `$recap{$h}{'status-ipv4'}` directly. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - $config{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); + if (exists($recap{$h}{$v})) { + $config{$h}{$v} = $recap{$h}{$v}; + } else { + delete($config{$h}{$v}); + } } + delete($recap{$h}) if !%{$recap{$h}}; } } ###################################################################### diff --git a/t/read_recap.pl b/t/read_recap.pl index ebec050..240bb8b 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -137,7 +137,6 @@ my @test_cases = ( 'warned-min-error-interval' => $DOES_NOT_EXIST, # `var_a` should remain untouched. }}, - want_config_changes_TODO => "longstanding bug", }, { desc => "non-recap vars are not loaded to %recap or copied to %config", From 8359eff6ea7676a5877f05547054b4caa9e0006d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 22 Aug 2024 03:21:20 -0400 Subject: [PATCH 15/45] read_recap: Check variable definedness, not existence --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 3ddd4a5..ce445fb 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1648,7 +1648,7 @@ sub read_recap { # `$recap{$h}{'status-ipv4'}` directly. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - if (exists($recap{$h}{$v})) { + if (defined($recap{$h}{$v})) { $config{$h}{$v} = $recap{$h}{$v}; } else { delete($config{$h}{$v}); From 70e2b5137753a57afa7533b12c1a5fb365042f33 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 02:53:18 -0400 Subject: [PATCH 16/45] read_recap: Don't copy non-recap values to `%config` --- ddclient.in | 2 ++ t/read_recap.pl | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index ce445fb..bc6fdb9 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1636,6 +1636,7 @@ sub read_recap { $recap{$_} //= {} for keys(%config); for my $h (keys(%recap)) { next if !exists($config{$h}); + my $vars = $protocols{opt('protocol', $h)}{variables}; # Only status variables are copied from `%recap` into `%config` because the recap should # not change the configuration. See the documenting comments for `%recap`. # TODO: Hard-coding this list impairs readability and maintainability. In particular, @@ -1648,6 +1649,7 @@ sub read_recap { # `$recap{$h}{'status-ipv4'}` directly. for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { + next if !$vars->{$v} || !$vars->{$v}{recap}; if (defined($recap{$h}{$v})) { $config{$h}{$v} = $recap{$h}{$v}; } else { diff --git a/t/read_recap.pl b/t/read_recap.pl index 240bb8b..cafabd3 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -143,7 +143,6 @@ my @test_cases = ( cachefile_lines => ["mtime=1234567890 host_b"], want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", - want_config_changes_TODO => "longstanding bug", }, { desc => "non-recap vars are scrubbed from %recap", @@ -151,7 +150,6 @@ my @test_cases = ( recap => {host_b => {host => 'host_b', mtime => 1234567891}}, want => {host_b => {host => 'host_b'}}, want_TODO => "longstanding minor issue, doesn't affect functionality", - want_config_changes_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "unknown hosts are scrubbed from %recap", From f2c9ef66418cdee50f6bb708a16f9e1f7a92e7cd Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 1 Sep 2024 03:06:46 -0400 Subject: [PATCH 17/45] read_recap: Scrub recap values without var declarations --- ddclient.in | 8 +++++++- t/read_recap.pl | 5 ----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ddclient.in b/ddclient.in index bc6fdb9..86b86d6 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1635,8 +1635,14 @@ sub read_recap { %opt = %saved; $recap{$_} //= {} for keys(%config); for my $h (keys(%recap)) { - next if !exists($config{$h}); + if (!exists($config{$h})) { + delete($recap{$h}); + next; + } my $vars = $protocols{opt('protocol', $h)}{variables}; + for my $v (keys(%{$recap{$h}})) { + delete($recap{$h}{$v}) if !$vars->{$v} || !$vars->{$v}{recap}; + } # Only status variables are copied from `%recap` into `%config` because the recap should # not change the configuration. See the documenting comments for `%recap`. # TODO: Hard-coding this list impairs readability and maintainability. In particular, diff --git a/t/read_recap.pl b/t/read_recap.pl index cafabd3..b601eae 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -50,13 +50,11 @@ my @test_cases = ( desc => "unknown host", cachefile_lines => ["var_a=yes host_c"], want => {}, - want_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "unknown var", cachefile_lines => ["var_b=123 host_a"], want => {host_a => {host => 'host_a'}}, - want_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "invalid value", @@ -142,21 +140,18 @@ my @test_cases = ( desc => "non-recap vars are not loaded to %recap or copied to %config", cachefile_lines => ["mtime=1234567890 host_b"], want => {host_b => {host => 'host_b'}}, - want_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "non-recap vars are scrubbed from %recap", cachefile_lines => ["mtime=1234567890 host_b"], recap => {host_b => {host => 'host_b', mtime => 1234567891}}, want => {host_b => {host => 'host_b'}}, - want_TODO => "longstanding minor issue, doesn't affect functionality", }, { desc => "unknown hosts are scrubbed from %recap", cachefile_lines => ["host_a", "host_c"], recap => {host_a => {host => 'host_a'}, host_c => {host => 'host_c'}}, want => {host_a => {host => 'host_a'}}, - want_TODO => "longstanding minor issue, doesn't affect functionality", }, ); From c64e432bf106ba6d054aad19e3ae260bd413e6b2 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 01:35:48 -0400 Subject: [PATCH 18/45] write_recap: Update all status recap vars when writing recap --- ddclient.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ddclient.in b/ddclient.in index 86b86d6..f7b807b 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1575,9 +1575,7 @@ sub write_recap { $recap{$h}{$v} = opt($v, $h); } } else { - # TODO: Why aren't all "status" variables included in this update? (missing: mtime - # ipv4 ipv6) - for my $v (qw(atime wtime status-ipv4 status-ipv6 + for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { $recap{$h}{$v} = opt($v, $h); } From 94ce6367ec28b86180d0779a7e2f3accb73630d4 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 00:58:56 -0400 Subject: [PATCH 19/45] write_recap: Also clear out non-recap and stale values Before, if a non-`undef` value was in `%recap` and the corresponding value in `%config` became `undef`, the `%recap` value would remain untouched. Now it is deleted to match `%config`. Also, any `%recap` values without a corresponding recap variable declaration are deleted. --- ddclient.in | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/ddclient.in b/ddclient.in index f7b807b..5c14788 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1554,9 +1554,15 @@ sub write_recap { my ($file) = @_; for my $h (keys %config) { + my $vars = $protocols{opt('protocol', $h)}{variables}; # nic_updateable (called from update_nics for each host) sets `$config{$h}{update}` # according to whether an update was attempted. # + # Entries in `%recap` with `undef` values are deleted to avoid needing to figure out how to + # represent `undef` in the cache file and to simplify testing. Also, entries for non-recap + # variables (which can happen after changing a host's protocol or after switching to a + # different version of ddclient) are deleted. + # # TODO: The "configuration change detection" variables are also syncronized if `$recap{$h}` # doesn't exist. Why is this done? Perhaps the intention was to prevent the # force-if-config-changed logic from triggering the very first cycle (when there is no @@ -1569,7 +1575,7 @@ sub write_recap { # `$recap{$h}` doesn't exist, and that check is done before the force-if-config-changed # check.) if (!exists $recap{$h} || $config{$h}{'update'}) { - my $vars = $protocols{opt('protocol', $h)}{variables}; + $recap{$h} = {}; for my $v (keys(%$vars)) { next if !$vars->{$v}{recap} || !defined(opt($v, $h)); $recap{$h}{$v} = opt($v, $h); @@ -1577,14 +1583,13 @@ sub write_recap { } else { for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 warned-min-interval warned-min-error-interval)) { - $recap{$h}{$v} = opt($v, $h); + if ($vars->{$v} && $vars->{$v}{recap} && defined(my $val = opt($v, $h))) { + $recap{$h}{$v} = $val; + } else { + delete($recap{$h}{$v}); + } } } - # Clear out entries with `undef` values to avoid needing to figure out how to represent - # `undef` in the cache file and to simplify testing. - for my $v (keys(%{$recap{$h}})) { - delete($recap{$h}{$v}) if !defined($recap{$h}{$v}); - } } my $recap = ""; @@ -3610,8 +3615,6 @@ sub nic_updateable { $config{$host}{'update'} = 1; $config{$host}{'atime'} = $now; delete($config{$host}{$_}) for qw(wtime warned-min-interval warned-min-error-interval); - # TODO: Why aren't all of the above "status" variables (the ones deleted from `%config`) - # also removed from `%recap`? } else { for (qw(status-ipv4 status-ipv6)) { $config{$host}{$_} = $recap{$host}{$_} if defined($recap{$host}{$_}); From 499318fbe029691e8e6c9513774b0919ad4ff9aa Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 24 Aug 2024 23:27:03 -0400 Subject: [PATCH 20/45] update_nics: Always overwrite `status-ipv*` with value from `status` --- ddclient.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 5c14788..9ce8077 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1489,9 +1489,8 @@ sub update_nics { if (defined(my $vstatus = $config{$h}{"status-ipv$ipv"})) { warning("ddclient bug: legacy protocol set both 'status' (to '$status') " . "and 'status-ipv$ipv' (to '$vstatus')"); - } else { - $config{$h}{"status-ipv$ipv"} = $status; } + $config{$h}{"status-ipv$ipv"} = $status; # TODO: See above comment for $ip_option. This is the same situation, but for # 'ipv4' and 'ipv6'. my $vip_option = opt("usev$ipv", $h) eq "ipv$ipv"; From 78be40fe2c91b4e97434cbe6d6ecc466803d2b5c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 24 Aug 2024 23:29:46 -0400 Subject: [PATCH 21/45] update_nics: Remove unnecessary assertions These just add cold code paths and impair readability and maintainability. --- ddclient.in | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ddclient.in b/ddclient.in index 9ce8077..5fdf82d 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1470,11 +1470,6 @@ sub update_nics { my $status = delete($config{$h}{'status'}) || next; my $ip = $config{$h}{'ip'}; my $ipv = is_ipv4($ip) ? '4' : is_ipv6($ip) ? '6' : undef; - if (!defined($ipv)) { - warning("ddclient bug: legacy protocol set 'status' but did not set 'ip' " . - "to an IPv4 or IPv6 address: " . ($ip // '')); - next; - } # TODO: Currently $config{$h}{'ip'} is used for two distinct purposes: it holds the # value of the --ip option, and it is updated by legacy protocols to hold the new # IP address after an update. Fortunately, the --ip option is not used very often, @@ -1486,10 +1481,6 @@ sub update_nics { my $ip_option = opt('use', $h) eq 'ip' || opt('usev6', $h) eq 'ip'; delete($config{$h}{'ip'}) if !$ip_option; debug("legacy protocol; moving status to status-ipv$ipv and ip to ipv$ipv"); - if (defined(my $vstatus = $config{$h}{"status-ipv$ipv"})) { - warning("ddclient bug: legacy protocol set both 'status' (to '$status') " . - "and 'status-ipv$ipv' (to '$vstatus')"); - } $config{$h}{"status-ipv$ipv"} = $status; # TODO: See above comment for $ip_option. This is the same situation, but for # 'ipv4' and 'ipv6'. From cb668700197085a1b50a9c968c874d5a622f0a6e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sat, 24 Aug 2024 23:55:54 -0400 Subject: [PATCH 22/45] update_nics: Refine debug message for consistency/readability --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 5fdf82d..f829684 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1480,7 +1480,7 @@ sub update_nics { # configuration, not update status (same goes for 'status', 'mtime', etc.). my $ip_option = opt('use', $h) eq 'ip' || opt('usev6', $h) eq 'ip'; delete($config{$h}{'ip'}) if !$ip_option; - debug("legacy protocol; moving status to status-ipv$ipv and ip to ipv$ipv"); + debug("legacy protocol; moving 'status' to 'status-ipv$ipv', 'ip' to 'ipv$ipv'"); $config{$h}{"status-ipv$ipv"} = $status; # TODO: See above comment for $ip_option. This is the same situation, but for # 'ipv4' and 'ipv6'. From 20439bc130ad0508c7e4803951159124e2d57558 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 25 Aug 2024 00:01:39 -0400 Subject: [PATCH 23/45] update_nics: Refine comment --- ddclient.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index f829684..f9b655e 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1489,8 +1489,9 @@ sub update_nics { debug("unable to update 'ipv$ipv' to '$ip' " . "because it is already set to '$vip'") if $vip_option; } else { - # A previous update will have set "ipv$ipv" if !$vip_option, so it's safe to - # overwrite it here because $vip_option was checked above. + # A previous update would have moved `$config{$h}{'ip'}` to + # `$config{$h}{"ipv$ipv"}`, so it is OK to overwrite `$config{$h}{"ipv$ipv"}` + # if it is already set. debug("updating 'ipv$ipv' from '$vip' to '$ip'") if defined($vip) && $vip ne $ip; $config{$h}{"ipv$ipv"} = $ip; From c5df774b7e6af66821b647ba27ba325a3e609b04 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 00:32:11 -0400 Subject: [PATCH 24/45] update_nics: Change `|| next` to `or next` (for readability) --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index f9b655e..30fb94f 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1467,7 +1467,7 @@ sub update_nics { # version-specific equivalents. for my $h (@hosts) { local $_l = pushlogctx($h); - my $status = delete($config{$h}{'status'}) || next; + my $status = delete($config{$h}{'status'}) or next; my $ip = $config{$h}{'ip'}; my $ipv = is_ipv4($ip) ? '4' : is_ipv6($ip) ? '6' : undef; # TODO: Currently $config{$h}{'ip'} is used for two distinct purposes: it holds the From bf83ba032c30b163e4da195f04bcfcbec1f5992b Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 13:18:20 -0400 Subject: [PATCH 25/45] update_nics: Move legacy `wantip` assignment to `update` call This consolidates the legacy support with other legacy support logic, which will make it easier to refactor in a future commit. --- ddclient.in | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ddclient.in b/ddclient.in index 30fb94f..4ac3927 100755 --- a/ddclient.in +++ b/ddclient.in @@ -685,10 +685,10 @@ our %variables = ( # TODO: Create a timestamp type and change this to that type. 'atime' => setv(T_NUMBER,0, 1, 0, undef), # Disposition of the most recent (or currently in progress) attempt to update the DDNS - # service with the IP address in `wantipv4` (or `wantip`, if an IPv4 address). Anything - # other than `good`, including undef, is treated as a failure. + # service with the IP address in `wantipv4`. Anything other than `good`, including undef, + # is treated as a failure. 'status-ipv4' => setv(T_ANY, 0, 1, undef, undef), - # As `status-ipv4`, but with `wantipv6` (or `wantip`, if an IPv6 address). + # As `status-ipv4`, but with `wantipv6`. 'status-ipv6' => setv(T_ANY, 0, 1, undef, undef), # Timestamp (seconds since epoch) of the most recent attempt that would have been made had # `min-interval` not inhibited the attempt. This is reset to 0 once an attempt is actually @@ -1437,11 +1437,10 @@ sub update_nics { $ip //= $ipv4 // $ipv6; $ipv4 //= $ip if is_ipv4($ip); $ipv6 //= $ip if is_ipv6($ip); - $config{$h}{'wantip'} = $ip; $config{$h}{'wantipv4'} = $ipv4; $config{$h}{'wantipv6'} = $ipv6; - if (!$ip && !$ipv4 && !$ipv6) { + if (!$ipv4 && !$ipv6) { warning('unable to determine IP address'); next; } @@ -1455,6 +1454,11 @@ sub update_nics { if (@hosts) { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); + for my $h (@hosts) { + # TODO: Remove this once all legacy protocol implementations have been upgraded to + # read `wantipv4` and `wantipv6` directly. + $config{$h}{'wantip'} = $config{$h}{'wantipv4'} // $config{$h}{'wantipv6'}; + } &$update(@hosts); for my $h (@hosts) { delete($config{$h}{$_}) for qw(wantip wantipv4 wantipv6); From a178d40633c3cdd64b20f09174ec5ecac88985ed Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 13:20:01 -0400 Subject: [PATCH 26/45] update_nics: Combine post-`update` host loops --- ddclient.in | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ddclient.in b/ddclient.in index 4ac3927..15e508a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1460,17 +1460,14 @@ sub update_nics { $config{$h}{'wantip'} = $config{$h}{'wantipv4'} // $config{$h}{'wantipv6'}; } &$update(@hosts); - for my $h (@hosts) { - delete($config{$h}{$_}) for qw(wantip wantipv4 wantipv6); - } - - # Backwards compatibility: Legacy protocol implementations read `wantip` and set `ip` - # and `status`. Modern protocol implementations read `wantipv4` and `wantipv6` and set - # `ipv4`, `ipv6`, `status-ipv4`, and `status-ipv6`. Make legacy implementations look - # like modern implementations by moving `ip` and `status` to the modern - # version-specific equivalents. for my $h (@hosts) { local $_l = pushlogctx($h); + delete($config{$h}{$_}) for qw(wantip wantipv4 wantipv6); + # Backwards compatibility: Legacy protocol implementations read `wantip` and set + # `ip` and `status`. Modern protocol implementations read `wantipv4` and + # `wantipv6` and set `ipv4`, `ipv6`, `status-ipv4`, and `status-ipv6`. Make legacy + # implementations look like modern implementations by moving `ip` and `status` to + # the modern version-specific equivalents. my $status = delete($config{$h}{'status'}) or next; my $ip = $config{$h}{'ip'}; my $ipv = is_ipv4($ip) ? '4' : is_ipv6($ip) ? '6' : undef; From 5256a1d02ccdf1b8027374eef51116be211d0b0c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 16:39:43 -0400 Subject: [PATCH 27/45] update_nics: Move legacy protocol support to an adapter function --- ddclient.in | 128 ++++++++++++++++++++++++----------------------- t/update_nics.pl | 4 +- 2 files changed, 68 insertions(+), 64 deletions(-) diff --git a/ddclient.in b/ddclient.in index 15e508a..b6e3d17 100755 --- a/ddclient.in +++ b/ddclient.in @@ -709,9 +709,55 @@ our %variables = ( 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), }, ); + +# Converts a legacy protocol update method to behave like a modern implementation by moving +# `$config{$h}{status}` and `$config{$h}{ip}` to the version-specific modern equivalents. +sub adapt_legacy_update { + my ($update) = @_; + return sub { + my (@hosts) = @_; + my %ipv; + for my $h (@hosts) { + $ipv{$h} = defined($config{$h}{'wantipv4'}) ? '4' : '6'; + $config{$h}{'wantip'} = delete($config{$h}{"wantipv$ipv{$h}"}); + delete($config{$h}{$_}) for qw(ip status); + } + $update->(@hosts); + for my $h (@hosts) { + local $_l = pushlogctx($h); + delete($config{$h}{'wantip'}); + debug( + "legacy protocol; moving 'status' to 'status-ipv$ipv{$h}', 'ip' to 'ipv$ipv{$h}'"); + $config{$h}{"status-ipv$ipv{$h}"} = delete($config{$h}{'status'}); + # TODO: Currently $config{$h}{'ip'} is used for two distinct purposes: it holds the + # value of the --ip option, and it is updated by legacy protocols to hold the new IP + # address after an update. Fortunately, the --ip option is not used very often, and if + # it is, the values for the two use cases are usually (but not always) the same. This + # boolean is an imperfect attempt to identify whether 'ip' is being used for the --ip + # option, to avoid breaking some user's configuration. Protocols should be updated to + # not set $config{$h}{'ip'} because %config is for configuration, not update status + # (same goes for 'status', 'mtime', etc.). + my $ip_option = opt('use', $h) eq 'ip' || opt('usev6', $h) eq 'ip'; + my $ip = $config{$h}{'ip'}; + delete($config{$h}{'ip'}) if !$ip_option; + # TODO: See above comment for $ip_option. This is the same situation, but for 'ipv4' + # and 'ipv6'. + my $vip_option = opt("usev$ipv{$h}", $h) eq "ipv$ipv{$h}"; + if ($vip_option && defined(my $vip = $config{$h}{"ipv$ipv{$h}"})) { + if (!defined($ip) || $ip ne $vip) { + my $fip = defined($ip) ? "'$ip'" : ''; + debug("unable to set 'ipv$ipv{$h}' to $fip; already set to '$vip'"); + } + next; + } + $config{$h}{"ipv$ipv{$h}"} = $ip; + } + }; +} + our %protocols = ( '1984' => { - 'update' => \&nic_1984_update, + 'update' => adapt_legacy_update(\&nic_1984_update), 'examples' => \&nic_1984_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -720,7 +766,7 @@ our %protocols = ( }, }, 'changeip' => { - 'update' => \&nic_changeip_update, + 'update' => adapt_legacy_update(\&nic_changeip_update), 'examples' => \&nic_changeip_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -739,7 +785,7 @@ our %protocols = ( }, }, 'cloudns' => { - 'update' => \&nic_cloudns_update, + 'update' => adapt_legacy_update(\&nic_cloudns_update), 'examples' => \&nic_cloudns_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -768,7 +814,7 @@ our %protocols = ( }, }, 'dinahosting' => { - 'update' => \&nic_dinahosting_update, + 'update' => adapt_legacy_update(\&nic_dinahosting_update), 'examples' => \&nic_dinahosting_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -789,7 +835,7 @@ our %protocols = ( }, }, 'dnsmadeeasy' => { - 'update' => \&nic_dnsmadeeasy_update, + 'update' => adapt_legacy_update(\&nic_dnsmadeeasy_update), 'examples' => \&nic_dnsmadeeasy_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -798,7 +844,7 @@ our %protocols = ( }, }, 'dondominio' => { - 'update' => \&nic_dondominio_update, + 'update' => adapt_legacy_update(\&nic_dondominio_update), 'examples' => \&nic_dondominio_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -806,7 +852,7 @@ our %protocols = ( }, }, 'dslreports1' => { - 'update' => \&nic_dslreports1_update, + 'update' => adapt_legacy_update(\&nic_dslreports1_update), 'examples' => \&nic_dslreports1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -831,7 +877,7 @@ our %protocols = ( }, }, 'dyndns1' => { - 'update' => \&nic_dyndns1_update, + 'update' => adapt_legacy_update(\&nic_dyndns1_update), 'examples' => \&nic_dyndns1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -873,7 +919,7 @@ our %protocols = ( }, }, 'freemyip' => { - 'update' => \&nic_freemyip_update, + 'update' => adapt_legacy_update(\&nic_freemyip_update), 'examples' => \&nic_freemyip_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -947,7 +993,7 @@ our %protocols = ( }, }, 'namecheap' => { - 'update' => \&nic_namecheap_update, + 'update' => adapt_legacy_update(\&nic_namecheap_update), 'examples' => \&nic_namecheap_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -956,7 +1002,7 @@ our %protocols = ( }, }, 'nfsn' => { - 'update' => \&nic_nfsn_update, + 'update' => adapt_legacy_update(\&nic_nfsn_update), 'examples' => \&nic_nfsn_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -996,7 +1042,7 @@ our %protocols = ( }, }, 'ovh' => { - 'update' => \&nic_ovh_update, + 'update' => adapt_legacy_update(\&nic_ovh_update), 'examples' => \&nic_ovh_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1018,7 +1064,7 @@ our %protocols = ( }, }, 'sitelutions' => { - 'update' => \&nic_sitelutions_update, + 'update' => adapt_legacy_update(\&nic_sitelutions_update), 'examples' => \&nic_sitelutions_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1027,7 +1073,7 @@ our %protocols = ( }, }, 'yandex' => { - 'update' => \&nic_yandex_update, + 'update' => adapt_legacy_update(\&nic_yandex_update), 'examples' => \&nic_yandex_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1036,7 +1082,7 @@ our %protocols = ( }, }, 'zoneedit1' => { - 'update' => \&nic_zoneedit1_update, + 'update' => adapt_legacy_update(\&nic_zoneedit1_update), 'examples' => \&nic_zoneedit1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1046,7 +1092,7 @@ our %protocols = ( }, }, 'keysystems' => { - 'update' => \&nic_keysystems_update, + 'update' => adapt_legacy_update(\&nic_keysystems_update), 'examples' => \&nic_keysystems_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1077,7 +1123,7 @@ our %protocols = ( }, }, 'enom' => { - 'update' => \&nic_enom_update, + 'update' => adapt_legacy_update(\&nic_enom_update), 'examples' => \&nic_enom_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1454,49 +1500,9 @@ sub update_nics { if (@hosts) { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); - for my $h (@hosts) { - # TODO: Remove this once all legacy protocol implementations have been upgraded to - # read `wantipv4` and `wantipv6` directly. - $config{$h}{'wantip'} = $config{$h}{'wantipv4'} // $config{$h}{'wantipv6'}; - } &$update(@hosts); for my $h (@hosts) { - local $_l = pushlogctx($h); - delete($config{$h}{$_}) for qw(wantip wantipv4 wantipv6); - # Backwards compatibility: Legacy protocol implementations read `wantip` and set - # `ip` and `status`. Modern protocol implementations read `wantipv4` and - # `wantipv6` and set `ipv4`, `ipv6`, `status-ipv4`, and `status-ipv6`. Make legacy - # implementations look like modern implementations by moving `ip` and `status` to - # the modern version-specific equivalents. - my $status = delete($config{$h}{'status'}) or next; - my $ip = $config{$h}{'ip'}; - my $ipv = is_ipv4($ip) ? '4' : is_ipv6($ip) ? '6' : undef; - # TODO: Currently $config{$h}{'ip'} is used for two distinct purposes: it holds the - # value of the --ip option, and it is updated by legacy protocols to hold the new - # IP address after an update. Fortunately, the --ip option is not used very often, - # and if it is, the values for the two use cases are usually (but not always) the - # same. This boolean is an imperfect attempt to identify whether 'ip' is being - # used for the --ip option, to avoid breaking some user's configuration. Protocols - # should be updated to not set $config{$h}{'ip'} because %config is for - # configuration, not update status (same goes for 'status', 'mtime', etc.). - my $ip_option = opt('use', $h) eq 'ip' || opt('usev6', $h) eq 'ip'; - delete($config{$h}{'ip'}) if !$ip_option; - debug("legacy protocol; moving 'status' to 'status-ipv$ipv', 'ip' to 'ipv$ipv'"); - $config{$h}{"status-ipv$ipv"} = $status; - # TODO: See above comment for $ip_option. This is the same situation, but for - # 'ipv4' and 'ipv6'. - my $vip_option = opt("usev$ipv", $h) eq "ipv$ipv"; - if (defined(my $vip = $config{$h}{"ipv$ipv"}) && $vip_option) { - debug("unable to update 'ipv$ipv' to '$ip' " . - "because it is already set to '$vip'") if $vip_option; - } else { - # A previous update would have moved `$config{$h}{'ip'}` to - # `$config{$h}{"ipv$ipv"}`, so it is OK to overwrite `$config{$h}{"ipv$ipv"}` - # if it is already set. - debug("updating 'ipv$ipv' from '$vip' to '$ip'") - if defined($vip) && $vip ne $ip; - $config{$h}{"ipv$ipv"} = $ip; - } + delete($config{$h}{$_}) for qw(wantipv4 wantipv6); } runpostscript(join ' ', keys %ipsv4, keys %ipsv6); @@ -3600,9 +3606,7 @@ sub nic_updateable { } } - # TODO: `status` is set by legacy protocol implementations. Remove it from this list once all - # legacy protocol implementations have been upgraded. - delete($config{$host}{$_}) for qw(status status-ipv4 status-ipv6 update); + delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6 update); if ($update) { $config{$host}{'update'} = 1; $config{$host}{'atime'} = $now; @@ -3611,7 +3615,7 @@ sub nic_updateable { for (qw(status-ipv4 status-ipv6)) { $config{$host}{$_} = $recap{$host}{$_} if defined($recap{$host}{$_}); } - delete($config{$host}{$_}) for qw(wantip wantipv4 wantipv6); + delete($config{$host}{$_}) for qw(wantipv4 wantipv6); } return $update; diff --git a/t/update_nics.pl b/t/update_nics.pl index c7f8a9c..461f036 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -48,7 +48,7 @@ local %ddclient::protocols = ( # properties. (Modern protocol implementations read `wantipv4` and `wantipv6` and set `ipv4`, # `ipv6`, `status-ipv4`, and `status-ipv6`.) It always succeeds. legacy => { - update => sub { + update => ddclient::adapt_legacy_update(sub { ddclient::debug('in update'); for my $h (@_) { local $ddclient::_l = ddclient::pushlogctx($h); @@ -59,7 +59,7 @@ local %ddclient::protocols = ( $ddclient::config{$h}{mtime} = $ddclient::now; } ddclient::debug('returning from update'); - }, + }), variables => { %{$ddclient::variables{'protocol-common-defaults'}}, }, From 25fac765a06196efe207689d76888bc9aadcc16c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 02:18:43 -0400 Subject: [PATCH 28/45] nic_updateable: Move clearing of `update` to `write_recap` for readability (the logic that uses the `update` boolean should be responsible for clearing it). --- ddclient.in | 3 ++- t/update_nics.pl | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index b6e3d17..a834e74 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1573,6 +1573,7 @@ sub write_recap { # `$recap{$h}` doesn't exist, and that check is done before the force-if-config-changed # check.) if (!exists $recap{$h} || $config{$h}{'update'}) { + delete($config{$h}{'update'}); $recap{$h} = {}; for my $v (keys(%$vars)) { next if !$vars->{$v}{recap} || !defined(opt($v, $h)); @@ -3606,7 +3607,7 @@ sub nic_updateable { } } - delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6 update); + delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6); if ($update) { $config{$host}{'update'} = 1; $config{$host}{'atime'} = $now; diff --git a/t/update_nics.pl b/t/update_nics.pl index 461f036..c249abd 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -345,7 +345,6 @@ for my $tc (@test_cases) { Names => ['*got', '*want'])); } my %want_cfg = (host => { - $tc->{want_update} ? (update => 1) : (), %cfg, %{$tc->{want_cfg_changes} // {}}, }); From 75552f80f7ae1aa5bb88755b4d148bf778cde12c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 23 Aug 2024 21:43:54 -0400 Subject: [PATCH 29/45] nic_updateable: Don't mutate `status-ipv*` vars if not updating This simplifies the logic a bit and improves readability. --- ddclient.in | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ddclient.in b/ddclient.in index a834e74..959f8a6 100755 --- a/ddclient.in +++ b/ddclient.in @@ -3607,15 +3607,12 @@ sub nic_updateable { } } - delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6); if ($update) { $config{$host}{'update'} = 1; $config{$host}{'atime'} = $now; - delete($config{$host}{$_}) for qw(wtime warned-min-interval warned-min-error-interval); + delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6 wtime + warned-min-interval warned-min-error-interval); } else { - for (qw(status-ipv4 status-ipv6)) { - $config{$host}{$_} = $recap{$host}{$_} if defined($recap{$host}{$_}); - } delete($config{$host}{$_}) for qw(wantipv4 wantipv6); } From 974bba4d93df8e2c03a170b5f1753f05947986f4 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 23 Aug 2024 23:55:09 -0400 Subject: [PATCH 30/45] update_nics: Don't set `wantip*` if they're all `undef` This helps keep `%config` "clean", which helps with testing and debugging. --- ddclient.in | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ddclient.in b/ddclient.in index 959f8a6..3d630fe 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1483,13 +1483,12 @@ sub update_nics { $ip //= $ipv4 // $ipv6; $ipv4 //= $ip if is_ipv4($ip); $ipv6 //= $ip if is_ipv6($ip); - $config{$h}{'wantipv4'} = $ipv4; - $config{$h}{'wantipv6'} = $ipv6; - if (!$ipv4 && !$ipv6) { warning('unable to determine IP address'); next; } + $config{$h}{'wantipv4'} = $ipv4; + $config{$h}{'wantipv6'} = $ipv6; next if !nic_updateable($h); push @hosts, $h; From 2927f205ea812787425482bdad135521a01ec799 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 21 Jun 2024 01:22:41 -0400 Subject: [PATCH 31/45] update_nics: Move non-config recap var reset to update call for readability --- ddclient.in | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/ddclient.in b/ddclient.in index 3d630fe..71e63fe 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1489,8 +1489,10 @@ sub update_nics { } $config{$h}{'wantipv4'} = $ipv4; $config{$h}{'wantipv6'} = $ipv6; - - next if !nic_updateable($h); + if (!nic_updateable($h)) { + delete($config{$h}{$_}) for qw(wantipv4 wantipv6); + next; + } push @hosts, $h; $ipsv4{$ipv4} = $h if ($ipv4); @@ -1499,6 +1501,12 @@ sub update_nics { if (@hosts) { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); + for my $h (@hosts) { + $config{$h}{'update'} = 1; + $config{$h}{'atime'} = $now; + delete($config{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime + warned-min-interval warned-min-error-interval); + } &$update(@hosts); for my $h (@hosts) { delete($config{$h}{$_}) for qw(wantipv4 wantipv6); @@ -3605,16 +3613,6 @@ sub nic_updateable { if defined($ipv6); } } - - if ($update) { - $config{$host}{'update'} = 1; - $config{$host}{'atime'} = $now; - delete($config{$host}{$_}) for qw(status-ipv4 status-ipv6 wtime - warned-min-interval warned-min-error-interval); - } else { - delete($config{$host}{$_}) for qw(wantipv4 wantipv6); - } - return $update; } From 7660ca52bf1a82591dd6becde9697885f561e67f Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 03:18:07 -0400 Subject: [PATCH 32/45] write_recap: Remove unnecessary recap existence check --- ddclient.in | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/ddclient.in b/ddclient.in index 71e63fe..8c39e98 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1567,19 +1567,7 @@ sub write_recap { # represent `undef` in the cache file and to simplify testing. Also, entries for non-recap # variables (which can happen after changing a host's protocol or after switching to a # different version of ddclient) are deleted. - # - # TODO: The "configuration change detection" variables are also syncronized if `$recap{$h}` - # doesn't exist. Why is this done? Perhaps the intention was to prevent the - # force-if-config-changed logic from triggering the very first cycle (when there is no - # previous update attempt). However: - # * There's a bug: This function isn't called until after `update_nics` has already - # checked the force-if-config-changed condition, so syncing the values here is too late - # to suppress the first cycle's force-if-config-changed logic. - # * It's unnecessary: If there have not been any update attempts, an update attempt - # should be made anyway, and already is. (`update_nics` already forces an update if - # `$recap{$h}` doesn't exist, and that check is done before the force-if-config-changed - # check.) - if (!exists $recap{$h} || $config{$h}{'update'}) { + if ($config{$h}{'update'}) { delete($config{$h}{'update'}); $recap{$h} = {}; for my $v (keys(%$vars)) { From 1a748e7a86e060da1aab159693eb087704de68fa Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 15:03:46 -0400 Subject: [PATCH 33/45] write_recap: Only update variables that could have changed This is a step toward improving readability of `%config`/`%recap` synchronization. --- ddclient.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 8c39e98..ac35d75 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1575,8 +1575,7 @@ sub write_recap { $recap{$h}{$v} = opt($v, $h); } } else { - for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 - warned-min-interval warned-min-error-interval)) { + for my $v (qw(warned-min-interval warned-min-error-interval)) { if ($vars->{$v} && $vars->{$v}{recap} && defined(my $val = opt($v, $h))) { $recap{$h}{$v} = $val; } else { From e478117d4e0f3607ebf2bc0fc5cbbea2db378a54 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 15:13:40 -0400 Subject: [PATCH 34/45] write_recap: Move `warned-min-*` recap sync to where they are set This is a step toward separating `%recap` from `%config`. --- ddclient.in | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/ddclient.in b/ddclient.in index ac35d75..f967da5 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1559,29 +1559,19 @@ sub write_recap { my ($file) = @_; for my $h (keys %config) { - my $vars = $protocols{opt('protocol', $h)}{variables}; # nic_updateable (called from update_nics for each host) sets `$config{$h}{update}` # according to whether an update was attempted. - # + next if !$config{$h}{'update'}; + delete($config{$h}{'update'}); + my $vars = $protocols{opt('protocol', $h)}{variables}; # Entries in `%recap` with `undef` values are deleted to avoid needing to figure out how to # represent `undef` in the cache file and to simplify testing. Also, entries for non-recap # variables (which can happen after changing a host's protocol or after switching to a # different version of ddclient) are deleted. - if ($config{$h}{'update'}) { - delete($config{$h}{'update'}); - $recap{$h} = {}; - for my $v (keys(%$vars)) { - next if !$vars->{$v}{recap} || !defined(opt($v, $h)); - $recap{$h}{$v} = opt($v, $h); - } - } else { - for my $v (qw(warned-min-interval warned-min-error-interval)) { - if ($vars->{$v} && $vars->{$v}{recap} && defined(my $val = opt($v, $h))) { - $recap{$h}{$v} = $val; - } else { - delete($recap{$h}{$v}); - } - } + $recap{$h} = {}; + for my $v (keys(%$vars)) { + next if !$vars->{$v}{recap} || !defined(opt($v, $h)); + $recap{$h}{$v} = opt($v, $h); } } @@ -3537,6 +3527,7 @@ sub nic_updateable { if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); $config{$host}{'warned-min-interval'} = $now; + $recap{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv4'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { @@ -3552,6 +3543,7 @@ sub nic_updateable { } $config{$host}{'warned-min-error-interval'} = $now; + $recap{$host}{'warned-min-error-interval'} = $now; } else { $update = 1; @@ -3564,6 +3556,7 @@ sub nic_updateable { if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); $config{$host}{'warned-min-interval'} = $now; + $recap{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv6'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { @@ -3579,6 +3572,7 @@ sub nic_updateable { } $config{$host}{'warned-min-error-interval'} = $now; + $recap{$host}{'warned-min-error-interval'} = $now; } else { $update = 1; From 0348ded46b4fa24375226681abe8f9e1c0a6f37c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 26 Aug 2024 18:23:36 -0400 Subject: [PATCH 35/45] write_recap: Move update-specific `%recap` sync to `update_nics` This is a step toward separating `%recap` from `%config`. --- ddclient.in | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/ddclient.in b/ddclient.in index f967da5..957b05b 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1502,7 +1502,6 @@ sub update_nics { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); for my $h (@hosts) { - $config{$h}{'update'} = 1; $config{$h}{'atime'} = $now; delete($config{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime warned-min-interval warned-min-error-interval); @@ -1511,7 +1510,21 @@ sub update_nics { for my $h (@hosts) { delete($config{$h}{$_}) for qw(wantipv4 wantipv6); } - + for my $h (@hosts) { + # Update `%recap` with the latest values in `%config`. This is done after the + # protocol's update method returns in case that method mutates any of the `%config` + # values or needs access to the previous iteration's `%recap` values. Entries in + # `%recap` with `undef` values are deleted to avoid needing to figure out how to + # represent `undef` in the cache file and to simplify testing. Also, entries for + # non-recap variables (which can happen after changing a host's protocol or after + # switching to a different version of ddclient) are deleted. + my $vars = $protocols{$p}{variables}; + $recap{$h} = {}; + for my $v (keys(%$vars)) { + next if !$vars->{$v}{recap} || !defined(opt($v, $h)); + $recap{$h}{$v} = opt($v, $h); + } + } runpostscript(join ' ', keys %ipsv4, keys %ipsv6); } } @@ -1557,24 +1570,6 @@ sub write_pid { ###################################################################### sub write_recap { my ($file) = @_; - - for my $h (keys %config) { - # nic_updateable (called from update_nics for each host) sets `$config{$h}{update}` - # according to whether an update was attempted. - next if !$config{$h}{'update'}; - delete($config{$h}{'update'}); - my $vars = $protocols{opt('protocol', $h)}{variables}; - # Entries in `%recap` with `undef` values are deleted to avoid needing to figure out how to - # represent `undef` in the cache file and to simplify testing. Also, entries for non-recap - # variables (which can happen after changing a host's protocol or after switching to a - # different version of ddclient) are deleted. - $recap{$h} = {}; - for my $v (keys(%$vars)) { - next if !$vars->{$v}{recap} || !defined(opt($v, $h)); - $recap{$h}{$v} = opt($v, $h); - } - } - my $recap = ""; for my $h (sort keys %recap) { my $opt = join(',', map("$_=$recap{$h}{$_}", sort(keys(%{$recap{$h}})))); From 268369a05eb5ab2e38ae1334744ef2828da24083 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 25 Aug 2024 00:31:42 -0400 Subject: [PATCH 36/45] Write update status to `%recap`, not `%config` Status is not configuration so it doesn't belong in `%config`. `wantip`, `wantipv4`, and `wantipv6` are still passed along in `%config` because `group_hosts_by` needs access to them like other settings. --- ChangeLog.md | 2 + ddclient.in | 466 ++++++++++++++++++---------------------- t/protocol_directnic.pl | 30 ++- t/protocol_dyndns2.pl | 40 ++-- t/read_recap.pl | 119 ++-------- t/update_nics.pl | 48 +---- 6 files changed, 254 insertions(+), 451 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 69c2f76..3a0215a 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -100,6 +100,8 @@ repository history](https://github.com/ddclient/ddclient/commits/master). ### Bug fixes + * Fixed numerous bugs in cache file (recap) handling. + [#740](https://github.com/ddclient/ddclient/pull/740) * Fixed numerous bugs in command-line option and configuration file processing. [#733](https://github.com/ddclient/ddclient/pull/733) * `noip`: Fixed failure to honor IP discovery settings in some circumstances. diff --git a/ddclient.in b/ddclient.in index 957b05b..a2c250b 100755 --- a/ddclient.in +++ b/ddclient.in @@ -145,15 +145,10 @@ our %config; # host's "recap variables" -- the host's protocol's variables with a truthy `recap` property -- are # included. # -# `%config` is the source of truth for recap variable values. The recap variable values in -# `%config` are initialized with values from the cache file (via `%recap`). After initialization, -# any change to a recap variable's value is made to `%config` and later copied to `%recap` before -# being written to the cache file. Changes made directly to `%recap` are erroneous because they -# will be overwritten by the value in `%config`. -# -# There are two classes of recap variables: +# `%recap` is independent of `%config`, although they both share the same set of variable +# declarations. There are two classes of recap variables: # * "Status" variables: These track update success/failure, the IP address of the last successful -# update, etc. These do not hold configuration data. These should always be kept in sync with +# update, etc. These do not hold configuration data; they are unrelated to any entries in # `%config`. # * "Configuration change detection" variables: These are used to force an update if the value in # the same-named entry in `%config` has changed since the previous update attempt. The value @@ -654,20 +649,13 @@ our %variables = ( 'min-error-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), # The desired IP address (IPv4 or IPv6, but almost always IPv4) that should be saved at the - # DDNS service. - # TODO: Legacy protocol implementations write the IP address most recently saved at the - # DDNS service to this variable so that it can be saved in the recap (as `ipv4` or `ipv6`). - # Update the legacy implementations to use `ipv4` or `ipv6` instead, though see the TODO - # for those variables. + # DDNS service (when `use=ip`). 'ip' => setv(T_IP, 0, 0, undef, undef), # As a recap value, this is the IPv4 address most recently saved at the DDNS service. As a # setting, this is the desired IPv4 address that should be saved at the DDNS service. - # TODO: The use of `ipv4` as a recap status variable is supposed to be independent of the - # use of `ipv4` as a configuration setting. Unfortunately, because the `%config` value is - # synced to `%recap`, the two uses conflict which causes the bug "skipped: IP address was - # already set to a.b.c.d" when the IP was never set to a.b.c.d. Rename the `%recap` status - # variable to something like `saved-ipv4` to avoid the collision (and confusion) with the - # `%config` setting variable. + # TODO: The use of `ipv4` as a recap status variable is independent of the use of `ipv4` as + # a configuration setting. Rename the `%recap` status variable to something like + # `saved-ipv4` to avoid confusion with the `%config` setting variable. 'ipv4' => setv(T_IPV4, 0, 1, undef, undef), # As `ipv4`, but for an IPv6 address. 'ipv6' => setv(T_IPV6, 0, 1, undef, undef), @@ -711,7 +699,7 @@ our %variables = ( ); # Converts a legacy protocol update method to behave like a modern implementation by moving -# `$config{$h}{status}` and `$config{$h}{ip}` to the version-specific modern equivalents. +# `$recap{$h}{status}` and `$recap{$h}{ip}` to the version-specific modern equivalents. sub adapt_legacy_update { my ($update) = @_; return sub { @@ -720,7 +708,7 @@ sub adapt_legacy_update { for my $h (@hosts) { $ipv{$h} = defined($config{$h}{'wantipv4'}) ? '4' : '6'; $config{$h}{'wantip'} = delete($config{$h}{"wantipv$ipv{$h}"}); - delete($config{$h}{$_}) for qw(ip status); + delete($recap{$h}{$_}) for qw(ip status); } $update->(@hosts); for my $h (@hosts) { @@ -728,29 +716,8 @@ sub adapt_legacy_update { delete($config{$h}{'wantip'}); debug( "legacy protocol; moving 'status' to 'status-ipv$ipv{$h}', 'ip' to 'ipv$ipv{$h}'"); - $config{$h}{"status-ipv$ipv{$h}"} = delete($config{$h}{'status'}); - # TODO: Currently $config{$h}{'ip'} is used for two distinct purposes: it holds the - # value of the --ip option, and it is updated by legacy protocols to hold the new IP - # address after an update. Fortunately, the --ip option is not used very often, and if - # it is, the values for the two use cases are usually (but not always) the same. This - # boolean is an imperfect attempt to identify whether 'ip' is being used for the --ip - # option, to avoid breaking some user's configuration. Protocols should be updated to - # not set $config{$h}{'ip'} because %config is for configuration, not update status - # (same goes for 'status', 'mtime', etc.). - my $ip_option = opt('use', $h) eq 'ip' || opt('usev6', $h) eq 'ip'; - my $ip = $config{$h}{'ip'}; - delete($config{$h}{'ip'}) if !$ip_option; - # TODO: See above comment for $ip_option. This is the same situation, but for 'ipv4' - # and 'ipv6'. - my $vip_option = opt("usev$ipv{$h}", $h) eq "ipv$ipv{$h}"; - if ($vip_option && defined(my $vip = $config{$h}{"ipv$ipv{$h}"})) { - if (!defined($ip) || $ip ne $vip) { - my $fip = defined($ip) ? "'$ip'" : ''; - debug("unable to set 'ipv$ipv{$h}' to $fip; already set to '$vip'"); - } - next; - } - $config{$h}{"ipv$ipv{$h}"} = $ip; + $recap{$h}{"status-ipv$ipv{$h}"} = delete($recap{$h}{'status'}); + $recap{$h}{"ipv$ipv{$h}"} = delete($recap{$h}{'ip'}); } }; } @@ -1502,29 +1469,39 @@ sub update_nics { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); for my $h (@hosts) { - $config{$h}{'atime'} = $now; - delete($config{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime - warned-min-interval warned-min-error-interval); + $recap{$h}{'atime'} = $now; + delete($recap{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime + warned-min-interval warned-min-error-interval); + # Update configuration change detection variables in `%recap` with the latest + # values in `%config`. Notes about why this is done here: + # * The vars must not be updated if the host is not being updated because change + # detection is defined relative to the previous update attempt. In particular, + # these can't be updated when the protocol's `force_update` method is called + # because that method is not always called before an update is attempted. + # * The vars must be updated after the `force_update` method would be called so + # that `force_update` can check whether any settings have changed since the + # last time an update was attempted. + # * The vars are updated before the protocol's `update` method is called so that + # `update` sees consistent values between `%recap` and `%config`. This reduces + # the impact of Hyrum's Law; if a protocol needs a variable to be updated after + # the `update` method is called then that behavior should be made explicit. + my $vars = $protocols{opt('protocol', $h)}{variables}; + for my $v (qw(static wildcard mx backupmx)) { + next if !$vars->{$v} || !$vars->{$v}{recap}; + if (defined(my $val = opt($v, $h))) { + $recap{$h}{$v} = $val; + } else { + # Entries in `%recap` with `undef` values are deleted to avoid needing to + # figure out how to represent `undef` in the cache file and to simplify + # testing. + delete($recap{$h}{$v}); + } + } } &$update(@hosts); for my $h (@hosts) { delete($config{$h}{$_}) for qw(wantipv4 wantipv6); } - for my $h (@hosts) { - # Update `%recap` with the latest values in `%config`. This is done after the - # protocol's update method returns in case that method mutates any of the `%config` - # values or needs access to the previous iteration's `%recap` values. Entries in - # `%recap` with `undef` values are deleted to avoid needing to figure out how to - # represent `undef` in the cache file and to simplify testing. Also, entries for - # non-recap variables (which can happen after changing a host's protocol or after - # switching to a different version of ddclient) are deleted. - my $vars = $protocols{$p}{variables}; - $recap{$h} = {}; - for my $v (keys(%$vars)) { - next if !$vars->{$v}{recap} || !defined(opt($v, $h)); - $recap{$h}{$v} = opt($v, $h); - } - } runpostscript(join ' ', keys %ipsv4, keys %ipsv6); } } @@ -1614,7 +1591,6 @@ sub read_recap { %opt = (); $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file); %opt = %saved; - $recap{$_} //= {} for keys(%config); for my $h (keys(%recap)) { if (!exists($config{$h})) { delete($recap{$h}); @@ -1624,26 +1600,6 @@ sub read_recap { for my $v (keys(%{$recap{$h}})) { delete($recap{$h}{$v}) if !$vars->{$v} || !$vars->{$v}{recap}; } - # Only status variables are copied from `%recap` into `%config` because the recap should - # not change the configuration. See the documenting comments for `%recap`. - # TODO: Hard-coding this list impairs readability and maintainability. In particular, - # different pieces of code need to know the list of status variables, and keeping them all - # in sync is error-prone (DRY). Also, different protocols might need different status - # variables. - # TODO: None of the recap variables should be copied into `%config` because `%config` is - # not the semantically appropriate place to record update status. Code that currently - # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access - # `$recap{$h}{'status-ipv4'}` directly. - for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 - warned-min-interval warned-min-error-interval)) { - next if !$vars->{$v} || !$vars->{$v}{recap}; - if (defined($recap{$h}{$v})) { - $config{$h}{$v} = $recap{$h}{$v}; - } else { - delete($config{$h}{$v}); - } - } - delete($recap{$h}) if !%{$recap{$h}}; } } ###################################################################### @@ -3519,15 +3475,14 @@ sub nic_updateable { if (($recap{$host}{'status-ipv4'} // '') eq 'good' && !interval_expired($host, 'mtime', 'min-interval')) { warning("skipped update from $previpv4 to $ipv4 because it has been less than $prettyi{'min-interval'} since the previous update (on $prettyt{'mtime'})") - if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); + if opt('verbose') || !($recap{$host}{'warned-min-interval'} // 0); - $config{$host}{'warned-min-interval'} = $now; $recap{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv4'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { - if (opt('verbose') || (!$config{$host}{'warned-min-error-interval'} && + if (opt('verbose') || (!$recap{$host}{'warned-min-error-interval'} && ($warned_ipv4{$host} // 0) < $inv_ip_warn_count)) { warning("skipped update from $previpv4 to $ipv4 because it has been less than $prettyi{'min-error-interval'} since the previous update attempt (on $prettyt{'atime'}), which failed"); if (!$ipv4 && !opt('verbose')) { @@ -3537,7 +3492,6 @@ sub nic_updateable { } } - $config{$host}{'warned-min-error-interval'} = $now; $recap{$host}{'warned-min-error-interval'} = $now; } else { @@ -3548,15 +3502,14 @@ sub nic_updateable { if (($recap{$host}{'status-ipv6'} // '') eq 'good' && !interval_expired($host, 'mtime', 'min-interval')) { warning("skipped update from $previpv6 to $ipv6 because it has been less than $prettyi{'min-interval'} since the previous update (on $prettyt{'mtime'})") - if opt('verbose') || !($config{$host}{'warned-min-interval'} // 0); + if opt('verbose') || !($recap{$host}{'warned-min-interval'} // 0); - $config{$host}{'warned-min-interval'} = $now; $recap{$host}{'warned-min-interval'} = $now; } elsif (($recap{$host}{'status-ipv6'} // '') ne 'good' && !interval_expired($host, 'atime', 'min-error-interval')) { - if (opt('verbose') || (!$config{$host}{'warned-min-error-interval'} && + if (opt('verbose') || (!$recap{$host}{'warned-min-error-interval'} && ($warned_ipv6{$host} // 0) < $inv_ip_warn_count)) { warning("skipped update from $previpv6 to $ipv6 because it has been less than $prettyi{'min-error-interval'} since the previous update attempt (on $prettyt{'atime'}, which failed"); if (!$ipv6 && !opt('verbose')) { @@ -3566,7 +3519,6 @@ sub nic_updateable { } } - $config{$host}{'warned-min-error-interval'} = $now; $recap{$host}{'warned-min-error-interval'} = $now; } else { @@ -3712,16 +3664,16 @@ sub nic_dyndns1_update { } if ($return_code ne 'NOERROR' || $error_code ne 'NOERROR' || !$title) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; $title = 'incomplete response from ' . opt('server', $h) unless $title; warning("SENT: %s", $url) unless opt('verbose'); warning("REPLIED: %s", $reply); failed($title); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("$return_code: IP address set to $ip ($title)"); } } @@ -3868,8 +3820,8 @@ sub nic_dyndns2_update { warning("$status: $errors{$status}"); $status = 'good'; } - $config{$h}{'status-ipv4'} = $status if $ipv4; - $config{$h}{'status-ipv6'} = $status if $ipv6; + $recap{$h}{'status-ipv4'} = $status if $ipv4; + $recap{$h}{'status-ipv6'} = $status if $ipv6; if ($status ne 'good') { if (exists($errors{$status})) { failed("$status: $errors{$status}"); @@ -3887,9 +3839,9 @@ sub nic_dyndns2_update { # some services do not return the IP; and (2) comparison is brittle (e.g., # 192.000.002.001 vs. 192.0.2.1) and false errors could cause high load on the service # (an update attempt every min-error-interval instead of every max-interval). - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; - $config{$h}{'mtime'} = $now; + $recap{$h}{'ipv4'} = $ipv4 if $ipv4; + $recap{$h}{'ipv6'} = $ipv6 if $ipv6; + $recap{$h}{'mtime'} = $now; success("IPv4 address set to $ipv4") if $ipv4; success("IPv6 address set to $ipv6") if $ipv6; } @@ -3970,7 +3922,7 @@ sub dnsexit2_update_host { my $ip = delete($config{$h}{"wantipv$ipv"}) or next; $ips{$ipv} = $ip; info("updating IPv$ipv address to $ip"); - $config{$h}{"status-ipv$ipv"} = 'failed'; + $recap{$h}{"status-ipv$ipv"} = 'failed'; push(@updates, { name => $name, type => ($ipv eq '6') ? 'AAAA' : 'A', @@ -4036,11 +3988,11 @@ sub dnsexit2_update_host { return; } success($message); - $config{$h}{'mtime'} = $now; + $recap{$h}{'mtime'} = $now; keys(%ips); # Reset internal iterator. while (my ($ipv, $ip) = each(%ips)) { - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("updated IPv$ipv address to $ip"); } } @@ -4100,27 +4052,27 @@ sub nic_noip_update { for my $ip (split_by_comma($returnedips)) { next if (!$ip); my $ipv = ($ip eq ($ipv6 // '')) ? '6' : '4'; - $config{$h}{"status-ipv$ipv"} = $status; + $recap{$h}{"status-ipv$ipv"} = $status; } if ($status eq 'good') { - $config{$h}{'mtime'} = $now; + $recap{$h}{'mtime'} = $now; for my $ip (split_by_comma($returnedips)) { next if (!$ip); my $ipv = ($ip eq ($ipv6 // '')) ? '6' : '4'; - $config{$h}{"ipv$ipv"} = $ip; + $recap{$h}{"ipv$ipv"} = $ip; success("$status: IPv$ipv address set to $ip"); } } elsif (exists $errors{$status}) { if ($status eq 'nochg') { warning("$status: $errors{$status}"); - $config{$h}{'mtime'} = $now; + $recap{$h}{'mtime'} = $now; for my $ip (split_by_comma($returnedips)) { next if (!$ip); my $ipv = ($ip eq ($ipv6 // '')) ? '6' : '4'; - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{"status-ipv$ipv"} = 'good'; } } else { failed("$status: $errors{$status}"); @@ -4135,7 +4087,7 @@ sub nic_noip_update { ($scale, $units) = (60*60, 'hours') if $units eq 'h'; $sec = $wait * $scale; - $config{$h}{'wtime'} = $now + $sec; + $recap{$h}{'wtime'} = $now + $sec; warning("$status: wait $wait $units before further updates"); } else { @@ -4236,13 +4188,13 @@ sub nic_dslreports1_update { } if ($return_code !~ /NOERROR/) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed($reply); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("$return_code: IP address set to $ip"); } } @@ -4291,9 +4243,9 @@ sub nic_domeneshop_update { password => opt('password', $h), ); next if !header_ok($reply); - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("IPv$ipv address set to $ip"); } } @@ -4381,14 +4333,14 @@ sub nic_zoneedit1_update { $status_ip = $var{'IP'} if exists $var{'IP'}; if ($status eq 'SUCCESS' || ($status eq 'ERROR' && $var{'CODE'} eq '707')) { - $config{$h}{'ip'} = $status_ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $status_ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip ($status_code: $status_text)"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("$status_code: $status_text"); } shift @hosts; @@ -4491,7 +4443,7 @@ sub nic_easydns_update { # values are considered to be failures and will result in frequent retries (every # min-error-interval, which defaults to 5m). $status = 'good' if ($status // '') =~ qr/^NOERROR|OK$/; - $config{$h}{"status-ipv$ipv"} = $status; + $recap{$h}{"status-ipv$ipv"} = $status; if ($status ne 'good') { if (exists $errors{$status}) { failed("$status: $errors{$status}"); @@ -4500,8 +4452,8 @@ sub nic_easydns_update { } next; } - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; success("IPv$ipv address set to $ip"); } } @@ -4568,12 +4520,12 @@ sub nic_namecheap_update { my @reply = split /\n/, $reply; if (grep /0/i, @reply) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("invalid reply: $reply"); } } @@ -4739,7 +4691,7 @@ sub nic_nfsn_update { if ($h eq $zone) { $name = ''; } elsif ($h !~ /$zone$/) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("$h is outside zone $zone"); next; } else { @@ -4754,7 +4706,7 @@ sub nic_nfsn_update { my $list_body = encode_www_form_urlencoded({name => $name, type => 'A'}); my $list_resp = nic_nfsn_make_request($h, $list_path, 'POST', $list_body); if (!header_ok($list_resp)) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; nic_nfsn_handle_error($list_resp, $h); next; } @@ -4762,7 +4714,7 @@ sub nic_nfsn_update { $list_resp =~ s/^.*?\n\n//s; # Strip header my $list = eval { decode_json($list_resp) }; if ($@) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("JSON decoding failure"); next; } @@ -4779,7 +4731,7 @@ sub nic_nfsn_update { my $rm_resp = nic_nfsn_make_request($h, $rm_path, 'POST', $rm_body); if (!header_ok($rm_resp)) { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; nic_nfsn_handle_error($rm_resp, $h); next; } @@ -4794,12 +4746,12 @@ sub nic_nfsn_update { my $add_resp = nic_nfsn_make_request($h, $add_path, 'POST', $add_body); if (header_ok($add_resp)) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; nic_nfsn_handle_error($add_resp, $h); } } @@ -4903,11 +4855,11 @@ sub nic_njalla_update { } } if ($status eq 'good') { - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; + $recap{$h}{'ipv4'} = $ipv4 if $ipv4; + $recap{$h}{'ipv6'} = $ipv6 if $ipv6; } - $config{$h}{'status-ipv4'} = $status if $ipv4; - $config{$h}{'status-ipv6'} = $status if $ipv6; + $recap{$h}{'status-ipv4'} = $status if $ipv4; + $recap{$h}{'status-ipv6'} = $status if $ipv6; } } @@ -4968,12 +4920,12 @@ sub nic_sitelutions_update { my @reply = split /\n/, $reply; if (grep /success/i, @reply) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; warning("SENT: %s", $url) unless opt('verbose'); warning("REPLIED: %s", $reply); failed("invalid reply"); @@ -5075,8 +5027,8 @@ sub nic_freedns_update { my $ipv6 = delete $config{$h}{'wantipv6'}; if ($record_list_error ne '') { - $config{$h}{'status-ipv4'} = 'failed' if ($ipv4); - $config{$h}{'status-ipv6'} = 'failed' if ($ipv6); + $recap{$h}{'status-ipv4'} = 'failed' if ($ipv4); + $recap{$h}{'status-ipv6'} = 'failed' if ($ipv6); failed($record_list_error); next; } @@ -5094,12 +5046,12 @@ sub nic_freedns_update { } info("setting IP address to $ip"); - $config{$h}{"status-ipv$ipv"} = 'failed'; + $recap{$h}{"status-ipv$ipv"} = 'failed'; if ($ip eq $rec->[1]) { - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("update not necessary, '$type' record already set to $ip") if (!$daemon || opt('verbose')); } else { @@ -5113,9 +5065,9 @@ sub nic_freedns_update { if (header_ok($reply)) { $reply =~ s/^.*?\n\n//s; # Strip the headers. if ($reply =~ /Updated.*$h.*to.*$ip/) { - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("IPv$ipv address set to $ip"); } else { warning("SENT: %s", $url_tmpl) unless opt('verbose'); @@ -5191,8 +5143,8 @@ sub nic_1984_update { next; } - $config{$host}{'status'} = 'good'; - $config{$host}{'ip'} = $ip; + $recap{$host}{'status'} = 'good'; + $recap{$host}{'ip'} = $ip; if ($response->{msg} =~ /unaltered/) { success("skipped: IP was already set to $response->{ip}"); } else { @@ -5259,12 +5211,12 @@ sub nic_changeip_update { my @reply = split /\n/, $reply; if (grep /success/i, @reply) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; warning("SENT: %s", $url) unless opt('verbose'); warning("REPLIED: %s", $reply); failed("invalid reply"); @@ -5380,9 +5332,9 @@ sub nic_godaddy_update { failed($msg); next; } - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("updated successfully to $ip (status: $code)"); } } @@ -5444,7 +5396,7 @@ sub nic_henet_update { my ($line) = split(/\n/, $body, 2); my ($status, $returnedip) = split(/ /, lc($line)); $status = 'good' if $status eq 'nochg'; - $config{$h}{"status-ipv$ipv"} = $status; + $recap{$h}{"status-ipv$ipv"} = $status; if ($status ne 'good') { if (exists($errors{$status})) { failed("$status: $errors{$status}"); @@ -5454,8 +5406,8 @@ sub nic_henet_update { next; } success("$status: IPv$ipv address set to $returnedip"); - $config{$h}{"ipv$ipv"} = $returnedip; - $config{$h}{'mtime'} = $now; + $recap{$h}{"ipv$ipv"} = $returnedip; + $recap{$h}{'mtime'} = $now; } } } @@ -5524,8 +5476,8 @@ sub nic_mythicdyn_update { ); my $ok = header_ok($reply); if ($ok) { - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$mythver"} = "good"; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$mythver"} = "good"; success("IPv$mythver updated successfully"); } @@ -5634,12 +5586,12 @@ EoINSTR4 my $status = pipecmd($command, $instructions); if ($status eq 1) { for (@hosts) { - $config{$_}{'mtime'} = $now; + $recap{$_}{'mtime'} = $now; for my $ip ($ipv4, $ipv6) { next if (!$ip); my $ipv = ($ip eq ($ipv6 // '')) ? '6' : '4'; - $config{$_}{"ipv$ipv"} = $ip; - $config{$_}{"status-ipv$ipv"} = 'good'; + $recap{$_}{"ipv$ipv"} = $ip; + $recap{$_}{"status-ipv$ipv"} = 'good'; } } success("IPv4 address set to $ipv4") if $ipv4; @@ -5751,7 +5703,7 @@ sub nic_cloudflare_update { my $type = ($ip eq ($ipv6 // '')) ? 'AAAA' : 'A'; info("setting IPv$ipv address to $ip"); - $config{$domain}{"status-ipv$ipv"} = 'failed'; + $recap{$domain}{"status-ipv$ipv"} = 'failed'; # Get DNS 'A' or 'AAAA' record ID $url = "https://" . opt('server', $domain) . "/zones/$zone_id/dns_records?"; @@ -5790,9 +5742,9 @@ sub nic_cloudflare_update { $response = eval {decode_json(${^MATCH})}; if ($response && $response->{result}) { success("IPv$ipv address set to $ip"); - $config{$domain}{"ipv$ipv"} = $ip; - $config{$domain}{'mtime'} = $now; - $config{$domain}{"status-ipv$ipv"} = 'good'; + $recap{$domain}{"ipv$ipv"} = $ip; + $recap{$domain}{'mtime'} = $now; + $recap{$domain}{"status-ipv$ipv"} = 'good'; } else { failed("invalid json or result"); } @@ -5874,7 +5826,7 @@ sub nic_hetzner_update { my $type = ($ip eq ($ipv6 // '')) ? 'AAAA' : 'A'; info("setting IPv$ipv address to $ip"); - $config{$domain}{"status-ipv$ipv"} = 'failed'; + $recap{$domain}{"status-ipv$ipv"} = 'failed'; # Get DNS 'A' or 'AAAA' record ID $url = "https://" . opt('server', $domain) . "/records?zone_id=$zone_id"; @@ -5919,9 +5871,9 @@ sub nic_hetzner_update { $response = eval {decode_json(${^MATCH})}; if ($response && $response->{record}) { success("IPv$ipv address set to $ip"); - $config{$domain}{"ipv$ipv"} = $ip; - $config{$domain}{'mtime'} = $now; - $config{$domain}{"status-ipv$ipv"} = 'good'; + $recap{$domain}{"ipv$ipv"} = $ip; + $recap{$domain}{'mtime'} = $now; + $recap{$domain}{"status-ipv$ipv"} = 'good'; } else { failed("invalid json or result"); } @@ -6067,8 +6019,8 @@ sub nic_inwx_update { $status = 'good'; } for my $h (@hosts) { - $config{$h}{'status-ipv4'} = $status if $ipv4; - $config{$h}{'status-ipv6'} = $status if $ipv6; + $recap{$h}{'status-ipv4'} = $status if $ipv4; + $recap{$h}{'status-ipv6'} = $status if $ipv6; } if ($status ne 'good') { if (exists($errors{$status})) { @@ -6079,9 +6031,9 @@ sub nic_inwx_update { next; } for my $h (@hosts) { - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; - $config{$h}{'mtime'} = $now; + $recap{$h}{'ipv4'} = $ipv4 if $ipv4; + $recap{$h}{'ipv6'} = $ipv6 if $ipv6; + $recap{$h}{'mtime'} = $now; } success("IPv4 address set to $ipv4") if $ipv4; success("IPv6 address set to $ipv6") if $ipv6; @@ -6181,9 +6133,9 @@ sub nic_yandex_update { failed("%s", $response->{error}); next; } - $config{$host}{'ip'} = $ip; - $config{$host}{'mtime'} = $now; - $config{$host}{'status'} = 'good'; + $recap{$host}{'ip'} = $ip; + $recap{$host}{'mtime'} = $now; + $recap{$host}{'status'} = 'good'; success("updated successfully to $ip"); } } @@ -6247,11 +6199,11 @@ sub nic_duckdns_update { next; } for my $h (@hosts) { - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; - $config{$h}{'mtime'} = $now; - $config{$h}{'status-ipv4'} = 'good' if $ipv4; - $config{$h}{'status-ipv6'} = 'good' if $ipv6; + $recap{$h}{'ipv4'} = $ipv4 if $ipv4; + $recap{$h}{'ipv6'} = $ipv6 if $ipv6; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status-ipv4'} = 'good' if $ipv4; + $recap{$h}{'status-ipv6'} = 'good' if $ipv6; } success("IPv4 address set to $ipv4") if $ipv4; success("IPv6 address set to $ipv6") if $ipv6; @@ -6302,9 +6254,9 @@ sub nic_freemyip_update { failed("server said: $body"); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } } @@ -6352,9 +6304,9 @@ sub nic_ddnsfm_update { url => opt('server', $h) . "/update?key=" . opt('password', $h) . "&domain=$h&myip=$ip", ); next if !header_ok($reply); - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$ipv"} = 'good'; success("IPv$ipv address set to $ip"); } } @@ -6401,9 +6353,9 @@ sub nic_dondominio_update { failed("server said: $returned"); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } } @@ -6465,9 +6417,9 @@ sub nic_dnsmadeeasy_update { failed("server said: $err"); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } } @@ -6533,16 +6485,16 @@ sub nic_ovh_update { my @reply = split /\n/, $reply; my $returned = List::Util::first { $_ =~ /good/ || $_ =~ /nochg/ } @reply; if ($returned) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; if ($returned =~ /good/) { success("IP address set to $ip"); } else { success("skipped: IP address was already set to $ip"); } } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("server said: $reply"); } } @@ -6678,7 +6630,7 @@ sub nic_porkbun_update { warning("There are multiple applicable records. Only first record is used. Overwrite all with the same content.") if @$records > 1; if ($records->[0]{'content'} eq $ip) { - $config{$h}{"status-ipv$ipv"} = "good"; + $recap{$h}{"status-ipv$ipv"} = "good"; success("skipped: IPv$ipv address was already set to $ip"); next; } @@ -6700,7 +6652,7 @@ sub nic_porkbun_update { }), ); next if !header_ok($reply); - $config{$h}{"status-ipv$ipv"} = "good"; + $recap{$h}{"status-ipv$ipv"} = "good"; success("IPv$ipv address set to $ip"); } } @@ -6755,14 +6707,14 @@ sub nic_cloudns_update { $reply =~ s/^.*?\n\n//s; # Strip the headers. chomp($reply); if ($reply eq "The record's key is wrong!" || $reply eq "Invalid request.") { - $config{$_}{'status'} = 'failed' for @hosts; + $recap{$_}{'status'} = 'failed' for @hosts; failed($reply); next; } # There's no documentation explaining possible return values, so we assume success. - $config{$_}{'ip'} = $ip for @hosts; - $config{$_}{'mtime'} = $now for @hosts; - $config{$_}{'status'} = 'good' for @hosts; + $recap{$_}{'ip'} = $ip for @hosts; + $recap{$_}{'mtime'} = $now for @hosts; + $recap{$_}{'status'} = 'good' for @hosts; success("IP address set to $ip"); } } @@ -6811,7 +6763,7 @@ sub nic_dinahosting_update { password => opt('password', $h), url => $url, ); - $config{$h}{'status'} = 'failed'; # assume failure until otherwise determined + $recap{$h}{'status'} = 'failed'; # assume failure until otherwise determined next if !header_ok($reply); $reply =~ s/^.*?\n\n//s; # Strip the headers. if ($reply !~ /Success/i) { @@ -6822,9 +6774,9 @@ sub nic_dinahosting_update { failed("error $code: $message"); next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } } @@ -6876,20 +6828,20 @@ sub nic_directnic_update { (my $body = $reply) =~ s/^.*?\n\n//s; my $response = eval {decode_json($body)}; if (ref($response) ne 'HASH') { - $config{$h}{"status-ipv$ipv"} = 'bad'; + $recap{$h}{"status-ipv$ipv"} = 'bad'; failed("response is not a JSON object:\n$body"); next; } if ($response->{'result'} ne 'success') { - $config{$h}{"status-ipv$ipv"} = 'failed'; + $recap{$h}{"status-ipv$ipv"} = 'failed'; failed("server said:\n$body"); next; } - $config{$h}{"ipv$ipv"} = $ip; - $config{$h}{"status-ipv$ipv"} = 'good'; - $config{$h}{'mtime'} = $now; + $recap{$h}{"ipv$ipv"} = $ip; + $recap{$h}{"status-ipv$ipv"} = 'good'; + $recap{$h}{'mtime'} = $now; success("IPv$ipv address set to $ip"); } } @@ -6972,15 +6924,15 @@ sub nic_gandi_update { $reply =~ s/^.*?\n\n//s; my $response = eval { decode_json($reply) }; if (ref($response) ne 'HASH') { - $config{$h}{"status-$ipv"} = "bad"; + $recap{$h}{"status-$ipv"} = "bad"; failed("response is not a JSON object: $reply"); next; } if ($response->{'rrset_values'}->[0] eq $ip && (!defined(opt('ttl', $h)) || $response->{'rrset_ttl'} eq opt('ttl', $h))) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-$ipv"} = "good"; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-$ipv"} = "good"; success("skipped: address was already set to $ip"); next; } @@ -6995,7 +6947,7 @@ sub nic_gandi_update { }), ); if (!header_ok($reply)) { - $config{$h}{"status-$ipv"} = "bad"; + $recap{$h}{"status-$ipv"} = "bad"; $reply =~ s/^.*?\n\n//s; my $response = eval { decode_json($reply) }; if (ref($response) eq 'HASH' && ($response->{message} // '') ne '') { @@ -7005,9 +6957,9 @@ sub nic_gandi_update { } next; } - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-$ipv"} = "good"; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-$ipv"} = "good"; success("updated successfully to $ip"); } } @@ -7055,12 +7007,12 @@ sub nic_keysystems_update { last if !header_ok($reply); if ($reply =~ /code = 200/) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; failed("server said: $reply"); } } @@ -7109,11 +7061,11 @@ sub nic_regfishde_update { failed("server said: $reply"); next; } - $config{$h}{'ipv4'} = $ipv4 if $ipv4; - $config{$h}{'ipv6'} = $ipv6 if $ipv6; - $config{$h}{'status-ipv4'} = 'good' if $ipv4; - $config{$h}{'status-ipv6'} = 'good' if $ipv6; - $config{$h}{'mtime'} = $now; + $recap{$h}{'ipv4'} = $ipv4 if $ipv4; + $recap{$h}{'ipv6'} = $ipv6 if $ipv6; + $recap{$h}{'status-ipv4'} = 'good' if $ipv4; + $recap{$h}{'status-ipv6'} = 'good' if $ipv6; + $recap{$h}{'mtime'} = $now; success("IPv4 address set to $ipv4") if $ipv4; success("IPv6 address set to $ipv6") if $ipv6; } @@ -7183,12 +7135,12 @@ sub nic_enom_update { my @reply = split /\n/, $reply; if (grep /Done=true/i, @reply) { - $config{$h}{'ip'} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{'status'} = 'good'; + $recap{$h}{'ip'} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{'status'} = 'good'; success("IP address set to $ip"); } else { - $config{$h}{'status'} = 'failed'; + $recap{$h}{'status'} = 'failed'; warning("SENT: %s", $url) unless opt('verbose'); warning("REPLIED: %s", $reply); failed("invalid reply"); @@ -7252,7 +7204,7 @@ sub nic_digitalocean_update_one { my $list = eval { decode_json($list_resp) }; if ($@) { - $config{$h}{"status-$ipv"} = 'failed'; + $recap{$h}{"status-$ipv"} = 'failed'; failed("listing $ipv: JSON decoding failure"); return; } @@ -7261,7 +7213,7 @@ sub nic_digitalocean_update_one { unless ((ref($elem) eq 'HASH') && (ref ($elem = $elem->{'domain_records'}) eq 'ARRAY') && (@$elem == 1 && ref ($elem = $elem->[0]) eq 'HASH')) { - $config{$h}{"status-$ipv"} = 'failed'; + $recap{$h}{"status-$ipv"} = 'failed'; failed("listing $ipv: no record, multiple records, or malformed JSON"); return; } @@ -7283,9 +7235,9 @@ sub nic_digitalocean_update_one { return if !header_ok($update_resp); } - $config{$h}{"status-$ipv"} = 'good'; - $config{$h}{"ip-$ipv"} = $ip; - $config{$h}{"mtime"} = $now; + $recap{$h}{"status-$ipv"} = 'good'; + $recap{$h}{"ip-$ipv"} = $ip; + $recap{$h}{"mtime"} = $now; } sub nic_digitalocean_update { @@ -7393,9 +7345,9 @@ sub nic_infomaniak_update { next; } success($msg); - $config{$h}{"ipv$v"} = $ip; - $config{$h}{'mtime'} = $now; - $config{$h}{"status-ipv$v"} = 'good'; + $recap{$h}{"ipv$v"} = $ip; + $recap{$h}{'mtime'} = $now; + $recap{$h}{"status-ipv$v"} = 'good'; } } } @@ -7417,11 +7369,11 @@ sub nic_emailonly_update { logmsg(email => 1, raw => 1, join("\n", 'Host IP addresses:', map({ my $ipv4 = delete($config{$_}{'wantipv4'}); my $ipv6 = delete($config{$_}{'wantipv6'}); - $config{$_}{'status-ipv4'} = 'good' if $ipv4; - $config{$_}{'status-ipv6'} = 'good' if $ipv6; - $config{$_}{'ipv4'} = $ipv4 if $ipv4; - $config{$_}{'ipv6'} = $ipv6 if $ipv6; - $config{$_}{'mtime'} = $now; + $recap{$_}{'status-ipv4'} = 'good' if $ipv4; + $recap{$_}{'status-ipv6'} = 'good' if $ipv6; + $recap{$_}{'ipv4'} = $ipv4 if $ipv4; + $recap{$_}{'ipv6'} = $ipv6 if $ipv6; + $recap{$_}{'mtime'} = $now; sprintf('%30s %s', $_, join(' ', grep(defined($_), $ipv4, $ipv6))); } @_))); } diff --git a/t/protocol_directnic.pl b/t/protocol_directnic.pl index d6c8e54..fc24a28 100644 --- a/t/protocol_directnic.pl +++ b/t/protocol_directnic.pl @@ -52,7 +52,7 @@ my @test_cases = ( { desc => 'IPv4, good', cfg => {h1 => {urlv4 => "$hostname/dns/gateway/abc/", wantipv4 => '192.0.2.1'}}, - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, wantlogs => [ @@ -62,7 +62,7 @@ my @test_cases = ( { desc => 'IPv4, failed', cfg => {h1 => {urlv4 => "$hostname/dns/gateway/bad_token/", wantipv4 => '192.0.2.1'}}, - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'failed'}, }, wantlogs => [ @@ -72,7 +72,7 @@ my @test_cases = ( { desc => 'IPv4, bad', cfg => {h1 => {urlv4 => "$hostname/bad/path/", wantipv4 => '192.0.2.1'}}, - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'bad'}, }, wantlogs => [ @@ -82,7 +82,7 @@ my @test_cases = ( { desc => 'IPv4, unexpected response', cfg => {h1 => {urlv4 => "$hostname/unexpected/path/", wantipv4 => '192.0.2.1'}}, - wantstatus => {h1 => {}}, + wantrecap => {}, wantlogs => [ {label => 'FAILED', ctx => ['h1'], msg => qr/400 Bad Request/}, ], @@ -90,7 +90,7 @@ my @test_cases = ( { desc => 'IPv4, no urlv4', cfg => {h1 => {wantipv4 => '192.0.2.1'}}, - wantstatus => {h1 => {}}, + wantrecap => {}, wantlogs => [ {label => 'FAILED', ctx => ['h1'], msg => qr/missing urlv4 option/}, ], @@ -98,7 +98,7 @@ my @test_cases = ( { desc => 'IPv6, good', cfg => {h1 => {urlv6 => "$hostname/dns/gateway/abc/", wantipv6 => '2001:db8::1'}}, - wantstatus => { + wantrecap => { h1 => {'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, }, wantlogs => [ @@ -113,7 +113,7 @@ my @test_cases = ( wantipv4 => '192.0.2.1', wantipv6 => '2001:db8::1', }}, - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, @@ -132,7 +132,7 @@ my @test_cases = ( wantipv6 => '2001:db8::1', }}, wantips => {h1 => {wantipv4 => '192.0.2.1', wantipv6 => '2001:db8::1'}}, - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'failed', 'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, @@ -152,20 +152,14 @@ for my $tc (@test_cases) { local $ddclient::globals{verbose} = 1; my $l = Logger->new($ddclient::_l); local %ddclient::config = %{$tc->{cfg}}; + local %ddclient::recap; { local $ddclient::_l = $l; ddclient::nic_directnic_update(sort(keys(%{$tc->{cfg}}))); } - # These are the properties in %ddclient::config to check against $tc->{wantstatus}. - my %statuskeys = map(($_ => undef), qw(atime ip ipv4 ipv6 mtime status status-ipv4 status-ipv6 - wantip wantipv4 wantipv6 wtime)); - my %gotstatus; - for my $h (keys(%ddclient::config)) { - $gotstatus{$h} = {map(($_ => $ddclient::config{$h}{$_}), - grep(exists($statuskeys{$_}), keys(%{$ddclient::config{$h}})))}; - } - is_deeply(\%gotstatus, $tc->{wantstatus}, "$tc->{desc}: status") - or diag(ddclient::repr(\%ddclient::config, Names => ['*ddclient::config'])); + is_deeply(\%ddclient::recap, $tc->{wantrecap}, "$tc->{desc}: recap") + or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{wantrecap}], + Names => ['*got', '*want'])); $tc->{wantlogs} //= []; subtest("$tc->{desc}: logs" => sub { my @got = @{$l->{logs}}; diff --git a/t/protocol_dyndns2.pl b/t/protocol_dyndns2.pl index c0d407d..cd79658 100644 --- a/t/protocol_dyndns2.pl +++ b/t/protocol_dyndns2.pl @@ -45,7 +45,7 @@ my @test_cases = ( cfg => {h1 => {wantipv4 => '192.0.2.1'}}, resp => ['good'], wantquery => 'hostname=h1&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, wantlogs => [ @@ -57,7 +57,7 @@ my @test_cases = ( cfg => {h1 => {wantipv4 => '192.0.2.1'}}, resp => ['nochg'], wantquery => 'hostname=h1&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, wantlogs => [ @@ -70,7 +70,7 @@ my @test_cases = ( cfg => {h1 => {wantipv4 => '192.0.2.1'}}, resp => ['nohost'], wantquery => 'hostname=h1&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'nohost'}, }, wantlogs => [ @@ -82,7 +82,7 @@ my @test_cases = ( cfg => {h1 => {wantipv4 => '192.0.2.1'}}, resp => ['WAT'], wantquery => 'hostname=h1&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'WAT'}, }, wantlogs => [ @@ -100,7 +100,7 @@ my @test_cases = ( 'good', ], wantquery => 'hostname=h1,h2&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, @@ -122,7 +122,7 @@ my @test_cases = ( 'dnserr', ], wantquery => 'hostname=h1,h2,h3&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h3 => {'status-ipv4' => 'dnserr'}, @@ -139,7 +139,7 @@ my @test_cases = ( cfg => {h1 => {wantipv6 => '2001:db8::1'}}, resp => ['good'], wantquery => 'hostname=h1&myip=2001:db8::1', - wantstatus => { + wantrecap => { h1 => {'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, }, wantlogs => [ @@ -151,7 +151,7 @@ my @test_cases = ( cfg => {h1 => {wantipv4 => '192.0.2.1', wantipv6 => '2001:db8::1'}}, resp => ['good'], wantquery => 'hostname=h1&myip=192.0.2.1,2001:db8::1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'status-ipv6' => 'good', 'ipv6' => '2001:db8::1', 'mtime' => $ddclient::now}, @@ -173,7 +173,7 @@ my @test_cases = ( 'WAT', ], wantquery => 'hostname=h1,h2&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, @@ -191,7 +191,7 @@ my @test_cases = ( }, resp => ['abuse'], wantquery => 'hostname=h1,h2&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'abuse'}, h2 => {'status-ipv4' => 'abuse'}, }, @@ -208,7 +208,7 @@ my @test_cases = ( }, resp => ['good'], wantquery => 'hostname=h1,h2&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, }, @@ -230,7 +230,7 @@ my @test_cases = ( 'nochg', ], wantquery => 'hostname=h1,h2,h3&myip=192.0.2.1', - wantstatus => { + wantrecap => { h1 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h2 => {'status-ipv4' => 'good', 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now}, h3 => {'status-ipv4' => 'unknown'}, @@ -252,6 +252,7 @@ for my $tc (@test_cases) { local $ddclient::globals{verbose} = 1; my $l = Logger->new($ddclient::_l); local %ddclient::config; + local %ddclient::recap; $ddclient::config{$_} = { login => 'username', password => 'password', @@ -267,18 +268,9 @@ for my $tc (@test_cases) { local $ddclient::_l = $l; ddclient::nic_dyndns2_update(sort(keys(%{$tc->{cfg}}))); } - # These are the properties in %ddclient::config to check against $tc->{wantstatus}. Keys are - # explicitly listed here rather than read from $tc->{wantstatus} to ensure that entries that - # should not exist (e.g., wantipv4 and friends) are deleted (or never set). - my %statuskeys = map(($_ => undef), qw(atime ip ipv4 ipv6 mtime status status-ipv4 status-ipv6 - wantip wantipv4 wantipv6 wtime)); - my %gotstatus; - for my $h (keys(%ddclient::config)) { - $gotstatus{$h} = {map(($_ => $ddclient::config{$h}{$_}), - grep(exists($statuskeys{$_}), keys(%{$ddclient::config{$h}})))}; - } - is_deeply(\%gotstatus, $tc->{wantstatus}, "$tc->{desc}: status") - or diag(ddclient::repr(\%ddclient::config, Names => ['*ddclient::config'])); + is_deeply(\%ddclient::recap, $tc->{wantrecap}, "$tc->{desc}: recap") + or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{wantrecap}], + Names => ['*got', '*want'])); $tc->{wantlogs} //= []; subtest("$tc->{desc}: logs" => sub { my @got = @{$l->{logs}}; diff --git a/t/read_recap.pl b/t/read_recap.pl index b601eae..923d971 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -1,6 +1,5 @@ use Test::More; use File::Temp; -use Scalar::Util qw(refaddr); SKIP: { eval { require Test::Warnings; } or skip($@, 1); } eval { require 'ddclient'; } or BAIL_OUT($@); @@ -9,42 +8,26 @@ local $ddclient::globals{verbose} = 1; local %ddclient::protocols = ( protocol_a => { variables => { - 'host' => {type => ddclient::T_STRING(), recap => 1}, - 'mtime' => {type => ddclient::T_NUMBER(), recap => 1}, - 'atime' => {type => ddclient::T_NUMBER(), recap => 1}, - 'wtime' => {type => ddclient::T_NUMBER(), recap => 1}, - 'ip' => {type => ddclient::T_IP(), recap => 1}, - 'ipv4' => {type => ddclient::T_IPV4(), recap => 1}, - 'ipv6' => {type => ddclient::T_IPV6(), recap => 1}, - 'status' => {type => ddclient::T_ANY(), recap => 1}, - 'status-ipv4' => {type => ddclient::T_ANY(), recap => 1}, - 'status-ipv6' => {type => ddclient::T_ANY(), recap => 1}, - 'warned-min-error-interval' => {type => ddclient::T_ANY(), recap => 1}, - 'warned-min-interval' => {type => ddclient::T_ANY(), recap => 1}, - - 'var_a' => {type => ddclient::T_BOOL(), recap => 1}, + host => {type => ddclient::T_STRING(), recap => 1}, + var_a => {type => ddclient::T_BOOL(), recap => 1}, }, }, protocol_b => { variables => { - 'host' => {type => ddclient::T_STRING(), recap => 1}, - 'mtime' => {type => ddclient::T_NUMBER()}, # Intentionally not a recap var. - 'var_b' => {type => ddclient::T_NUMBER(), recap => 1}, + host => {type => ddclient::T_STRING(), recap => 1}, + var_b => {type => ddclient::T_NUMBER(), recap => 1}, + var_b_non_recap => {type => ddclient::T_ANY()}, }, }, ); local %ddclient::variables = (merged => {map({ %{$ddclient::protocols{$_}{variables}}; } sort(keys(%ddclient::protocols)))}); -# Sentinel value that means "this hash entry should be deleted." -my $DOES_NOT_EXIST = []; - my @test_cases = ( { desc => "ok value", cachefile_lines => ["var_a=yes host_a"], want => {host_a => {host => 'host_a', var_a => 1}}, - # No config changes are expected because `var_a` is not a "status" recap var. }, { desc => "unknown host", @@ -71,80 +54,16 @@ my @test_cases = ( host_a => {host => 'host_a', var_a => 1}, host_b => {host => 'host_b', var_b => 123}, }, - # No config changes are expected because `var_a` and `var_b` are not "status" recap vars. }, { - desc => "used to be status vars", - cachefile_lines => ["ip=192.0.2.1,status=good host_a"], - want => {host_a => {host => 'host_a', ip => '192.0.2.1', status => 'good'}}, - # No config changes are expected because `ip` and `status` are no longer "status" recap - # vars. - }, - { - desc => "status vars", - cachefile_lines => ["mtime=1234567890,atime=1234567891,wtime=1234567892,ipv4=192.0.2.1,ipv6=2001:db8::1,status-ipv4=good,status-ipv6=bad,warned-min-interval=1234567893,warned-min-error-interval=1234567894 host_a"], - want => {host_a => { - 'host' => 'host_a', - 'mtime' => 1234567890, - 'atime' => 1234567891, - 'wtime' => 1234567892, - 'ipv4' => '192.0.2.1', - 'ipv6' => '2001:db8::1', - 'status-ipv4' => 'good', - 'status-ipv6' => 'bad', - 'warned-min-interval' => 1234567893, - 'warned-min-error-interval' => 1234567894, - }}, - want_config_changes => {host_a => { - 'mtime' => 1234567890, - 'atime' => 1234567891, - 'wtime' => 1234567892, - 'ipv4' => '192.0.2.1', - 'ipv6' => '2001:db8::1', - 'status-ipv4' => 'good', - 'status-ipv6' => 'bad', - 'warned-min-interval' => 1234567893, - 'warned-min-error-interval' => 1234567894, - }}, - }, - { - desc => "unset status var clears config", - cachefile_lines => ["host_a"], - config => {host_a => { - 'mtime' => 1234567890, - 'atime' => 1234567891, - 'wtime' => 1234567892, - 'ipv4' => '192.0.2.1', - 'ipv6' => '2001:db8::1', - 'status-ipv4' => 'good', - 'status-ipv6' => 'bad', - 'warned-min-interval' => 1234567893, - 'warned-min-error-interval' => 1234567894, - 'var_a' => 1, - }}, - want => {host_a => {host => 'host_a'}}, - want_config_changes => {host_a => { - 'mtime' => $DOES_NOT_EXIST, - 'atime' => $DOES_NOT_EXIST, - 'wtime' => $DOES_NOT_EXIST, - 'ipv4' => $DOES_NOT_EXIST, - 'ipv6' => $DOES_NOT_EXIST, - 'status-ipv4' => $DOES_NOT_EXIST, - 'status-ipv6' => $DOES_NOT_EXIST, - 'warned-min-interval' => $DOES_NOT_EXIST, - 'warned-min-error-interval' => $DOES_NOT_EXIST, - # `var_a` should remain untouched. - }}, - }, - { - desc => "non-recap vars are not loaded to %recap or copied to %config", - cachefile_lines => ["mtime=1234567890 host_b"], + desc => "non-recap vars are not loaded to %recap", + cachefile_lines => ["var_b_non_recap=foo host_b"], want => {host_b => {host => 'host_b'}}, }, { desc => "non-recap vars are scrubbed from %recap", - cachefile_lines => ["mtime=1234567890 host_b"], - recap => {host_b => {host => 'host_b', mtime => 1234567891}}, + cachefile_lines => ["var_b_non_recap=foo host_b"], + recap => {host_b => {host => 'host_b', var_b_non_recap => 'foo'}}, want => {host_b => {host => 'host_b'}}, }, { @@ -166,8 +85,6 @@ for my $tc (@test_cases) { host_a => {protocol => 'protocol_a'}, host_b => {protocol => 'protocol_b'}, ); - $tc->{config} //= {}; - $want_config{$_} = {%{$want_config{$_} // {}}, %{$tc->{config}{$_}}} for keys(%{$tc->{config}}); # Deep clone %want_config so we can check for changes. local %ddclient::config; $ddclient::config{$_} = {%{$want_config{$_}}} for keys(%want_config); @@ -180,21 +97,9 @@ for my $tc (@test_cases) { or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{want}], Names => ['*got', '*want'])); } - TODO: { - local $TODO = $tc->{want_config_changes_TODO}; - $tc->{want_config_changes} //= {}; - $want_config{$_} = {%{$want_config{$_} // {}}, %{$tc->{want_config_changes}{$_}}} - for keys(%{$tc->{want_config_changes}}); - for my $h (keys(%want_config)) { - for my $k (keys(%{$want_config{$h}})) { - my $a = refaddr($want_config{$h}{$k}); - delete($want_config{$h}{$k}) if defined($a) && $a == refaddr($DOES_NOT_EXIST); - } - } - is_deeply(\%ddclient::config, \%want_config, "$tc->{desc}: %config") - or diag(ddclient::repr(Values => [\%ddclient::config, \%want_config], - Names => ['*got', '*want'])); - } + is_deeply(\%ddclient::config, \%want_config, "$tc->{desc}: %config") + or diag(ddclient::repr(Values => [\%ddclient::config, \%want_config], + Names => ['*got', '*want'])); } done_testing(); diff --git a/t/update_nics.pl b/t/update_nics.pl index c249abd..85d2126 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -54,9 +54,9 @@ local %ddclient::protocols = ( local $ddclient::_l = ddclient::pushlogctx($h); ddclient::debug('updating host'); push(@updates, [@_]); - $ddclient::config{$h}{status} = 'good'; - $ddclient::config{$h}{ip} = delete($ddclient::config{$h}{wantip}); - $ddclient::config{$h}{mtime} = $ddclient::now; + $ddclient::recap{$h}{status} = 'good'; + $ddclient::recap{$h}{ip} = delete($ddclient::config{$h}{wantip}); + $ddclient::recap{$h}{mtime} = $ddclient::now; } ddclient::debug('returning from update'); }), @@ -83,12 +83,6 @@ my @test_cases = ( 'mtime' => $ddclient::now, 'status-ipv4' => 'good', }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv4' => '192.0.2.1', - 'mtime' => $ddclient::now, - 'status-ipv4' => 'good', - }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -107,12 +101,6 @@ my @test_cases = ( 'mtime' => $ddclient::now, 'status-ipv6' => 'good', }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv6' => '2001:db8::1', - 'mtime' => $ddclient::now, - 'status-ipv6' => 'good', - }, }, { desc => 'legacy, fresh, usev6=webv6', @@ -128,12 +116,6 @@ my @test_cases = ( 'mtime' => $ddclient::now, 'status-ipv6' => 'good', }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv6' => '2001:db8::1', - 'mtime' => $ddclient::now, - 'status-ipv6' => 'good', - }, }, { desc => 'legacy, fresh, usev4=webv4 usev6=webv6', @@ -150,12 +132,6 @@ my @test_cases = ( 'mtime' => $ddclient::now, 'status-ipv4' => 'good', }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv4' => '192.0.2.1', - 'mtime' => $ddclient::now, - 'status-ipv4' => 'good', - }, }, map({ my %cfg = %{delete($_->{cfg})}; @@ -211,9 +187,6 @@ my @test_cases = ( want_recap_changes => { 'warned-min-interval' => $ddclient::now, }, - want_cfg_changes => { - 'warned-min-interval' => $ddclient::now, - }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -238,11 +211,6 @@ my @test_cases = ( 'ipv4' => '192.0.2.1', 'mtime' => $ddclient::now, }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv4' => '192.0.2.1', - 'mtime' => $ddclient::now, - }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -265,9 +233,6 @@ my @test_cases = ( want_recap_changes => { 'warned-min-error-interval' => $ddclient::now, }, - want_cfg_changes => { - 'warned-min-error-interval' => $ddclient::now, - }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -293,12 +258,6 @@ my @test_cases = ( 'mtime' => $ddclient::now, 'status-ipv4' => 'good', }, - want_cfg_changes => { - 'atime' => $ddclient::now, - 'ipv4' => '192.0.2.1', - 'mtime' => $ddclient::now, - 'status-ipv4' => 'good', - }, %$_, }; } {cfg => {use => 'web'}}, {cfg => {usev4 => 'webv4'}}), @@ -317,7 +276,6 @@ for my $tc (@test_cases) { # $cachef is an object that stringifies to a filename. local $ddclient::globals{cache} = "$cachef"; my %cfg = ( - %{$tc->{recap} // {}}, # Simulate a previous update. web => 'v4', webv4 => 'v4', webv6 => 'v6', From 803621a9eeb371592a48fb9ac2abe0045d5d63d8 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 17:32:38 -0400 Subject: [PATCH 37/45] Switch "magic constant" list of change detection vars to a var --- ddclient.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index a2c250b..538af5b 100755 --- a/ddclient.in +++ b/ddclient.in @@ -168,6 +168,8 @@ our %config; # compatibility concerns with the public `--cache` option.) our %recap; +our @recap_config_change_detection_vars = qw(static wildcard mx backupmx); + my $result; my $saved_recap; my %saved_opt; @@ -1486,7 +1488,7 @@ sub update_nics { # the impact of Hyrum's Law; if a protocol needs a variable to be updated after # the `update` method is called then that behavior should be made explicit. my $vars = $protocols{opt('protocol', $h)}{variables}; - for my $v (qw(static wildcard mx backupmx)) { + for my $v (@recap_config_change_detection_vars) { next if !$vars->{$v} || !$vars->{$v}{recap}; if (defined(my $val = opt($v, $h))) { $recap{$h}{$v} = $val; @@ -3529,7 +3531,7 @@ sub nic_updateable { $update = 1; } elsif (my @changed = grep({ my $rv = $recap{$host}{$_}; my $cv = opt($_, $host); defined($rv) && defined($cv) && $rv ne $cv; } - qw(static wildcard mx backupmx))) { + @recap_config_change_detection_vars)) { info("update forced because options changed: " . join(', ', @changed)); $update = 1; From 273af1c82131fcdf1636559f30cc478f458e7885 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 23 Aug 2024 03:36:43 -0400 Subject: [PATCH 38/45] nic_updateable: Ignore non-recap vars when detecting config change --- ddclient.in | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ddclient.in b/ddclient.in index 538af5b..e299a2e 100755 --- a/ddclient.in +++ b/ddclient.in @@ -3442,7 +3442,8 @@ EoEXAMPLE ###################################################################### sub nic_updateable { my ($host) = @_; - my $force_update = $protocols{opt('protocol', $host)}{force_update}; + my $protocol = $protocols{opt('protocol', $host)}; + my $force_update = $protocol->{force_update}; my $update = 0; my $ipv4 = $config{$host}{'wantipv4'}; my $ipv6 = $config{$host}{'wantipv6'}; @@ -3529,9 +3530,13 @@ sub nic_updateable { } elsif (defined($force_update) && $force_update->($host)) { $update = 1; - } elsif (my @changed = grep({ my $rv = $recap{$host}{$_}; my $cv = opt($_, $host); - defined($rv) && defined($cv) && $rv ne $cv; } - @recap_config_change_detection_vars)) { + } elsif (my @changed = grep({ + return 0 if (!$protocol->{variables}{$_} || + !$protocol->{variables}{$_}{recap}); + my $rv = $recap{$host}{$_}; + my $cv = opt($_, $host); + defined($rv) && defined($cv) && $rv ne $cv; + } @recap_config_change_detection_vars)) { info("update forced because options changed: " . join(', ', @changed)); $update = 1; From 2da08cceb96a4d884712fd785fab522497745b46 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 28 Aug 2024 17:50:25 -0400 Subject: [PATCH 39/45] Convert static list of config change detection vars to per-protocol --- ddclient.in | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ddclient.in b/ddclient.in index e299a2e..4c4e6cf 100755 --- a/ddclient.in +++ b/ddclient.in @@ -156,8 +156,8 @@ our %config; # it was just before the previous update attempt. Values are synchronized from `%config` to # `%recap` during each update attempt. # -# The set of config change detection variables is currently hard-coded; all other recap variables -# are assumed to be status variables. +# A protocol's set of config change detection variables can be found in the protocol's +# `force_update_if_changed` property; all other recap variables are assumed to be status variables. # # A note about terminology: This was previously named `%cache`, but "cache" implies that the # purpose is to reduce the cost or latency of data retrieval or computation, and that deletion only @@ -168,8 +168,6 @@ our %config; # compatibility concerns with the public `--cache` option.) our %recap; -our @recap_config_change_detection_vars = qw(static wildcard mx backupmx); - my $result; my $saved_recap; my %saved_opt; @@ -853,6 +851,7 @@ our %protocols = ( %{$variables{'dyndns-common-defaults'}}, 'static' => setv(T_BOOL, 0, 1, 0, undef), }, + 'force_update_if_changed' => [qw(static wildcard mx backupmx)], }, 'dyndns2' => { 'update' => \&nic_dyndns2_update, @@ -862,6 +861,7 @@ our %protocols = ( %{$variables{'dyndns-common-defaults'}}, 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), }, + 'force_update_if_changed' => [qw(wildcard mx backupmx)], }, 'easydns' => { 'update' => \&nic_easydns_update, @@ -877,6 +877,7 @@ our %protocols = ( 'script' => setv(T_STRING, 0, 0, '/dyn/generic.php', undef), 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), }, + 'force_update_if_changed' => [qw(wildcard mx backupmx)], }, 'freedns' => { 'update' => \&nic_freedns_update, @@ -1487,8 +1488,9 @@ sub update_nics { # `update` sees consistent values between `%recap` and `%config`. This reduces # the impact of Hyrum's Law; if a protocol needs a variable to be updated after # the `update` method is called then that behavior should be made explicit. - my $vars = $protocols{opt('protocol', $h)}{variables}; - for my $v (@recap_config_change_detection_vars) { + my $protocol = $protocols{$p}; + my $vars = $protocol->{variables}; + for my $v (@{$protocol->{force_update_if_changed} // []}) { next if !$vars->{$v} || !$vars->{$v}{recap}; if (defined(my $val = opt($v, $h))) { $recap{$h}{$v} = $val; @@ -3536,7 +3538,7 @@ sub nic_updateable { my $rv = $recap{$host}{$_}; my $cv = opt($_, $host); defined($rv) && defined($cv) && $rv ne $cv; - } @recap_config_change_detection_vars)) { + } @{$protocol->{force_update_if_changed} // []})) { info("update forced because options changed: " . join(', ', @changed)); $update = 1; From 1e3bebc60d39bb9db368580cb7d8f339e1135723 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 16 Jun 2024 15:22:58 -0400 Subject: [PATCH 40/45] Object-oriented protocol definitions This improves readability and will make it easier to refactor to fix issues or add features. --- ddclient.in | 473 +++++++++++++++++++++++++--------------- t/protocol_directnic.pl | 2 +- t/protocol_dnsexit2.pl | 2 +- t/protocol_dyndns2.pl | 2 +- t/read_recap.pl | 8 +- t/update_nics.pl | 10 +- t/variable_defaults.pl | 3 +- 7 files changed, 312 insertions(+), 188 deletions(-) diff --git a/ddclient.in b/ddclient.in index 4c4e6cf..8042edd 100755 --- a/ddclient.in +++ b/ddclient.in @@ -698,49 +698,149 @@ our %variables = ( }, ); -# Converts a legacy protocol update method to behave like a modern implementation by moving -# `$recap{$h}{status}` and `$recap{$h}{ip}` to the version-specific modern equivalents. -sub adapt_legacy_update { - my ($update) = @_; - return sub { - my (@hosts) = @_; +{ + package ddclient::Protocol; + + # Keyword arguments: + # * `update`: Required coderef that takes `($self, @hosts)` and updates the given hosts. + # * `examples`: Required coderef that takes `($self)` and returns a string showing + # configuration examples for using the protocol. + # * `variables`: Optional hashref of variable declarations. If omitted or `undef`, + # `$variables{'protocol-common-defaults'}` is used. + # * `force_update`: Optional coderef that takes `($self, $h)` and returns truthy to force the + # given host to update. Omitting or passing `undef` is equivalent to passing a subroutine + # that always returns falsy. + # * `force_update_if_changed`: Optional arrayref of variable names to watch for changes. If + # any of the named values in `%config` have changed since the previous update attempt + # (successful or not), the host update is forced. If omitted or `undef`, an empty array is + # used. + sub new { + my ($class, %args) = @_; + my $self = bless({%args}, $class); + # Set defaults and normalize. + $self->{variables} //= $ddclient::variables{'protocol-common-defaults'}; + $self->{variables} = {%{$self->{variables}}}; # Shallow clone. + # Delete `undef` variable declarations to make it easier to cancel previously declared + # variables. + delete($self->{variables}{$_}) for grep(!defined($self->{variables}{$_}), + keys(%{$self->{variables}})); + $self->{force_update} //= sub { return 0; }; + $self->{force_update_if_changed} //= []; + # Eliminate duplicates and non-recap variables. + my %fvs = map({ ($_ => undef); } @{$self->{force_update_if_changed}}); + $self->{force_update_if_changed} = + [grep({ $self->{variables}{$_} && $self->{variables}{$_}{recap}; } sort(keys(%fvs)))]; + return $self; + } + + sub force_update { + my ($self, $h) = @_; + my @changed = grep({ + my $rv = $ddclient::recap{$h}{$_}; + my $cv = ddclient::opt($_, $h); + return defined($rv) && defined($cv) && $rv ne $cv; + } @{$self->{force_update_if_changed}}); + if (@changed) { + ddclient::info("update forced because options changed: " . join(', ', @changed)); + return 1; + } + my $f = $self->{force_update}; + return $f if ref($f) ne 'CODE'; + return $f->($self, $h); + } + + sub update { + my ($self, @hosts) = @_; + for my $h (@hosts) { + $ddclient::recap{$h}{'atime'} = $now; + delete($ddclient::recap{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime + warned-min-interval warned-min-error-interval); + # Update the configuration change detection variables. The vars are updated regardless + # of whether the update actually succeeds because update failures should be retried at + # the error retry rate (`min-error-interval`), not forced by `force_update`. Notes + # about why the recap vars are updated here in this method: + # * The vars must not be updated if the host is not being updated because change + # detection is defined relative to the previous update attempt. In particular, + # these can't be updated when the protocol's `force_update` method is called + # because that method is not always called before an update is attempted. + # * The vars must be updated after the `force_update` method would be called so that + # `force_update` can check whether any settings have changed since the last time an + # update was attempted. + # * The vars are updated before the protocol's `update` method is called so that + # `update` sees consistent values between `%recap` and `%config`. This reduces the + # impact of Hyrum's Law; if a protocol needs a variable to be updated after the + # `update` method is called then that behavior should be made explicit. + for my $v (@{$self->{force_update_if_changed}}) { + if (defined(my $val = ddclient::opt($v, $h))) { + $ddclient::recap{$h}{$v} = $val; + } else { + # Entries in `%recap` with `undef` values are deleted to avoid needing to + # figure out how to represent `undef` in the cache file and to simplify + # testing. + delete($ddclient::recap{$h}{$v}); + } + } + } + $self->_update(@hosts); + } + + sub _update { + my $self = shift; + $self->{update}($self, @_); + } + + sub examples { + my ($self) = @_; + return $self->{examples}($self); + } +} + +{ + # A legacy protocol implementation reads `$config{$h}{wantip}` and sets `$recap{$h}{status}` + # and `$recap{$h}{ip}`, rather than reading `wantipv4` and `wantipv6` and setting + # `status-ipv4`, `status-ipv6`, `ipv4`, and `ipv6`. + package ddclient::LegacyProtocol; + use parent qw(-norequire ddclient::Protocol); + + sub _update { + my ($self, @hosts) = @_; my %ipv; for my $h (@hosts) { - $ipv{$h} = defined($config{$h}{'wantipv4'}) ? '4' : '6'; - $config{$h}{'wantip'} = delete($config{$h}{"wantipv$ipv{$h}"}); - delete($recap{$h}{$_}) for qw(ip status); + $ipv{$h} = defined($ddclient::config{$h}{'wantipv4'}) ? '4' : '6'; + $ddclient::config{$h}{'wantip'} //= delete($ddclient::config{$h}{"wantipv$ipv{$h}"}); + delete($ddclient::recap{$h}{$_}) for qw(ip status); } - $update->(@hosts); + $self->SUPER::_update(@hosts); for my $h (@hosts) { - local $_l = pushlogctx($h); - delete($config{$h}{'wantip'}); - debug( + local $ddclient::_l = ddclient::pushlogctx($h); + delete($ddclient::config{$h}{'wantip'}); + ddclient::debug( "legacy protocol; moving 'status' to 'status-ipv$ipv{$h}', 'ip' to 'ipv$ipv{$h}'"); - $recap{$h}{"status-ipv$ipv{$h}"} = delete($recap{$h}{'status'}); - $recap{$h}{"ipv$ipv{$h}"} = delete($recap{$h}{'ip'}); + $ddclient::recap{$h}{"status-ipv$ipv{$h}"} = delete($ddclient::recap{$h}{'status'}); + $ddclient::recap{$h}{"ipv$ipv{$h}"} = delete($ddclient::recap{$h}{'ip'}); } - }; + } } our %protocols = ( - '1984' => { - 'update' => adapt_legacy_update(\&nic_1984_update), + '1984' => ddclient::LegacyProtocol->new( + 'update' => \&nic_1984_update, 'examples' => \&nic_1984_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'api.1984.is', undef), }, - }, - 'changeip' => { - 'update' => adapt_legacy_update(\&nic_changeip_update), + ), + 'changeip' => ddclient::LegacyProtocol->new( + 'update' => \&nic_changeip_update, 'examples' => \&nic_changeip_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'nic.changeip.com', undef), }, - }, - 'cloudflare' => { + ), + 'cloudflare' => ddclient::Protocol->new( 'update' => \&nic_cloudflare_update, 'examples' => \&nic_cloudflare_examples, 'variables' => { @@ -750,9 +850,9 @@ our %protocols = ( 'server' => setv(T_FQDNP, 0, 0, 'api.cloudflare.com/client/v4', undef), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, - }, - 'cloudns' => { - 'update' => adapt_legacy_update(\&nic_cloudns_update), + ), + 'cloudns' => ddclient::LegacyProtocol->new( + 'update' => \&nic_cloudns_update, 'examples' => \&nic_cloudns_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -760,8 +860,8 @@ our %protocols = ( 'password' => undef, 'dynurl' => setv(T_STRING, 1, 0, undef, undef), }, - }, - 'ddns.fm' => { + ), + 'ddns.fm' => ddclient::Protocol->new( 'update' => \&nic_ddnsfm_update, 'examples' => \&nic_ddnsfm_examples, 'variables' => { @@ -769,8 +869,8 @@ our %protocols = ( 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'https://api.ddns.fm', undef), }, - }, - 'digitalocean' => { + ), + 'digitalocean' => ddclient::Protocol->new( 'update' => \&nic_digitalocean_update, 'examples' => \&nic_digitalocean_examples, 'variables' => { @@ -779,9 +879,9 @@ our %protocols = ( 'server' => setv(T_FQDNP, 0, 0, 'api.digitalocean.com', undef), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, - }, - 'dinahosting' => { - 'update' => adapt_legacy_update(\&nic_dinahosting_update), + ), + 'dinahosting' => ddclient::LegacyProtocol->new( + 'update' => \&nic_dinahosting_update, 'examples' => \&nic_dinahosting_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -789,8 +889,8 @@ our %protocols = ( 'script' => setv(T_STRING, 0, 0, '/special/api.php', undef), 'server' => setv(T_FQDNP, 0, 0, 'dinahosting.com', undef), }, - }, - 'directnic' => { + ), + 'directnic' => ddclient::Protocol->new( 'update' => \&nic_directnic_update, 'examples' => \&nic_directnic_examples, 'variables' => { @@ -800,41 +900,41 @@ our %protocols = ( 'urlv4' => setv(T_URL, 0, 0, undef, undef), 'urlv6' => setv(T_URL, 0, 0, undef, undef), }, - }, - 'dnsmadeeasy' => { - 'update' => adapt_legacy_update(\&nic_dnsmadeeasy_update), + ), + 'dnsmadeeasy' => ddclient::LegacyProtocol->new( + 'update' => \&nic_dnsmadeeasy_update, 'examples' => \&nic_dnsmadeeasy_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'script' => setv(T_STRING, 0, 0, '/servlet/updateip', undef), 'server' => setv(T_FQDNP, 0, 0, 'cp.dnsmadeeasy.com', undef), }, - }, - 'dondominio' => { - 'update' => adapt_legacy_update(\&nic_dondominio_update), + ), + 'dondominio' => ddclient::LegacyProtocol->new( + 'update' => \&nic_dondominio_update, 'examples' => \&nic_dondominio_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'dondns.dondominio.com', undef), }, - }, - 'dslreports1' => { - 'update' => adapt_legacy_update(\&nic_dslreports1_update), + ), + 'dslreports1' => ddclient::LegacyProtocol->new( + 'update' => \&nic_dslreports1_update, 'examples' => \&nic_dslreports1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'www.dslreports.com', undef), }, - }, - 'domeneshop' => { + ), + 'domeneshop' => ddclient::Protocol->new( 'update' => \&nic_domeneshop_update, 'examples' => \&nic_domeneshop_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'api.domeneshop.no', undef), }, - }, - 'duckdns' => { + ), + 'duckdns' => ddclient::Protocol->new( 'update' => \&nic_duckdns_update, 'examples' => \&nic_duckdns_examples, 'variables' => { @@ -842,9 +942,9 @@ our %protocols = ( 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'www.duckdns.org', undef), }, - }, - 'dyndns1' => { - 'update' => adapt_legacy_update(\&nic_dyndns1_update), + ), + 'dyndns1' => ddclient::LegacyProtocol->new( + 'update' => \&nic_dyndns1_update, 'examples' => \&nic_dyndns1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -852,8 +952,8 @@ our %protocols = ( 'static' => setv(T_BOOL, 0, 1, 0, undef), }, 'force_update_if_changed' => [qw(static wildcard mx backupmx)], - }, - 'dyndns2' => { + ), + 'dyndns2' => ddclient::Protocol->new( 'update' => \&nic_dyndns2_update, 'examples' => \&nic_dyndns2_examples, 'variables' => { @@ -862,8 +962,8 @@ our %protocols = ( 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), }, 'force_update_if_changed' => [qw(wildcard mx backupmx)], - }, - 'easydns' => { + ), + 'easydns' => ddclient::Protocol->new( 'update' => \&nic_easydns_update, 'examples' => \&nic_easydns_examples, 'variables' => { @@ -878,8 +978,8 @@ our %protocols = ( 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), }, 'force_update_if_changed' => [qw(wildcard mx backupmx)], - }, - 'freedns' => { + ), + 'freedns' => ddclient::Protocol->new( 'update' => \&nic_freedns_update, 'examples' => \&nic_freedns_examples, 'variables' => { @@ -887,17 +987,17 @@ our %protocols = ( 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), 'server' => setv(T_FQDNP, 0, 0, 'freedns.afraid.org', undef), }, - }, - 'freemyip' => { - 'update' => adapt_legacy_update(\&nic_freemyip_update), + ), + 'freemyip' => ddclient::LegacyProtocol->new( + 'update' => \&nic_freemyip_update, 'examples' => \&nic_freemyip_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'freemyip.com', undef), }, - }, - 'gandi' => { + ), + 'gandi' => ddclient::Protocol->new( 'update' => \&nic_gandi_update, 'examples' => \&nic_gandi_examples, 'variables' => { @@ -910,8 +1010,8 @@ our %protocols = ( 'ttl' => setv(T_DELAY, 0, 0, undef, interval('5m')), 'zone' => setv(T_FQDN, 1, 0, undef, undef), } - }, - 'godaddy' => { + ), + 'godaddy' => ddclient::Protocol->new( 'update' => \&nic_godaddy_update, 'examples' => \&nic_godaddy_examples, 'variables' => { @@ -921,8 +1021,8 @@ our %protocols = ( 'ttl' => setv(T_NUMBER, 0, 0, 600, undef), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, - }, - 'he.net' => { + ), + 'he.net' => ddclient::Protocol->new( 'update' => \&nic_henet_update, 'examples' => \&nic_henet_examples, 'variables' => { @@ -931,8 +1031,8 @@ our %protocols = ( 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), 'server' => setv(T_FQDNP, 0, 0, 'dyn.dns.he.net', undef), }, - }, - 'hetzner' => { + ), + 'hetzner' => ddclient::Protocol->new( 'update' => \&nic_hetzner_update, 'examples' => \&nic_hetzner_examples, 'variables' => { @@ -943,8 +1043,8 @@ our %protocols = ( 'ttl' => setv(T_NUMBER, 0, 0, 60, 60), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, - }, - 'inwx' => { + ), + 'inwx' => ddclient::Protocol->new( 'update' => \&nic_inwx_update, 'examples' => \&nic_inwx_examples, 'variables' => { @@ -952,8 +1052,8 @@ our %protocols = ( 'server' => setv(T_FQDNP, 0, 0, 'dyndns.inwx.com', undef), 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), }, - }, - 'mythicdyn' => { + ), + 'mythicdyn' => ddclient::Protocol->new( 'update' => \&nic_mythicdyn_update, 'examples' => \&nic_mythicdyn_examples, 'variables' => { @@ -961,18 +1061,18 @@ our %protocols = ( 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), 'server' => setv(T_FQDNP, 0, 0, 'api.mythic-beasts.com', undef), }, - }, - 'namecheap' => { - 'update' => adapt_legacy_update(\&nic_namecheap_update), + ), + 'namecheap' => ddclient::LegacyProtocol->new( + 'update' => \&nic_namecheap_update, 'examples' => \&nic_namecheap_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), 'server' => setv(T_FQDNP, 0, 0, 'dynamicdns.park-your-domain.com', undef), }, - }, - 'nfsn' => { - 'update' => adapt_legacy_update(\&nic_nfsn_update), + ), + 'nfsn' => ddclient::LegacyProtocol->new( + 'update' => \&nic_nfsn_update, 'examples' => \&nic_nfsn_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -981,8 +1081,8 @@ our %protocols = ( 'ttl' => setv(T_NUMBER, 0, 0, 300, undef), 'zone' => setv(T_FQDN, 1, 0, undef, undef), }, - }, - 'njalla' => { + ), + 'njalla' => ddclient::Protocol->new( 'update' => \&nic_njalla_update, 'examples' => \&nic_njalla_examples, 'variables' => { @@ -991,16 +1091,16 @@ our %protocols = ( 'server' => setv(T_FQDNP, 0, 0, 'njal.la', undef), 'quietreply' => setv(T_BOOL, 0, 0, 0, undef), }, - }, - 'noip' => { + ), + 'noip' => ddclient::Protocol->new( 'update' => \&nic_noip_update, 'examples' => \&nic_noip_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'dynupdate.no-ip.com', undef), }, - }, - 'nsupdate' => { + ), + 'nsupdate' => ddclient::Protocol->new( 'update' => \&nic_nsupdate_update, 'examples' => \&nic_nsupdate_examples, 'variables' => { @@ -1010,17 +1110,17 @@ our %protocols = ( 'ttl' => setv(T_NUMBER, 0, 0, 600, undef), 'zone' => setv(T_STRING, 1, 0, undef, undef), }, - }, - 'ovh' => { - 'update' => adapt_legacy_update(\&nic_ovh_update), + ), + 'ovh' => ddclient::LegacyProtocol->new( + 'update' => \&nic_ovh_update, 'examples' => \&nic_ovh_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), 'server' => setv(T_FQDNP, 0, 0, 'www.ovh.com', undef), }, - }, - 'porkbun' => { + ), + 'porkbun' => ddclient::Protocol->new( 'update' => \&nic_porkbun_update, 'examples' => \&nic_porkbun_examples, 'variables' => { @@ -1032,27 +1132,27 @@ our %protocols = ( 'root-domain' => setv(T_FQDN, 0, 0, undef, undef), 'on-root-domain' => setv(T_BOOL, 0, 0, 0, undef), }, - }, - 'sitelutions' => { - 'update' => adapt_legacy_update(\&nic_sitelutions_update), + ), + 'sitelutions' => ddclient::LegacyProtocol->new( + 'update' => \&nic_sitelutions_update, 'examples' => \&nic_sitelutions_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'www.sitelutions.com', undef), 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), }, - }, - 'yandex' => { - 'update' => adapt_legacy_update(\&nic_yandex_update), + ), + 'yandex' => ddclient::LegacyProtocol->new( + 'update' => \&nic_yandex_update, 'examples' => \&nic_yandex_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), 'server' => setv(T_FQDNP, 0, 0, 'pddimp.yandex.ru', undef), }, - }, - 'zoneedit1' => { - 'update' => adapt_legacy_update(\&nic_zoneedit1_update), + ), + 'zoneedit1' => ddclient::LegacyProtocol->new( + 'update' => \&nic_zoneedit1_update, 'examples' => \&nic_zoneedit1_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, @@ -1060,17 +1160,17 @@ our %protocols = ( 'server' => setv(T_FQDNP, 0, 0, 'dynamic.zoneedit.com', undef), 'zone' => setv(T_FQDN, 0, 0, undef, undef), }, - }, - 'keysystems' => { - 'update' => adapt_legacy_update(\&nic_keysystems_update), + ), + 'keysystems' => ddclient::LegacyProtocol->new( + 'update' => \&nic_keysystems_update, 'examples' => \&nic_keysystems_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'dynamicdns.key-systems.net', undef), }, - }, - 'dnsexit2' => { + ), + 'dnsexit2' => ddclient::Protocol->new( 'update' => \&nic_dnsexit2_update, 'examples' => \&nic_dnsexit2_examples, 'variables' => { @@ -1082,8 +1182,8 @@ our %protocols = ( 'ttl' => setv(T_NUMBER, 0, 0, 5, 0), 'zone' => setv(T_STRING, 0, 0, undef, undef), }, - }, - 'regfishde' => { + ), + 'regfishde' => ddclient::Protocol->new( 'update' => \&nic_regfishde_update, 'examples' => \&nic_regfishde_examples, 'variables' => { @@ -1091,25 +1191,25 @@ our %protocols = ( 'login' => undef, 'server' => setv(T_FQDNP, 0, 0, 'dyndns.regfish.de', undef), }, - }, - 'enom' => { - 'update' => adapt_legacy_update(\&nic_enom_update), + ), + 'enom' => ddclient::LegacyProtocol->new( + 'update' => \&nic_enom_update, 'examples' => \&nic_enom_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => setv(T_FQDNP, 0, 0, 'dynamic.name-services.com', undef), 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), }, - }, - 'infomaniak' => { + ), + 'infomaniak' => ddclient::Protocol->new( 'update' => \&nic_infomaniak_update, 'examples' => \&nic_infomaniak_examples, 'variables' => { %{$variables{'protocol-common-defaults'}}, 'server' => undef, }, - }, - 'emailonly' => { + ), + 'emailonly' => ddclient::Protocol->new( 'update' => \&nic_emailonly_update, 'examples' => \&nic_emailonly_examples, 'variables' => { @@ -1119,13 +1219,8 @@ our %protocols = ( # Change default to never re-notify if IP address has not changed. 'max-interval' => setv(T_DELAY, 0, 0, 'inf', 0), }, - }, + ), ); -# Delete undefined variables to make it easier to cancel previously defined variables. -for my $proto (values(%protocols)) { - my $vars = $proto->{variables}; - delete(@$vars{grep(!defined($vars->{$_}), keys(%$vars))}); -} $variables{'merged'} = { map({ %{$protocols{$_}{'variables'}} } keys(%protocols)), %{$variables{'dyndns-common-defaults'}}, @@ -1370,8 +1465,6 @@ sub update_nics { for my $p (sort keys %protocols) { my (@hosts, %ipsv4, %ipsv6) = (); - my $update = $protocols{$p}{'update'}; - for my $h (sort keys %config) { local $_l = pushlogctx($h); next if opt('protocol', $h) ne $p; @@ -1471,38 +1564,7 @@ sub update_nics { if (@hosts) { $0 = sprintf("%s - updating %s", $program, join(',', @hosts)); local $_l = pushlogctx($p); - for my $h (@hosts) { - $recap{$h}{'atime'} = $now; - delete($recap{$h}{$_}) for qw(status-ipv4 status-ipv6 wtime - warned-min-interval warned-min-error-interval); - # Update configuration change detection variables in `%recap` with the latest - # values in `%config`. Notes about why this is done here: - # * The vars must not be updated if the host is not being updated because change - # detection is defined relative to the previous update attempt. In particular, - # these can't be updated when the protocol's `force_update` method is called - # because that method is not always called before an update is attempted. - # * The vars must be updated after the `force_update` method would be called so - # that `force_update` can check whether any settings have changed since the - # last time an update was attempted. - # * The vars are updated before the protocol's `update` method is called so that - # `update` sees consistent values between `%recap` and `%config`. This reduces - # the impact of Hyrum's Law; if a protocol needs a variable to be updated after - # the `update` method is called then that behavior should be made explicit. - my $protocol = $protocols{$p}; - my $vars = $protocol->{variables}; - for my $v (@{$protocol->{force_update_if_changed} // []}) { - next if !$vars->{$v} || !$vars->{$v}{recap}; - if (defined(my $val = opt($v, $h))) { - $recap{$h}{$v} = $val; - } else { - # Entries in `%recap` with `undef` values are deleted to avoid needing to - # figure out how to represent `undef` in the cache file and to simplify - # testing. - delete($recap{$h}{$v}); - } - } - } - &$update(@hosts); + $protocols{$p}->update(@hosts); for my $h (@hosts) { delete($config{$h}{$_}) for qw(wantipv4 wantipv6); } @@ -3380,15 +3442,11 @@ sub nic_examples { my $examples = ""; my $separator = ""; for my $p (sort keys %protocols) { - my $subr = $protocols{$p}{'examples'}; - my $example; - - if (defined($subr) && ($example = &$subr())) { - chomp($example); - $examples .= $example; - $examples .= "\n\n$separator"; - $separator = "\n"; - } + my $example = $protocols{$p}->examples(); + chomp($example); + $examples .= $example; + $examples .= "\n\n$separator"; + $separator = "\n"; } my $intro = <<"EoEXAMPLE"; == CONFIGURING ${program} @@ -3445,7 +3503,6 @@ EoEXAMPLE sub nic_updateable { my ($host) = @_; my $protocol = $protocols{opt('protocol', $host)}; - my $force_update = $protocol->{force_update}; my $update = 0; my $ipv4 = $config{$host}{'wantipv4'}; my $ipv6 = $config{$host}{'wantipv6'}; @@ -3530,18 +3587,8 @@ sub nic_updateable { $update = 1; } - } elsif (defined($force_update) && $force_update->($host)) { + } elsif ($protocol->force_update($host)) { $update = 1; - } elsif (my @changed = grep({ - return 0 if (!$protocol->{variables}{$_} || - !$protocol->{variables}{$_}{recap}); - my $rv = $recap{$host}{$_}; - my $cv = opt($_, $host); - defined($rv) && defined($cv) && $rv ne $cv; - } @{$protocol->{force_update_if_changed} // []})) { - info("update forced because options changed: " . join(', ', @changed)); - $update = 1; - } else { if (opt('verbose')) { success("skipped update because IPv4 address is already set to $ipv4") @@ -3601,6 +3648,7 @@ sub header_ok { ## nic_dyndns1_examples ###################################################################### sub nic_dyndns1_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dyndns1' @@ -3638,6 +3686,7 @@ EoEXAMPLE ## nic_dyndns1_update ###################################################################### sub nic_dyndns1_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -3691,6 +3740,7 @@ sub nic_dyndns1_update { ## nic_dyndns2_examples ###################################################################### sub nic_dyndns2_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dyndns2' @@ -3736,6 +3786,7 @@ EoEXAMPLE ## nic_dyndns2_update ###################################################################### sub nic_dyndns2_update { + my $self = shift; my %errors = ( 'badauth' => 'Bad authorization (username or password)', 'badsys' => 'The system parameter given was not valid', @@ -3864,6 +3915,7 @@ sub nic_dyndns2_update { ## nic_dnsexit2_examples ###################################################################### sub nic_dnsexit2_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dnsexit2' @@ -3903,6 +3955,7 @@ EoEXAMPLE ## ###################################################################### sub nic_dnsexit2_update { + my $self = shift; # The DNSExit API does not support updating hosts with different zones at the same time, # handling update per host. for my $h (@_) { @@ -4011,6 +4064,7 @@ sub dnsexit2_update_host { ## Note: uses same features as nic_dyndns2_update, less return codes ###################################################################### sub nic_noip_update { + my $self = shift; my %errors = ( 'badauth' => 'Invalid username or password', 'badagent' => 'Invalid user agent', @@ -4110,6 +4164,7 @@ sub nic_noip_update { ## nic_noip_examples ###################################################################### sub nic_noip_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'noip' @@ -4138,6 +4193,7 @@ EoEXAMPLE ## nic_dslreports1_examples ###################################################################### sub nic_dslreports1_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dslreports1' @@ -4166,6 +4222,7 @@ EoEXAMPLE ## nic_dslreports1_update ###################################################################### sub nic_dslreports1_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -4212,6 +4269,7 @@ sub nic_dslreports1_update { ## nic_domeneshop_examples ###################################################################### sub nic_domeneshop_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'domeneshop' @@ -4240,6 +4298,7 @@ EoEXAMPLE ## nic_domeneshop_update ###################################################################### sub nic_domeneshop_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); for my $ipv ('4', '6') { @@ -4265,6 +4324,7 @@ sub nic_domeneshop_update { ## nic_zoneedit1_examples ###################################################################### sub nic_zoneedit1_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'zoneedit1' @@ -4301,6 +4361,7 @@ EoEXAMPLE # ###################################################################### sub nic_zoneedit1_update { + my $self = shift; for my $group (group_hosts_by(\@_, qw(login password server zone wantip))) { my @hosts = @{$group->{hosts}}; my %groupcfg = %{$group->{cfg}}; @@ -4372,6 +4433,7 @@ sub nic_zoneedit1_update { ## nic_easydns_examples ###################################################################### sub nic_easydns_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'easydns' @@ -4417,6 +4479,7 @@ EoEXAMPLE ## nic_easydns_update ###################################################################### sub nic_easydns_update { + my $self = shift; my %errors = ( 'NOACCESS' => 'Authentication failed. This happens if the username/password OR host or domain are wrong.', 'NO_AUTH' => 'Authentication failed. This happens if the username/password OR host or domain are wrong.', @@ -4472,6 +4535,7 @@ sub nic_easydns_update { ## nic_namecheap_examples ###################################################################### sub nic_namecheap_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'namecheap' @@ -4507,6 +4571,7 @@ EoEXAMPLE ## ###################################################################### sub nic_namecheap_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -4546,6 +4611,7 @@ sub nic_namecheap_update { ## nic_nfsn_examples ###################################################################### sub nic_nfsn_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'nfsn' @@ -4691,6 +4757,7 @@ sub nic_nfsn_handle_error { ## ###################################################################### sub nic_nfsn_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -4772,6 +4839,7 @@ sub nic_nfsn_update { ## nic_njalla_examples ###################################################################### sub nic_njalla_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'njalla' @@ -4804,6 +4872,7 @@ EoEXAMPLE ## response contains "code 200" on succesful completion ###################################################################### sub nic_njalla_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); # Read input params @@ -4876,6 +4945,7 @@ sub nic_njalla_update { ## nic_sitelutions_examples ###################################################################### sub nic_sitelutions_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'sitelutions' @@ -4910,6 +4980,7 @@ EoEXAMPLE ## ###################################################################### sub nic_sitelutions_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -4948,6 +5019,7 @@ sub nic_sitelutions_update { ## nic_freedns_examples ###################################################################### sub nic_freedns_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'freedns' @@ -4998,6 +5070,7 @@ EoEXAMPLE ## failure. ###################################################################### sub nic_freedns_update { + my $self = shift; # Separate the records that are currently holding IPv4 addresses from the records that are # currently holding IPv6 addresses so that we can avoid switching a record to a different # address type. @@ -5093,6 +5166,7 @@ sub nic_freedns_update { ## nic_1984_examples ###################################################################### sub nic_1984_examples { + my $self = shift; return <<"EoEXAMPLE"; o '1984' @@ -5123,6 +5197,7 @@ EoEXAMPLE ## - lookup: if domain or subdomain was not found lookup will contain a list of names tried ###################################################################### sub nic_1984_update { + my $self = shift; for my $host (@_) { local $_l = pushlogctx($host); my $ip = delete $config{$host}{'wantip'}; @@ -5166,6 +5241,7 @@ sub nic_1984_update { ## nic_changeip_examples ###################################################################### sub nic_changeip_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'changeip' @@ -5198,6 +5274,7 @@ EoEXAMPLE ## ###################################################################### sub nic_changeip_update { + my $self = shift; ## update each configured host for my $h (@_) { local $_l = pushlogctx($h); @@ -5240,6 +5317,7 @@ sub nic_changeip_update { ## ###################################################################### sub nic_godaddy_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'godaddy' @@ -5275,6 +5353,7 @@ EoEXAMPLE ## nic_godaddy_update ###################################################################### sub nic_godaddy_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); my $zone = opt('zone', $h); @@ -5356,6 +5435,7 @@ sub nic_godaddy_update { ## ###################################################################### sub nic_henet_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'he.net' @@ -5378,6 +5458,7 @@ EoEXAMPLE ## nic_henet_update ###################################################################### sub nic_henet_update { + my $self = shift; my %errors = ( 'badauth' => 'Bad authorization (username or password)', 'badsys' => 'The system parameter given was not valid', @@ -5428,6 +5509,7 @@ sub nic_henet_update { ## ###################################################################### sub nic_mythicdyn_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'mythicdyn' @@ -5465,6 +5547,7 @@ EoEXAMPLE ## nic_mythicdyn_update ###################################################################### sub nic_mythicdyn_update { + my $self = shift; # Update each configured host. for my $h (@_) { local $_l = pushlogctx($h); @@ -5501,6 +5584,7 @@ sub nic_mythicdyn_update { ## nic_nsupdate_examples ###################################################################### sub nic_nsupdate_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'nsupdate' @@ -5549,6 +5633,7 @@ EoEXAMPLE ## by Daniel Roethlisberger ###################################################################### sub nic_nsupdate_update { + my $self = shift; for my $group (group_hosts_by(\@_, qw(login password server tcp zone wantipv4 wantipv6))) { my @hosts = @{$group->{hosts}}; my %groupcfg = %{$group->{cfg}}; @@ -5620,6 +5705,7 @@ EoINSTR4 ## ###################################################################### sub nic_cloudflare_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'cloudflare' @@ -5660,6 +5746,7 @@ EoEXAMPLE ## nic_cloudflare_update ###################################################################### sub nic_cloudflare_update { + my $self = shift; for my $group (group_hosts_by(\@_, qw(login password))) { my @hosts = @{$group->{hosts}}; my %groupcfg = %{$group->{cfg}}; @@ -5769,6 +5856,7 @@ sub nic_cloudflare_update { ## ###################################################################### sub nic_hetzner_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'hetzner' @@ -5792,6 +5880,7 @@ EoEXAMPLE ## nic_hetzner_update ###################################################################### sub nic_hetzner_update { + my $self = shift; for my $domain (@_) { local $_l = pushlogctx($domain); my $headers = "Auth-API-Token: " . opt('password', $domain) . "\n"; @@ -5894,6 +5983,7 @@ sub nic_hetzner_update { ## nic_inwx_examples ###################################################################### sub nic_inwx_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'inwx' @@ -5941,6 +6031,7 @@ EoEXAMPLE ## nic_inwx_update ###################################################################### sub nic_inwx_update { + my $self = shift; my %errors = ( 'badauth' => 'Bad authorization (username or password)', 'badsys' => 'The system parameter given was not valid', @@ -6054,6 +6145,7 @@ sub nic_inwx_update { ## nic_yandex_examples ###################################################################### sub nic_yandex_examples { + my $self = shift; return <<"EoEXAMPLE"; o Yandex @@ -6088,6 +6180,7 @@ EoEXAMPLE ## ###################################################################### sub nic_yandex_update { + my $self = shift; for my $host (@_) { local $_l = pushlogctx($host); my $ip = delete $config{$host}{'wantip'}; @@ -6153,6 +6246,7 @@ sub nic_yandex_update { ## nic_duckdns_examples ###################################################################### sub nic_duckdns_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'duckdns' @@ -6182,6 +6276,7 @@ EoEXAMPLE ## response contains OK or KO ###################################################################### sub nic_duckdns_update { + my $self = shift; for my $group (group_hosts_by(\@_, qw(password server wantipv4 wantipv6))) { my @hosts = @{$group->{hosts}}; my %groupcfg = %{$group->{cfg}}; @@ -6223,6 +6318,7 @@ sub nic_duckdns_update { ## nic_freemyip_examples ###################################################################### sub nic_freemyip_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'freemyip' @@ -6251,6 +6347,7 @@ EoEXAMPLE ## response contains OK or ERROR ###################################################################### sub nic_freemyip_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; @@ -6274,6 +6371,7 @@ sub nic_freemyip_update { ## nic_ddnsfm_examples ###################################################################### sub nic_ddnsfm_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'ddns.fm' @@ -6299,6 +6397,7 @@ EoEXAMPLE ## nic_ddnsfm_update ###################################################################### sub nic_ddnsfm_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); # ddns.fm behavior as of 2024-07-14: @@ -6325,6 +6424,7 @@ sub nic_ddnsfm_update { ## nic_dondominio_examples ###################################################################### sub nic_dondominio_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dondominio' The 'dondominio' protocol is used by DNS service offered by www.dondominio.com/ . @@ -6349,6 +6449,7 @@ EoEXAMPLE ###################################################################### sub nic_dondominio_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; @@ -6373,6 +6474,7 @@ sub nic_dondominio_update { ## nic_dnsmadeeasy_examples ###################################################################### sub nic_dnsmadeeasy_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dnsmadeeasy' @@ -6401,6 +6503,7 @@ EoEXAMPLE ## nic_dnsmadeeasy_update ###################################################################### sub nic_dnsmadeeasy_update { + my $self = shift; my %messages = ( 'error-auth' => 'Invalid username or password, or invalid IP syntax', 'error-auth-suspend' => 'User has had their account suspended due to complaints or misuse of the service.', @@ -6437,6 +6540,7 @@ sub nic_dnsmadeeasy_update { ## nic_ovh_examples ###################################################################### sub nic_ovh_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'ovh' @@ -6465,6 +6569,7 @@ EoEXAMPLE ## nic_ovh_update ###################################################################### sub nic_ovh_update { + my $self = shift; ## update each configured host ## should improve to update in one pass for my $h (@_) { @@ -6513,6 +6618,7 @@ sub nic_ovh_update { ## nic_porkbun_examples ###################################################################### sub nic_porkbun_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'porkbun' @@ -6586,6 +6692,7 @@ EoEXAMPLE ## nic_porkbun_update ###################################################################### sub nic_porkbun_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); my ($sub_domain, $domain); @@ -6668,6 +6775,7 @@ sub nic_porkbun_update { } sub nic_cloudns_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'cloudns' @@ -6695,6 +6803,7 @@ EoEXAMPLE } sub nic_cloudns_update { + my $self = shift; for my $group (group_hosts_by(\@_, qw(dynurl wantip))) { my @hosts = @{$group->{hosts}}; my %groupcfg = %{$group->{cfg}}; @@ -6732,6 +6841,7 @@ sub nic_cloudns_update { ## nic_dinahosting_examples ###################################################################### sub nic_dinahosting_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'dinahosting' @@ -6756,6 +6866,7 @@ EoEXAMPLE ## nic_dinahosting_update ###################################################################### sub nic_dinahosting_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); my $ip = delete $config{$h}{'wantip'}; @@ -6794,6 +6905,7 @@ sub nic_dinahosting_update { ## nic_directnic_examples ###################################################################### sub nic_directnic_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'directnic' @@ -6818,6 +6930,7 @@ EoEXAMPLE ## nic_directnic_update ###################################################################### sub nic_directnic_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); for my $ipv ('4', '6') { @@ -6861,6 +6974,7 @@ sub nic_directnic_update { ## by Jimmy Thrasibule ###################################################################### sub nic_gandi_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'gandi' @@ -6908,6 +7022,7 @@ EoEXAMPLE ## nic_gandi_update ###################################################################### sub nic_gandi_update { + my $self = shift; for my $h (@_) { local $_l = pushlogctx($h); for my $ipv ('ipv4', 'ipv6') { @@ -6978,6 +7093,7 @@ sub nic_gandi_update { ## nic_keysystems_examples ###################################################################### sub nic_keysystems_examples { + my $self = shift; return < 1, raw => 1, join("\n", 'Host IP addresses:', map({ @@ -7391,6 +7517,7 @@ sub nic_emailonly_update { ## nic_emailonly_examples ###################################################################### sub nic_emailonly_examples { + my $self = shift; return <<"EoEXAMPLE"; o 'emailonly' diff --git a/t/protocol_directnic.pl b/t/protocol_directnic.pl index fc24a28..855500d 100644 --- a/t/protocol_directnic.pl +++ b/t/protocol_directnic.pl @@ -155,7 +155,7 @@ for my $tc (@test_cases) { local %ddclient::recap; { local $ddclient::_l = $l; - ddclient::nic_directnic_update(sort(keys(%{$tc->{cfg}}))); + ddclient::nic_directnic_update(undef, sort(keys(%{$tc->{cfg}}))); } is_deeply(\%ddclient::recap, $tc->{wantrecap}, "$tc->{desc}: recap") or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{wantrecap}], diff --git a/t/protocol_dnsexit2.pl b/t/protocol_dnsexit2.pl index ec5137d..76071ea 100644 --- a/t/protocol_dnsexit2.pl +++ b/t/protocol_dnsexit2.pl @@ -41,7 +41,7 @@ my $ua = LWP::UserAgent->new; sub test_nic_dnsexit2_update { my ($config, @hostnames) = @_; %ddclient::config = %$config; - ddclient::nic_dnsexit2_update(@hostnames); + ddclient::nic_dnsexit2_update(undef, @hostnames); } sub decode_and_sort_array { diff --git a/t/protocol_dyndns2.pl b/t/protocol_dyndns2.pl index cd79658..6cc4e2a 100644 --- a/t/protocol_dyndns2.pl +++ b/t/protocol_dyndns2.pl @@ -266,7 +266,7 @@ for my $tc (@test_cases) { map("line: $_", @{$tc->{resp}}), ); local $ddclient::_l = $l; - ddclient::nic_dyndns2_update(sort(keys(%{$tc->{cfg}}))); + ddclient::nic_dyndns2_update(undef, sort(keys(%{$tc->{cfg}}))); } is_deeply(\%ddclient::recap, $tc->{wantrecap}, "$tc->{desc}: recap") or diag(ddclient::repr(Values => [\%ddclient::recap, $tc->{wantrecap}], diff --git a/t/read_recap.pl b/t/read_recap.pl index 923d971..db44bb5 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -6,19 +6,19 @@ eval { require 'ddclient'; } or BAIL_OUT($@); local $ddclient::globals{debug} = 1; local $ddclient::globals{verbose} = 1; local %ddclient::protocols = ( - protocol_a => { + protocol_a => ddclient::Protocol->new( variables => { host => {type => ddclient::T_STRING(), recap => 1}, var_a => {type => ddclient::T_BOOL(), recap => 1}, }, - }, - protocol_b => { + ), + protocol_b => ddclient::Protocol->new( variables => { host => {type => ddclient::T_STRING(), recap => 1}, var_b => {type => ddclient::T_NUMBER(), recap => 1}, var_b_non_recap => {type => ddclient::T_ANY()}, }, - }, + ), ); local %ddclient::variables = (merged => {map({ %{$ddclient::protocols{$_}{variables}}; } sort(keys(%ddclient::protocols)))}); diff --git a/t/update_nics.pl b/t/update_nics.pl index 85d2126..e772021 100644 --- a/t/update_nics.pl +++ b/t/update_nics.pl @@ -47,8 +47,9 @@ local %ddclient::protocols = ( # The `legacy` protocol reads the legacy `wantip` property and sets the legacy `ip` and `status` # properties. (Modern protocol implementations read `wantipv4` and `wantipv6` and set `ipv4`, # `ipv6`, `status-ipv4`, and `status-ipv6`.) It always succeeds. - legacy => { - update => ddclient::adapt_legacy_update(sub { + legacy => ddclient::LegacyProtocol->new( + update => sub { + my $self = shift; ddclient::debug('in update'); for my $h (@_) { local $ddclient::_l = ddclient::pushlogctx($h); @@ -59,11 +60,8 @@ local %ddclient::protocols = ( $ddclient::recap{$h}{mtime} = $ddclient::now; } ddclient::debug('returning from update'); - }), - variables => { - %{$ddclient::variables{'protocol-common-defaults'}}, }, - }, + ), ); my @test_cases = ( diff --git a/t/variable_defaults.pl b/t/variable_defaults.pl index 8d82347..537b642 100644 --- a/t/variable_defaults.pl +++ b/t/variable_defaults.pl @@ -77,8 +77,7 @@ my @use_test_cases = ( ); for my $tc (@use_test_cases) { my $desc = "'use' dynamic default: $tc->{desc}"; - local %ddclient::protocols = - (protocol => {variables => $ddclient::variables{'protocol-common-defaults'}}); + local %ddclient::protocols = (protocol => ddclient::Protocol->new()); local %ddclient::variables = (merged => { 'protocol' => $ddclient::variables{'merged'}{'protocol'}, 'use' => $ddclient::variables{'protocol-common-defaults'}{'use'}, From a18efcbe320a70e325e9587fa863f377df2b0ad1 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 1 Sep 2024 01:22:27 -0400 Subject: [PATCH 41/45] Force an update if a host's protocol changes --- ddclient.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddclient.in b/ddclient.in index 8042edd..2b1bb0a 100755 --- a/ddclient.in +++ b/ddclient.in @@ -583,7 +583,7 @@ our %variables = ( 'cache' => setv(T_FILE, 0, 0, "$cachedir/$program.cache", undef), 'pid' => setv(T_FILE, 0, 0, undef, undef), 'proxy' => setv(T_FQDNP, 0, 0, undef, undef), - 'protocol' => setv(T_PROTO, 0, 0, 'dyndns2', undef), + 'protocol' => setv(T_PROTO, 0, 1, 'dyndns2', undef), 'timeout' => setv(T_DELAY, 0, 0, interval('120s'), interval('120s')), 'force' => setv(T_BOOL, 0, 0, 0, undef), @@ -727,7 +727,7 @@ our %variables = ( $self->{force_update} //= sub { return 0; }; $self->{force_update_if_changed} //= []; # Eliminate duplicates and non-recap variables. - my %fvs = map({ ($_ => undef); } @{$self->{force_update_if_changed}}); + my %fvs = map({ ($_ => undef); } @{$self->{force_update_if_changed}}, 'protocol'); $self->{force_update_if_changed} = [grep({ $self->{variables}{$_} && $self->{variables}{$_}{recap}; } sort(keys(%fvs)))]; return $self; From ac67c04f1372c1348cf23a5fe640f63f04daf6c9 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 30 Aug 2024 17:19:55 -0400 Subject: [PATCH 42/45] _read_config: Check `host` definedness, not existence --- ddclient.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddclient.in b/ddclient.in index 2b1bb0a..f5bb1ec 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1893,7 +1893,7 @@ sub _read_config { } } %passwords = (); - if (exists($locals{'host'})) { + if (defined($locals{'host'})) { $args[0] = (@args ? "$args[0]," : '') . $locals{host}; } ## accumulate globals From 0f1ea65fd775890f0fbad30413b6d7894ca287ce Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 30 Aug 2024 18:23:08 -0400 Subject: [PATCH 43/45] _read_config: Minor refactor for readability and maintainability --- ddclient.in | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/ddclient.in b/ddclient.in index f5bb1ec..6d097e7 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1896,32 +1896,20 @@ sub _read_config { if (defined($locals{'host'})) { $args[0] = (@args ? "$args[0]," : '') . $locals{host}; } - ## accumulate globals - if (!@args) { + my ($host, $login, $password) = @args; + $locals{'login'} = $login if defined $login; + $locals{'password'} = $password if defined $password; + my @hosts = split_by_comma($host); + if (!@hosts) { %globals = (%globals, %locals); next; } - - ## process this host definition - my ($host, $login, $password) = @args; - - ## add in any globals.. - %locals = (%globals, %locals); - - ## override login and password if specified the old way. - $locals{'login'} = $login if defined $login; - $locals{'password'} = $password if defined $password; - - ## allow {host} to be a comma separated list of hosts - for my $h (split_by_comma($host)) { + for my $h (@hosts) { # TODO: Shouldn't %locals go after $config{h}? Later lines should override earlier # lines, no? Otherwise, later assignments will have a mixed effect: assignments to new # variables will take effect but assignments to variables that already have a value - # will not. One problem with swapping the order: due to the `%locals = (%globals, - # %locals)` line above, any values in %globals would override any locals in the - # previous host line. - $config{$h} = {%locals, %{$config{$h} // {}}}; - $config{$h}{'host'} = $h; + # will not. + $config{$h} = {%globals, %locals, %{$config{$h} // {}}, 'host' => $h}; } } close(FD); From 76afbb667305e1234496dded214e00f57aef575e Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Fri, 30 Aug 2024 19:40:22 -0400 Subject: [PATCH 44/45] _read_config: Add infrastructure for host-dependent validation This is a preparatory step for separating recap variables from config variables. --- ddclient.in | 90 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 36 deletions(-) diff --git a/ddclient.in b/ddclient.in index 6d097e7..5e479cd 100755 --- a/ddclient.in +++ b/ddclient.in @@ -1776,6 +1776,49 @@ sub _read_config { my %globals = (); my %config = (); my $content = ''; + # Checks a single key-value pair for a host and writes the normalized value output to an + # optional output ref. + my $check = sub { + # TODO: The checks below are incorrect for a few reasons: + # + # * It is not protocol-aware. Different protocols can have different sets of variables, + # with different normalization and validation behaviors. + # * It does not check for missing required values. Note that a later line or a + # command-line argument might define a missing required value. + # * A later line or command-line argument might override an invalid value, changing it to + # valid. + # + # Fixing this is not simple. Values should be checked and normalized after processing the + # entire file and command-line arguments, but then we lose line number context. The line + # number could be recorded along with each variable's value to provide context in case + # validation fails, but that adds considerable complexity. Fortunately, a variable's type + # is unlikely to change even if the protocol changes (`$variables{merged}{$var}{type}` will + # likely equal `$protocols{$proto}{variables}{$var}{type}` for each variable `$var` for + # each protocol `$proto`), so normalizing and validating values on a line-by-line basis is + # likely to be safe. + my ($h, $k, $v, $normout) = @_; + if (!exists $variables{'merged'}{$k}) { + warning("unrecognized keyword"); + return 0; + } + my $def = $variables{'merged'}{$k}; + my $norm; + if (!eval { $norm = check_value($v, $def); 1; }) { + my $vf = defined($v) ? "'$v'" : ''; + warning("invalid value $vf: $@"); + return 0; + } + $$normout = $norm if defined($normout); + return 1; + }; + # Calls $check on each entry in the given hashref, deleting any entries that don't pass. + my $checkall = sub { + my ($h, $l) = @_; + for my $k (keys(%$l)) { + local $_l = pushlogctx($k); + delete($l->{$k}) if !$check->($h, $k, $l->{$k}, \$l->{$k}); + } + }; local *FD; if (!open(FD, "< $file")) { @@ -1858,39 +1901,8 @@ sub _read_config { ($_, %locals) = parse_assignments($_); s/\s*,\s*/,/g; my @args = split; - - ## verify that keywords are valid...and check the value for my $k (keys %locals) { $locals{$k} = $passwords{$k} if defined $passwords{$k}; - # TODO: The checks below are incorrect for a few reasons: - # - # * It is not protocol-aware. Different protocols can have different sets of - # variables, with different normalization and validation behaviors. - # * It does not check for missing required values. Note that a later line or a - # command-line argument might define a missing required value. - # * A later line or command-line argument might override an invalid value, changing - # it to valid. - # - # Fixing this is not simple. Values should be checked and normalized after processing - # the entire file and command-line arguments, but then we lose line number context. - # The line number could be recorded along with each variable's value to provide context - # in case validation fails, but that adds considerable complexity. Fortunately, a - # variable's type is unlikely to change even if the protocol changes - # (`$variables{merged}{$var}{type}` will likely equal - # `$protocols{$proto}{variables}{$var}{type}` for each variable `$var` for each - # protocol `$proto`), so normalizing and validating values on a line-by-line basis is - # likely to be safe. - if (!exists $variables{'merged'}{$k}) { - warning("unrecognized keyword '%s' (ignored)", $k); - delete $locals{$k}; - next; - } - my $def = $variables{'merged'}{$k}; - if (!eval { $locals{$k} = check_value($locals{$k}, $def); 1; }) { - warning("invalid variable value '$k=$locals{$k}': $@"); - delete $locals{$k}; - next; - } } %passwords = (); if (defined($locals{'host'})) { @@ -1901,15 +1913,21 @@ sub _read_config { $locals{'password'} = $password if defined $password; my @hosts = split_by_comma($host); if (!@hosts) { + local $_l = pushlogctx('globals'); + $checkall->(undef, \%locals); %globals = (%globals, %locals); next; } for my $h (@hosts) { - # TODO: Shouldn't %locals go after $config{h}? Later lines should override earlier - # lines, no? Otherwise, later assignments will have a mixed effect: assignments to new - # variables will take effect but assignments to variables that already have a value - # will not. - $config{$h} = {%globals, %locals, %{$config{$h} // {}}, 'host' => $h}; + local $_l = pushlogctx($h); + # Shallow clone of %locals for host-dependent validation and normalization. + my %hlocals = %locals; + $checkall->($h, \%hlocals); + # TODO: Shouldn't `%hlocals` go after `$config{h}`? Later lines should override + # earlier lines, no? Otherwise, later assignments will have a mixed effect: + # assignments to new variables will take effect but assignments to variables that + # already have a value will not. + $config{$h} = {%globals, %hlocals, %{$config{$h} // {}}, 'host' => $h}; } } close(FD); From 695c3c4be82d7bffab65657912c4079f204d238c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Thu, 29 Aug 2024 18:16:04 -0400 Subject: [PATCH 45/45] Separate recap variables from configuration variables --- ddclient.in | 729 ++++++++++++++++++++++------------------- t/read_recap.pl | 18 +- t/variable_defaults.pl | 20 +- 3 files changed, 409 insertions(+), 358 deletions(-) diff --git a/ddclient.in b/ddclient.in index 5e479cd..463661b 100755 --- a/ddclient.in +++ b/ddclient.in @@ -142,11 +142,10 @@ our %config; # service-specific and protocol-independent mechanisms such as `min-interval`. This data is # persisted in the cache file (`--cache`) so that it survives ddclient restarts. This hash maps a # hostname to a hashref with entries that map variable names to values. Only entries for the -# host's "recap variables" -- the host's protocol's variables with a truthy `recap` property -- are +# host's "recap variables" -- those declared in the host's protocol's `recapvars` property -- are # included. # -# `%recap` is independent of `%config`, although they both share the same set of variable -# declarations. There are two classes of recap variables: +# There are two classes of recap variables: # * "Status" variables: These track update success/failure, the IP address of the last successful # update, etc. These do not hold configuration data; they are unrelated to any entries in # `%config`. @@ -206,6 +205,58 @@ sub T_IPV4 { 'ipv4' } sub T_IPV6 { 'ipv6' } sub T_POSTS { 'postscript' } +# `%recapvars` contains common recap variable declarations that are used by multiple protocols (see +# the protocol `recapvars` property). +our %recapvars = ( + 'common' => { + 'host' => T_STRING, + 'protocol' => T_PROTO, + # The IPv4 address most recently saved at the DDNS service. + # TODO: This is independent of the `ipv4` configuration setting. Rename the `%recap` + # status variable to something like `saved-ipv4` to avoid confusion with the `%config` + # setting variable. + 'ipv4' => T_IPV4, + # As `ipv4`, but for an IPv6 address. + 'ipv6' => T_IPV6, + # Timestamp (seconds since epoch) indicating the earliest time the next update is + # permitted. + # TODO: Create a timestamp type and change this to that type. + 'wtime' => T_NUMBER, + # Timestamp (seconds since epoch) indicating when an IP address was last sent to the DDNS + # service, even if the IP address was not different from what was already stored. + # TODO: Create a timestamp type and change this to that type. + 'mtime' => T_NUMBER, + # Timestamp (seconds since epoch) of the most recent attempt to update the DDNS service + # (including attempts to update with the same IP address). This equals mtime if the most + # recent attempt was successful, otherwise it will be more recent than mtime. + # TODO: Create a timestamp type and change this to that type. + 'atime' => T_NUMBER, + # Disposition of the most recent (or currently in progress) attempt to update the DDNS + # service with the IP address in `wantipv4`. Anything other than `good`, including undef, + # is treated as a failure. + 'status-ipv4' => T_ANY, + # As `status-ipv4`, but with `wantipv6`. + 'status-ipv6' => T_ANY, + # Timestamp (seconds since epoch) of the most recent attempt that would have been made had + # `min-interval` not inhibited the attempt. This is reset to 0 once an attempt is actually + # made. This is used as a boolean to suppress repeated warnings to the user that indicate + # that `min-interval` has inhibited an update attempt. + # TODO: Change to a boolean and rename to improve readability. + 'warned-min-interval' => T_ANY, + # Timestamp (seconds since epoch) of the most recent attempt that would have been made had + # `min-error-interval` not inhibited the attempt. This is reset to 0 once an attempt is + # actually made. This is used as a boolean to suppress repeated warnings to the user that + # indicate that `min-error-interval` has inhibited an update attempt. + # TODO: Change to a boolean and rename to improve readability. + 'warned-min-error-interval' => T_ANY, + }, + 'dyndns-common' => { + 'backupmx' => T_BOOL, + 'mx' => T_FQDN, + 'wildcard' => T_BOOL, + }, +); + ## strategies for obtaining an ip address. our %builtinweb = ( 'dyndns' => {'url' => 'http://checkip.dyndns.org/', 'skip' => 'Current IP Address:'}, @@ -570,131 +621,90 @@ sub setv { return { 'type' => shift, 'required' => shift, - 'recap' => shift, 'default' => shift, 'minimum' => shift, }; } -our %variables = ( +our %cfgvars = ( 'global-defaults' => { - 'daemon' => setv(T_DELAY, 0, 0, $daemon_default, interval('60s')), - 'foreground' => setv(T_BOOL, 0, 0, 0, undef), - 'file' => setv(T_FILE, 0, 0, "$etc/$program.conf", undef), - 'cache' => setv(T_FILE, 0, 0, "$cachedir/$program.cache", undef), - 'pid' => setv(T_FILE, 0, 0, undef, undef), - 'proxy' => setv(T_FQDNP, 0, 0, undef, undef), - 'protocol' => setv(T_PROTO, 0, 1, 'dyndns2', undef), + 'daemon' => setv(T_DELAY, 0, $daemon_default, interval('60s')), + 'foreground' => setv(T_BOOL, 0, 0, undef), + 'file' => setv(T_FILE, 0, "$etc/$program.conf", undef), + 'cache' => setv(T_FILE, 0, "$cachedir/$program.cache", undef), + 'pid' => setv(T_FILE, 0, undef, undef), + 'proxy' => setv(T_FQDNP, 0, undef, undef), + 'protocol' => setv(T_PROTO, 0, 'dyndns2', undef), - 'timeout' => setv(T_DELAY, 0, 0, interval('120s'), interval('120s')), - 'force' => setv(T_BOOL, 0, 0, 0, undef), - 'ssl' => setv(T_BOOL, 0, 0, 1, undef), - 'syslog' => setv(T_BOOL, 0, 0, 0, undef), - 'facility' => setv(T_STRING,0, 0, 'daemon', undef), - 'priority' => setv(T_STRING,0, 0, 'notice', undef), - 'mail' => setv(T_EMAIL, 0, 0, undef, undef), - 'mail-failure' => setv(T_EMAIL, 0, 0, undef, undef), - 'max-warn' => setv(T_NUMBER,0, 0, 1, undef), + 'timeout' => setv(T_DELAY, 0, interval('120s'), interval('120s')), + 'force' => setv(T_BOOL, 0, 0, undef), + 'ssl' => setv(T_BOOL, 0, 1, undef), + 'syslog' => setv(T_BOOL, 0, 0, undef), + 'facility' => setv(T_STRING,0, 'daemon', undef), + 'priority' => setv(T_STRING,0, 'notice', undef), + 'mail' => setv(T_EMAIL, 0, undef, undef), + 'mail-failure' => setv(T_EMAIL, 0, undef, undef), + 'max-warn' => setv(T_NUMBER,0, 1, undef), - 'exec' => setv(T_BOOL, 0, 0, 1, undef), - 'debug' => setv(T_BOOL, 0, 0, 0, undef), - 'verbose' => setv(T_BOOL, 0, 0, 0, undef), - 'quiet' => setv(T_BOOL, 0, 0, 0, undef), - 'test' => setv(T_BOOL, 0, 0, 0, undef), + 'exec' => setv(T_BOOL, 0, 1, undef), + 'debug' => setv(T_BOOL, 0, 0, undef), + 'verbose' => setv(T_BOOL, 0, 0, undef), + 'quiet' => setv(T_BOOL, 0, 0, undef), + 'test' => setv(T_BOOL, 0, 0, undef), - 'postscript' => setv(T_POSTS, 0, 0, undef, undef), - 'ssl_ca_dir' => setv(T_FILE, 0, 0, undef, undef), - 'ssl_ca_file' => setv(T_FILE, 0, 0, undef, undef), - 'redirect' => setv(T_NUMBER,0, 0, 0, undef) + 'postscript' => setv(T_POSTS, 0, undef, undef), + 'ssl_ca_dir' => setv(T_FILE, 0, undef, undef), + 'ssl_ca_file' => setv(T_FILE, 0, undef, undef), + 'redirect' => setv(T_NUMBER,0, 0, undef) }, 'protocol-common-defaults' => { - 'server' => setv(T_FQDNP, 0, 0, 'members.dyndns.org', undef), - 'login' => setv(T_LOGIN, 1, 0, undef, undef), - 'password' => setv(T_PASSWD,1, 0, undef, undef), - 'host' => setv(T_STRING,1, 1, undef, undef), + 'server' => setv(T_FQDNP, 0, 'members.dyndns.org', undef), + 'login' => setv(T_LOGIN, 1, undef, undef), + 'password' => setv(T_PASSWD,1, undef, undef), + 'host' => setv(T_STRING,1, undef, undef), - 'use' => setv(T_USE, 0, 0, sub { + 'use' => setv(T_USE, 0, sub { my ($h) = @_; return "'disabled' if '--usev4' or '--usev6' is enabled, otherwise 'ip'" if ($h // '') eq ''; return 'disabled' if opt('usev4', $h) ne 'disabled' || opt('usev6', $h) ne 'disabled'; return 'ip'; }, undef), - 'usev4' => setv(T_USEV4, 0, 0, 'disabled', undef), - 'usev6' => setv(T_USEV6, 0, 0, 'disabled', undef), - 'if' => setv(T_IF, 0, 0, 'ppp0', undef), - 'ifv4' => setv(T_IF, 0, 0, 'default', undef), - 'ifv6' => setv(T_IF, 0, 0, 'default', undef), - 'web' => setv(T_STRING,0, 0, 'dyndns', undef), - 'web-skip' => setv(T_STRING,0, 0, undef, undef), - 'web-ssl-validate' => setv(T_BOOL, 0, 0, 1, undef), - 'webv4' => setv(T_STRING,0, 0, 'ipify-ipv4', undef), - 'webv4-skip' => setv(T_STRING,0, 0, undef, undef), - 'webv6' => setv(T_STRING,0, 0, 'ipify-ipv6', undef), - 'webv6-skip' => setv(T_STRING,0, 0, undef, undef), - 'fw' => setv(T_ANY, 0, 0, undef, undef), - 'fw-skip' => setv(T_STRING,0, 0, undef, undef), - 'fw-login' => setv(T_LOGIN, 0, 0, undef, undef), - 'fw-password' => setv(T_PASSWD,0, 0, undef, undef), - 'fw-ssl-validate' => setv(T_BOOL, 0, 0, 1, undef), - 'fwv4' => setv(T_ANY, 0, 0, undef, undef), - 'fwv4-skip' => setv(T_STRING,0, 0, undef, undef), - 'fwv6' => setv(T_ANY, 0, 0, undef, undef), - 'fwv6-skip' => setv(T_STRING,0, 0, undef, undef), - 'cmd' => setv(T_PROG, 0, 0, undef, undef), - 'cmd-skip' => setv(T_STRING,0, 0, undef, undef), - 'cmdv4' => setv(T_PROG, 0, 0, undef, undef), - 'cmdv6' => setv(T_PROG, 0, 0, undef, undef), - 'min-interval' => setv(T_DELAY, 0, 0, interval('30s'), 0), - 'max-interval' => setv(T_DELAY, 0, 0, interval('25d'), 0), - 'min-error-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - - # The desired IP address (IPv4 or IPv6, but almost always IPv4) that should be saved at the - # DDNS service (when `use=ip`). - 'ip' => setv(T_IP, 0, 0, undef, undef), - # As a recap value, this is the IPv4 address most recently saved at the DDNS service. As a - # setting, this is the desired IPv4 address that should be saved at the DDNS service. - # TODO: The use of `ipv4` as a recap status variable is independent of the use of `ipv4` as - # a configuration setting. Rename the `%recap` status variable to something like - # `saved-ipv4` to avoid confusion with the `%config` setting variable. - 'ipv4' => setv(T_IPV4, 0, 1, undef, undef), - # As `ipv4`, but for an IPv6 address. - 'ipv6' => setv(T_IPV6, 0, 1, undef, undef), - # Timestamp (seconds since epoch) indicating the earliest time the next update is - # permitted. - # TODO: Create a timestamp type and change this to that type. - 'wtime' => setv(T_NUMBER,0, 1, undef, undef), - # Timestamp (seconds since epoch) indicating when an IP address was last sent to the DDNS - # service, even if the IP address was not different from what was already stored. - # TODO: Create a timestamp type and change this to that type. - 'mtime' => setv(T_NUMBER,0, 1, 0, undef), - # Timestamp (seconds since epoch) of the most recent attempt to update the DDNS service - # (including attempts to update with the same IP address). This equals mtime if the most - # recent attempt was successful, otherwise it will be more recent than mtime. - # TODO: Create a timestamp type and change this to that type. - 'atime' => setv(T_NUMBER,0, 1, 0, undef), - # Disposition of the most recent (or currently in progress) attempt to update the DDNS - # service with the IP address in `wantipv4`. Anything other than `good`, including undef, - # is treated as a failure. - 'status-ipv4' => setv(T_ANY, 0, 1, undef, undef), - # As `status-ipv4`, but with `wantipv6`. - 'status-ipv6' => setv(T_ANY, 0, 1, undef, undef), - # Timestamp (seconds since epoch) of the most recent attempt that would have been made had - # `min-interval` not inhibited the attempt. This is reset to 0 once an attempt is actually - # made. This is used as a boolean to suppress repeated warnings to the user that indicate - # that `min-interval` has inhibited an update attempt. - # TODO: Change to a boolean and rename to improve readability. - 'warned-min-interval' => setv(T_ANY, 0, 1, undef, undef), - # Timestamp (seconds since epoch) of the most recent attempt that would have been made had - # `min-error-interval` not inhibited the attempt. This is reset to 0 once an attempt is - # actually made. This is used as a boolean to suppress repeated warnings to the user that - # indicate that `min-error-interval` has inhibited an update attempt. - # TODO: Change to a boolean and rename to improve readability. - 'warned-min-error-interval' => setv(T_ANY, 0, 1, undef, undef), + 'usev4' => setv(T_USEV4, 0, 'disabled', undef), + 'usev6' => setv(T_USEV6, 0, 'disabled', undef), + 'if' => setv(T_IF, 0, 'ppp0', undef), + 'ifv4' => setv(T_IF, 0, 'default', undef), + 'ifv6' => setv(T_IF, 0, 'default', undef), + 'web' => setv(T_STRING,0, 'dyndns', undef), + 'web-skip' => setv(T_STRING,0, undef, undef), + 'web-ssl-validate' => setv(T_BOOL, 0, 1, undef), + 'webv4' => setv(T_STRING,0, 'ipify-ipv4', undef), + 'webv4-skip' => setv(T_STRING,0, undef, undef), + 'webv6' => setv(T_STRING,0, 'ipify-ipv6', undef), + 'webv6-skip' => setv(T_STRING,0, undef, undef), + 'fw' => setv(T_ANY, 0, undef, undef), + 'fw-skip' => setv(T_STRING,0, undef, undef), + 'fw-login' => setv(T_LOGIN, 0, undef, undef), + 'fw-password' => setv(T_PASSWD,0, undef, undef), + 'fw-ssl-validate' => setv(T_BOOL, 0, 1, undef), + 'fwv4' => setv(T_ANY, 0, undef, undef), + 'fwv4-skip' => setv(T_STRING,0, undef, undef), + 'fwv6' => setv(T_ANY, 0, undef, undef), + 'fwv6-skip' => setv(T_STRING,0, undef, undef), + 'cmd' => setv(T_PROG, 0, undef, undef), + 'cmd-skip' => setv(T_STRING,0, undef, undef), + 'cmdv4' => setv(T_PROG, 0, undef, undef), + 'cmdv6' => setv(T_PROG, 0, undef, undef), + 'min-interval' => setv(T_DELAY, 0, interval('30s'), 0), + 'max-interval' => setv(T_DELAY, 0, interval('25d'), 0), + 'min-error-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'ip' => setv(T_IP, 0, undef, undef), + 'ipv4' => setv(T_IPV4, 0, undef, undef), + 'ipv6' => setv(T_IPV6, 0, undef, undef), }, 'dyndns-common-defaults' => { - 'backupmx' => setv(T_BOOL, 0, 1, 0, undef), - 'mx' => setv(T_FQDN, 0, 1, undef, undef), - 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), + 'backupmx' => setv(T_BOOL, 0, 0, undef), + 'mx' => setv(T_FQDN, 0, undef, undef), + 'wildcard' => setv(T_BOOL, 0, 0, undef), }, ); @@ -705,8 +715,10 @@ our %variables = ( # * `update`: Required coderef that takes `($self, @hosts)` and updates the given hosts. # * `examples`: Required coderef that takes `($self)` and returns a string showing # configuration examples for using the protocol. - # * `variables`: Optional hashref of variable declarations. If omitted or `undef`, - # `$variables{'protocol-common-defaults'}` is used. + # * `cfgvars`: Optional hashref of configuration variable declarations. If omitted or + # `undef`, `$cfgvars{'protocol-common-defaults'}` is used. + # * `recapvars`: Optional hashref of recap variable declarations. If omitted or `undef`, + # `$recapvars{'common'}` is used. # * `force_update`: Optional coderef that takes `($self, $h)` and returns truthy to force the # given host to update. Omitting or passing `undef` is equivalent to passing a subroutine # that always returns falsy. @@ -718,18 +730,21 @@ our %variables = ( my ($class, %args) = @_; my $self = bless({%args}, $class); # Set defaults and normalize. - $self->{variables} //= $ddclient::variables{'protocol-common-defaults'}; - $self->{variables} = {%{$self->{variables}}}; # Shallow clone. - # Delete `undef` variable declarations to make it easier to cancel previously declared - # variables. - delete($self->{variables}{$_}) for grep(!defined($self->{variables}{$_}), - keys(%{$self->{variables}})); + $self->{cfgvars} //= $ddclient::cfgvars{'protocol-common-defaults'}; + $self->{recapvars} //= $ddclient::recapvars{'common'}; + for my $varset (qw(cfgvars recapvars)) { + $self->{$varset} = {%{$self->{$varset}}}; # Shallow clone. + # Delete `undef` variable declarations to make it easier to cancel previously declared + # variables. + delete($self->{$varset}{$_}) for grep(!defined($self->{$varset}{$_}), + keys(%{$self->{$varset}})); + } $self->{force_update} //= sub { return 0; }; $self->{force_update_if_changed} //= []; # Eliminate duplicates and non-recap variables. my %fvs = map({ ($_ => undef); } @{$self->{force_update_if_changed}}, 'protocol'); $self->{force_update_if_changed} = - [grep({ $self->{variables}{$_} && $self->{variables}{$_}{recap}; } sort(keys(%fvs)))]; + [grep({ $self->{cfgvars}{$_} && $self->{recapvars}{$_}; } sort(keys(%fvs)))]; return $self; } @@ -826,406 +841,419 @@ our %protocols = ( '1984' => ddclient::LegacyProtocol->new( 'update' => \&nic_1984_update, 'examples' => \&nic_1984_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'api.1984.is', undef), + 'server' => setv(T_FQDNP, 0, 'api.1984.is', undef), }, ), 'changeip' => ddclient::LegacyProtocol->new( 'update' => \&nic_changeip_update, 'examples' => \&nic_changeip_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'nic.changeip.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'nic.changeip.com', undef), }, ), 'cloudflare' => ddclient::Protocol->new( 'update' => \&nic_cloudflare_update, 'examples' => \&nic_cloudflare_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'login' => setv(T_LOGIN, 0, 0, 'token', undef), - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'api.cloudflare.com/client/v4', undef), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'login' => setv(T_LOGIN, 0, 'token', undef), + 'min-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'server' => setv(T_FQDNP, 0, 'api.cloudflare.com/client/v4', undef), + 'zone' => setv(T_FQDN, 1, undef, undef), }, ), 'cloudns' => ddclient::LegacyProtocol->new( 'update' => \&nic_cloudns_update, 'examples' => \&nic_cloudns_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, 'password' => undef, - 'dynurl' => setv(T_STRING, 1, 0, undef, undef), + 'dynurl' => setv(T_STRING, 1, undef, undef), }, ), 'ddns.fm' => ddclient::Protocol->new( 'update' => \&nic_ddnsfm_update, 'examples' => \&nic_ddnsfm_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'https://api.ddns.fm', undef), + 'server' => setv(T_FQDNP, 0, 'https://api.ddns.fm', undef), }, ), 'digitalocean' => ddclient::Protocol->new( 'update' => \&nic_digitalocean_update, 'examples' => \&nic_digitalocean_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'api.digitalocean.com', undef), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'server' => setv(T_FQDNP, 0, 'api.digitalocean.com', undef), + 'zone' => setv(T_FQDN, 1, undef, undef), }, ), 'dinahosting' => ddclient::LegacyProtocol->new( 'update' => \&nic_dinahosting_update, 'examples' => \&nic_dinahosting_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-error-interval' => setv(T_DELAY, 0, 0, interval('8m'), 0), - 'script' => setv(T_STRING, 0, 0, '/special/api.php', undef), - 'server' => setv(T_FQDNP, 0, 0, 'dinahosting.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-error-interval' => setv(T_DELAY, 0, interval('8m'), 0), + 'script' => setv(T_STRING, 0, '/special/api.php', undef), + 'server' => setv(T_FQDNP, 0, 'dinahosting.com', undef), }, ), 'directnic' => ddclient::Protocol->new( 'update' => \&nic_directnic_update, 'examples' => \&nic_directnic_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, 'password' => undef, - 'urlv4' => setv(T_URL, 0, 0, undef, undef), - 'urlv6' => setv(T_URL, 0, 0, undef, undef), + 'urlv4' => setv(T_URL, 0, undef, undef), + 'urlv6' => setv(T_URL, 0, undef, undef), }, ), 'dnsmadeeasy' => ddclient::LegacyProtocol->new( 'update' => \&nic_dnsmadeeasy_update, 'examples' => \&nic_dnsmadeeasy_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'script' => setv(T_STRING, 0, 0, '/servlet/updateip', undef), - 'server' => setv(T_FQDNP, 0, 0, 'cp.dnsmadeeasy.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'script' => setv(T_STRING, 0, '/servlet/updateip', undef), + 'server' => setv(T_FQDNP, 0, 'cp.dnsmadeeasy.com', undef), }, ), 'dondominio' => ddclient::LegacyProtocol->new( 'update' => \&nic_dondominio_update, 'examples' => \&nic_dondominio_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'dondns.dondominio.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'dondns.dondominio.com', undef), }, ), 'dslreports1' => ddclient::LegacyProtocol->new( 'update' => \&nic_dslreports1_update, 'examples' => \&nic_dslreports1_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'www.dslreports.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'www.dslreports.com', undef), }, ), 'domeneshop' => ddclient::Protocol->new( 'update' => \&nic_domeneshop_update, 'examples' => \&nic_domeneshop_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'api.domeneshop.no', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'api.domeneshop.no', undef), }, ), 'duckdns' => ddclient::Protocol->new( 'update' => \&nic_duckdns_update, 'examples' => \&nic_duckdns_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'www.duckdns.org', undef), + 'server' => setv(T_FQDNP, 0, 'www.duckdns.org', undef), }, ), 'dyndns1' => ddclient::LegacyProtocol->new( 'update' => \&nic_dyndns1_update, 'examples' => \&nic_dyndns1_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - %{$variables{'dyndns-common-defaults'}}, - 'static' => setv(T_BOOL, 0, 1, 0, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + %{$cfgvars{'dyndns-common-defaults'}}, + 'static' => setv(T_BOOL, 0, 0, undef), + }, + 'recapvars' => { + %{$recapvars{'common'}}, + %{$recapvars{'dyndns-common'}}, + 'static' => T_BOOL, }, 'force_update_if_changed' => [qw(static wildcard mx backupmx)], ), 'dyndns2' => ddclient::Protocol->new( 'update' => \&nic_dyndns2_update, 'examples' => \&nic_dyndns2_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - %{$variables{'dyndns-common-defaults'}}, - 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + %{$cfgvars{'dyndns-common-defaults'}}, + 'script' => setv(T_STRING, 0, '/nic/update', undef), + }, + 'recapvars' => { + %{$recapvars{'common'}}, + %{$recapvars{'dyndns-common'}}, }, 'force_update_if_changed' => [qw(wildcard mx backupmx)], ), 'easydns' => ddclient::Protocol->new( 'update' => \&nic_easydns_update, 'examples' => \&nic_easydns_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'backupmx' => setv(T_BOOL, 0, 1, 0, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'backupmx' => setv(T_BOOL, 0, 0, undef), # From : "You need to wait at least 10 # minutes between updates." - 'min-interval' => setv(T_DELAY, 0, 0, interval('10m'), 0), - 'mx' => setv(T_FQDN, 0, 1, undef, undef), - 'server' => setv(T_FQDNP, 0, 0, 'api.cp.easydns.com', undef), - 'script' => setv(T_STRING, 0, 0, '/dyn/generic.php', undef), - 'wildcard' => setv(T_BOOL, 0, 1, 0, undef), + 'min-interval' => setv(T_DELAY, 0, interval('10m'), 0), + 'mx' => setv(T_FQDN, 0, undef, undef), + 'server' => setv(T_FQDNP, 0, 'api.cp.easydns.com', undef), + 'script' => setv(T_STRING, 0, '/dyn/generic.php', undef), + 'wildcard' => setv(T_BOOL, 0, 0, undef), + }, + 'recapvars' => { + %{$recapvars{'common'}}, + %{$recapvars{'dyndns-common'}}, }, 'force_update_if_changed' => [qw(wildcard mx backupmx)], ), 'freedns' => ddclient::Protocol->new( 'update' => \&nic_freedns_update, 'examples' => \&nic_freedns_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), - 'server' => setv(T_FQDNP, 0, 0, 'freedns.afraid.org', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), + 'server' => setv(T_FQDNP, 0, 'freedns.afraid.org', undef), }, ), 'freemyip' => ddclient::LegacyProtocol->new( 'update' => \&nic_freemyip_update, 'examples' => \&nic_freemyip_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'freemyip.com', undef), + 'server' => setv(T_FQDNP, 0, 'freemyip.com', undef), }, ), 'gandi' => ddclient::Protocol->new( 'update' => \&nic_gandi_update, 'examples' => \&nic_gandi_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), - 'server' => setv(T_FQDNP, 0, 0, 'api.gandi.net', undef), - 'script' => setv(T_STRING, 0, 0, '/v5', undef), - 'use-personal-access-token' => setv(T_BOOL, 0, 0, 0, undef), - 'ttl' => setv(T_DELAY, 0, 0, undef, interval('5m')), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), + 'server' => setv(T_FQDNP, 0, 'api.gandi.net', undef), + 'script' => setv(T_STRING, 0, '/v5', undef), + 'use-personal-access-token' => setv(T_BOOL, 0, 0, undef), + 'ttl' => setv(T_DELAY, 0, undef, interval('5m')), + 'zone' => setv(T_FQDN, 1, undef, undef), } ), 'godaddy' => ddclient::Protocol->new( 'update' => \&nic_godaddy_update, 'examples' => \&nic_godaddy_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'api.godaddy.com/v1/domains', undef), - 'ttl' => setv(T_NUMBER, 0, 0, 600, undef), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'server' => setv(T_FQDNP, 0, 'api.godaddy.com/v1/domains', undef), + 'ttl' => setv(T_NUMBER, 0, 600, undef), + 'zone' => setv(T_FQDN, 1, undef, undef), }, ), 'he.net' => ddclient::Protocol->new( 'update' => \&nic_henet_update, 'examples' => \&nic_henet_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'dyn.dns.he.net', undef), + 'min-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'server' => setv(T_FQDNP, 0, 'dyn.dns.he.net', undef), }, ), 'hetzner' => ddclient::Protocol->new( 'update' => \&nic_hetzner_update, 'examples' => \&nic_hetzner_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'min-interval' => setv(T_DELAY, 0, 0, interval('1m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'dns.hetzner.com/api/v1', undef), - 'ttl' => setv(T_NUMBER, 0, 0, 60, 60), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'min-interval' => setv(T_DELAY, 0, interval('1m'), 0), + 'server' => setv(T_FQDNP, 0, 'dns.hetzner.com/api/v1', undef), + 'ttl' => setv(T_NUMBER, 0, 60, 60), + 'zone' => setv(T_FQDN, 1, undef, undef), }, ), 'inwx' => ddclient::Protocol->new( 'update' => \&nic_inwx_update, 'examples' => \&nic_inwx_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'dyndns.inwx.com', undef), - 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'dyndns.inwx.com', undef), + 'script' => setv(T_STRING, 0, '/nic/update', undef), }, ), 'mythicdyn' => ddclient::Protocol->new( 'update' => \&nic_mythicdyn_update, 'examples' => \&nic_mythicdyn_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'api.mythic-beasts.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'server' => setv(T_FQDNP, 0, 'api.mythic-beasts.com', undef), }, ), 'namecheap' => ddclient::LegacyProtocol->new( 'update' => \&nic_namecheap_update, 'examples' => \&nic_namecheap_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), - 'server' => setv(T_FQDNP, 0, 0, 'dynamicdns.park-your-domain.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), + 'server' => setv(T_FQDNP, 0, 'dynamicdns.park-your-domain.com', undef), }, ), 'nfsn' => ddclient::LegacyProtocol->new( 'update' => \&nic_nfsn_update, 'examples' => \&nic_nfsn_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), - 'server' => setv(T_FQDNP, 0, 0, 'api.nearlyfreespeech.net', undef), - 'ttl' => setv(T_NUMBER, 0, 0, 300, undef), - 'zone' => setv(T_FQDN, 1, 0, undef, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), + 'server' => setv(T_FQDNP, 0, 'api.nearlyfreespeech.net', undef), + 'ttl' => setv(T_NUMBER, 0, 300, undef), + 'zone' => setv(T_FQDN, 1, undef, undef), }, ), 'njalla' => ddclient::Protocol->new( 'update' => \&nic_njalla_update, 'examples' => \&nic_njalla_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'njal.la', undef), - 'quietreply' => setv(T_BOOL, 0, 0, 0, undef), + 'server' => setv(T_FQDNP, 0, 'njal.la', undef), + 'quietreply' => setv(T_BOOL, 0, 0, undef), }, ), 'noip' => ddclient::Protocol->new( 'update' => \&nic_noip_update, 'examples' => \&nic_noip_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'dynupdate.no-ip.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'dynupdate.no-ip.com', undef), }, ), 'nsupdate' => ddclient::Protocol->new( 'update' => \&nic_nsupdate_update, 'examples' => \&nic_nsupdate_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'login' => setv(T_LOGIN, 0, 0, '/usr/bin/nsupdate', undef), - 'tcp' => setv(T_BOOL, 0, 0, 0, undef), - 'ttl' => setv(T_NUMBER, 0, 0, 600, undef), - 'zone' => setv(T_STRING, 1, 0, undef, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'login' => setv(T_LOGIN, 0, '/usr/bin/nsupdate', undef), + 'tcp' => setv(T_BOOL, 0, 0, undef), + 'ttl' => setv(T_NUMBER, 0, 600, undef), + 'zone' => setv(T_STRING, 1, undef, undef), }, ), 'ovh' => ddclient::LegacyProtocol->new( 'update' => \&nic_ovh_update, 'examples' => \&nic_ovh_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'script' => setv(T_STRING, 0, 0, '/nic/update', undef), - 'server' => setv(T_FQDNP, 0, 0, 'www.ovh.com', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'script' => setv(T_STRING, 0, '/nic/update', undef), + 'server' => setv(T_FQDNP, 0, 'www.ovh.com', undef), }, ), 'porkbun' => ddclient::Protocol->new( 'update' => \&nic_porkbun_update, 'examples' => \&nic_porkbun_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, 'password' => undef, - 'apikey' => setv(T_PASSWD, 1, 0, undef, undef), - 'secretapikey' => setv(T_PASSWD, 1, 0, undef, undef), - 'root-domain' => setv(T_FQDN, 0, 0, undef, undef), - 'on-root-domain' => setv(T_BOOL, 0, 0, 0, undef), + 'apikey' => setv(T_PASSWD, 1, undef, undef), + 'secretapikey' => setv(T_PASSWD, 1, undef, undef), + 'root-domain' => setv(T_FQDN, 0, undef, undef), + 'on-root-domain' => setv(T_BOOL, 0, 0, undef), }, ), 'sitelutions' => ddclient::LegacyProtocol->new( 'update' => \&nic_sitelutions_update, 'examples' => \&nic_sitelutions_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'www.sitelutions.com', undef), - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'www.sitelutions.com', undef), + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), }, ), 'yandex' => ddclient::LegacyProtocol->new( 'update' => \&nic_yandex_update, 'examples' => \&nic_yandex_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'pddimp.yandex.ru', undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('5m'), 0), + 'server' => setv(T_FQDNP, 0, 'pddimp.yandex.ru', undef), }, ), 'zoneedit1' => ddclient::LegacyProtocol->new( 'update' => \&nic_zoneedit1_update, 'examples' => \&nic_zoneedit1_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'min-interval' => setv(T_DELAY, 0, 0, interval('10m'), 0), - 'server' => setv(T_FQDNP, 0, 0, 'dynamic.zoneedit.com', undef), - 'zone' => setv(T_FQDN, 0, 0, undef, undef), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'min-interval' => setv(T_DELAY, 0, interval('10m'), 0), + 'server' => setv(T_FQDNP, 0, 'dynamic.zoneedit.com', undef), + 'zone' => setv(T_FQDN, 0, undef, undef), }, ), 'keysystems' => ddclient::LegacyProtocol->new( 'update' => \&nic_keysystems_update, 'examples' => \&nic_keysystems_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'dynamicdns.key-systems.net', undef), + 'server' => setv(T_FQDNP, 0, 'dynamicdns.key-systems.net', undef), }, ), 'dnsexit2' => ddclient::Protocol->new( 'update' => \&nic_dnsexit2_update, 'examples' => \&nic_dnsexit2_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'ssl' => setv(T_BOOL, 0, 0, 1, undef), - 'server' => setv(T_FQDNP, 0, 0, 'api.dnsexit.com', undef), - 'path' => setv(T_STRING, 0, 0, '/dns/', undef), - 'ttl' => setv(T_NUMBER, 0, 0, 5, 0), - 'zone' => setv(T_STRING, 0, 0, undef, undef), + 'ssl' => setv(T_BOOL, 0, 1, undef), + 'server' => setv(T_FQDNP, 0, 'api.dnsexit.com', undef), + 'path' => setv(T_STRING, 0, '/dns/', undef), + 'ttl' => setv(T_NUMBER, 0, 5, 0), + 'zone' => setv(T_STRING, 0, undef, undef), }, ), 'regfishde' => ddclient::Protocol->new( 'update' => \&nic_regfishde_update, 'examples' => \&nic_regfishde_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, - 'server' => setv(T_FQDNP, 0, 0, 'dyndns.regfish.de', undef), + 'server' => setv(T_FQDNP, 0, 'dyndns.regfish.de', undef), }, ), 'enom' => ddclient::LegacyProtocol->new( 'update' => \&nic_enom_update, 'examples' => \&nic_enom_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, - 'server' => setv(T_FQDNP, 0, 0, 'dynamic.name-services.com', undef), - 'min-interval' => setv(T_DELAY, 0, 0, interval('5m'), interval('5m')), + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, + 'server' => setv(T_FQDNP, 0, 'dynamic.name-services.com', undef), + 'min-interval' => setv(T_DELAY, 0, interval('5m'), interval('5m')), }, ), 'infomaniak' => ddclient::Protocol->new( 'update' => \&nic_infomaniak_update, 'examples' => \&nic_infomaniak_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'server' => undef, }, ), 'emailonly' => ddclient::Protocol->new( 'update' => \&nic_emailonly_update, 'examples' => \&nic_emailonly_examples, - 'variables' => { - %{$variables{'protocol-common-defaults'}}, + 'cfgvars' => { + %{$cfgvars{'protocol-common-defaults'}}, 'login' => undef, 'password' => undef, # Change default to never re-notify if IP address has not changed. - 'max-interval' => setv(T_DELAY, 0, 0, 'inf', 0), + 'max-interval' => setv(T_DELAY, 0, 'inf', 0), }, ), ); -$variables{'merged'} = { - map({ %{$protocols{$_}{'variables'}} } keys(%protocols)), - %{$variables{'dyndns-common-defaults'}}, - %{$variables{'protocol-common-defaults'}}, - %{$variables{'global-defaults'}}, +$cfgvars{'merged'} = { + map({ %{$protocols{$_}{'cfgvars'}} } keys(%protocols)), + %{$cfgvars{'dyndns-common-defaults'}}, + %{$cfgvars{'protocol-common-defaults'}}, + %{$cfgvars{'global-defaults'}}, }; # This will hold the processed args. @@ -1655,16 +1683,40 @@ sub read_recap { return if !(-e $file); my %saved = %opt; %opt = (); - $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file); + $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file, sub { + my ($h, $k, $v, $normout) = @_; + if (!defined($h) && $k eq 'host') { + return 0 if !defined($v); + $$normout = $v; + return 1; + } + if (!defined($h) || !$config{$h}) { + warning("ignoring '$k=$v' for unknown host: " . ($h // '')); + return 0; + } + my $p = opt('protocol', $h); + my $type = $protocols{$p}{recapvars}{$k}; + if (!$type) { + warning("ignoring unrecognized recap variable for host '$h' with protocol '$p': $k"); + return 0; + } + my $norm; + if (!eval { $norm = check_value($v, {type => $type}); 1; }) { + warning("invalid value '$k=$v' for host '$h' with protocol '$p': $@"); + return 0; + } + $$normout = $norm if defined($normout); + return 1; + }); %opt = %saved; for my $h (keys(%recap)) { if (!exists($config{$h})) { delete($recap{$h}); next; } - my $vars = $protocols{opt('protocol', $h)}{variables}; + my $vars = $protocols{opt('protocol', $h)}{recapvars}; for my $v (keys(%{$recap{$h}})) { - delete($recap{$h}{$v}) if !$vars->{$v} || !$vars->{$v}{recap}; + delete($recap{$h}{$v}) if !$vars->{$v}; } } } @@ -1737,7 +1789,39 @@ sub parse_assignment { ###################################################################### sub read_config { my ($file, $config, $globals) = @_; - _read_config($config, $globals, '', $file); + _read_config($config, $globals, '', $file, sub { + # TODO: The checks below are incorrect for a few reasons: + # + # * It is not protocol-aware. Different protocols can have different sets of variables, + # with different normalization and validation behaviors. + # * It does not check for missing required values. Note that a later line or a + # command-line argument might define a missing required value. + # * A later line or command-line argument might override an invalid value, changing it to + # valid. + # + # Fixing this is not simple. Values should be checked and normalized after processing the + # entire file and command-line arguments, but then we lose line number context. The line + # number could be recorded along with each variable's value to provide context in case + # validation fails, but that adds considerable complexity. Fortunately, a variable's type + # is unlikely to change even if the protocol changes (`$cfgvars{merged}{$var}{type}` will + # likely equal `$protocols{$proto}{cfgvars}{$var}{type}` for each variable `$var` for each + # protocol `$proto`), so normalizing and validating values on a line-by-line basis is + # likely to be safe. + my ($h, $k, $v, $normout) = @_; + if (!exists($cfgvars{'merged'}{$k})) { + warning("unrecognized keyword"); + return 0; + } + my $def = $cfgvars{'merged'}{$k}; + my $norm; + if (!eval { $norm = check_value($v, $def); 1; }) { + my $vf = defined($v) ? "'$v'" : ''; + warning("invalid value $vf: $@"); + return 0; + } + $$normout = $norm if defined($normout); + return 1; + }); } sub _read_config { # Configuration line format after comment and continuation @@ -1771,46 +1855,11 @@ sub _read_config { # accumulated thus far and stored in $1->{$host} for each # referenced host. - my ($config, $globals, $stamp, $file) = @_; + my ($config, $globals, $stamp, $file, $check) = @_; local $_l = pushlogctx("file $file"); my %globals = (); my %config = (); my $content = ''; - # Checks a single key-value pair for a host and writes the normalized value output to an - # optional output ref. - my $check = sub { - # TODO: The checks below are incorrect for a few reasons: - # - # * It is not protocol-aware. Different protocols can have different sets of variables, - # with different normalization and validation behaviors. - # * It does not check for missing required values. Note that a later line or a - # command-line argument might define a missing required value. - # * A later line or command-line argument might override an invalid value, changing it to - # valid. - # - # Fixing this is not simple. Values should be checked and normalized after processing the - # entire file and command-line arguments, but then we lose line number context. The line - # number could be recorded along with each variable's value to provide context in case - # validation fails, but that adds considerable complexity. Fortunately, a variable's type - # is unlikely to change even if the protocol changes (`$variables{merged}{$var}{type}` will - # likely equal `$protocols{$proto}{variables}{$var}{type}` for each variable `$var` for - # each protocol `$proto`), so normalizing and validating values on a line-by-line basis is - # likely to be safe. - my ($h, $k, $v, $normout) = @_; - if (!exists $variables{'merged'}{$k}) { - warning("unrecognized keyword"); - return 0; - } - my $def = $variables{'merged'}{$k}; - my $norm; - if (!eval { $norm = check_value($v, $def); 1; }) { - my $vf = defined($v) ? "'$v'" : ''; - warning("invalid value $vf: $@"); - return 0; - } - $$normout = $norm if defined($normout); - return 1; - }; # Calls $check on each entry in the given hashref, deleting any entries that don't pass. my $checkall = sub { my ($h, $l) = @_; @@ -1949,10 +1998,10 @@ sub init_config { %opt = %saved_opt; # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause # surprising behavior. - for my $var (keys(%{$variables{'merged'}})) { + for my $var (keys(%{$cfgvars{'merged'}})) { # TODO: Also validate $opt{'options'}. next if !defined($opt{$var}) || ref($opt{$var}); - if (!eval { $opt{$var} = check_value($opt{$var}, $variables{'merged'}{$var}); 1; }) { + if (!eval { $opt{$var} = check_value($opt{$var}, $cfgvars{'merged'}{$var}); 1; }) { fatal("invalid argument '--$var=$opt{$var}': $@"); } } @@ -1989,7 +2038,7 @@ sub init_config { my $proto = $options{'protocol'} // opt('protocol', $h); my $protodef = $protocols{$proto} or fatal("host $h: invalid protocol: $proto"); for my $var (keys(%options)) { - my $def = $protodef->{variables}{$var} + my $def = $protodef->{cfgvars}{$var} or fatal("host $h: unknown option '--options=$var=$options{$var}'"); eval { $config{$h}{$var} = check_value($options{$var}, $def); 1; } or fatal("host $h: invalid option value '--options=$var=$options{$var}': $@"); @@ -2000,7 +2049,7 @@ sub init_config { for my $var (keys(%options)) { # TODO: This might grab an arbitrary protocol-specific variable definition, which # could cause surprising behavior. - my $def = $variables{'merged'}{$var} + my $def = $cfgvars{'merged'}{$var} or fatal("unknown option '--options=$var=$options{$var}'"); # TODO: Why not merge the values into %opt? eval { $globals{$var} = check_value($options{$var}, $def); 1; } @@ -2011,7 +2060,7 @@ sub init_config { ## override global options with those on the command-line. for my $o (keys %opt) { - if (defined $opt{$o} && exists $variables{'merged'}{$o}) { + if (defined $opt{$o} && exists $cfgvars{'merged'}{$o}) { # TODO: What's the point of this? The opt() function will fall back to %globals if # %opt doesn't have a value, so this shouldn't be necessary. $globals{$o} = $opt{$o}; @@ -2335,11 +2384,11 @@ sub default { my $var; if (defined($h) && $config{$h}) { my $proto = $protocols{opt('protocol', $v eq 'protocol' ? undef : $h)}; - $var = $proto->{variables}{$v} if $proto; + $var = $proto->{cfgvars}{$v} if $proto; } # TODO: This might grab an arbitrary protocol-specific variable definition, which could cause # surprising behavior. - $var //= $variables{'merged'}{$v}; + $var //= $cfgvars{'merged'}{$v}; return undef if !defined($var); return $var->{'default'}($h) if ref($var->{default}) eq 'CODE'; return $var->{'default'}; diff --git a/t/read_recap.pl b/t/read_recap.pl index db44bb5..b2a62c5 100644 --- a/t/read_recap.pl +++ b/t/read_recap.pl @@ -7,21 +7,23 @@ local $ddclient::globals{debug} = 1; local $ddclient::globals{verbose} = 1; local %ddclient::protocols = ( protocol_a => ddclient::Protocol->new( - variables => { - host => {type => ddclient::T_STRING(), recap => 1}, - var_a => {type => ddclient::T_BOOL(), recap => 1}, + recapvars => { + host => ddclient::T_STRING(), + var_a => ddclient::T_BOOL(), }, ), protocol_b => ddclient::Protocol->new( - variables => { - host => {type => ddclient::T_STRING(), recap => 1}, - var_b => {type => ddclient::T_NUMBER(), recap => 1}, + recapvars => { + host => ddclient::T_STRING(), + var_b => ddclient::T_NUMBER(), + }, + cfgvars => { var_b_non_recap => {type => ddclient::T_ANY()}, }, ), ); -local %ddclient::variables = - (merged => {map({ %{$ddclient::protocols{$_}{variables}}; } sort(keys(%ddclient::protocols)))}); +local %ddclient::cfgvars = (merged => {map({ %{$ddclient::protocols{$_}{cfgvars} // {}}; } + sort(keys(%ddclient::protocols)))}); my @test_cases = ( { diff --git a/t/variable_defaults.pl b/t/variable_defaults.pl index 537b642..c0e8320 100644 --- a/t/variable_defaults.pl +++ b/t/variable_defaults.pl @@ -4,8 +4,8 @@ SKIP: { eval { require Test::Warnings; } or skip($@, 1); } eval { require 'ddclient'; } or BAIL_OUT($@); my %variable_collections = ( - map({ ($_ => $ddclient::variables{$_}) } grep($_ ne 'merged', keys(%ddclient::variables))), - map({ ("protocol=$_" => $ddclient::protocols{$_}{variables}); } keys(%ddclient::protocols)), + map({ ($_ => $ddclient::cfgvars{$_}) } grep($_ ne 'merged', keys(%ddclient::cfgvars))), + map({ ("protocol=$_" => $ddclient::protocols{$_}{cfgvars}); } keys(%ddclient::protocols)), ); my %seen; my @test_cases = ( @@ -24,10 +24,10 @@ for my $tc (@test_cases) { if ($tc->{def}{required}) { is($tc->{def}{default}, undef, "'$tc->{desc}' (required) has no default"); } else { - # Preserve all existing variables in $variables{merged} so that variables with dynamic + # Preserve all existing variables in $cfgvars{merged} so that variables with dynamic # defaults can reference them. - local %ddclient::variables = (merged => { - %{$ddclient::variables{merged}}, + local %ddclient::cfgvars = (merged => { + %{$ddclient::cfgvars{merged}}, 'var for test' => $tc->{def}, }); # Variables with dynamic defaults will need their own unit tests, but we can still check the @@ -78,11 +78,11 @@ my @use_test_cases = ( for my $tc (@use_test_cases) { my $desc = "'use' dynamic default: $tc->{desc}"; local %ddclient::protocols = (protocol => ddclient::Protocol->new()); - local %ddclient::variables = (merged => { - 'protocol' => $ddclient::variables{'merged'}{'protocol'}, - 'use' => $ddclient::variables{'protocol-common-defaults'}{'use'}, - 'usev4' => $ddclient::variables{'merged'}{'usev4'}, - 'usev6' => $ddclient::variables{'merged'}{'usev6'}, + local %ddclient::cfgvars = (merged => { + 'protocol' => $ddclient::cfgvars{'merged'}{'protocol'}, + 'use' => $ddclient::cfgvars{'protocol-common-defaults'}{'use'}, + 'usev4' => $ddclient::cfgvars{'merged'}{'usev4'}, + 'usev6' => $ddclient::cfgvars{'merged'}{'usev6'}, }); local %ddclient::config = (host => {protocol => 'protocol', %{$tc->{cfg} // {}}}); local %ddclient::opt;