WIP: Make it possible to resume an IUK download from within Tails (refs: #15875)feature/15875-upgrader-resume-iuk-download

5 files changed, 238 insertions, 52 deletions

diff --git a/config/chroot_local-includes/usr/src/iuk/dist.ini b/config/chroot_local-includes/usr/src/iuk/dist.ini
index 0c86fcc..5fb2224 100644
--- a/config/chroot_local-includes/usr/src/iuk/dist.ini
+++ b/config/chroot_local-includes/usr/src/iuk/dist.ini
@@ -45,6 +45,7 @@ Sys::Filesystem = 1.28
 Test::BDD::Cucumber = 0.16
 Test::EOL = 0.9
 Test::Fatal = 0.010
+Test::LWP::UserAgent = 0.031
 Test::Most = 0.22
 Test::Perl::Critic = 1.02
 Test::Spec = 0.40
diff --git a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature
index 0aeb793..9c4722c 100644
--- a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature
+++ b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/Download_Target_File.feature
@@ -29,6 +29,19 @@ Feature: download and verify a target file
       | whatever1.file | /        | abc     |    3 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad |
       | whatever2.file | /sub/dir | 123     |    3 | a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3 |
 
+  Scenario: Successfully resuming an interrupted download
+    Given a HTTP server that supports Range requests and serves "<file>" in "<webdir>" with content "<content>" and hash "<sha256>"
+    When I download "<file>" (of expected size <size>) from "<webdir>", and check its hash is "<sha256>"
+    Then it should succeed
+    And I should be told "Resuming download after 1 bytes"
+    And I should see the downloaded file in the temporary directory
+    And the SHA-256 of the downloaded file should be "<sha256>"
+    And the downloaded file should be world-readable
+    Examples:
+      | file           | webdir   | content | size | sha256                                                           |
+      | whatever1.file | /        | abc     |    3 | ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad |
+      | whatever2.file | /sub/dir | 123     |    3 | a665a45920422f9d417e4867efdc4fb8a04a1f3fff1fa07e998e86f7f7a27ae3 |
+
   Scenario: Successful download and verification with redirect to another hostname over HTTPS
     Given a HTTP server that redirects to 127.0.0.2 over HTTPS
     And a HTTPS server on 127.0.0.2 that serves "<file>" in "<webdir>" with content "<content>" and hash "<sha256>"
diff --git a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl
index 147c8f5..2312a26 100644
--- a/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl
+++ b/config/chroot_local-includes/usr/src/iuk/features/download_target_file/step_definitions/Download_Target_File_steps.pl
@@ -12,6 +12,8 @@ use Data::Dumper;
 use English qw{-no_match_vars};
 use Fcntl ':mode';
 use Function::Parameters;
+use IPC::System::Simple qw{systemx};
+use LWP::UserAgent;
 use Test::More;
 use Test::BDD::Cucumber::StepFile;
 
@@ -22,6 +24,7 @@ use Test::WebServer::RedirectToHTTPS;
 use Test::WebServer::Static;
 use Test::WebServer::Static::SSL;
 use Types::Path::Tiny qw{AbsPath};
+use Types::Standard qw{Int};
 
 my $bindir = path(__FILE__)->parent->parent->parent->parent->child('bin')->absolute;
 
@@ -77,21 +80,77 @@ fun prepare_webroot (AbsPath $webroot, $filename, $present, $webdir, $spec_type,
     }
 }
 
