Fixes http.Verifier so it uses its http client (#4270) (#4271)

also add test with proxy for the Verifier.

(cherry picked from commit 1d63dde)

Co-authored-by: Anderson Queiroz <[email protected]>
Co-authored-by: Pierre HILBERT <[email protected]>

changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Fixes an issue where the Elastic Agent did not utilize the download settings when downloading the artifact signature file.
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; a word indicating the component this changeset affects.
+component:
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+issue: https://github.com/elastic/elastic-agent/issues/4237

internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go

@@ -171,8 +171,7 @@ func (v *Verifier) getPublicAsc(sourceURI string) ([]byte, error) {
 		return nil, errors.New(err, "failed create request for loading public key", errors.TypeNetwork, errors.M(errors.MetaKeyURI, sourceURI))
 	}
 
-	// TODO: receive a http.Client
-	resp, err := http.DefaultClient.Do(req)
+	resp, err := v.client.Do(req)
 	if err != nil {
 		return nil, errors.New(err, "failed loading public key", errors.TypeNetwork, errors.M(errors.MetaKeyURI, sourceURI))
 	}

internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go

@@ -9,6 +9,8 @@ import (
 	"fmt"
 	"math/rand"
 	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"testing"
 	"time"
@@ -19,42 +21,76 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/details"
 	"github.com/elastic/elastic-agent/pkg/core/logger"
+	"github.com/elastic/elastic-agent/testing/proxytest"
 )
 
 func TestVerify(t *testing.T) {
 	targetDir := t.TempDir()
 
 	log, _ := logger.New("", false)
 	timeout := 30 * time.Second
-	testCases := getRandomTestCases()
+	testCases := getRandomTestCases()[0:1]
 	server, pub := getElasticCoServer(t)
-	elasticClient := getElasticCoClient(server)
-	// artifact/download/http.Verifier uses http.DefaultClient, thus we need to
-	// change it.
-	http.DefaultClient = &elasticClient
 
 	config := &artifact.Config{
-		SourceURI:       source,
+		SourceURI:       server.URL + "/downloads",
 		TargetDirectory: targetDir,
 		HTTPTransportSettings: httpcommon.HTTPTransportSettings{
 			Timeout: timeout,
 		},
 	}
 
-	for _, testCase := range testCases {
-		testName := fmt.Sprintf("%s-binary-%s", testCase.system, testCase.arch)
+	t.Run("without proxy", func(t *testing.T) {
+		runTests(t, testCases, config, log, pub)
+	})
+
+	t.Run("with proxy", func(t *testing.T) {
+		brokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusTeapot)
+			t.Log("[brokenServer] wrong server, is the proxy working?")
+			_, _ = w.Write([]byte(`wrong server, is the proxy working?`))
+		}))
+		serverURL, err := url.Parse(server.URL)
+		require.NoError(t, err, "could not parse server URL \"%s\"",
+			server.URL)
+
+		proxy := proxytest.New(t,
+			proxytest.WithRewriteFn(func(u *url.URL) {
+				u.Host = serverURL.Host
+			}),
+			proxytest.WithRequestLog("proxy", func(_ string, _ ...any) {}))
+
+		proxyURL, err := url.Parse(proxy.LocalhostURL)
+		require.NoError(t, err, "could not parse server URL \"%s\"",
+			server.URL)
+
+		config := *config
+		config.SourceURI = brokenServer.URL + "/downloads"
+		config.Proxy = httpcommon.HTTPClientProxySettings{
+			URL: (*httpcommon.ProxyURI)(proxyURL),
+		}
+
+		runTests(t, testCases, &config, log, pub)
+	})
+}
+
+func runTests(t *testing.T, testCases []testCase, config *artifact.Config, log *logger.Logger, pub []byte) {
+	for _, tc := range testCases {
+		testName := fmt.Sprintf("%s-binary-%s", tc.system, tc.arch)
 		t.Run(testName, func(t *testing.T) {
-			config.OperatingSystem = testCase.system
-			config.Architecture = testCase.arch
+			config.OperatingSystem = tc.system
+			config.Architecture = tc.arch
 
-			upgradeDetails := details.NewDetails("8.12.0", details.StateRequested, "")
-			testClient := NewDownloaderWithClient(log, config, elasticClient, upgradeDetails)
-			artifact, err := testClient.Download(context.Background(), beatSpec, version)
-			if err != nil {
-				t.Fatal(err)
-			}
+			upgradeDetails := details.NewDetails(
+				"8.12.0", details.StateRequested, "")
+			downloader, err := NewDownloader(log, config, upgradeDetails)
+			require.NoError(t, err, "could not create new downloader")
+
+			pkgPath, err := downloader.Download(context.Background(), beatSpec, version)
+			require.NoErrorf(t, err, "failed downloading %s v%s",
+				beatSpec.Artifact, version)
 
-			_, err = os.Stat(artifact)
+			_, err = os.Stat(pkgPath)
 			if err != nil {
 				t.Fatal(err)
 			}