Merge pull request #684 from rhansen/dnsexit2

dnsexit2: Update multiple hosts at a time when possible

ChangeLog.md

@@ -16,6 +16,11 @@ repository history](https://github.com/ddclient/ddclient/commits/main).
     special characters are preserved literally.
     [#766](https://github.com/ddclient/ddclient/pull/766)
 
+### New features
+
+  * `dnsexit2`: Multiple hosts are updated in a single API call when possible.
+    [#684](https://github.com/ddclient/ddclient/pull/684)
+
 ## 2025-01-07 v4.0.0-rc.2
 
 ### Breaking changes

Makefile.am

@@ -160,5 +160,6 @@ EXTRA_DIST += $(handwritten_tests) \
 	t/lib/ddclient/Test/Fake/HTTPD/other-ca-cert.pem \
 	t/lib/ddclient/t.pm \
 	t/lib/ddclient/t/HTTPD.pm \
+	t/lib/ddclient/t/Logger.pm \
 	t/lib/ddclient/t/ip.pm \
 	t/lib/ok.pm

ddclient.in

@@ -2510,9 +2510,11 @@ sub ynu {
     # provided (it is ignored if the `msg` keyword is present).
     sub log {
         my $self = shift;
-        my %args = (@_ % 2 ? (msg => pop) : (), @_);
+        my %args = (label => '', @_ % 2 ? (msg => pop) : (), @_);
         $args{ctx} = [$args{ctx} // ()] if ref($args{ctx}) eq '';
-        return $self->_log(\%args);
+        $self->_log(\%args);
+        $self->_failed() if $args{label} eq 'FAILED';
+        $self->_abort() if $args{label} eq 'FATAL';
     }
 
     sub _log {
@@ -2521,10 +2523,11 @@ sub ynu {
         # the caller's arrayref (in case it is reused in a future call).
         $args->{ctx} = [@{$self->{ctx}}, @{$args->{ctx}}];
         return $self->{parent}->_log($args) if defined($self->{parent});
+        return if $args->{label} eq 'DEBUG' && !ddclient::opt('debug');
+        return if $args->{label} eq 'INFO' && !ddclient::opt('verbose');
         my $buffer = $args->{msg} // '';
         chomp($buffer);
         if (!$args->{raw}) {
-            $args->{label} //= '';
             my $prefix = $args->{label} ne '' ? sprintf("%-8s ", $args->{label} . ':') : '';
             $prefix .= "[$_]" for @{$args->{ctx}};
             $prefix .= '> ' if $prefix;
@@ -2544,6 +2547,20 @@ sub ynu {
             }
         }
     }
+
+    sub _failed {
+        my ($self) = @_;
+        return $self->{parent}->_failed() if defined($self->{parent});
+        $ddclient::result = 'FAILED';
+        $ddclient::result if 0;  # Suppress spurious "used only once: possible typo" warning.
+    }
+
+    sub _abort {
+        my ($self) = @_;
+        return $self->{parent}->_abort() if defined($self->{parent});
+        ddclient::sendmail();
+        exit(1);
+    }
 }
 
 # Intended use:
@@ -2552,12 +2569,12 @@ sub pushlogctx { my ($ctx) = @_; return ddclient::Logger->new($ctx, $_l); }
 
 sub logmsg { $_l->log(@_); }
 sub _logmsg_fmt { $_[0] eq 'ctx' ? (shift, shift) : (), (@_ > 1) ? sprintf(shift, @_) : shift; }
-sub info    { logmsg(email => 1, label => 'INFO',    _logmsg_fmt(@_)) if opt('verbose'); }
+sub info    { logmsg(email => 1, label => 'INFO',    _logmsg_fmt(@_)); }
-sub debug   { logmsg(            label => 'DEBUG',   _logmsg_fmt(@_)) if opt('debug'); }
+sub debug   { logmsg(            label => 'DEBUG',   _logmsg_fmt(@_)); }
 sub warning { logmsg(email => 1, label => 'WARNING', _logmsg_fmt(@_)); }
-sub fatal   { logmsg(email => 1, label => 'FATAL',   _logmsg_fmt(@_)); sendmail(); exit(1); }
+sub fatal   { logmsg(email => 1, label => 'FATAL',   _logmsg_fmt(@_)); }
 sub success { logmsg(email => 1, label => 'SUCCESS', _logmsg_fmt(@_)); }
-sub failed  { logmsg(email => 1, label => 'FAILED',  _logmsg_fmt(@_)); $result = 'FAILED'; }
+sub failed  { logmsg(email => 1, label => 'FAILED',  _logmsg_fmt(@_)); }
 
 sub prettytime { return scalar(localtime(shift)); }
 
@@ -4080,33 +4097,33 @@ 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 (@_) {
         $config{$h}{'zone'} = $h if !defined(opt('zone', $h));
-        dnsexit2_update_host($h);
     }
+    dnsexit2_update_hostgroup($_) for group_hosts_by(\@_, qw(password path server ssl zone));
 }
 
-sub dnsexit2_update_host {
+sub dnsexit2_update_hostgroup {
-    my ($h) = @_;
+    my ($group) = @_;
-    local $_l = pushlogctx($h);
+    return unless @{$group->{hosts}} > 0;
+    local $_l = pushlogctx(join(', ', @{$group->{hosts}}));
+    my %hostips;
+    my @updates;
+    for my $h (@{$group->{hosts}}) {
+        local $_l = pushlogctx($h) if @{$group->{hosts}} > 1;
         my $name = $h;
         # Remove the zone suffix from $name.  If the zone eq $name, $name can be left alone or
         # set to the empty string; both have identical semantics.  For consistency, always
         # remove the zone even if it means $name becomes the empty string.
-    my $zone = opt('zone', $h);
+        if ($name =~ s/(?:^|\.)\Q$group->{cfg}{'zone'}\E$//) {
-    if ($name =~ s/(?:^|\.)\Q$zone\E$//) {
             # The zone was successfully trimmed from $name.
         } else {
-        fatal("hostname does not end with the zone: " . opt('zone', $h));
+            fatal("hostname does not end with the zone: $group->{cfg}{'zone'}");
         }
         # The IPv4 and IPv6 addresses must be updated together in a single API call.
-    my %ips;
-    my @updates;
         for my $ipv ('4', '6') {
             my $ip = delete($config{$h}{"wantipv$ipv"}) or next;
-        $ips{$ipv} = $ip;
+            $hostips{$h}{$ipv} = $ip;
             info("updating IPv$ipv address to $ip");
             $recap{$h}{"status-ipv$ipv"} = 'failed';
             push(@updates, {
@@ -4115,19 +4132,20 @@ sub dnsexit2_update_host {
                 content => $ip,
                 ttl => opt('ttl', $h),
             });
-    };
+        }
-    my $url = opt('server', $h) . opt('path', $h);
+    }
+    return unless @updates > 0;
     my $reply = geturl(
         proxy => opt('proxy'),
-        url => $url,
+        url => $group->{cfg}{'server'} . $group->{cfg}{'path'},
         headers => [
             'Content-Type: application/json',
             'Accept: application/json',
         ],
         method => 'POST',
         data => encode_json({
-            apikey => opt('password', $h),
+            apikey => $group->{cfg}{'password'},
-            domain => $zone,
+            domain => $group->{cfg}{'zone'},
             update => \@updates,
         }),
     );
@@ -4174,14 +4192,17 @@ sub dnsexit2_update_host {
         return;
     }
     success($message);
+    keys(%hostips);  # Reset internal iterator.
+    while (my ($h, $ips) = each(%hostips)) {
         $recap{$h}{'mtime'} = $now;
-    keys(%ips);  # Reset internal iterator.
+        keys(%$ips);  # Reset internal iterator.
-    while (my ($ipv, $ip) = each(%ips)) {
+        while (my ($ipv, $ip) = each(%$ips)) {
             $recap{$h}{"ipv$ipv"} = $ip;
             $recap{$h}{"status-ipv$ipv"} = 'good';
             success("updated IPv$ipv address to $ip");
         }
     }
+}
 
 ######################################################################
 ## nic_noip_update

t/lib/ddclient/t/Logger.pm

@@ -0,0 +1,39 @@
+package ddclient::t::Logger;
+BEGIN { eval { require 'ddclient'; } or BAIL_OUT($@); }
+use parent qw(-norequire ddclient::Logger);
+
+{
+    package ddclient::t::LoggerAbort;
+    use overload '""' => qw(stringify);
+    sub new {
+        my ($class, %args) = @_;
+        return bless(\%args, $class);
+    }
+    sub stringify {
+        return 'logged a FATAL message';
+    }
+}
+
+sub new {
+    my ($class, $parent, $labelre) = @_;
+    my $self = $class->SUPER::new(undef, $parent);
+    $self->{logs} = [];
+    $self->{_labelre} = $labelre;
+    return $self;
+}
+
+sub _log {
+    my ($self, $args) = @_;
+    my $lre = $self->{_labelre};
+    my $lbl = $args->{label};
+    push(@{$self->{logs}}, $args) if !defined($lre) || (defined($lbl) && $lbl =~ $lre);
+    return $self->SUPER::_log($args);
+}
+
+sub _abort {
+    my ($self) = @_;
+    push(@{$self->{logs}}, 'aborted');
+    die(ddclient::t::LoggerAbort->new());
+}
+
+1;

t/protocol_directnic.pl

@@ -3,6 +3,7 @@ BEGIN { SKIP: { eval { require Test::Warnings; 1; } or skip($@, 1); } }
 BEGIN { eval { require JSON::PP; 1; } or plan(skip_all => $@); JSON::PP->import(); }
 BEGIN { eval { require 'ddclient'; } or BAIL_OUT($@); }
 use ddclient::t::HTTPD;
+use ddclient::t::Logger;
 
 httpd_required();
 
@@ -29,23 +30,6 @@ httpd()->run(sub {
     return [400, $headers, ['unexpected request: ' . $req->uri()]]
 });
 
-{
-    package Logger;
-    use parent qw(-norequire ddclient::Logger);
-    sub new {
-        my ($class, $parent) = @_;
-        my $self = $class->SUPER::new(undef, $parent);
-        $self->{logs} = [];
-        return $self;
-    }
-    sub _log {
-        my ($self, $args) = @_;
-        push(@{$self->{logs}}, $args)
-            if ($args->{label} // '') =~ qr/^(?:WARNING|FATAL|SUCCESS|FAILED)$/;
-        return $self->SUPER::_log($args);
-    }
-}
-
 my $hostname = httpd()->endpoint();
 my @test_cases = (
     {
@@ -149,7 +133,7 @@ for my $tc (@test_cases) {
     diag('==============================================================================');
     local $ddclient::globals{debug} = 1;
     local $ddclient::globals{verbose} = 1;
-    my $l = Logger->new($ddclient::_l);
+    my $l = ddclient::t::Logger->new($ddclient::_l, qr/^(?:WARNING|FATAL|SUCCESS|FAILED)$/);
     local %ddclient::config = %{$tc->{cfg}};
     local %ddclient::recap;
     {

t/protocol_dnsexit2.pl

@@ -3,9 +3,13 @@ BEGIN { SKIP: { eval { require Test::Warnings; 1; } or skip($@, 1); } }
 BEGIN { eval { require JSON::PP; 1; } or plan(skip_all => $@); JSON::PP->import(); }
 BEGIN { eval { require 'ddclient'; } or BAIL_OUT($@); }
 use ddclient::t::HTTPD;
+use ddclient::t::Logger;
 
 httpd_required();
 
+local $ddclient::globals{debug} = 1;
+local $ddclient::globals{verbose} = 1;
+
 ddclient::load_json_support('dnsexit2');
 
 httpd()->run(sub {
@@ -17,143 +21,222 @@ httpd()->run(sub {
     })]];
 });
 
-local $ddclient::globals{verbose} = 1;
+sub cmp_update {
-
+    my ($a, $b) = @_;
-sub decode_and_sort_array {
+    return $a->{name} cmp $b->{name} || $a->{type} cmp $b->{type};
-    my ($data) = @_;
-    if (!ref $data) {
-        $data = decode_json($data);
-    }
-    @{$data->{update}} = sort { $a->{type} cmp $b->{type} } @{$data->{update}};
-    return $data;
 }
 
-subtest 'Testing nic_dnsexit2_update' => sub {
+sub sort_updates {
-    httpd()->reset();
+    my ($req) = @_;
-    local %ddclient::config = (
+    return {
-        'host.my.example.com' => {
+        %$req,
-            'usev4'    => 'ipv4',
+        update => [sort({ cmp_update($a, $b); } @{$req->{update}})],
-            'wantipv4' => '192.0.2.1',
+    };
-            'usev6'    => 'ipv6',
+}
-            'wantipv6' => '2001:db8::1',
+
-            'protocol' => 'dnsexit2',
+sub sort_reqs {
-            'password' => 'mytestingpassword',
+    my @reqs = map(sort_updates($_), @_);
-            'zone'     => 'my.example.com',
+    my @sorted = sort({
-            'server'   => httpd()->endpoint(),
+        my $ret = $a->{domain} cmp $b->{domain};
-            'path'     => '/update',
+        $ret = @{$a->{update}} <=> @{$b->{update}} if !$ret;
-            'ttl'      => 5
+        my $i = 0;
-    });
+        while (!$ret && $i < @{$a->{update}} && $i < @{$b->{update}}) {
-    ddclient::nic_dnsexit2_update(undef, 'host.my.example.com');
+            $ret = cmp_update($a->{update}[$i], $b->{update}[$i]);
-    my @requests = httpd()->reset();
+        }
-    is(scalar(@requests), 1, 'expected number of update requests');
+        return $ret;
-    my $req = shift(@requests);
+    } @reqs);
-    is($req->method(), 'POST', 'Method is correct');
+    return @sorted;
-    is($req->uri()->as_string(), '/update', 'URI contains correct path');
+}
-    is($req->header('content-type'), 'application/json', 'Content-Type header is correct');
+
-    is($req->header('accept'), 'application/json', 'Accept header is correct');
+my @test_cases = (
-    my $got = decode_and_sort_array($req->content());
-    my $want = decode_and_sort_array({
-        'domain'     => 'my.example.com',
-        'apikey'     => 'mytestingpassword',
-        'update' => [
     {
-                'type' => 'A',
+        desc => 'both IPv4 and IPv6 are updated together',
-                'name' => 'host',
+        cfg => {
-                'content' => '192.0.2.1',
+            'host.my.example.com' => {
-                'ttl' => 5,
+                ttl => 5,
+                wantipv4 => '192.0.2.1',
+                wantipv6 => '2001:db8::1',
+                zone => 'my.example.com',
+            },
+        },
+        want => [{
+            apikey => 'key',
+            domain => 'my.example.com',
+            update => [
+                {
+                    content => '192.0.2.1',
+                    name => 'host',
+                    ttl => 5,
+                    type => 'A',
                 },
                 {
-                'type' => 'AAAA',
+                    content => '2001:db8::1',
-                'name' => 'host',
+                    name => 'host',
-                'content' => '2001:db8::1',
+                    ttl => 5,
-                'ttl' => 5,
+                    type => 'AAAA',
-            }
+                },
-        ]
+            ],
-    });
+        }],
-    is_deeply($got, $want, 'Data is correct');
+    },
-};
-
-subtest 'Testing nic_dnsexit2_update without a zone set' => sub {
-    httpd()->reset();
-    local %ddclient::config = (
-        'myhost.example.com' => {
-            'usev4'    => 'ipv4',
-            'wantipv4' => '192.0.2.1',
-            'protocol' => 'dnsexit2',
-            'password' => 'anotherpassword',
-            'server'   => httpd()->endpoint(),
-            'path'     => '/update-alt',
-            'ttl'      => 10
-    });
-    ddclient::nic_dnsexit2_update(undef, 'myhost.example.com');
-    my @requests = httpd()->reset();
-    is(scalar(@requests), 1, 'expected number of update requests');
-    my $req = shift(@requests);
-    my $got = decode_and_sort_array($req->content());
-    my $want = decode_and_sort_array({
-        'domain'     => 'myhost.example.com',
-        'apikey'     => 'anotherpassword',
-        'update' => [
     {
-                'type' => 'A',
+        desc => 'zone defaults to host',
-                'name' => '',
+        cfg => {
-                'content' => '192.0.2.1',
+            'host.my.example.com' => {
-                'ttl' => 10,
+                ttl => 10,
-            }
+                wantipv4 => '192.0.2.1',
-        ]
+            },
-    });
+        },
-    is_deeply($got, $want, 'Data is correct');
+        want => [{
-};
+            apikey => 'key',
-
+            domain => 'host.my.example.com',
-subtest 'Testing nic_dnsexit2_update with two hostnames, one with a zone and one without' => sub {
+            update => [
-    httpd()->reset();
+                {
-    local %ddclient::config = (
+                    content => '192.0.2.1',
+                    name => '',
+                    ttl => 10,
+                    type => 'A',
+                },
+            ],
+        }],
+    },
+    {
+        desc => 'two hosts, different zones',
+        cfg => {
             'host1.example.com' => {
-            'usev4'    => 'ipv4',
+                ttl => 5,
-            'wantipv4' => '192.0.2.1',
+                wantipv4 => '192.0.2.1',
-            'protocol' => 'dnsexit2',
+                # 'zone' intentionally not set, so it will default to 'host1.example.com'.
-            'password' => 'testingpassword',
-            'server'   => httpd()->endpoint(),
-            'path'     => '/update',
-            'ttl'      => 5
             },
             'host2.example.com' => {
-            'usev6'    => 'ipv6',
+                ttl => 10,
-            'wantipv6' => '2001:db8::1',
+                wantipv6 => '2001:db8::1',
-            'protocol' => 'dnsexit2',
+                zone => 'example.com',
-            'password' => 'testingpassword',
+            },
-            'server'   => httpd()->endpoint(),
+        },
-            'path'     => '/update',
+        want => [
-            'ttl'      => 10,
+            {
-            'zone'     => 'example.com'
+                apikey => 'key',
-        }
+                domain => 'host1.example.com',
+                update => [
+                    {
+                        content => '192.0.2.1',
+                        name => '',
+                        ttl => 5,
+                        type => 'A',
+                    },
+                ],
+            },
+            {
+                apikey => 'key',
+                domain => 'example.com',
+                update => [
+                    {
+                        content => '2001:db8::1',
+                        name => 'host2',
+                        ttl => 10,
+                        type => 'AAAA',
+                    },
+                ],
+            },
+        ],
+    },
+    {
+        desc => 'two hosts, same zone',
+        cfg => {
+            'host1.example.com' => {
+                ttl => 5,
+                wantipv4 => '192.0.2.1',
+                zone => 'example.com',
+            },
+            'host2.example.com' => {
+                ttl => 10,
+                wantipv6 => '2001:db8::1',
+                zone => 'example.com',
+            },
+        },
+        want => [
+            {
+                apikey => 'key',
+                domain => 'example.com',
+                update => [
+                    {
+                        content => '192.0.2.1',
+                        name => 'host1',
+                        ttl => 5,
+                        type => 'A',
+                    },
+                    {
+                        content => '2001:db8::1',
+                        name => 'host2',
+                        ttl => 10,
+                        type => 'AAAA',
+                    },
+                ],
+            },
+        ],
+    },
+    {
+        desc => 'host outside of zone',
+        cfg => {
+            'host.example' => {
+                wantipv4 => '192.0.2.1',
+                zone => 'example.com',
+            },
+        },
+        want_fatal => qr{hostname does not end with the zone: example.com},
+    },
 );
-    ddclient::nic_dnsexit2_update(undef, 'host1.example.com', 'host2.example.com');
+
-    my @requests = httpd()->reset();
+for my $tc (@test_cases) {
-    my @got = map(decode_and_sort_array($_->content()), @requests);
+    subtest($tc->{desc} => sub {
-    my @want = (
+        local $ddclient::_l = ddclient::pushlogctx($tc->{desc});
-        decode_and_sort_array({
+        local %ddclient::config = ();
-            'domain' => 'host1.example.com',
+        my @hosts = keys(%{$tc->{cfg}});
-            'apikey' => 'testingpassword',
+        for my $h (@hosts) {
-            'update' => [{
+            $ddclient::config{$h} = {
-                'type' => 'A',
+                password => 'key',
-                'name' => '',
+                path => '/update',
-                'content' => '192.0.2.1',
+                server => httpd()->endpoint(),
-                'ttl' => 5,
+                %{$tc->{cfg}{$h}},
-            }],
-        }),
-        decode_and_sort_array({
-            'domain' => 'example.com',
-            'apikey' => 'testingpassword',
-            'update' => [{
-                'type' => 'AAAA',
-                'name' => 'host2',
-                'content' => '2001:db8::1',
-                'ttl' => 10,
-            }],
-        }),
-    );
-    is_deeply(\@got, \@want, 'data is correct');
             };
+        }
+        my $l = ddclient::t::Logger->new($ddclient::_l, qr/^FATAL$/);
+        my $err = do {
+            local $ddclient::_l = $l;
+            local $@;
+            (eval { ddclient::nic_dnsexit2_update(undef, @hosts); 1; })
+                ? undef : ($@ // 'unknown error');
+        };
+        my @requests = httpd()->reset();
+        my @got;
+        for (my $i = 0; $i < @requests; $i++) {
+            subtest("request $i" => sub {
+                my $req = $requests[$i];
+                is($req->method(), 'POST', 'method is POST');
+                is($req->uri()->as_string(), '/update', 'path is /update');
+                is($req->header('content-type'), 'application/json', 'Content-Type is JSON');
+                is($req->header('accept'), 'application/json', 'Accept is JSON');
+                my $got = decode_json($req->content());
+                is(ref($got), 'HASH', 'request content is a JSON object');
+                is(ref($got->{update}), 'ARRAY', 'JSON object has array "update" property');
+                push(@got, $got);
+            });
+        }
+        @got = sort_reqs(@got);
+        my @want = sort_reqs(@{$tc->{want} // []});
+        is_deeply(\@got, \@want, 'request objects match');
+        subtest('expected (or lack of) error' => sub {
+            if (is(defined($err), defined($tc->{want_fatal}), 'error existence') && defined($err)) {
+                my @got = @{$l->{logs}};
+                if (is(scalar(@got), 2, 'logged two events')) {
+                    is($got[0]->{label}, 'FATAL', 'first logged event is a FATAL message');
+                    like($got[0]->{msg}, $tc->{want_fatal}, 'first logged event message matches');
+                    is($got[1], 'aborted', 'second logged event is an "aborted" event');
+                    isa_ok($err, qw(ddclient::t::LoggerAbort));
+                }
+            }
+        });
+    });
+}
 
 done_testing();

t/protocol_dyndns2.pl

@@ -3,6 +3,7 @@ BEGIN { SKIP: { eval { require Test::Warnings; 1; } or skip($@, 1); } }
 use MIME::Base64;
 BEGIN { eval { require 'ddclient'; } or BAIL_OUT($@); }
 use ddclient::t::HTTPD;
+use ddclient::t::Logger;
 
 httpd_required();
 
@@ -18,23 +19,6 @@ httpd()->run(sub {
     return undef;
 });
 
-{
-    package Logger;
-    use parent qw(-norequire ddclient::Logger);
-    sub new {
-        my ($class, $parent) = @_;
-        my $self = $class->SUPER::new(undef, $parent);
-        $self->{logs} = [];
-        return $self;
-    }
-    sub _log {
-        my ($self, $args) = @_;
-        push(@{$self->{logs}}, $args)
-            if ($args->{label} // '') =~ qr/^(?:WARNING|FATAL|SUCCESS|FAILED)$/;
-        return $self->SUPER::_log($args);
-    }
-}
-
 my @test_cases = (
     {
         desc => 'IPv4, single host, good',
@@ -246,7 +230,7 @@ for my $tc (@test_cases) {
     diag('==============================================================================');
     local $ddclient::globals{debug} = 1;
     local $ddclient::globals{verbose} = 1;
-    my $l = Logger->new($ddclient::_l);
+    my $l = ddclient::t::Logger->new($ddclient::_l, qr/^(?:WARNING|FATAL|SUCCESS|FAILED)$/);
     local %ddclient::config;
     local %ddclient::recap;
     $ddclient::config{$_} = {