From ef967881bea8899cff4711ee62d0f599ab26f450 Mon Sep 17 00:00:00 2001 From: Anderson Queiroz Date: Thu, 15 Feb 2024 14:20:49 +0100 Subject: [PATCH 1/2] http.Verifier uses its http client also add test with proxy for the downloader and verifier. --- ...nloading-the-artifact-signature-file..yaml | 32 +++++++++ .../artifact/download/http/verifier.go | 3 +- .../artifact/download/http/verifier_test.go | 70 ++++++++++++++----- 3 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml diff --git a/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml b/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml new file mode 100644 index 00000000000..a5d75e196bf --- /dev/null +++ b/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: feature + +# 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 diff --git a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go index 587b968ff66..1fcbdca9f94 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go +++ b/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)) } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go index 3d5c74a9a8a..7e2b888debc 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier_test.go +++ b/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,6 +21,7 @@ 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) { @@ -26,35 +29,68 @@ func TestVerify(t *testing.T) { 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) } From 7c54529941437282bbf4d323ab19dc4beeb217ce Mon Sep 17 00:00:00 2001 From: Anderson Queiroz Date: Thu, 15 Feb 2024 14:54:52 +0100 Subject: [PATCH 2/2] Update changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml Co-authored-by: Blake Rouse --- ...-settings-when-downloading-the-artifact-signature-file..yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml b/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml index a5d75e196bf..ad4de0ab76b 100644 --- a/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml +++ b/changelog/fragments/1707999059-Fixes-an-issue-where-the-Elastic-Agent-did-not-utilize-the-download-settings-when-downloading-the-artifact-signature-file..yaml @@ -8,7 +8,7 @@ # - 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: feature +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.