Use only fs.NewDownloader for file://. (#3682)

* Use only fs.NewDownloader for file://.

* Add changelog entry.

* Don't retry download with its file://.

changelog/fragments/1698851635-Only-try-to-download-upgrade-locally-when-file---prefix-is-used-for-source-URI.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: Only try to download upgrade locally when file:// prefix is used for source URI
+
+# 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; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+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/elastic/elastic-agent/pull/3682
+
+# 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/owner/repo/1234

internal/pkg/agent/application/upgrade/step_download.go

@@ -36,6 +36,10 @@ const (
 	fleetUpgradeFallbackPGPFormat = "/api/agents/upgrades/%d.%d.%d/pgp-public-key"
 )
 
+type downloaderFactory func(*agtversion.ParsedSemVer, *logger.Logger, *artifact.Config, *details.Details) (download.Downloader, error)
+
+type downloader func(context.Context, downloaderFactory, *agtversion.ParsedSemVer, *artifact.Config, *details.Details) (string, error)
+
 func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI string, upgradeDetails *details.Details, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ string, err error) {
 	span, ctx := apm.StartSpan(ctx, "downloadArtifact", "app.internal")
 	defer func() {
@@ -45,32 +49,63 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri
 
 	pgpBytes = u.appendFallbackPGP(version, pgpBytes)
 
+	parsedVersion, err := agtversion.ParseVersion(version)
+	if err != nil {
+		return "", fmt.Errorf("error parsing version %q: %w", version, err)
+	}
+
 	// do not update source config
 	settings := *u.settings
+	var downloaderFunc downloader
+	var factory downloaderFactory
+	var verifier download.Verifier
 	if sourceURI != "" {
 		if strings.HasPrefix(sourceURI, "file://") {
 			// update the DropPath so the fs.Downloader can download from this
 			// path instead of looking into the installed downloads directory
 			settings.DropPath = strings.TrimPrefix(sourceURI, "file://")
+
+			// use specific function that doesn't perform retries on download as its
+			// local and no retry should be performed
+			downloaderFunc = u.downloadOnce
+
+			// set specific downloader, local file just uses the fs.NewDownloader
+			// no fallback is allowed because it was requested that this specific source be used
+			factory = func(ver *agtversion.ParsedSemVer, l *logger.Logger, config *artifact.Config, d *details.Details) (download.Downloader, error) {
+				return fs.NewDownloader(config), nil
+			}
+
+			// set specific verifier, local file verifies locally only
+			verifier, err = fs.NewVerifier(u.log, &settings, release.PGP())
+			if err != nil {
+				return "", errors.New(err, "initiating verifier")
+			}
+
+			// log that a local upgrade artifact is being used
+			u.log.Infow("Using local upgrade artifact", "version", version,
+				"drop_path", settings.DropPath,
+				"target_path", settings.TargetDirectory, "install_path", settings.InstallPath)
 		} else {
 			settings.SourceURI = sourceURI
 		}
 	}
 
-	u.log.Infow("Downloading upgrade artifact", "version", version,
-		"source_uri", settings.SourceURI, "drop_path", settings.DropPath,
-		"target_path", settings.TargetDirectory, "install_path", settings.InstallPath)
-
-	parsedVersion, err := agtversion.ParseVersion(version)
-	if err != nil {
-		return "", fmt.Errorf("error parsing version %q: %w", version, err)
+	if factory == nil {
+		// set the factory to the newDownloader factory
+		factory = newDownloader
+		u.log.Infow("Downloading upgrade artifact", "version", version,
+			"source_uri", settings.SourceURI, "drop_path", settings.DropPath,
+			"target_path", settings.TargetDirectory, "install_path", settings.InstallPath)
+	}
+	if downloaderFunc == nil {
+		downloaderFunc = u.downloadWithRetries
 	}
 
 	if err := os.MkdirAll(paths.Downloads(), 0750); err != nil {
 		return "", errors.New(err, fmt.Sprintf("failed to create download directory at %s", paths.Downloads()))
 	}
 
-	path, err := u.downloadWithRetries(ctx, newDownloader, parsedVersion, &settings, upgradeDetails)
+	path, err := downloaderFunc(ctx, factory, parsedVersion, &settings, upgradeDetails)
 	if err != nil {
 		return "", errors.New(err, "failed download of agent binary")
 	}
@@ -79,9 +114,11 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri
 		return path, nil
 	}
 
-	verifier, err := newVerifier(parsedVersion, u.log, &settings)
-	if err != nil {
-		return "", errors.New(err, "initiating verifier")
+	if verifier == nil {
+		verifier, err = newVerifier(parsedVersion, u.log, &settings)
+		if err != nil {
+			return "", errors.New(err, "initiating verifier")
+		}
 	}
 
 	if err := verifier.Verify(agentArtifact, parsedVersion.VersionWithPrerelease(), skipDefaultPgp, pgpBytes...); err != nil {
@@ -168,9 +205,32 @@ func newVerifier(version *agtversion.ParsedSemVer, log *logger.Logger, settings
 	return composed.NewVerifier(log, fsVerifier, snapshotVerifier, remoteVerifier), nil
 }
 
+func (u *Upgrader) downloadOnce(
+	ctx context.Context,
+	factory downloaderFactory,
+	version *agtversion.ParsedSemVer,
+	settings *artifact.Config,
+	upgradeDetails *details.Details,
+) (string, error) {
+	downloader, err := factory(version, u.log, settings, upgradeDetails)
+	if err != nil {
+		return "", fmt.Errorf("unable to create fetcher: %w", err)
+	}
+	// All download artifacts expect a name that includes <major>.<minor.<patch>[-SNAPSHOT] so we have to
+	// make sure not to include build metadata we might have in the parsed version (for snapshots we already
+	// used that to configure the URL we download the files from)
+	path, err := downloader.Download(ctx, agentArtifact, version.VersionWithPrerelease())
+	if err != nil {
+		return "", fmt.Errorf("unable to download package: %w", err)
+	}
+
+	// Download successful
+	return path, nil
+}
+
 func (u *Upgrader) downloadWithRetries(
 	ctx context.Context,
-	downloaderCtor func(*agtversion.ParsedSemVer, *logger.Logger, *artifact.Config, *details.Details) (download.Downloader, error),
+	factory downloaderFactory,
 	version *agtversion.ParsedSemVer,
 	settings *artifact.Config,
 	upgradeDetails *details.Details,
@@ -188,20 +248,11 @@ func (u *Upgrader) downloadWithRetries(
 	opFn := func() error {
 		attempt++
 		u.log.Infof("download attempt %d", attempt)
-
-		downloader, err := downloaderCtor(version, u.log, settings, upgradeDetails)
-		if err != nil {
-			return fmt.Errorf("unable to create fetcher: %w", err)
-		}
-		// All download artifacts expect a name that includes <major>.<minor.<patch>[-SNAPSHOT] so we have to
-		// make sure not to include build metadata we might have in the parsed version (for snapshots we already
-		// used that to configure the URL we download the files from)
-		path, err = downloader.Download(cancelCtx, agentArtifact, version.VersionWithPrerelease())
+		var err error
+		path, err = u.downloadOnce(cancelCtx, factory, version, settings, upgradeDetails)
 		if err != nil {
-			return fmt.Errorf("unable to download package: %w", err)
+			return err
 		}
-
-		// Download successful
 		return nil
 	}