Skip to content

Commit

Permalink
elastic-agent: don't swallow download errors (elastic#23235)
Browse files Browse the repository at this point in the history
* elastic-agent: don't swallow download errors

Stop swallowing the error from io.Copy when reading
from response bodies in the http downloader. This
prevents storing a partial download, which leads to
a permanent error state.

(cherry picked from commit 3cb1aa3)
  • Loading branch information
axw committed Dec 22, 2020
1 parent e1e0c16 commit 051ed1f
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 0 deletions.
1 change: 1 addition & 0 deletions x-pack/elastic-agent/CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
- Fix sysv init files for deb/rpm installation {pull}22543[22543]
- Fix shell wrapper for deb/rpm packaging {pull}23038[23038]
- Fixed parsing of npipe URI {pull}22978[22978]
- Remove artifacts on transient download errors {pull}23235[23235]

==== New features

Expand Down
4 changes: 4 additions & 0 deletions x-pack/elastic-agent/pkg/artifact/download/http/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,5 +151,9 @@ func (e *Downloader) downloadFile(ctx context.Context, artifactName, filename, f
}

_, err = io.Copy(destinationFile, resp.Body)
if err != nil {
return "", errors.New(err, "fetching package failed", errors.TypeNetwork, errors.M(errors.MetaKeyURI, sourceURI))
}

return fullPath, nil
}
55 changes: 55 additions & 0 deletions x-pack/elastic-agent/pkg/artifact/download/http/downloader_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package http

import (
"context"
"io/ioutil"
"net"
"net/http"
"net/http/httptest"
"os"
"testing"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/artifact"
)

func TestDownloadBodyError(t *testing.T) {
// This tests the scenario where the download encounters a network error
// part way through the download, while copying the response body.

type connKey struct{}
var srv *httptest.Server
srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.(http.Flusher).Flush()
conn := r.Context().Value(connKey{}).(net.Conn)
conn.Close()
}))
defer srv.Close()
client := srv.Client()
srv.Config.ConnContext = func(ctx context.Context, c net.Conn) context.Context {
return context.WithValue(ctx, connKey{}, c)
}

targetDir, err := ioutil.TempDir(os.TempDir(), "")
if err != nil {
t.Fatal(err)
}

config := &artifact.Config{
SourceURI: srv.URL,
TargetDirectory: targetDir,
OperatingSystem: "linux",
Architecture: "64",
}

testClient := NewDownloaderWithClient(config, *client)
artifactPath, err := testClient.Download(context.Background(), beatSpec, version)
if err == nil {
os.Remove(artifactPath)
t.Fatal("expected Download to return an error")
}
}

0 comments on commit 051ed1f

Please sign in to comment.