-Given qr{^a HTTP server that(| does not) serve[s]? "([^"]+)" in "([^"]+)"(?: with (content|size) "?([^"]*)"?)?}, fun ($c) {
-    my $present    = $c->matches->[0] ? 0 : 1;
-    my $filename   = $c->matches->[1];
-    my $webdir     = $c->matches->[2];
-    my $spec_type  = $c->matches->[3];
-    my $spec_value = $c->matches->[4];
+fun generate_nginx_conf (
+    AbsPath :$pid_file,
+    AbsPath :$conf_file,
+    AbsPath :$webroot,
+    Int :$port
+  ) {
+    $conf_file->spew(
+        <<EOTEMPLATE
+pid $pid_file;
+error_log stderr;
+events { }
+http {
+    server {
+        listen $port;
+        root   $webroot;
+        access_log /dev/null;
+        autoindex on;
+    }
+}
+EOTEMPLATE
+    );
+}
+
+Given qr{^a HTTP server that( supports Range requests and)?(| does not) serve[s]? "([^"]+)" in "([^"]+)"(?: with (content|size) "?([^"]*)"?)?}, fun ($c) {
+    my $range_req  = $c->matches->[0];
+    my $present    = $c->matches->[1] ? 0 : 1;
+    my $filename   = $c->matches->[2];
+    my $webdir     = $c->matches->[3];
+    my $spec_type  = $c->matches->[4];
+    my $spec_value = $c->matches->[5];
 
     my $webroot = path($c->{stash}->{scenario}->{tempdir}, 'webroot');
     prepare_webroot($webroot, $filename, $present, $webdir, $spec_type, $spec_value);
-
     my $port  = $c->{stash}->{scenario}->{server}->{port};
-    my $s     = Test::WebServer::Static->new({webroot => $webroot}, $port);
-    is($s->port(), $port, "Constructor set port correctly");
-    my $pid   = $c->{stash}->{scenario}->{server}->{http_pid}  = $s->background();
-    like($pid, '/^-?\d+$/', 'PID is numeric');
+
+    if ($range_req) {
+        $c->{stash}->{scenario}->{test_resume} = 1;
+        my $nginx_unit = 'test-tails-iuk-nginx.service';
+        # Clean up leftovers from a previous test suite run that
+        # failed badly enough that the After hook did not trigger.
+        `systemctl --user stop '$nginx_unit' 2>&1`;
+        my $nginx_conf_file = Path::Tiny->tempfile;
+        my $nginx_pid_file = Path::Tiny->tempfile;
+        generate_nginx_conf(
+            conf_file => $nginx_conf_file,
+            pid_file  => $nginx_pid_file,
+            webroot   => $webroot,
+            port      => $port,
+        );
+        push @{$c->{stash}->{scenario}->{systemd_transient_units}}, $nginx_unit;
+        systemx(
+            'systemd-run',
+            '--user',
+            "--unit=${nginx_unit}",
+            '--service-type=forking',
+            '--quiet',
+            # XXX: enable this once we require Buster or newer for running
+            # this test suite
+            # '--collect',
+            '/usr/sbin/nginx', '-c', $nginx_conf_file,
+        );
+        for (1..16) {
+            last if LWP::UserAgent->new->get("http://127.0.0.1:$port/")->is_success;
+            sleep $_;
+        }
+    } else {
+        my $s     = Test::WebServer::Static->new({webroot => $webroot}, $port);
+        is($s->port(), $port, "Constructor set port correctly");
+        my $pid   = $c->{stash}->{scenario}->{server}->{http_pid}  = $s->background();
+        like($pid, '/^-?\d+$/', 'PID is numeric');
+    }
 };
 
 Given qr{^a HTTP server that redirects to ([^ ]+) over HTTPS$}, fun ($c) {
@@ -160,13 +219,10 @@ When qr{^I download "([^"]+)" \(of expected size (\d+)\) from "([^"]+)", and che
     my $webdir        = $c->matches->[2];
     my $expected_hash = $c->matches->[3];
 
-    my $uri = sprintf("%s:%d/%s/%s",
+    my $uri = sprintf("http://%s:%d%s",
                       "127.0.0.1",
                       $c->{stash}->{scenario}->{server}->{port},
-                      $webdir,
-                      $filename);
-    $uri =~ s{//}{/}gxms;
-    $uri = "http://$uri";
+                      path($webdir, $filename));
 
     my $output_filename
         = $c->{stash}->{scenario}->{output_filename}
@@ -180,6 +236,15 @@ When qr{^I download "([^"]+)" \(of expected size (\d+)\) from "([^"]+)", and che
         ' --output_file "' . $output_filename . '"' .
         ' --size "'        . $expected_size   . '"'
     ;
+
+    if ($c->{stash}->{scenario}->{test_resume}) {
+        my $dl_file = path($c->{stash}->{scenario}->{tempdir}, 'webroot', $webdir, $filename);
+        if ($dl_file->is_file) {
+            my ($first_byte) = unpack('H*', $dl_file->openr_raw->getc);
+            $cmdline .= " --first_byte $first_byte";
+        }
+    }
+
     $c->{stash}->{scenario}->{output} = `umask 077 && $cmdline 2>&1`;
     $c->{stash}->{scenario}->{exit_code} = ${^CHILD_ERROR_NATIVE};
 };
@@ -245,4 +310,9 @@ After fun ($c) {
     if (defined $c->{stash}->{scenario}->{server}->{http_pid}) {
         kill $c->{stash}->{scenario}->{server}->{http_pid};
     }
+    for (@{$c->{stash}->{scenario}->{systemd_transient_units}}) {
+        systemx('systemctl', '--user', 'stop', $_);
+    }
+    # XXX: remove once we use --collect for all our transient units
+    systemx('systemctl', '--user', 'reset-failed');
 };
diff --git a/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm b/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm
index a51a60d..fef6e1d 100644
--- a/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm
+++ b/config/chroot_local-includes/usr/src/iuk/lib/Tails/IUK/TargetFile/Download.pm
@@ -17,7 +17,6 @@ use Carp;
 use Carp::Assert;
 use Cwd;
 use Digest::SHA;
-use File::Temp qw{tempfile};
 use Function::Parameters;
 use HTTP::Request;
 use Path::Tiny;
@@ -25,7 +24,7 @@ use String::Errf qw{errf};
 use Tails::IUK::LWP::UserAgent::WithProgress;
 use Tails::IUK::Utils qw{space_available_in};
 use Types::Path::Tiny qw{AbsPath};
-use Types::Standard qw{Enum Int Str};
+use Types::Standard qw{Enum InstanceOf Int Object Str};
 
 use namespace::clean;
 
@@ -65,11 +64,68 @@ option 'size' => (
     format   => 's',
 );
 
+option 'max_retries' => (
+    is        => 'ro',
+    isa       => Int,
+    format    => 's',
+    default   => sub {
+        my $self = shift;
+        if ($ENV{HARNESS_ACTIVE}) {
+            $self->has_first_byte
+                ? 1  # in this case we are testing resuming a download
+                : 0 # in other cases, let's things speed up and abort early
+        } else {
+            6
+        }
+    },
+);
+
+has 'ua' => (
+    is  => 'lazy',
+    isa => InstanceOf['LWP::UserAgent'],
+);
+
+# Test suite only!
+# In unpack('H*', $data) format
+option 'first_byte' => (
+    is        => 'ro',
+    isa       => Str,
+    format    => 's',
+    predicate => 1,
+);
+
 
 =head1 METHODS
 
 =cut
 
+method _build_ua () {
+    my $ua;
+    if ($ENV{HARNESS_ACTIVE} && $self->has_first_byte) {
+        # When running under the test suite and --first_byte is
+        # passed, simulate a HTTP server that only sends the first
+        # byte of the requested file, for the first download attempt:
+        # this allows exercising this class' ability to resume an
+        # interrupted download.
+        require Test::LWP::UserAgent;
+        Test::LWP::UserAgent->import;
+        $ua = Test::LWP::UserAgent->new;
+        $self->patch_response_handler($ua);
+    } else {
+        $ua = Tails::IUK::LWP::UserAgent::WithProgress->new(ssl_opts => {
+            verify_hostname => 0,
+            SSL_verify_mode => 0,
+        });
+    }
+    unless ($ENV{HARNESS_ACTIVE} or $ENV{DISABLE_PROXY}) {
+        $ua->proxy([qw(http https)] => 'socks://127.0.0.1:9062');
+    }
+    $ua->protocols_allowed([qw(http https)]);
+    $ua->max_size($self->size);
+
+    return $ua;
+}
+
 method fatal (@msg) {
     Tails::IUK::Utils::fatal(msg => \@msg);
 }
@@ -89,21 +145,33 @@ method check_available_space () {
     ));
 }
 
+method patch_response_handler (Object $ua) {
+    $ua->map_response(
+        qr{},
+        sub {
+            # Disable this mapped response, so it does not trigger
+            # for subsequent download attempts, that will instead
+            # go directly to the test web server
+            $ua->unmap_all;
+            $ua->network_fallback(1);
+            # Return the mocked response
+            HTTP::Response->new(
+                200, 'OK',
+                HTTP::Headers->new(),
+                pack('H*', $self->first_byte)
+            );
+        }
+    );
+}
+
 method run () {
     $self->check_available_space;
 
-    my $ua = Tails::IUK::LWP::UserAgent::WithProgress->new(ssl_opts => {
-        verify_hostname => 0,
-        SSL_verify_mode => 0,
-    });
-    unless ($ENV{HARNESS_ACTIVE} or $ENV{DISABLE_PROXY}) {
-        $ua->proxy([qw(http https)] => 'socks://127.0.0.1:9062');
-    }
-    $ua->protocols_allowed([qw(http https)]);
     my $req = HTTP::Request->new('GET', $self->uri);
 
-    my ($temp_fh, $temp_filename) = tempfile;
-    close $temp_fh;
+    my $temp_file = Path::Tiny->tempfile;
+    my $temp_fh = $temp_file->opena_raw;
+    $temp_fh->autoflush(1);
 
     sub clean_fatal {
         my $self   = shift;
@@ -112,38 +180,71 @@ method run () {
         $self->fatal(@_);
     }
 
-    $ua->max_size($self->size);
-    my $res = $ua->request($req, $temp_filename);
+    my $retries = 0;
+    my $sleep = 1;
+    my $success;
+    while ($retries <= $self->max_retries) {
+        if ($retries) {
+            sleep $sleep;
+            $sleep *= 2;
+            my $range_start = -s $temp_fh;
+            say STDERR "Resuming download after $range_start bytes (retry no. $retries)";
+            $req->header(Range => "bytes=${range_start}-");
+        }
 
-    defined $res or clean_fatal($self, $temp_filename, sprintf(
-        "Could not download '%s' to '%s': undefined result",
-        $self->uri, $temp_filename,
-    ));
+        $retries++;
+        my $res = $self->ua->request($req, sub { print $temp_fh shift; });
 
-    for my $lwp_failure_header (qw{Client-Aborted X-Died}) {
-        my $header = $res->header($lwp_failure_header);
-        ! defined $header or clean_fatal($self, $temp_filename, sprintf(
-            "Could not download '%s' to '%s' (%s): %s",
-            $self->uri, $temp_filename, $lwp_failure_header, $header,
-        ));
-    }
+        unless (defined $res) {
+            warn(sprintf(
+                "Could not download '%s' to '%s': undefined result",
+                $self->uri, $temp_file,
+            ));
+            next;
+        }
 
-    $res->is_success or clean_fatal($self, $temp_filename, sprintf(
-        "Could not download '%s' to '%s', request failed:\n%s\n",
-        $self->uri, $temp_filename, $res->status_line,
-    ));
+        for my $lwp_failure_header (qw{Client-Aborted X-Died}) {
+            my $header = $res->header($lwp_failure_header);
+            if (defined $header) {
+                warn(sprintf(
+                    "Could not download '%s' to '%s' (%s): %s",
+                    $self->uri, $temp_file, $lwp_failure_header, $header,
+                ));
+                next;
+            }
+        }
+
+        unless ($res->is_success) {
+            warn(sprintf(
+                "Could not download '%s' to '%s', request failed:\n%s\n",
+                $self->uri, $temp_file, $res->status_line,
+            ));
+            next;
+        }
+
+        if (-s $temp_file != $self->size) {
+            warn(sprintf(
+                "The file '%s' was downloaded but its size (%d) should be %d",
+                $self->uri, -s $temp_file, $self->size,
+            ));
+            next;
+        }
+
+        $temp_fh->close;
+        $success = 1;
+        last;
+    }
 
-    -s $temp_filename == $self->size or clean_fatal(
-        $self, $temp_filename, sprintf(
-            "The file '%s' was downloaded but its size (%d) should be %d",
-            $self->uri, -s $temp_filename, $self->size,
+    $success or clean_fatal($self, $temp_file, sprintf(
+        "Could not download '%s' to '%s'",
+        $self->uri, $temp_file,
     ));
 
     my $sha = Digest::SHA->new(256);
-    $sha->addfile($temp_filename);
+    $sha->addfile($temp_file->stringify);
     my $actual_hash = $sha->hexdigest;
     $actual_hash eq $self->hash_value or clean_fatal(
-        $self, $temp_filename, sprintf(
+        $self, $temp_file, sprintf(
             "The file '%s' was downloaded but its hash is not correct:\n"
                 . "  - expected: %s\n"
                 . "  - actual:   %s",
@@ -152,7 +253,7 @@ method run () {
             $actual_hash,
     ));
 
-    rename($temp_filename, $self->output_file);
+    $temp_file->move($self->output_file);
     # autodie is supposed to throw an exception on rename error,
     # but one can't be too careful.
     assert(-e $self->output_file);
diff --git a/wiki/src/contribute/release_process/tails-iuk.mdwn b/wiki/src/contribute/release_process/tails-iuk.mdwn
index c27ddd9..a1c8479 100644
--- a/wiki/src/contribute/release_process/tails-iuk.mdwn
+++ b/wiki/src/contribute/release_process/tails-iuk.mdwn
@@ -37,6 +37,7 @@ Install dependencies needed to generate IUKs, UDFs, and to run the test suite
        libdata-dumper-concise-perl \
        libdatetime-perl \
        libfile-copy-recursive-perl \
+       nginx \
        rsync \
        squashfs-tools \
        syslinux